Skip to content

Commit eb4f139

Browse files
author
Rene Damm
authored
FIX: Special chars in HID names causing rebinding to not work (case 1335465, #1451).
1 parent da6732a commit eb4f139

7 files changed

Lines changed: 142 additions & 43 deletions

File tree

Assets/Tests/InputSystem/Plugins/HIDTests.cs

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
using UnityEngine.InputSystem.Layouts;
1414
using UnityEngine.InputSystem.Processors;
1515
using UnityEngine.InputSystem.Utilities;
16-
using UnityEngine.TestTools;
1716
using UnityEngine.TestTools.Utils;
1817

1918
////TODO: add test to make sure we're not grabbing HIDs that have more specific layouts
@@ -1312,5 +1311,59 @@ public void Utilities_CanRecognizeVendorDefinedUsages()
13121311
Assert.That(HID.UsagePageToString((HID.UsagePage) 0xff01), Is.EqualTo("Vendor-Defined"));
13131312
Assert.That(HID.UsageToString((HID.UsagePage) 0xff01, 0x33), Is.Null);
13141313
}
1314+
1315+
// https://fogbugz.unity3d.com/f/cases/1335465/
1316+
[Test]
1317+
[Category("Devices")]
1318+
[TestCase("/")]
1319+
[TestCase("\\")]
1320+
[TestCase(">")]
1321+
[TestCase("<")]
1322+
[TestCase(" ")]
1323+
public void Devices_CanHaveReservedCharactersInHIDDeviceNames(string characterStr)
1324+
{
1325+
// Il2cpp seems to have trouble with slashes in character literals used in attributes
1326+
// so we pass the character as a string instead.
1327+
var character = characterStr[0];
1328+
1329+
var hidDescriptor = new HID.HIDDeviceDescriptor
1330+
{
1331+
usage = (int)HID.GenericDesktop.Joystick,
1332+
usagePage = HID.UsagePage.GenericDesktop,
1333+
elements = new[]
1334+
{
1335+
new HID.HIDElementDescriptor { usage = (int)HID.GenericDesktop.X, usagePage = HID.UsagePage.GenericDesktop, reportType = HID.HIDReportType.Input, reportId = 1, reportOffsetInBits = 0, reportSizeInBits = 16 },
1336+
new HID.HIDElementDescriptor { usage = (int)HID.GenericDesktop.Y, usagePage = HID.UsagePage.GenericDesktop, reportType = HID.HIDReportType.Input, reportId = 1, reportOffsetInBits = 16, reportSizeInBits = 16 },
1337+
new HID.HIDElementDescriptor { usage = (int)HID.Button.Primary, usagePage = HID.UsagePage.Button, reportType = HID.HIDReportType.Input, reportId = 1, reportOffsetInBits = 32, reportSizeInBits = 1 },
1338+
new HID.HIDElementDescriptor { usage = (int)HID.Button.Secondary, usagePage = HID.UsagePage.Button, reportType = HID.HIDReportType.Input, reportId = 1, reportOffsetInBits = 33, reportSizeInBits = 1 },
1339+
}
1340+
};
1341+
1342+
var device = InputSystem.AddDevice(
1343+
new InputDeviceDescription
1344+
{
1345+
interfaceName = HID.kHIDInterface,
1346+
manufacturer = "TestVendor",
1347+
product = "Test" + character + "HID",
1348+
capabilities = hidDescriptor.ToJson()
1349+
});
1350+
1351+
Assert.That(device.name, Is.EqualTo("TestVendor Test" + (character == '/' ? InputControlPath.SeparatorReplacement : character) + "HID"));
1352+
Assert.That(device.displayName, Is.EqualTo("Test" + character + "HID"));
1353+
1354+
var action = new InputAction(binding: "<Keyboard>/space");
1355+
1356+
// Interactively rebind the action to refer to the device's button.
1357+
using (action.PerformInteractiveRebinding()
1358+
.OnMatchWaitForAnother(0)
1359+
.Start())
1360+
{
1361+
Press((ButtonControl)device["button2"]);
1362+
}
1363+
1364+
Assert.That(InputControlPath.TryGetDeviceLayout(action.bindings[0].effectivePath), Is.EqualTo(device.layout));
1365+
Assert.That(InputControlPath.TryGetControlLayout(action.bindings[0].effectivePath), Is.EqualTo("Button"));
1366+
Assert.That(action.controls, Is.EquivalentTo(new[] { device["button2"]}));
1367+
}
13151368
}
13161369
#endif

Assets/Tests/InputSystem/Plugins/UITests.cs

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3412,7 +3412,7 @@ public IEnumerator UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule
34123412

