From 2ae5a34c0e3ca9cca42b2880e5f9c3300a40a2d9 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 28 Jul 2022 15:02:58 +0900
Subject: [PATCH 01/15] Add test coverage of beatmap updates transferring
 collection hashes

---
 .../Database/BeatmapImporterUpdateTests.cs    | 117 ++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs
index a82386fd51..90648c616e 100644
--- a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs
+++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs
@@ -9,6 +9,7 @@ using System.Linq.Expressions;
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Game.Beatmaps;
+using osu.Game.Collections;
 using osu.Game.Database;
 using osu.Game.Models;
 using osu.Game.Overlays.Notifications;
@@ -400,6 +401,122 @@ namespace osu.Game.Tests.Database
             });
         }
 
+        /// <summary>
+        /// If all difficulties in the original beatmap set are in a collection, presume the user also wants new difficulties added.
+        /// </summary>
+        [TestCase(false)]
+        [TestCase(true)]
+        public void TestCollectionTransferNewBeatmap(bool allOriginalBeatmapsInCollection)
+        {
+            RunTestWithRealmAsync(async (realm, storage) =>
+            {
+                var importer = new BeatmapImporter(storage, realm);
+                using var rulesets = new RealmRulesetStore(realm, storage);
+
+                using var __ = getBeatmapArchive(out string pathOriginal);
+                using var _ = getBeatmapArchiveWithModifications(out string pathMissingOneBeatmap, directory =>
+                {
+                    // remove one difficulty before first import
+                    directory.GetFiles("*.osu").First().Delete();
+                });
+
+                var importBeforeUpdate = await importer.Import(new ImportTask(pathMissingOneBeatmap));
+
+                Assert.That(importBeforeUpdate, Is.Not.Null);
+                Debug.Assert(importBeforeUpdate != null);
+
+                int beatmapsToAddToCollection = 0;
+
+                importBeforeUpdate.PerformWrite(s =>
+                {
+                    var beatmapCollection = s.Realm.Add(new BeatmapCollection("test collection"));
+                    beatmapsToAddToCollection = s.Beatmaps.Count - (allOriginalBeatmapsInCollection ? 0 : 1);
+
+                    for (int i = 0; i < beatmapsToAddToCollection; i++)
+                        beatmapCollection.BeatmapMD5Hashes.Add(s.Beatmaps[i].MD5Hash);
+                });
+
+                // Second import matches first but contains one extra .osu file.
+                var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), importBeforeUpdate.Value);
+
+                Assert.That(importAfterUpdate, Is.Not.Null);
+                Debug.Assert(importAfterUpdate != null);
+
+                importAfterUpdate.PerformRead(updated =>
+                {
+                    string[] hashes = updated.Realm.All<BeatmapCollection>().Single().BeatmapMD5Hashes.ToArray();
+
+                    if (allOriginalBeatmapsInCollection)
+                    {
+                        Assert.That(updated.Beatmaps.Count, Is.EqualTo(beatmapsToAddToCollection + 1));
+                        Assert.That(hashes, Has.Length.EqualTo(updated.Beatmaps.Count));
+                    }
+                    else
+                    {
+                        // Collection contains one less than the original beatmap, and two less after update (new difficulty included).
+                        Assert.That(updated.Beatmaps.Count, Is.EqualTo(beatmapsToAddToCollection + 2));
+                        Assert.That(hashes, Has.Length.EqualTo(beatmapsToAddToCollection));
+                    }
+                });
+            });
+        }
+
+        /// <summary>
+        /// If a difficulty in the original beatmap set is modified, the updated version should remain in any collections it was in.
+        /// </summary>
+        [Test]
+        public void TestCollectionTransferModifiedBeatmap()
+        {
+            RunTestWithRealmAsync(async (realm, storage) =>
+            {
+                var importer = new BeatmapImporter(storage, realm);
+                using var rulesets = new RealmRulesetStore(realm, storage);
+
+                using var __ = getBeatmapArchive(out string pathOriginal);
+                using var _ = getBeatmapArchiveWithModifications(out string pathModified, directory =>
+                {
+                    // Modify one .osu file with different content.
+                    var firstOsuFile = directory.GetFiles("*[Hard]*.osu").First();
+
+                    string existingContent = File.ReadAllText(firstOsuFile.FullName);
+
+                    File.WriteAllText(firstOsuFile.FullName, existingContent + "\n# I am new content");
+                });
+
+                var importBeforeUpdate = await importer.Import(new ImportTask(pathOriginal));
+
+                Assert.That(importBeforeUpdate, Is.Not.Null);
+                Debug.Assert(importBeforeUpdate != null);
+
+                string originalHash = string.Empty;
+
+                importBeforeUpdate.PerformWrite(s =>
+                {
+                    var beatmapCollection = s.Realm.Add(new BeatmapCollection("test collection"));
+                    originalHash = s.Beatmaps.Single(b => b.DifficultyName == "Hard").MD5Hash;
+
+                    beatmapCollection.BeatmapMD5Hashes.Add(originalHash);
+                });
+
+                // Second import matches first but contains one extra .osu file.
+                var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathModified), importBeforeUpdate.Value);
+
+                Assert.That(importAfterUpdate, Is.Not.Null);
+                Debug.Assert(importAfterUpdate != null);
+
+                importAfterUpdate.PerformRead(updated =>
+                {
+                    string[] hashes = updated.Realm.All<BeatmapCollection>().Single().BeatmapMD5Hashes.ToArray();
+                    string updatedHash = updated.Beatmaps.Single(b => b.DifficultyName == "Hard").MD5Hash;
+
+                    Assert.That(hashes, Has.Length.EqualTo(1));
+                    Assert.That(hashes.First(), Is.EqualTo(updatedHash));
+
+                    Assert.That(updatedHash, Is.Not.EqualTo(originalHash));
+                });
+            });
+        }
+
         private static void checkCount<T>(RealmAccess realm, int expected, Expression<Func<T, bool>>? condition = null) where T : RealmObject
         {
             var query = realm.Realm.All<T>();

From 2209afd0e8c82405701b8b6807d9a280cbf1aacc Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 28 Jul 2022 15:03:08 +0900
Subject: [PATCH 02/15] Mark `Live` methods as `InstantHandleAttribute`

---
 osu.Game/Database/Live.cs | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/osu.Game/Database/Live.cs b/osu.Game/Database/Live.cs
index e9f99e1e44..52e1d420f7 100644
--- a/osu.Game/Database/Live.cs
+++ b/osu.Game/Database/Live.cs
@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using JetBrains.Annotations;
 
 namespace osu.Game.Database
 {
@@ -18,19 +19,19 @@ namespace osu.Game.Database
         /// Perform a read operation on this live object.
         /// </summary>
         /// <param name="perform">The action to perform.</param>
-        public abstract void PerformRead(Action<T> perform);
+        public abstract void PerformRead([InstantHandle] Action<T> perform);
 
         /// <summary>
         /// Perform a read operation on this live object.
         /// </summary>
         /// <param name="perform">The action to perform.</param>
-        public abstract TReturn PerformRead<TReturn>(Func<T, TReturn> perform);
+        public abstract TReturn PerformRead<TReturn>([InstantHandle] Func<T, TReturn> perform);
 
         /// <summary>
         /// Perform a write operation on this live object.
         /// </summary>
         /// <param name="perform">The action to perform.</param>
-        public abstract void PerformWrite(Action<T> perform);
+        public abstract void PerformWrite([InstantHandle] Action<T> perform);
 
         /// <summary>
         /// Whether this instance is tracking data which is managed by the database backing.

From 070f56c30ce0ce51b3d30781b36fca5bf64e52a1 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 28 Jul 2022 15:03:23 +0900
Subject: [PATCH 03/15] Add collection transfer logic to beatmap
 import-as-update flow

---
 osu.Game/Beatmaps/BeatmapImporter.cs | 37 ++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs
index ef0e76234a..d1f7a5c6a8 100644
--- a/osu.Game/Beatmaps/BeatmapImporter.cs
+++ b/osu.Game/Beatmaps/BeatmapImporter.cs
@@ -14,6 +14,7 @@ using osu.Framework.Logging;
 using osu.Framework.Platform;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps.Formats;
+using osu.Game.Collections;
 using osu.Game.Database;
 using osu.Game.Extensions;
 using osu.Game.IO;
@@ -71,6 +72,8 @@ namespace osu.Game.Beatmaps
                 // Transfer local values which should be persisted across a beatmap update.
                 updated.DateAdded = original.DateAdded;
 
+                transferCollectionReferences(realm, original, updated);
+
                 foreach (var beatmap in original.Beatmaps.ToArray())
                 {
                     var updatedBeatmap = updated.Beatmaps.FirstOrDefault(b => b.Hash == beatmap.Hash);
@@ -112,6 +115,40 @@ namespace osu.Game.Beatmaps
             return first;
         }
 
+        private static void transferCollectionReferences(Realm realm, BeatmapSetInfo original, BeatmapSetInfo updated)
+        {
+            // First check if every beatmap in the original set is in any collections.
+            // In this case, we will assume they also want any newly added difficulties added to the collection.
+            foreach (var c in realm.All<BeatmapCollection>())
+            {
+                if (original.Beatmaps.Select(b => b.MD5Hash).All(c.BeatmapMD5Hashes.Contains))
+                {
+                    foreach (var b in original.Beatmaps)
+                        c.BeatmapMD5Hashes.Remove(b.MD5Hash);
+
+                    foreach (var b in updated.Beatmaps)
+                        c.BeatmapMD5Hashes.Add(b.MD5Hash);
+                }
+            }
+
+            // Handle collections using permissive difficulty name to track difficulties.
+            foreach (var originalBeatmap in original.Beatmaps)
+            {
+                var updatedBeatmap = updated.Beatmaps.FirstOrDefault(b => b.DifficultyName == originalBeatmap.DifficultyName);
+
+                if (updatedBeatmap == null)
+                    continue;
+
+                var collections = realm.All<BeatmapCollection>().AsEnumerable().Where(c => c.BeatmapMD5Hashes.Contains(originalBeatmap.MD5Hash));
+
+                foreach (var c in collections)
+                {
+                    c.BeatmapMD5Hashes.Remove(originalBeatmap.MD5Hash);
+                    c.BeatmapMD5Hashes.Add(updatedBeatmap.MD5Hash);
+                }
+            }
+        }
+
         protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path).ToLowerInvariant() == ".osz";
 
         protected override void Populate(BeatmapSetInfo beatmapSet, ArchiveReader? archive, Realm realm, CancellationToken cancellationToken = default)

From 67c44552cb25f00ae7b46d87fb811956b5e4902b Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 28 Jul 2022 15:18:04 +0900
Subject: [PATCH 04/15] Add realm `Refresh` calls to ensure tests are stable

---
 osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs
index 90648c616e..07bd08d326 100644
--- a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs
+++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs
@@ -444,6 +444,8 @@ namespace osu.Game.Tests.Database
 
                 importAfterUpdate.PerformRead(updated =>
                 {
+                    updated.Realm.Refresh();
+
                     string[] hashes = updated.Realm.All<BeatmapCollection>().Single().BeatmapMD5Hashes.ToArray();
 
                     if (allOriginalBeatmapsInCollection)
@@ -506,6 +508,8 @@ namespace osu.Game.Tests.Database
 
                 importAfterUpdate.PerformRead(updated =>
                 {
+                    updated.Realm.Refresh();
+
                     string[] hashes = updated.Realm.All<BeatmapCollection>().Single().BeatmapMD5Hashes.ToArray();
                     string updatedHash = updated.Beatmaps.Single(b => b.DifficultyName == "Hard").MD5Hash;
 

From 17a3fd30fbf535cfb16cc8d22e65b9f16fc4724f Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 28 Jul 2022 16:08:27 +0900
Subject: [PATCH 05/15] Move scheduler from `OnlineLookupQueue` to
 `BeatmapUpdater`

---
 osu.Game/Beatmaps/BeatmapUpdater.cs           | 22 ++++++++++-----
 ...eue.cs => BeatmapUpdaterMetadataLookup.cs} | 27 ++++---------------
 2 files changed, 21 insertions(+), 28 deletions(-)
 rename osu.Game/Beatmaps/{BeatmapOnlineLookupQueue.cs => BeatmapUpdaterMetadataLookup.cs} (85%)

diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs
index d2c5e5616a..5ffe4ee291 100644
--- a/osu.Game/Beatmaps/BeatmapUpdater.cs
+++ b/osu.Game/Beatmaps/BeatmapUpdater.cs
@@ -8,6 +8,7 @@ using System.Threading.Tasks;
 using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Logging;
 using osu.Framework.Platform;
+using osu.Framework.Threading;
 using osu.Game.Database;
 using osu.Game.Online.API;
 using osu.Game.Rulesets.Objects;
@@ -20,15 +21,21 @@ namespace osu.Game.Beatmaps
     public class BeatmapUpdater : IDisposable
     {
         private readonly IWorkingBeatmapCache workingBeatmapCache;
-        private readonly BeatmapOnlineLookupQueue onlineLookupQueue;
+
         private readonly BeatmapDifficultyCache difficultyCache;
 
+        private readonly BeatmapUpdaterMetadataLookup metadataLookup;
+
+        private const int update_queue_request_concurrency = 4;
+
+        private readonly ThreadedTaskScheduler updateScheduler = new ThreadedTaskScheduler(update_queue_request_concurrency, nameof(BeatmapUpdaterMetadataLookup));
+
         public BeatmapUpdater(IWorkingBeatmapCache workingBeatmapCache, BeatmapDifficultyCache difficultyCache, IAPIProvider api, Storage storage)
         {
             this.workingBeatmapCache = workingBeatmapCache;
             this.difficultyCache = difficultyCache;
 
-            onlineLookupQueue = new BeatmapOnlineLookupQueue(api, storage);
+            metadataLookup = new BeatmapUpdaterMetadataLookup(api, storage);
         }
 
         /// <summary>
@@ -37,7 +44,7 @@ namespace osu.Game.Beatmaps
         public void Queue(Live<BeatmapSetInfo> beatmap)
         {
             Logger.Log($"Queueing change for local beatmap {beatmap}");
-            Task.Factory.StartNew(() => beatmap.PerformRead(Process));
+            Task.Factory.StartNew(() => beatmap.PerformRead(b => Process(b)), default, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
         }
 
         /// <summary>
@@ -50,7 +57,7 @@ namespace osu.Game.Beatmaps
 
             // TODO: this call currently uses the local `online.db` lookup.
             // We probably don't want this to happen after initial import (as the data may be stale).
-            onlineLookupQueue.Update(beatmapSet);
+            metadataLookup.Update(beatmapSet);
 
             foreach (var beatmap in beatmapSet.Beatmaps)
             {
@@ -90,8 +97,11 @@ namespace osu.Game.Beatmaps
 
         public void Dispose()
         {
-            if (onlineLookupQueue.IsNotNull())
-                onlineLookupQueue.Dispose();
+            if (metadataLookup.IsNotNull())
+                metadataLookup.Dispose();
+
+            if (updateScheduler.IsNotNull())
+                updateScheduler.Dispose();
         }
 
         #endregion
diff --git a/osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs
similarity index 85%
rename from osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs
rename to osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs
index 6a3383cc92..94846599e8 100644
--- a/osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs
+++ b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs
@@ -6,8 +6,6 @@
 using System;
 using System.Diagnostics;
 using System.IO;
-using System.Linq;
-using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Data.Sqlite;
 using osu.Framework.Development;
@@ -15,7 +13,6 @@ using osu.Framework.IO.Network;
 using osu.Framework.Logging;
 using osu.Framework.Platform;
 using osu.Framework.Testing;
-using osu.Framework.Threading;
 using osu.Game.Database;
 using osu.Game.Online.API;
 using osu.Game.Online.API.Requests;
@@ -32,20 +29,16 @@ namespace osu.Game.Beatmaps
     /// This will always be checked before doing a second online query to get required metadata.
     /// </remarks>
     [ExcludeFromDynamicCompile]
-    public class BeatmapOnlineLookupQueue : IDisposable
+    public class BeatmapUpdaterMetadataLookup : IDisposable
     {
         private readonly IAPIProvider api;
         private readonly Storage storage;
 
-        private const int update_queue_request_concurrency = 4;
-
-        private readonly ThreadedTaskScheduler updateScheduler = new ThreadedTaskScheduler(update_queue_request_concurrency, nameof(BeatmapOnlineLookupQueue));
-
         private FileWebRequest cacheDownloadRequest;
 
         private const string cache_database_name = "online.db";
 
-        public BeatmapOnlineLookupQueue(IAPIProvider api, Storage storage)
+        public BeatmapUpdaterMetadataLookup(IAPIProvider api, Storage storage)
         {
             this.api = api;
             this.storage = storage;
@@ -61,15 +54,6 @@ namespace osu.Game.Beatmaps
                 lookup(beatmapSet, b);
         }
 
-        public Task UpdateAsync(BeatmapSetInfo beatmapSet, CancellationToken cancellationToken)
-        {
-            return Task.WhenAll(beatmapSet.Beatmaps.Select(b => UpdateAsync(beatmapSet, b, cancellationToken)).ToArray());
-        }
-
-        // todo: expose this when we need to do individual difficulty lookups.
-        protected Task UpdateAsync(BeatmapSetInfo beatmapSet, BeatmapInfo beatmapInfo, CancellationToken cancellationToken)
-            => Task.Factory.StartNew(() => lookup(beatmapSet, beatmapInfo), cancellationToken, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
-
         private void lookup(BeatmapSetInfo set, BeatmapInfo beatmapInfo)
         {
             if (checkLocalCache(set, beatmapInfo))
@@ -134,7 +118,7 @@ namespace osu.Game.Beatmaps
                 File.Delete(compressedCacheFilePath);
                 File.Delete(cacheFilePath);
 
-                Logger.Log($"{nameof(BeatmapOnlineLookupQueue)}'s online cache download failed: {ex}", LoggingTarget.Database);
+                Logger.Log($"{nameof(BeatmapUpdaterMetadataLookup)}'s online cache download failed: {ex}", LoggingTarget.Database);
             };
 
             cacheDownloadRequest.Finished += () =>
@@ -151,7 +135,7 @@ namespace osu.Game.Beatmaps
                 }
                 catch (Exception ex)
                 {
-                    Logger.Log($"{nameof(BeatmapOnlineLookupQueue)}'s online cache extraction failed: {ex}", LoggingTarget.Database);
+                    Logger.Log($"{nameof(BeatmapUpdaterMetadataLookup)}'s online cache extraction failed: {ex}", LoggingTarget.Database);
                     File.Delete(cacheFilePath);
                 }
                 finally
@@ -238,12 +222,11 @@ namespace osu.Game.Beatmaps
         }
 
         private void logForModel(BeatmapSetInfo set, string message) =>
-            RealmArchiveModelImporter<BeatmapSetInfo>.LogForModel(set, $"[{nameof(BeatmapOnlineLookupQueue)}] {message}");
+            RealmArchiveModelImporter<BeatmapSetInfo>.LogForModel(set, $"[{nameof(BeatmapUpdaterMetadataLookup)}] {message}");
 
         public void Dispose()
         {
             cacheDownloadRequest?.Dispose();
-            updateScheduler?.Dispose();
         }
     }
 }

From c35da6222469b366890960a89d4c33d253ebd916 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 28 Jul 2022 16:18:30 +0900
Subject: [PATCH 06/15] Add flow for bypassing local cache lookups when
 refreshing beatmap metadata

---
 osu.Game/Beatmaps/BeatmapImporter.cs              |  8 ++++----
 osu.Game/Beatmaps/BeatmapManager.cs               |  6 +++---
 osu.Game/Beatmaps/BeatmapOnlineChangeIngest.cs    |  2 +-
 osu.Game/Beatmaps/BeatmapUpdater.cs               | 10 ++++------
 osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs |  8 ++++----
 osu.Game/Database/RealmArchiveModelImporter.cs    |  5 +++--
 osu.Game/OsuGameBase.cs                           |  2 +-
 osu.Game/Scoring/ScoreImporter.cs                 |  4 ++--
 8 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs
index ef0e76234a..df8b18313c 100644
--- a/osu.Game/Beatmaps/BeatmapImporter.cs
+++ b/osu.Game/Beatmaps/BeatmapImporter.cs
@@ -34,7 +34,7 @@ namespace osu.Game.Beatmaps
 
         protected override string[] HashableFileTypes => new[] { ".osu" };
 
-        public Action<BeatmapSetInfo>? ProcessBeatmap { private get; set; }
+        public Action<BeatmapSetInfo, bool>? ProcessBeatmap { private get; set; }
 
         public BeatmapImporter(Storage storage, RealmAccess realm)
             : base(storage, realm)
@@ -168,11 +168,11 @@ namespace osu.Game.Beatmaps
             }
         }
 
-        protected override void PostImport(BeatmapSetInfo model, Realm realm)
+        protected override void PostImport(BeatmapSetInfo model, Realm realm, bool batchImport)
         {
-            base.PostImport(model, realm);
+            base.PostImport(model, realm, batchImport);
 
-            ProcessBeatmap?.Invoke(model);
+            ProcessBeatmap?.Invoke(model, batchImport);
         }
 
         private void validateOnlineIds(BeatmapSetInfo beatmapSet, Realm realm)
diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs
index debe4c6829..abf3d43d94 100644
--- a/osu.Game/Beatmaps/BeatmapManager.cs
+++ b/osu.Game/Beatmaps/BeatmapManager.cs
@@ -42,7 +42,7 @@ namespace osu.Game.Beatmaps
 
         private readonly WorkingBeatmapCache workingBeatmapCache;
 
-        public Action<BeatmapSetInfo>? ProcessBeatmap { private get; set; }
+        public Action<BeatmapSetInfo, bool>? ProcessBeatmap { private get; set; }
 
         public BeatmapManager(Storage storage, RealmAccess realm, RulesetStore rulesets, IAPIProvider? api, AudioManager audioManager, IResourceStore<byte[]> gameResources, GameHost? host = null,
                               WorkingBeatmap? defaultBeatmap = null, BeatmapDifficultyCache? difficultyCache = null, bool performOnlineLookups = false)
@@ -62,7 +62,7 @@ namespace osu.Game.Beatmaps
             BeatmapTrackStore = audioManager.GetTrackStore(userResources);
 
             beatmapImporter = CreateBeatmapImporter(storage, realm);
-            beatmapImporter.ProcessBeatmap = obj => ProcessBeatmap?.Invoke(obj);
+            beatmapImporter.ProcessBeatmap = (obj, isBatch) => ProcessBeatmap?.Invoke(obj, isBatch);
             beatmapImporter.PostNotification = obj => PostNotification?.Invoke(obj);
 
             workingBeatmapCache = CreateWorkingBeatmapCache(audioManager, gameResources, userResources, defaultBeatmap, host);
@@ -323,7 +323,7 @@ namespace osu.Game.Beatmaps
 
                     setInfo.CopyChangesToRealm(liveBeatmapSet);
 
-                    ProcessBeatmap?.Invoke(liveBeatmapSet);
+                    ProcessBeatmap?.Invoke(liveBeatmapSet, false);
                 });
             }
 
