From 7c9adc7ad3c1d4c72620d92cb44bd665c6e8a28c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 10 Jan 2024 16:51:40 +0100 Subject: [PATCH] Fix incorrect score conversion on selected beatmaps due to incorrect `difficultyPeppyStars` rounding 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. --- .../Difficulty/CatchLegacyScoreSimulator.cs | 9 ++--- .../Difficulty/OsuLegacyScoreSimulator.cs | 9 ++--- .../Difficulty/TaikoLegacyScoreSimulator.cs | 7 ++-- .../Objects/Legacy/LegacyRulesetExtensions.cs | 35 +++++++++++++++++++ osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 7 +++- 5 files changed, 47 insertions(+), 20 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Difficulty/CatchLegacyScoreSimulator.cs b/osu.Game.Rulesets.Catch/Difficulty/CatchLegacyScoreSimulator.cs index f65b6ef381..f931795ff2 100644 --- a/osu.Game.Rulesets.Catch/Difficulty/CatchLegacyScoreSimulator.cs +++ b/osu.Game.Rulesets.Catch/Difficulty/CatchLegacyScoreSimulator.cs @@ -10,6 +10,7 @@ using osu.Game.Rulesets.Catch.Objects; 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; @@ -62,13 +63,7 @@ namespace osu.Game.Rulesets.Catch.Difficulty 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(); diff --git a/osu.Game.Rulesets.Osu/Difficulty/OsuLegacyScoreSimulator.cs b/osu.Game.Rulesets.Osu/Difficulty/OsuLegacyScoreSimulator.cs index a76054b42c..b808deab5c 100644 --- a/osu.Game.Rulesets.Osu/Difficulty/OsuLegacyScoreSimulator.cs +++ b/osu.Game.Rulesets.Osu/Difficulty/OsuLegacyScoreSimulator.cs @@ -7,6 +7,7 @@ using System.Linq; 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; @@ -62,13 +63,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty 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(); diff --git a/osu.Game.Rulesets.Taiko/Difficulty/TaikoLegacyScoreSimulator.cs b/osu.Game.Rulesets.Taiko/Difficulty/TaikoLegacyScoreSimulator.cs index b20aa4f2b6..66ff0fc3d9 100644 --- a/osu.Game.Rulesets.Taiko/Difficulty/TaikoLegacyScoreSimulator.cs +++ b/osu.Game.Rulesets.Taiko/Difficulty/TaikoLegacyScoreSimulator.cs @@ -7,6 +7,7 @@ using System.Linq; 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; @@ -65,11 +66,7 @@ namespace osu.Game.Rulesets.Taiko.Difficulty 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(); diff --git a/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs b/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs index 53cf835248..2a5a11161b 100644 --- a/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs +++ b/osu.Game/Rulesets/Objects/Legacy/LegacyRulesetExtensions.cs @@ -57,5 +57,40 @@ namespace osu.Game.Rulesets.Objects.Legacy 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 *mostly* accurately, after checking across all ranked maps - emulate this, + * use `decimal`, which is slow, but has bigger precision than `double`. + * At the time of writing, there is _one_ ranked exception to this - namely https://osu.ppy.sh/beatmapsets/1156087#osu/2625853 - + * but it is considered an "acceptable casualty", since in that case scores aren't inflated by _that_ much compared to others. + */ + + 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); + } } } diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index 110cf63e5c..389b20b5c8 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -13,6 +13,7 @@ using osu.Game.Extensions; 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; @@ -38,9 +39,13 @@ namespace osu.Game.Scoring.Legacy /// 30000009: Fix edge cases in conversion for scores which have 0.0x mod multiplier on stable. Reconvert all scores. /// 30000010: Fix mania score V1 conversion using score V1 accuracy rather than V2 accuracy. Reconvert all scores. /// 30000011: Re-do catch scoring to mirror stable Score V2 as closely as feasible. Reconvert all scores. + /// + /// 30000012: Fix incorrect total score conversion on selected beatmaps after implementing the more correct + /// method. Reconvert all scores. + /// /// /// - public const int LATEST_VERSION = 30000011; + public const int LATEST_VERSION = 30000012; /// /// The first stable-compatible YYYYMMDD format version given to lazer usage of replays.