Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Custom devices lost on domain reload (case 1192379). #931

Merged
merged 4 commits into from
Oct 31, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
97 changes: 78 additions & 19 deletions Assets/Tests/InputSystem/CoreTests_Editor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,49 +141,108 @@ public void Editor_DomainReload_PreservesUsagesOnDevices()
var device = InputSystem.AddDevice<Gamepad>();
InputSystem.SetDeviceUsage(device, CommonUsages.LeftHand);

InputSystem.SaveAndReset();
InputSystem.Restore();
SimulateDomainReload();

var newDevice = InputSystem.devices.First(x => x is Gamepad);
var newDevice = InputSystem.devices[0];

Assert.That(newDevice.usages, Has.Count.EqualTo(1));
Assert.That(newDevice.usages, Has.Exactly(1).EqualTo(CommonUsages.LeftHand));
}

// We have code that will automatically query the enabled state of devices on creation
// but if the IOCTL is not implemented, we still need to be able to maintain a device's
// enabled state.
[Test]
[Category("Editor")]
public void Editor_DomainReload_FirstPlayerLoopUpdateCausesDevicesToBeRecreated()
public void Editor_DomainReload_PreservesEnabledState()
{
InputSystem.AddDevice<Gamepad>();
var device = InputSystem.AddDevice<Gamepad>();
InputSystem.DisableDevice(device);

Assert.That(device.enabled, Is.False);

// This test quite invasively goes into InputSystem internals. Unfortunately, we
// have no proper way of simulating domain reloads ATM. So we directly call various
// internal methods here in a sequence similar to what we'd get during a domain reload.
SimulateDomainReload();

InputSystem.s_SystemObject.OnBeforeSerialize();
runtime.onPlayModeChanged(PlayModeStateChange.ExitingEditMode);
runtime.isInPlayMode = false;
InputSystem.s_SystemObject = null;
InputSystem.InitializeInEditor(runtime);
runtime.isInPlayMode = true;
runtime.onPlayModeChanged(PlayModeStateChange.EnteredPlayMode);
var newDevice = InputSystem.devices[0];

Assert.That(newDevice.enabled, Is.False);
}

[Test]
[Category("Editor")]
public void Editor_DomainReload_InputSystemInitializationCausesDevicesToBeRecreated()
{
InputSystem.AddDevice<Gamepad>();

SimulateDomainReload();

Assert.That(InputSystem.devices, Has.Count.EqualTo(1));
Assert.That(InputSystem.devices[0], Is.TypeOf<Gamepad>());
}

// https://fogbugz.unity3d.com/f/cases/1192379/
[Test]
[Category("Editor")]
[Ignore("TODO")]
public void TODO_Editor_DomainReload_PreservesVariantsOnDevices()
public void Editor_DomainReload_CustomDevicesAreRestoredAsLayoutsBecomeAvailable()
{
////REVIEW: Consider switching away from explicit registration and switch to implicit discovery
//// through reflection. Explicit registration has proven surprisingly fickle and puts the
//// burden squarely on users.

// We may have several [InitializeOnLoad] classes each registering a piece of data
// with the input system. The first [InitializeOnLoad] code that gets picked by the
// Unity runtime is the one that will trigger initialization of the input system.
//
// However, if we have a later one in the sequence registering a device layout, we
// cannot successfully recreate devices using that layout until that code has executed,
// too.
//
// What we do to solve this is to keep information on devices that we fail to restore
// after a domain around until the very first full input update. At that point, we
// warn about every

const string kLayout = @"
{
""name"" : ""CustomDevice"",
""extend"" : ""Gamepad""
}
";

InputSystem.RegisterLayout(kLayout);
InputSystem.AddDevice("CustomDevice");

SimulateDomainReload();

Assert.That(InputSystem.devices, Is.Empty);

InputSystem.RegisterLayout(kLayout);

Assert.That(InputSystem.devices, Has.Count.EqualTo(1));
Assert.That(InputSystem.devices[0].layout, Is.EqualTo("CustomDevice"));
}

