diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 6af53df725d3..518035fb822a 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -464,8 +464,6 @@ public void TestSelectionRetainedOnBeatmapUpdate() manager.Import(testBeatmapSetInfo); }, 10); - AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh())); - AddUntilStep("has selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID, () => Is.EqualTo(originalOnlineSetID)); Task?> updateTask = null!; @@ -478,8 +476,6 @@ public void TestSelectionRetainedOnBeatmapUpdate() }); AddUntilStep("wait for update completion", () => updateTask.IsCompleted); - AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh())); - AddUntilStep("retained selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID, () => Is.EqualTo(originalOnlineSetID)); } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 4c6c67c34847..47cdbe34f499 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -269,8 +269,28 @@ private void deletedBeatmapSetsChanged(IRealmCollection sender, if (changes == null) return; - foreach (int i in changes.InsertedIndices) - removeBeatmapSet(sender[i].ID); + var removeableSets = changes.InsertedIndices.Select(i => sender[i].ID).ToHashSet(); + + // This schedule is required to retain selection of beatmaps over an ImportAsUpdate operation. + // This is covered by TestPlaySongSelect.TestSelectionRetainedOnBeatmapUpdate. + // + // In short, we have specialised logic in `beatmapSetsChanged` (directly below) to infer that an + // update operation has occurred. For this to work, we need to confirm the `DeletePending` flag + // of the current selection. + // + // If we don't schedule the following code, it is possible for the `deleteBeatmapSetsChanged` handler + // to be invoked before the `beatmapSetsChanged` handler (realm call order seems non-deterministic) + // which will lead to the currently selected beatmap changing via `CarouselGroupEagerSelect`. + // + // We need a better path forward here. A few ideas: + // - Avoid the necessity of having realm subscriptions on deleted/hidden items, maybe by storing all guids in realm + // to a local list so we can better look them up on receiving `DeletedIndices`. + // - Add a new property on `BeatmapSetInfo` to link to the pre-update set, and use that to handle the update case. + Schedule(() => + { + foreach (var set in removeableSets) + removeBeatmapSet(set); + }); } private void beatmapSetsChanged(IRealmCollection sender, ChangeSet? changes)