Move beatmap processing tasks to new BeatmapUpdater class

osu.Game.Tests/Database/BeatmapImporterTests.cs

@@ -35,7 +35,8 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using (var importer = new BeatmapImporter(storage, realm))
+                var importer = new BeatmapImporter(storage, realm);
+
                 using (new RealmRulesetStore(realm, storage))
                 {
                     var beatmapSet = await importer.Import(new ImportTask(TestResources.GetTestBeatmapStream(), "renatus.osz"));
@@ -76,7 +77,8 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using (var importer = new BeatmapImporter(storage, realm))
+                var importer = new BeatmapImporter(storage, realm);
+
                 using (new RealmRulesetStore(realm, storage))
                 {
                     var beatmapSet = await importer.Import(new ImportTask(TestResources.GetTestBeatmapStream(), "renatus.osz"));
@@ -134,7 +136,8 @@ namespace osu.Game.Tests.Database
 
                 var manager = new ModelManager<BeatmapSetInfo>(storage, realm);
 
-                using (var importer = new BeatmapImporter(storage, realm))
+                var importer = new BeatmapImporter(storage, realm);
+
                 using (new RealmRulesetStore(realm, storage))
                 {
                     Task.Run(async () =>
@@ -160,7 +163,8 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using (var importer = new BeatmapImporter(storage, realm))
+                var importer = new BeatmapImporter(storage, realm);
+
                 using (new RealmRulesetStore(realm, storage))
                 {
                     var imported = await importer.Import(new ImportTask(TestResources.GetTestBeatmapStream(), "renatus.osz"));
@@ -187,7 +191,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 await LoadOszIntoStore(importer, realm.Realm);
@@ -199,7 +203,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 var imported = await LoadOszIntoStore(importer, realm.Realm);
@@ -217,7 +221,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 var imported = await LoadOszIntoStore(importer, realm.Realm);
@@ -231,7 +235,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? tempPath = TestResources.GetTestBeatmapForImport();
@@ -261,7 +265,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 var imported = await LoadOszIntoStore(importer, realm.Realm);
@@ -281,7 +285,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? temp = TestResources.GetTestBeatmapForImport();
@@ -317,7 +321,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? temp = TestResources.GetTestBeatmapForImport();
@@ -366,7 +370,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? temp = TestResources.GetTestBeatmapForImport();
@@ -417,7 +421,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? temp = TestResources.GetTestBeatmapForImport();
@@ -465,7 +469,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? temp = TestResources.GetTestBeatmapForImport();
@@ -513,7 +517,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 var imported = await LoadOszIntoStore(importer, realm.Realm);
@@ -548,7 +552,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 var progressNotification = new ImportProgressNotification();
@@ -586,7 +590,7 @@ namespace osu.Game.Tests.Database
                         Interlocked.Increment(ref loggedExceptionCount);
                 };
 
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 var imported = await LoadOszIntoStore(importer, realm.Realm);
@@ -638,7 +642,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 var imported = await LoadOszIntoStore(importer, realm.Realm, batchImport: true);
@@ -665,7 +669,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realmFactory, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realmFactory);
+                var importer = new BeatmapImporter(storage, realmFactory);
                 using var store = new RealmRulesetStore(realmFactory, storage);
 
                 var imported = await LoadOszIntoStore(importer, realmFactory.Realm);
@@ -697,7 +701,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 var imported = await LoadOszIntoStore(importer, realm.Realm);
@@ -724,7 +728,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 var imported = await LoadOszIntoStore(importer, realm.Realm);
@@ -750,7 +754,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealm((realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 var metadata = new BeatmapMetadata
@@ -798,7 +802,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? temp = TestResources.GetTestBeatmapForImport();
@@ -815,7 +819,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? temp = TestResources.GetTestBeatmapForImport();
@@ -851,7 +855,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? temp = TestResources.GetTestBeatmapForImport();
@@ -893,7 +897,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? temp = TestResources.GetTestBeatmapForImport();
@@ -944,7 +948,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
-                using var importer = new BeatmapImporter(storage, realm);
+                var importer = new BeatmapImporter(storage, realm);
                 using var store = new RealmRulesetStore(realm, storage);
 
                 string? temp = TestResources.GetTestBeatmapForImport();

osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs

@@ -212,17 +212,17 @@ namespace osu.Game.Tests.Online
             {
             }
 
-            protected override BeatmapImporter CreateBeatmapImporter(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapOnlineLookupQueue onlineLookupQueue)
+            protected override BeatmapImporter CreateBeatmapImporter(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapUpdater beatmapUpdater)
             {
-                return new TestBeatmapImporter(this, storage, realm, onlineLookupQueue);
+                return new TestBeatmapImporter(this, storage, realm, beatmapUpdater);
             }
 
             internal class TestBeatmapImporter : BeatmapImporter
             {
                 private readonly TestBeatmapManager testBeatmapManager;
 
-                public TestBeatmapImporter(TestBeatmapManager testBeatmapManager, Storage storage, RealmAccess databaseAccess, BeatmapOnlineLookupQueue beatmapOnlineLookupQueue)
+                public TestBeatmapImporter(TestBeatmapManager testBeatmapManager, Storage storage, RealmAccess databaseAccess, BeatmapUpdater beatmapUpdater)
-                    : base(storage, databaseAccess, beatmapOnlineLookupQueue)
+                    : base(storage, databaseAccess, beatmapUpdater)
                 {
                     this.testBeatmapManager = testBeatmapManager;
                 }

osu.Game/Beatmaps/BeatmapImporter.cs

@@ -6,10 +6,8 @@ using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Threading;
-using osu.Framework.Audio.Track;
 using osu.Framework.Extensions;
 using osu.Framework.Extensions.IEnumerableExtensions;
-using osu.Framework.Graphics.Textures;
 using osu.Framework.Logging;
 using osu.Framework.Platform;
 using osu.Framework.Testing;
@@ -20,8 +18,6 @@ using osu.Game.IO;
 using osu.Game.IO.Archives;
 using osu.Game.Models;
 using osu.Game.Rulesets;
-using osu.Game.Rulesets.Objects;
-using osu.Game.Skinning;
 using Realms;
 
 namespace osu.Game.Beatmaps
@@ -30,18 +26,18 @@ namespace osu.Game.Beatmaps
     /// Handles the storage and retrieval of Beatmaps/WorkingBeatmaps.
     /// </summary>
     [ExcludeFromDynamicCompile]
-    public class BeatmapImporter : RealmArchiveModelImporter<BeatmapSetInfo>, IDisposable
+    public class BeatmapImporter : RealmArchiveModelImporter<BeatmapSetInfo>
     {
         public override IEnumerable<string> HandledExtensions => new[] { ".osz" };
 
         protected override string[] HashableFileTypes => new[] { ".osu" };
 
-        private readonly BeatmapOnlineLookupQueue? onlineLookupQueue;
+        private readonly BeatmapUpdater? beatmapUpdater;
 
-        public BeatmapImporter(Storage storage, RealmAccess realm, BeatmapOnlineLookupQueue? onlineLookupQueue = null)
+        public BeatmapImporter(Storage storage, RealmAccess realm, BeatmapUpdater? beatmapUpdater = null)
             : base(storage, realm)
         {
-            this.onlineLookupQueue = onlineLookupQueue;
+            this.beatmapUpdater = beatmapUpdater;
         }
 
         protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path).ToLowerInvariant() == ".osz";
@@ -65,8 +61,7 @@ namespace osu.Game.Beatmaps
 
             bool hadOnlineIDs = beatmapSet.Beatmaps.Any(b => b.OnlineID > 0);
 
-            onlineLookupQueue?.Update(beatmapSet);
+            // TODO: this may no longer be valid as we aren't doing an online population at this point.
-
             // ensure at least one beatmap was able to retrieve or keep an online ID, else drop the set ID.
             if (hadOnlineIDs && !beatmapSet.Beatmaps.Any(b => b.OnlineID > 0))
             {
@@ -85,6 +80,8 @@ namespace osu.Game.Beatmaps
             // If this is ever an issue, we can consider marking as pending delete but not resetting the IDs (but care will be required for
             // beatmaps, which don't have their own `DeletePending` state).
 
+            beatmapUpdater?.Process(beatmapSet, realm);
+
             if (beatmapSet.OnlineID > 0)
             {
                 var existingSetWithSameOnlineID = realm.All<BeatmapSetInfo>().SingleOrDefault(b => b.OnlineID == beatmapSet.OnlineID);
@@ -278,64 +275,11 @@ namespace osu.Game.Beatmaps
                         MD5Hash = memoryStream.ComputeMD5Hash(),
                     };
 
-                    updateBeatmapStatistics(beatmap, decoded);
-
                     beatmaps.Add(beatmap);
                 }
             }
 
             return beatmaps;
         }
-
-        private void updateBeatmapStatistics(BeatmapInfo beatmap, IBeatmap decoded)
-        {
-            var rulesetInstance = ((IRulesetInfo)beatmap.Ruleset).CreateInstance();
-
-            decoded.BeatmapInfo.Ruleset = rulesetInstance.RulesetInfo;
-
-            // TODO: this should be done in a better place once we actually need to dynamically update it.
-            beatmap.StarRating = rulesetInstance.CreateDifficultyCalculator(new DummyConversionBeatmap(decoded)).Calculate().StarRating;
-            beatmap.Length = calculateLength(decoded);
-            beatmap.BPM = 60000 / decoded.GetMostCommonBeatLength();
-        }
-
-        private double calculateLength(IBeatmap b)
-        {
-            if (!b.HitObjects.Any())
-                return 0;
-
-            var lastObject = b.HitObjects.Last();
-
-            //TODO: this isn't always correct (consider mania where a non-last object may last for longer than the last in the list).
-            double endTime = lastObject.GetEndTime();
-            double startTime = b.HitObjects.First().StartTime;
-
-            return endTime - startTime;
-        }
-
-        public void Dispose()
-        {
-            onlineLookupQueue?.Dispose();
-        }
-
-        /// <summary>
-        /// A dummy WorkingBeatmap for the purpose of retrieving a beatmap for star difficulty calculation.
-        /// </summary>
-        private class DummyConversionBeatmap : WorkingBeatmap
-        {
-            private readonly IBeatmap beatmap;
-
-            public DummyConversionBeatmap(IBeatmap beatmap)
-                : base(beatmap.BeatmapInfo, null)
-            {
-                this.beatmap = beatmap;
-            }
-
-            protected override IBeatmap GetBeatmap() => beatmap;
-            protected override Texture? GetBackground() => null;
-            protected override Track? GetBeatmapTrack() => null;
-            protected internal override ISkin? GetSkin() => null;
-            public override Stream? GetStream(string storagePath) => null;
-        }
     }
 }

osu.Game/Beatmaps/BeatmapManager.cs

@@ -42,9 +42,10 @@ namespace osu.Game.Beatmaps
 
         private readonly WorkingBeatmapCache workingBeatmapCache;
         private readonly BeatmapOnlineLookupQueue? onlineBeatmapLookupQueue;
+        private readonly BeatmapUpdater? beatmapUpdater;
 
         public BeatmapManager(Storage storage, RealmAccess realm, RulesetStore rulesets, IAPIProvider? api, AudioManager audioManager, IResourceStore<byte[]> gameResources, GameHost? host = null,
-                              WorkingBeatmap? defaultBeatmap = null, bool performOnlineLookups = false)
+                              WorkingBeatmap? defaultBeatmap = null, BeatmapDifficultyCache? difficultyCache = null, bool performOnlineLookups = false)
             : base(storage, realm)
         {
             if (performOnlineLookups)
@@ -52,14 +53,18 @@ namespace osu.Game.Beatmaps
                 if (api == null)
                     throw new ArgumentNullException(nameof(api), "API must be provided if online lookups are required.");
 
+                if (difficultyCache == null)
+                    throw new ArgumentNullException(nameof(difficultyCache), "Difficulty cache must be provided if online lookups are required.");
+
                 onlineBeatmapLookupQueue = new BeatmapOnlineLookupQueue(api, storage);
+                beatmapUpdater = new BeatmapUpdater(this, onlineBeatmapLookupQueue, difficultyCache);
             }
 
             var userResources = new RealmFileStore(realm, storage).Store;
 
             BeatmapTrackStore = audioManager.GetTrackStore(userResources);
 
-            beatmapImporter = CreateBeatmapImporter(storage, realm, rulesets, onlineBeatmapLookupQueue);
+            beatmapImporter = CreateBeatmapImporter(storage, realm, rulesets, beatmapUpdater);
             beatmapImporter.PostNotification = obj => PostNotification?.Invoke(obj);
 
             workingBeatmapCache = CreateWorkingBeatmapCache(audioManager, gameResources, userResources, defaultBeatmap, host);
@@ -71,8 +76,8 @@ namespace osu.Game.Beatmaps
             return new WorkingBeatmapCache(BeatmapTrackStore, audioManager, resources, storage, defaultBeatmap, host);
         }
 
