Merge pull request #25152 from bdach/key-binding-deduplication

Clear pre-existing bindings of same key combination to single action

osu.Game.Rulesets.Catch/CatchRuleset.cs

@@ -54,7 +54,7 @@ public class CatchRuleset : Ruleset, ILegacyRuleset
             new KeyBinding(InputKey.X, CatchAction.MoveRight),
             new KeyBinding(InputKey.Right, CatchAction.MoveRight),
             new KeyBinding(InputKey.Shift, CatchAction.Dash),
-            new KeyBinding(InputKey.Shift, CatchAction.Dash),
+            new KeyBinding(InputKey.MouseLeft, CatchAction.Dash),
         };
 
         public override IEnumerable<Mod> ConvertFromLegacyMods(LegacyMods mods)

osu.Game.Tests/Input/RealmKeyBindingStoreTest.cs

@@ -0,0 +1,117 @@
+// 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 NUnit.Framework;
+using osu.Framework.Input.Bindings;
+using osu.Game.Input;
+using osu.Game.Input.Bindings;
+using osuTK.Input;
+
+namespace osu.Game.Tests.Input
+{
+    [TestFixture]
+    public class RealmKeyBindingStoreTest
+    {
+        [Test]
+        public void TestBindingsWithoutDuplicatesAreNotModified()
+        {
+            var bindings = new List<RealmKeyBinding>
+            {
+                new RealmKeyBinding(GlobalAction.Back, KeyCombination.FromKey(Key.Escape)),
+                new RealmKeyBinding(GlobalAction.Back, KeyCombination.FromMouseButton(MouseButton.Button1)),
+                new RealmKeyBinding(GlobalAction.MusicPrev, KeyCombination.FromKey(Key.F1)),
+                new RealmKeyBinding(GlobalAction.MusicNext, KeyCombination.FromKey(Key.F5))
+            };
+
+            int countCleared = RealmKeyBindingStore.ClearDuplicateBindings(bindings);
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(countCleared, Is.Zero);
+
+                Assert.That(bindings[0].Action, Is.EqualTo((int)GlobalAction.Back));
+                Assert.That(bindings[0].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.Escape)));
+
+                Assert.That(bindings[1].Action, Is.EqualTo((int)GlobalAction.Back));
+                Assert.That(bindings[1].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.ExtraMouseButton1)));
+
+                Assert.That(bindings[2].Action, Is.EqualTo((int)GlobalAction.MusicPrev));
+                Assert.That(bindings[2].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.F1)));
+
+                Assert.That(bindings[3].Action, Is.EqualTo((int)GlobalAction.MusicNext));
+                Assert.That(bindings[3].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.F5)));
+            });
+        }
+
+        [Test]
+        public void TestDuplicateBindingsAreCleared()
+        {
+            var bindings = new List<RealmKeyBinding>
+            {
+                new RealmKeyBinding(GlobalAction.Back, KeyCombination.FromKey(Key.Escape)),
+                new RealmKeyBinding(GlobalAction.Back, KeyCombination.FromMouseButton(MouseButton.Button1)),
+                new RealmKeyBinding(GlobalAction.MusicPrev, KeyCombination.FromKey(Key.F1)),
+                new RealmKeyBinding(GlobalAction.IncreaseVolume, KeyCombination.FromKey(Key.Escape)),
+                new RealmKeyBinding(GlobalAction.MusicNext, KeyCombination.FromKey(Key.F5)),
+                new RealmKeyBinding(GlobalAction.ExportReplay, KeyCombination.FromKey(Key.F1)),
+                new RealmKeyBinding(GlobalAction.TakeScreenshot, KeyCombination.FromKey(Key.PrintScreen)),
+            };
+
+            int countCleared = RealmKeyBindingStore.ClearDuplicateBindings(bindings);
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(countCleared, Is.EqualTo(4));
+
+                Assert.That(bindings[0].Action, Is.EqualTo((int)GlobalAction.Back));
+                Assert.That(bindings[0].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.None)));
+
+                Assert.That(bindings[1].Action, Is.EqualTo((int)GlobalAction.Back));
+                Assert.That(bindings[1].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.ExtraMouseButton1)));
+
+                Assert.That(bindings[2].Action, Is.EqualTo((int)GlobalAction.MusicPrev));
+                Assert.That(bindings[2].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.None)));
+
+                Assert.That(bindings[3].Action, Is.EqualTo((int)GlobalAction.IncreaseVolume));
+                Assert.That(bindings[3].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.None)));
+
+                Assert.That(bindings[4].Action, Is.EqualTo((int)GlobalAction.MusicNext));
+                Assert.That(bindings[4].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.F5)));
+
+                Assert.That(bindings[5].Action, Is.EqualTo((int)GlobalAction.ExportReplay));
+                Assert.That(bindings[5].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.None)));
+
+                Assert.That(bindings[6].Action, Is.EqualTo((int)GlobalAction.TakeScreenshot));
+                Assert.That(bindings[6].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.PrintScreen)));
+            });
+        }
+
+        [Test]
+        public void TestDuplicateBindingsAllowedIfBoundToSameAction()
+        {
+            var bindings = new List<RealmKeyBinding>
+            {
+                new RealmKeyBinding(GlobalAction.Back, KeyCombination.FromKey(Key.Escape)),
+                new RealmKeyBinding(GlobalAction.Back, KeyCombination.FromKey(Key.Escape)),
+                new RealmKeyBinding(GlobalAction.MusicPrev, KeyCombination.FromKey(Key.F1)),
+            };
+
+            int countCleared = RealmKeyBindingStore.ClearDuplicateBindings(bindings);
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(countCleared, Is.EqualTo(0));
+
+                Assert.That(bindings[0].Action, Is.EqualTo((int)GlobalAction.Back));
+                Assert.That(bindings[0].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.Escape)));
+
+                Assert.That(bindings[1].Action, Is.EqualTo((int)GlobalAction.Back));
+                Assert.That(bindings[1].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.Escape)));
+
+                Assert.That(bindings[2].Action, Is.EqualTo((int)GlobalAction.MusicPrev));
+                Assert.That(bindings[2].KeyCombination, Is.EqualTo(new KeyCombination(InputKey.F1)));
+            });
+        }
+    }
+}