diff --git a/osu.Game/Beatmaps/BeatmapOnlineChangeIngest.cs b/osu.Game/Beatmaps/BeatmapOnlineChangeIngest.cs
index b6968f4e06..5d0765641b 100644
--- a/osu.Game/Beatmaps/BeatmapOnlineChangeIngest.cs
+++ b/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));
+                        beatmapUpdater.Queue(matchingSet.ToLive(realm), true);
                 }
             });
         }
diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs
index 5ffe4ee291..a86aab4ac1 100644
--- a/osu.Game/Beatmaps/BeatmapUpdater.cs
+++ b/osu.Game/Beatmaps/BeatmapUpdater.cs
@@ -41,23 +41,21 @@ namespace osu.Game.Beatmaps
         /// <summary>
         /// Queue a beatmap for background processing.
         /// </summary>
-        public void Queue(Live<BeatmapSetInfo> beatmap)
+        public void Queue(Live<BeatmapSetInfo> beatmap, bool forceOnlineFetch = false)
         {
             Logger.Log($"Queueing change for local beatmap {beatmap}");
-            Task.Factory.StartNew(() => beatmap.PerformRead(b => Process(b)), default, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
+            Task.Factory.StartNew(() => beatmap.PerformRead(b => Process(b, forceOnlineFetch)), default, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
         }
 
         /// <summary>
         /// Run all processing on a beatmap immediately.
         /// </summary>
-        public void Process(BeatmapSetInfo beatmapSet) => beatmapSet.Realm.Write(r =>
+        public void Process(BeatmapSetInfo beatmapSet, bool forceOnlineFetch = false) => beatmapSet.Realm.Write(r =>
         {
             // Before we use below, we want to invalidate.
             workingBeatmapCache.Invalidate(beatmapSet);
 
-            // TODO: this call currently uses the local `online.db` lookup.
-            // We probably don't want this to happen after initial import (as the data may be stale).
-            metadataLookup.Update(beatmapSet);
+            metadataLookup.Update(beatmapSet, forceOnlineFetch);
 
             foreach (var beatmap in beatmapSet.Beatmaps)
             {
diff --git a/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs
index 94846599e8..c7c8f0ceb0 100644
--- a/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs
+++ b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs
@@ -48,15 +48,15 @@ namespace osu.Game.Beatmaps
                 prepareLocalCache();
         }
 
-        public void Update(BeatmapSetInfo beatmapSet)
+        public void Update(BeatmapSetInfo beatmapSet, bool forceOnlineFetch)
         {
             foreach (var b in beatmapSet.Beatmaps)
-                lookup(beatmapSet, b);
+                lookup(beatmapSet, b, forceOnlineFetch);
         }
 
-        private void lookup(BeatmapSetInfo set, BeatmapInfo beatmapInfo)
+        private void lookup(BeatmapSetInfo set, BeatmapInfo beatmapInfo, bool forceOnlineFetch)
         {
-            if (checkLocalCache(set, beatmapInfo))
+            if (!forceOnlineFetch && checkLocalCache(set, beatmapInfo))
                 return;
 
             if (api?.State.Value != APIState.Online)
diff --git a/osu.Game/Database/RealmArchiveModelImporter.cs b/osu.Game/Database/RealmArchiveModelImporter.cs
index a0cf98b978..b340d0ee4b 100644
--- a/osu.Game/Database/RealmArchiveModelImporter.cs
+++ b/osu.Game/Database/RealmArchiveModelImporter.cs
@@ -340,7 +340,7 @@ namespace osu.Game.Database
                     // import to store
                     realm.Add(item);
 
-                    PostImport(item, realm);
+                    PostImport(item, realm, batchImport);
 
                     transaction.Commit();
                 }
@@ -485,7 +485,8 @@ namespace osu.Game.Database
         /// </summary>
         /// <param name="model">The model prepared for import.</param>
         /// <param name="realm">The current realm context.</param>
-        protected virtual void PostImport(TModel model, Realm realm)
+        /// <param name="batchImport">Whether the import was part of a batch.</param>
+        protected virtual void PostImport(TModel model, Realm realm, bool batchImport)
         {
         }
 
diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs
index 7b6cda17a2..6514fc6aee 100644
--- a/osu.Game/OsuGameBase.cs
+++ b/osu.Game/OsuGameBase.cs
@@ -287,7 +287,7 @@ namespace osu.Game
 
             AddInternal(new BeatmapOnlineChangeIngest(beatmapUpdater, realm, metadataClient));
 
-            BeatmapManager.ProcessBeatmap = set => beatmapUpdater.Process(set);
+            BeatmapManager.ProcessBeatmap = (set, isBatch) => beatmapUpdater.Process(set, forceOnlineFetch: !isBatch);
 
             dependencies.Cache(userCache = new UserLookupCache());
             AddInternal(userCache);
diff --git a/osu.Game/Scoring/ScoreImporter.cs b/osu.Game/Scoring/ScoreImporter.cs
index 4107c66dfe..0902f1636b 100644
--- a/osu.Game/Scoring/ScoreImporter.cs
+++ b/osu.Game/Scoring/ScoreImporter.cs
@@ -75,9 +75,9 @@ namespace osu.Game.Scoring
                 model.StatisticsJson = JsonConvert.SerializeObject(model.Statistics);
         }
 
-        protected override void PostImport(ScoreInfo model, Realm realm)
+        protected override void PostImport(ScoreInfo model, Realm realm, bool batchImport)
         {
-            base.PostImport(model, realm);
+            base.PostImport(model, realm, batchImport);
 
             var userRequest = new GetUserRequest(model.RealmUser.Username);
 

From 6d4023b933dd97a65b9f3abe977f1671de8b91d0 Mon Sep 17 00:00:00 2001
From: Dan Balasescu <smoogipoo@smgi.me>
Date: Thu, 28 Jul 2022 16:56:11 +0900
Subject: [PATCH 07/15] Adjust comment

---
 osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs
index 07bd08d326..76339d4a1c 100644
--- a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs
+++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs
@@ -500,7 +500,7 @@ namespace osu.Game.Tests.Database
                     beatmapCollection.BeatmapMD5Hashes.Add(originalHash);
                 });
 
-                // Second import matches first but contains one extra .osu file.
+                // Second import matches first but contains a modified .osu file.
                 var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathModified), importBeforeUpdate.Value);
 
                 Assert.That(importAfterUpdate, Is.Not.Null);

From 8cb4fb35e005b414fd5055dd080e6f74313cf957 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 28 Jul 2022 16:55:46 +0900
Subject: [PATCH 08/15] Rename parameter to read better (and still use local
 cache if no online API is available)

---
 osu.Game/Beatmaps/BeatmapUpdater.cs           | 14 +++++++++-----
 .../Beatmaps/BeatmapUpdaterMetadataLookup.cs  | 19 ++++++++++++++-----
 osu.Game/OsuGameBase.cs                       |  2 +-
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs
index a86aab4ac1..d7b1fac7b3 100644
--- a/osu.Game/Beatmaps/BeatmapUpdater.cs
+++ b/osu.Game/Beatmaps/BeatmapUpdater.cs
@@ -41,21 +41,25 @@ namespace osu.Game.Beatmaps
         /// <summary>
         /// Queue a beatmap for background processing.
         /// </summary>
-        public void Queue(Live<BeatmapSetInfo> beatmap, bool forceOnlineFetch = false)
+        /// <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>
+        public void Queue(Live<BeatmapSetInfo> beatmapSet, bool preferOnlineFetch = false)
         {
-            Logger.Log($"Queueing change for local beatmap {beatmap}");
-            Task.Factory.StartNew(() => beatmap.PerformRead(b => Process(b, forceOnlineFetch)), default, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
+            Logger.Log($"Queueing change for local beatmap {beatmapSet}");
+            Task.Factory.StartNew(() => beatmapSet.PerformRead(b => Process(b, preferOnlineFetch)), default, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
         }
 
         /// <summary>
         /// Run all processing on a beatmap immediately.
         /// </summary>
-        public void Process(BeatmapSetInfo beatmapSet, bool forceOnlineFetch = false) => beatmapSet.Realm.Write(r =>
+        /// <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>
+        public void Process(BeatmapSetInfo beatmapSet, bool preferOnlineFetch = false) => beatmapSet.Realm.Write(r =>
         {
             // Before we use below, we want to invalidate.
             workingBeatmapCache.Invalidate(beatmapSet);
 
-            metadataLookup.Update(beatmapSet, forceOnlineFetch);
+            metadataLookup.Update(beatmapSet, preferOnlineFetch);
 
             foreach (var beatmap in beatmapSet.Beatmaps)
             {
diff --git a/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs
index c7c8f0ceb0..02fb69b8f5 100644
--- a/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs
+++ b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs
@@ -48,18 +48,27 @@ namespace osu.Game.Beatmaps
                 prepareLocalCache();
         }
 
-        public void Update(BeatmapSetInfo beatmapSet, bool forceOnlineFetch)
+        /// <summary>
+        /// Queue an update for a beatmap set.
+        /// </summary>
+        /// <param name="beatmapSet">The beatmap set to update. Updates will be applied directly (so a transaction should be started if this instance is managed).</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>
+        public void Update(BeatmapSetInfo beatmapSet, bool preferOnlineFetch)
         {
             foreach (var b in beatmapSet.Beatmaps)
-                lookup(beatmapSet, b, forceOnlineFetch);
+                lookup(beatmapSet, b, preferOnlineFetch);
         }
 
-        private void lookup(BeatmapSetInfo set, BeatmapInfo beatmapInfo, bool forceOnlineFetch)
+        private void lookup(BeatmapSetInfo set, BeatmapInfo beatmapInfo, bool preferOnlineFetch)
         {
-            if (!forceOnlineFetch && checkLocalCache(set, beatmapInfo))
+            bool apiAvailable = api?.State.Value == APIState.Online;
+
+            bool useLocalCache = !apiAvailable || !preferOnlineFetch;
+
+            if (useLocalCache && checkLocalCache(set, beatmapInfo))
                 return;
 
-            if (api?.State.Value != APIState.Online)
+            if (!apiAvailable)
                 return;
 
             var req = new GetBeatmapRequest(beatmapInfo);
diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs
index 6514fc6aee..41eeece76d 100644
--- a/osu.Game/OsuGameBase.cs
+++ b/osu.Game/OsuGameBase.cs
@@ -287,7 +287,7 @@ namespace osu.Game
 
             AddInternal(new BeatmapOnlineChangeIngest(beatmapUpdater, realm, metadataClient));
 
-            BeatmapManager.ProcessBeatmap = (set, isBatch) => beatmapUpdater.Process(set, forceOnlineFetch: !isBatch);
+            BeatmapManager.ProcessBeatmap = (set, isBatch) => beatmapUpdater.Process(set, preferOnlineFetch: !isBatch);
 
             dependencies.Cache(userCache = new UserLookupCache());
             AddInternal(userCache);

From 1039338d804dd33faf12ec8bc96b361cad280de4 Mon Sep 17 00:00:00 2001
From: Dan Balasescu <smoogipoo@smgi.me>
Date: Thu, 28 Jul 2022 17:58:07 +0900
Subject: [PATCH 09/15] Fix intermittent HUD tests

---
 osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs | 1 +
 osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs
index dd0f965914..fb97f94dbb 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs
@@ -159,6 +159,7 @@ namespace osu.Game.Tests.Visual.Gameplay
 
             AddUntilStep("wait for hud load", () => hudOverlay.IsLoaded);
             AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType<SkinnableTargetContainer>().Single().Alpha == 0);
+            AddUntilStep("wait for hud load", () => hudOverlay.ChildrenOfType<SkinnableTargetContainer>().All(c => c.ComponentsLoaded));
 
             AddStep("bind on update", () =>
             {
diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs
index f319290441..bd274dfef5 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditor.cs
@@ -14,6 +14,7 @@ using osu.Game.Overlays.Settings;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Screens.Play.HUD.HitErrorMeters;
+using osu.Game.Skinning;
 using osu.Game.Skinning.Editor;
 using osuTK.Input;
 
@@ -33,6 +34,8 @@ namespace osu.Game.Tests.Visual.Gameplay
         {
             base.SetUpSteps();
 
+            AddUntilStep("wait for hud load", () => Player.ChildrenOfType<SkinnableTargetContainer>().All(c => c.ComponentsLoaded));
+
             AddStep("reload skin editor", () =>
             {
                 skinEditor?.Expire();

From ce694123ebeb2b0364c149a28e8f290828aed30d Mon Sep 17 00:00:00 2001
From: Dan Balasescu <smoogipoo@smgi.me>
Date: Thu, 28 Jul 2022 20:44:02 +0900
Subject: [PATCH 10/15] Move spectator begin/end playing to SubmittingPlayer

---
 .../Gameplay/TestSceneSpectatorPlayback.cs    | 59 +++++++++----------
 osu.Game/Rulesets/UI/ReplayRecorder.cs        | 12 ----
 osu.Game/Screens/Play/Player.cs               |  9 ---
 osu.Game/Screens/Play/SubmittingPlayer.cs     |  9 +++
 4 files changed, 37 insertions(+), 52 deletions(-)

diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs
index 5fad661e9b..9c41c70a0e 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs
@@ -27,7 +27,6 @@ using osu.Game.Rulesets.Replays;
 using osu.Game.Rulesets.Replays.Types;
 using osu.Game.Rulesets.UI;
 using osu.Game.Scoring;
-using osu.Game.Screens.Play;
 using osu.Game.Tests.Gameplay;
 using osu.Game.Tests.Mods;
 using osu.Game.Tests.Visual.Spectator;
@@ -41,16 +40,12 @@ namespace osu.Game.Tests.Visual.Gameplay
         private TestRulesetInputManager playbackManager;
         private TestRulesetInputManager recordingManager;
 
-        private Replay replay;
-
+        private Score recordingScore;
+        private Replay playbackReplay;
         private TestSpectatorClient spectatorClient;
-
         private ManualClock manualClock;
-
         private TestReplayRecorder recorder;
-
         private OsuSpriteText latencyDisplay;
-
         private TestFramedReplayInputHandler replayHandler;
 
         [SetUpSteps]
@@ -58,7 +53,16 @@ namespace osu.Game.Tests.Visual.Gameplay
         {
             AddStep("Setup containers", () =>
             {
-                replay = new Replay();
+                recordingScore = new Score
+                {
+                    ScoreInfo =
+                    {
+                        BeatmapInfo = new BeatmapInfo(),
+                        Ruleset = new OsuRuleset().RulesetInfo,
+                    }
+                };
+
+                playbackReplay = new Replay();
                 manualClock = new ManualClock();
 
                 Child = new DependencyProvidingContainer
@@ -67,7 +71,6 @@ namespace osu.Game.Tests.Visual.Gameplay
                     CachedDependencies = new[]
                     {
                         (typeof(SpectatorClient), (object)(spectatorClient = new TestSpectatorClient())),
-                        (typeof(GameplayState), TestGameplayState.Create(new OsuRuleset()))
                     },
                     Children = new Drawable[]
                     {
@@ -81,7 +84,7 @@ namespace osu.Game.Tests.Visual.Gameplay
                                 {
                                     recordingManager = new TestRulesetInputManager(TestCustomisableModRuleset.CreateTestRulesetInfo(), 0, SimultaneousBindingMode.Unique)
                                     {
-                                        Recorder = recorder = new TestReplayRecorder
+                                        Recorder = recorder = new TestReplayRecorder(recordingScore)
                                         {
                                             ScreenSpaceToGamefield = pos => recordingManager.ToLocalSpace(pos),
                                         },
@@ -112,7 +115,7 @@ namespace osu.Game.Tests.Visual.Gameplay
                                     playbackManager = new TestRulesetInputManager(TestCustomisableModRuleset.CreateTestRulesetInfo(), 0, SimultaneousBindingMode.Unique)
                                     {
                                         Clock = new FramedClock(manualClock),
-                                        ReplayInputHandler = replayHandler = new TestFramedReplayInputHandler(replay)
+                                        ReplayInputHandler = replayHandler = new TestFramedReplayInputHandler(playbackReplay)
                                         {
                                             GamefieldToScreenSpace = pos => playbackManager.ToScreenSpace(pos),
                                         },
@@ -144,6 +147,7 @@ namespace osu.Game.Tests.Visual.Gameplay
                     }
                 };
 
+                spectatorClient.BeginPlaying(TestGameplayState.Create(new OsuRuleset()), recordingScore);
                 spectatorClient.OnNewFrames += onNewFrames;
             });
         }
@@ -151,15 +155,15 @@ namespace osu.Game.Tests.Visual.Gameplay
         [Test]
         public void TestBasic()
         {
-            AddUntilStep("received frames", () => replay.Frames.Count > 50);
+            AddUntilStep("received frames", () => playbackReplay.Frames.Count > 50);
             AddStep("stop sending frames", () => recorder.Expire());
-            AddUntilStep("wait for all frames received", () => replay.Frames.Count == recorder.SentFrames.Count);
+            AddUntilStep("wait for all frames received", () => playbackReplay.Frames.Count == recorder.SentFrames.Count);
         }
 
         [Test]
         public void TestWithSendFailure()
         {
-            AddUntilStep("received frames", () => replay.Frames.Count > 50);
+            AddUntilStep("received frames", () => playbackReplay.Frames.Count > 50);
 
             int framesReceivedSoFar = 0;
             int frameSendAttemptsSoFar = 0;
@@ -172,21 +176,21 @@ namespace osu.Game.Tests.Visual.Gameplay
 
             AddUntilStep("wait for next send attempt", () =>
             {
-                framesReceivedSoFar = replay.Frames.Count;
+                framesReceivedSoFar = playbackReplay.Frames.Count;
                 return spectatorClient.FrameSendAttempts > frameSendAttemptsSoFar + 1;
             });
 
             AddUntilStep("wait for more send attempts", () => spectatorClient.FrameSendAttempts > frameSendAttemptsSoFar + 10);
-            AddAssert("frames did not increase", () => framesReceivedSoFar == replay.Frames.Count);
+            AddAssert("frames did not increase", () => framesReceivedSoFar == playbackReplay.Frames.Count);
 
             AddStep("stop failing sends", () => spectatorClient.ShouldFailSendingFrames = false);
 
-            AddUntilStep("wait for next frames", () => framesReceivedSoFar < replay.Frames.Count);
+            AddUntilStep("wait for next frames", () => framesReceivedSoFar < playbackReplay.Frames.Count);
 
             AddStep("stop sending frames", () => recorder.Expire());
 
-            AddUntilStep("wait for all frames received", () => replay.Frames.Count == recorder.SentFrames.Count);
-            AddAssert("ensure frames were received in the correct sequence", () => replay.Frames.Select(f => f.Time).SequenceEqual(recorder.SentFrames.Select(f => f.Time)));
+            AddUntilStep("wait for all frames received", () => playbackReplay.Frames.Count == recorder.SentFrames.Count);
+            AddAssert("ensure frames were received in the correct sequence", () => playbackReplay.Frames.Select(f => f.Time).SequenceEqual(recorder.SentFrames.Select(f => f.Time)));
         }
 
         private void onNewFrames(int userId, FrameDataBundle frames)
@@ -195,10 +199,10 @@ namespace osu.Game.Tests.Visual.Gameplay
             {
                 var frame = new TestReplayFrame();
                 frame.FromLegacy(legacyFrame, null);
-                replay.Frames.Add(frame);
+                playbackReplay.Frames.Add(frame);
             }
 
-            Logger.Log($"Received {frames.Frames.Count} new frames (total {replay.Frames.Count} of {recorder.SentFrames.Count})");
+            Logger.Log($"Received {frames.Frames.Count} new frames (total {playbackReplay.Frames.Count} of {recorder.SentFrames.Count})");
         }
 
         private double latency = SpectatorClient.TIME_BETWEEN_SENDS;
@@ -219,7 +223,7 @@ namespace osu.Game.Tests.Visual.Gameplay
             if (!replayHandler.HasFrames)
                 return;
 
-            var lastFrame = replay.Frames.LastOrDefault();
+            var lastFrame = playbackReplay.Frames.LastOrDefault();
 
             // this isn't perfect as we basically can't be aware of the rate-of-send here (the streamer is not sending data when not being moved).
             // in gameplay playback, the case where NextFrame is null would pause gameplay and handle this correctly; it's strictly a test limitation / best effort implementation.
@@ -360,15 +364,8 @@ namespace osu.Game.Tests.Visual.Gameplay
         {
             public List<ReplayFrame> SentFrames = new List<ReplayFrame>();
 
-            public TestReplayRecorder()
-                : base(new Score
-                {
-                    ScoreInfo =
-                    {
-                        BeatmapInfo = new BeatmapInfo(),
-                        Ruleset = new OsuRuleset().RulesetInfo,
-                    }
-                })
+            public TestReplayRecorder(Score score)
+                : base(score)
             {
             }
 
diff --git a/osu.Game/Rulesets/UI/ReplayRecorder.cs b/osu.Game/Rulesets/UI/ReplayRecorder.cs
index b04807e475..79da56fc8a 100644
--- a/osu.Game/Rulesets/UI/ReplayRecorder.cs
+++ b/osu.Game/Rulesets/UI/ReplayRecorder.cs
@@ -14,7 +14,6 @@ using osu.Framework.Input.Events;
 using osu.Game.Online.Spectator;
 using osu.Game.Rulesets.Replays;
 using osu.Game.Scoring;
-using osu.Game.Screens.Play;
 using osuTK;
 
 namespace osu.Game.Rulesets.UI
@@ -33,9 +32,6 @@ namespace osu.Game.Rulesets.UI
         [Resolved]
         private SpectatorClient spectatorClient { get; set; }
 
-        [Resolved]
-        private GameplayState gameplayState { get; set; }
-
         protected ReplayRecorder(Score target)
         {
             this.target = target;
@@ -48,15 +44,7 @@ namespace osu.Game.Rulesets.UI
         protected override void LoadComplete()
         {
             base.LoadComplete();
-
             inputManager = GetContainingInputManager();
-            spectatorClient.BeginPlaying(gameplayState, target);
-        }
-
-        protected override void Dispose(bool isDisposing)
-        {
-            base.Dispose(isDisposing);
-            spectatorClient?.EndPlaying(gameplayState);
         }
 
         protected override void Update()
diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs
index 9a058e45c5..9c08c77d91 100644
--- a/osu.Game/Screens/Play/Player.cs
+++ b/osu.Game/Screens/Play/Player.cs
@@ -26,7 +26,6 @@ using osu.Game.Extensions;
 using osu.Game.Graphics.Containers;
 using osu.Game.IO.Archives;
 using osu.Game.Online.API;
-using osu.Game.Online.Spectator;
 using osu.Game.Overlays;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
@@ -101,9 +100,6 @@ namespace osu.Game.Screens.Play
         [Resolved]
         private MusicController musicController { get; set; }
 
-        [Resolved]
-        private SpectatorClient spectatorClient { get; set; }
-
         public GameplayState GameplayState { get; private set; }
 
         private Ruleset ruleset;
@@ -1030,11 +1026,6 @@ namespace osu.Game.Screens.Play
                 // if arriving here and the results screen preparation task hasn't run, it's safe to say the user has not completed the beatmap.
                 if (prepareScoreForDisplayTask == null)
                     ScoreProcessor.FailScore(Score.ScoreInfo);
-
-                // EndPlaying() is typically called from ReplayRecorder.Dispose(). Disposal is currently asynchronous.
-                // To resolve test failures, forcefully end playing synchronously when this screen exits.
-                // Todo: Replace this with a more permanent solution once osu-framework has a synchronous cleanup method.
-                spectatorClient.EndPlaying(GameplayState);
             }
 
             // GameplayClockContainer performs seeks / start / stop operations on the beatmap's track.
diff --git a/osu.Game/Screens/Play/SubmittingPlayer.cs b/osu.Game/Screens/Play/SubmittingPlayer.cs
index ad63925b93..02a95ae9eb 100644
--- a/osu.Game/Screens/Play/SubmittingPlayer.cs
+++ b/osu.Game/Screens/Play/SubmittingPlayer.cs
@@ -15,6 +15,7 @@ using osu.Game.Beatmaps;
 using osu.Game.Database;
 using osu.Game.Online.API;
 using osu.Game.Online.Rooms;
+using osu.Game.Online.Spectator;
 using osu.Game.Rulesets.Scoring;
 using osu.Game.Scoring;
 
@@ -33,6 +34,9 @@ namespace osu.Game.Screens.Play
         [Resolved]
         private IAPIProvider api { get; set; }
 
+        [Resolved]
+        private SpectatorClient spectatorClient { get; set; }
+
         private TaskCompletionSource<bool> scoreSubmissionSource;
 
         protected SubmittingPlayer(PlayerConfiguration configuration = null)
@@ -134,6 +138,8 @@ namespace osu.Game.Screens.Play
                 if (realmBeatmap != null)
                     realmBeatmap.LastPlayed = DateTimeOffset.Now;
             });
+
+            spectatorClient.BeginPlaying(GameplayState, Score);
         }
 
         public override bool OnExiting(ScreenExitEvent e)
@@ -141,7 +147,10 @@ namespace osu.Game.Screens.Play
             bool exiting = base.OnExiting(e);
 
             if (LoadedBeatmapSuccessfully)
+            {
                 submitScore(Score.DeepClone());
+                spectatorClient.EndPlaying(GameplayState);
+            }
 
             return exiting;
         }

From fd091559909c8d7720d5d91a1455dce23aa054d5 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Fri, 29 Jul 2022 12:24:53 +0900
Subject: [PATCH 11/15] Revert blocking call when sending spectator frames

There are a lot of these requests, and we don't really care about
waiting on them to finish sending. This may have negatively affected
send performance for users with very high latency.

Reverts part of 0533249d11ac220e223369f774fafcabc3f30c51.

Addresses concerns in https://github.com/ppy/osu/discussions/19429#discussioncomment-3276400.
---
 osu.Game/Online/Spectator/OnlineSpectatorClient.cs | 2 +-
 osu.Game/Online/Spectator/SpectatorClient.cs       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/osu.Game/Online/Spectator/OnlineSpectatorClient.cs b/osu.Game/Online/Spectator/OnlineSpectatorClient.cs
index 030ca724c4..a012bf49b6 100644
--- a/osu.Game/Online/Spectator/OnlineSpectatorClient.cs
+++ b/osu.Game/Online/Spectator/OnlineSpectatorClient.cs
@@ -80,7 +80,7 @@ namespace osu.Game.Online.Spectator
 
             Debug.Assert(connection != null);
 
-            return connection.InvokeAsync(nameof(ISpectatorServer.SendFrameData), bundle);
+            return connection.SendAsync(nameof(ISpectatorServer.SendFrameData), bundle);
         }
 
         protected override Task EndPlayingInternal(SpectatorState state)
diff --git a/osu.Game/Online/Spectator/SpectatorClient.cs b/osu.Game/Online/Spectator/SpectatorClient.cs
index b5e1c8a45f..745c968992 100644
--- a/osu.Game/Online/Spectator/SpectatorClient.cs
+++ b/osu.Game/Online/Spectator/SpectatorClient.cs
@@ -304,7 +304,7 @@ namespace osu.Game.Online.Spectator
 
             SendFramesInternal(bundle).ContinueWith(t =>
             {
-                // Handle exception outside of `Schedule` to ensure it doesn't go unovserved.
+                // Handle exception outside of `Schedule` to ensure it doesn't go unobserved.
                 bool wasSuccessful = t.Exception == null;
 
                 return Schedule(() =>

From aaa6f963bd782c0630a5f6a5d1201b3ba0833665 Mon Sep 17 00:00:00 2001
From: Dan Balasescu <smoogipoo@smgi.me>
Date: Fri, 29 Jul 2022 15:27:39 +0900
Subject: [PATCH 12/15] Fix potential test failures due to Setup/SetUpSteps
 ordering

---
 .../TestSceneDrawableRoomParticipantsList.cs  | 44 +++++----
 .../TestSceneLoungeRoomsContainer.cs          | 32 ++++---
 .../TestSceneMatchBeatmapDetailArea.cs        | 25 ++---
 .../Multiplayer/TestSceneMatchLeaderboard.cs  | 95 ++++++++++---------
 .../TestSceneMultiSpectatorLeaderboard.cs     |  4 +-
 .../TestSceneMultiSpectatorScreen.cs          |  8 +-
 .../TestSceneMultiplayerMatchFooter.cs        | 31 +++---
 .../TestSceneMultiplayerMatchSubScreen.cs     | 13 ++-
 .../TestSceneMultiplayerPlaylist.cs           | 27 +++---
 .../TestSceneMultiplayerSpectateButton.cs     | 60 ++++++------
 .../TestSceneStarRatingRangeDisplay.cs        | 20 ++--
 .../TestScenePlaylistsMatchSettingsOverlay.cs | 20 ++--
 .../TestScenePlaylistsParticipantsList.cs     | 26 ++---
 .../Multiplayer/MultiplayerTestScene.cs       | 15 ++-
 .../Visual/OnlinePlay/OnlinePlayTestScene.cs  | 59 ++++++------
 15 files changed, 258 insertions(+), 221 deletions(-)

diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomParticipantsList.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomParticipantsList.cs
index 0a59e0e858..b26481387d 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomParticipantsList.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomParticipantsList.cs
@@ -19,29 +19,33 @@ namespace osu.Game.Tests.Visual.Multiplayer
     {
         private DrawableRoomParticipantsList list;
 
-        [SetUp]
-        public new void Setup() => Schedule(() =>
+        public override void SetUpSteps()
         {
-            SelectedRoom.Value = new Room
-            {
-                Name = { Value = "test room" },
-                Host =
-                {
-                    Value = new APIUser
-                    {
-                        Id = 2,
-                        Username = "peppy",
-                    }
-                }
-            };
+            base.SetUpSteps();
 
-            Child = list = new DrawableRoomParticipantsList
+            AddStep("create list", () =>
             {
-                Anchor = Anchor.Centre,
-                Origin = Anchor.Centre,
-                NumberOfCircles = 4
-            };
-        });
+                SelectedRoom.Value = new Room
+                {
+                    Name = { Value = "test room" },
+                    Host =
+                    {
+                        Value = new APIUser
+                        {
+                            Id = 2,
+                            Username = "peppy",
+                        }
+                    }
+                };
+
+                Child = list = new DrawableRoomParticipantsList
+                {
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                    NumberOfCircles = 4
+                };
+            });
+        }
 
         [Test]
         public void TestCircleCountNearLimit()
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs
index 82e7bf8969..3d6d4f0a90 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs
@@ -25,23 +25,27 @@ namespace osu.Game.Tests.Visual.Multiplayer
 
         private RoomsContainer container;
 
-        [SetUp]
-        public new void Setup() => Schedule(() =>
+        public override void SetUpSteps()
         {
-            Child = new PopoverContainer
-            {
-                RelativeSizeAxes = Axes.X,
-                AutoSizeAxes = Axes.Y,
-                Anchor = Anchor.Centre,
-                Origin = Anchor.Centre,
-                Width = 0.5f,
+            base.SetUpSteps();
 
-                Child = container = new RoomsContainer
+            AddStep("create container", () =>
+            {
+                Child = new PopoverContainer
                 {
-                    SelectedRoom = { BindTarget = SelectedRoom }
-                }
-            };
-        });
+                    RelativeSizeAxes = Axes.X,
+                    AutoSizeAxes = Axes.Y,
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                    Width = 0.5f,
+
+                    Child = container = new RoomsContainer
+                    {
+                        SelectedRoom = { BindTarget = SelectedRoom }
+                    }
+                };
+            });
+        }
 
         [Test]
         public void TestBasicListChanges()
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchBeatmapDetailArea.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchBeatmapDetailArea.cs
index 8cdcdfdfdf..b113352117 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchBeatmapDetailArea.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchBeatmapDetailArea.cs
@@ -3,7 +3,6 @@
 
 #nullable disable
 
-using NUnit.Framework;
 using osu.Framework.Graphics;
 using osu.Game.Online.API;
 using osu.Game.Online.Rooms;
@@ -18,19 +17,23 @@ namespace osu.Game.Tests.Visual.Multiplayer
 {
     public class TestSceneMatchBeatmapDetailArea : OnlinePlayTestScene
     {
-        [SetUp]
-        public new void Setup() => Schedule(() =>
+        public override void SetUpSteps()
         {
-            SelectedRoom.Value = new Room();
+            base.SetUpSteps();
 
-            Child = new MatchBeatmapDetailArea
+            AddStep("create area", () =>
             {
-                Anchor = Anchor.Centre,
-                Origin = Anchor.Centre,
-                Size = new Vector2(500),
-                CreateNewItem = createNewItem
-            };
-        });
+                SelectedRoom.Value = new Room();
+
+                Child = new MatchBeatmapDetailArea
+                {
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                    Size = new Vector2(500),
+                    CreateNewItem = createNewItem
+                };
+            });
+        }
 
         private void createNewItem()
         {
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchLeaderboard.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchLeaderboard.cs
index 506d7541a7..d2468ae005 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchLeaderboard.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchLeaderboard.cs
@@ -4,8 +4,6 @@
 #nullable disable
 
 using System.Collections.Generic;
-using NUnit.Framework;
-using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Game.Online.API;
 using osu.Game.Online.API.Requests.Responses;
@@ -19,59 +17,62 @@ namespace osu.Game.Tests.Visual.Multiplayer
 {
     public class TestSceneMatchLeaderboard : OnlinePlayTestScene
     {
-        [BackgroundDependencyLoader]
-        private void load()
+        public override void SetUpSteps()
         {
-            ((DummyAPIAccess)API).HandleRequest = r =>
+            base.SetUpSteps();
+
+            AddStep("setup API", () =>
             {
-                switch (r)
+                ((DummyAPIAccess)API).HandleRequest = r =>
                 {
-                    case GetRoomLeaderboardRequest leaderboardRequest:
-                        leaderboardRequest.TriggerSuccess(new APILeaderboard
-                        {
-                            Leaderboard = new List<APIUserScoreAggregate>
+                    switch (r)
+                    {
+                        case GetRoomLeaderboardRequest leaderboardRequest:
+                            leaderboardRequest.TriggerSuccess(new APILeaderboard
                             {
-                                new APIUserScoreAggregate
+                                Leaderboard = new List<APIUserScoreAggregate>
                                 {
-                                    UserID = 2,
-                                    User = new APIUser { Id = 2, Username = "peppy" },
-                                    TotalScore = 995533,
-                                    RoomID = 3,
-                                    CompletedBeatmaps = 1,
-                                    TotalAttempts = 6,
-                                    Accuracy = 0.9851
-                                },
-                                new APIUserScoreAggregate
-                                {
-                                    UserID = 1040328,
-                                    User = new APIUser { Id = 1040328, Username = "smoogipoo" },
-                                    TotalScore = 981100,
-                                    RoomID = 3,
-                                    CompletedBeatmaps = 1,
-                                    TotalAttempts = 9,
-                                    Accuracy = 0.937
+                                    new APIUserScoreAggregate
+                                    {
+                                        UserID = 2,
+                                        User = new APIUser { Id = 2, Username = "peppy" },
+                                        TotalScore = 995533,
+                                        RoomID = 3,
+                                        CompletedBeatmaps = 1,
+                                        TotalAttempts = 6,
+                                        Accuracy = 0.9851
+                                    },
+                                    new APIUserScoreAggregate
+                                    {
+                                        UserID = 1040328,
+                                        User = new APIUser { Id = 1040328, Username = "smoogipoo" },
+                                        TotalScore = 981100,
+                                        RoomID = 3,
+                                        CompletedBeatmaps = 1,
+                                        TotalAttempts = 9,
+                                        Accuracy = 0.937
+                                    }
                                 }
-                            }
-                        });
-                        return true;
-                }
+                            });
+                            return true;
+                    }
 
-                return false;
-            };
-        }
+                    return false;
+                };
+            });
 
-        [SetUp]
-        public new void Setup() => Schedule(() =>
-        {
-            SelectedRoom.Value = new Room { RoomID = { Value = 3 } };
-
-            Child = new MatchLeaderboard
+            AddStep("create leaderboard", () =>
             {
-                Origin = Anchor.Centre,
-                Anchor = Anchor.Centre,
-                Size = new Vector2(550f, 450f),
-                Scope = MatchLeaderboardScope.Overall,
-            };
-        });
+                SelectedRoom.Value = new Room { RoomID = { Value = 3 } };
+
+                Child = new MatchLeaderboard
+                {
+                    Origin = Anchor.Centre,
+                    Anchor = Anchor.Centre,
+                    Size = new Vector2(550f, 450f),
+                    Scope = MatchLeaderboardScope.Overall,
+                };
+            });
+        }
     }
 }
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs
index 80c356ec67..9e6941738a 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs
@@ -22,8 +22,10 @@ namespace osu.Game.Tests.Visual.Multiplayer
         private MultiSpectatorLeaderboard leaderboard;
 
         [SetUpSteps]
-        public new void SetUpSteps()
+        public override void SetUpSteps()
         {
+            base.SetUpSteps();
+
             AddStep("reset", () =>
             {
                 leaderboard?.RemoveAndDisposeImmediately();
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs
index 7df68392cf..d626426e6d 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorScreen.cs
@@ -56,8 +56,12 @@ namespace osu.Game.Tests.Visual.Multiplayer
             importedBeatmapId = importedBeatmap.OnlineID;
         }
 
-        [SetUp]
-        public new void Setup() => Schedule(() => playingUsers.Clear());
+        public override void SetUpSteps()
+        {
+            base.SetUpSteps();
+
+            AddStep("clear playing users", () => playingUsers.Clear());
+        }
 
         [Test]
         public void TestDelayedStart()
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchFooter.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchFooter.cs
index a98030e1e3..83e7ef6a81 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchFooter.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchFooter.cs
@@ -3,7 +3,6 @@
 
 #nullable disable
 
-using NUnit.Framework;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Cursor;
@@ -13,23 +12,27 @@ namespace osu.Game.Tests.Visual.Multiplayer
 {
     public class TestSceneMultiplayerMatchFooter : MultiplayerTestScene
     {
-        [SetUp]
-        public new void Setup() => Schedule(() =>
+        public override void SetUpSteps()
         {
-            Child = new PopoverContainer
+            base.SetUpSteps();
+
+            AddStep("create footer", () =>
             {
-                Anchor = Anchor.Centre,
-                Origin = Anchor.Centre,
-                RelativeSizeAxes = Axes.Both,
-                Child = new Container
+                Child = new PopoverContainer
                 {
                     Anchor = Anchor.Centre,
                     Origin = Anchor.Centre,
-                    RelativeSizeAxes = Axes.X,
-                    Height = 50,
-                    Child = new MultiplayerMatchFooter()
-                }
-            };
-        });
+                    RelativeSizeAxes = Axes.Both,
+                    Child = new Container
+                    {
+                        Anchor = Anchor.Centre,
+                        Origin = Anchor.Centre,
+                        RelativeSizeAxes = Axes.X,
+                        Height = 50,
+                        Child = new MultiplayerMatchFooter()
+                    }
+                };
+            });
+        }
     }
 }
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs
index 5ebafbaabb..8d31e9c723 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs
@@ -59,16 +59,15 @@ namespace osu.Game.Tests.Visual.Multiplayer
             importedSet = beatmaps.GetAllUsableBeatmapSets().First();
         }
 
-        [SetUp]
-        public new void Setup() => Schedule(() =>
-        {
-            SelectedRoom.Value = new Room { Name = { Value = "Test Room" } };
-        });
-
         [SetUpSteps]
         public void SetupSteps()
         {
-            AddStep("load match", () => LoadScreen(screen = new MultiplayerMatchSubScreen(SelectedRoom.Value)));
+            AddStep("load match", () =>
+            {
+                SelectedRoom.Value = new Room { Name = { Value = "Test Room" } };
+                LoadScreen(screen = new MultiplayerMatchSubScreen(SelectedRoom.Value));
+            });
+
             AddUntilStep("wait for load", () => screen.IsCurrentScreen());
         }
 
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs
index 75e6088b0d..8dbad4e330 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerPlaylist.cs
@@ -42,21 +42,22 @@ namespace osu.Game.Tests.Visual.Multiplayer
             Dependencies.Cache(Realm);
         }
 
-        [SetUp]
-        public new void Setup() => Schedule(() =>
-        {
-            Child = list = new MultiplayerPlaylist
-            {
-                Anchor = Anchor.Centre,
-                Origin = Anchor.Centre,
-                RelativeSizeAxes = Axes.Both,
-                Size = new Vector2(0.4f, 0.8f)
-            };
-        });
-
         [SetUpSteps]
-        public new void SetUpSteps()
+        public override void SetUpSteps()
         {
+            base.SetUpSteps();
+
+            AddStep("create list", () =>
+            {
+                Child = list = new MultiplayerPlaylist
+                {
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                    RelativeSizeAxes = Axes.Both,
+                    Size = new Vector2(0.4f, 0.8f)
+                };
+            });
+
             AddStep("import beatmap", () =>
             {
                 beatmaps.Import(TestResources.GetQuickTestBeatmapForImport()).WaitSafely();
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerSpectateButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerSpectateButton.cs
index e70c414bef..9b4cb722f3 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerSpectateButton.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerSpectateButton.cs
@@ -46,43 +46,47 @@ namespace osu.Game.Tests.Visual.Multiplayer
             beatmaps.Import(TestResources.GetQuickTestBeatmapForImport()).WaitSafely();
         }
 
-        [SetUp]
-        public new void Setup() => Schedule(() =>
+        public override void SetUpSteps()
         {
-            AvailabilityTracker.SelectedItem.BindTo(selectedItem);
+            base.SetUpSteps();
 
-            importedSet = beatmaps.GetAllUsableBeatmapSets().First();
-            Beatmap.Value = beatmaps.GetWorkingBeatmap(importedSet.Beatmaps.First());
-            selectedItem.Value = new PlaylistItem(Beatmap.Value.BeatmapInfo)
+            AddStep("create button", () =>
             {
-                RulesetID = Beatmap.Value.BeatmapInfo.Ruleset.OnlineID,
-            };
+                AvailabilityTracker.SelectedItem.BindTo(selectedItem);
 
-            Child = new PopoverContainer
-            {
-                RelativeSizeAxes = Axes.Both,
-                Child = new FillFlowContainer
+                importedSet = beatmaps.GetAllUsableBeatmapSets().First();
+                Beatmap.Value = beatmaps.GetWorkingBeatmap(importedSet.Beatmaps.First());
+                selectedItem.Value = new PlaylistItem(Beatmap.Value.BeatmapInfo)
                 {
-                    AutoSizeAxes = Axes.Both,
-                    Direction = FillDirection.Vertical,
-                    Children = new Drawable[]
+                    RulesetID = Beatmap.Value.BeatmapInfo.Ruleset.OnlineID,
+                };
+
+                Child = new PopoverContainer
+                {
+                    RelativeSizeAxes = Axes.Both,
+                    Child = new FillFlowContainer
                     {
-                        spectateButton = new MultiplayerSpectateButton
+                        AutoSizeAxes = Axes.Both,
+                        Direction = FillDirection.Vertical,
+                        Children = new Drawable[]
                         {
-                            Anchor = Anchor.Centre,
-                            Origin = Anchor.Centre,
-                            Size = new Vector2(200, 50),
-                        },
-                        startControl = new MatchStartControl
-                        {
-                            Anchor = Anchor.Centre,
-                            Origin = Anchor.Centre,
-                            Size = new Vector2(200, 50),
+                            spectateButton = new MultiplayerSpectateButton
+                            {
+                                Anchor = Anchor.Centre,
+                                Origin = Anchor.Centre,
+                                Size = new Vector2(200, 50),
+                            },
+                            startControl = new MatchStartControl
+                            {
+                                Anchor = Anchor.Centre,
+                                Origin = Anchor.Centre,
+                                Size = new Vector2(200, 50),
+                            }
                         }
                     }
-                }
-            };
-        });
+                };
+            });
+        }
 
         [TestCase(MultiplayerRoomState.Open)]
         [TestCase(MultiplayerRoomState.WaitingForLoad)]
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneStarRatingRangeDisplay.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneStarRatingRangeDisplay.cs
index 321e0c2c89..5bccabcf2f 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneStarRatingRangeDisplay.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneStarRatingRangeDisplay.cs
@@ -14,17 +14,21 @@ namespace osu.Game.Tests.Visual.Multiplayer
 {
     public class TestSceneStarRatingRangeDisplay : OnlinePlayTestScene
     {
-        [SetUp]
-        public new void Setup() => Schedule(() =>
+        public override void SetUpSteps()
         {
-            SelectedRoom.Value = new Room();
+            base.SetUpSteps();
 
-            Child = new StarRatingRangeDisplay
+            AddStep("create display", () =>
             {
-                Anchor = Anchor.Centre,
-                Origin = Anchor.Centre
-            };
-        });
+                SelectedRoom.Value = new Room();
+
+                Child = new StarRatingRangeDisplay
+                {
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre
+                };
+            });
+        }
 
         [Test]
         public void TestRange([Values(0, 2, 3, 4, 6, 7)] double min, [Values(0, 2, 3, 4, 6, 7)] double max)
diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsMatchSettingsOverlay.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsMatchSettingsOverlay.cs
index e6882081dd..c71bdb3a06 100644
--- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsMatchSettingsOverlay.cs
+++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsMatchSettingsOverlay.cs
@@ -25,17 +25,21 @@ namespace osu.Game.Tests.Visual.Playlists
 
         protected override OnlinePlayTestSceneDependencies CreateOnlinePlayDependencies() => new TestDependencies();
 
-        [SetUp]
-        public new void Setup() => Schedule(() =>
+        public override void SetUpSteps()
         {
-            SelectedRoom.Value = new Room();
+            base.SetUpSteps();
 
-            Child = settings = new TestRoomSettings(SelectedRoom.Value)
+            AddStep("create overlay", () =>
             {
-                RelativeSizeAxes = Axes.Both,
-                State = { Value = Visibility.Visible }
-            };
-        });
+                SelectedRoom.Value = new Room();
+
+                Child = settings = new TestRoomSettings(SelectedRoom.Value)
+                {
+                    RelativeSizeAxes = Axes.Both,
+                    State = { Value = Visibility.Visible }
+                };
+            });
+        }
 
         [Test]
         public void TestButtonEnabledOnlyWithNameAndBeatmap()
diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsParticipantsList.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsParticipantsList.cs
index 5961ed74ad..9a0dda056a 100644
--- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsParticipantsList.cs
+++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsParticipantsList.cs
@@ -15,21 +15,25 @@ namespace osu.Game.Tests.Visual.Playlists
 {
     public class TestScenePlaylistsParticipantsList : OnlinePlayTestScene
     {
-        [SetUp]
-        public new void Setup() => Schedule(() =>
+        public override void SetUpSteps()
         {
-            SelectedRoom.Value = new Room { RoomID = { Value = 7 } };
+            base.SetUpSteps();
 
-            for (int i = 0; i < 50; i++)
+            AddStep("create list", () =>
             {
-                SelectedRoom.Value.RecentParticipants.Add(new APIUser
+                SelectedRoom.Value = new Room { RoomID = { Value = 7 } };
+
+                for (int i = 0; i < 50; i++)
                 {
-                    Username = "peppy",
-                    Statistics = new UserStatistics { GlobalRank = 1234 },
-                    Id = 2
-                });
-            }
-        });
+                    SelectedRoom.Value.RecentParticipants.Add(new APIUser
+                    {
+                        Username = "peppy",
+                        Statistics = new UserStatistics { GlobalRank = 1234 },
+                        Id = 2
+                    });
+                }
+            });
+        }
 
         [Test]
         public void TestHorizontalLayout()
