Use correct check for slider path extension

[Magnus-Cosmos]

Currently, Lazer uses the last two control points of a slider to check if it needs an extension:

osu/osu.Game/Rulesets/Objects/SliderPath.cs

Lines 331 to 336 in 163b6df

	// In osu-stable, if the last two control points of a slider are equal, extension is not performed.
	if (ControlPoints.Count >= 2 && ControlPoints[^1].Position == ControlPoints[^2].Position && expectedDistance > calculatedLength)
	{
	cumulativeLength.Add(calculatedLength);
	return;
	}

However Stable uses the points of the last calculated path. In most cases this makes no difference, but for 42196, this discrepancy causes the sliders at 46060ms and 123348ms to skip path extension in Lazer, making the sliders shorter compared to Stable.

Closes #24798.

---

While fixing this, I noticed that there are also some other slider length discrepancies:

• In the test case provided by this PR, the sliders have 64 calculated paths in Stable but 32 in Lazer. For some reason Lazer converts the first segment of the slider into a Linear path, but uses Bezier for the second segment.
• Lazer calculates slider curves using points relative to the start position. This causes the floating point precision loss to be different during calculations compared to Stable, leading to slightly different path lengths.

I believe these issues can be ignored or addressed later, as I have yet to encounter any edge cases where these make a noticeable difference.

[Magnus-Cosmos]

Currently failing this edge case due to a discrepancy between Stable and Lazer path calculation:

osu/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs

Line 870 in 163b6df

public void TestSliderLengthExtensionEdgeCase()

Slider used in the test, from #15303:
261,171,25305,6,0,B|262:171|262:171|262:171,1,2,8|0,0:0|0:0,0:0:0:0:

Relevant methods:

osu/osu.Game/Rulesets/Objects/SliderPath.cs

Line 242 in 163b6df

private void calculatePath()

osu/osu.Game/Rulesets/Objects/SliderPath.cs

Line 282 in 163b6df

private List<Vector2> calculateSubPath(ReadOnlySpan<Vector2> subControlPoints, PathType type)

Results for the methods:

subControlPoints	type	output
[{(0,0)}]	Bezier	[{(0,0)}, {(0,0)}]
[{(0,0)}, {(1,0)}]	Bezier	[{(0,0)}, {(1,0)}]
[{(1,0)}, {(1,0)}]	Bezier	[{(1,0)}, {(1,0)}]

calculatePath()	expected
[{(0,0)}, {(1,0)}]	[{(0,0)}, {(1,0)}, {(1,0)}]

In calculatePath(), when adding the calculated sub paths to calculatedPath, for every path of the sub paths, it checks if the sub path is equal to the last path added. The purpose of this is to get rid of the points shared between each path.

osu/osu.Game/Rulesets/Objects/SliderPath.cs

Line 267 in 163b6df

if (calculatedPath.Count == 0 || calculatedPath.Last() != t)

This works for most scenarios, since having 3 consecutive curve points with the same position shouldn't happen normally. However, when it does happen, Lazer ends up skipping the 3rd sub path completely, resulting in 2 path points instead of 3 like in Stable.

There's 2 simple ways to fix this, either exclude the starting point of each sub path in calculateSubPath()'s output, or exclude it in calculatePath() when merging the sub paths. I've opted to do the latter since it's simpler.

[bdach]

Doing a SR sheet for this overnight as a preliminary step. Haven't reviewed the logic closely yet, just want to see how much changes.

[bdach]

Seems like diffcalc... doesn't care again? https://docs.google.com/spreadsheets/d/15Vgipe7vv7sK-4SUDwL1kj0WFE6g1v2VsPaXlIcqEoM/edit#gid=11450370

I'll have to do a manual check against stable source then...

[bdach]

Ok, upon comparison with stable source I believe this is correct, although I was almost convinced it wasn't for a while...

Relevant snippet:

                while (path.Count > 0)
                {
                    Line lastLine = path[path.Count - 1];
                    float lastLineLength = Vector2.Distance(lastLine.p1, lastLine.p2);

                    if (lastLineLength > excess + MIN_SEGMENT_LENGTH)
                    {
                        if (lastLine.p2 != lastLine.p1)
                        {
                            lastLine.p2 = lastLine.p1 +
                                      Vector2.Normalize(lastLine.p2 - lastLine.p1) * (lastLine.rho - (float)excess);
                        }
                        break;
                    }

                    path.Remove(lastLine);
                    excess -= lastLineLength;
                }

(source: https://github.com/peppy/osu-stable-reference/blob/1531237b63392e82c003c712faa028406073aa8f/osu!/GameplayElements/HitObjects/Osu/SliderOsu.cs#L712-L729).

And yes, path is indeed the path post-approximation rather than the list of path control points.

@Magnus-Cosmos Since you appear to somehow have access to stable source (judging by the debugger screenshots from #24708), it would be appreciated if you provided specific information as to which parts of stable specifically these types of changes are based on. It would make review much faster in the future if you should wish to continue submitting such PRs.

[bdach]

This new special case appears to be a mostly unrelated optimisation, but I can't see it being wrong and sheets are silent, so I guess I'll let it be...

[bdach]

Did some further checking against maps mentioned in #15109, confirmed that this change doesn't regress either case mentioned there.

I can't find any more fault with this change so I'm just gonna take a somewhat calculated risk on it and hope for the best...