Integrate key binding conflict popover into keybindings panel

osu.Game.Tests/Visual/Settings/TestSceneKeyBindingConflictPopover.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 NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Extensions;
@@ -12,6 +13,7 @@ using osu.Game.Graphics.UserInterfaceV2;
 using osu.Game.Overlays;
 using osu.Game.Overlays.Settings.Sections.Input;
 using osu.Game.Rulesets.Osu;
+using osuTK.Input;
 
 namespace osu.Game.Tests.Visual.Settings
 {
@@ -50,10 +52,15 @@ namespace osu.Game.Tests.Visual.Settings
                 Action = this.ShowPopover;
             }
 
-            public Popover GetPopover() => new KeyBindingConflictPopover(
+            public Popover GetPopover() => new KeyBindingConflictPopover
-                OsuAction.LeftButton,
+            {
-                OsuAction.RightButton,
+                ConflictInfo =
-                new KeyCombination(InputKey.Z));
+                {
+                    Value = new KeyBindingConflictInfo(
+                        new ConflictingKeyBinding(Guid.NewGuid(), OsuAction.LeftButton, KeyCombination.FromKey(Key.X), new KeyCombination(InputKey.None)),
+                        new ConflictingKeyBinding(Guid.NewGuid(), OsuAction.RightButton, KeyCombination.FromKey(Key.Z), KeyCombination.FromKey(Key.X)))
+                }
+            };
         }
     }
 }

osu.Game/Overlays/Settings/Sections/Input/KeyBindingConflictInfo.cs

@@ -0,0 +1,15 @@
+// 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 osu.Framework.Input.Bindings;
+
+namespace osu.Game.Overlays.Settings.Sections.Input
+{
+    /// <summary>
+    /// Contains information about the key binding conflict to be resolved.
+    /// </summary>
+    public record KeyBindingConflictInfo(ConflictingKeyBinding Existing, ConflictingKeyBinding New);
+
+    public record ConflictingKeyBinding(Guid ID, object Action, KeyCombination CombinationWhenChosen, KeyCombination CombinationWhenNotChosen);
+}

osu.Game/Overlays/Settings/Sections/Input/KeyBindingConflictPopover.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 osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Extensions;
@@ -10,34 +11,34 @@ using osu.Framework.Graphics.Shapes;
 using osu.Framework.Input;
 using osu.Framework.Input.Bindings;
 using osu.Framework.Input.Events;
+using osu.Game.Database;
 using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.Sprites;
 using osu.Game.Graphics.UserInterfaceV2;
+using osu.Game.Input.Bindings;
 using osuTK;
 
 namespace osu.Game.Overlays.Settings.Sections.Input
 {
     public partial class KeyBindingConflictPopover : OsuPopover
     {
-        private readonly object existingAction;
+        public Bindable<KeyBindingConflictInfo> ConflictInfo { get; } = new Bindable<KeyBindingConflictInfo>();
-        private readonly object newAction;
+
-        private readonly KeyCombination conflictingCombination;
+        public Action? BindingConflictResolved { get; init; }
 
         private ConflictingKeyBindingPreview newPreview = null!;
         private ConflictingKeyBindingPreview existingPreview = null!;
         private HoverableRoundedButton keepExistingButton = null!;
         private HoverableRoundedButton applyNewButton = null!;
 
-        public KeyBindingConflictPopover(object existingAction, object newAction, KeyCombination conflictingCombination)
+        [Resolved]
-        {
+        private RealmAccess realm { get; set; } = null!;
-            this.existingAction = existingAction;
-            this.newAction = newAction;
-            this.conflictingCombination = conflictingCombination;
-        }
 
         [BackgroundDependencyLoader]
-        private void load()
+        private void load() => recreateDisplay();
+
+        private void recreateDisplay()
         {
             Child = new FillFlowContainer
             {
@@ -54,8 +55,8 @@ namespace osu.Game.Overlays.Settings.Sections.Input
                         Text = "The binding you've selected conflicts with another existing binding.",
                         Margin = new MarginPadding { Bottom = 10 }
                     },
-                    existingPreview = new ConflictingKeyBindingPreview(existingAction, conflictingCombination),
+                    existingPreview = new ConflictingKeyBindingPreview(ConflictInfo.Value.Existing.Action, ConflictInfo.Value.Existing.CombinationWhenChosen, ConflictInfo.Value.Existing.CombinationWhenNotChosen),
-                    newPreview = new ConflictingKeyBindingPreview(newAction, conflictingCombination),
+                    newPreview = new ConflictingKeyBindingPreview(ConflictInfo.Value.New.Action, ConflictInfo.Value.New.CombinationWhenChosen, ConflictInfo.Value.New.CombinationWhenNotChosen),
                     new Container
                     {
                         RelativeSizeAxes = Axes.X,
@@ -79,7 +80,7 @@ namespace osu.Game.Overlays.Settings.Sections.Input
                                 Width = 0.48f,
                                 Anchor = Anchor.CentreRight,
                                 Origin = Anchor.CentreRight,
-                                Action = Hide
+                                Action = applyNew
                             }
                         }
                     }
@@ -87,6 +88,32 @@ namespace osu.Game.Overlays.Settings.Sections.Input
             };
         }
 
