From 017003deea88739c244e88facd84252a940a6372 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 18 Dec 2023 21:56:56 +0100 Subject: [PATCH 1/4] Fix osu! standardised score conversion sometimes exceeding bounds Co-authored-by: Zyf 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: https://github.com/ppy/osu/blob/8e8d9b2cd96be4c4b3d8f1f01dc013fc9d41f765/osu.Game/Database/StandardisedScoreMigrationTools.cs#L323 which then propagates negative numbers onward until https://github.com/ppy/osu/blob/8e8d9b2cd96be4c4b3d8f1f01dc013fc9d41f765/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. --- .../Database/StandardisedScoreMigrationTools.cs | 15 +++++++++++++-- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 3 ++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/StandardisedScoreMigrationTools.cs b/osu.Game/Database/StandardisedScoreMigrationTools.cs index 6484500bcc..a30c40fdb0 100644 --- a/osu.Game/Database/StandardisedScoreMigrationTools.cs +++ b/osu.Game/Database/StandardisedScoreMigrationTools.cs @@ -293,13 +293,24 @@ namespace osu.Game.Database // Roughly corresponds to integrating f(combo) = combo ^ COMBO_EXPONENT (omitting constants) double maximumAchievableComboPortionInStandardisedScore = Math.Pow(maximumLegacyCombo, 1 + ScoreProcessor.COMBO_EXPONENT); - double comboPortionInScoreV1 = maximumAchievableComboPortionInScoreV1 * comboProportion / score.Accuracy; - // This is - roughly - how much score, in the combo portion, the longest combo on this particular play would gain in score V1. double comboPortionFromLongestComboInScoreV1 = Math.Pow(score.MaxCombo, 2); // Same for standardised score. double comboPortionFromLongestComboInStandardisedScore = Math.Pow(score.MaxCombo, 1 + ScoreProcessor.COMBO_EXPONENT); + // We estimate the combo portion of the score in score V1 terms. + // The division by accuracy is supposed to lessen the impact of accuracy on the combo portion, + // but in some edge cases it cannot sanely undo it. + // Therefore the resultant value is clamped from both sides for sanity. + // The clamp from below to `comboPortionFromLongestComboInScoreV1` targets near-FC scores wherein + // the player had bad accuracy at the end of their longest combo, which causes the division by accuracy + // to underestimate the combo portion. + // The clamp from above to `maximumAchievableComboPortionInScoreV1` targets FC scores wherein + // the player had bad accuracy at the start of the map, which causes the division by accuracy + // to overestimate the combo portion. + double comboPortionInScoreV1 = Math.Clamp(maximumAchievableComboPortionInScoreV1 * comboProportion / score.Accuracy, + comboPortionFromLongestComboInScoreV1, maximumAchievableComboPortionInScoreV1); + // Calculate how many times the longest combo the user has achieved in the play can repeat // without exceeding the combo portion in score V1 as achieved by the player. // This is a pessimistic estimate; it intentionally does not operate on object count and uses only score instead. diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index 00b1f04782..5c51e68d9f 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -32,9 +32,10 @@ namespace osu.Game.Scoring.Legacy /// 30000003: First version after converting legacy total score to standardised. /// 30000004: Fixed mod multipliers during legacy score conversion. Reconvert all scores. /// 30000005: Introduce combo exponent in the osu! gamemode. Reconvert all scores. + /// 30000006: Fix edge cases in conversion after combo exponent introduction that lead to NaNs. Reconvert all scores. /// /// - public const int LATEST_VERSION = 30000005; + public const int LATEST_VERSION = 30000006; /// /// The first stable-compatible YYYYMMDD format version given to lazer usage of replays. From 9c8df4e6d1a041c0b2fc19873aa8ae8e1d0d9fb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 18 Dec 2023 22:23:58 +0100 Subject: [PATCH 2/4] Run score conversion for previously-imported scores --- osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs | 3 +++ osu.Game/BackgroundDataStoreProcessor.cs | 3 +-- osu.Game/Database/StandardisedScoreMigrationTools.cs | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs b/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs index e65088ca2e..43ce7200d2 100644 --- a/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs +++ b/osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs @@ -127,8 +127,11 @@ namespace osu.Game.Tests.Database }); } + [TestCase(30000001)] [TestCase(30000002)] [TestCase(30000003)] + [TestCase(30000004)] + [TestCase(30000005)] public void TestScoreUpgradeSuccess(int scoreVersion) { ScoreInfo scoreInfo = null!; diff --git a/osu.Game/BackgroundDataStoreProcessor.cs b/osu.Game/BackgroundDataStoreProcessor.cs index 33b66ecfc7..0d5cb84359 100644 --- a/osu.Game/BackgroundDataStoreProcessor.cs +++ b/osu.Game/BackgroundDataStoreProcessor.cs @@ -316,8 +316,7 @@ namespace osu.Game HashSet scoreIds = realmAccess.Run(r => new HashSet(r.All() .Where(s => !s.BackgroundReprocessingFailed && s.BeatmapInfo != null - && (s.TotalScoreVersion == 30000002 - || s.TotalScoreVersion == 30000003)) + && s.TotalScoreVersion < LegacyScoreEncoder.LATEST_VERSION) .AsEnumerable().Select(s => s.ID))); Logger.Log($"Found {scoreIds.Count} scores which require total score conversion."); diff --git a/osu.Game/Database/StandardisedScoreMigrationTools.cs b/osu.Game/Database/StandardisedScoreMigrationTools.cs index a30c40fdb0..6980e81f58 100644 --- a/osu.Game/Database/StandardisedScoreMigrationTools.cs +++ b/osu.Game/Database/StandardisedScoreMigrationTools.cs @@ -26,7 +26,7 @@ namespace osu.Game.Database if (score.IsLegacyScore) return false; - if (score.TotalScoreVersion > 30000004) + if (score.TotalScoreVersion > 30000005) return false; // Recalculate the old-style standardised score to see if this was an old lazer score. From ddb67c87a8c20680c6ef77d8faa512b50d090be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 19 Dec 2023 08:13:02 +0100 Subject: [PATCH 3/4] Roll back incorrect change in `ShouldMigrateToNewStandardised()` --- osu.Game/Database/StandardisedScoreMigrationTools.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/StandardisedScoreMigrationTools.cs b/osu.Game/Database/StandardisedScoreMigrationTools.cs index 6980e81f58..11eacd1c6b 100644 --- a/osu.Game/Database/StandardisedScoreMigrationTools.cs +++ b/osu.Game/Database/StandardisedScoreMigrationTools.cs @@ -26,7 +26,7 @@ namespace osu.Game.Database if (score.IsLegacyScore) return false; - if (score.TotalScoreVersion > 30000005) + if (score.TotalScoreVersion > 30000002) return false; // Recalculate the old-style standardised score to see if this was an old lazer score. From 3f41c20ac6badc09efb5cf61205ecc106c1cabc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 19 Dec 2023 17:25:15 +0100 Subject: [PATCH 4/4] Use safer fix for now --- osu.Game/Database/StandardisedScoreMigrationTools.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/osu.Game/Database/StandardisedScoreMigrationTools.cs b/osu.Game/Database/StandardisedScoreMigrationTools.cs index 11eacd1c6b..d2321f4fc4 100644 --- a/osu.Game/Database/StandardisedScoreMigrationTools.cs +++ b/osu.Game/Database/StandardisedScoreMigrationTools.cs @@ -305,11 +305,10 @@ namespace osu.Game.Database // The clamp from below to `comboPortionFromLongestComboInScoreV1` targets near-FC scores wherein // the player had bad accuracy at the end of their longest combo, which causes the division by accuracy // to underestimate the combo portion. - // The clamp from above to `maximumAchievableComboPortionInScoreV1` targets FC scores wherein - // the player had bad accuracy at the start of the map, which causes the division by accuracy - // to overestimate the combo portion. - double comboPortionInScoreV1 = Math.Clamp(maximumAchievableComboPortionInScoreV1 * comboProportion / score.Accuracy, - comboPortionFromLongestComboInScoreV1, maximumAchievableComboPortionInScoreV1); + // Ideally, this would be clamped from above to `maximumAchievableComboPortionInScoreV1` too, + // but in practice this appears to fail for some scores (https://github.com/ppy/osu/pull/25876#issuecomment-1862248413). + // TODO: investigate the above more closely + double comboPortionInScoreV1 = Math.Max(maximumAchievableComboPortionInScoreV1 * comboProportion / score.Accuracy, comboPortionFromLongestComboInScoreV1); // Calculate how many times the longest combo the user has achieved in the play can repeat // without exceeding the combo portion in score V1 as achieved by the player.