Merge pull request ppy#24368 from peppy/fix-score-import-fail-fail-fail

Avoid reprocessing scores which already failed an upgrade previously

osu.Game.Tests/Database/BackgroundBeatmapProcessorTests.cs → osu.Game.Tests/Database/BackgroundDataStoreProcessorTests.cs

@@ -8,14 +8,17 @@
 using osu.Framework.Extensions;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
+using osu.Game.Rulesets;
+using osu.Game.Scoring;
+using osu.Game.Scoring.Legacy;
 using osu.Game.Screens.Play;
 using osu.Game.Tests.Beatmaps.IO;
 using osu.Game.Tests.Visual;
 
 namespace osu.Game.Tests.Database
 {
     [HeadlessTest]
-    public partial class BackgroundBeatmapProcessorTests : OsuTestScene, ILocalUserPlayInfo
+    public partial class BackgroundDataStoreProcessorTests : OsuTestScene, ILocalUserPlayInfo
     {
         public IBindable<bool> IsPlaying => isPlaying;
 
@@ -59,7 +62,7 @@ public void TestDifficultyProcessing()
 
             AddStep("Run background processor", () =>
             {
-                Add(new TestBackgroundBeatmapProcessor());
+                Add(new TestBackgroundDataStoreProcessor());
             });
 
             AddUntilStep("wait for difficulties repopulated", () =>
@@ -98,7 +101,7 @@ public void TestDifficultyProcessingWhilePlaying()
 
             AddStep("Run background processor", () =>
             {
-                Add(new TestBackgroundBeatmapProcessor());
+                Add(new TestBackgroundDataStoreProcessor());
             });
 
             AddWaitStep("wait some", 500);
@@ -124,7 +127,58 @@ public void TestDifficultyProcessingWhilePlaying()
             });
         }
 
-        public partial class TestBackgroundBeatmapProcessor : BackgroundBeatmapProcessor
+        [Test]
+        public void TestScoreUpgradeSuccess()
+        {
+            ScoreInfo scoreInfo = null!;
+
+            AddStep("Add score which requires upgrade (and has beatmap)", () =>
+            {
+                Realm.Write(r =>
+                {
+                    r.Add(scoreInfo = new ScoreInfo(ruleset: r.All<RulesetInfo>().First(), beatmap: r.All<BeatmapInfo>().First())
+                    {
+                        TotalScoreVersion = 30000002,
+                        LegacyTotalScore = 123456,
+                        IsLegacyScore = true,
+                    });
+                });
+            });
+
+            AddStep("Run background processor", () => Add(new TestBackgroundDataStoreProcessor()));
+
+            AddUntilStep("Score version upgraded", () => Realm.Run(r => r.Find<ScoreInfo>(scoreInfo.ID)!.TotalScoreVersion), () => Is.EqualTo(LegacyScoreEncoder.LATEST_VERSION));
+            AddAssert("Score not marked as failed", () => Realm.Run(r => r.Find<ScoreInfo>(scoreInfo.ID)!.BackgroundReprocessingFailed), () => Is.False);
+        }
+
+        [Test]
+        public void TestScoreUpgradeFailed()
+        {
+            ScoreInfo scoreInfo = null!;
+
+            AddStep("Add score which requires upgrade (but has no beatmap)", () =>
+            {
+                Realm.Write(r =>
+                {
+                    r.Add(scoreInfo = new ScoreInfo(ruleset: r.All<RulesetInfo>().First(), beatmap: new BeatmapInfo
+                    {
+                        BeatmapSet = new BeatmapSetInfo(),
+                        Ruleset = r.All<RulesetInfo>().First(),
+                    })
+                    {
+                        TotalScoreVersion = 30000002,
+                        IsLegacyScore = true,
+                    });
+                });
+            });
+
+            AddStep("Run background processor", () => Add(new TestBackgroundDataStoreProcessor()));
+
+            AddUntilStep("Score marked as failed", () => Realm.Run(r => r.Find<ScoreInfo>(scoreInfo.ID)!.BackgroundReprocessingFailed), () => Is.True);
+            AddAssert("Score version not upgraded", () => Realm.Run(r => r.Find<ScoreInfo>(scoreInfo.ID)!.TotalScoreVersion), () => Is.EqualTo(30000002));
+        }
+
+        public partial class TestBackgroundDataStoreProcessor : BackgroundDataStoreProcessor
         {
             protected override int TimeToSleepDuringGameplay => 10;
         }

osu.Game/BackgroundBeatmapProcessor.cs → osu.Game/BackgroundDataStoreProcessor.cs

@@ -24,7 +24,10 @@
 
 namespace osu.Game
 {
-    public partial class BackgroundBeatmapProcessor : Component
+    /// <summary>
+    /// Performs background updating of data stores at startup.
+    /// </summary>
+    public partial class BackgroundDataStoreProcessor : Component
     {
         [Resolved]
         private RulesetStore rulesetStore { get; set; } = null!;
@@ -61,7 +64,8 @@ protected override void LoadComplete()
 
             Task.Factory.StartNew(() =>
             {
-                Logger.Log("Beginning background beatmap processing..");
+                Logger.Log("Beginning background data store processing..");
+
                 checkForOutdatedStarRatings();
                 processBeatmapSetsWithMissingMetrics();
                 processScoresWithMissingStatistics();
@@ -74,7 +78,7 @@ protected override void LoadComplete()
                     return;
                 }
 
-                Logger.Log("Finished background beatmap processing!");
+                Logger.Log("Finished background data store processing!");
             });
         }
 
