From f30d63107a73c1a4f059366a12483f086e26c22f Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 27 Jan 2022 15:19:27 +0900
Subject: [PATCH] Add `SortID` to `RulesetInfo` to allow stable ordering of
 rulesets for display

---
 .../NonVisual/RulesetInfoOrderingTest.cs      | 38 +++++++++++++++++++
 osu.Game/Rulesets/RulesetInfo.cs              |  5 +++
 osu.Game/Rulesets/RulesetStore.cs             |  5 +--
 .../Select/Carousel/CarouselBeatmap.cs        |  2 +-
 .../Select/Carousel/CarouselBeatmapSet.cs     |  2 +-
 5 files changed, 46 insertions(+), 6 deletions(-)
 create mode 100644 osu.Game.Tests/NonVisual/RulesetInfoOrderingTest.cs

diff --git a/osu.Game.Tests/NonVisual/RulesetInfoOrderingTest.cs b/osu.Game.Tests/NonVisual/RulesetInfoOrderingTest.cs
new file mode 100644
index 0000000000..e7c04d27c1
--- /dev/null
+++ b/osu.Game.Tests/NonVisual/RulesetInfoOrderingTest.cs
@@ -0,0 +1,38 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Linq;
+using NUnit.Framework;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Catch;
+using osu.Game.Rulesets.Osu;
+
+namespace osu.Game.Tests.NonVisual
+{
+    [TestFixture]
+    public class RulesetInfoOrderingTest
+    {
+        [Test]
+        public void TestOrdering()
+        {
+            var rulesets = new[]
+            {
+                new RulesetInfo("custom2", "Custom Ruleset 2", string.Empty, -1),
+                new OsuRuleset().RulesetInfo,
+                new RulesetInfo("custom3", "Custom Ruleset 3", string.Empty, -1),
+                new RulesetInfo("custom2", "Custom Ruleset 2", string.Empty, -1),
+                new CatchRuleset().RulesetInfo,
+                new RulesetInfo("custom3", "Custom Ruleset 3", string.Empty, -1),
+            };
+
+            var orderedRulesets = rulesets.OrderBy(r => r.SortID);
+
+            // Ensure all customs are after official.
+            Assert.That(orderedRulesets.Select(r => r.OnlineID), Is.EqualTo(new[] { 0, 2, -1, -1, -1, -1 }));
+
+            // Ensure customs are grouped next to each other (ie. stably sorted).
+            Assert.That(orderedRulesets.SkipWhile(r => r.ShortName != "custom2").Skip(1).First().ShortName, Is.EqualTo("custom2"));
+            Assert.That(orderedRulesets.SkipWhile(r => r.ShortName != "custom3").Skip(1).First().ShortName, Is.EqualTo("custom3"));
+        }
+    }
+}
diff --git a/osu.Game/Rulesets/RulesetInfo.cs b/osu.Game/Rulesets/RulesetInfo.cs
index 2e2ec5c024..d721926e95 100644
--- a/osu.Game/Rulesets/RulesetInfo.cs
+++ b/osu.Game/Rulesets/RulesetInfo.cs
@@ -24,6 +24,11 @@ namespace osu.Game.Rulesets
 
         public string InstantiationInfo { get; set; } = string.Empty;
 
+        /// <summary>
+        /// A best effort sort ID which provides stable ordering and puts online rulesets before non-online rulesets.
+        /// </summary>
+        public int SortID => OnlineID >= 0 ? OnlineID : Math.Abs(ShortName.GetHashCode());
+
         public RulesetInfo(string shortName, string name, string instantiationInfo, int onlineID)
         {
             ShortName = shortName;
diff --git a/osu.Game/Rulesets/RulesetStore.cs b/osu.Game/Rulesets/RulesetStore.cs
index 0f518be639..a9fd8f430e 100644
--- a/osu.Game/Rulesets/RulesetStore.cs
+++ b/osu.Game/Rulesets/RulesetStore.cs
@@ -163,10 +163,7 @@ namespace osu.Game.Rulesets
                     }
                 }
 
-                // add known official rulesets first..
-                availableRulesets.AddRange(detachedRulesets.Where(r => r.OnlineID >= 0).OrderBy(r => r.OnlineID));
-                // .. then add any customs
-                availableRulesets.AddRange(detachedRulesets.Where(r => r.OnlineID < 0).OrderBy(r => r.ShortName));
+                availableRulesets.AddRange(detachedRulesets.OrderBy(r => r.SortID));
             });
         }
 
diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs
index e6dee0233f..9d49fdaf6e 100644
--- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs
+++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs
@@ -89,7 +89,7 @@ namespace osu.Game.Screens.Select.Carousel
             {
                 default:
                 case SortMode.Difficulty:
-                    int ruleset = BeatmapInfo.RulesetID.CompareTo(otherBeatmap.BeatmapInfo.RulesetID);
+                    int ruleset = BeatmapInfo.Ruleset.SortID.CompareTo(otherBeatmap.BeatmapInfo.Ruleset.SortID);
                     if (ruleset != 0) return ruleset;
 
                     return BeatmapInfo.StarRating.CompareTo(otherBeatmap.BeatmapInfo.StarRating);
diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs
index b2b3b5411c..274be7b73a 100644
--- a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs
+++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs
@@ -39,7 +39,7 @@ namespace osu.Game.Screens.Select.Carousel
 
             beatmapSet.Beatmaps
                       .Where(b => !b.Hidden)
-                      .OrderBy(b => b.RulesetID)
+                      .OrderBy(b => b.Ruleset.SortID)
                       .ThenBy(b => b.StarRating)
                       .Select(b => new CarouselBeatmap(b))
                       .ForEach(AddChild);