Merge pull request #23189 from bdach/do-not-lookup-local-beatmaps

Do not look up metadata from online sources for locally-modified beatmaps on save

osu.Game/Beatmaps/BeatmapImporter.cs

@@ -35,7 +35,7 @@ namespace osu.Game.Beatmaps
 
         protected override string[] HashableFileTypes => new[] { ".osu" };
 
-        public Action<(BeatmapSetInfo beatmapSet, bool isBatch)>? ProcessBeatmap { private get; set; }
+        public ProcessBeatmapDelegate? ProcessBeatmap { private get; set; }
 
         public BeatmapImporter(Storage storage, RealmAccess realm)
             : base(storage, realm)
@@ -59,7 +59,7 @@ namespace osu.Game.Beatmaps
                 first.PerformRead(s =>
                 {
                     // Re-run processing even in this case. We might have outdated metadata.
-                    ProcessBeatmap?.Invoke((s, false));
+                    ProcessBeatmap?.Invoke(s, MetadataLookupScope.OnlineFirst);
                 });
                 return first;
             }
@@ -206,7 +206,7 @@ namespace osu.Game.Beatmaps
         protected override void PostImport(BeatmapSetInfo model, Realm realm, ImportParameters parameters)
         {
             base.PostImport(model, realm, parameters);
-            ProcessBeatmap?.Invoke((model, parameters.Batch));
+            ProcessBeatmap?.Invoke(model, parameters.Batch ? MetadataLookupScope.LocalCacheFirst : MetadataLookupScope.OnlineFirst);
         }
 
         private void validateOnlineIds(BeatmapSetInfo beatmapSet, Realm realm)

osu.Game/Beatmaps/BeatmapManager.cs

@@ -42,7 +42,7 @@ namespace osu.Game.Beatmaps
 
         private readonly WorkingBeatmapCache workingBeatmapCache;
 
-        public Action<(BeatmapSetInfo beatmapSet, bool isBatch)>? ProcessBeatmap { private get; set; }
+        public ProcessBeatmapDelegate? ProcessBeatmap { private get; set; }
 
         public override bool PauseImports
         {
@@ -72,7 +72,7 @@ namespace osu.Game.Beatmaps
             BeatmapTrackStore = audioManager.GetTrackStore(userResources);
 
             beatmapImporter = CreateBeatmapImporter(storage, realm);
-            beatmapImporter.ProcessBeatmap = args => ProcessBeatmap?.Invoke(args);
+            beatmapImporter.ProcessBeatmap = (beatmapSet, scope) => ProcessBeatmap?.Invoke(beatmapSet, scope);
             beatmapImporter.PostNotification = obj => PostNotification?.Invoke(obj);
 
             workingBeatmapCache = CreateWorkingBeatmapCache(audioManager, gameResources, userResources, defaultBeatmap, host);
@@ -454,7 +454,9 @@ namespace osu.Game.Beatmaps
                     if (transferCollections)
                         beatmapInfo.TransferCollectionReferences(r, oldMd5Hash);
 
-                    ProcessBeatmap?.Invoke((liveBeatmapSet, false));
+                    // do not look up metadata.
+                    // this is a locally-modified set now, so looking up metadata is busy work at best and harmful at worst.
+                    ProcessBeatmap?.Invoke(liveBeatmapSet, MetadataLookupScope.None);
                 });
             }
 
@@ -542,4 +544,11 @@ namespace osu.Game.Beatmaps
 
         public override string HumanisedModelName => "beatmap";
     }
+
+    /// <summary>
+    /// Delegate type for beatmap processing callbacks.
+    /// </summary>
+    /// <param name="beatmapSet">The beatmap set to be processed.</param>
+    /// <param name="lookupScope">The scope to use when looking up metadata.</param>
+    public delegate void ProcessBeatmapDelegate(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope);
 }

osu.Game/Beatmaps/BeatmapOnlineChangeIngest.cs