-        protected virtual BeatmapImporter CreateBeatmapImporter(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapOnlineLookupQueue? onlineLookupQueue) =>
+        protected virtual BeatmapImporter CreateBeatmapImporter(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapUpdater? beatmapUpdater) =>
-            new BeatmapImporter(storage, realm, onlineLookupQueue);
+            new BeatmapImporter(storage, realm, beatmapUpdater);
 
         /// <summary>
         /// Create a new beatmap set, backed by a <see cref="BeatmapSetInfo"/> model,

osu.Game/Beatmaps/BeatmapUpdater.cs

@@ -0,0 +1,78 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+#nullable enable
+
+using System.Linq;
+using System.Threading.Tasks;
+using osu.Framework.Extensions;
+using osu.Game.Database;
+using osu.Game.Rulesets.Objects;
+
+namespace osu.Game.Beatmaps
+{
+    /// <summary>
+    /// Handles all processing required to ensure a local beatmap is in a consistent state with any changes.
+    /// </summary>
+    public class BeatmapUpdater
+    {
+        private readonly BeatmapManager beatmapManager;
+        private readonly BeatmapOnlineLookupQueue onlineLookupQueue;
+        private readonly BeatmapDifficultyCache difficultyCache;
+
+        public BeatmapUpdater(BeatmapManager beatmapManager, BeatmapOnlineLookupQueue onlineLookupQueue, BeatmapDifficultyCache difficultyCache)
+        {
+            this.beatmapManager = beatmapManager;
+            this.onlineLookupQueue = onlineLookupQueue;
+            this.difficultyCache = difficultyCache;
+        }
+
+        /// <summary>
+        /// Queue a beatmap for background processing.
+        /// </summary>
+        public void Queue(Live<BeatmapSetInfo> beatmap)
+        {
+            // For now, just fire off a task.
+            // TODO: Add actual queueing probably.
+            Task.Factory.StartNew(() => beatmap.PerformRead(Process));
+        }
+
+        /// <summary>
+        /// Run all processing on a beatmap immediately.
+        /// </summary>
+        public void Process(BeatmapSetInfo beatmapSet)
+        {
+            beatmapSet.Realm.Write(() =>
+            {
+                onlineLookupQueue.Update(beatmapSet);
+
+                foreach (var beatmap in beatmapSet.Beatmaps)
+                {
+                    var working = beatmapManager.GetWorkingBeatmap(beatmap);
+
+                    // Because we aren't guaranteed all processing will happen on this thread, it's very hard to use the live realm object.
+                    // This can be fixed by adding a synchronous flow to `BeatmapDifficultyCache`.
+                    var detachedBeatmap = beatmap.Detach();
+
+                    beatmap.StarRating = difficultyCache.GetDifficultyAsync(detachedBeatmap).GetResultSafely()?.Stars ?? 0;
+                    beatmap.Length = calculateLength(working.Beatmap);
+                    beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength();
+                }
+            });
+        }
+
+        private double calculateLength(IBeatmap b)
+        {
+            if (!b.HitObjects.Any())
+                return 0;
+
+            var lastObject = b.HitObjects.Last();
+
+            //TODO: this isn't always correct (consider mania where a non-last object may last for longer than the last in the list).
+            double endTime = lastObject.GetEndTime();
+            double startTime = b.HitObjects.First().StartTime;
+
+            return endTime - startTime;
+        }
+    }
+}

osu.Game/OsuGameBase.cs

@@ -272,7 +272,7 @@ namespace osu.Game
 
             // ordering is important here to ensure foreign keys rules are not broken in ModelStore.Cleanup()
             dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, realm, Scheduler, difficultyCache, LocalConfig));
-            dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, realm, RulesetStore, API, Audio, Resources, Host, defaultBeatmap, performOnlineLookups: true));
+            dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, realm, RulesetStore, API, Audio, Resources, Host, defaultBeatmap, difficultyCache, performOnlineLookups: true));
 
             dependencies.Cache(BeatmapDownloader = new BeatmapModelDownloader(BeatmapManager, API));
             dependencies.Cache(ScoreDownloader = new ScoreModelDownloader(ScoreManager, API));