Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Assets/Tests/Editor/KeyboardKeyNameSuggesterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> suggestions = KeyboardKeyNameSuggester.Suggest("3");

Assert.That(suggestions, Does.Not.Contain("Digit3"));
Assert.That(suggestions, Does.Not.Contain("Numpad3"));
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()
Expand Down
188 changes: 188 additions & 0 deletions Assets/Tests/PlayMode/SimulateKeyboardTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,194 @@ public IEnumerator Press_WithInvalidKey_Should_ReturnFailure()
StringAssert.Contains("Invalid key name", lastResponse.Message);
}

/// <summary>
/// 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.
/// </summary>
[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);
}

/// <summary>
/// Verifies that a signed numeric key is rejected, closing the Enum.TryParse signed-ordinal path.
/// </summary>
[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);
}

/// <summary>
/// Verifies that a comma-separated key list is rejected, closing the Enum.TryParse flag-OR path.
/// </summary>
[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);
}

/// <summary>
/// Verifies that a whitespace-padded digit is rejected, closing the Enum.TryParse path that
/// trimmed the input before reading it as an enum ordinal.
/// </summary>
[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);
}

/// <summary>
/// Verifies that an out-of-range numeric key fails as a validation error instead of throwing
/// ArgumentOutOfRangeException from the keyboard indexer.
/// </summary>
[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);
}

/// <summary>
/// Verifies that the existing Return-to-Enter alias still resolves after key names are whitelisted.
/// </summary>
[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);
}

/// <summary>
/// Verifies that key names resolve case-insensitively, which the whitelist must preserve
/// because Enum.TryParse was previously called with ignoreCase.
/// </summary>
[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);
}

/// <summary>
/// Verifies that Digit3, the name the rejection message advertises for bare digits, is
/// actually accepted.
/// </summary>
[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);
}

/// <summary>
/// Verifies that a whitespace-padded valid key name keeps resolving, since padded correct
/// input was already accepted before key names were whitelisted.
/// </summary>
[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);
}

/// <summary>
/// Verifies that the Return-to-Enter alias still applies when the name is whitespace-padded.
/// </summary>
[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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ public static IReadOnlyList<string> Suggest(string invalidKeyName)
string trimmed = invalidKeyName.Trim();
List<string> suggestions = new();

if (trimmed.Length == 1 && char.IsDigit(trimmed[0]))
// Why not char.IsDigit: it is true for non-ASCII digits such as "3", 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}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,23 +72,34 @@ public async Task<SimulateKeyboardResponse> 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<Key>(normalizedKey, ignoreCase: true, out Key key) || key == Key.None)
if (!DefinedKeysByName.TryGetValue(normalizedKey, out Key key) || key == Key.None)
{
IReadOnlyList<string> 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<string> 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()
};
}
Expand Down Expand Up @@ -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<string, Key> DefinedKeysByName = BuildDefinedKeysByName();

private static IReadOnlyDictionary<string, Key> BuildDefinedKeysByName()
{
string[] names = Enum.GetNames(typeof(Key));
Array values = Enum.GetValues(typeof(Key));
Dictionary<string, Key> keysByName = new(names.Length, StringComparer.OrdinalIgnoreCase);
for (int index = 0; index < names.Length; index++)
{
keysByName[names[index]] = (Key)values.GetValue(index);
}

return keysByName;
}

/// <summary>
/// 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.
/// </summary>
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
Expand Down