From a1def8d3fefb56f7993c9926abb758b6862bd08f Mon Sep 17 00:00:00 2001 From: Susko3 Date: Wed, 27 Mar 2024 16:43:01 +0100 Subject: [PATCH 01/16] Fix wrong plurals in param names --- osu.Framework/Input/Bindings/KeyCombination.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index 489ca886ab..78c5818ba5 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -106,39 +106,39 @@ public bool IsPressed(KeyCombination pressedKeys, KeyCombinationMatchingMode mat /// /// Check whether the provided set of pressed keys matches the candidate binding. /// - /// The candidate key binding to match against. - /// The keys which have been pressed by a user. + /// The candidate key binding to match against. + /// The keys which have been pressed by a user. /// The matching mode to be used when checking. /// Whether this is a match. [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static bool ContainsAll(ImmutableArray candidateKey, ImmutableArray pressedKey, KeyCombinationMatchingMode matchingMode) + internal static bool ContainsAll(ImmutableArray candidate, ImmutableArray pressedKeys, KeyCombinationMatchingMode matchingMode) { // first, check that all the candidate keys are contained in the provided pressed keys. // regardless of the matching mode, every key needs to at least be present (matching modes only change // the behaviour of excess keys). - foreach (var key in candidateKey) + foreach (var key in candidate) { - if (!ContainsKey(pressedKey, key)) + if (!ContainsKey(pressedKeys, key)) return false; } switch (matchingMode) { case KeyCombinationMatchingMode.Exact: - foreach (var key in pressedKey) + foreach (var key in pressedKeys) { // in exact matching mode, every pressed key needs to be in the candidate. - if (!ContainsKeyPermissive(candidateKey, key)) + if (!ContainsKeyPermissive(candidate, key)) return false; } break; case KeyCombinationMatchingMode.Modifiers: - foreach (var key in pressedKey) + foreach (var key in pressedKeys) { // in modifiers match mode, the same check applies as exact but only for modifier keys. - if (IsModifierKey(key) && !ContainsKeyPermissive(candidateKey, key)) + if (IsModifierKey(key) && !ContainsKeyPermissive(candidate, key)) return false; } From e05083a750ce673b13d358db98edc17053495a31 Mon Sep 17 00:00:00 2001 From: Susko3 Date: Wed, 27 Mar 2024 17:05:32 +0100 Subject: [PATCH 02/16] Don't use `KeyCombination.ContainsKey` for two different purposes --- osu.Framework/Input/Bindings/KeyCombination.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index 78c5818ba5..187e594480 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -192,7 +192,7 @@ internal static bool ContainsKeyPermissive(ImmutableArray candidate, I break; } - return ContainsKey(candidate, key); + return candidate.Contains(key); } /// From 3e021ddd197138a97504d59d00fb217aa7868510 Mon Sep 17 00:00:00 2001 From: Susko3 Date: Wed, 27 Mar 2024 17:08:08 +0100 Subject: [PATCH 03/16] Remove invalid failing tests These are invalid because they have the virtual `InputKey.Shift` as a pressed key, which never happens in real usage. This is because `KeyCombination.FromKey` will never return a virtual key like `Shift` or `Control`. --- osu.Framework.Tests/Input/KeyCombinationTest.cs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/osu.Framework.Tests/Input/KeyCombinationTest.cs b/osu.Framework.Tests/Input/KeyCombinationTest.cs index 618c260ce3..d495dee1d5 100644 --- a/osu.Framework.Tests/Input/KeyCombinationTest.cs +++ b/osu.Framework.Tests/Input/KeyCombinationTest.cs @@ -29,22 +29,6 @@ public class KeyCombinationTest new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.Shift), KeyCombinationMatchingMode.Modifiers, true }, new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Modifiers, false }, new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Modifiers, true }, - - // test multiple combination matches. - new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.Shift, InputKey.LShift), KeyCombinationMatchingMode.Any, true }, - new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.Shift, InputKey.RShift), KeyCombinationMatchingMode.Any, true }, - new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.Shift, InputKey.RShift), KeyCombinationMatchingMode.Any, false }, - new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.Shift, InputKey.RShift), KeyCombinationMatchingMode.Any, true }, - - new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.Shift, InputKey.LShift), KeyCombinationMatchingMode.Exact, true }, - new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.Shift, InputKey.RShift), KeyCombinationMatchingMode.Exact, true }, - new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.Shift, InputKey.RShift), KeyCombinationMatchingMode.Exact, false }, - new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.Shift, InputKey.RShift), KeyCombinationMatchingMode.Exact, true }, - - new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.Shift, InputKey.LShift), KeyCombinationMatchingMode.Modifiers, true }, - new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.Shift, InputKey.RShift), KeyCombinationMatchingMode.Modifiers, true }, - new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.Shift, InputKey.RShift), KeyCombinationMatchingMode.Modifiers, false }, - new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.Shift, InputKey.RShift), KeyCombinationMatchingMode.Modifiers, true }, }; [TestCaseSource(nameof(key_combination_display_test_cases))] From e2316289efcba705a5a48390d3fc6c32495861a6 Mon Sep 17 00:00:00 2001 From: Susko3 Date: Wed, 27 Mar 2024 17:14:58 +0100 Subject: [PATCH 04/16] Fix `KeyCombination.ContainsKey` xmldoc and param names --- .../Input/Bindings/KeyCombination.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index 187e594480..0722e559ee 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -197,41 +197,41 @@ internal static bool ContainsKeyPermissive(ImmutableArray candidate, I /// /// Check whether a single key from a candidate binding is relevant to the currently pressed keys. - /// If the contains a left/right specific modifier, the must also for this to match. + /// If the contain a left/right specific modifier, the needs to be either that specific modifier or its generic variant to match. /// - /// The candidate key binding to match against. - /// The key which has been pressed by a user. + /// The currently pressed keys to match against. + /// The candidate key to check. /// Whether this is a match. - internal static bool ContainsKey(ImmutableArray candidate, InputKey key) + internal static bool ContainsKey(ImmutableArray pressedKeys, InputKey candidateKey) { - switch (key) + switch (candidateKey) { case InputKey.Control: - if (candidate.Contains(InputKey.LControl) || candidate.Contains(InputKey.RControl)) + if (pressedKeys.Contains(InputKey.LControl) || pressedKeys.Contains(InputKey.RControl)) return true; break; case InputKey.Shift: - if (candidate.Contains(InputKey.LShift) || candidate.Contains(InputKey.RShift)) + if (pressedKeys.Contains(InputKey.LShift) || pressedKeys.Contains(InputKey.RShift)) return true; break; case InputKey.Alt: - if (candidate.Contains(InputKey.LAlt) || candidate.Contains(InputKey.RAlt)) + if (pressedKeys.Contains(InputKey.LAlt) || pressedKeys.Contains(InputKey.RAlt)) return true; break; case InputKey.Super: - if (candidate.Contains(InputKey.LSuper) || candidate.Contains(InputKey.RSuper)) + if (pressedKeys.Contains(InputKey.LSuper) || pressedKeys.Contains(InputKey.RSuper)) return true; break; } - return candidate.Contains(key); + return pressedKeys.Contains(candidateKey); } public bool Equals(KeyCombination other) => Keys.SequenceEqual(other.Keys); From 0ab0adc4c7e8206dc41e6e145feaf86c0e0594ff Mon Sep 17 00:00:00 2001 From: Susko3 Date: Wed, 27 Mar 2024 17:42:16 +0100 Subject: [PATCH 05/16] Simplify `ContainsKeyPermissive` and `ContainsKey` by introducing the concept of virtual keys Where and how exactly `getVirtualKey()` works is subject to change. This is just a MVP. --- .../Input/Bindings/KeyCombination.cs | 105 ++++++------------ 1 file changed, 37 insertions(+), 68 deletions(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index 0722e559ee..3d1251b88f 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -103,6 +103,30 @@ public bool IsPressed(KeyCombination pressedKeys, KeyCombinationMatchingMode mat return ContainsAll(Keys, pressedKeys.Keys, matchingMode); } + private static InputKey? getVirtualKey(InputKey key) + { + switch (key) + { + case InputKey.LShift: + case InputKey.RShift: + return InputKey.Shift; + + case InputKey.LControl: + case InputKey.RControl: + return InputKey.Control; + + case InputKey.LAlt: + case InputKey.RAlt: + return InputKey.Alt; + + case InputKey.LSuper: + case InputKey.RSuper: + return InputKey.Super; + } + + return null; + } + /// /// Check whether the provided set of pressed keys matches the candidate binding. /// @@ -113,32 +137,34 @@ public bool IsPressed(KeyCombination pressedKeys, KeyCombinationMatchingMode mat [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static bool ContainsAll(ImmutableArray candidate, ImmutableArray pressedKeys, KeyCombinationMatchingMode matchingMode) { + ImmutableArray<(InputKey Physical, InputKey? Virtual)> pressed = pressedKeys.Select(k => (k, getVirtualKey(k))).ToImmutableArray(); + // first, check that all the candidate keys are contained in the provided pressed keys. // regardless of the matching mode, every key needs to at least be present (matching modes only change // the behaviour of excess keys). foreach (var key in candidate) { - if (!ContainsKey(pressedKeys, key)) + if (!ContainsKey(pressed, key)) return false; } switch (matchingMode) { case KeyCombinationMatchingMode.Exact: - foreach (var key in pressedKeys) + foreach (var key in pressed) { // in exact matching mode, every pressed key needs to be in the candidate. - if (!ContainsKeyPermissive(candidate, key)) + if (!ContainsKeyPermissive(candidate, key.Physical, key.Virtual)) return false; } break; case KeyCombinationMatchingMode.Modifiers: - foreach (var key in pressedKeys) + foreach (var key in pressed) { // in modifiers match mode, the same check applies as exact but only for modifier keys. - if (IsModifierKey(key) && !ContainsKeyPermissive(candidate, key)) + if (IsModifierKey(key.Physical) && !ContainsKeyPermissive(candidate, key.Physical, key.Virtual)) return false; } @@ -157,42 +183,12 @@ internal static bool ContainsAll(ImmutableArray candidate, ImmutableAr /// This will match bidirectionally for modifier keys (LShift and Shift being present in both of the two parameters in either order will return true). /// /// The candidate key binding to match against. - /// The key which has been pressed by a user. + /// The key which has been pressed by a user. + /// The virtual key corresponding to the physical key, if any. /// Whether this is a match. - internal static bool ContainsKeyPermissive(ImmutableArray candidate, InputKey key) + internal static bool ContainsKeyPermissive(ImmutableArray candidate, InputKey physicalKey, InputKey? virtualKey) { - switch (key) - { - case InputKey.LControl: - case InputKey.RControl: - if (candidate.Contains(InputKey.Control)) - return true; - - break; - - case InputKey.LShift: - case InputKey.RShift: - if (candidate.Contains(InputKey.Shift)) - return true; - - break; - - case InputKey.RAlt: - case InputKey.LAlt: - if (candidate.Contains(InputKey.Alt)) - return true; - - break; - - case InputKey.LSuper: - case InputKey.RSuper: - if (candidate.Contains(InputKey.Super)) - return true; - - break; - } - - return candidate.Contains(key); + return candidate.Contains(physicalKey) || (virtualKey != null && candidate.Contains(virtualKey.Value)); } /// @@ -202,36 +198,9 @@ internal static bool ContainsKeyPermissive(ImmutableArray candidate, I /// The currently pressed keys to match against. /// The candidate key to check. /// Whether this is a match. - internal static bool ContainsKey(ImmutableArray pressedKeys, InputKey candidateKey) + internal static bool ContainsKey(ImmutableArray<(InputKey Physical, InputKey? Virtual)> pressedKeys, InputKey candidateKey) { - switch (candidateKey) - { - case InputKey.Control: - if (pressedKeys.Contains(InputKey.LControl) || pressedKeys.Contains(InputKey.RControl)) - return true; - - break; - - case InputKey.Shift: - if (pressedKeys.Contains(InputKey.LShift) || pressedKeys.Contains(InputKey.RShift)) - return true; - - break; - - case InputKey.Alt: - if (pressedKeys.Contains(InputKey.LAlt) || pressedKeys.Contains(InputKey.RAlt)) - return true; - - break; - - case InputKey.Super: - if (pressedKeys.Contains(InputKey.LSuper) || pressedKeys.Contains(InputKey.RSuper)) - return true; - - break; - } - - return pressedKeys.Contains(candidateKey); + return pressedKeys.Any(k => candidateKey == k.Physical || candidateKey == k.Virtual); } public bool Equals(KeyCombination other) => Keys.SequenceEqual(other.Keys); From 560172da50a31dd71550a7dc7c434467e9dee029 Mon Sep 17 00:00:00 2001 From: Susko3 Date: Wed, 27 Mar 2024 17:53:56 +0100 Subject: [PATCH 06/16] Reword xmldoc --- osu.Framework/Input/Bindings/KeyCombination.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index 3d1251b88f..b340e3457d 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -192,8 +192,7 @@ internal static bool ContainsKeyPermissive(ImmutableArray candidate, I } /// - /// Check whether a single key from a candidate binding is relevant to the currently pressed keys. - /// If the contain a left/right specific modifier, the needs to be either that specific modifier or its generic variant to match. + /// Check whether a single physical or virtual key from a candidate binding is relevant to the currently pressed keys. /// /// The currently pressed keys to match against. /// The candidate key to check. From b15194d5db6520aca1a581985c9607c92b44d539 Mon Sep 17 00:00:00 2001 From: Susko3 Date: Wed, 27 Mar 2024 20:00:24 +0100 Subject: [PATCH 07/16] Remove old xmldoc --- osu.Framework/Input/Bindings/KeyCombination.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index b340e3457d..abd6e94872 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -180,7 +180,6 @@ internal static bool ContainsAll(ImmutableArray candidate, ImmutableAr /// /// Check whether the provided key is part of the candidate binding. - /// This will match bidirectionally for modifier keys (LShift and Shift being present in both of the two parameters in either order will return true). /// /// The candidate key binding to match against. /// The key which has been pressed by a user. From fe629c6050f24c2e8a446ddf5c4b94758fcee21d Mon Sep 17 00:00:00 2001 From: Susko3 Date: Fri, 29 Mar 2024 19:58:18 +0100 Subject: [PATCH 08/16] Make the code explicit about physical vs virtual InputKeys --- .../Extensions/InputKeyExtensions.cs | 42 +++++++++++++++++++ .../Input/Bindings/KeyCombination.cs | 12 +++++- 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 osu.Framework/Extensions/InputKeyExtensions.cs diff --git a/osu.Framework/Extensions/InputKeyExtensions.cs b/osu.Framework/Extensions/InputKeyExtensions.cs new file mode 100644 index 0000000000..50b83acd0e --- /dev/null +++ b/osu.Framework/Extensions/InputKeyExtensions.cs @@ -0,0 +1,42 @@ +// Copyright (c) ppy Pty Ltd . 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.Framework.Extensions +{ + public static class InputKeyExtensions + { + public static bool IsPhysical(this InputKey key) + { + if (!Enum.IsDefined(key) || IsVirtual(key)) + return false; + + switch (key) + { + case InputKey.None: + case InputKey.LastKey: + return false; + + default: + return true; + } + } + + public static bool IsVirtual(this InputKey key) + { + switch (key) + { + case InputKey.Shift: + case InputKey.Control: + case InputKey.Alt: + case InputKey.Super: + return true; + + default: + return false; + } + } + } +} diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index abd6e94872..b86fc7e21f 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -7,6 +7,7 @@ using System.Diagnostics; using System.Linq; using System.Runtime.CompilerServices; +using osu.Framework.Extensions; using osu.Framework.Input.States; using osuTK; using osuTK.Input; @@ -137,8 +138,11 @@ public bool IsPressed(KeyCombination pressedKeys, KeyCombinationMatchingMode mat [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static bool ContainsAll(ImmutableArray candidate, ImmutableArray pressedKeys, KeyCombinationMatchingMode matchingMode) { + Debug.Assert(pressedKeys.All(k => k.IsPhysical())); ImmutableArray<(InputKey Physical, InputKey? Virtual)> pressed = pressedKeys.Select(k => (k, getVirtualKey(k))).ToImmutableArray(); + Debug.Assert(pressed.All(k => k.Virtual == null || k.Virtual.Value.IsVirtual())); + // first, check that all the candidate keys are contained in the provided pressed keys. // regardless of the matching mode, every key needs to at least be present (matching modes only change // the behaviour of excess keys). @@ -198,7 +202,13 @@ internal static bool ContainsKeyPermissive(ImmutableArray candidate, I /// Whether this is a match. internal static bool ContainsKey(ImmutableArray<(InputKey Physical, InputKey? Virtual)> pressedKeys, InputKey candidateKey) { - return pressedKeys.Any(k => candidateKey == k.Physical || candidateKey == k.Virtual); + if (candidateKey.IsPhysical()) + { + return pressedKeys.Any(k => k.Physical == candidateKey); + } + + Debug.Assert(candidateKey.IsVirtual()); + return pressedKeys.Any(k => k.Virtual == candidateKey); } public bool Equals(KeyCombination other) => Keys.SequenceEqual(other.Keys); From 2334498915dc7403cba43eb848a1bf3f980287ed Mon Sep 17 00:00:00 2001 From: Susko3 Date: Fri, 29 Mar 2024 19:58:55 +0100 Subject: [PATCH 09/16] Remove even more invalid tests These may have been useful at some point, but not anymore. --- osu.Framework.Tests/Input/KeyCombinationTest.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Framework.Tests/Input/KeyCombinationTest.cs b/osu.Framework.Tests/Input/KeyCombinationTest.cs index d495dee1d5..efaadf57c1 100644 --- a/osu.Framework.Tests/Input/KeyCombinationTest.cs +++ b/osu.Framework.Tests/Input/KeyCombinationTest.cs @@ -14,19 +14,16 @@ public class KeyCombinationTest // test single combination matches. new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.LShift), KeyCombinationMatchingMode.Any, true }, new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Any, true }, - new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.Shift), KeyCombinationMatchingMode.Any, true }, new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Any, false }, new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Any, true }, new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.LShift), KeyCombinationMatchingMode.Exact, true }, new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Exact, true }, - new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.Shift), KeyCombinationMatchingMode.Exact, true }, new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Exact, false }, new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Exact, true }, new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.LShift), KeyCombinationMatchingMode.Modifiers, true }, new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Modifiers, true }, - new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.Shift), KeyCombinationMatchingMode.Modifiers, true }, new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Modifiers, false }, new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Modifiers, true }, }; From 588fd36203d9092352ad9da2e3a68820bbfd309e Mon Sep 17 00:00:00 2001 From: Susko3 Date: Fri, 29 Mar 2024 20:58:43 +0100 Subject: [PATCH 10/16] Rename methods Could be better but it's a start. --- osu.Framework/Input/Bindings/KeyCombination.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index b86fc7e21f..1be0130ab8 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -148,7 +148,7 @@ internal static bool ContainsAll(ImmutableArray candidate, ImmutableAr // the behaviour of excess keys). foreach (var key in candidate) { - if (!ContainsKey(pressed, key)) + if (!IsPressed(pressed, key)) return false; } @@ -158,7 +158,7 @@ internal static bool ContainsAll(ImmutableArray candidate, ImmutableAr foreach (var key in pressed) { // in exact matching mode, every pressed key needs to be in the candidate. - if (!ContainsKeyPermissive(candidate, key.Physical, key.Virtual)) + if (!ContainsKey(candidate, key.Physical, key.Virtual)) return false; } @@ -168,7 +168,7 @@ internal static bool ContainsAll(ImmutableArray candidate, ImmutableAr foreach (var key in pressed) { // in modifiers match mode, the same check applies as exact but only for modifier keys. - if (IsModifierKey(key.Physical) && !ContainsKeyPermissive(candidate, key.Physical, key.Virtual)) + if (IsModifierKey(key.Physical) && !ContainsKey(candidate, key.Physical, key.Virtual)) return false; } @@ -189,7 +189,7 @@ internal static bool ContainsAll(ImmutableArray candidate, ImmutableAr /// The key which has been pressed by a user. /// The virtual key corresponding to the physical key, if any. /// Whether this is a match. - internal static bool ContainsKeyPermissive(ImmutableArray candidate, InputKey physicalKey, InputKey? virtualKey) + internal static bool ContainsKey(ImmutableArray candidate, InputKey physicalKey, InputKey? virtualKey) { return candidate.Contains(physicalKey) || (virtualKey != null && candidate.Contains(virtualKey.Value)); } @@ -200,7 +200,7 @@ internal static bool ContainsKeyPermissive(ImmutableArray candidate, I /// The currently pressed keys to match against. /// The candidate key to check. /// Whether this is a match. - internal static bool ContainsKey(ImmutableArray<(InputKey Physical, InputKey? Virtual)> pressedKeys, InputKey candidateKey) + internal static bool IsPressed(ImmutableArray<(InputKey Physical, InputKey? Virtual)> pressedKeys, InputKey candidateKey) { if (candidateKey.IsPhysical()) { From e21e951059c83c68d5219822f2d345f0a141d93d Mon Sep 17 00:00:00 2001 From: Susko3 Date: Wed, 3 Apr 2024 11:40:27 +0200 Subject: [PATCH 11/16] Use more descriptive names Per bdach's suggestion. --- .../Input/Bindings/KeyCombination.cs | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index 1be0130ab8..83ce7faf96 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -131,22 +131,22 @@ public bool IsPressed(KeyCombination pressedKeys, KeyCombinationMatchingMode mat /// /// Check whether the provided set of pressed keys matches the candidate binding. /// - /// The candidate key binding to match against. - /// The keys which have been pressed by a user. + /// The candidate key binding to match against. + /// The keys which have been pressed by a user. /// The matching mode to be used when checking. /// Whether this is a match. [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static bool ContainsAll(ImmutableArray candidate, ImmutableArray pressedKeys, KeyCombinationMatchingMode matchingMode) + internal static bool ContainsAll(ImmutableArray candidateKeyBinding, ImmutableArray pressedPhysicalKeys, KeyCombinationMatchingMode matchingMode) { - Debug.Assert(pressedKeys.All(k => k.IsPhysical())); - ImmutableArray<(InputKey Physical, InputKey? Virtual)> pressed = pressedKeys.Select(k => (k, getVirtualKey(k))).ToImmutableArray(); + Debug.Assert(pressedPhysicalKeys.All(k => k.IsPhysical())); + ImmutableArray<(InputKey Physical, InputKey? Virtual)> pressed = pressedPhysicalKeys.Select(k => (k, getVirtualKey(k))).ToImmutableArray(); Debug.Assert(pressed.All(k => k.Virtual == null || k.Virtual.Value.IsVirtual())); // first, check that all the candidate keys are contained in the provided pressed keys. // regardless of the matching mode, every key needs to at least be present (matching modes only change // the behaviour of excess keys). - foreach (var key in candidate) + foreach (var key in candidateKeyBinding) { if (!IsPressed(pressed, key)) return false; @@ -158,7 +158,7 @@ internal static bool ContainsAll(ImmutableArray candidate, ImmutableAr foreach (var key in pressed) { // in exact matching mode, every pressed key needs to be in the candidate. - if (!ContainsKey(candidate, key.Physical, key.Virtual)) + if (!KeyBindingContains(candidateKeyBinding, key)) return false; } @@ -168,7 +168,7 @@ internal static bool ContainsAll(ImmutableArray candidate, ImmutableAr foreach (var key in pressed) { // in modifiers match mode, the same check applies as exact but only for modifier keys. - if (IsModifierKey(key.Physical) && !ContainsKey(candidate, key.Physical, key.Virtual)) + if (IsModifierKey(key.Physical) && !KeyBindingContains(candidateKeyBinding, key)) return false; } @@ -185,13 +185,12 @@ internal static bool ContainsAll(ImmutableArray candidate, ImmutableAr /// /// Check whether the provided key is part of the candidate binding. /// - /// The candidate key binding to match against. - /// The key which has been pressed by a user. - /// The virtual key corresponding to the physical key, if any. + /// The candidate key binding to match against. + /// Tuple of a physical key that has been pressed by a user and its corresponding virtual key (if any). /// Whether this is a match. - internal static bool ContainsKey(ImmutableArray candidate, InputKey physicalKey, InputKey? virtualKey) + internal static bool KeyBindingContains(ImmutableArray candidateKeyBinding, (InputKey Physical, InputKey? Virtual) key) { - return candidate.Contains(physicalKey) || (virtualKey != null && candidate.Contains(virtualKey.Value)); + return candidateKeyBinding.Contains(key.Physical) || (key.Virtual != null && candidateKeyBinding.Contains(key.Virtual.Value)); } /// From 3a6017e5871ca5294bcd4f45ea479372c9dba604 Mon Sep 17 00:00:00 2001 From: Susko3 Date: Mon, 22 Apr 2024 14:23:40 +0200 Subject: [PATCH 12/16] Refactor to call `getVirtualKey()` as needed Co-authored-by: Dan Balasescu --- .../Input/Bindings/KeyCombination.cs | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index 83ce7faf96..e9e1af88a8 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -139,23 +139,20 @@ public bool IsPressed(KeyCombination pressedKeys, KeyCombinationMatchingMode mat internal static bool ContainsAll(ImmutableArray candidateKeyBinding, ImmutableArray pressedPhysicalKeys, KeyCombinationMatchingMode matchingMode) { Debug.Assert(pressedPhysicalKeys.All(k => k.IsPhysical())); - ImmutableArray<(InputKey Physical, InputKey? Virtual)> pressed = pressedPhysicalKeys.Select(k => (k, getVirtualKey(k))).ToImmutableArray(); - - Debug.Assert(pressed.All(k => k.Virtual == null || k.Virtual.Value.IsVirtual())); // first, check that all the candidate keys are contained in the provided pressed keys. // regardless of the matching mode, every key needs to at least be present (matching modes only change // the behaviour of excess keys). foreach (var key in candidateKeyBinding) { - if (!IsPressed(pressed, key)) + if (!IsPressed(pressedPhysicalKeys, key)) return false; } switch (matchingMode) { case KeyCombinationMatchingMode.Exact: - foreach (var key in pressed) + foreach (var key in pressedPhysicalKeys) { // in exact matching mode, every pressed key needs to be in the candidate. if (!KeyBindingContains(candidateKeyBinding, key)) @@ -165,10 +162,10 @@ internal static bool ContainsAll(ImmutableArray candidateKeyBinding, I break; case KeyCombinationMatchingMode.Modifiers: - foreach (var key in pressed) + foreach (var key in pressedPhysicalKeys) { // in modifiers match mode, the same check applies as exact but only for modifier keys. - if (IsModifierKey(key.Physical) && !KeyBindingContains(candidateKeyBinding, key)) + if (IsModifierKey(key) && !KeyBindingContains(candidateKeyBinding, key)) return false; } @@ -186,28 +183,27 @@ internal static bool ContainsAll(ImmutableArray candidateKeyBinding, I /// Check whether the provided key is part of the candidate binding. /// /// The candidate key binding to match against. - /// Tuple of a physical key that has been pressed by a user and its corresponding virtual key (if any). + /// The physical key that has been pressed. /// Whether this is a match. - internal static bool KeyBindingContains(ImmutableArray candidateKeyBinding, (InputKey Physical, InputKey? Virtual) key) + internal static bool KeyBindingContains(ImmutableArray candidateKeyBinding, InputKey physicalKey) { - return candidateKeyBinding.Contains(key.Physical) || (key.Virtual != null && candidateKeyBinding.Contains(key.Virtual.Value)); + return candidateKeyBinding.Contains(physicalKey) + || (getVirtualKey(physicalKey) is InputKey vKey && candidateKeyBinding.Contains(vKey)); } /// /// Check whether a single physical or virtual key from a candidate binding is relevant to the currently pressed keys. /// - /// The currently pressed keys to match against. + /// The currently pressed keys to match against. /// The candidate key to check. /// Whether this is a match. - internal static bool IsPressed(ImmutableArray<(InputKey Physical, InputKey? Virtual)> pressedKeys, InputKey candidateKey) + internal static bool IsPressed(ImmutableArray pressedPhysicalKeys, InputKey candidateKey) { if (candidateKey.IsPhysical()) - { - return pressedKeys.Any(k => k.Physical == candidateKey); - } + return pressedPhysicalKeys.Contains(candidateKey); Debug.Assert(candidateKey.IsVirtual()); - return pressedKeys.Any(k => k.Virtual == candidateKey); + return pressedPhysicalKeys.Any(k => getVirtualKey(k) == candidateKey); } public bool Equals(KeyCombination other) => Keys.SequenceEqual(other.Keys); From f3fd47646f2f96b3ad46199edd2d81ba9f13c7d8 Mon Sep 17 00:00:00 2001 From: Susko3 Date: Mon, 22 Apr 2024 14:26:20 +0200 Subject: [PATCH 13/16] Add `InputState` param to `getVirtualKey()` To be used in the future. --- .../Input/KeyCombinationTest.cs | 3 +- ...TestSceneReadableKeyCombinationProvider.cs | 4 +-- .../Input/Bindings/KeyBindingContainer.cs | 6 ++-- .../Input/Bindings/KeyCombination.cs | 28 +++++++++++-------- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/osu.Framework.Tests/Input/KeyCombinationTest.cs b/osu.Framework.Tests/Input/KeyCombinationTest.cs index efaadf57c1..40099a9976 100644 --- a/osu.Framework.Tests/Input/KeyCombinationTest.cs +++ b/osu.Framework.Tests/Input/KeyCombinationTest.cs @@ -3,6 +3,7 @@ using NUnit.Framework; using osu.Framework.Input.Bindings; +using osu.Framework.Input.States; namespace osu.Framework.Tests.Input { @@ -30,7 +31,7 @@ public class KeyCombinationTest [TestCaseSource(nameof(key_combination_display_test_cases))] public void TestLeftRightModifierHandling(KeyCombination candidate, KeyCombination pressed, KeyCombinationMatchingMode matchingMode, bool shouldContain) - => Assert.AreEqual(shouldContain, KeyCombination.ContainsAll(candidate.Keys, pressed.Keys, matchingMode)); + => Assert.AreEqual(shouldContain, KeyCombination.ContainsAll(candidate.Keys, pressed.Keys, new InputState(), matchingMode)); [Test] public void TestCreationNoDuplicates() diff --git a/osu.Framework.Tests/Visual/Input/TestSceneReadableKeyCombinationProvider.cs b/osu.Framework.Tests/Visual/Input/TestSceneReadableKeyCombinationProvider.cs index 89df608755..21d8e43af4 100644 --- a/osu.Framework.Tests/Visual/Input/TestSceneReadableKeyCombinationProvider.cs +++ b/osu.Framework.Tests/Visual/Input/TestSceneReadableKeyCombinationProvider.cs @@ -139,7 +139,7 @@ protected override void LoadComplete() protected override bool OnKeyDown(KeyDownEvent e) { - if (keyCombination.IsPressed(new KeyCombination(KeyCombination.FromKey(e.Key)), KeyCombinationMatchingMode.Any)) + if (keyCombination.IsPressed(new KeyCombination(KeyCombination.FromKey(e.Key)), e.CurrentState, KeyCombinationMatchingMode.Any)) box.Colour = Color4.Navy; return base.OnKeyDown(e); @@ -147,7 +147,7 @@ protected override bool OnKeyDown(KeyDownEvent e) protected override void OnKeyUp(KeyUpEvent e) { - if (keyCombination.IsPressed(new KeyCombination(KeyCombination.FromKey(e.Key)), KeyCombinationMatchingMode.Any)) + if (keyCombination.IsPressed(new KeyCombination(KeyCombination.FromKey(e.Key)), e.CurrentState, KeyCombinationMatchingMode.Any)) box.Colour = Color4.DarkGray; base.OnKeyUp(e); diff --git a/osu.Framework/Input/Bindings/KeyBindingContainer.cs b/osu.Framework/Input/Bindings/KeyBindingContainer.cs index 39c1a0fe7c..5f0149ded2 100644 --- a/osu.Framework/Input/Bindings/KeyBindingContainer.cs +++ b/osu.Framework/Input/Bindings/KeyBindingContainer.cs @@ -224,7 +224,7 @@ private bool handleNewPressed(InputState state, InputKey newKey, Vector2? scroll if (pressedBindings.Contains(binding)) continue; - if (binding.KeyCombination.IsPressed(pressedCombination, matchingMode)) + if (binding.KeyCombination.IsPressed(pressedCombination, state, matchingMode)) newlyPressed.Add(binding); } } @@ -251,7 +251,7 @@ private bool handleNewPressed(InputState state, InputKey newKey, Vector2? scroll if (simultaneousMode == SimultaneousBindingMode.None && (matchingMode == KeyCombinationMatchingMode.Exact || matchingMode == KeyCombinationMatchingMode.Modifiers)) { // only want to release pressed actions if no existing bindings would still remain pressed - if (pressedBindings.Count > 0 && !pressedBindings.Any(m => m.KeyCombination.IsPressed(pressedCombination, matchingMode))) + if (pressedBindings.Count > 0 && !pressedBindings.Any(m => m.KeyCombination.IsPressed(pressedCombination, state, matchingMode))) releasePressedActions(state); } @@ -365,7 +365,7 @@ private void handleNewReleased(InputState state, InputKey releasedKey) { var binding = pressedBindings[i]; - if (pressedInputKeys.Count == 0 || !binding.KeyCombination.IsPressed(pressedCombination, KeyCombinationMatchingMode.Any)) + if (pressedInputKeys.Count == 0 || !binding.KeyCombination.IsPressed(pressedCombination, state, KeyCombinationMatchingMode.Any)) { pressedBindings.RemoveAt(i--); PropagateReleased(getInputQueue(binding).Where(d => d.IsRootedAt(this)), state, binding.GetAction()); diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index e9e1af88a8..8778604127 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -92,19 +92,20 @@ private KeyCombination(ImmutableArray keys) /// Check whether the provided pressed keys are valid for this . /// /// The potential pressed keys for this . + /// The current input state. /// The method for handling exact key matches. /// Whether the pressedKeys keys are valid. - public bool IsPressed(KeyCombination pressedKeys, KeyCombinationMatchingMode matchingMode) + public bool IsPressed(KeyCombination pressedKeys, InputState inputState, KeyCombinationMatchingMode matchingMode) { Debug.Assert(!pressedKeys.Keys.Contains(InputKey.None)); // Having None in pressed keys will break IsPressed if (Keys == pressedKeys.Keys) // Fast test for reference equality of underlying array return true; - return ContainsAll(Keys, pressedKeys.Keys, matchingMode); + return ContainsAll(Keys, pressedKeys.Keys, inputState, matchingMode); } - private static InputKey? getVirtualKey(InputKey key) + private static InputKey? getVirtualKey(InputKey key, InputState inputState) { switch (key) { @@ -133,10 +134,11 @@ public bool IsPressed(KeyCombination pressedKeys, KeyCombinationMatchingMode mat /// /// The candidate key binding to match against. /// The keys which have been pressed by a user. + /// The current input state. /// The matching mode to be used when checking. /// Whether this is a match. [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static bool ContainsAll(ImmutableArray candidateKeyBinding, ImmutableArray pressedPhysicalKeys, KeyCombinationMatchingMode matchingMode) + internal static bool ContainsAll(ImmutableArray candidateKeyBinding, ImmutableArray pressedPhysicalKeys, InputState inputState, KeyCombinationMatchingMode matchingMode) { Debug.Assert(pressedPhysicalKeys.All(k => k.IsPhysical())); @@ -145,7 +147,7 @@ internal static bool ContainsAll(ImmutableArray candidateKeyBinding, I // the behaviour of excess keys). foreach (var key in candidateKeyBinding) { - if (!IsPressed(pressedPhysicalKeys, key)) + if (!IsPressed(pressedPhysicalKeys, key, inputState)) return false; } @@ -155,7 +157,7 @@ internal static bool ContainsAll(ImmutableArray candidateKeyBinding, I foreach (var key in pressedPhysicalKeys) { // in exact matching mode, every pressed key needs to be in the candidate. - if (!KeyBindingContains(candidateKeyBinding, key)) + if (!KeyBindingContains(candidateKeyBinding, key, inputState)) return false; } @@ -165,7 +167,7 @@ internal static bool ContainsAll(ImmutableArray candidateKeyBinding, I foreach (var key in pressedPhysicalKeys) { // in modifiers match mode, the same check applies as exact but only for modifier keys. - if (IsModifierKey(key) && !KeyBindingContains(candidateKeyBinding, key)) + if (IsModifierKey(key) && !KeyBindingContains(candidateKeyBinding, key, inputState)) return false; } @@ -184,11 +186,12 @@ internal static bool ContainsAll(ImmutableArray candidateKeyBinding, I /// /// The candidate key binding to match against. /// The physical key that has been pressed. + /// The current input state. /// Whether this is a match. - internal static bool KeyBindingContains(ImmutableArray candidateKeyBinding, InputKey physicalKey) + internal static bool KeyBindingContains(ImmutableArray candidateKeyBinding, InputKey physicalKey, InputState inputState) { - return candidateKeyBinding.Contains(physicalKey) - || (getVirtualKey(physicalKey) is InputKey vKey && candidateKeyBinding.Contains(vKey)); + return candidateKeyBinding.Contains(physicalKey) || + (getVirtualKey(physicalKey, inputState) is InputKey vKey && candidateKeyBinding.Contains(vKey)); } /// @@ -196,14 +199,15 @@ internal static bool KeyBindingContains(ImmutableArray candidateKeyBin /// /// The currently pressed keys to match against. /// The candidate key to check. + /// The current input state. /// Whether this is a match. - internal static bool IsPressed(ImmutableArray pressedPhysicalKeys, InputKey candidateKey) + internal static bool IsPressed(ImmutableArray pressedPhysicalKeys, InputKey candidateKey, InputState inputState) { if (candidateKey.IsPhysical()) return pressedPhysicalKeys.Contains(candidateKey); Debug.Assert(candidateKey.IsVirtual()); - return pressedPhysicalKeys.Any(k => getVirtualKey(k) == candidateKey); + return pressedPhysicalKeys.Any(k => getVirtualKey(k, inputState) == candidateKey); } public bool Equals(KeyCombination other) => Keys.SequenceEqual(other.Keys); From 8335d73b7757650372a08174c7639bdc32d8de4a Mon Sep 17 00:00:00 2001 From: Susko3 Date: Mon, 20 May 2024 16:37:59 +0200 Subject: [PATCH 14/16] Don't pass in `InputState` to inner methods It's still there in `public` `IsPressed` for the breaking change. --- .../Input/Bindings/KeyCombination.cs | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index 8778604127..72c659fb39 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -102,10 +102,10 @@ public bool IsPressed(KeyCombination pressedKeys, InputState inputState, KeyComb if (Keys == pressedKeys.Keys) // Fast test for reference equality of underlying array return true; - return ContainsAll(Keys, pressedKeys.Keys, inputState, matchingMode); + return ContainsAll(Keys, pressedKeys.Keys, matchingMode); } - private static InputKey? getVirtualKey(InputKey key, InputState inputState) + private static InputKey? getVirtualKey(InputKey key) { switch (key) { @@ -134,11 +134,10 @@ public bool IsPressed(KeyCombination pressedKeys, InputState inputState, KeyComb /// /// The candidate key binding to match against. /// The keys which have been pressed by a user. - /// The current input state. /// The matching mode to be used when checking. /// Whether this is a match. [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static bool ContainsAll(ImmutableArray candidateKeyBinding, ImmutableArray pressedPhysicalKeys, InputState inputState, KeyCombinationMatchingMode matchingMode) + internal static bool ContainsAll(ImmutableArray candidateKeyBinding, ImmutableArray pressedPhysicalKeys, KeyCombinationMatchingMode matchingMode) { Debug.Assert(pressedPhysicalKeys.All(k => k.IsPhysical())); @@ -147,7 +146,7 @@ internal static bool ContainsAll(ImmutableArray candidateKeyBinding, I // the behaviour of excess keys). foreach (var key in candidateKeyBinding) { - if (!IsPressed(pressedPhysicalKeys, key, inputState)) + if (!IsPressed(pressedPhysicalKeys, key)) return false; } @@ -157,7 +156,7 @@ internal static bool ContainsAll(ImmutableArray candidateKeyBinding, I foreach (var key in pressedPhysicalKeys) { // in exact matching mode, every pressed key needs to be in the candidate. - if (!KeyBindingContains(candidateKeyBinding, key, inputState)) + if (!KeyBindingContains(candidateKeyBinding, key)) return false; } @@ -167,7 +166,7 @@ internal static bool ContainsAll(ImmutableArray candidateKeyBinding, I foreach (var key in pressedPhysicalKeys) { // in modifiers match mode, the same check applies as exact but only for modifier keys. - if (IsModifierKey(key) && !KeyBindingContains(candidateKeyBinding, key, inputState)) + if (IsModifierKey(key) && !KeyBindingContains(candidateKeyBinding, key)) return false; } @@ -186,12 +185,11 @@ internal static bool ContainsAll(ImmutableArray candidateKeyBinding, I /// /// The candidate key binding to match against. /// The physical key that has been pressed. - /// The current input state. /// Whether this is a match. - internal static bool KeyBindingContains(ImmutableArray candidateKeyBinding, InputKey physicalKey, InputState inputState) + internal static bool KeyBindingContains(ImmutableArray candidateKeyBinding, InputKey physicalKey) { return candidateKeyBinding.Contains(physicalKey) || - (getVirtualKey(physicalKey, inputState) is InputKey vKey && candidateKeyBinding.Contains(vKey)); + (getVirtualKey(physicalKey) is InputKey vKey && candidateKeyBinding.Contains(vKey)); } /// @@ -199,15 +197,14 @@ internal static bool KeyBindingContains(ImmutableArray candidateKeyBin /// /// The currently pressed keys to match against. /// The candidate key to check. - /// The current input state. /// Whether this is a match. - internal static bool IsPressed(ImmutableArray pressedPhysicalKeys, InputKey candidateKey, InputState inputState) + internal static bool IsPressed(ImmutableArray pressedPhysicalKeys, InputKey candidateKey) { if (candidateKey.IsPhysical()) return pressedPhysicalKeys.Contains(candidateKey); Debug.Assert(candidateKey.IsVirtual()); - return pressedPhysicalKeys.Any(k => getVirtualKey(k, inputState) == candidateKey); + return pressedPhysicalKeys.Any(k => getVirtualKey(k) == candidateKey); } public bool Equals(KeyCombination other) => Keys.SequenceEqual(other.Keys); From dbfed584082560fc39c6b9914e792bd381e570b9 Mon Sep 17 00:00:00 2001 From: Susko3 Date: Mon, 20 May 2024 17:35:14 +0200 Subject: [PATCH 15/16] Fix tests --- osu.Framework.Tests/Input/KeyCombinationTest.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Framework.Tests/Input/KeyCombinationTest.cs b/osu.Framework.Tests/Input/KeyCombinationTest.cs index 40099a9976..efaadf57c1 100644 --- a/osu.Framework.Tests/Input/KeyCombinationTest.cs +++ b/osu.Framework.Tests/Input/KeyCombinationTest.cs @@ -3,7 +3,6 @@ using NUnit.Framework; using osu.Framework.Input.Bindings; -using osu.Framework.Input.States; namespace osu.Framework.Tests.Input { @@ -31,7 +30,7 @@ public class KeyCombinationTest [TestCaseSource(nameof(key_combination_display_test_cases))] public void TestLeftRightModifierHandling(KeyCombination candidate, KeyCombination pressed, KeyCombinationMatchingMode matchingMode, bool shouldContain) - => Assert.AreEqual(shouldContain, KeyCombination.ContainsAll(candidate.Keys, pressed.Keys, new InputState(), matchingMode)); + => Assert.AreEqual(shouldContain, KeyCombination.ContainsAll(candidate.Keys, pressed.Keys, matchingMode)); [Test] public void TestCreationNoDuplicates() From 8162563e786629b5aed1b360c8594a7c85dd6cfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 23 May 2024 08:03:43 +0200 Subject: [PATCH 16/16] Bring back removed test coverage by adjusting it Sure, the tests may have been "invalid" due to specifying a pressed virtual key, but the actual coverage they were trying to exercise is useful, and can be recovered just by adjusting the test to actually press both physical modifiers rather than just straight-up removing. --- osu.Framework.Tests/Input/KeyCombinationTest.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/osu.Framework.Tests/Input/KeyCombinationTest.cs b/osu.Framework.Tests/Input/KeyCombinationTest.cs index efaadf57c1..71c5897398 100644 --- a/osu.Framework.Tests/Input/KeyCombinationTest.cs +++ b/osu.Framework.Tests/Input/KeyCombinationTest.cs @@ -26,6 +26,22 @@ public class KeyCombinationTest new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Modifiers, true }, new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Modifiers, false }, new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.RShift), KeyCombinationMatchingMode.Modifiers, true }, + + // test multiple combination matches. + new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.LShift, InputKey.RShift), KeyCombinationMatchingMode.Any, true }, + new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.LShift, InputKey.RShift), KeyCombinationMatchingMode.Any, true }, + new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.LShift, InputKey.RShift), KeyCombinationMatchingMode.Any, true }, + new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.RShift, InputKey.A), KeyCombinationMatchingMode.Any, true }, + + new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.LShift, InputKey.RShift), KeyCombinationMatchingMode.Exact, true }, + new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.LShift, InputKey.RShift), KeyCombinationMatchingMode.Exact, false }, + new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.LShift, InputKey.RShift), KeyCombinationMatchingMode.Exact, false }, + new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.RShift, InputKey.A), KeyCombinationMatchingMode.Exact, false }, + + new object[] { new KeyCombination(InputKey.Shift), new KeyCombination(InputKey.LShift, InputKey.RShift), KeyCombinationMatchingMode.Modifiers, true }, + new object[] { new KeyCombination(InputKey.LShift), new KeyCombination(InputKey.LShift, InputKey.RShift), KeyCombinationMatchingMode.Modifiers, false }, + new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.LShift, InputKey.RShift), KeyCombinationMatchingMode.Modifiers, false }, + new object[] { new KeyCombination(InputKey.RShift), new KeyCombination(InputKey.RShift, InputKey.A), KeyCombinationMatchingMode.Modifiers, true }, }; [TestCaseSource(nameof(key_combination_display_test_cases))]