From 682192dbd7fc53d8e642d84a3f2c9cf780ec83e4 Mon Sep 17 00:00:00 2001
From: Salman Ahmed <frenzibyte@gmail.com>
Date: Mon, 1 Aug 2022 18:43:01 +0300
Subject: [PATCH 1/2] Add failing test case

---
 .../SongSelect/TestSceneBeatmapCarousel.cs    | 53 +++++++++++++++++--
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs
index 59932f8781..bb9e83a21c 100644
--- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs
+++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs
@@ -486,9 +486,6 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddAssert("Something is selected", () => carousel.SelectedBeatmapInfo != null);
         }
 
-        /// <summary>
-        /// Test sorting
-        /// </summary>
         [Test]
         public void TestSorting()
         {
@@ -517,6 +514,9 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddAssert($"Check {zzz_string} is at bottom", () => carousel.BeatmapSets.Last().Metadata.Artist == zzz_string);
         }
 
+        /// <summary>
+        /// Ensures stability is maintained on different sort modes for items with equal properties.
+        /// </summary>
         [Test]
         public void TestSortingStability()
         {
@@ -549,6 +549,53 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddAssert("Items reset to original order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == idOffset + index).All(b => b));
         }
 
+        /// <summary>
+        /// Ensures stability is maintained on different sort modes while a new item is added to the carousel.
+        /// </summary>
+        [Test]
+        public void TestSortingStabilityWithNewItems()
+        {
+            List<BeatmapSetInfo> sets = new List<BeatmapSetInfo>();
+
+            for (int i = 0; i < 3; i++)
+            {
+                var set = TestResources.CreateTestBeatmapSetInfo(3);
+
+                // only need to set the first as they are a shared reference.
+                var beatmap = set.Beatmaps.First();
+
+                beatmap.Metadata.Artist = "same artist";
+                beatmap.Metadata.Title = "same title";
+
+                sets.Add(set);
+            }
+
+            int idOffset = sets.First().OnlineID;
+
+            loadBeatmaps(sets);
+
+            AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false));
+            AddAssert("Items remain in original order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == idOffset + index).All(b => b));
+
+            AddStep("Add new item", () =>
+            {
+                var set = TestResources.CreateTestBeatmapSetInfo();
+
+                // only need to set the first as they are a shared reference.
+                var beatmap = set.Beatmaps.First();
+
+                beatmap.Metadata.Artist = "same artist";
+                beatmap.Metadata.Title = "same title";
+
+                carousel.UpdateBeatmapSet(set);
+            });
+
+            AddAssert("Items remain in original order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == idOffset + index).All(b => b));
+
+            AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false));
+            AddAssert("Items remain in original order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == idOffset + index).All(b => b));
+        }
+
         [Test]
         public void TestSortingWithFiltered()
         {

From fc7fc3d673cf0a843d4eeea0225bcde76b987c36 Mon Sep 17 00:00:00 2001
From: Salman Ahmed <frenzibyte@gmail.com>
Date: Mon, 1 Aug 2022 19:13:57 +0300
Subject: [PATCH 2/2] Fix newly imported beatmaps not using correct comparer
 for sorting

---
 .../Screens/Select/Carousel/CarouselGroup.cs  | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs
index 8d141b6f72..9302578038 100644
--- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs
+++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs
@@ -2,7 +2,6 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System.Collections.Generic;
-using System.Linq;
 
 namespace osu.Game.Screens.Select.Carousel
 {
@@ -15,7 +14,7 @@ namespace osu.Game.Screens.Select.Carousel
 
         public IReadOnlyList<CarouselItem> Items => items;
 
-        private List<CarouselItem> items = new List<CarouselItem>();
+        private readonly List<CarouselItem> items = new List<CarouselItem>();
 
         /// <summary>
         /// Used to assign a monotonically increasing ID to items as they are added. This member is
@@ -24,9 +23,6 @@ namespace osu.Game.Screens.Select.Carousel
         private ulong currentItemID;
 
         private Comparer<CarouselItem>? criteriaComparer;
-
-        private static readonly Comparer<CarouselItem> item_id_comparer = Comparer<CarouselItem>.Create((x, y) => x.ItemID.CompareTo(y.ItemID));
-
         private FilterCriteria? lastCriteria;
 
         protected int GetIndexOfItem(CarouselItem lastSelected) => items.IndexOf(lastSelected);
@@ -90,9 +86,16 @@ namespace osu.Game.Screens.Select.Carousel
 
             items.ForEach(c => c.Filter(criteria));
 
-            // IEnumerable<T>.OrderBy() is used instead of List<T>.Sort() to ensure sorting stability
-            criteriaComparer = Comparer<CarouselItem>.Create((x, y) => x.CompareTo(criteria, y));
-            items = items.OrderBy(c => c, criteriaComparer).ThenBy(c => c, item_id_comparer).ToList();
+            criteriaComparer = Comparer<CarouselItem>.Create((x, y) =>
+            {
+                int comparison = x.CompareTo(criteria, y);
+                if (comparison != 0)
+                    return comparison;
+
+                return x.ItemID.CompareTo(y.ItemID);
+            });
+
+            items.Sort(criteriaComparer);
 
             lastCriteria = criteria;
         }