diff --git a/Assets/Tests/InputSystem/Plugins/HIDTests.cs b/Assets/Tests/InputSystem/Plugins/HIDTests.cs index 584fdb9518..7d1f919bc6 100644 --- a/Assets/Tests/InputSystem/Plugins/HIDTests.cs +++ b/Assets/Tests/InputSystem/Plugins/HIDTests.cs @@ -13,7 +13,6 @@ using UnityEngine.InputSystem.Layouts; using UnityEngine.InputSystem.Processors; using UnityEngine.InputSystem.Utilities; -using UnityEngine.TestTools; using UnityEngine.TestTools.Utils; ////TODO: add test to make sure we're not grabbing HIDs that have more specific layouts @@ -1312,5 +1311,59 @@ public void Utilities_CanRecognizeVendorDefinedUsages() Assert.That(HID.UsagePageToString((HID.UsagePage) 0xff01), Is.EqualTo("Vendor-Defined")); Assert.That(HID.UsageToString((HID.UsagePage) 0xff01, 0x33), Is.Null); } + + // https://fogbugz.unity3d.com/f/cases/1335465/ + [Test] + [Category("Devices")] + [TestCase("/")] + [TestCase("\\")] + [TestCase(">")] + [TestCase("<")] + [TestCase(" ")] + public void Devices_CanHaveReservedCharactersInHIDDeviceNames(string characterStr) + { + // Il2cpp seems to have trouble with slashes in character literals used in attributes + // so we pass the character as a string instead. + var character = characterStr[0]; + + var hidDescriptor = new HID.HIDDeviceDescriptor + { + usage = (int)HID.GenericDesktop.Joystick, + usagePage = HID.UsagePage.GenericDesktop, + elements = new[] + { + new HID.HIDElementDescriptor { usage = (int)HID.GenericDesktop.X, usagePage = HID.UsagePage.GenericDesktop, reportType = HID.HIDReportType.Input, reportId = 1, reportOffsetInBits = 0, reportSizeInBits = 16 }, + new HID.HIDElementDescriptor { usage = (int)HID.GenericDesktop.Y, usagePage = HID.UsagePage.GenericDesktop, reportType = HID.HIDReportType.Input, reportId = 1, reportOffsetInBits = 16, reportSizeInBits = 16 }, + new HID.HIDElementDescriptor { usage = (int)HID.Button.Primary, usagePage = HID.UsagePage.Button, reportType = HID.HIDReportType.Input, reportId = 1, reportOffsetInBits = 32, reportSizeInBits = 1 }, + new HID.HIDElementDescriptor { usage = (int)HID.Button.Secondary, usagePage = HID.UsagePage.Button, reportType = HID.HIDReportType.Input, reportId = 1, reportOffsetInBits = 33, reportSizeInBits = 1 }, + } + }; + + var device = InputSystem.AddDevice( + new InputDeviceDescription + { + interfaceName = HID.kHIDInterface, + manufacturer = "TestVendor", + product = "Test" + character + "HID", + capabilities = hidDescriptor.ToJson() + }); + + Assert.That(device.name, Is.EqualTo("TestVendor Test" + (character == '/' ? InputControlPath.SeparatorReplacement : character) + "HID")); + Assert.That(device.displayName, Is.EqualTo("Test" + character + "HID")); + + var action = new InputAction(binding: "/space"); + + // Interactively rebind the action to refer to the device's button. + using (action.PerformInteractiveRebinding() + .OnMatchWaitForAnother(0) + .Start()) + { + Press((ButtonControl)device["button2"]); + } + + Assert.That(InputControlPath.TryGetDeviceLayout(action.bindings[0].effectivePath), Is.EqualTo(device.layout)); + Assert.That(InputControlPath.TryGetControlLayout(action.bindings[0].effectivePath), Is.EqualTo("Button")); + Assert.That(action.controls, Is.EquivalentTo(new[] { device["button2"]})); + } } #endif diff --git a/Assets/Tests/InputSystem/Plugins/UITests.cs b/Assets/Tests/InputSystem/Plugins/UITests.cs index 3ea80be773..2ae6e4a380 100644 --- a/Assets/Tests/InputSystem/Plugins/UITests.cs +++ b/Assets/Tests/InputSystem/Plugins/UITests.cs @@ -3412,7 +3412,7 @@ public IEnumerator UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule var scene = SceneManager.LoadScene("UITKTestScene", new LoadSceneParameters(LoadSceneMode.Additive)); yield return null; - Assert.That(scene.isLoaded, Is.True); + Assert.That(scene.isLoaded, Is.True, "UITKTestScene did not load as expected"); try { @@ -3446,17 +3446,17 @@ public IEnumerator UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule yield return null; - Assert.That(uiButton.HasMouseCapture(), Is.True); + Assert.That(uiButton.HasMouseCapture(), Is.True, "Expected uiButton to have mouse capture"); Release(mouse.leftButton, queueEventOnly: true); yield return null; - Assert.That(uiButton.HasMouseCapture(), Is.False); + Assert.That(uiButton.HasMouseCapture(), Is.False, "Expected uiButton to no longer have mouse capture"); Assert.That(clickReceived, Is.True); // Put mouse in upper right corner and scroll down. - Assert.That(scrollView.verticalScroller.value, Is.Zero); + Assert.That(scrollView.verticalScroller.value, Is.Zero, "Expected verticalScroller to be all the way up"); Set(mouse.position, scrollViewCenter, queueEventOnly: true); yield return null; Set(mouse.scroll, new Vector2(0, -100), queueEventOnly: true); @@ -3477,7 +3477,7 @@ public IEnumerator UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule PressAndRelease(gamepad.buttonSouth, queueEventOnly: true); yield return null; - Assert.That(clickReceived, Is.True); + Assert.That(clickReceived, Is.True, "Expected to have received click"); ////TODO: tracked device support (not yet supported by UITK) @@ -3491,40 +3491,40 @@ static bool IsActive(VisualElement ve) yield return null; InputSystem.RemoveDevice(mouse); - int uiButtonDownCount = 0; - int uiButtonUpCount = 0; + var uiButtonDownCount = 0; + var uiButtonUpCount = 0; uiButton.RegisterCallback(e => uiButtonDownCount++, TrickleDown.TrickleDown); uiButton.RegisterCallback(e => uiButtonUpCount++, TrickleDown.TrickleDown); // Case 1369081: Make sure button doesn't get "stuck" in an active state when multiple fingers are used. BeginTouch(1, buttonCenter, screen: touchscreen); yield return null; - Assert.That(uiButtonDownCount, Is.EqualTo(1)); - Assert.That(uiButtonUpCount, Is.EqualTo(0)); - Assert.That(IsActive(uiButton), Is.True); + Assert.That(uiButtonDownCount, Is.EqualTo(1), "Expected uiButtonDownCount to be 0"); + Assert.That(uiButtonUpCount, Is.EqualTo(0), "Expected uiButtonUpCount to be 0"); + Assert.That(IsActive(uiButton), Is.True, "Expected uiButton to be active"); BeginTouch(2, buttonOutside, screen: touchscreen); yield return null; EndTouch(2, buttonOutside, screen: touchscreen); yield return null; - Assert.That(uiButtonDownCount, Is.EqualTo(1)); + Assert.That(uiButtonDownCount, Is.EqualTo(1), "Expected uiButtonDownCount to be 1"); if (pointerBehavior == UIPointerBehavior.SingleUnifiedPointer) { - Assert.That(uiButtonUpCount, Is.EqualTo(1)); - Assert.That(IsActive(uiButton), Is.False); + Assert.That(uiButtonUpCount, Is.EqualTo(1), "Expected uiButtonUpCount to be 1"); + Assert.That(IsActive(uiButton), Is.False, "Expected uiButton to no longer be active"); } else { - Assert.That(uiButtonUpCount, Is.EqualTo(0)); - Assert.That(IsActive(uiButton), Is.True); + Assert.That(uiButtonUpCount, Is.EqualTo(0), "Expected uiButtonUpCount to be 0"); + Assert.That(IsActive(uiButton), Is.True, "Expected uiButton to be active"); } EndTouch(1, buttonCenter, screen: touchscreen); yield return null; - Assert.That(uiButtonDownCount, Is.EqualTo(1)); - Assert.That(uiButtonUpCount, Is.EqualTo(1)); - Assert.That(IsActive(uiButton), Is.False); + Assert.That(uiButtonDownCount, Is.EqualTo(1), "Expected uiButtonDownCount to be 1"); + Assert.That(uiButtonUpCount, Is.EqualTo(1), "Expected uiButtonUpCount to be 1"); + Assert.That(IsActive(uiButton), Is.False, "Expected uiButton to no longer be active"); InputSystem.RemoveDevice(touchscreen); } diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 8f3714857d..0fff8772e9 100755 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -32,6 +32,7 @@ however, it has to be formatted properly to pass verification tests. - Fixed DualSense on iOS not inheriting from `DualShockGamepad` ([case 1378308](https://issuetracker.unity3d.com/issues/input-dualsense-detection-ios)). - Fixed a device becoming `.current` (e.g. `Gamepad.current`, etc) when sending a new state event that contains no control changes (case 1377952). - Fixed calling `IsPressed` on an entire device returning `true` ([case 1374024](https://issuetracker.unity3d.com/issues/inputcontrol-dot-ispressed-always-returns-true-when-using-new-input-system)). +- Fixed HIDs having blackslashes in their vendor or product names leading to binding paths generated by interactive rebinding that failed to resolve to controls and thus lead to no input being received ([case 1335465](https://issuetracker.unity3d.com/product/unity/issues/guid/1335465/)). - Fixed `InputSystem.RegisterLayoutOverride` resulting in the layout that overrides are being applied to losing the connection to its base layout ([case 1377719](https://fogbugz.unity3d.com/f/cases/1377719/)). - Fixed `Touch.activeTouches` still registering touches after the app loses focus ([case 1364017](https://issuetracker.unity3d.com/issues/input-system-new-input-system-registering-active-touches-when-app-loses-focus)). - Fixed `MultiplayerEventSystem` not preventing keyboard and gamepad/joystick navigation from one player's UI moving to another player's UI ([case 1306361](https://issuetracker.unity3d.com/issues/input-system-ui-input-module-lets-the-player-navigate-across-other-canvases)). diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs index 72e860bd4e..b151596d5d 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs @@ -646,7 +646,7 @@ private void EnableControls(InputAction action) var map = action.m_ActionMap; var mapIndex = map.m_MapIndexInState; - Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range"); + Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range in EnableControls"); // Go through all bindings in the map and for all that belong to the given action, // enable the associated controls. @@ -682,7 +682,7 @@ public void DisableAllActions(InputActionMap map) // Mark all actions as disabled. var mapIndex = map.m_MapIndexInState; - Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range"); + Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range in DisableAllActions"); var actionStartIndex = mapIndices[mapIndex].actionStartIndex; var actionCount = mapIndices[mapIndex].actionCount; for (var i = 0; i < actionCount; ++i) @@ -708,7 +708,7 @@ private void DisableControls(InputActionMap map) Debug.Assert(maps.Contains(map), "Map must be contained in state"); var mapIndex = map.m_MapIndexInState; - Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range"); + Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range in DisableControls(InputActionMap)"); // Remove state monitors from all controls. var controlCount = mapIndices[mapIndex].controlCount; @@ -742,7 +742,7 @@ private void DisableControls(InputAction action) var map = action.m_ActionMap; var mapIndex = map.m_MapIndexInState; - Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range"); + Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range in DisableControls(InputAction)"); // Go through all bindings in the map and for all that belong to the given action, // disable the associated controls. @@ -1012,7 +1012,7 @@ private static long ToCombinedMapAndControlAndBindingIndex(int mapIndex, int con /// private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindingIndex, double time, InputEventPtr eventPtr) { - Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range"); + Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range in ProcessControlStateChange"); Debug.Assert(controlIndex >= 0 && controlIndex < totalControlCount, "Control index out of range"); Debug.Assert(bindingIndex >= 0 && bindingIndex < totalBindingCount, "Binding index out of range"); diff --git a/Packages/com.unity.inputsystem/InputSystem/Controls/InputControlExtensions.cs b/Packages/com.unity.inputsystem/InputSystem/Controls/InputControlExtensions.cs index f3e678c19a..d227530c3c 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Controls/InputControlExtensions.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Controls/InputControlExtensions.cs @@ -919,7 +919,7 @@ internal static string BuildPath(this InputControl control, string deviceLayout, var device = control.device; builder.Append('<'); - builder.Append(deviceLayout); + builder.Append(deviceLayout.Escape("\\>", "\\>")); builder.Append('>'); // Add usages of device, if any. @@ -927,14 +927,16 @@ internal static string BuildPath(this InputControl control, string deviceLayout, for (var i = 0; i < deviceUsages.Count; ++i) { builder.Append('{'); - builder.Append(deviceUsages[i]); + builder.Append(deviceUsages[i].ToString().Escape("\\}", "\\}")); builder.Append('}'); } - builder.Append('/'); + builder.Append(InputControlPath.Separator); - var devicePath = device.path; - var controlPath = control.path; + // If any of the components contains a backslash, double it up as in control paths, + // these serve as escape characters. + var devicePath = device.path.Replace("\\", "\\\\"); + var controlPath = control.path.Replace("\\", "\\\\"); builder.Append(controlPath, devicePath.Length + 1, controlPath.Length - devicePath.Length - 1); return builder.ToString(); diff --git a/Packages/com.unity.inputsystem/InputSystem/Controls/InputControlPath.cs b/Packages/com.unity.inputsystem/InputSystem/Controls/InputControlPath.cs index b83385408b..16126758da 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Controls/InputControlPath.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Controls/InputControlPath.cs @@ -97,6 +97,19 @@ public static class InputControlPath public const char Separator = '/'; + // We consider / a reserved character in control names. So, when this character does creep + // in there (e.g. from a device product name), we need to do something about it. We replace + // it with this character here. + // NOTE: Display names have no such restriction. + // NOTE: There are some Unicode characters that look sufficiently like a slash (e.g. FULLWIDTH SOLIDUS) + // but that only makes for rather confusing output. So we just replace with a blank. + internal const char SeparatorReplacement = ' '; + + internal static string CleanSlashes(this String pathComponent) + { + return pathComponent.Replace(Separator, SeparatorReplacement); + } + public static string Combine(InputControl parent, string path) { if (parent == null) @@ -116,7 +129,7 @@ public static string Combine(InputControl parent, string path) } /// - /// Options for customizing the behavior of . + /// Options for customizing the behavior of . /// [Flags] public enum HumanReadableStringOptions @@ -356,7 +369,7 @@ public static string TryGetDeviceLayout(string path) return null; if (parser.current.m_Layout.length > 0) - return parser.current.m_Layout.ToString(); + return parser.current.m_Layout.ToString().Unescape(); if (parser.current.isWildcard) return Wildcard; @@ -410,7 +423,7 @@ public static string TryGetControlLayout(string path) if (parser.current.isWildcard) return Wildcard; - return FindControlLayoutRecursive(ref parser, deviceLayoutName); + return FindControlLayoutRecursive(ref parser, deviceLayoutName.Unescape()); } private static string FindControlLayoutRecursive(ref PathParser parser, string layoutName) @@ -535,6 +548,8 @@ private static bool StringMatches(Substring str, InternedString matchTo) while (posInStr < strLength && posInMatchTo < matchToLength) { var nextChar = str[posInStr]; + if (nextChar == '\\' && posInStr + 1 < strLength) + nextChar = str[++posInStr]; if (nextChar == '*') { ////TODO: make sure we don't end up with ** here @@ -1047,7 +1062,7 @@ private static bool MatchPathComponent(string component, string path, ref int in } else { - if (nextCharInPath == '/') + if (nextCharInPath == '/' && componentType == PathComponentType.Name) break; if ((nextCharInPath == '>' && componentType == PathComponentType.Layout) || (nextCharInPath == '}' && componentType == PathComponentType.Usage) @@ -1329,17 +1344,13 @@ public bool Matches(InputControl control) if (!m_Layout.isEmpty) { // Check for direct match. - var layoutMatches = Substring.Compare(m_Layout, control.layout, - StringComparison.OrdinalIgnoreCase) == 0; + var layoutMatches = ComparePathElementToString(m_Layout, control.layout); if (!layoutMatches) { // No direct match but base layout may match. var baseLayout = control.m_Layout; while (InputControlLayout.s_Layouts.baseLayoutTable.TryGetValue(baseLayout, out baseLayout) && !layoutMatches) - { - layoutMatches = Substring.Compare(m_Layout, baseLayout.ToString(), - StringComparison.OrdinalIgnoreCase) == 0; - } + layoutMatches = ComparePathElementToString(m_Layout, baseLayout.ToString()); } if (!layoutMatches) @@ -1356,7 +1367,7 @@ public bool Matches(InputControl control) var controlUsages = control.usages; var haveUsageMatch = false; for (var ci = 0; ci < controlUsages.Count; ++ci) - if (Substring.Compare(controlUsages[ci].ToString(), m_Usages[i], StringComparison.OrdinalIgnoreCase) == 0) + if (ComparePathElementToString(m_Usages[i], controlUsages[ci])) { haveUsageMatch = true; break; @@ -1372,15 +1383,39 @@ public bool Matches(InputControl control) if (!m_Name.isEmpty && !isWildcard) { ////FIXME: unlike the matching path we have in MatchControlsRecursive, this does not take aliases into account - if (Substring.Compare(control.name, m_Name, StringComparison.OrdinalIgnoreCase) != 0) + if (!ComparePathElementToString(m_Name, control.name)) return false; } // Match display name. if (!m_DisplayName.isEmpty) { - if (Substring.Compare(control.displayName, m_DisplayName, - StringComparison.OrdinalIgnoreCase) != 0) + if (!ComparePathElementToString(m_DisplayName, control.displayName)) + return false; + } + + return true; + } + + // In a path, characters may be escaped so in those cases, we can't just compare + // character-by-character. + private static bool ComparePathElementToString(Substring pathElement, string element) + { + var pathElementLength = pathElement.length; + var elementLength = element.Length; + + // `element` is expected to not include escape sequence. `pathElement` may. + // So if `element` is longer than `pathElement`, the two can't be a match. + if (elementLength > pathElementLength) + return false; + + for (var i = 0; i < pathElementLength && i < elementLength; ++i) + { + var ch = pathElement[i]; + if (ch == '\\' && i + 1 < pathElementLength) + ch = pathElement[++i]; + + if (char.ToLowerInvariant(ch) != char.ToLowerInvariant(element[i])) return false; } @@ -1515,7 +1550,11 @@ private Substring ParseComponentPart(char terminator) var partStartIndex = rightIndexInPath; while (rightIndexInPath < length && path[rightIndexInPath] != terminator) + { + if (path[rightIndexInPath] == '\\' && rightIndexInPath + 1 < length) + ++rightIndexInPath; ++rightIndexInPath; + } var partLength = rightIndexInPath - partStartIndex; if (rightIndexInPath < length && terminator != '/') diff --git a/Packages/com.unity.inputsystem/InputSystem/Devices/InputDeviceBuilder.cs b/Packages/com.unity.inputsystem/InputSystem/Devices/InputDeviceBuilder.cs index 03246a3884..8d8a3ace52 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Devices/InputDeviceBuilder.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Devices/InputDeviceBuilder.cs @@ -173,6 +173,10 @@ private InputControl InstantiateLayout(InternedString layout, InternedString var name = new InternedString(name.ToString().Substring(indexOfLastColon + 1)); } + // Make sure name does not contain any slashes. + if (name.ToString().IndexOf(InputControlPath.Separator) != -1) + name = new InternedString(name.ToString().CleanSlashes()); + // Variant defaults to variants of layout. if (variants.IsEmpty()) {