diff --git a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs
index 5ea98bdbb1..101a347749 100644
--- a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs
+++ b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs
@@ -3,7 +3,6 @@
 
 #nullable disable
 
-using NUnit.Framework;
 using osu.Game.Online.Rooms;
 using osu.Game.Tests.Beatmaps;
 using osu.Game.Tests.Visual.OnlinePlay;
@@ -34,13 +33,6 @@ namespace osu.Game.Tests.Visual.Multiplayer
             this.joinRoom = joinRoom;
         }
 
-        [SetUp]
-        public new void Setup() => Schedule(() =>
-        {
-            if (joinRoom)
-                SelectedRoom.Value = CreateRoom();
-        });
-
         protected virtual Room CreateRoom()
         {
             return new Room
@@ -63,7 +55,12 @@ namespace osu.Game.Tests.Visual.Multiplayer
 
             if (joinRoom)
             {
-                AddStep("join room", () => RoomManager.CreateRoom(SelectedRoom.Value));
+                AddStep("join room", () =>
+                {
+                    SelectedRoom.Value = CreateRoom();
+                    RoomManager.CreateRoom(SelectedRoom.Value);
+                });
+
                 AddUntilStep("wait for room join", () => RoomJoined);
             }
         }
diff --git a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs
index 6577057c17..b9c293c3aa 100644
--- a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs
+++ b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs
@@ -4,7 +4,6 @@
 #nullable disable
 
 using System;
