From 7318ff3f98cee2e689d1f20ea736a8ec6b4fdd3d Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Mon, 13 Dec 2021 15:26:49 +0900
Subject: [PATCH] Refactor `KeyBindingStore` to clean up any excess bindings
 for individual actions

While this isn't strictly required (outside of rulesets, potentially),
I think it's best that we keep these counts in a sane state.

Right now, it will remove any excess. Arguably, in the case an entry is
found with too many, we should wipe all entries and re-populate the
defaults. Interested in opinions on that before merging. See
https://github.com/ppy/osu/discussions/15925 for an example where wiping
may be the more appropriate behaviour.
---
 osu.Game/Input/RealmKeyBindingStore.cs | 39 ++++++++++++++++++--------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/osu.Game/Input/RealmKeyBindingStore.cs b/osu.Game/Input/RealmKeyBindingStore.cs
index 3bdb0a180d..cb51797685 100644
--- a/osu.Game/Input/RealmKeyBindingStore.cs
+++ b/osu.Game/Input/RealmKeyBindingStore.cs
@@ -81,20 +81,37 @@ namespace osu.Game.Input
             // compare counts in database vs defaults for each action type.
             foreach (var defaultsForAction in defaults.GroupBy(k => k.Action))
             {
-                // avoid performing redundant queries when the database is empty and needs to be re-filled.
-                int existingCount = existingBindings.Count(k => k.RulesetName == rulesetName && k.Variant == variant && k.ActionInt == (int)defaultsForAction.Key);
+                IEnumerable<RealmKeyBinding> existing = existingBindings.Where(k =>
+                    k.RulesetName == rulesetName
+                    && k.Variant == variant
+                    && k.ActionInt == (int)defaultsForAction.Key);
 
-                if (defaultsForAction.Count() <= existingCount)
-                    continue;
+                int defaultsCount = defaultsForAction.Count();
+                int existingCount = existing.Count();
 
-                // insert any defaults which are missing.
-                realm.Add(defaultsForAction.Skip(existingCount).Select(k => new RealmKeyBinding
+                if (defaultsCount > existingCount)
                 {
-                    KeyCombinationString = k.KeyCombination.ToString(),
-                    ActionInt = (int)k.Action,
-                    RulesetName = rulesetName,
-                    Variant = variant
-                }));
+                    // insert any defaults which are missing.
+                    realm.Add(defaultsForAction.Skip(existingCount).Select(k => new RealmKeyBinding
+                    {
+                        KeyCombinationString = k.KeyCombination.ToString(),
+                        ActionInt = (int)k.Action,
+                        RulesetName = rulesetName,
+                        Variant = variant
+                    }));
+                }
+                else if (defaultsCount < existingCount)
+                {
+                    // generally this shouldn't happen, but if the user has more key bindings for an action than we expect,
+                    // remove the last entries until the count matches for sanity.
+                    foreach (var k in existing.TakeLast(existingCount - defaultsCount).ToArray())
+                    {
+                        realm.Remove(k);
+
+                        // Remove from the local flattened/cached list so future lookups don't query now deleted rows.
+                        existingBindings.Remove(k);
+                    }
+                }
             }
         }