Fix stutter on returning to song select when updating large beatmap sets

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

@@ -430,6 +430,151 @@ public void TestSortingStabilityWithNewItems()
             AddAssert("Order didn't change", () => Carousel.PostFilterBeatmaps.Select(b => b.ID), () => Is.EqualTo(originalOrder));
         }
 
+        /// <summary>
+        /// Replicates a whole-set replace as applied by the carousel when a set is updated: some difficulties are
+        /// matched-and-replaced (by online ID), some are removed, and some new ones are added.
+        /// The replace handling applies the entire diff as a single range replace (including a count change),
+        /// so this guards both the splice itself and the count-change path of the carousel's change handling.
+        /// </summary>
+        [Test]
+        public void TestBeatmapSetReplacedWithMixedDifficultyMutations()
+        {
+            List<Guid> expectedIds = null!;
+            Guid removedId = Guid.Empty;
+
+            AddStep("update set with mixed difficulty mutations", () =>
+            {
+                removedId = baseTestBeatmap.Beatmaps[1].ID;
+
+                var updatedSet = new BeatmapSetInfo
+                {
+                    ID = baseTestBeatmap.ID,
+                    OnlineID = baseTestBeatmap.OnlineID,
+                    DateAdded = baseTestBeatmap.DateAdded,
+                    DateSubmitted = baseTestBeatmap.DateSubmitted,
+                    DateRanked = baseTestBeatmap.DateRanked,
+                    Status = baseTestBeatmap.Status,
+                    StatusInt = baseTestBeatmap.StatusInt,
+                    DeletePending = baseTestBeatmap.DeletePending,
+                    Hash = baseTestBeatmap.Hash,
+                    Protected = baseTestBeatmap.Protected,
+                };
+
+                // keep the first difficulty (matched by online ID, but with changed metadata => valid replace);
+                // drop the second difficulty entirely; and introduce a brand new third difficulty.
+                var keptDifficulty = baseTestBeatmap.Beatmaps[0];
+                var kept = new BeatmapInfo
+                {
+                    ID = keptDifficulty.ID,
+                    Metadata = new BeatmapMetadata { Artist = "updated test", Title = "updated title" },
+                    Ruleset = keptDifficulty.Ruleset,
+                    DifficultyName = keptDifficulty.DifficultyName,
+                    BeatmapSet = updatedSet,
+                    Status = keptDifficulty.Status,
+                    OnlineID = keptDifficulty.OnlineID,
+                    Length = keptDifficulty.Length,
+                    BPM = keptDifficulty.BPM,
+                    Hash = "new hash",
+                    StarRating = keptDifficulty.StarRating,
+                    MD5Hash = keptDifficulty.MD5Hash,
+                    OnlineMD5Hash = keptDifficulty.OnlineMD5Hash,
+                };
+
+                var added = createBeatmap(updatedSet);
+                added.ID = Guid.NewGuid();
+                added.OnlineID = -2;
+                added.DifficultyName = "new difficulty";
+
+                updatedSet.Beatmaps.Add(kept);
+                updatedSet.Beatmaps.Add(added);
+
+                expectedIds = updatedSet.Beatmaps.Select(b => b.ID).ToList();
+
+                int originalIndex = BeatmapSets.IndexOf(baseTestBeatmap);
+
+                Realm.Write(r => r.Add(updatedSet, update: true));
+                BeatmapSets.ReplaceRange(originalIndex, 1, [updatedSet.Detach()]);
+            });
+
+            WaitForFiltering();
+
+            AddAssert("updated set has exactly two difficulties", () => Carousel.PostFilterBeatmaps.Count(b => expectedIds.Contains(b.ID)), () => Is.EqualTo(2));
+            AddAssert("kept difficulty present", () => Carousel.PostFilterBeatmaps.Any(b => b.ID == expectedIds[0]), () => Is.True);
+            AddAssert("added difficulty present", () => Carousel.PostFilterBeatmaps.Any(b => b.ID == expectedIds[1]), () => Is.True);
+            AddAssert("removed difficulty is gone", () => Carousel.PostFilterBeatmaps.Any(b => b.ID == removedId), () => Is.False);
+        }
+
+        /// <summary>
+        /// Replicates the #34826 scenario: the matched beatmap is present in the replace snapshot, but has been
+        /// deleted from realm by the time the replace is processed (multiple updates to the same set can be queued
+        /// before the carousel processes them). The replace handling must not request selection of a beatmap that
+        /// no longer exists in realm — song select would otherwise load it as the global beatmap (NRE in
+        /// <see cref="FooterButtonOptions"/>, see https://github.com/ppy/osu/issues/34826).
+        /// </summary>
+        /// <remarks>
+        /// Regression guard: this test fails if the per-difficulty realm check from #34914 is removed outright,
+        /// and passes with both the #34914 check and the current selection-path-only check.
+        /// </remarks>
+        [Test]
+        public void TestBeatmapSetReplacedWithDeletedCurrentBeatmap()
+        {
+            BeatmapInfo selectedBeatmap = null!;
+            BeatmapInfo kept = null!;
+
+            AddStep("select first difficulty", () =>
+            {
+                selectedBeatmap = baseTestBeatmap.Beatmaps[0];
+                Carousel.CurrentBeatmap = selectedBeatmap;
+            });
+
+            AddStep("update set with a matched difficulty no longer in realm", () =>
+            {
+                var updatedSet = new BeatmapSetInfo
+                {
+                    ID = baseTestBeatmap.ID,
+                    OnlineID = baseTestBeatmap.OnlineID,
+                    DateAdded = baseTestBeatmap.DateAdded,
+                    DateSubmitted = baseTestBeatmap.DateSubmitted,
+                    DateRanked = baseTestBeatmap.DateRanked,
+                    Status = baseTestBeatmap.Status,
+                    StatusInt = baseTestBeatmap.StatusInt,
+                    DeletePending = baseTestBeatmap.DeletePending,
+                    Hash = baseTestBeatmap.Hash,
+                    Protected = baseTestBeatmap.Protected,
+                };
+
+                // The matched difficulty carries an ID which was never written to realm, simulating a beatmap deleted
+                // since the replace snapshot was taken. The set itself is intentionally not added to realm either.
+                kept = new BeatmapInfo
+                {
+                    ID = Guid.NewGuid(),
+                    Metadata = new BeatmapMetadata { Artist = "updated test", Title = "updated title" },
+                    Ruleset = selectedBeatmap.Ruleset,
+                    DifficultyName = selectedBeatmap.DifficultyName,
+                    BeatmapSet = updatedSet,
+                    Status = selectedBeatmap.Status,
+                    OnlineID = selectedBeatmap.OnlineID,
+                    Length = selectedBeatmap.Length,
+                    BPM = selectedBeatmap.BPM,
+                    Hash = "new hash",
+                    StarRating = selectedBeatmap.StarRating,
+                    MD5Hash = selectedBeatmap.MD5Hash,
+                    OnlineMD5Hash = selectedBeatmap.OnlineMD5Hash,
+                };
+
+                updatedSet.Beatmaps.Add(kept);
+
+                int originalIndex = BeatmapSets.IndexOf(baseTestBeatmap);
+
+                BeatmapSets.ReplaceRange(originalIndex, 1, [updatedSet.Detach()]);
+            });
+
+            WaitForFiltering();
+
+            AddAssert("selection unchanged", () => Carousel.CurrentBeatmap, () => Is.EqualTo(selectedBeatmap));
+            AddAssert("deleted match never requested for selection", () => BeatmapRequestedSelections.Contains(kept), () => Is.False);
+        }
+
         private void assertDidFilter(int count = 1) => AddAssert("did filter", () => Carousel.FilterCount, () => Is.EqualTo(initial_filter_count + count));
 
         private void assertDidNotFilter() => AddAssert("did not filter", () => Carousel.FilterCount, () => Is.EqualTo(initial_filter_count));

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -248,44 +248,72 @@
                     // In the case of difficulty reprocessing, this will trigger multiple times per beatmap as it's always triggering a set update.
                     // We may want to look to improve this in the future either here or at the source (only trigger an update after all difficulties
                     // have been processed) if it becomes an issue for animation or performance reasons.