@@ -182,7 +186,7 @@ private void processScoresWithMissingStatistics()
 
             realmAccess.Run(r =>
             {
-                foreach (var score in r.All<ScoreInfo>())
+                foreach (var score in r.All<ScoreInfo>().Where(s => !s.BackgroundReprocessingFailed))
                 {
                     if (score.BeatmapInfo != null
                         && score.Statistics.Sum(kvp => kvp.Value) > 0
@@ -221,6 +225,7 @@ private void processScoresWithMissingStatistics()
                 catch (Exception e)
                 {
                     Logger.Log(@$"Failed to populate maximum statistics for {id}: {e}");
+                    realmAccess.Write(r => r.Find<ScoreInfo>(id)!.BackgroundReprocessingFailed = true);
                 }
             }
         }
@@ -230,7 +235,7 @@ private void convertLegacyTotalScoreToStandardised()
             Logger.Log("Querying for scores that need total score conversion...");
 
             HashSet<Guid> scoreIds = realmAccess.Run(r => new HashSet<Guid>(r.All<ScoreInfo>()
-                                                                             .Where(s => s.BeatmapInfo != null && s.TotalScoreVersion == 30000002)
+                                                                             .Where(s => !s.BackgroundReprocessingFailed && s.BeatmapInfo != null && s.TotalScoreVersion == 30000002)
                                                                              .AsEnumerable().Select(s => s.ID)));
 
             Logger.Log($"Found {scoreIds.Count} scores which require total score conversion.");
@@ -279,6 +284,7 @@ private void convertLegacyTotalScoreToStandardised()
                 catch (Exception e)
                 {
                     Logger.Log($"Failed to convert total score for {id}: {e}");
+                    realmAccess.Write(r => r.Find<ScoreInfo>(id)!.BackgroundReprocessingFailed = true);
                     ++failedCount;
                 }
             }

osu.Game/Database/RealmAccess.cs

@@ -83,8 +83,9 @@ public class RealmAccess : IDisposable
         /// 31   2023-06-26    Add Version and LegacyTotalScore to ScoreInfo, set Version to 30000002 and copy TotalScore into LegacyTotalScore for legacy scores.
         /// 32   2023-07-09    Populate legacy scores with the ScoreV2 mod (and restore TotalScore to the legacy total for such scores) using replay files.
         /// 33   2023-08-16    Reset default chat toggle key binding to avoid conflict with newly added leaderboard toggle key binding.
+        /// 34   2023-08-21    Add BackgroundReprocessingFailed flag to ScoreInfo to track upgrade failures.
         /// </summary>
-        private const int schema_version = 33;
+        private const int schema_version = 34;
 
         /// <summary>
         /// Lock object which is held during <see cref="BlockAllOperations"/> sections, blocking realm retrieval during blocking periods.

osu.Game/OsuGame.cs

@@ -1025,7 +1025,7 @@ protected override void LoadComplete()
 
             loadComponentSingleFile(CreateHighPerformanceSession(), Add);
 
-            loadComponentSingleFile(new BackgroundBeatmapProcessor(), Add);
+            loadComponentSingleFile(new BackgroundDataStoreProcessor(), Add);
 
             Add(difficultyRecommender);
             Add(externalLinkOpener = new ExternalLinkOpener());

osu.Game/Scoring/ScoreInfo.cs

@@ -66,7 +66,7 @@ public class ScoreInfo : RealmObject, IHasGuidPrimaryKey, IHasRealmFiles, ISoftD
         /// If this does not match <see cref="LegacyScoreEncoder.LATEST_VERSION"/>,
         /// the total score has not yet been updated to reflect the current scoring values.
         ///
-        /// See <see cref="BackgroundBeatmapProcessor"/>'s conversion logic.
+        /// See <see cref="BackgroundDataStoreProcessor"/>'s conversion logic.
         /// </summary>
         /// <remarks>
         /// This may not match the version stored in the replay files.
@@ -81,6 +81,15 @@ public class ScoreInfo : RealmObject, IHasGuidPrimaryKey, IHasRealmFiles, ISoftD
         /// </remarks>
         public long? LegacyTotalScore { get; set; }
 
+        /// <summary>
+        /// If background processing of this beatmap failed in some way, this flag will become <c>true</c>.
+        /// Should be used to ensure we don't repeatedly attempt to reprocess the same scores each startup even though we already know they will fail.
+        /// </summary>
+        /// <remarks>
+        /// See https://github.com/ppy/osu/issues/24301 for one example of how this can occur (missing beatmap file on disk).
+        /// </remarks>
+        public bool BackgroundReprocessingFailed { get; set; }
+
         public int MaxCombo { get; set; }
 
         public double Accuracy { get; set; }