Merge pull request #17918 from bdach/mod-overlay/incompatibility-panels-clickable

Allow selecting mods regardless of incompatibility state on new mod select

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

@@ -47,12 +47,22 @@ namespace osu.Game.Tests.Visual.UserInterface
         {
             IncompatibilityDisplayingModPanel panel = null;
 
-            AddStep("create panel with DT", () => Child = panel = new IncompatibilityDisplayingModPanel(new OsuModDoubleTime())
+            AddStep("create panel with DT", () =>
+            {
+                Child = panel = new IncompatibilityDisplayingModPanel(new OsuModDoubleTime())
                 {
                     Anchor = Anchor.Centre,
                     Origin = Anchor.Centre,
                     RelativeSizeAxes = Axes.None,
-                Width = 300
+                    Width = 300,
+                };
+
+                panel.Active.BindValueChanged(active =>
+                {
+                    SelectedMods.Value = active.NewValue
+                        ? Array.Empty<Mod>()
+                        : new[] { panel.Mod };
+                });
             });
 
             clickPanel();
@@ -63,11 +73,6 @@ namespace osu.Game.Tests.Visual.UserInterface
 
             AddStep("set incompatible mod", () => SelectedMods.Value = new[] { new OsuModHalfTime() });
 
-            clickPanel();
-            AddAssert("panel not active", () => !panel.Active.Value);
-
-            AddStep("reset mods", () => SelectedMods.Value = Array.Empty<Mod>());
-
             clickPanel();
             AddAssert("panel active", () => panel.Active.Value);
 

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

@@ -1,6 +1,7 @@
 // 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;
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
@@ -89,6 +90,27 @@ namespace osu.Game.Tests.Visual.UserInterface
             changeRuleset(3);
         }
 
+        [Test]
+        public void TestIncompatibilityToggling()
+        {
+            createScreen();
+            changeRuleset(0);
+
+            AddStep("activate DT", () => getPanelForMod(typeof(OsuModDoubleTime)).TriggerClick());
+            AddAssert("DT active", () => SelectedMods.Value.Single().GetType() == typeof(OsuModDoubleTime));
+
+            AddStep("activate NC", () => getPanelForMod(typeof(OsuModNightcore)).TriggerClick());
+            AddAssert("only NC active", () => SelectedMods.Value.Single().GetType() == typeof(OsuModNightcore));
+
+            AddStep("activate HR", () => getPanelForMod(typeof(OsuModHardRock)).TriggerClick());
+            AddAssert("NC+HR active", () => SelectedMods.Value.Any(mod => mod.GetType() == typeof(OsuModNightcore))
+                                            && SelectedMods.Value.Any(mod => mod.GetType() == typeof(OsuModHardRock)));
+
+            AddStep("activate MR", () => getPanelForMod(typeof(OsuModMirror)).TriggerClick());
+            AddAssert("NC+MR active", () => SelectedMods.Value.Any(mod => mod.GetType() == typeof(OsuModNightcore))
+                                            && SelectedMods.Value.Any(mod => mod.GetType() == typeof(OsuModMirror)));
+        }
+
         [Test]
         public void TestCustomisationToggleState()
         {
@@ -136,5 +158,8 @@ namespace osu.Game.Tests.Visual.UserInterface
             AddAssert($"customisation toggle is {(disabled ? "" : "not ")}disabled", () => getToggle().Active.Disabled == disabled);
             AddAssert($"customisation toggle is {(active ? "" : "not ")}active", () => getToggle().Active.Value == active);
         }
+
+        private ModPanel getPanelForMod(Type modType)
+            => modSelectScreen.ChildrenOfType<ModPanel>().Single(panel => panel.Mod.GetType() == modType);
     }
 }

osu.Game/Overlays/Mods/IncompatibilityDisplayingModPanel.cs

@@ -1,15 +1,12 @@
 // 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;
 using System.Collections.Generic;
 using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
-using osu.Framework.Graphics.Colour;
 using osu.Framework.Graphics.Cursor;
-using osu.Framework.Input.Events;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Utils;
 
@@ -42,41 +39,13 @@ namespace osu.Game.Overlays.Mods
                                  && !ModUtils.CheckCompatibleSet(selectedMods.Value.Append(Mod));
         }
 