+                    //
+                    // A set's difficulties always occupy a contiguous range in `Items` (they are only ever added or replaced as a whole set),
+                    // so the entire diff is applied as a single replace operation below rather than one per difficulty. This avoids running
+                    // the carousel's change handling / relayout (and a linear `IndexOf` lookup) once per difficulty.
+                    List<BeatmapInfo> newBeatmaps = new List<BeatmapInfo>(oldSetBeatmaps.Count);
+
                     foreach (var beatmap in oldSetBeatmaps)
                     {
-                        int previousIndex = Items.IndexOf(beatmap);
-                        Debug.Assert(previousIndex >= 0);
-
                         // we're intentionally being lenient with there being two difficulties with equal online ID or difficulty name.
                         // this can be the case when the user modifies the beatmap using the editor's "external edit" feature.
                         BeatmapInfo? matchingNewBeatmap =
                             newSetBeatmaps.FirstOrDefault(b => b.OnlineID > 0 && b.OnlineID == beatmap.OnlineID) ??
                             newSetBeatmaps.FirstOrDefault(b => b.DifficultyName == beatmap.DifficultyName && b.Ruleset.Equals(beatmap.Ruleset));
 
-                        // The matching beatmap may have been deleted or invalidated in some way since this event was fired.
-                        // Let's make sure we have the most up-to-date realm state.
-                        if (matchingNewBeatmap?.ID is Guid matchingID)
-                            matchingNewBeatmap = realm.Run(r => r.FindWithRefresh<BeatmapInfo>(matchingID)?.Detach());
+                        if (matchingNewBeatmap == null)
+                            continue;
 
-                        if (matchingNewBeatmap != null)
-                        {
-                            // TODO: should this exist in song select instead of here?
-                            // we need to ensure the global beatmap is also updated alongside changes.
-                            if (CurrentBeatmap != null && beatmap.Equals(CurrentBeatmap))
-                                // we don't know in which group the matching new beatmap is, but that's fine - we can keep the previous one for now.
-                                // we are about to modify `Items`, which - if required - will trigger a re-filter,
-                                // which will pick a correct group - if one is present - via `HandleFilterCompleted()`.
-                                RequestSelection(new GroupedBeatmap(CurrentGroupedBeatmap?.Group, matchingNewBeatmap));
-
-                            Items.ReplaceRange(previousIndex, 1, [matchingNewBeatmap]);
-                            newSetBeatmaps.Remove(matchingNewBeatmap);
-                        }
-                        else
+                        // The matched beatmap may have been deleted since the snapshot was taken, as multiple updates to the same set can be queued
+                        // before the carousel processes them. Replacing an item with a stale beatmap converges via the follow-up update queued for the
+                        // deletion, but selecting one would load a beatmap that no longer exists in realm. Only the selection path needs a freshness
+                        // check, which limits this to a single realm round-trip per replace event.
+                        if (CurrentBeatmap != null && beatmap.Equals(CurrentBeatmap))
                         {
-                            Items.RemoveAt(previousIndex);
+                            var refreshedBeatmap = realm.Run(r => r.FindWithRefresh<BeatmapInfo>(matchingNewBeatmap.ID)?.Detach());
+
+                            if (refreshedBeatmap == null)
+                            {
+                                // The matched beatmap was deleted since the snapshot was taken. Retain the stale match in the list (it will be
+                                // removed by the queued follow-up update) and leave the current selection untouched rather than selecting a beatmap
+                                // that no longer exists in realm.
+                                newBeatmaps.Add(matchingNewBeatmap);
+                                newSetBeatmaps.Remove(matchingNewBeatmap);
+                                continue;
+                            }
+
+                            matchingNewBeatmap = refreshedBeatmap;
                         }
+
+                        // TODO: should this exist in song select instead of here?
+                        // we need to ensure the global beatmap is also updated alongside changes.
+                        if (CurrentBeatmap != null && beatmap.Equals(CurrentBeatmap))
+                            // we don't know in which group the matching new beatmap is, but that's fine - we can keep the previous one for now.
+                            // we are about to modify `Items`, which - if required - will trigger a re-filter,
+                            // which will pick a correct group - if one is present - via `HandleFilterCompleted()`.
+                            RequestSelection(new GroupedBeatmap(CurrentGroupedBeatmap?.Group, matchingNewBeatmap));
+
+                        newBeatmaps.Add(matchingNewBeatmap);
+                        newSetBeatmaps.Remove(matchingNewBeatmap);
                     }
 
                     // Add any items which weren't found in the previous pass (difficulty names didn't match).
