Merge pull request #16083 from peppy/fix-online-availability-after-reimport

Fix `OnlinePlayBeatmapAvailabilityTracker` failng after modified reimport of existing beatmap

osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs

@@ -114,18 +114,23 @@ namespace osu.Game.Tests.Online
         public void TestTrackerRespectsChecksum()
         {
             AddStep("allow importing", () => beatmaps.AllowImport.SetResult(true));
+            AddStep("import beatmap", () => beatmaps.Import(testBeatmapFile).Wait());
+            addAvailabilityCheckStep("initially locally available", BeatmapAvailability.LocallyAvailable);
 
             AddStep("import altered beatmap", () =>
             {
                 beatmaps.Import(TestResources.GetTestBeatmapForImport(true)).Wait();
             });
-            addAvailabilityCheckStep("state still not downloaded", BeatmapAvailability.NotDownloaded);
+            addAvailabilityCheckStep("state not downloaded", BeatmapAvailability.NotDownloaded);
 
             AddStep("recreate tracker", () => Child = availabilityTracker = new OnlinePlayBeatmapAvailabilityTracker
             {
                 SelectedItem = { BindTarget = selectedItem }
             });
             addAvailabilityCheckStep("state not downloaded as well", BeatmapAvailability.NotDownloaded);
+
+            AddStep("reimport original beatmap", () => beatmaps.Import(TestResources.GetQuickTestBeatmapForImport()).Wait());
+            addAvailabilityCheckStep("locally available after re-import", BeatmapAvailability.LocallyAvailable);
         }
 
         private void addAvailabilityCheckStep(string description, Func<BeatmapAvailability> expected)

osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs

@@ -15,9 +15,11 @@ using osu.Game.Database;
 using osu.Game.Online.Rooms;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Osu;
+using osu.Game.Rulesets.Osu.Objects;
 using osu.Game.Screens.OnlinePlay.Components;
 using osu.Game.Screens.OnlinePlay.Playlists;
 using osu.Game.Screens.Play;
+using osu.Game.Tests.Beatmaps;
 using osu.Game.Tests.Visual.OnlinePlay;
 using osuTK.Input;
 
@@ -109,6 +111,85 @@ namespace osu.Game.Tests.Visual.Playlists
             AddAssert("first playlist item selected", () => match.SelectedItem.Value == SelectedRoom.Value.Playlist[0]);
         }
 
+        [Test]
+        public void TestBeatmapUpdatedOnReImport()
+        {
+            string realHash = null;
+            int realOnlineId = 0;
+            int realOnlineSetId = 0;
+
+            AddStep("store real beatmap values", () =>
+            {
+                realHash = importedBeatmap.Value.Beatmaps[0].MD5Hash;
+                realOnlineId = importedBeatmap.Value.Beatmaps[0].OnlineID ?? -1;
+                realOnlineSetId = importedBeatmap.Value.OnlineID ?? -1;
+            });
+
+            AddStep("import modified beatmap", () =>
+            {
+                var modifiedBeatmap = new TestBeatmap(new OsuRuleset().RulesetInfo)
+                {
+                    BeatmapInfo =
+                    {
+                        OnlineID = realOnlineId,
+                        BeatmapSet =
+                        {
+                            OnlineID = realOnlineSetId
+                        }
+                    },
+                };
+
+                modifiedBeatmap.HitObjects.Clear();
+                modifiedBeatmap.HitObjects.Add(new HitCircle { StartTime = 5000 });
+
+                manager.Import(modifiedBeatmap.BeatmapInfo.BeatmapSet).Wait();
+            });
+
+            // Create the room using the real beatmap values.
+            setupAndCreateRoom(room =>
+            {
+                room.Name.Value = "my awesome room";
+                room.Host.Value = API.LocalUser.Value;
+                room.Playlist.Add(new PlaylistItem
+                {
+                    Beatmap =
+                    {
+                        Value = new BeatmapInfo
+                        {
+                            MD5Hash = realHash,
+                            OnlineID = realOnlineId,
+                            BeatmapSet = new BeatmapSetInfo
+                            {
+                                OnlineID = realOnlineSetId,
+                            }
+                        }
+                    },
+                    Ruleset = { Value = new OsuRuleset().RulesetInfo }
+                });
+            });
+
+            AddAssert("match has default beatmap", () => match.Beatmap.IsDefault);
+
+            AddStep("reimport original beatmap", () =>
+            {
+                var originalBeatmap = new TestBeatmap(new OsuRuleset().RulesetInfo)
+                {
+                    BeatmapInfo =
+                    {
+                        OnlineID = realOnlineId,
+                        BeatmapSet =
+                        {
+                            OnlineID = realOnlineSetId
+                        }
+                    },
+                };
+
+                manager.Import(originalBeatmap.BeatmapInfo.BeatmapSet).Wait();
+            });
+
+            AddUntilStep("match has correct beatmap", () => realHash == match.Beatmap.Value.BeatmapInfo.MD5Hash);
+        }
+
         private void setupAndCreateRoom(Action<Room> room)
         {
             AddStep("setup room", () => room(SelectedRoom.Value));

osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs

@@ -40,6 +40,11 @@ namespace osu.Game.Online.Rooms
 
         private BeatmapDownloadTracker downloadTracker;
 
+        /// <summary>
+        /// The beatmap matching the required hash (and providing a final <see cref="BeatmapAvailability.LocallyAvailable"/> state).
+        /// </summary>
+        private BeatmapInfo matchingHash;
+
         protected override void LoadComplete()
         {
             base.LoadComplete();
@@ -71,13 +76,34 @@ namespace osu.Game.Online.Rooms
                         progressUpdate = Scheduler.AddDelayed(updateAvailability, progressUpdate == null ? 0 : 500);
                 }, true);
             }, true);