+        protected override Colour4 BackgroundColour => incompatible.Value ? (Colour4)ColourProvider.Background6 : base.BackgroundColour;
+        protected override Colour4 ForegroundColour => incompatible.Value ? (Colour4)ColourProvider.Background5 : base.ForegroundColour;
+
         protected override void UpdateState()
         {
-            Action = incompatible.Value ? () => { } : (Action)Active.Toggle;
-
-            if (incompatible.Value)
-            {
-                Colour4 backgroundColour = ColourProvider.Background6;
-                Colour4 textBackgroundColour = ColourProvider.Background5;
-
-                Content.TransformTo(nameof(BorderColour), ColourInfo.GradientVertical(backgroundColour, textBackgroundColour), TRANSITION_DURATION, Easing.OutQuint);
-                Background.FadeColour(backgroundColour, TRANSITION_DURATION, Easing.OutQuint);
-
-                SwitchContainer.ResizeWidthTo(IDLE_SWITCH_WIDTH, TRANSITION_DURATION, Easing.OutQuint);
-                SwitchContainer.FadeColour(Colour4.Gray, TRANSITION_DURATION, Easing.OutQuint);
-                MainContentContainer.TransformTo(nameof(Padding), new MarginPadding
-                {
-                    Left = IDLE_SWITCH_WIDTH,
-                    Right = CORNER_RADIUS
-                }, TRANSITION_DURATION, Easing.OutQuint);
-
-                TextBackground.FadeColour(textBackgroundColour, TRANSITION_DURATION, Easing.OutQuint);
-                TextFlow.FadeColour(Colour4.White.Opacity(0.5f), TRANSITION_DURATION, Easing.OutQuint);
-                return;
-            }
-
-            SwitchContainer.FadeColour(Colour4.White, TRANSITION_DURATION, Easing.OutQuint);
             base.UpdateState();
-        }
+            SwitchContainer.FadeColour(incompatible.Value ? Colour4.Gray : Colour4.White, TRANSITION_DURATION, Easing.OutQuint);
-
-        protected override bool OnMouseDown(MouseDownEvent e)
-        {
-            if (incompatible.Value)
-                return true; // bypasses base call purposely in order to not play out the intermediate state animation.
-
-            return base.OnMouseDown(e);
         }
 
         #region IHasCustomTooltip

osu.Game/Overlays/Mods/ModPanel.cs

@@ -203,20 +203,24 @@ namespace osu.Game.Overlays.Mods
             base.OnMouseUp(e);
         }
 
+        protected virtual Colour4 BackgroundColour => Active.Value ? activeColour.Darken(0.3f) : (Colour4)ColourProvider.Background3;
+        protected virtual Colour4 ForegroundColour => Active.Value ? activeColour : (Colour4)ColourProvider.Background2;
+        protected virtual Colour4 TextColour => Active.Value ? (Colour4)ColourProvider.Background6 : Colour4.White;
+
         protected virtual void UpdateState()
         {
             float targetWidth = Active.Value ? EXPANDED_SWITCH_WIDTH : IDLE_SWITCH_WIDTH;
             double transitionDuration = TRANSITION_DURATION;
 
-            Colour4 textBackgroundColour = Active.Value ? activeColour : (Colour4)ColourProvider.Background2;
+            Colour4 backgroundColour = BackgroundColour;
-            Colour4 mainBackgroundColour = Active.Value ? activeColour.Darken(0.3f) : (Colour4)ColourProvider.Background3;
+            Colour4 foregroundColour = ForegroundColour;
-            Colour4 textColour = Active.Value ? (Colour4)ColourProvider.Background6 : Colour4.White;
+            Colour4 textColour = TextColour;
 
             // Hover affects colour of button background
             if (IsHovered)
             {
-                textBackgroundColour = textBackgroundColour.Lighten(0.1f);
+                backgroundColour = backgroundColour.Lighten(0.1f);
-                mainBackgroundColour = mainBackgroundColour.Lighten(0.1f);
+                foregroundColour = foregroundColour.Lighten(0.1f);
             }
 
             // Mouse down adds a halfway tween of the movement
@@ -226,15 +230,15 @@ namespace osu.Game.Overlays.Mods
                 transitionDuration *= 4;
             }
 
-            Content.TransformTo(nameof(BorderColour), ColourInfo.GradientVertical(mainBackgroundColour, textBackgroundColour), transitionDuration, Easing.OutQuint);
+            Content.TransformTo(nameof(BorderColour), ColourInfo.GradientVertical(backgroundColour, foregroundColour), transitionDuration, Easing.OutQuint);
-            Background.FadeColour(mainBackgroundColour, transitionDuration, Easing.OutQuint);
+            Background.FadeColour(backgroundColour, transitionDuration, Easing.OutQuint);
             SwitchContainer.ResizeWidthTo(targetWidth, transitionDuration, Easing.OutQuint);
             MainContentContainer.TransformTo(nameof(Padding), new MarginPadding
             {
                 Left = targetWidth,
                 Right = CORNER_RADIUS
             }, transitionDuration, Easing.OutQuint);