[Test]
[Category("Editor")]
public void Editor_DomainReload_RetainsUnsupportedDevices()
{
Assert.Fail();
runtime.ReportNewInputDevice(new InputDeviceDescription
{
interfaceName = "SomethingUnknown",
product = "UnknownProduct"
});
InputSystem.Update();

SimulateDomainReload();

Assert.That(InputSystem.GetUnsupportedDevices(), Has.Count.EqualTo(1));
Assert.That(InputSystem.GetUnsupportedDevices()[0].interfaceName, Is.EqualTo("SomethingUnknown"));
Assert.That(InputSystem.GetUnsupportedDevices()[0].product, Is.EqualTo("UnknownProduct"));
}

[Test]
[Category("Editor")]
[Ignore("TODO")]
public void TODO_Editor_DomainReload_PreservesCurrentStatusOfDevices()
public void TODO_Editor_DomainReload_PreservesVariantsOnDevices()
{
Assert.Fail();
}
Expand Down
29 changes: 29 additions & 0 deletions Assets/Tests/InputSystem/CoreTests_Layouts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,35 @@ public void Layouts_CanAlterDeviceDescriptionsForDiscoveredDevices()
Assert.That(device.description.product, Is.EqualTo("Test"));
}

// By the time someone adds a callback to onFindLayoutForDevice, we may already
// have devices that have been reported that we didn't recognize but which would
// now be recognizable. The system should use any new hook added to the callback
// to find out if that is the case.
[Test]
[Category("Layouts")]
public void Layouts_CanRetroactivelyFindLayoutForAlreadyReportedDevice()
{
var deviceDescription = new InputDeviceDescription
{
interfaceName = "TestInterface",
product = "TestProduct"
};

var deviceId = runtime.ReportNewInputDevice(deviceDescription);

InputSystem.Update();

Assert.That(InputSystem.GetUnsupportedDevices(), Is.EquivalentTo(new[] {deviceDescription}));
Assert.That(InputSystem.devices, Is.Empty);

InputSystem.onFindLayoutForDevice +=
(ref InputDeviceDescription description, string layoutMatch, InputDeviceExecuteCommandDelegate executeCommandDelegate) =>
"Keyboard";

Assert.That(InputSystem.GetUnsupportedDevices(), Is.Empty);
Assert.That(InputSystem.devices, Has.Exactly(1).TypeOf<Keyboard>().With.Property("deviceId").EqualTo(deviceId));
}

[Test]
[Category("Layouts")]
public void Layouts_CanRegisterMultipleMatchersForSingleLayout()
Expand Down
4 changes: 3 additions & 1 deletion Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ however, it has to be formatted properly to pass verification tests.
### Fixed

- Fixed touch taps triggering when they shouldn't on Android.
- OpenVR Touchpad actions (touchpadClicked & touchpadPressed) now report accurate data.
- Fixed custom devices registered from `[InitializeOnLoad]` code being lost on domain reload (case 1192379).
* This happened when there were multiple pieces of `[InitializeOnLoad]` code that accessed the input system in the project and the `RegisterLayout` for the custom device happened to not be the first in sequence.
- OpenVR touchpad controls (`touchpadClicked` & `touchpadPressed`) now report accurate data.

#### Actions

Expand Down
23 changes: 21 additions & 2 deletions Packages/com.unity.inputsystem/Documentation~/Devices.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# Devices

