Fix incorrect score conversion on selected beatmaps due to incorrect difficultyPeppyStars
rounding
#26471
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes issue that occurs on about 245 beatmaps1 and was first described by me on discord and then rediscovered again during work on #26405.
Explanation
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 final rounding of
difficultyPeppyStars
would not produce correct values due to insufficient precision. See following gist for corroboration of the above (compare dump of JIT asm calculating the value for lazer and stable).Thus, to crudely - but, seemingly mostly accurately, after checking across all ranked maps - emulate this, use
decimal
, which is slow, but has bigger precision thandouble
. 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 (read: after spending a whole full day of dev on this, I do not consider a single beatmap's stable scores getting buffed by 10% significant enough to matter).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:
which will print
Note that despite
d1
being converted from a less-precise floating-point value thand2
, it still is represented 100% accurately as a decimal number.Yes, this is a hack. Yes, it doesn't work completely. I think it's better to start with this rather than consider the alternatives, though (which is having something hard-stuck on windows+x86 just to compute this one value for correct score conversion until stable can't submit scores anymore, i.e. forevermore).
Implications
A catch sheet showing the effects of this change is available for preview here, although this really impacts all rulesets. Some sampling of scores from there shows the change having the desired effect:
Most gains, except for the one known-broken beatmap mentioned above (https://osu.ppy.sh/beatmapsets/1156087#osu/2625853), are happening because the combo score was previously overestimated, thus capping the following expression
osu/osu.Game/Database/StandardisedScoreMigrationTools.cs
Line 333 in 7d5e8ff
to 0, and thus underestimating the bonus portion.
The losses are 200% expected and what this PR is primarily intending to fix (scores way in excess of 1M).
Warning
Note that after applying this change, recomputation of legacy scoring attributes for all rulesets will be required for the change to take the desired effect.
Footnotes
An approximate list of affected maps can be found here. The list is approximate because the source of data used to compare computation of
difficultyPeppyStars
between .NET Framework and .NET (Core) was a data dump provided by @peppy, which unfortunately was not 100% accurate due todrainTime
sometimes being off by one. ↩