Merge pull request #27513 from bdach/osu-score-conversion-bad-very-not-good

Fix osu! standardised score estimation algorithm violating basic invariants

osu.Game/Database/StandardisedScoreMigrationTools.cs

@@ -415,7 +415,7 @@ namespace osu.Game.Database
 
                     // 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.
+                    // This intentionally does not operate on object count and uses only score instead.
                     double maximumOccurrencesOfLongestCombo = Math.Floor(comboPortionInScoreV1 / comboPortionFromLongestComboInScoreV1);
                     double comboPortionFromRepeatedLongestCombosInScoreV1 = maximumOccurrencesOfLongestCombo * comboPortionFromLongestComboInScoreV1;
 
@@ -426,13 +426,12 @@ namespace osu.Game.Database
                     // ...and then based on that raw combo length, we calculate how much this last combo is worth in standardised score.
                     double remainingComboPortionInStandardisedScore = Math.Pow(remainingCombo, 1 + ScoreProcessor.COMBO_EXPONENT);
 
-                    double lowerEstimateOfComboPortionInStandardisedScore
+                    double scoreBasedEstimateOfComboPortionInStandardisedScore
                         = maximumOccurrencesOfLongestCombo * comboPortionFromLongestComboInStandardisedScore
                           + remainingComboPortionInStandardisedScore;
 
                     // Compute approximate upper estimate new score for that play.
                     // This time, divide the remaining combo among remaining objects equally to achieve longest possible combo lengths.
-                    // There is no rigorous proof that doing this will yield a correct upper bound, but it seems to work out in practice.
                     remainingComboPortionInScoreV1 = comboPortionInScoreV1 - comboPortionFromLongestComboInScoreV1;
                     double remainingCountOfObjectsGivingCombo = maximumLegacyCombo - score.MaxCombo - score.Statistics.GetValueOrDefault(HitResult.Miss);
                     // Because we assumed all combos were equal, `remainingComboPortionInScoreV1`
@@ -449,7 +448,17 @@ namespace osu.Game.Database
                     // we can skip adding the 1 and just multiply by x ^ 0.5.
                     remainingComboPortionInStandardisedScore = remainingCountOfObjectsGivingCombo * Math.Pow(lengthOfRemainingCombos, ScoreProcessor.COMBO_EXPONENT);
 
-                    double upperEstimateOfComboPortionInStandardisedScore = comboPortionFromLongestComboInStandardisedScore + remainingComboPortionInStandardisedScore;
+                    double objectCountBasedEstimateOfComboPortionInStandardisedScore = comboPortionFromLongestComboInStandardisedScore + remainingComboPortionInStandardisedScore;
+
+                    // Enforce some invariants on both of the estimates.
+                    // In rare cases they can produce invalid results.
+                    scoreBasedEstimateOfComboPortionInStandardisedScore =
+                        Math.Clamp(scoreBasedEstimateOfComboPortionInStandardisedScore, 0, maximumAchievableComboPortionInStandardisedScore);
+                    objectCountBasedEstimateOfComboPortionInStandardisedScore =
+                        Math.Clamp(objectCountBasedEstimateOfComboPortionInStandardisedScore, 0, maximumAchievableComboPortionInStandardisedScore);
+
+                    double lowerEstimateOfComboPortionInStandardisedScore = Math.Min(scoreBasedEstimateOfComboPortionInStandardisedScore, objectCountBasedEstimateOfComboPortionInStandardisedScore);
+                    double upperEstimateOfComboPortionInStandardisedScore = Math.Max(scoreBasedEstimateOfComboPortionInStandardisedScore, objectCountBasedEstimateOfComboPortionInStandardisedScore);
 
                     // Approximate by combining lower and upper estimates.
                     // As the lower-estimate is very pessimistic, we use a 30/70 ratio

osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs

@@ -45,9 +45,10 @@ namespace osu.Game.Scoring.Legacy
         /// </description></item>
         /// <item><description>30000013: All local scores will use lazer definitions of ranks for consistency. Recalculates the rank of all scores.</description></item>
         /// <item><description>30000014: Fix edge cases in conversion for osu! scores on selected beatmaps. Reconvert all scores.</description></item>
+        /// <item><description>30000015: Fix osu! standardised score estimation algorithm violating basic invariants. Reconvert all scores.</description></item>
         /// </list>
         /// </remarks>
-        public const int LATEST_VERSION = 30000014;
+        public const int LATEST_VERSION = 30000015;
 
         /// <summary>
         /// The first stable-compatible YYYYMMDD format version given to lazer usage of replays.