osu.Game/Database/RealmAccess.cs

@@ -25,6 +25,7 @@
 using osu.Game.Beatmaps.Legacy;
 using osu.Game.Configuration;
 using osu.Game.Extensions;
+using osu.Game.Input;
 using osu.Game.Input.Bindings;
 using osu.Game.Models;
 using osu.Game.Online.API;
@@ -34,6 +35,7 @@
 using osu.Game.Scoring;
 using osu.Game.Scoring.Legacy;
 using osu.Game.Skinning;
+using osuTK.Input;
 using Realms;
 using Realms.Exceptions;
 
@@ -84,8 +86,9 @@ public class RealmAccess : IDisposable
         /// 32   2023-07-09    Populate legacy scores with the ScoreV2 mod (and restore TotalScore to the legacy total for such scores) using replay files.
         /// 33   2023-08-16    Reset default chat toggle key binding to avoid conflict with newly added leaderboard toggle key binding.
         /// 34   2023-08-21    Add BackgroundReprocessingFailed flag to ScoreInfo to track upgrade failures.
+        /// 35   2023-10-16    Clear key combinations of keybindings that are assigned to more than one action in a given settings section.
         /// </summary>
-        private const int schema_version = 34;
+        private const int schema_version = 35;
 
         /// <summary>
         /// Lock object which is held during <see cref="BlockAllOperations"/> sections, blocking realm retrieval during blocking periods.
@@ -1031,6 +1034,47 @@ private void applyMigrationsForVersion(Migration migration, ulong targetVersion)
 
                     break;
                 }
