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.
Partially addresses https://github.com/ppy/osu/discussions/26416
As pointed out in the discussion thread above, the total score
conversion process for mania was using accuracy directly from the
replay. In mania accuracy is calculated differently in score V1 than in
score V2, which meant that scores coming from stable were treated more
favourably (due to weighting GREAT and PERFECT equally).
To fix, recompute accuracy locally and use that for the accuracy
portion.
Note that this will still not be (and cannot be made) 100% accurate, as
in stable score V2, as well as in lazer, hold notes are *two*
judgements, not one as in stable score V1, meaning that full and correct
score statistics are not available without playing back the replay.
The effects of the change can be previewed on the following spreadsheet:
https://docs.google.com/spreadsheets/d/1wxD4UwLjwcr7n9y5Yq7EN0lgiLBN93kpd4gBnAlG-E0/edit#gid=1711190356
Top 5 changed scores with replays:
| score | master | this PR | replay |
| :------------------------------------------------------------------------------------------------------------------------------- | ------: | ------: | ------: |
| [Outlasted on Uwa!! So Holiday by toby fox [[4K] easy] (0.71\*)](https://osu.ppy.sh/scores/mania/460404716) | 935,917 | 927,269 | 920,579 |
| [ag0 on Emotional Uplifting Orchestral by bradbreeck [[4K] Rocket's Normal] (0.76\*)](https://osu.ppy.sh/scores/mania/453133066) | 921,636 | 913,535 | 875,549 |
| [rlarkgus on Zen Zen Zense by Gom (HoneyWorks) [[5K] Normal] (1.68\*)](https://osu.ppy.sh/scores/mania/458368312) | 934,340 | 926,787 | 918,855 |
| [YuJJun on Harumachi Clover by R3 Music Box [4K Catastrophe] (1.80\*)](https://osu.ppy.sh/scores/mania/548215786) | 918,606 | 911,111 | 885,454 |
| [Fritte on 45-byou by respon feat. Hatsune Miku & Megpoid [[5K] Normal] (1.52\*)](https://osu.ppy.sh/scores/mania/516079410) | 900,024 | 892,569 | 907,456 |
Co-authored-by: Zyf <zyfarok@gmail.com>
Closes https://github.com/ppy/osu/issues/25860
Users reported that some stable scores would convert to large negative
total scores in lazer after the introduction of combo exponent. Those
large negative total scores were actually mangled NaNs.
The root cause of this was the following calculation going below zero
unexpectedly:
8e8d9b2cd9/osu.Game/Database/StandardisedScoreMigrationTools.cs (L323)
which then propagates negative numbers onward until
8e8d9b2cd9/osu.Game/Database/StandardisedScoreMigrationTools.cs (L337)
which yields a NaN due to attempting to take the square root of a
negative number.
To fix, clamp `comboPortionInScoreV1` to sane limits: to
`comboPortionFromLongestComboInScoreV1` from below, and to
`maximumAchievableComboPortionInScoreV1` from above. This is a less
direct fix than perhaps imagined, but it seems like a better one as it
will also affect the calculation of both the lower and the upper
estimate of the score.
Mostly places that can interact with imported replays.
There are other places that use the online ID as a sort tiebreaker, or
to check presence of a score on results screens, but they should
probably still continue to only use `OnlineID`, since all scores with a
legacy online ID should have an online ID, but the converse is not
generally true.
Causes several knock-on inspections in `OsuGame` et al. Probably best
addressed in a separate pass, because treatment is mixed at best (some
places nullcheck, some expect non-null).
