Merge pull request #20758 from frenzibyte/maintain-common-mod-selection

Maintain selection of common mods when switching rulesets

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

@@ -2,7 +2,6 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
-using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
 using System.Threading.Tasks;
@@ -32,7 +31,6 @@ using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Rulesets.Osu.Mods;
-using osu.Game.Rulesets.Taiko;
 using osu.Game.Scoring;
 using osu.Game.Screens.Play;
 using osu.Game.Screens.Select;
@@ -538,36 +536,6 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddUntilStep("selection shown on wedge", () => songSelect!.CurrentBeatmapDetailsBeatmap.BeatmapInfo.MatchesOnlineID(target));
         }
 
-        [Test]
-        public void TestRulesetChangeResetsMods()
-        {
-            createSongSelect();
-            changeRuleset(0);
-
-            changeMods(new OsuModHardRock());
-
-            int actionIndex = 0;
-            int modChangeIndex = 0;
-            int rulesetChangeIndex = 0;
-
-            AddStep("change ruleset", () =>
-            {
-                SelectedMods.ValueChanged += onModChange;
-                songSelect!.Ruleset.ValueChanged += onRulesetChange;
-
-                Ruleset.Value = new TaikoRuleset().RulesetInfo;
-
-                SelectedMods.ValueChanged -= onModChange;
-                songSelect!.Ruleset.ValueChanged -= onRulesetChange;
-            });
-
-            AddAssert("mods changed before ruleset", () => modChangeIndex < rulesetChangeIndex);
-            AddAssert("empty mods", () => !SelectedMods.Value.Any());
-
-            void onModChange(ValueChangedEvent<IReadOnlyList<Mod>> e) => modChangeIndex = actionIndex++;
-            void onRulesetChange(ValueChangedEvent<RulesetInfo> e) => rulesetChangeIndex = actionIndex++;
-        }
-
         [Test]
         public void TestModsRetainedBetweenSongSelect()
         {

osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs

@@ -17,6 +17,7 @@ using osu.Game.Graphics.UserInterface;
 using osu.Game.Overlays.Mods;
 using osu.Game.Overlays.Settings;
 using osu.Game.Rulesets;
+using osu.Game.Rulesets.Catch.Mods;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Rulesets.Osu.Mods;
@@ -338,26 +339,36 @@ namespace osu.Game.Tests.Visual.UserInterface
         }
 
         [Test]
-        public void TestRulesetChanges()
+        public void TestCommonModsMaintainedOnRulesetChange()
         {
             createScreen();
             changeRuleset(0);
 
-            var noFailMod = new OsuRuleset().GetModsFor(ModType.DifficultyReduction).FirstOrDefault(m => m is OsuModNoFail);
+            AddStep("select relax mod", () => SelectedMods.Value = new[] { Ruleset.Value.CreateInstance().CreateMod<ModRelax>() });
-
-            AddStep("set mods externally", () => { SelectedMods.Value = new[] { noFailMod }; });
 
             changeRuleset(0);
+            AddAssert("ensure mod still selected", () => SelectedMods.Value.SingleOrDefault() is OsuModRelax);
 
-            AddAssert("ensure mods still selected", () => SelectedMods.Value.SingleOrDefault(m => m is OsuModNoFail) != null);
+            changeRuleset(2);
+            AddAssert("catch variant selected", () => SelectedMods.Value.SingleOrDefault() is CatchModRelax);
 
             changeRuleset(3);
+            AddAssert("no mod selected", () => SelectedMods.Value.Count == 0);
+        }
 
-            AddAssert("ensure mods not selected", () => SelectedMods.Value.Count == 0);
+        [Test]
-
+        public void TestUncommonModsDiscardedOnRulesetChange()
+        {
+            createScreen();
             changeRuleset(0);
 
-            AddAssert("ensure mods not selected", () => SelectedMods.Value.Count == 0);
+            AddStep("select single tap mod", () => SelectedMods.Value = new[] { new OsuModSingleTap() });
+
+            changeRuleset(0);
+            AddAssert("ensure mod still selected", () => SelectedMods.Value.SingleOrDefault() is OsuModSingleTap);
+
+            changeRuleset(3);
+            AddAssert("no mod selected", () => SelectedMods.Value.Count == 0);
         }
 
         [Test]

osu.Game/OsuGameBase.cs

@@ -616,11 +616,16 @@ namespace osu.Game
                 return;
             }
 
+            var previouslySelectedMods = SelectedMods.Value.ToArray();
+
             if (!SelectedMods.Disabled)
                 SelectedMods.Value = Array.Empty<Mod>();
 
             AvailableMods.Value = dict;
 
+            if (!SelectedMods.Disabled)
+                SelectedMods.Value = previouslySelectedMods.Select(m => instance.CreateModFromAcronym(m.Acronym)).Where(m => m != null).ToArray();
+
             void revertRulesetChange() => Ruleset.Value = r.OldValue?.Available == true ? r.OldValue : RulesetStore.AvailableRulesets.First();
         }
 

osu.Game/Screens/Select/SongSelect.cs

@@ -502,8 +502,6 @@ namespace osu.Game.Screens.Select
 
                 if (transferRulesetValue())
                 {
-                    Mods.Value = Array.Empty<Mod>();
-
                     // transferRulesetValue() may trigger a re-filter. If the current selection does not match the new ruleset, we want to switch away from it.
                     // The default logic on WorkingBeatmap change is to switch to a matching ruleset (see workingBeatmapChanged()), but we don't want that here.
                     // We perform an early selection attempt and clear out the beatmap selection to avoid a second ruleset change (revert).