+        private void applyNew()
+        {
+            // only "apply new" needs to cause actual realm changes, since the flow in `KeyBindingsSubsection` does not actually make db changes
+            // if it detects a binding conflict.
+            // the temporary visual changes will be reverted by calling `Hide()` / `BindingConflictResolved`.
+            realm.Write(r =>
+            {
+                var existingBinding = r.Find<RealmKeyBinding>(ConflictInfo.Value.Existing.ID);
+                existingBinding!.KeyCombinationString = ConflictInfo.Value.Existing.CombinationWhenNotChosen.ToString();
+
+                var newBinding = r.Find<RealmKeyBinding>(ConflictInfo.Value.New.ID);
+                newBinding!.KeyCombinationString = ConflictInfo.Value.Existing.CombinationWhenChosen.ToString();
+            });
+
+            Hide();
+        }
+
+        protected override void PopOut()
+        {
+            base.PopOut();
+
+            // workaround for `VisibilityContainer.PopOut()` being called in `LoadAsyncComplete()`
+            if (IsLoaded)
+                BindingConflictResolved?.Invoke();
+        }
+
         protected override void LoadComplete()
         {
             base.LoadComplete();
@@ -111,7 +138,8 @@ namespace osu.Game.Overlays.Settings.Sections.Input
         private partial class ConflictingKeyBindingPreview : CompositeDrawable
         {
             private readonly object action;
-            private readonly KeyCombination keyCombination;
+            private readonly KeyCombination combinationWhenChosen;
+            private readonly KeyCombination combinationWhenNotChosen;
 
             private OsuSpriteText newBindingText = null!;
 
@@ -123,10 +151,11 @@ namespace osu.Game.Overlays.Settings.Sections.Input
             [Resolved]
             private OsuColour colours { get; set; } = null!;
 
-            public ConflictingKeyBindingPreview(object action, KeyCombination keyCombination)
+            public ConflictingKeyBindingPreview(object action, KeyCombination combinationWhenChosen, KeyCombination combinationWhenNotChosen)
             {
                 this.action = action;
-                this.keyCombination = keyCombination;
+                this.combinationWhenChosen = combinationWhenChosen;
+                this.combinationWhenNotChosen = combinationWhenNotChosen;
             }
 
             [BackgroundDependencyLoader]
@@ -187,7 +216,6 @@ namespace osu.Game.Overlays.Settings.Sections.Input
                                             {
                                                 Font = OsuFont.Numeric.With(size: 10),
                                                 Margin = new MarginPadding(5),
-                                                Text = keyCombinationProvider.GetReadableString(keyCombination),
                                                 Anchor = Anchor.Centre,
                                                 Origin = Anchor.Centre
                                             }
@@ -209,23 +237,30 @@ namespace osu.Game.Overlays.Settings.Sections.Input
 
             private void updateState()
             {
+                string newBinding;
+
                 switch (IsChosen.Value)
                 {
                     case true:
-                        newBindingText.Text = keyCombinationProvider.GetReadableString(keyCombination);
+                        newBinding = keyCombinationProvider.GetReadableString(combinationWhenChosen);
                         newBindingText.Colour = colours.Green1;
                         break;
 
                     case false:
-                        newBindingText.Text = "(none)";
+                        newBinding = keyCombinationProvider.GetReadableString(combinationWhenNotChosen);
                         newBindingText.Colour = colours.Red1;
                         break;
 
                     case null:
-                        newBindingText.Text = keyCombinationProvider.GetReadableString(keyCombination);
+                        newBinding = keyCombinationProvider.GetReadableString(combinationWhenChosen);
                         newBindingText.Colour = Colour4.White;
                         break;
                 }
+
+                if (string.IsNullOrEmpty(newBinding))
+                    newBinding = "(none)";
+
+                newBindingText.Text = newBinding;
             }
         }
 

osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs

@@ -12,8 +12,10 @@ using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.Cursor;
 using osu.Framework.Graphics.Effects;
 using osu.Framework.Graphics.Shapes;
+using osu.Framework.Graphics.UserInterface;
 using osu.Framework.Input;
 using osu.Framework.Input.Bindings;
 using osu.Framework.Input.Events;
@@ -31,7 +33,7 @@ using osuTK.Input;
 
 namespace osu.Game.Overlays.Settings.Sections.Input
 {
-    public partial class KeyBindingRow : Container, IFilterable
+    public partial class KeyBindingRow : Container, IFilterable, IHasPopover
     {
         /// <summary>
         /// Invoked when the binding of this row is updated with a change being written.
@@ -40,7 +42,9 @@ namespace osu.Game.Overlays.Settings.Sections.Input
 
         public delegate void KeyBindingUpdated(KeyBindingRow sender, KeyBindingUpdatedEventArgs args);
 
-        public record KeyBindingUpdatedEventArgs(Guid KeyBindingID, string KeyCombinationString);
+        public record KeyBindingUpdatedEventArgs(object Action, Guid KeyBindingID, KeyCombination KeyCombination);
+
+        public Action? BindingConflictResolved { get; set; }
 
         /// <summary>
         /// Whether left and right mouse button clicks should be included in the edited bindings.
@@ -77,7 +81,7 @@ namespace osu.Game.Overlays.Settings.Sections.Input
 
         #endregion
 
-        private readonly object action;
+        public readonly object Action;
 
         private Bindable<bool> isDefault { get; } = new BindableBool(true);
 
@@ -107,7 +111,7 @@ namespace osu.Game.Overlays.Settings.Sections.Input
         /// <param name="action">The action that this row contains bindings for.</param>
         public KeyBindingRow(object action)
         {
-            this.action = action;
+            this.Action = action;
 
             RelativeSizeAxes = Axes.X;
             AutoSizeAxes = Axes.Y;
@@ -163,7 +167,7 @@ namespace osu.Game.Overlays.Settings.Sections.Input
                                 },
                                 text = new OsuSpriteText
                                 {
-                                    Text = action.GetLocalisableDescription(),
+                                    Text = Action.GetLocalisableDescription(),
                                     Margin = new MarginPadding(1.5f * padding),
                                 },
                                 buttons = new FillFlowContainer<KeyButton>
@@ -220,7 +224,9 @@ namespace osu.Game.Overlays.Settings.Sections.Input
             {
                 var button = buttons[i++];
                 button.UpdateKeyCombination(d);
-                finalise();
+
+                var args = new KeyBindingUpdatedEventArgs(Action, button.KeyBinding.Value.ID, button.KeyBinding.Value.KeyCombination);
+                BindingUpdated?.Invoke(this, args);
             }
 
             isDefault.Value = true;
@@ -280,7 +286,7 @@ namespace osu.Game.Overlays.Settings.Sections.Input
             Debug.Assert(bindTarget != null);
 
             if (bindTarget.IsHovered)
-                finalise(false);
+                finalise();
             // prevent updating bind target before clear button's action
             else if (!cancelAndClearButtons.Any(b => b.IsHovered))
                 updateBindTarget();
@@ -429,7 +435,7 @@ namespace osu.Game.Overlays.Settings.Sections.Input
                 return;
 
             bindTarget.UpdateKeyCombination(InputKey.None);
-            finalise(false);
+            finalise();
         }
 
         private void finalise(bool hasChanged = true)
@@ -439,7 +445,7 @@ namespace osu.Game.Overlays.Settings.Sections.Input
                 updateIsDefaultValue();
 
                 bindTarget.IsBinding = false;
-                var args = new KeyBindingUpdatedEventArgs(bindTarget.KeyBinding.Value.ID, bindTarget.KeyBinding.Value.KeyCombinationString);
+                var args = new KeyBindingUpdatedEventArgs(Action, bindTarget.KeyBinding.Value.ID, bindTarget.KeyBinding.Value.KeyCombination);
                 Schedule(() =>
                 {
                     // schedule to ensure we don't instantly get focus back on next OnMouseClick (see AcceptFocus impl.)
@@ -489,6 +495,24 @@ namespace osu.Game.Overlays.Settings.Sections.Input
             isDefault.Value = KeyBindings.Select(b => b.KeyCombination).SequenceEqual(Defaults);
         }
 
+        #region Handling conflicts
+
+        private readonly Bindable<KeyBindingConflictInfo> keyBindingConflictInfo = new Bindable<KeyBindingConflictInfo>();
+
+        public Popover GetPopover() => new KeyBindingConflictPopover
+        {
+            ConflictInfo = { BindTarget = keyBindingConflictInfo },
+            BindingConflictResolved = BindingConflictResolved
+        };
+
+        public void ShowBindingConflictPopover(KeyBindingConflictInfo conflictInfo)
+        {
+            keyBindingConflictInfo.Value = conflictInfo;
+            this.ShowPopover();
+        }
+
+        #endregion
+
         private partial class CancelButton : RoundedButton
         {
             public CancelButton()

osu.Game/Overlays/Settings/Sections/Input/KeyBindingsSubsection.cs

@@ -38,15 +38,18 @@ namespace osu.Game.Overlays.Settings.Sections.Input
         [BackgroundDependencyLoader]
         private void load()
         {
-            var bindings = realm.Run(r => GetKeyBindings(r).Detach());
+            var bindings = getAllBindings();
 
             foreach (var defaultGroup in Defaults.GroupBy(d => d.Action))
             {
                 int intKey = (int)defaultGroup.Key;
 
-                // one row per valid action.
                 var row = CreateKeyBindingRow(defaultGroup.Key, defaultGroup)
-                    .With(row => row.BindingUpdated = onBindingUpdated);
+                    .With(row =>
+                    {
+                        row.BindingUpdated = onBindingUpdated;
+                        row.BindingConflictResolved = reloadAllBindings;
+                    });
                 row.KeyBindings.AddRange(bindings.Where(b => b.ActionInt.Equals(intKey)));
                 Add(row);
             }
@@ -59,6 +62,8 @@ namespace osu.Game.Overlays.Settings.Sections.Input
 
         protected abstract IEnumerable<RealmKeyBinding> GetKeyBindings(Realm realm);
 
+        private List<RealmKeyBinding> getAllBindings() => realm.Run(r => GetKeyBindings(r).Detach());
+
         protected virtual KeyBindingRow CreateKeyBindingRow(object action, IEnumerable<KeyBinding> defaults)
             => new KeyBindingRow(action)
             {
@@ -66,9 +71,40 @@ namespace osu.Game.Overlays.Settings.Sections.Input
                 Defaults = defaults.Select(d => d.KeyCombination),
             };
 
+        private void reloadAllBindings()
+        {
+            var bindings = getAllBindings();
+
+            foreach (var row in Children.OfType<KeyBindingRow>())
+            {
+                row.KeyBindings.Clear();
+                row.KeyBindings.AddRange(bindings.Where(b => b.ActionInt.Equals((int)row.Action)));
+            }
+        }
+
         private void onBindingUpdated(KeyBindingRow sender, KeyBindingRow.KeyBindingUpdatedEventArgs args)
         {
-            realm.WriteAsync(r => r.Find<RealmKeyBinding>(args.KeyBindingID)!.KeyCombinationString = args.KeyCombinationString);
+            var bindings = getAllBindings();
+            var existingBinding = args.KeyCombination.Equals(new KeyCombination(InputKey.None))
+                ? null
+                : bindings.FirstOrDefault(kb => kb.ID != args.KeyBindingID && kb.KeyCombination.Equals(args.KeyCombination));
+
+            if (existingBinding != null)
+            {
+                // `RealmKeyBinding`'s  `Action` is just an int, always.
+                // we need more than that for proper display, so leverage `Defaults` (which have the correct enum-typed object in `Action` inside).
+                object existingAssignedAction = Defaults.First(binding => (int)binding.Action == existingBinding.ActionInt).Action;
+                var bindingBeforeUpdate = bindings.Single(binding => binding.ID == args.KeyBindingID);
+
+                sender.ShowBindingConflictPopover(
+                    new KeyBindingConflictInfo(
+                        new ConflictingKeyBinding(existingBinding.ID, existingAssignedAction, existingBinding.KeyCombination, new KeyCombination(InputKey.None)),
+                        new ConflictingKeyBinding(bindingBeforeUpdate.ID, args.Action, args.KeyCombination, bindingBeforeUpdate.KeyCombination)));
+
+                return;
+            }
+
+            realm.WriteAsync(r => r.Find<RealmKeyBinding>(args.KeyBindingID)!.KeyCombinationString = args.KeyCombination.ToString());
 
             if (AutoAdvanceTarget)
             {

osu.Game/Overlays/SettingsPanel.cs

@@ -14,6 +14,7 @@ using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.Cursor;
 using osu.Framework.Graphics.Effects;
 using osu.Framework.Graphics.Primitives;
 using osu.Framework.Graphics.Shapes;
@@ -106,39 +107,43 @@ namespace osu.Game.Overlays
                 }
             };
 
-            Add(SectionsContainer = new SettingsSectionsContainer
+            Add(new PopoverContainer
             {
-                Masking = true,
-                EdgeEffect = new EdgeEffectParameters
-                {
-                    Colour = Color4.Black.Opacity(0),
-                    Type = EdgeEffectType.Shadow,
-                    Hollow = true,
-                    Radius = 10
-                },
-                MaskingSmoothness = 0,
                 RelativeSizeAxes = Axes.Both,
-                ExpandableHeader = CreateHeader(),
+                Child = SectionsContainer = new SettingsSectionsContainer
-                SelectedSection = { BindTarget = CurrentSection },
-                FixedHeader = new Container
                 {
-                    RelativeSizeAxes = Axes.X,
+                    Masking = true,
-                    AutoSizeAxes = Axes.Y,
+                    EdgeEffect = new EdgeEffectParameters
-                    Padding = new MarginPadding
                     {
-                        Vertical = 20,
+                        Colour = Color4.Black.Opacity(0),
-                        Horizontal = CONTENT_MARGINS
+                        Type = EdgeEffectType.Shadow,
+                        Hollow = true,
+                        Radius = 10
                     },
-                    Anchor = Anchor.TopCentre,
+                    MaskingSmoothness = 0,
-                    Origin = Anchor.TopCentre,
+                    RelativeSizeAxes = Axes.Both,
-                    Child = searchTextBox = new SeekLimitedSearchTextBox
+                    ExpandableHeader = CreateHeader(),
+                    SelectedSection = { BindTarget = CurrentSection },
+                    FixedHeader = new Container
                     {
                         RelativeSizeAxes = Axes.X,
-                        Origin = Anchor.TopCentre,
+                        AutoSizeAxes = Axes.Y,
+                        Padding = new MarginPadding
+                        {
+                            Vertical = 20,
+                            Horizontal = CONTENT_MARGINS
+                        },
                         Anchor = Anchor.TopCentre,
-                    }
+                        Origin = Anchor.TopCentre,
-                },
+                        Child = searchTextBox = new SeekLimitedSearchTextBox
-                Footer = CreateFooter().With(f => f.Alpha = 0)
+                        {
+                            RelativeSizeAxes = Axes.X,
+                            Origin = Anchor.TopCentre,
+                            Anchor = Anchor.TopCentre,
+                        }
+                    },
+                    Footer = CreateFooter().With(f => f.Alpha = 0)
+                }
             });
 
             if (showSidebar)