-using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
@@ -56,39 +55,43 @@ namespace osu.Game.Tests.Visual.OnlinePlay
             return dependencies;
         }
 
-        [SetUp]
-        public void Setup() => Schedule(() =>
+        public override void SetUpSteps()
         {
-            // Reset the room dependencies to a fresh state.
-            drawableDependenciesContainer.Clear();
-            dependencies.OnlinePlayDependencies = CreateOnlinePlayDependencies();
-            drawableDependenciesContainer.AddRange(OnlinePlayDependencies.DrawableComponents);
+            base.SetUpSteps();
 
-            var handler = OnlinePlayDependencies.RequestsHandler;
-
-            // Resolving the BeatmapManager in the test scene will inject the game-wide BeatmapManager, while many test scenes cache their own BeatmapManager instead.
-            // To get around this, the BeatmapManager is looked up from the dependencies provided to the children of the test scene instead.
-            var beatmapManager = dependencies.Get<BeatmapManager>();
-
-            ((DummyAPIAccess)API).HandleRequest = request =>
+            AddStep("setup dependencies", () =>
             {
-                try
+                // Reset the room dependencies to a fresh state.
+                drawableDependenciesContainer.Clear();
+                dependencies.OnlinePlayDependencies = CreateOnlinePlayDependencies();
+                drawableDependenciesContainer.AddRange(OnlinePlayDependencies.DrawableComponents);
+
+                var handler = OnlinePlayDependencies.RequestsHandler;
+
+                // Resolving the BeatmapManager in the test scene will inject the game-wide BeatmapManager, while many test scenes cache their own BeatmapManager instead.
+                // To get around this, the BeatmapManager is looked up from the dependencies provided to the children of the test scene instead.
+                var beatmapManager = dependencies.Get<BeatmapManager>();
+
+                ((DummyAPIAccess)API).HandleRequest = request =>
                 {
-                    return handler.HandleRequest(request, API.LocalUser.Value, beatmapManager);
-                }
-                catch (ObjectDisposedException)
-                {
-                    // These requests can be fired asynchronously, but potentially arrive after game components
-                    // have been disposed (ie. realm in BeatmapManager).
-                    // This only happens in tests and it's easiest to ignore them for now.
-                    Logger.Log($"Handled {nameof(ObjectDisposedException)} in test request handling");
-                    return true;
-                }
-            };
-        });
+                    try
+                    {
+                        return handler.HandleRequest(request, API.LocalUser.Value, beatmapManager);
+                    }
+                    catch (ObjectDisposedException)
+                    {
+                        // These requests can be fired asynchronously, but potentially arrive after game components
+                        // have been disposed (ie. realm in BeatmapManager).
+                        // This only happens in tests and it's easiest to ignore them for now.
+                        Logger.Log($"Handled {nameof(ObjectDisposedException)} in test request handling");
+                        return true;
+                    }
+                };
+            });
+        }
 
         /// <summary>