34133413
var scene = SceneManager.LoadScene("UITKTestScene", new LoadSceneParameters(LoadSceneMode.Additive));
34143414
yield return null;
3415-
Assert.That(scene.isLoaded, Is.True);
3415+
Assert.That(scene.isLoaded, Is.True, "UITKTestScene did not load as expected");
34163416

34173417
try
34183418
{
@@ -3446,17 +3446,17 @@ public IEnumerator UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule
34463446

34473447
yield return null;
34483448

3449-
Assert.That(uiButton.HasMouseCapture(), Is.True);
3449+
Assert.That(uiButton.HasMouseCapture(), Is.True, "Expected uiButton to have mouse capture");
34503450

34513451
Release(mouse.leftButton, queueEventOnly: true);
34523452

34533453
yield return null;
34543454

3455-
Assert.That(uiButton.HasMouseCapture(), Is.False);
3455+
Assert.That(uiButton.HasMouseCapture(), Is.False, "Expected uiButton to no longer have mouse capture");
34563456
Assert.That(clickReceived, Is.True);
34573457

34583458
// Put mouse in upper right corner and scroll down.
3459-
Assert.That(scrollView.verticalScroller.value, Is.Zero);
3459+
Assert.That(scrollView.verticalScroller.value, Is.Zero, "Expected verticalScroller to be all the way up");
34603460
Set(mouse.position, scrollViewCenter, queueEventOnly: true);
34613461
yield return null;
34623462
Set(mouse.scroll, new Vector2(0, -100), queueEventOnly: true);
@@ -3477,7 +3477,7 @@ public IEnumerator UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule
34773477
PressAndRelease(gamepad.buttonSouth, queueEventOnly: true);
34783478
yield return null;
34793479

3480-
Assert.That(clickReceived, Is.True);
3480+
Assert.That(clickReceived, Is.True, "Expected to have received click");
34813481

34823482
////TODO: tracked device support (not yet supported by UITK)
34833483

@@ -3491,40 +3491,40 @@ static bool IsActive(VisualElement ve)
34913491
yield return null;
34923492
InputSystem.RemoveDevice(mouse);
34933493

3494-
int uiButtonDownCount = 0;
3495-
int uiButtonUpCount = 0;
3494+
var uiButtonDownCount = 0;
3495+
var uiButtonUpCount = 0;
34963496
uiButton.RegisterCallback<PointerDownEvent>(e => uiButtonDownCount++, TrickleDown.TrickleDown);
34973497
uiButton.RegisterCallback<PointerUpEvent>(e => uiButtonUpCount++, TrickleDown.TrickleDown);
34983498

34993499
// Case 1369081: Make sure button doesn't get "stuck" in an active state when multiple fingers are used.
35003500
BeginTouch(1, buttonCenter, screen: touchscreen);
35013501
yield return null;
3502-
Assert.That(uiButtonDownCount, Is.EqualTo(1));
3503-
Assert.That(uiButtonUpCount, Is.EqualTo(0));
3504-
Assert.That(IsActive(uiButton), Is.True);
3502+
Assert.That(uiButtonDownCount, Is.EqualTo(1), "Expected uiButtonDownCount to be 0");
3503+
Assert.That(uiButtonUpCount, Is.EqualTo(0), "Expected uiButtonUpCount to be 0");
3504+
Assert.That(IsActive(uiButton), Is.True, "Expected uiButton to be active");
35053505

35063506
BeginTouch(2, buttonOutside, screen: touchscreen);
35073507
yield return null;
35083508
EndTouch(2, buttonOutside, screen: touchscreen);
35093509
yield return null;
3510-
Assert.That(uiButtonDownCount, Is.EqualTo(1));
3510+
Assert.That(uiButtonDownCount, Is.EqualTo(1), "Expected uiButtonDownCount to be 1");
35113511

35123512
if (pointerBehavior == UIPointerBehavior.SingleUnifiedPointer)
35133513
{
3514-
Assert.That(uiButtonUpCount, Is.EqualTo(1));
3515-
Assert.That(IsActive(uiButton), Is.False);
3514+
Assert.That(uiButtonUpCount, Is.EqualTo(1), "Expected uiButtonUpCount to be 1");
3515+
Assert.That(IsActive(uiButton), Is.False, "Expected uiButton to no longer be active");
35163516
}
35173517
else
35183518
{
3519-
Assert.That(uiButtonUpCount, Is.EqualTo(0));
3520-
Assert.That(IsActive(uiButton), Is.True);
3519+
Assert.That(uiButtonUpCount, Is.EqualTo(0), "Expected uiButtonUpCount to be 0");
3520+
Assert.That(IsActive(uiButton), Is.True, "Expected uiButton to be active");
35213521
}
35223522

35233523
EndTouch(1, buttonCenter, screen: touchscreen);
35243524
yield return null;
3525-
Assert.That(uiButtonDownCount, Is.EqualTo(1));
3526-
Assert.That(uiButtonUpCount, Is.EqualTo(1));
3527-
Assert.That(IsActive(uiButton), Is.False);
3525+
Assert.That(uiButtonDownCount, Is.EqualTo(1), "Expected uiButtonDownCount to be 1");
3526+
Assert.That(uiButtonUpCount, Is.EqualTo(1), "Expected uiButtonUpCount to be 1");
3527+
Assert.That(IsActive(uiButton), Is.False, "Expected uiButton to no longer be active");
35283528

35293529
InputSystem.RemoveDevice(touchscreen);
35303530
}

