From b42fa23e42bfd449f534a73d3aefe9551827ff80 Mon Sep 17 00:00:00 2001
From: Luke Knight <lukektx@gmail.com>
Date: Wed, 30 Oct 2024 02:04:03 -0500
Subject: [PATCH 1/4] Prevent key bind conflict on reversion
---
osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs b/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs
index ddf831c23e..97ebde4e2c 100644
--- a/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs
+++ b/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs
@@ -222,7 +222,7 @@ namespace osu.Game.Overlays.Settings.Sections.Input
var button = buttons[i++];
button.UpdateKeyCombination(d);
- tryPersistKeyBinding(button.KeyBinding.Value, advanceToNextBinding: false);
+ tryPersistKeyBinding(button.KeyBinding.Value, advanceToNextBinding: false, restoringBinding: true);
}
isDefault.Value = true;
@@ -489,13 +489,12 @@ namespace osu.Game.Overlays.Settings.Sections.Input
base.OnFocusLost(e);
}
- private void tryPersistKeyBinding(RealmKeyBinding keyBinding, bool advanceToNextBinding)
+ private void tryPersistKeyBinding(RealmKeyBinding keyBinding, bool advanceToNextBinding, bool restoringBinding = false)
{
List<RealmKeyBinding> bindings = GetAllSectionBindings();
RealmKeyBinding? existingBinding = keyBinding.KeyCombination.Equals(new KeyCombination(InputKey.None))
? null
- : bindings.FirstOrDefault(other => other.ID != keyBinding.ID && other.KeyCombination.Equals(keyBinding.KeyCombination));
-
+ : bindings.FirstOrDefault(other => other.ID != keyBinding.ID && other.KeyCombination.Equals(keyBinding.KeyCombination) && (!restoringBinding || other.ActionInt != keyBinding.ActionInt));
if (existingBinding == null)
{
realm.Write(r => r.Find<RealmKeyBinding>(keyBinding.ID)!.KeyCombinationString = keyBinding.KeyCombination.ToString());
From 0f61e19857238a4491a07aeb84140811f8626fe8 Mon Sep 17 00:00:00 2001
From: Luke Knight <lukektx@gmail.com>
Date: Wed, 30 Oct 2024 03:02:51 -0500
Subject: [PATCH 2/4] Fixed formatting for InspectCode
---
osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs | 1 +
1 file changed, 1 insertion(+)
diff --git a/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs b/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs
index 97ebde4e2c..2003c7fef6 100644
--- a/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs
+++ b/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs
@@ -495,6 +495,7 @@ namespace osu.Game.Overlays.Settings.Sections.Input
RealmKeyBinding? existingBinding = keyBinding.KeyCombination.Equals(new KeyCombination(InputKey.None))
? null
: bindings.FirstOrDefault(other => other.ID != keyBinding.ID && other.KeyCombination.Equals(keyBinding.KeyCombination) && (!restoringBinding || other.ActionInt != keyBinding.ActionInt));
+
if (existingBinding == null)
{
realm.Write(r => r.Find<RealmKeyBinding>(keyBinding.ID)!.KeyCombinationString = keyBinding.KeyCombination.ToString());
From 0a33d71671895398743a2ea8b853b588b2ff1794 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Fri, 1 Nov 2024 19:26:56 +0100
Subject: [PATCH 3/4] Add test coverage
---
.../Settings/TestSceneKeyBindingPanel.cs | 55 +++++++++++++++----
1 file changed, 45 insertions(+), 10 deletions(-)
diff --git a/osu.Game.Tests/Visual/Settings/TestSceneKeyBindingPanel.cs b/osu.Game.Tests/Visual/Settings/TestSceneKeyBindingPanel.cs
index 86008a56a4..4cad283833 100644
--- a/osu.Game.Tests/Visual/Settings/TestSceneKeyBindingPanel.cs
+++ b/osu.Game.Tests/Visual/Settings/TestSceneKeyBindingPanel.cs
@@ -414,11 +414,7 @@ namespace osu.Game.Tests.Visual.Settings
});
AddStep("move mouse to centre", () => InputManager.MoveMouseTo(panel.ScreenSpaceDrawQuad.Centre));
scrollToAndStartBinding("Left (centre)");
- AddStep("clear binding", () =>
- {
- var row = panel.ChildrenOfType<KeyBindingRow>().First(r => r.ChildrenOfType<OsuSpriteText>().Any(s => s.Text.ToString() == "Left (centre)"));
- row.ChildrenOfType<KeyBindingRow.ClearButton>().Single().TriggerClick();
- });
+ clearBinding();
scrollToAndStartBinding("Left (rim)");
AddStep("bind M1", () => InputManager.Click(MouseButton.Left));
@@ -431,6 +427,45 @@ namespace osu.Game.Tests.Visual.Settings
AddUntilStep("conflict popover not shown", () => panel.ChildrenOfType<KeyBindingConflictPopover>().SingleOrDefault(), () => Is.Null);
}
+ [Test]
+ public void TestResettingRowCannotConflictWithItself()
+ {
+ AddStep("reset taiko section to default", () =>
+ {
+ var section = panel.ChildrenOfType<VariantBindingsSubsection>().First(section => new TaikoRuleset().RulesetInfo.Equals(section.Ruleset));
+ section.ChildrenOfType<ResetButton>().Single().TriggerClick();
+ });
+ AddStep("move mouse to centre", () => InputManager.MoveMouseTo(panel.ScreenSpaceDrawQuad.Centre));
+
+ scrollToAndStartBinding("Left (centre)");
+ clearBinding();
+ scrollToAndStartBinding("Left (centre)", 1);
+ clearBinding();
+
+ scrollToAndStartBinding("Left (centre)");
+ AddStep("bind F", () => InputManager.Key(Key.F));
+ scrollToAndStartBinding("Left (centre)", 1);
+ AddStep("bind M1", () => InputManager.Click(MouseButton.Left));
+
+ AddStep("revert row to default", () =>
+ {
+ var row = panel.ChildrenOfType<KeyBindingRow>().First(r => r.ChildrenOfType<OsuSpriteText>().Any(s => s.Text.ToString() == "Left (centre)"));
+ InputManager.MoveMouseTo(row.ChildrenOfType<RevertToDefaultButton<bool>>().Single());
+ InputManager.Click(MouseButton.Left);
+ });
+ AddWaitStep("wait a bit", 3);
+ AddUntilStep("conflict popover not shown", () => panel.ChildrenOfType<KeyBindingConflictPopover>().SingleOrDefault(), () => Is.Null);
+ }
+
+ private void clearBinding()
+ {
+ AddStep("clear binding", () =>
+ {
+ var row = panel.ChildrenOfType<KeyBindingRow>().First(r => r.ChildrenOfType<OsuSpriteText>().Any(s => s.Text.ToString() == "Left (centre)"));
+ row.ChildrenOfType<KeyBindingRow.ClearButton>().Single().TriggerClick();
+ });
+ }
+
private void checkBinding(string name, string keyName)
{
AddAssert($"Check {name} is bound to {keyName}", () =>
@@ -442,23 +477,23 @@ namespace osu.Game.Tests.Visual.Settings
}, () => Is.EqualTo(keyName));
}
- private void scrollToAndStartBinding(string name)
+ private void scrollToAndStartBinding(string name, int bindingIndex = 0)
{
- KeyBindingRow.KeyButton firstButton = null;
+ KeyBindingRow.KeyButton targetButton = null;
AddStep($"Scroll to {name}", () =>
{
var firstRow = panel.ChildrenOfType<KeyBindingRow>().First(r => r.ChildrenOfType<OsuSpriteText>().Any(s => s.Text.ToString() == name));
- firstButton = firstRow.ChildrenOfType<KeyBindingRow.KeyButton>().First();
+ targetButton = firstRow.ChildrenOfType<KeyBindingRow.KeyButton>().ElementAt(bindingIndex);
- panel.ChildrenOfType<SettingsPanel.SettingsSectionsContainer>().First().ScrollTo(firstButton);
+ panel.ChildrenOfType<SettingsPanel.SettingsSectionsContainer>().First().ScrollTo(targetButton);
});
AddWaitStep("wait for scroll", 5);
AddStep("click to bind", () =>
{
- InputManager.MoveMouseTo(firstButton);
+ InputManager.MoveMouseTo(targetButton);
InputManager.Click(MouseButton.Left);
});
}
From f5a2674f6696cce7ba483bd25bf57c897af1bb30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Fri, 1 Nov 2024 19:26:59 +0100
Subject: [PATCH 4/4] Rewrite fix in a more legible way
- Use better param name ("restoring" what bindings? the key information
there is that the *defaults* are being restored)
- Split ugly and not easily understandable (but probably
best-that-can-be-done) conditional out to a method and comment it
appropriately
---
.../Settings/Sections/Input/KeyBindingRow.cs | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs b/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs
index 2003c7fef6..083c678176 100644
--- a/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs
+++ b/osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.cs
@@ -222,7 +222,7 @@ namespace osu.Game.Overlays.Settings.Sections.Input
var button = buttons[i++];
button.UpdateKeyCombination(d);
- tryPersistKeyBinding(button.KeyBinding.Value, advanceToNextBinding: false, restoringBinding: true);
+ tryPersistKeyBinding(button.KeyBinding.Value, advanceToNextBinding: false, restoringDefaults: true);
}
isDefault.Value = true;
@@ -489,12 +489,25 @@ namespace osu.Game.Overlays.Settings.Sections.Input
base.OnFocusLost(e);
}
- private void tryPersistKeyBinding(RealmKeyBinding keyBinding, bool advanceToNextBinding, bool restoringBinding = false)
+ private bool isConflictingBinding(RealmKeyBinding first, RealmKeyBinding second, bool restoringDefaults)
+ {
+ if (first.ID == second.ID)
+ return false;
+
+ // ignore conflicts with same action bindings during revert. the assumption is that the other binding will be reverted subsequently in the same higher-level operation.
+ // this happens if the bindings for an action are rebound to the same keys, but the ordering of the bindings itself is different.
+ if (restoringDefaults && first.ActionInt == second.ActionInt)
+ return false;
+
+ return first.KeyCombination.Equals(second.KeyCombination);
+ }
+
+ private void tryPersistKeyBinding(RealmKeyBinding keyBinding, bool advanceToNextBinding, bool restoringDefaults = false)
{
List<RealmKeyBinding> bindings = GetAllSectionBindings();
RealmKeyBinding? existingBinding = keyBinding.KeyCombination.Equals(new KeyCombination(InputKey.None))
? null
- : bindings.FirstOrDefault(other => other.ID != keyBinding.ID && other.KeyCombination.Equals(keyBinding.KeyCombination) && (!restoringBinding || other.ActionInt != keyBinding.ActionInt));
+ : bindings.FirstOrDefault(other => isConflictingBinding(keyBinding, other, restoringDefaults));
if (existingBinding == null)
{