Skip to content

Commit

Permalink
FIX: Special chars in HID names causing rebinding to not work (case 1…
Browse files Browse the repository at this point in the history
…335465, #1451).
  • Loading branch information
Rene Damm committed Nov 29, 2021
1 parent da6732a commit eb4f139
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 43 deletions.
55 changes: 54 additions & 1 deletion Assets/Tests/InputSystem/Plugins/HIDTests.cs
Expand Up @@ -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
Expand Down Expand Up @@ -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: "<Keyboard>/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
36 changes: 18 additions & 18 deletions Assets/Tests/InputSystem/Plugins/UITests.cs
Expand Up @@ -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
{
Expand Down Expand Up @@ -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);
Expand All @@ -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)

Expand All @@ -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<PointerDownEvent>(e => uiButtonDownCount++, TrickleDown.TrickleDown);
uiButton.RegisterCallback<PointerUpEvent>(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);
}
Expand Down
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Expand Up @@ -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)).
Expand Down
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -1012,7 +1012,7 @@ private static long ToCombinedMapAndControlAndBindingIndex(int mapIndex, int con
/// </remarks>
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");

Expand Down
Expand Up @@ -919,22 +919,24 @@ 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.
var deviceUsages = device.usages;
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();
Expand Down

0 comments on commit eb4f139

Please sign in to comment.