Packages/com.unity.inputsystem/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ however, it has to be formatted properly to pass verification tests.
3232
- Fixed DualSense on iOS not inheriting from `DualShockGamepad` ([case 1378308](https://issuetracker.unity3d.com/issues/input-dualsense-detection-ios)).
3333
- Fixed a device becoming `.current` (e.g. `Gamepad.current`, etc) when sending a new state event that contains no control changes (case 1377952).
3434
- 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)).
35+
- 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/)).
3536
- 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/)).
3637
- 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)).
3738
- 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)).

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ private void EnableControls(InputAction action)
646646

647647
var map = action.m_ActionMap;
648648
var mapIndex = map.m_MapIndexInState;
649-
Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range");
649+
Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range in EnableControls");
650650

651651
// Go through all bindings in the map and for all that belong to the given action,
652652
// enable the associated controls.
@@ -682,7 +682,7 @@ public void DisableAllActions(InputActionMap map)
682682

683683
// Mark all actions as disabled.
684684
var mapIndex = map.m_MapIndexInState;
685-
Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range");
685+
Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range in DisableAllActions");
686686
var actionStartIndex = mapIndices[mapIndex].actionStartIndex;
687687
var actionCount = mapIndices[mapIndex].actionCount;
688688
for (var i = 0; i < actionCount; ++i)
@@ -708,7 +708,7 @@ private void DisableControls(InputActionMap map)
708708
Debug.Assert(maps.Contains(map), "Map must be contained in state");
709709

710710
var mapIndex = map.m_MapIndexInState;
711-
Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range");
711+
Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range in DisableControls(InputActionMap)");
712712

713713
// Remove state monitors from all controls.
714714
var controlCount = mapIndices[mapIndex].controlCount;
@@ -742,7 +742,7 @@ private void DisableControls(InputAction action)
742742

743743
var map = action.m_ActionMap;
744744
var mapIndex = map.m_MapIndexInState;
745-
Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range");
745+
Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range in DisableControls(InputAction)");
746746

747747
// Go through all bindings in the map and for all that belong to the given action,
748748
// disable the associated controls.
@@ -1012,7 +1012,7 @@ private static void SplitUpMapAndControlAndBindingIndex(long mapControlAndBindin
10121012
/// </remarks>
10131013
private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindingIndex, double time, InputEventPtr eventPtr)
10141014
{
1015-
Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range");
1015+
Debug.Assert(mapIndex >= 0 && mapIndex < totalMapCount, "Map index out of range in ProcessControlStateChange");
10161016
Debug.Assert(controlIndex >= 0 && controlIndex < totalControlCount, "Control index out of range");
10171017
Debug.Assert(bindingIndex >= 0 && bindingIndex < totalBindingCount, "Binding index out of range");
10181018

Packages/com.unity.inputsystem/InputSystem/Controls/InputControlExtensions.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -919,22 +919,24 @@ internal static string BuildPath(this InputControl control, string deviceLayout,
919919
var device = control.device;
920920

921921
builder.Append('<');
922-
builder.Append(deviceLayout);
922+
builder.Append(deviceLayout.Escape("\\>", "\\>"));
923923
builder.Append('>');
924924

925925
// Add usages of device, if any.
926926
var deviceUsages = device.usages;
927927
for (var i = 0; i < deviceUsages.Count; ++i)
928928
{
929929
builder.Append('{');
930-
builder.Append(deviceUsages[i]);
930+
builder.Append(deviceUsages[i].ToString().Escape("\\}", "\\}"));
931931
builder.Append('}');
932932
}
933933

934-
builder.Append('/');
934+
builder.Append(InputControlPath.Separator);
935935

936-
var devicePath = device.path;
937-
var controlPath = control.path;
936+
// If any of the components contains a backslash, double it up as in control paths,
937+
// these serve as escape characters.
938+
var devicePath = device.path.Replace("\\", "\\\\");
939+
var controlPath = control.path.Replace("\\", "\\\\");
938940
builder.Append(controlPath, devicePath.Length + 1, controlPath.Length - devicePath.Length - 1);
939941

940942
return builder.ToString();

0 commit comments

Comments
 (0)