-        /// Creates the room dependencies. Called every <see cref="Setup"/>.
+        /// Creates the room dependencies. Called every <see cref="SetUpSteps"/>.
         /// </summary>
         /// <remarks>
         /// Any custom dependencies required for online play sub-classes should be added here.

From e07e761c1011cf7202d3baa9360c1f6dc31e46ef Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Fri, 29 Jul 2022 15:54:03 +0900
Subject: [PATCH 13/15] Ensure realm is in a good state before asserts in
 `TestSceneFilterControl`

---
 .../SongSelect/TestSceneFilterControl.cs      | 42 +++++++++++--------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs
index 56f252f47d..82314f9764 100644
--- a/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs
+++ b/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs
@@ -1,6 +1,7 @@
 // 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.
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using NUnit.Framework;
@@ -19,6 +20,7 @@ using osu.Game.Rulesets;
 using osu.Game.Screens.Select;
 using osu.Game.Tests.Resources;
 using osuTK.Input;
+using Realms;
 
 namespace osu.Game.Tests.Visual.SongSelect
 {
@@ -47,7 +49,7 @@ namespace osu.Game.Tests.Visual.SongSelect
         [SetUp]
         public void SetUp() => Schedule(() =>
         {
-            Realm.Write(r => r.RemoveAll<BeatmapCollection>());
+            writeAndRefresh(r => r.RemoveAll<BeatmapCollection>());
 
             Child = control = new FilterControl
             {
@@ -68,8 +70,8 @@ namespace osu.Game.Tests.Visual.SongSelect
         [Test]
         public void TestCollectionAddedToDropdown()
         {
-            AddStep("add collection", () => Realm.Write(r => r.Add(new BeatmapCollection(name: "1"))));
-            AddStep("add collection", () => Realm.Write(r => r.Add(new BeatmapCollection(name: "2"))));
+            AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
+            AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "2"))));
             assertCollectionDropdownContains("1");
             assertCollectionDropdownContains("2");
         }
