Difference between sliders in lazer and stable

[apollo-dw]

Type

Game behaviour

Bug description

Slider generation is leading to two very severely different results between stable and lazer. Example sliders in the map: 00:21:142 (1,2,3,4) - 00:06:142 (1,2,3,4) -

Really wacky slider node arrangement is likely the cause, 😕

Map link: https://osu.ppy.sh/beatmapsets/1765052#osu/3612917 Alternative .osz download: https://cdn.discordapp.com/attachments/786626810086686740/975950870258802778/Sporty-O_-_Let_Me_Hit_It.osz

Screenshots or videos

Version

2022.515.0-lazer

Logs

n/a

[peppy]

Is this present in any ranked beatmap? If not we may look to fix it in stable.

[smoogipoo]

Please let me investigate first before contributing to this discussion further. I know the cause and am doing a bunch of tests myself.

[smoogipoo]

This is caused by Catmull sliders. In stable, a Catmull slider with multiple "red" control points in it is treated as one segment. In lazer, these are treated as separate Catmull segments. This is basically a special case just for catmull sliders because all other types have multiple segments or don't support multiple segments at all and transform into bezier (e.g. perfect-curve).

The differences can be summarised as:	beatmap	stable	lazer
/b/22316
/b/42223

Based on diffcalc alone, here's a selection of the top SR differences in ranked beatmaps:

+------------+--------------+--------------+
| beatmap_id | diff_unified | diff_unified |
+------------+--------------+--------------+
|      42223 |      2.44393 |      2.42827 |
|      42223 |       2.3387 |      2.32657 |
|      26788 |      3.92047 |      3.93245 |
|      26788 |      5.22002 |      5.23179 |
|      42223 |      2.59259 |      2.58095 |
|      26788 |      3.25435 |      3.26595 |
|      42223 |      2.63576 |      2.62499 |
|      26788 |       3.8481 |      3.85839 |
|      42223 |      1.82955 |      1.81944 |
|      26788 |      3.19094 |      3.20093 |
|      26788 |      5.13611 |      5.14575 |
|      42223 |      2.46671 |      2.45768 |
|    2285424 |      13.4025 |      13.3935 |
|    2285424 |       12.982 |      12.9732 |
|    2285424 |      11.0776 |      11.0688 |
|    2285424 |      11.5443 |      11.5356 |
...
|    3358506 |      7.92029 |      7.92029 |

tl;dr: Seems to only be 2 beatmaps that have been ranked recently which experience differences. The older maps don't seem to be affected quite as much visually, however the differences on the recently-ranked ones are somewhat more impactful since they lead to partially-offscreen hitobjects or incorrect slider lengths:	beatmap	stable	lazer
/b/2285424
/b/3358506

From the above two, only the first (/b/2285424) is impacted in a more meaningful way. The second only has 3 such sliders which break in the same (minor) fashion.

Here's a quick-and-dirty hack demonstrating the changes required to fix this in lazer:

diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs
index 2a7f2b037f..abdf1c1e8d 100644
--- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs
+++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs
@@ -337,7 +337,7 @@ private IEnumerable<Memory<PathControlPoint>> convertPoints(ReadOnlyMemory<strin
             while (++endIndex < vertices.Length - endPointLength)
             {
                 // Keep incrementing while an implicit segment doesn't need to be started
-                if (vertices[endIndex].Position != vertices[endIndex - 1].Position)
+                if ((endIndex > 1 && type == PathType.Catmull) || vertices[endIndex].Position != vertices[endIndex - 1].Position)
                     continue;

                 // The last control point of each segment is not allowed to start a new implicit segment.
diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs
index e0c62fe359..b0178f959f 100644
--- a/osu.Game/Rulesets/Objects/SliderPath.cs
+++ b/osu.Game/Rulesets/Objects/SliderPath.cs
@@ -221,7 +221,7 @@ private void calculatePath()

             for (int i = 0; i < ControlPoints.Count; i++)
             {
-                if (ControlPoints[i].Type == null && i < ControlPoints.Count - 1)
+                if ((ControlPoints[i].Type == null || ControlPoints[i].Type == PathType.Catmull) && i < ControlPoints.Count - 1)
                     continue;

                 // The current vertex ends the segment

It's possible that the change to SliderPath doesn't need to exist, however I'm not sure on this change in general as it would mean truly separate Catmull segments wouldn't be supported in lazer.

Curious on others' thoughts.

[peppy]

I'd be okay if it's only in the converter.

[smoogipoo]

@peppy That's only half of it. The question would be what to do for the editor, and what about beatmaps created via lazer.

Essentially, "red" control points wouldn't exist for Catmull sliders.

So, do we keep showing "red" control points in the editor but silently disable them for Catmull, or add in special logic? Do we switch on the beatmap version in parsing and allow lazer to use segmented Catmull sliders (current logic), or treat the legacy behaviour as the permanent behaviour and work towards more properly supporting that in the editor.

[peppy]

There's already other breaking changes in the lazer editor regarding slider curves. I'd leave it as-is (ie. not apply the legacy behaviour to vLazer beatmaps). If we add support for mutli-type sliders back to stable this would also be supported in the process.

ie. in the converter, if the converter sees that the beatmap is created in lazer it should use the previous line, else use the change you've proposed.