-            TextBackground.FadeColour(textBackgroundColour, transitionDuration, Easing.OutQuint);
+            TextBackground.FadeColour(foregroundColour, transitionDuration, Easing.OutQuint);
             TextFlow.FadeColour(textColour, transitionDuration, Easing.OutQuint);
         }
 

osu.Game/Overlays/Mods/ModSelectScreen.cs

@@ -168,7 +168,7 @@ namespace osu.Game.Overlays.Mods
 
             foreach (var column in columnFlow)
             {
-                column.SelectedMods.BindValueChanged(_ => updateBindableFromSelection());
+                column.SelectedMods.BindValueChanged(updateBindableFromSelection);
             }
 
             customisationVisible.BindValueChanged(_ => updateCustomisationVisualState(), true);
@@ -237,33 +237,36 @@ namespace osu.Game.Overlays.Mods
             TopLevelContent.MoveToY(-modAreaHeight, transition_duration, Easing.InOutCubic);
         }
 
-        private bool selectionBindableSyncInProgress;
-
         private void updateSelectionFromBindable()
         {
-            if (selectionBindableSyncInProgress)
+            // note that selectionBindableSyncInProgress is purposefully not checked here.
-                return;
+            // this is because in the case of mod selection in solo gameplay, a user selection of a mod can actually lead to deselection of other incompatible mods.
-
+            // to synchronise state correctly, updateBindableFromSelection() computes the final mods (including incompatibility rules) and updates SelectedMods,
-            selectionBindableSyncInProgress = true;
+            // and this method then runs unconditionally again to make sure the new visual selection accurately reflects the final set of selected mods.
-
+            // selectionBindableSyncInProgress ensures that mutual infinite recursion does not happen after that unconditional call.
             foreach (var column in columnFlow)
                 column.SelectedMods.Value = SelectedMods.Value.Where(mod => mod.Type == column.ModType).ToArray();
-
-            selectionBindableSyncInProgress = false;
         }
 
-        private void updateBindableFromSelection()
+        private bool selectionBindableSyncInProgress;
+
+        private void updateBindableFromSelection(ValueChangedEvent<IReadOnlyList<Mod>> modSelectionChange)
         {
             if (selectionBindableSyncInProgress)
                 return;
 
             selectionBindableSyncInProgress = true;
 
-            SelectedMods.Value = columnFlow.SelectMany(column => column.SelectedMods.Value).ToArray();
+            SelectedMods.Value = ComputeNewModsFromSelection(
+                modSelectionChange.NewValue.Except(modSelectionChange.OldValue),
+                modSelectionChange.OldValue.Except(modSelectionChange.NewValue));
 
             selectionBindableSyncInProgress = false;
         }
 
+        protected virtual IReadOnlyList<Mod> ComputeNewModsFromSelection(IEnumerable<Mod> addedMods, IEnumerable<Mod> removedMods)
+            => columnFlow.SelectMany(column => column.SelectedMods.Value).ToArray();
+
         protected override void PopIn()
         {
             const double fade_in_duration = 400;

osu.Game/Overlays/Mods/UserModSelectScreen.cs

@@ -1,8 +1,11 @@
 // 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.Collections.Generic;
+using System.Linq;
 using JetBrains.Annotations;
 using osu.Game.Rulesets.Mods;
+using osu.Game.Utils;
 using osuTK.Input;
 
 namespace osu.Game.Overlays.Mods
@@ -11,6 +14,24 @@ namespace osu.Game.Overlays.Mods
     {
         protected override ModColumn CreateModColumn(ModType modType, Key[] toggleKeys = null) => new UserModColumn(modType, false, toggleKeys);
 
+        protected override IReadOnlyList<Mod> ComputeNewModsFromSelection(IEnumerable<Mod> addedMods, IEnumerable<Mod> removedMods)
+        {
+            IEnumerable<Mod> modsAfterRemoval = SelectedMods.Value.Except(removedMods).ToList();
+
+            // the preference is that all new mods should override potential incompatible old mods.
+            // in general that's a bit difficult to compute if more than one mod is added at a time,
+            // so be conservative and just remove all mods that aren't compatible with any one added mod.
+            foreach (var addedMod in addedMods)
+            {
+                if (!ModUtils.CheckCompatibleSet(modsAfterRemoval.Append(addedMod), out var invalidMods))
+                    modsAfterRemoval = modsAfterRemoval.Except(invalidMods);
+
+                modsAfterRemoval = modsAfterRemoval.Append(addedMod).ToList();
+            }
+
+            return modsAfterRemoval.ToList();
+        }
+
         private class UserModColumn : ModColumn
         {
             public UserModColumn(ModType modType, bool allowBulkSelection, [CanBeNull] Key[] toggleKeys = null)