Taiko barlines with SV changes are broken/inconsistent

[XeZey]

Type

Game behaviour

Bug description

Usually, upon a SV change on a bar line, that bar line should/will stay in speed with the note. In lazer, this doesn't always happen, and the bar lines will be treated as if the SV change happened "after" the beat. this doesn't make sense as the SV change does affect the note which is placed exactly upon the beat. Using "clone to current time" and inputting the exact time in ms manually can also give different results, which probably shouldn't be the case. Placing SVs before the beat is a way to avoid these issues, but it's definitely not optimal. The barlines will appear in gameplay exactly as in the editor.

Screenshots or videos

https://github.com/ppy/osu/assets/83080509/f13d70a3-d8ee-479d-bb1d-2750d55cdb4a

Version

2024.521.2

Logs

compressed-logs.zip

[bdach]

Please link the beatmap that you were testing this on. I'm reluctant to even begin investigating that before that because I suspect this will be floating-point related and thus the specific arrangement of timing points may make a difference.

[XeZey]

The map I tested this on was my own in lazer, so I cant link it, but this can easily be observed in other maps. https://osu.ppy.sh/beatmapsets/1754329#taiko/3590121 at 1:28:294 ,if you delete the sv change and remake it with "clone to current time", and then update the position by redragging the circle, it will show that weird behaviour. here you can also see that the beat snapping indicates that the circle shouldnt be affected by the sv change but the circle is regardless (which would be intended but the beat snapping here is displaying incorrect)

This bug doesn't exist for maps that were made in stable and are then played in lazer, so the conversion works flawlessly (which is every map currently online obviously), but if you update the hit circles position by dragging the notes around and try to test these sv changes in actual lazer, thats where these issues will appear sometimes.

also possibly important stuff to mention I think:

here the circle isnt affected by the sv change but the bar line actually is. (the greyed out white line after the circle is the bar line that is adjusted to the sv change) this is fixed after remaking the sv change tho and appears to be a result of resnapping all notes at once. (there doesnt appear to be a way to fix this other than by deleting and creating the sv change again). sometimes the beat snaps that appear in the editor are also still messed up despite the sv affecting the note and the bar line correctly (there shouldnt be a white line before the blue circle, as that is the starting bar line after the sv change, which does also appear at the same time)

These are just extra examples to show that this issue can spread out to other parts as well, and these are all probably connected to the same fundamental issue

[bdach]

Well as expected this is yet another episode of what I at this point can't call anything other than floating point bullshit.

The original map has the slider velocity change at

88294,-90.9090909090909,4,1,0,80,0,0

When it is remade, the new time is determined by the implementation of beat snapping in EditorBeatmap / ControlPointInfo.GetClosestSnappedTime():

https://github.com/ppy/osu/blob/7b14c77e43e4ee96775a9fcb6843324170fa70bb/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs#L189

which yields

88294.72727272726,-90.9090909090909,4,1,0,80,0,0

The object at time 88294 is also at 88294.72727272726 when it is moved around, but the bar line is at 88294.72727272725 which is slightly earlier because bar lines are calculated via a completely different pathway:

https://github.com/ppy/osu/blob/7b14c77e43e4ee96775a9fcb6843324170fa70bb/osu.Game/Rulesets/Objects/BarLineGenerator.cs#L73-L90

Notably this second pathway uses iterative addition on floats rather than incrementing a counter and then multiplying by the floats, which means that the subsequent precision loss / catastrophic cancellation may yield perturbations in the values produced. That said, while multiplying is more correct, it is not what stable does, which means that things can diverge elsewhere.

Which is to say that something like the following patch:

diff --git a/osu.Game/Rulesets/Objects/BarLineGenerator.cs b/osu.Game/Rulesets/Objects/BarLineGenerator.cs
index affbcbd878..8c244e9b9c 100644
--- a/osu.Game/Rulesets/Objects/BarLineGenerator.cs
+++ b/osu.Game/Rulesets/Objects/BarLineGenerator.cs
@@ -70,7 +70,9 @@ public BarLineGenerator(IBeatmap beatmap)
                     startTime += barLength;
                 }

-                for (double t = startTime; Precision.AlmostBigger(endTime, t); t += barLength, currentBeat++)
+                double t;
+
+                for (t = startTime; Precision.AlmostBigger(endTime, t); currentBeat += 1, t = startTime + currentBeat * barLength)
                 {
                     double roundedTime = Math.Round(t, MidpointRounding.AwayFromZero);

diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs
index 766d5b5601..9b2b958ea1 100644
--- a/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs
+++ b/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs
@@ -140,7 +140,7 @@ private void createLines()
                 }

                 beat++;
-                time += timingPoint.BeatLength / beatDivisor.Value;
+                time = timingPoint.Time + beat * (timingPoint.BeatLength / beatDivisor.Value);
             }

             foreach (var grid in grids)

will fix this case but may break five other things somewhere else and I'm not even entirely sure how to find what would break.

@ppy/team-client any bright ideas on how to proceed?