diff --git a/Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs b/Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs index 3e3fe00ac..d5ff020b0 100644 --- a/Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs +++ b/Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs @@ -19,6 +19,17 @@ public void Suggest_ForBareDigit_IncludesDigitAndNumpadNames() Assert.That(suggestions, Does.Contain("Numpad3")); } + // Verifies non-ASCII digits do not produce Digit/Numpad names that no Key enum value has. + [Test] + public void Suggest_ForFullWidthDigit_DoesNotSuggestDigitNames() + { + IReadOnlyList suggestions = KeyboardKeyNameSuggester.Suggest("３"); + + Assert.That(suggestions, Does.Not.Contain("Digit３")); + Assert.That(suggestions, Does.Not.Contain("Numpad３")); + Assert.That(suggestions.Any(name => name.StartsWith("Digit")), Is.False); + } + // Verifies partial key names still return close enum matches. [Test] public void Suggest_ForPartialName_ReturnsPrefixMatches() diff --git a/Assets/Tests/PlayMode/SimulateKeyboardTests.cs b/Assets/Tests/PlayMode/SimulateKeyboardTests.cs index 38fafa755..a63ba4fd9 100644 --- a/Assets/Tests/PlayMode/SimulateKeyboardTests.cs +++ b/Assets/Tests/PlayMode/SimulateKeyboardTests.cs @@ -634,6 +634,194 @@ public IEnumerator Press_WithInvalidKey_Should_ReturnFailure() StringAssert.Contains("Invalid key name", lastResponse.Message); } + /// + /// Verifies that a bare digit key is rejected instead of being parsed as an enum ordinal, + /// and that the failure message offers both the Digit and Numpad candidates. + /// + [UnityTest] + public IEnumerator Press_WithBareDigitKey_Should_ReturnFailureSuggestingDigitAndNumpad() + { + yield return null; + + yield return RunTool(new JObject + { + ["action"] = KeyboardAction.Press.ToString(), + ["key"] = "3" + }); + + Assert.IsFalse(lastResponse.Success, "A bare digit must not be accepted as a Key enum ordinal"); + StringAssert.Contains("Digit3", lastResponse.Message); + StringAssert.Contains("Numpad3", lastResponse.Message); + StringAssert.Contains("Digits are not key names", lastResponse.Message); + StringAssert.Contains("re-check any earlier results", lastResponse.Message); + } + + /// + /// Verifies that a signed numeric key is rejected, closing the Enum.TryParse signed-ordinal path. + /// + [UnityTest] + public IEnumerator Press_WithSignedNumericKey_Should_ReturnFailure() + { + yield return null; + + yield return RunTool(new JObject + { + ["action"] = KeyboardAction.Press.ToString(), + ["key"] = "+3" + }); + + Assert.IsFalse(lastResponse.Success, "A signed numeric key must not be accepted as a Key enum ordinal"); + StringAssert.Contains("Digits are not key names", lastResponse.Message); + StringAssert.Contains("re-check any earlier results", lastResponse.Message); + } + + /// + /// Verifies that a comma-separated key list is rejected, closing the Enum.TryParse flag-OR path. + /// + [UnityTest] + public IEnumerator Press_WithCommaSeparatedKeyNames_Should_ReturnFailure() + { + yield return null; + + yield return RunTool(new JObject + { + ["action"] = KeyboardAction.Press.ToString(), + ["key"] = "Space,Enter" + }); + + Assert.IsFalse(lastResponse.Success, "A comma-separated key list must not be OR-ed into a single key"); + StringAssert.Contains("Invalid key name", lastResponse.Message); + // The digit guidance is scoped to numeric input, so it must not appear here. + StringAssert.DoesNotContain("Digits are not key names", lastResponse.Message); + } + + /// + /// Verifies that a whitespace-padded digit is rejected, closing the Enum.TryParse path that + /// trimmed the input before reading it as an enum ordinal. + /// + [UnityTest] + public IEnumerator Press_WithPaddedDigitKey_Should_ReturnFailure() + { + yield return null; + + yield return RunTool(new JObject + { + ["action"] = KeyboardAction.Press.ToString(), + ["key"] = " 3 " + }); + + Assert.IsFalse(lastResponse.Success, "A whitespace-padded digit must not be accepted as a Key enum ordinal"); + StringAssert.Contains("Digits are not key names", lastResponse.Message); + } + + /// + /// Verifies that an out-of-range numeric key fails as a validation error instead of throwing + /// ArgumentOutOfRangeException from the keyboard indexer. + /// + [UnityTest] + public IEnumerator Press_WithUndefinedNumericKey_Should_ReturnFailure() + { + yield return null; + + yield return RunTool(new JObject + { + ["action"] = KeyboardAction.Press.ToString(), + ["key"] = "300" + }); + + Assert.IsFalse(lastResponse.Success, "An undefined numeric key must fail validation rather than throw"); + StringAssert.Contains("Digits are not key names", lastResponse.Message); + StringAssert.Contains("re-check any earlier results", lastResponse.Message); + } + + /// + /// Verifies that the existing Return-to-Enter alias still resolves after key names are whitelisted. + /// + [UnityTest] + public IEnumerator Press_WithReturnAlias_Should_Succeed() + { + yield return null; + + yield return RunTool(new JObject + { + ["action"] = KeyboardAction.Press.ToString(), + ["key"] = "Return" + }); + + Assert.IsTrue(lastResponse.Success, lastResponse.Message); + } + + /// + /// Verifies that key names resolve case-insensitively, which the whitelist must preserve + /// because Enum.TryParse was previously called with ignoreCase. + /// + [UnityTest] + public IEnumerator Press_WithLowercaseKeyName_Should_Succeed() + { + yield return null; + + yield return RunTool(new JObject + { + ["action"] = KeyboardAction.Press.ToString(), + ["key"] = "space" + }); + + Assert.IsTrue(lastResponse.Success, lastResponse.Message); + } + + /// + /// Verifies that Digit3, the name the rejection message advertises for bare digits, is + /// actually accepted. + /// + [UnityTest] + public IEnumerator Press_WithDigitKeyName_Should_Succeed() + { + yield return null; + + yield return RunTool(new JObject + { + ["action"] = KeyboardAction.Press.ToString(), + ["key"] = "Digit3" + }); + + Assert.IsTrue(lastResponse.Success, lastResponse.Message); + } + + /// + /// Verifies that a whitespace-padded valid key name keeps resolving, since padded correct + /// input was already accepted before key names were whitelisted. + /// + [UnityTest] + public IEnumerator Press_WithPaddedKeyName_Should_Succeed() + { + yield return null; + + yield return RunTool(new JObject + { + ["action"] = KeyboardAction.Press.ToString(), + ["key"] = " Space " + }); + + Assert.IsTrue(lastResponse.Success, lastResponse.Message); + } + + /// + /// Verifies that the Return-to-Enter alias still applies when the name is whitespace-padded. + /// + [UnityTest] + public IEnumerator Press_WithPaddedReturnAlias_Should_Succeed() + { + yield return null; + + yield return RunTool(new JObject + { + ["action"] = KeyboardAction.Press.ToString(), + ["key"] = " Return " + }); + + Assert.IsTrue(lastResponse.Success, lastResponse.Message); + } + [UnityTest] public IEnumerator Press_WithEmptyKey_Should_ReturnFailure() { diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs index fdbfa27bf..9c4ec3dbf 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardKeyNameSuggester.cs @@ -26,7 +26,9 @@ public static IReadOnlyList Suggest(string invalidKeyName) string trimmed = invalidKeyName.Trim(); List suggestions = new(); - if (trimmed.Length == 1 && char.IsDigit(trimmed[0])) + // Why not char.IsDigit: it is true for non-ASCII digits such as "３", which have no + // Digit/Numpad enum name, so suggesting them would point at keys that do not exist. + if (trimmed.Length == 1 && trimmed[0] >= '0' && trimmed[0] <= '9') { string digit = trimmed; AddUnique(suggestions, $"Digit{digit}"); diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs index 780e512a8..6b7601e98 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs @@ -72,23 +72,34 @@ public async Task ExecuteAsync( return new SimulateKeyboardResponse { Success = false, - Message = "Key parameter is required. Examples: \"W\", \"Space\", \"LeftShift\", \"A\", \"Enter\".", + Message = "Key parameter is required. Examples: \"W\", \"Space\", \"LeftShift\", \"A\", \"Enter\", \"Digit3\".", Action = parameters.Action.ToString() }; } + // Why not Enum.TryParse alone: it also accepts ordinals ("3"), signed ordinals ("+3"), + // whitespace-padded input, comma-separated names OR-ed together ("Space,Enter"), and + // undefined ordinals ("300") that later throw from the keyboard indexer. Only a name + // that is defined on the Key enum may resolve to a key. string normalizedKey = NormalizeKeyName(parameters.Key); - if (!Enum.TryParse(normalizedKey, ignoreCase: true, out Key key) || key == Key.None) + if (!DefinedKeysByName.TryGetValue(normalizedKey, out Key key) || key == Key.None) { - IReadOnlyList suggestions = KeyboardKeyNameSuggester.Suggest(parameters.Key); + // Suggest from the normalized form so padding does not degrade the candidates, + // while the message below still reports the raw input verbatim. + IReadOnlyList suggestions = KeyboardKeyNameSuggester.Suggest(normalizedKey); string suggestionText = suggestions.Count == 0 ? string.Empty : $" Did you mean: {string.Join(", ", suggestions)}?"; + // Why: digits used to resolve silently to unrelated keys, so earlier runs that + // reported success may have pressed something else and need to be re-checked. + string ordinalHistoryText = LooksLikeNumericKeyInput(parameters.Key) + ? " Digits are not key names: bare digits were previously parsed as enum ordinals (e.g. \"3\" pressed Tab), so re-check any earlier results or scripts that passed digits." + : string.Empty; return new SimulateKeyboardResponse { Success = false, Message = - $"Invalid key name: \"{parameters.Key}\". Use Input System Key enum names (e.g. \"W\", \"Space\", \"LeftShift\", \"A\", \"Enter\").{suggestionText}", + $"Invalid key name: \"{parameters.Key}\". Use Input System Key enum names (e.g. \"W\", \"Space\", \"LeftShift\", \"A\", \"Enter\", \"Digit3\").{suggestionText}{ordinalHistoryText}", Action = parameters.Action.ToString() }; } @@ -230,13 +241,64 @@ private static void EnsureOverlayExists() OverlayCanvasFactory.EnsureExists(); } + // Immutable name-to-value map of the Input System Key enum, so key resolution never falls + // back to Enum.TryParse's ordinal and flag-combination behavior. + private static readonly IReadOnlyDictionary DefinedKeysByName = BuildDefinedKeysByName(); + + private static IReadOnlyDictionary BuildDefinedKeysByName() + { + string[] names = Enum.GetNames(typeof(Key)); + Array values = Enum.GetValues(typeof(Key)); + Dictionary keysByName = new(names.Length, StringComparer.OrdinalIgnoreCase); + for (int index = 0; index < names.Length; index++) + { + keysByName[names[index]] = (Key)values.GetValue(index); + } + + return keysByName; + } + + /// + /// Reports whether the raw key input is the numeric form that Enum.TryParse used to accept + /// as an enum ordinal, so the rejection can explain what earlier runs actually pressed. + /// + private static bool LooksLikeNumericKeyInput(string keyName) + { + string trimmed = keyName.Trim(); + if (trimmed.Length > 0 && (trimmed[0] == '+' || trimmed[0] == '-')) + { + trimmed = trimmed.Substring(1); + } + + if (trimmed.Length == 0) + { + return false; + } + + for (int index = 0; index < trimmed.Length; index++) + { + // Why not char.IsDigit: it is true for non-ASCII digits, which Enum.TryParse never + // accepted as ordinals. Claiming they used to press another key would be false. + if (trimmed[index] < '0' || trimmed[index] > '9') + { + return false; + } + } + + return true; + } + private static string NormalizeKeyName(string keyName) { - if (string.Equals(keyName, "Return", StringComparison.OrdinalIgnoreCase)) + // Why trim here rather than at the whitelist comparison: Enum.TryParse used to accept + // whitespace-padded names, so padded correct input already worked. Blocking ambiguous + // input must not narrow correct input, and the alias has to see the padded form too. + string trimmed = keyName.Trim(); + if (string.Equals(trimmed, "Return", StringComparison.OrdinalIgnoreCase)) { return Key.Enter.ToString(); } - return keyName; + return trimmed; } #endif