Merge pull request #19409 from peppy/carousel-maintain-selection-over-update

Fix beatmap carousel not maintaining selection if currently selected beatmap is updated

osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs

@@ -6,15 +6,18 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading.Tasks;
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Audio;
 using osu.Framework.Bindables;
+using osu.Framework.Extensions;
 using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Platform;
 using osu.Framework.Screens;
 using osu.Framework.Testing;
+using osu.Framework.Utils;
 using osu.Game.Beatmaps;
 using osu.Game.Configuration;
 using osu.Game.Database;
@@ -24,6 +27,7 @@ using osu.Game.Online.API.Requests.Responses;
 using osu.Game.Online.Chat;
 using osu.Game.Overlays;
 using osu.Game.Overlays.Mods;
+using osu.Game.Overlays.Notifications;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Osu;
@@ -413,6 +417,55 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddUntilStep("no selection", () => songSelect.Carousel.SelectedBeatmapInfo == null);
         }
 
+        [Test]
+        public void TestSelectionRetainedOnBeatmapUpdate()
+        {
+            createSongSelect();
+            changeRuleset(0);
+
+            Live<BeatmapSetInfo> original = null!;
+            int originalOnlineSetID = 0;
+
+            AddStep(@"Sort by artist", () => config.SetValue(OsuSetting.SongSelectSortingMode, SortMode.Artist));
+
+            AddStep("import original", () =>
+            {
+                original = manager.Import(new ImportTask(TestResources.GetQuickTestBeatmapForImport())).GetResultSafely();
+                originalOnlineSetID = original!.Value.OnlineID;
+            });
+
+            // This will move the beatmap set to a different location in the carousel.
+            AddStep("Update original with bogus info", () =>
+            {
+                original.PerformWrite(set =>
+                {
+                    foreach (var beatmap in set.Beatmaps)
+                    {
+                        beatmap.Metadata.Artist = "ZZZZZ";
+                        beatmap.OnlineID = 12804;
+                    }
+                });
+            });
+
+            AddRepeatStep("import other beatmaps", () =>
+            {
+                var testBeatmapSetInfo = TestResources.CreateTestBeatmapSetInfo();
+
+                foreach (var beatmap in testBeatmapSetInfo.Beatmaps)
+                    beatmap.Metadata.Artist = ((char)RNG.Next('A', 'Z')).ToString();
+
+                manager.Import(testBeatmapSetInfo);
+            }, 10);
+
+            AddUntilStep("has selection", () => songSelect.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID == originalOnlineSetID);
+
+            Task<Live<BeatmapSetInfo>> updateTask = null!;
+            AddStep("update beatmap", () => updateTask = manager.ImportAsUpdate(new ProgressNotification(), new ImportTask(TestResources.GetQuickTestBeatmapForImport()), original.Value));
+            AddUntilStep("wait for update completion", () => updateTask.IsCompleted);
+
+            AddUntilStep("retained selection", () => songSelect.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID == originalOnlineSetID);
+        }
+
         [Test]
         public void TestPresentNewRulesetNewBeatmap()
         {

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -265,6 +265,43 @@ namespace osu.Game.Screens.Select
 
             foreach (int i in changes.InsertedIndices)
                 UpdateBeatmapSet(sender[i].Detach());
+
+            if (changes.DeletedIndices.Length > 0)
+            {
+                // To handle the beatmap update flow, attempt to track selection changes across delete-insert transactions.
+                // When an update occurs, the previous beatmap set is either soft or hard deleted.
+                // Check if the current selection was potentially deleted by re-querying its validity.
+                bool selectedSetMarkedDeleted = realm.Run(r => r.Find<BeatmapSetInfo>(SelectedBeatmapSet.ID))?.DeletePending != false;
+
+                int[] modifiedAndInserted = changes.NewModifiedIndices.Concat(changes.InsertedIndices).ToArray();
+
+                if (selectedSetMarkedDeleted && modifiedAndInserted.Any())
+                {
+                    // If it is no longer valid, make the bold assumption that an updated version will be available in the modified/inserted indices.
+                    // This relies on the full update operation being in a single transaction, so please don't change that.
+                    foreach (int i in modifiedAndInserted)
+                    {
+                        var beatmapSetInfo = sender[i];
+
+                        foreach (var beatmapInfo in beatmapSetInfo.Beatmaps)
+                        {
+                            if (!((IBeatmapMetadataInfo)beatmapInfo.Metadata).Equals(SelectedBeatmapInfo.Metadata))
+                                continue;
+
+                            // Best effort matching. We can't use ID because in the update flow a new version will get its own GUID.
+                            if (beatmapInfo.DifficultyName == SelectedBeatmapInfo.DifficultyName)
+                            {
+                                SelectBeatmap(beatmapInfo);
+                                return;
+                            }
+                        }
+                    }
+
+                    // If a direct selection couldn't be made, it's feasible that the difficulty name (or beatmap metadata) changed.
+                    // Let's attempt to follow set-level selection anyway.
+                    SelectBeatmap(sender[modifiedAndInserted.First()].Beatmaps.First());
+                }
+            }
         }
 
         private void beatmapsChanged(IRealmCollection<BeatmapInfo> sender, ChangeSet changes, Exception error)