ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.32k stars 2.28k forks source link

Slider length calculation does not match Stable for some edge cases #24798

Closed Magnus-Cosmos closed 1 year ago

Magnus-Cosmos commented 1 year ago

Copied over from https://github.com/ppy/osu/issues/24708#issuecomment-1717192869


List of approved maps with incorrect max_combo, majority can be ignored as they abuse things like infinite bpm/negative slider lengths/etc.

24708 covers beatmaps 1855 and 4182386. But for 42196, the slider length is off by a huge amount. This is because the length specified in the .osu file is different than the length calculated using the path points. For Stable, the calculated length is 222.004464186728 while the length in the beatmap file is 330. Stable attempts to resolve this by removing the "excess", which happens to add the missing amount back when the "excess" is negative. After doing this, the calculated length becomes 330.00000862032175.

For Lazer, the recalculation for the last path is skipped when the last two control points are equal and when the expected length is greater than the calculated length. Relevant Lazer code: https://github.com/ppy/osu/blob/f90f2491c349c58f5a038ab83e9934ca4b483787/osu.Game/Rulesets/Objects/SliderPath.cs#L331-L336 Meanwhile in Stable, the only scenario where recalculation is skipped is when the last two control points are equal and when the length of the final path is greater than the "excess" (calculated length - expected length). Applying this logic to Lazer results in the same slider length, although I'm unsure if this will have any unintended consequences (also not sure if the 0.0001 is needed):

diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs
index 34113285a4..b4507f8d6f 100644
--- a/osu.Game/Rulesets/Objects/SliderPath.cs
+++ b/osu.Game/Rulesets/Objects/SliderPath.cs
@@ -328,16 +328,16 @@ private void calculateLength()

             if (ExpectedDistance.Value is double expectedDistance && calculatedLength != expectedDistance)
             {
+                int pathEndIndex = calculatedPath.Count - 1;
+
                 // 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)
+                if (ControlPoints.Count >= 2 && ControlPoints[^1].Position == ControlPoints[^2].Position && calculatedPath[pathEndIndex].Length > calculatedLength - expectedDistance + 0.0001)
                 {
                     cumulativeLength.Add(calculatedLength);
                     return;
                 }

                 // The last length is always incorrect
                 cumulativeLength.RemoveAt(cumulativeLength.Count - 1);

-                int pathEndIndex = calculatedPath.Count - 1;
-
                 if (calculatedLength > expectedDistance)

Stable

screenshot4476

Lazer

osu_2023-09-13_03-06-20

Lazer with diff applied

osu_2023-09-13_03-33-00

peppy commented 1 year ago

Adding to project tentatively.