Intended as at least a partial solution to
https://github.com/ppy/osu/issues/26436. In testing, two of the three
replays from the issue are FCs with this change, and the third is
improved (and not fully fixed).
As it turns out, stable truncates the end time of sliders to integers:
79addff0f5/osu!/GameplayElements/HitObjects/Osu/SliderOsu.cs#L1037
and the start time of all juice stream parts is also truncated to
integers:
79addff0f5/osu!/GameplayElements/HitObjects/Fruits/SliderFruits.cs#L86-L166
This matters for replay playback when mappers push limits. For instance,
let's take one case from the issue. In one of the maps involved, there
was a juice-stream-ending fruit at time 7093.9655 according to lazer,
which was also a hyperfruit.
The broken replay on this map included one frame at time 7093, with the
catcher position before the hyperdash, and one frame at time 7097,
with the catcher position *after* the hyperdash. Which meant that
the replay handler moved *out of the way* of the hyperfruit at time
7093.9655, deciding that it is *after* the replay frame that was
supposed to be catching it.
(For reference, the relevant replay playback code is here:
3da5831075/osu.Game.Rulesets.Catch/Replays/CatchFramedReplayInputHandler.cs (L20-L31)
Note the handling of `position`. Any frame with an action is important,
which means that as long as any key is held, interpolation will not
take place.)
On stable this is not a thing, because the fruit's end time was being
truncated to `int`, therefore moving it back to time 7093 and restoring
temporal integrity.
This probably doesn't matter in other rulesets that much because
the input tolerances in something like osu! or taiko are much higher.
catch is rather knife-edge, what with mappers doing the "edge dash" /
"pixel jump" stuff (tl;dr: placing a circle just barely outside of
hyperdash range, so that a perfect normal dash is required to catch it).
Thus, this is applied locally to catch for now until proven necessary
to put it elsewhere too.
Stable does this:
46cd3a10af/osu!/GameplayElements/HitObjectManagerFruits.cs#L98-L102
I'd rather not say what I think about it doing that, since it's likely
to be unpublishable, but to approximate that, just make it so that
only the "default fail condition" is beholden to the weird ebbs
and flows of what the ruleset wants. This appears to fix the problem
case and I'm hoping it doesn't break something else but I'm like 50/50
on it happening anyway at this point. Just gotta add tests add nauseam.
Fixes issue that occurs on *about* 246 beatmaps and was first described
by me on discord:
https://discord.com/channels/188630481301012481/188630652340404224/1154367700378865715
and then rediscovered again during work on
https://github.com/ppy/osu/pull/26405:
https://gist.github.com/bdach/414d5289f65b0399fa8f9732245a4f7c#venenog-on-ultmate-end-by-blacky-overdose-631
It so happens that in stable, due to .NET Framework internals, float
math would be performed using x87 registers and opcodes.
.NET (Core) however uses SSE instructions on 32- and 64-bit words.
x87 registers are _80 bits_ wide. Which is notably wider than _both_
float and double. Therefore, on a significant number of beatmaps,
the rounding would not produce correct values due to insufficient
precision.
See following gist for corroboration of the above:
https://gist.github.com/bdach/dcde58d5a3607b0408faa3aa2b67bf10
Thus, to crudely - but, seemingly accurately, after checking across
all ranked maps - emulate this, use `decimal`, which is slow, but has
bigger precision than `double`. The single known exception beatmap
in whose case this results in an incorrect result is
https://osu.ppy.sh/beatmapsets/1156087#osu/2625853
which is considered an "acceptable casualty" of sorts.
Doing this requires some fooling of the compiler / runtime (see second
inline comment in new method). To corroborate that this is required,
you can try the following code snippet:
Console.WriteLine(string.Join(' ', BitConverter.GetBytes(1.3f).Select(x => x.ToString("X2"))));
Console.WriteLine(string.Join(' ', BitConverter.GetBytes(1.3).Select(x => x.ToString("X2"))));
Console.WriteLine();
decimal d1 = (decimal)1.3f;
decimal d2 = (decimal)1.3;
decimal d3 = (decimal)(double)1.3f;
Console.WriteLine(string.Join(' ', decimal.GetBits(d1).SelectMany(BitConverter.GetBytes).Select(x => x.ToString("X2"))));
Console.WriteLine(string.Join(' ', decimal.GetBits(d2).SelectMany(BitConverter.GetBytes).Select(x => x.ToString("X2"))));
Console.WriteLine(string.Join(' ', decimal.GetBits(d3).SelectMany(BitConverter.GetBytes).Select(x => x.ToString("X2"))));
which will print
66 66 A6 3F
CD CC CC CC CC CC F4 3F
0D 00 00 00 00 00 00 00 00 00 00 00 00 00 01 00
0D 00 00 00 00 00 00 00 00 00 00 00 00 00 01 00
8C 5D 89 FB 3B 76 00 00 00 00 00 00 00 00 0E 00
Note that despite `d1` being converted from a less-precise floating-
-point value than `d2`, it still is represented 100% accurately as
a decimal number.
After applying this change, recomputation of legacy scoring attributes
for *all* rulesets will be required.
Closes https://github.com/ppy/osu/issues/25827.
The logic cannot be easily abstracted out (because
`CompareReverseChildID()` is protected and non-static on
`CompositeDrawable`), so a local copy was applied instead.
No testing as any testing would have been purely visual anyways.
