Restructure everything to fix free mod overlay issue

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

@@ -179,7 +179,7 @@ namespace osu.Game.Tests.Visual.UserInterface
         }
 
         [Test]
-        public void TestClassicKeyboardSelection()
+        public void TestClassicKeyboardExclusiveSelection()
         {
             AddStep("set classic hotkey mode", () => configManager.SetValue(OsuSetting.ModSelectHotkeyStyle, ModSelectHotkeyStyle.Classic));
 
@@ -188,7 +188,7 @@ namespace osu.Game.Tests.Visual.UserInterface
             {
                 RelativeSizeAxes = Axes.Both,
                 Padding = new MarginPadding(30),
-                Child = column = new ModColumn(ModType.DifficultyIncrease, true)
+                Child = column = new ModColumn(ModType.DifficultyIncrease, false)
                 {
                     Anchor = Anchor.Centre,
                     Origin = Anchor.Centre,
@@ -228,6 +228,53 @@ namespace osu.Game.Tests.Visual.UserInterface
             AddAssert("no change", () => this.ChildrenOfType<ModPanel>().Single(panel => panel.Active.Value).Mod.Acronym == "NC");
         }
 
+        [Test]
+        public void TestClassicKeyboardIncompatibleSelection()
+        {
+            AddStep("set classic hotkey mode", () => configManager.SetValue(OsuSetting.ModSelectHotkeyStyle, ModSelectHotkeyStyle.Classic));
+
+            ModColumn column = null!;
+            AddStep("create content", () => Child = new Container
+            {
+                RelativeSizeAxes = Axes.Both,
+                Padding = new MarginPadding(30),
+                Child = column = new ModColumn(ModType.DifficultyIncrease, true)
+                {
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                    AvailableMods = getExampleModsFor(ModType.DifficultyIncrease)
+                }
+            });
+
+            AddUntilStep("wait for panel load", () => column.IsLoaded && column.ItemsLoaded);
+
+            AddStep("press A", () => InputManager.Key(Key.A));
+            AddAssert("HR panel selected", () => this.ChildrenOfType<ModPanel>().Single(panel => panel.Mod.Acronym == "HR").Active.Value);
+
+            AddStep("press A again", () => InputManager.Key(Key.A));
+            AddAssert("HR panel deselected", () => !this.ChildrenOfType<ModPanel>().Single(panel => panel.Mod.Acronym == "HR").Active.Value);
+
+            AddStep("press D", () => InputManager.Key(Key.D));
+            AddAssert("DT panel selected", () => this.ChildrenOfType<ModPanel>().Single(panel => panel.Mod.Acronym == "DT").Active.Value);
+            AddAssert("NC panel selected", () => this.ChildrenOfType<ModPanel>().Single(panel => panel.Mod.Acronym == "NC").Active.Value);
+
+            AddStep("press D again", () => InputManager.Key(Key.D));
+            AddAssert("DT panel deselected", () => !this.ChildrenOfType<ModPanel>().Single(panel => panel.Mod.Acronym == "DT").Active.Value);
+            AddAssert("NC panel deselected", () => !this.ChildrenOfType<ModPanel>().Single(panel => panel.Mod.Acronym == "NC").Active.Value);
+
+            AddStep("press Shift-D", () =>
+            {
+                InputManager.PressKey(Key.ShiftLeft);
+                InputManager.Key(Key.D);
+                InputManager.ReleaseKey(Key.ShiftLeft);
+            });
+            AddAssert("DT panel selected", () => this.ChildrenOfType<ModPanel>().Single(panel => panel.Mod.Acronym == "DT").Active.Value);
+            AddAssert("NC panel selected", () => this.ChildrenOfType<ModPanel>().Single(panel => panel.Mod.Acronym == "NC").Active.Value);
+
+            AddStep("press J", () => InputManager.Key(Key.J));
+            AddAssert("no change", () => this.ChildrenOfType<ModPanel>().Count(panel => panel.Active.Value) == 2);
+        }
+
         private void setFilter(Func<Mod, bool>? filter)
         {
             foreach (var modState in this.ChildrenOfType<ModColumn>().Single().AvailableMods)
@@ -238,8 +285,8 @@ namespace osu.Game.Tests.Visual.UserInterface
         {
             public new bool SelectionAnimationRunning => base.SelectionAnimationRunning;
 
-            public TestModColumn(ModType modType, bool allowBulkSelection)
+            public TestModColumn(ModType modType, bool allowIncompatibleSelection)
-                : base(modType, allowBulkSelection)
+                : base(modType, allowIncompatibleSelection)
             {
             }
         }

osu.Game/Overlays/Mods/Input/ClassicModHotkeyHandler.cs

@@ -30,6 +30,13 @@ namespace osu.Game.Overlays.Mods.Input
             [Key.V] = new[] { typeof(ModAutoplay), typeof(ModCinema) }
         };
 
+        private readonly bool allowIncompatibleSelection;
+
+        public ClassicModHotkeyHandler(bool allowIncompatibleSelection)
+        {
+            this.allowIncompatibleSelection = allowIncompatibleSelection;
+        }
+
         public bool HandleHotkeyPressed(KeyDownEvent e, IEnumerable<ModState> availableMods)
         {
             if (!mod_type_lookup.TryGetValue(e.Key, out var typesToMatch))
@@ -46,8 +53,19 @@ namespace osu.Game.Overlays.Mods.Input
                 return true;
             }
 
-            // we're assuming that only one mod from the group can be active at a time.
+            if (allowIncompatibleSelection)
-            // this is mostly ensured by `IncompatibleMods` definitions, but let's make sure just in case.
+            {
+                // easier path - multiple incompatible mods can be active at a time.
+                // this is used in the free mod select overlay.
+                // in this case, just toggle everything.
+                bool anyActive = matchingMods.Any(mod => mod.Active.Value);
+                foreach (var mod in matchingMods)
+                    mod.Active.Value = !anyActive;
+                return true;
+            }
+
+            // we now know there are multiple possible mods to handle, and only one of them can be active at a time.
+            // let's make sure of this just in case.
             Debug.Assert(matchingMods.Count(modState => modState.Active.Value) <= 1);
             int currentSelectedIndex = Array.FindIndex(matchingMods, modState => modState.Active.Value);
 

osu.Game/Overlays/Mods/Input/ModHotkeyHandler.cs

@@ -1,32 +0,0 @@
-// 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 osu.Game.Rulesets.Mods;
-
-namespace osu.Game.Overlays.Mods.Input
-{
-    /// <summary>
-    /// Static factory class for <see cref="IModHotkeyHandler"/>s.
-    /// </summary>
-    public static class ModHotkeyHandler
-    {
-        /// <summary>
-        /// Creates an appropriate <see cref="IModHotkeyHandler"/> for the given <paramref name="modType"/>.
-        /// </summary>
-        public static IModHotkeyHandler Create(ModType modType, ModSelectHotkeyStyle style)
-        {
-            switch (modType)
-            {
-                case ModType.DifficultyReduction:
-                case ModType.DifficultyIncrease:
-                case ModType.Automation:
-                    return style == ModSelectHotkeyStyle.Sequential
-                        ? (IModHotkeyHandler)SequentialModHotkeyHandler.Create(modType)
-                        : new ClassicModHotkeyHandler();
-
-                default:
-                    return new NoopModHotkeyHandler();
-            }
-        }
-    }
-}

osu.Game/Overlays/Mods/ModColumn.cs

@@ -71,8 +71,7 @@ namespace osu.Game.Overlays.Mods
 
         protected virtual ModPanel CreateModPanel(ModState mod) => new ModPanel(mod);
 
-        private Bindable<ModSelectHotkeyStyle> hotkeyStyle = null!;
+        private readonly bool allowIncompatibleSelection;
-        private IModHotkeyHandler hotkeyHandler = null!;
 
         private readonly TextFlowContainer headerText;
         private readonly Box headerBackground;
@@ -83,14 +82,18 @@ namespace osu.Game.Overlays.Mods
 
         private Colour4 accentColour;
 
+        private Bindable<ModSelectHotkeyStyle> hotkeyStyle = null!;
+        private IModHotkeyHandler hotkeyHandler = null!;
+
         private Task? latestLoadTask;
         internal bool ItemsLoaded => latestLoadTask == null;
 
         private const float header_height = 42;
 
-        public ModColumn(ModType modType, bool allowBulkSelection)
+        public ModColumn(ModType modType, bool allowIncompatibleSelection)
         {
             ModType = modType;
+            this.allowIncompatibleSelection = allowIncompatibleSelection;
 
             Width = 320;
             RelativeSizeAxes = Axes.Y;
@@ -198,7 +201,7 @@ namespace osu.Game.Overlays.Mods
 
             createHeaderText();
 
-            if (allowBulkSelection)
+            if (allowIncompatibleSelection)
             {
                 controlContainer.Height = 35;
                 controlContainer.Add(toggleAllCheckbox = new ToggleAllCheckbox(this)
@@ -253,7 +256,7 @@ namespace osu.Game.Overlays.Mods
             base.LoadComplete();
 
             toggleAllCheckbox?.Current.BindValueChanged(_ => updateToggleAllText(), true);
-            hotkeyStyle.BindValueChanged(val => hotkeyHandler = ModHotkeyHandler.Create(ModType, val.NewValue), true);
+            hotkeyStyle.BindValueChanged(val => hotkeyHandler = CreateHotkeyHandler(val.NewValue), true);
             asyncLoadPanels();
         }
 
@@ -427,6 +430,26 @@ namespace osu.Game.Overlays.Mods
 
         #region Keyboard selection support
 
+        /// <summary>
+        /// Creates an appropriate <see cref="IModHotkeyHandler"/> for this column's <see cref="ModType"/> and
+        /// the supplied <paramref name="hotkeyStyle"/>.
+        /// </summary>
+        protected virtual IModHotkeyHandler CreateHotkeyHandler(ModSelectHotkeyStyle hotkeyStyle)
+        {
+            switch (ModType)
+            {
+                case ModType.DifficultyReduction:
+                case ModType.DifficultyIncrease:
+                case ModType.Automation:
+                    return hotkeyStyle == ModSelectHotkeyStyle.Sequential
+                        ? (IModHotkeyHandler)SequentialModHotkeyHandler.Create(ModType)
+                        : new ClassicModHotkeyHandler(allowIncompatibleSelection);
+
+                default:
+                    return new NoopModHotkeyHandler();
+            }
+        }
+
         protected override bool OnKeyDown(KeyDownEvent e)
         {
             if (e.ControlPressed || e.AltPressed || e.SuperPressed || e.Repeat)

osu.Game/Overlays/Mods/UserModSelectOverlay.cs

@@ -40,8 +40,8 @@ namespace osu.Game.Overlays.Mods
 
         private class UserModColumn : ModColumn
         {
-            public UserModColumn(ModType modType, bool allowBulkSelection)
+            public UserModColumn(ModType modType, bool allowIncompatibleSelection)
-                : base(modType, allowBulkSelection)
+                : base(modType, allowIncompatibleSelection)
             {
             }