Merge pull request #35426 from bdach/collection-grouping-order

Fix song select collection group order not matching other collection lists when certain characters are used

Author: peppy

osu.Game/Screens/SelectV2/BeatmapCarousel.cs

@@ -809,7 +809,15 @@ protected override Task<IEnumerable<CarouselItem>> FilterAsync(bool clearExistin
         [Resolved]
         private RealmAccess realm { get; set; } = null!;
 
-        protected virtual List<BeatmapCollection> GetAllCollections() => realm.Run(r => r.All<BeatmapCollection>().AsEnumerable().Detach());
+        /// <remarks>
+        /// FOOTGUN WARNING: this being sorted on the realm side before detaching is IMPORTANT.
+        /// realm supports sorting as an internal operation, and realm's implementation of string sorting does NOT match dotnet's
+        /// with respect to treatment of punctuation characters like <c>-</c> or <c>_</c>, among others.
+        /// All other places that show lists of collections also use the realm-side sorting implementation,
+        /// because they use the sorting operation inside subscription queries for efficient drawable management,
+        /// so this usage kind of has to follow suit.
+        /// </remarks>
+        protected virtual List<BeatmapCollection> GetAllCollections() => realm.Run(r => r.All<BeatmapCollection>().OrderBy(c => c.Name).AsEnumerable().Detach());
 
         protected virtual Dictionary<Guid, ScoreRank> GetBeatmapInfoGuidToTopRankMapping(FilterCriteria criteria) => realm.Run(r =>
         {

osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs

@@ -398,15 +398,20 @@ private IEnumerable<GroupDefinition> defineGroupBySource(string source)
             return new GroupDefinition(0, source).Yield();
         }
 
-        private IEnumerable<GroupDefinition> defineGroupByCollection(BeatmapInfo beatmap, IEnumerable<BeatmapCollection> collections)
+        private IEnumerable<GroupDefinition> defineGroupByCollection(BeatmapInfo beatmap, List<BeatmapCollection> collections)
         {
             bool anyCollections = false;
 
-            foreach (var collection in collections)
+            for (int i = 0; i < collections.Count; i++)
             {
+                var collection = collections[i];
+
                 if (collection.BeatmapMD5Hashes.Contains(beatmap.MD5Hash))
                 {
-                    yield return new GroupDefinition(0, collection.Name);
+                    // NOTE: the ordering of the incoming collection list is significant and needs to be preserved.
+                    // the fallback to ordering by name cannot be relied on.
+                    // see xmldoc of `BeatmapCarousel.GetAllCollections()`.
+                    yield return new GroupDefinition(i, collection.Name);
 
                     anyCollections = true;
                 }
@@ -415,7 +420,7 @@ private IEnumerable<GroupDefinition> defineGroupByCollection(BeatmapInfo beatmap
             if (anyCollections)
                 yield break;
 
-            yield return new GroupDefinition(1, "Not in collection");
+            yield return new GroupDefinition(int.MaxValue, "Not in collection");
         }
 
         private IEnumerable<GroupDefinition> defineGroupByOwnMaps(BeatmapInfo beatmap, int? localUserId, string? localUserUsername)