Merge pull request #12933 from peppy/fix-scroll-wheel-gameplay-bindings

Disallow using scroll wheel for gameplay bindings

osu.Game.Tests/Visual/Settings/TestSceneKeyBindingPanel.cs

@@ -6,6 +6,7 @@ using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Testing;
 using osu.Framework.Threading;
+using osu.Game.Graphics.Sprites;
 using osu.Game.Overlays;
 using osu.Game.Overlays.KeyBinding;
 using osuTK.Input;
@@ -28,6 +29,39 @@ namespace osu.Game.Tests.Visual.Settings
             panel.Show();
         }
 
+        [SetUpSteps]
+        public void SetUpSteps()
+        {
+            AddStep("Scroll to top", () => panel.ChildrenOfType<SettingsPanel.SettingsSectionsContainer>().First().ScrollToTop());
+            AddWaitStep("wait for scroll", 5);
+        }
+
+        [Test]
+        public void TestBindingMouseWheelToNonGameplay()
+        {
+            scrollToAndStartBinding("Increase volume");
+            AddStep("press k", () => InputManager.Key(Key.K));
+            checkBinding("Increase volume", "K");
+
+            AddStep("click again", () => InputManager.Click(MouseButton.Left));
+            AddStep("scroll mouse wheel", () => InputManager.ScrollVerticalBy(1));
+
+            checkBinding("Increase volume", "Wheel Up");
+        }
+
+        [Test]
+        public void TestBindingMouseWheelToGameplay()
+        {
+            scrollToAndStartBinding("Left button");
+            AddStep("press k", () => InputManager.Key(Key.Z));
+            checkBinding("Left button", "Z");
+
+            AddStep("click again", () => InputManager.Click(MouseButton.Left));
+            AddStep("scroll mouse wheel", () => InputManager.ScrollVerticalBy(1));
+
+            checkBinding("Left button", "Z");
+        }
+
         [Test]
         public void TestClickTwiceOnClearButton()
         {
@@ -135,5 +169,37 @@ namespace osu.Game.Tests.Visual.Settings
 
             AddAssert("first binding selected", () => multiBindingRow.ChildrenOfType<KeyBindingRow.KeyButton>().First().IsBinding);
         }
+
+        private void checkBinding(string name, string keyName)
+        {
+            AddAssert($"Check {name} is bound to {keyName}", () =>
+            {
+                var firstRow = panel.ChildrenOfType<KeyBindingRow>().First(r => r.ChildrenOfType<OsuSpriteText>().Any(s => s.Text == name));
+                var firstButton = firstRow.ChildrenOfType<KeyBindingRow.KeyButton>().First();
+
+                return firstButton.Text.Text == keyName;
+            });
+        }
+
+        private void scrollToAndStartBinding(string name)
+        {
+            KeyBindingRow.KeyButton firstButton = null;
+
+            AddStep($"Scroll to {name}", () =>
+            {
+                var firstRow = panel.ChildrenOfType<KeyBindingRow>().First(r => r.ChildrenOfType<OsuSpriteText>().Any(s => s.Text == name));
+                firstButton = firstRow.ChildrenOfType<KeyBindingRow.KeyButton>().First();
+
+                panel.ChildrenOfType<SettingsPanel.SettingsSectionsContainer>().First().ScrollTo(firstButton);
+            });
+
+            AddWaitStep("wait for scroll", 5);
+
+            AddStep("click to bind", () =>
+            {
+                InputManager.MoveMouseTo(firstButton);
+                InputManager.Click(MouseButton.Left);
+            });
+        }
     }
 }

osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs

@@ -73,10 +73,11 @@ namespace osu.Game.Input.Bindings
             else
             {
                 KeyBindings = store.Query(ruleset?.ID, variant)
+                                   .OrderBy(b => defaults.FindIndex(d => (int)d.Action == b.IntAction))
                                    // this ordering is important to ensure that we read entries from the database in the order
                                    // enforced by DefaultKeyBindings. allow for song select to handle actions that may otherwise
                                    // have been eaten by the music controller due to query order.
-                                   .OrderBy(b => defaults.FindIndex(d => (int)d.Action == b.IntAction)).ToList();
+                                   .ToList();
             }
         }
     }

