Skip to content

Commit

Permalink
Fix incorrect score conversion on selected beatmaps due to incorrect …
Browse files Browse the repository at this point in the history
…`difficultyPeppyStars` rounding

Fixes issue that occurs on *about* 245 beatmaps and was first described
by me on discord:

    https://discord.com/channels/188630481301012481/188630652340404224/1154367700378865715

and then rediscovered again during work on
ppy#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.

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`.

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.
  • Loading branch information
bdach committed Jan 10, 2024
1 parent a4c9e9f commit 0dbbf57
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using osu.Game.Rulesets.Catch.Scoring;
using osu.Game.Rulesets.Mods;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Legacy;
using osu.Game.Rulesets.Objects.Types;
using osu.Game.Rulesets.Scoring;
using osu.Game.Rulesets.Scoring.Legacy;
Expand Down Expand Up @@ -62,13 +63,7 @@ public LegacyScoreAttributes Simulate(IWorkingBeatmap workingBeatmap, IBeatmap p
drainLength = ((int)Math.Round(baseBeatmap.HitObjects[^1].StartTime) - (int)Math.Round(baseBeatmap.HitObjects[0].StartTime) - breakLength) / 1000;
}

int difficultyPeppyStars = (int)Math.Round(
(baseBeatmap.Difficulty.DrainRate
+ baseBeatmap.Difficulty.OverallDifficulty
+ baseBeatmap.Difficulty.CircleSize
+ Math.Clamp((float)objectCount / drainLength * 8, 0, 16)) / 38 * 5);

scoreMultiplier = difficultyPeppyStars;
scoreMultiplier = LegacyRulesetExtensions.CalculateDifficultyPeppyStars(baseBeatmap.Difficulty, objectCount, drainLength);

LegacyScoreAttributes attributes = new LegacyScoreAttributes();

Expand Down
9 changes: 2 additions & 7 deletions osu.Game.Rulesets.Osu/Difficulty/OsuLegacyScoreSimulator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using osu.Game.Beatmaps;
using osu.Game.Rulesets.Mods;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Legacy;
using osu.Game.Rulesets.Objects.Types;
using osu.Game.Rulesets.Osu.Mods;
using osu.Game.Rulesets.Osu.Objects;
Expand Down Expand Up @@ -62,13 +63,7 @@ public LegacyScoreAttributes Simulate(IWorkingBeatmap workingBeatmap, IBeatmap p
drainLength = ((int)Math.Round(baseBeatmap.HitObjects[^1].StartTime) - (int)Math.Round(baseBeatmap.HitObjects[0].StartTime) - breakLength) / 1000;
}

int difficultyPeppyStars = (int)Math.Round(
(baseBeatmap.Difficulty.DrainRate
+ baseBeatmap.Difficulty.OverallDifficulty
+ baseBeatmap.Difficulty.CircleSize
+ Math.Clamp((float)objectCount / drainLength * 8, 0, 16)) / 38 * 5);

scoreMultiplier = difficultyPeppyStars;
scoreMultiplier = LegacyRulesetExtensions.CalculateDifficultyPeppyStars(baseBeatmap.Difficulty, objectCount, drainLength);

LegacyScoreAttributes attributes = new LegacyScoreAttributes();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using osu.Game.Beatmaps;
using osu.Game.Rulesets.Mods;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Legacy;
using osu.Game.Rulesets.Objects.Types;
using osu.Game.Rulesets.Scoring;
using osu.Game.Rulesets.Scoring.Legacy;
Expand Down Expand Up @@ -65,11 +66,7 @@ public LegacyScoreAttributes Simulate(IWorkingBeatmap workingBeatmap, IBeatmap p
drainLength = ((int)Math.Round(baseBeatmap.HitObjects[^1].StartTime) - (int)Math.Round(baseBeatmap.HitObjects[0].StartTime) - breakLength) / 1000;
}

difficultyPeppyStars = (int)Math.Round(
(baseBeatmap.Difficulty.DrainRate
+ baseBeatmap.Difficulty.OverallDifficulty
+ baseBeatmap.Difficulty.CircleSize
+ Math.Clamp((float)objectCount / drainLength * 8, 0, 16)) / 38 * 5);
difficultyPeppyStars = LegacyRulesetExtensions.CalculateDifficultyPeppyStars(baseBeatmap.Difficulty, objectCount, drainLength);

LegacyScoreAttributes attributes = new LegacyScoreAttributes();

Expand Down
33 changes: 33 additions & 0 deletions osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,38 @@ public static float CalculateScaleFromCircleSize(float circleSize, bool applyFud

return (float)(1.0f - 0.7f * IBeatmapDifficultyInfo.DifficultyRange(circleSize)) / 2 * (applyFudge ? broken_gamefield_rounding_allowance : 1);
}

public static int CalculateDifficultyPeppyStars(BeatmapDifficulty difficulty, int objectCount, int drainLength)
{
/*
* WARNING: DO NOT TOUCH IF YOU DO NOT KNOW WHAT YOU ARE DOING
*
* 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.
*
* 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`.
*/

decimal objectToDrainRatio = drainLength != 0
? Math.Clamp((decimal)objectCount / drainLength * 8, 0, 16)
: 16;

/*
* Notably, THE `double` CASTS BELOW ARE IMPORTANT AND MUST REMAIN.
* Their goal is to trick the compiler / runtime into NOT promoting from single-precision float, as doing so would prompt it
* to attempt to "silently" fix the single-precision values when converting to decimal,
* which is NOT what the x87 FPU does.
*/

decimal drainRate = (decimal)(double)difficulty.DrainRate;
decimal overallDifficulty = (decimal)(double)difficulty.OverallDifficulty;
decimal circleSize = (decimal)(double)difficulty.CircleSize;

return (int)Math.Round((drainRate + overallDifficulty + circleSize + objectToDrainRatio) / 38 * 5);
}
}
}
7 changes: 6 additions & 1 deletion osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using osu.Game.IO.Legacy;
using osu.Game.IO.Serialization;
using osu.Game.Replays.Legacy;
using osu.Game.Rulesets.Objects.Legacy;
using osu.Game.Rulesets.Replays;
using osu.Game.Rulesets.Replays.Types;
using SharpCompress.Compressors.LZMA;
Expand All @@ -38,9 +39,13 @@ public class LegacyScoreEncoder
/// <item><description>30000009: Fix edge cases in conversion for scores which have 0.0x mod multiplier on stable. Reconvert all scores.</description></item>
/// <item><description>30000010: Fix mania score V1 conversion using score V1 accuracy rather than V2 accuracy. Reconvert all scores.</description></item>
/// <item><description>30000011: Re-do catch scoring to mirror stable Score V2 as closely as feasible. Reconvert all scores.</description></item>
/// <item><description>
/// 30000012: Fix incorrect total score conversion on selected beatmaps after implementing the more correct
/// <see cref="LegacyRulesetExtensions.CalculateDifficultyPeppyStars"/> method. Reconvert all scores.
/// </description></item>
/// </list>
/// </remarks>
public const int LATEST_VERSION = 30000011;
public const int LATEST_VERSION = 30000012;

/// <summary>
/// The first stable-compatible YYYYMMDD format version given to lazer usage of replays.
Expand Down

0 comments on commit 0dbbf57

Please sign in to comment.