@@ -79,9 +81,9 @@ namespace osu.Game.Tests.Visual.SongSelect
         {
             BeatmapCollection first = null!;
 
-            AddStep("add collection", () => Realm.Write(r => r.Add(first = new BeatmapCollection(name: "1"))));
-            AddStep("add collection", () => Realm.Write(r => r.Add(new BeatmapCollection(name: "2"))));
-            AddStep("remove collection", () => Realm.Write(r => r.Remove(first)));
+            AddStep("add collection", () => writeAndRefresh(r => r.Add(first = new BeatmapCollection(name: "1"))));
+            AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "2"))));
+            AddStep("remove collection", () => writeAndRefresh(r => r.Remove(first)));
 
             assertCollectionDropdownContains("1", false);
             assertCollectionDropdownContains("2");
@@ -90,7 +92,7 @@ namespace osu.Game.Tests.Visual.SongSelect
         [Test]
         public void TestCollectionRenamed()
         {
-            AddStep("add collection", () => Realm.Write(r => r.Add(new BeatmapCollection(name: "1"))));
+            AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
             AddStep("select collection", () =>
             {
                 var dropdown = control.ChildrenOfType<CollectionDropdown>().Single();
@@ -99,7 +101,7 @@ namespace osu.Game.Tests.Visual.SongSelect
 
             addExpandHeaderStep();
 
-            AddStep("change name", () => Realm.Write(_ => getFirstCollection().Name = "First"));
+            AddStep("change name", () => writeAndRefresh(_ => getFirstCollection().Name = "First"));
 
             assertCollectionDropdownContains("First");
             assertCollectionHeaderDisplays("First");
@@ -117,7 +119,7 @@ namespace osu.Game.Tests.Visual.SongSelect
         public void TestCollectionFilterHasAddButton()
         {
             addExpandHeaderStep();
-            AddStep("add collection", () => Realm.Write(r => r.Add(new BeatmapCollection(name: "1"))));
+            AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
             AddStep("hover collection", () => InputManager.MoveMouseTo(getAddOrRemoveButton(1)));
             AddAssert("collection has add button", () => getAddOrRemoveButton(1).IsPresent);
         }
@@ -127,7 +129,7 @@ namespace osu.Game.Tests.Visual.SongSelect
         {
             addExpandHeaderStep();
 
-            AddStep("add collection", () => Realm.Write(r => r.Add(new BeatmapCollection(name: "1"))));
+            AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
 
             AddStep("select available beatmap", () => Beatmap.Value = beatmapManager.GetWorkingBeatmap(beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0]));
             AddAssert("button enabled", () => getAddOrRemoveButton(1).Enabled.Value);
@@ -143,13 +145,13 @@ namespace osu.Game.Tests.Visual.SongSelect
 
             AddStep("select available beatmap", () => Beatmap.Value = beatmapManager.GetWorkingBeatmap(beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0]));
 
-            AddStep("add collection", () => Realm.Write(r => r.Add(new BeatmapCollection(name: "1"))));
+            AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
             AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare));
 