* [Device descriptions](#device-descriptions)
* [Capabilities](#capabilities)
* [Matching](#matching)
* [Hijacking the matching process](#hijacking-the-matching-process)
* [Device creation](#device-creation)
* [Native Devices](#native-devices)
* [Disconnected Devices](#disconnected-devices)
* [Device IDs](#device-ids)
Expand Down Expand Up @@ -32,7 +35,7 @@ Every description has a set of standard fields:
|[`manufacturer`](../api/UnityEngine.InputSystem.Layouts.InputDeviceDescription.html#UnityEngine_InputSystem_Layouts_InputDeviceDescription_manufacturer)|Name of the manufacturer as reported by the Device/driver itself.|
|[`version`](../api/UnityEngine.InputSystem.Layouts.InputDeviceDescription.html#UnityEngine_InputSystem_Layouts_InputDeviceDescription_version)|If available, provides the version of the driver or hardware for the Device.|
|[`serial`](../api/UnityEngine.InputSystem.Layouts.InputDeviceDescription.html#UnityEngine_InputSystem_Layouts_InputDeviceDescription_serial)|If available, provides the serial number for the Device.|
|[`capabilities`](../api/UnityEngine.InputSystem.Layouts.InputDeviceDescription.html#UnityEngine_InputSystem_Layouts_InputDeviceDescription_capabilities)|A string in JSON format describing Device/interface-specific capabilities. See the [section on capabililities](#capabilities).|
|[`capabilities`](../api/UnityEngine.InputSystem.Layouts.InputDeviceDescription.html#UnityEngine_InputSystem_Layouts_InputDeviceDescription_capabilities)|A string in JSON format describing Device/interface-specific capabilities. See the [section on capabilities](#capabilities).|

### Capabilities

Expand Down Expand Up @@ -61,10 +64,26 @@ InputSystem.RegisterLayoutMatcher<MyDevice>(

If multiple matchers are matching the same [`InputDeviceDescription`](../api/UnityEngine.InputSystem.Layouts.InputDeviceDescription.html), the Input System chooses the matcher that has the larger number of properties to match against.

### Hijacking the matching process
#### Hijacking the matching process

You can overrule the internal matching process from outside and thus select a different layout for a Device than the system would normally choose. This also makes it possible to build new layouts on the fly. To do this, add a custom handler to the [`InputSystem.onFindControlLayoutForDevice`](../api/UnityEngine.InputSystem.InputSystem.html#UnityEngine_InputSystem_InputSystem_onFindLayoutForDevice) event. If your handler returns a non-null layout string, then the Input System will use this layout.

### Device creation

Once a [layout](Layouts.md) has been chosen for a device, it is used to instantiate an [`InputDevice`](../api/UnityEngine.InputSystem.InputDevice.html) and populate it with [`InputControls`](../api/UnityEngine.InputSystem.InputControl.html) as dictated by the layout. This process is performed automatically and internally.

>__Note__: Valid [`InputDevices`](../api/UnityEngine.InputSystem.InputDevice.html) and [`InputControls`](../api/UnityEngine.InputSystem.InputControl.html) cannot be created by manually instantiating them with `new`. To guide the creation process, [layouts](Layouts.md) must be used.

When the Input System has finished putting an [`InputDevice`](../api/UnityEngine.InputSystem.InputDevice.html) together, it will call [`FinishSetup`](../api/UnityEngine.InputSystem.InputControl.html#UnityEngine_InputSystem_InputControl_FinishSetup_) on each control of the device and on the device itself. This can be used to finalize the setup of controls.

After an [`InputDevice`](../api/UnityEngine.InputSystem.InputDevice.html) is fully assembled, it will be added to the system. As part of this process, [`MakeCurrent`](../api/UnityEngine.InputSystem.InputDevice.html#UnityEngine_InputSystem_InputDevice_MakeCurrent_) will be called on the device and [`InputDeviceChange.Added`](../api/UnityEngine.InputSystem.InputDeviceChange.html#UnityEngine_InputSystem_InputDeviceChange_Added) will be signaled on [`InputSystem.onDeviceChange`](../api/UnityEngine.InputSystem.InputSystem.html#UnityEngine_InputSystem_InputSystem_onDeviceChange).

#### Domain reloads in the Editor

In the Editor, the C# application domain will be reloaded whenever scripts are recompiled and reloaded or when going into play mode. This requires the Input System to reinitialize itself after each domain reload. During this process, it will attempt to recreate devices that had been instantiated before the domain reload. It will, however, not take the state of each such Device across. This means that Devices will reset to default state on domain reloads.

Note that layout registrations are __not__ persisted across domain reloads. Instead, the Input System relies on all registrations to become available as part of the initialization process (e.g. by using `[InitializeOnLoad]` to run registration as part of the domain startup code in the Editor). This allows registrations and layouts to be changed in script and the change to immediately take effect after a domain reload.

## Native Devices

Devices that the [native backend](Architecture.md#native-backend) reports are considered native (as opposed to Devices created from script code). You can identify these Devices by checking the [`InputDevice.native`](../api/UnityEngine.InputSystem.InputDevice.html#UnityEngine_InputSystem_InputDevice_native) property.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

////REVIEW: how do we do stuff like smoothing over time?

////TODO: allow easier access to the default state such that you can easily create a state event containing only default state

namespace UnityEngine.InputSystem
{
/// <summary>
Expand Down Expand Up @@ -323,10 +325,6 @@ public string path
///
/// The primary effect of being noise is on <see cref="InputDevice.MakeCurrent"/> and
/// on interactive rebinding (see <see cref="InputActionRebindingExtensions.RebindingOperation"/>).
///
/// If noise filtering on <c>.current</c> is enabled (see <see cref="InputSettings.filterNoiseOnCurrent"/>),
/// when seeing input for a potentially noisy device (i.e. any device with any control
/// marked as noisy), the system will perform a check
/// </remarks>
/// <seealso cref="InputControlLayout.ControlItem.isNoisy"/>
/// <seealso cref="InputControlAttribute.noisy"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,13 +225,14 @@ public override string ToString()
/// </remarks>
public bool Equals(InputDeviceDescription other)
{
return string.Equals(m_InterfaceName, other.m_InterfaceName, StringComparison.InvariantCultureIgnoreCase) &&
string.Equals(m_DeviceClass, other.m_DeviceClass, StringComparison.InvariantCultureIgnoreCase) &&
string.Equals(m_Manufacturer, other.m_Manufacturer, StringComparison.InvariantCultureIgnoreCase) &&
string.Equals(m_Product, other.m_Product, StringComparison.InvariantCultureIgnoreCase) &&
string.Equals(m_Serial, other.m_Serial, StringComparison.InvariantCultureIgnoreCase) &&
string.Equals(m_Version, other.m_Version, StringComparison.InvariantCultureIgnoreCase) &&
string.Equals(m_Capabilities, other.m_Capabilities, StringComparison.InvariantCultureIgnoreCase);
return m_InterfaceName.InvariantEqualsIgnoreCase(other.m_InterfaceName) &&
m_DeviceClass.InvariantEqualsIgnoreCase(other.m_DeviceClass) &&
m_Manufacturer.InvariantEqualsIgnoreCase(other.m_Manufacturer) &&
m_Product.InvariantEqualsIgnoreCase(other.m_Product) &&
m_Serial.InvariantEqualsIgnoreCase(other.m_Serial) &&
m_Version.InvariantEqualsIgnoreCase(other.m_Version) &&
////REVIEW: this would ideally compare JSON contents not just the raw string
m_Capabilities.InvariantEqualsIgnoreCase(other.m_Capabilities);
}

/// <summary>
Expand All @@ -245,7 +246,7 @@ public override bool Equals(object obj)
{
if (ReferenceEquals(null, obj))
return false;
return obj is InputDeviceDescription && Equals((InputDeviceDescription)obj);
return obj is InputDeviceDescription description && Equals(description);
}

/// <summary>
Expand All @@ -256,7 +257,7 @@ public override int GetHashCode()
{
unchecked
{
var hashCode = (m_InterfaceName != null ? m_InterfaceName.GetHashCode() : 0);
var hashCode = m_InterfaceName != null ? m_InterfaceName.GetHashCode() : 0;
hashCode = (hashCode * 397) ^ (m_DeviceClass != null ? m_DeviceClass.GetHashCode() : 0);
hashCode = (hashCode * 397) ^ (m_Manufacturer != null ? m_Manufacturer.GetHashCode() : 0);
hashCode = (hashCode * 397) ^ (m_Product != null ? m_Product.GetHashCode() : 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,9 +437,6 @@ public override string ToString()
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", MessageId = "0", Justification = "False positive.")]
public bool Equals(InputDeviceMatcher other)
{
if (other == null)
return false;

if (m_Patterns == other.m_Patterns)
return true;

Expand Down