+
+            // These events are needed for a fringe case where a modified/altered beatmap is imported with matching OnlineIDs.
+            // During the import process this will cause the existing beatmap set to be silently deleted and replaced with the new one.
+            // This is not exposed to us via `BeatmapDownloadTracker` so we have to take it into our own hands (as we care about the hash matching).
+            beatmapManager.ItemUpdated += itemUpdated;
+            beatmapManager.ItemRemoved += itemRemoved;
         }
 
+        private void itemUpdated(BeatmapSetInfo item) => Schedule(() =>
+        {
+            if (matchingHash?.BeatmapSet.ID == item.ID || SelectedItem.Value?.Beatmap.Value.BeatmapSet?.OnlineID == item.OnlineID)
+                updateAvailability();
+        });
+
+        private void itemRemoved(BeatmapSetInfo item) => Schedule(() =>
+        {
+            if (matchingHash?.BeatmapSet.ID == item.ID)
+                updateAvailability();
+        });
+
         private void updateAvailability()
         {
             if (downloadTracker == null)
                 return;
 
+            // will be repopulated below if still valid.
+            matchingHash = null;
+
             switch (downloadTracker.State.Value)
             {
                 case DownloadState.NotDownloaded:
@@ -93,7 +119,9 @@ namespace osu.Game.Online.Rooms
                     break;
 
                 case DownloadState.LocallyAvailable:
-                    bool hashMatches = checkHashValidity();
+                    matchingHash = findMatchingHash();
+
+                    bool hashMatches = matchingHash != null;
 
                     availability.Value = hashMatches ? BeatmapAvailability.LocallyAvailable() : BeatmapAvailability.NotDownloaded();
 
@@ -108,12 +136,23 @@ namespace osu.Game.Online.Rooms
             }
         }
 
-        private bool checkHashValidity()
+        private BeatmapInfo findMatchingHash()
         {
             int onlineId = SelectedItem.Value.Beatmap.Value.OnlineID;
             string checksum = SelectedItem.Value.Beatmap.Value.MD5Hash;
 
-            return beatmapManager.QueryBeatmap(b => b.OnlineID == onlineId && b.MD5Hash == checksum && !b.BeatmapSet.DeletePending) != null;
+            return beatmapManager.QueryBeatmap(b => b.OnlineID == onlineId && b.MD5Hash == checksum && !b.BeatmapSet.DeletePending);
+        }
+
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+
+            if (beatmapManager != null)
+            {
+                beatmapManager.ItemUpdated -= itemUpdated;
+                beatmapManager.ItemRemoved -= itemRemoved;
+            }
         }
     }
 }