@@ -36,7 +36,7 @@ namespace osu.Game.Beatmaps
                     var matchingSet = r.All<BeatmapSetInfo>().FirstOrDefault(s => s.OnlineID == id);
 
                     if (matchingSet != null)
-                        beatmapUpdater.Queue(matchingSet.ToLive(realm), true);
+                        beatmapUpdater.Queue(matchingSet.ToLive(realm), MetadataLookupScope.OnlineFirst);
                 }
             });
         }

osu.Game/Beatmaps/BeatmapUpdater.cs

@@ -42,24 +42,25 @@ namespace osu.Game.Beatmaps
         /// Queue a beatmap for background processing.
         /// </summary>
         /// <param name="beatmapSet">The managed beatmap set to update. A transaction will be opened to apply changes.</param>
-        /// <param name="preferOnlineFetch">Whether metadata from an online source should be preferred. If <c>true</c>, the local cache will be skipped to ensure the freshest data state possible.</param>
+        /// <param name="lookupScope">The preferred scope to use for metadata lookup.</param>
-        public void Queue(Live<BeatmapSetInfo> beatmapSet, bool preferOnlineFetch = false)
+        public void Queue(Live<BeatmapSetInfo> beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst)
         {
             Logger.Log($"Queueing change for local beatmap {beatmapSet}");
-            Task.Factory.StartNew(() => beatmapSet.PerformRead(b => Process(b, preferOnlineFetch)), default, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
+            Task.Factory.StartNew(() => beatmapSet.PerformRead(b => Process(b, lookupScope)), default, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
         }
 
         /// <summary>
         /// Run all processing on a beatmap immediately.
         /// </summary>
         /// <param name="beatmapSet">The managed beatmap set to update. A transaction will be opened to apply changes.</param>
-        /// <param name="preferOnlineFetch">Whether metadata from an online source should be preferred. If <c>true</c>, the local cache will be skipped to ensure the freshest data state possible.</param>
+        /// <param name="lookupScope">The preferred scope to use for metadata lookup.</param>
-        public void Process(BeatmapSetInfo beatmapSet, bool preferOnlineFetch = false) => beatmapSet.Realm.Write(r =>
+        public void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) => beatmapSet.Realm.Write(r =>
         {
             // Before we use below, we want to invalidate.
             workingBeatmapCache.Invalidate(beatmapSet);
 
-            metadataLookup.Update(beatmapSet, preferOnlineFetch);
+            if (lookupScope != MetadataLookupScope.None)
+                metadataLookup.Update(beatmapSet, lookupScope == MetadataLookupScope.OnlineFirst);
 
             foreach (var beatmap in beatmapSet.Beatmaps)
             {

osu.Game/Beatmaps/MetadataLookupScope.cs

@@ -0,0 +1,26 @@
+// 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.
+
+namespace osu.Game.Beatmaps
+{
+    /// <summary>
+    /// Determines which sources (if any at all) should be queried in which order for a beatmap's metadata.
+    /// </summary>
+    public enum MetadataLookupScope
+    {
+        /// <summary>
+        /// Do not attempt to look up the beatmap metadata either in the local cache or online.
+        /// </summary>
+        None,
+
+        /// <summary>
+        /// Try the local metadata cache first before querying online sources.
+        /// </summary>
+        LocalCacheFirst,
+
+        /// <summary>
+        /// Query online sources immediately.
+        /// </summary>
+        OnlineFirst
+    }
+}

osu.Game/OsuGameBase.cs

@@ -310,7 +310,7 @@ namespace osu.Game
 
             base.Content.Add(new BeatmapOnlineChangeIngest(beatmapUpdater, realm, metadataClient));
 
-            BeatmapManager.ProcessBeatmap = args => beatmapUpdater.Process(args.beatmapSet, !args.isBatch);
+            BeatmapManager.ProcessBeatmap = (beatmapSet, scope) => beatmapUpdater.Process(beatmapSet, scope);
 
             dependencies.Cache(userCache = new UserLookupCache());
             base.Content.Add(userCache);