From 453ecd21b32b552947ee1141f793e26d88774113 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 14 Dec 2021 19:11:56 +0900 Subject: [PATCH] Fix `OnlinePlayBeatmapAvailabilityTracker` potentially in incorrect state Adter an import of a modified version of a beatmap (that was already present in the local database), it's feasible that one of these trackers would not see the state change due to the nuances of the import process. --- .../OnlinePlayBeatmapAvailabilityTracker.cs | 45 +++++++++++++++++-- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs index aa0e37363b..a32f069470 100644 --- a/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game/Online/Rooms/OnlinePlayBeatmapAvailabilityTracker.cs @@ -40,6 +40,11 @@ public sealed class OnlinePlayBeatmapAvailabilityTracker : CompositeDrawable private BeatmapDownloadTracker downloadTracker; + /// + /// The beatmap matching the required hash (and providing a final state). + /// + private BeatmapInfo matchingHash; + protected override void LoadComplete() { base.LoadComplete(); @@ -71,13 +76,34 @@ protected override void LoadComplete() 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 @@ private void updateAvailability() break; case DownloadState.LocallyAvailable: - bool hashMatches = checkHashValidity(); + matchingHash = findMatchingHash(); + + bool hashMatches = matchingHash != null; availability.Value = hashMatches ? BeatmapAvailability.LocallyAvailable() : BeatmapAvailability.NotDownloaded(); @@ -108,12 +136,23 @@ private void updateAvailability() } } - 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; + } } } }