osu.Game/Input/KeyBindingStore.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using osu.Framework.Input.Bindings;
 using osu.Framework.Platform;
@@ -16,6 +17,17 @@ namespace osu.Game.Input
     {
         public event Action KeyBindingChanged;
 
+        /// <summary>
+        /// Keys which should not be allowed for gameplay input purposes.
+        /// </summary>
+        private static readonly IEnumerable<InputKey> banned_keys = new[]
+        {
+            InputKey.MouseWheelDown,
+            InputKey.MouseWheelLeft,
+            InputKey.MouseWheelUp,
+            InputKey.MouseWheelRight
+        };
+
         public KeyBindingStore(DatabaseContextFactory contextFactory, RulesetStore rulesets, Storage storage = null)
             : base(contextFactory, storage)
         {
@@ -93,6 +105,9 @@ namespace osu.Game.Input
             using (ContextFactory.GetForWrite())
             {
                 var dbKeyBinding = (DatabasedKeyBinding)keyBinding;
+
+                Debug.Assert(dbKeyBinding.RulesetID == null || CheckValidForGameplay(keyBinding.KeyCombination));
+
                 Refresh(ref dbKeyBinding);
 
                 if (dbKeyBinding.KeyCombination.Equals(keyBinding.KeyCombination))
@@ -103,5 +118,16 @@ namespace osu.Game.Input
 
             KeyBindingChanged?.Invoke();
         }
+
+        public static bool CheckValidForGameplay(KeyCombination combination)
+        {
+            foreach (var key in banned_keys)
+            {
+                if (combination.Keys.Contains(key))
+                    return false;
+            }
+
+            return true;
+        }
     }
 }

osu.Game/Overlays/KeyBinding/KeyBindingRow.cs

@@ -16,6 +16,7 @@ using osu.Game.Graphics;
 using osu.Game.Graphics.Sprites;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Input;
+using osu.Game.Input.Bindings;
 using osuTK;
 using osuTK.Graphics;
 using osuTK.Input;
@@ -445,6 +446,9 @@ namespace osu.Game.Overlays.KeyBinding
 
             public void UpdateKeyCombination(KeyCombination newCombination)
             {
+                if ((KeyBinding as DatabasedKeyBinding)?.RulesetID != null && !KeyBindingStore.CheckValidForGameplay(newCombination))
+                    return;
+
                 KeyBinding.KeyCombination = newCombination;
                 Text.Text = KeyBinding.KeyCombination.ReadableString();
             }

osu.Game/Overlays/SettingsPanel.cs

@@ -191,7 +191,7 @@ namespace osu.Game.Overlays
             Padding = new MarginPadding { Top = GetToolbarHeight?.Invoke() ?? 0 };
         }
 
-        protected class SettingsSectionsContainer : SectionsContainer<SettingsSection>
+        public class SettingsSectionsContainer : SectionsContainer<SettingsSection>
         {
             public SearchContainer<SettingsSection> SearchContainer;
 

osu.Game/Rulesets/UI/RulesetInputManager.cs

@@ -13,6 +13,7 @@ using osu.Framework.Input.Events;
 using osu.Framework.Input.StateChanges.Events;
 using osu.Framework.Input.States;
 using osu.Game.Configuration;
+using osu.Game.Input;
 using osu.Game.Input.Bindings;
 using osu.Game.Input.Handlers;
 using osu.Game.Screens.Play;
@@ -169,6 +170,13 @@ namespace osu.Game.Rulesets.UI
                 : base(ruleset, variant, unique)
             {
             }
+
+            protected override void ReloadMappings()
+            {
+                base.ReloadMappings();
+
+                KeyBindings = KeyBindings.Where(b => KeyBindingStore.CheckValidForGameplay(b.KeyCombination)).ToList();
+            }
         }
     }