-                    foreach (var beatmap in newSetBeatmaps)
-                        Items.Add(beatmap);
+                    newBeatmaps.AddRange(newSetBeatmaps);
+
+                    if (oldSetBeatmaps.Count == 0)
+                    {
+                        foreach (var beatmap in newBeatmaps)
+                            Items.Add(beatmap);
+
+                        break;
+                    }
+
+                    int previousIndex = Items.IndexOf(oldSetBeatmaps[0]);
+                    Debug.Assert(previousIndex >= 0);
+                    Debug.Assert(Items.Skip(previousIndex).Take(oldSetBeatmaps.Count).SequenceEqual(oldSetBeatmaps), "the set's difficulties should occupy a contiguous range in the carousel items");
+
+                    Items.ReplaceRange(previousIndex, oldSetBeatmaps.Count, newBeatmaps);
 
                     break;
 
@@ -426,6 +454,11 @@
                     var oldBeatmaps = args.OldItems!.OfType<BeatmapInfo>().ToList();
                     var newBeatmaps = args.NewItems!.OfType<BeatmapInfo>().ToList();
 
+                    // A replace may change the number of items, as the carousel replaces a whole set's difficulties in one operation
+                    // (see `beatmapSetsChanged`). Any count change requires a re-filter; only equal-sized replaces can be skipped.
+                    if (oldBeatmaps.Count != newBeatmaps.Count)
+                        return true;
+
                     for (int i = 0; i < oldBeatmaps.Count; i++)
                     {
                         var oldBeatmap = oldBeatmaps[i];