-            AddStep("add beatmap to collection", () => Realm.Write(r => getFirstCollection().BeatmapMD5Hashes.Add(Beatmap.Value.BeatmapInfo.MD5Hash)));
+            AddStep("add beatmap to collection", () => writeAndRefresh(r => getFirstCollection().BeatmapMD5Hashes.Add(Beatmap.Value.BeatmapInfo.MD5Hash)));
             AddAssert("button is minus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.MinusSquare));
 
-            AddStep("remove beatmap from collection", () => Realm.Write(r => getFirstCollection().BeatmapMD5Hashes.Clear()));
+            AddStep("remove beatmap from collection", () => writeAndRefresh(r => getFirstCollection().BeatmapMD5Hashes.Clear()));
             AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare));
         }
 
@@ -160,7 +162,7 @@ namespace osu.Game.Tests.Visual.SongSelect
 
             AddStep("select available beatmap", () => Beatmap.Value = beatmapManager.GetWorkingBeatmap(beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0]));
 
-            AddStep("add collection", () => Realm.Write(r => r.Add(new BeatmapCollection(name: "1"))));
+            AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
             AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare));
 
             addClickAddOrRemoveButtonStep(1);
@@ -179,7 +181,7 @@ namespace osu.Game.Tests.Visual.SongSelect
 
             addExpandHeaderStep();
 
-            AddStep("add collection", () => Realm.Write(r => r.Add(new BeatmapCollection(name: "1", new List<string> { "abc" }))));
+            AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1", new List<string> { "abc" }))));
             AddStep("select collection", () =>
             {
                 InputManager.MoveMouseTo(getCollectionDropdownItems().ElementAt(1));
@@ -205,14 +207,20 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddAssert("filter request not fired", () => !received);
         }
 
+        private void writeAndRefresh(Action<Realm> action) => Realm.Write(r =>
+        {
+            action(r);
+            r.Refresh();
+        });
+
         private BeatmapCollection getFirstCollection() => Realm.Run(r => r.All<BeatmapCollection>().First());
 
         private void assertCollectionHeaderDisplays(string collectionName, bool shouldDisplay = true)
-            => AddAssert($"collection dropdown header displays '{collectionName}'",
+            => AddUntilStep($"collection dropdown header displays '{collectionName}'",
                 () => shouldDisplay == (control.ChildrenOfType<CollectionDropdown.CollectionDropdownHeader>().Single().ChildrenOfType<SpriteText>().First().Text == collectionName));
 
         private void assertCollectionDropdownContains(string collectionName, bool shouldContain = true) =>
-            AddAssert($"collection dropdown {(shouldContain ? "contains" : "does not contain")} '{collectionName}'",
+            AddUntilStep($"collection dropdown {(shouldContain ? "contains" : "does not contain")} '{collectionName}'",
                 // A bit of a roundabout way of going about this, see: https://github.com/ppy/osu-framework/issues/3871 + https://github.com/ppy/osu-framework/issues/3872
                 () => shouldContain == (getCollectionDropdownItems().Any(i => i.ChildrenOfType<CompositeDrawable>().OfType<IHasText>().First().Text == collectionName)));
 

From 2ff6ff06d3ed25fce9bd69939b8fcae5c8f9ddb0 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Fri, 29 Jul 2022 16:05:41 +0900
Subject: [PATCH 14/15] Use tuple to better explain new `bool` parameter

---
 osu.Game/Beatmaps/BeatmapImporter.cs | 5 ++---
 osu.Game/Beatmaps/BeatmapManager.cs  | 6 +++---
 osu.Game/OsuGameBase.cs              | 2 +-
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs
index df8b18313c..6e954e0356 100644
--- a/osu.Game/Beatmaps/BeatmapImporter.cs
+++ b/osu.Game/Beatmaps/BeatmapImporter.cs
@@ -34,7 +34,7 @@ namespace osu.Game.Beatmaps
 
         protected override string[] HashableFileTypes => new[] { ".osu" };
 
-        public Action<BeatmapSetInfo, bool>? ProcessBeatmap { private get; set; }
+        public Action<(BeatmapSetInfo beatmapSet, bool isBatch)>? ProcessBeatmap { private get; set; }
 
         public BeatmapImporter(Storage storage, RealmAccess realm)
             : base(storage, realm)
@@ -171,8 +171,7 @@ namespace osu.Game.Beatmaps
         protected override void PostImport(BeatmapSetInfo model, Realm realm, bool batchImport)
         {
             base.PostImport(model, realm, batchImport);
-
-            ProcessBeatmap?.Invoke(model, batchImport);
+            ProcessBeatmap?.Invoke((model, batchImport));
         }
 
         private void validateOnlineIds(BeatmapSetInfo beatmapSet, Realm realm)
diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs
index abf3d43d94..387daf6906 100644
--- a/osu.Game/Beatmaps/BeatmapManager.cs
+++ b/osu.Game/Beatmaps/BeatmapManager.cs
@@ -42,7 +42,7 @@ namespace osu.Game.Beatmaps
 
         private readonly WorkingBeatmapCache workingBeatmapCache;
 
-        public Action<BeatmapSetInfo, bool>? ProcessBeatmap { private get; set; }
+        public Action<(BeatmapSetInfo beatmapSet, bool isBatch)>? ProcessBeatmap { private get; set; }
 
         public BeatmapManager(Storage storage, RealmAccess realm, RulesetStore rulesets, IAPIProvider? api, AudioManager audioManager, IResourceStore<byte[]> gameResources, GameHost? host = null,
                               WorkingBeatmap? defaultBeatmap = null, BeatmapDifficultyCache? difficultyCache = null, bool performOnlineLookups = false)
@@ -62,7 +62,7 @@ namespace osu.Game.Beatmaps
             BeatmapTrackStore = audioManager.GetTrackStore(userResources);
 
             beatmapImporter = CreateBeatmapImporter(storage, realm);
-            beatmapImporter.ProcessBeatmap = (obj, isBatch) => ProcessBeatmap?.Invoke(obj, isBatch);
+            beatmapImporter.ProcessBeatmap = args => ProcessBeatmap?.Invoke(args);
             beatmapImporter.PostNotification = obj => PostNotification?.Invoke(obj);
 
             workingBeatmapCache = CreateWorkingBeatmapCache(audioManager, gameResources, userResources, defaultBeatmap, host);
@@ -323,7 +323,7 @@ namespace osu.Game.Beatmaps
 
                     setInfo.CopyChangesToRealm(liveBeatmapSet);
 
-                    ProcessBeatmap?.Invoke(liveBeatmapSet, false);
+                    ProcessBeatmap?.Invoke((liveBeatmapSet, false));
                 });
             }
 
diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs
index 41eeece76d..dc7e2b47cf 100644
--- a/osu.Game/OsuGameBase.cs
+++ b/osu.Game/OsuGameBase.cs
@@ -287,7 +287,7 @@ namespace osu.Game
 
             AddInternal(new BeatmapOnlineChangeIngest(beatmapUpdater, realm, metadataClient));
 
-            BeatmapManager.ProcessBeatmap = (set, isBatch) => beatmapUpdater.Process(set, preferOnlineFetch: !isBatch);
+            BeatmapManager.ProcessBeatmap = args => beatmapUpdater.Process(args.beatmapSet, !args.isBatch);
 
             dependencies.Cache(userCache = new UserLookupCache());
             AddInternal(userCache);

From 07e3765b3492b1023039804e9e54b71c42e51cda Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Fri, 29 Jul 2022 17:25:30 +0900
Subject: [PATCH 15/15] Ensure collection is added to dropdown before trying to
 click it

---
 osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs
index 82314f9764..d0523b58fa 100644
--- a/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs
+++ b/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs
@@ -93,6 +93,7 @@ namespace osu.Game.Tests.Visual.SongSelect
         public void TestCollectionRenamed()
         {
             AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
+            assertCollectionDropdownContains("1");
             AddStep("select collection", () =>
             {
                 var dropdown = control.ChildrenOfType<CollectionDropdown>().Single();
@@ -120,6 +121,7 @@ namespace osu.Game.Tests.Visual.SongSelect
         {
             addExpandHeaderStep();
             AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
+            assertCollectionDropdownContains("1");
             AddStep("hover collection", () => InputManager.MoveMouseTo(getAddOrRemoveButton(1)));
             AddAssert("collection has add button", () => getAddOrRemoveButton(1).IsPresent);
         }
@@ -130,6 +132,7 @@ namespace osu.Game.Tests.Visual.SongSelect
             addExpandHeaderStep();
 
             AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
+            assertCollectionDropdownContains("1");
 
             AddStep("select available beatmap", () => Beatmap.Value = beatmapManager.GetWorkingBeatmap(beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0]));
             AddAssert("button enabled", () => getAddOrRemoveButton(1).Enabled.Value);
@@ -146,6 +149,7 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddStep("select available beatmap", () => Beatmap.Value = beatmapManager.GetWorkingBeatmap(beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0]));
 
             AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
+            assertCollectionDropdownContains("1");
             AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare));
 
             AddStep("add beatmap to collection", () => writeAndRefresh(r => getFirstCollection().BeatmapMD5Hashes.Add(Beatmap.Value.BeatmapInfo.MD5Hash)));
@@ -163,6 +167,7 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddStep("select available beatmap", () => Beatmap.Value = beatmapManager.GetWorkingBeatmap(beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0]));
 
             AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1"))));
+            assertCollectionDropdownContains("1");
             AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare));
 
             addClickAddOrRemoveButtonStep(1);
@@ -182,6 +187,8 @@ namespace osu.Game.Tests.Visual.SongSelect
             addExpandHeaderStep();
 
             AddStep("add collection", () => writeAndRefresh(r => r.Add(new BeatmapCollection(name: "1", new List<string> { "abc" }))));
+            assertCollectionDropdownContains("1");
+
             AddStep("select collection", () =>
             {
                 InputManager.MoveMouseTo(getCollectionDropdownItems().ElementAt(1));