This subtle detail was messing with server-side score import flows.
Server-side, legacy total score will *already* be in `LegacyTotalScore`
from the start, and `TotalScore` will be zero until recomputed via
`StandardisedScoreMigrationTools.UpdateFromLegacy()` - so in that
context, attempting to move it across is incorrect.
Closes https://github.com/ppy/osu/issues/24061.
The gist of this change is that if the `LegacyReplaySoloScoreInfo`
bolt-on is present in the replay, then it can (and is) used to recompute
the accuracy, and rank is computed based on that.
This was the missing part of
https://github.com/ppy/osu/issues/24061#issuecomment-1888438151.
The accuracy would change on import before that because the encode
process is _lossy_ if the `LegacyReplaySoloScoreInfo` bolt-on is not
used, as the legacy format only has 6 fields for encoding judgement
counts, and some judgements that affect accuracy in lazer do not fit
into that.
Note that this _only_ fixes _relatively_ new lazer scores looking wrong
after reimport.
- Very old lazer scores, i.e. ones that don't have the
`LegacyReplaySoloScoreInfo` bolt-on, obviously can't use it
to repopulate. There's really not much good that can be done there,
so the stable pathways are used as a fallback that always works.
- For stable replays, `ScoreImporter` recalculates the accuracy of
the score _again_ in
15a5fd7e4c/osu.Game/Scoring/ScoreImporter.cs (L106-L110)
as `StandardisedScoreMigrationTools.UpdateFromLegacy()` recomputes
_both_ total score and accuracy.
This makes a _semblance_ of sense as it attempts to make the accuracy
of stable and lazer replays comparable. In most cases it also won't
matter, as the only ruleset where accuracy changed between the legacy
implementation and current lazer accuracy is mania.
But it is also an inaccurate process (as, again, some of the required
data is not in the replay, namely judgement counts of ticks
and so on).
For whatever's worth, a similar thing happens server-side in
106c2948db/osu.Server.Queues.ScoreStatisticsProcessor/Commands/Queue/BatchInserter.cs (L319)
- However, _ranks_ of stable scores will still use the local stable
reimplementation of ranks, i.e. a 1-miss stable score in osu! ruleset
will be an A rather than an S. See importer:
106c2948db/osu.Server.Queues.ScoreStatisticsProcessor/Commands/Queue/BatchInserter.cs (L237)
(it's the same method which is renamed
to `PopulateLegacyAccuracyAndRank()` in this commit).
That is all a bit of a mess honestly, but I'm not sure where to even
begin there...
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.