+
+                case 35:
+                {
+                    // catch used `Shift` twice as a default key combination for dash, which generally was bothersome and causes issues elsewhere.
+                    // the duplicate binding logic below had to account for it, it could also break keybinding conflict resolution on revert-to-default.
+                    // as such, detect this situation and fix it before proceeding further.
+                    var catchDashBindings = migration.NewRealm.All<RealmKeyBinding>()
+                                                     .Where(kb => kb.RulesetName == @"fruits" && kb.ActionInt == 2)
+                                                     .ToList();
+
+                    if (catchDashBindings.All(kb => kb.KeyCombination.Equals(new KeyCombination(InputKey.Shift))))
+                    {
+                        Debug.Assert(catchDashBindings.Count == 2);
+                        catchDashBindings.Last().KeyCombination = KeyCombination.FromMouseButton(MouseButton.Left);
+                    }
+
+                    // with the catch case dealt with, de-duplicate the remaining bindings.
+                    int countCleared = 0;
+
+                    var globalBindings = migration.NewRealm.All<RealmKeyBinding>().Where(kb => kb.RulesetName == null).ToList();
+
+                    foreach (var category in Enum.GetValues<GlobalActionCategory>())
+                    {
+                        var categoryActions = GlobalActionContainer.GetGlobalActionsFor(category).Cast<int>().ToHashSet();
+                        var categoryBindings = globalBindings.Where(kb => categoryActions.Contains(kb.ActionInt));
+                        countCleared += RealmKeyBindingStore.ClearDuplicateBindings(categoryBindings);
+                    }
+
+                    var rulesetBindings = migration.NewRealm.All<RealmKeyBinding>().Where(kb => kb.RulesetName != null).ToList();
+
+                    foreach (var variantGroup in rulesetBindings.GroupBy(kb => (kb.RulesetName, kb.Variant)))
+                        countCleared += RealmKeyBindingStore.ClearDuplicateBindings(variantGroup);
+
+                    if (countCleared > 0)
+                    {
+                        Logger.Log($"{countCleared} of your keybinding(s) have been cleared due to being bound to multiple actions. "
+                                   + "Please choose new unique ones in the settings panel.", level: LogLevel.Important);
+                    }
+
+                    break;
+                }
             }
 
             Logger.Log($"Migration completed in {stopwatch.ElapsedMilliseconds}ms");

osu.Game/Input/RealmKeyBindingStore.cs

@@ -130,5 +130,31 @@ public static bool CheckValidForGameplay(KeyCombination combination)
 
             return true;
         }
+
+        /// <summary>
+        /// Clears all <see cref="RealmKeyBinding.KeyCombination"/>s from the provided <paramref name="keyBindings"/>
+        /// which are assigned to more than one binding.
+        /// </summary>
+        /// <param name="keyBindings">The <see cref="RealmKeyBinding"/>s to de-duplicate.</param>
+        /// <returns>Number of bindings cleared.</returns>
+        public static int ClearDuplicateBindings(IEnumerable<IKeyBinding> keyBindings)
+        {
+            int countRemoved = 0;
+
+            var lookup = keyBindings.ToLookup(kb => kb.KeyCombination);
+
+            foreach (var group in lookup)
+            {
+                if (group.Select(kb => kb.Action).Distinct().Count() <= 1)
+                    continue;
+
+                foreach (var binding in group)
+                    binding.KeyCombination = new KeyCombination(InputKey.None);
+
+                countRemoved += group.Count();
+            }
+
+            return countRemoved;
+        }
     }
 }

osu.Game/Rulesets/UI/RulesetInputManager.cs

@@ -218,6 +218,7 @@ protected override void ReloadMappings(IQueryable<RealmKeyBinding> realmKeyBindi
                 base.ReloadMappings(realmKeyBindings);
 
                 KeyBindings = KeyBindings.Where(b => RealmKeyBindingStore.CheckValidForGameplay(b.KeyCombination)).ToList();
+                RealmKeyBindingStore.ClearDuplicateBindings(KeyBindings);
             }
         }
     }