FIX: Device pairing ignoring controls that don't support magnitudes (#764, #781).

Author: Rene Damm

Assets/Tests/InputSystem/CoreTests_Devices.cs

@@ -921,20 +921,25 @@ public void Devices_ChangingStateOfDevice_TriggersNotification()
         var gamepad = InputSystem.AddDevice<Gamepad>();
 
         var receivedCalls = 0;
-        InputDevice receivedDevice = null;
+        InputDevice receivedDevice = default;
+        InputEventPtr receivedEventPtr = default;
 
         InputState.onChange +=
-            d =>
+            (d, e) =>
         {
             ++receivedCalls;
             receivedDevice = d;
+            receivedEventPtr = e;
         };
 
         InputSystem.QueueStateEvent(gamepad, new GamepadState { leftStick = new Vector2(0.5f, 0.5f) });
         InputSystem.Update();
 
         Assert.That(receivedCalls, Is.EqualTo(1));
         Assert.That(receivedDevice, Is.SameAs(gamepad));
+        Assert.That(receivedEventPtr.valid, Is.True);
+        Assert.That(receivedEventPtr.deviceId, Is.EqualTo(gamepad.id));
+        Assert.That(receivedEventPtr.IsA<StateEvent>(), Is.True);
     }
 
     private class TestDeviceThatResetsStateInCallback : InputDevice, IInputStateCallbackReceiver
@@ -967,19 +972,22 @@ public void Devices_ChangingStateOfDevice_InStateCallback_TriggersNotification()
         var device = InputSystem.AddDevice<TestDeviceThatResetsStateInCallback>();
 
         var receivedCalls = 0;
-        InputDevice receivedDevice = null;
+        InputDevice receivedDevice = default;
+        InputEventPtr receivedEventPtr = default;
 
         InputState.onChange +=
-            d =>
+            (d, e) =>
         {
             ++receivedCalls;
             receivedDevice = d;
+            receivedEventPtr = e;
         };
 
         InputSystem.Update();
 
         Assert.That(receivedCalls, Is.EqualTo(1));
         Assert.That(receivedDevice, Is.SameAs(device));
+        Assert.That(receivedEventPtr.valid, Is.False);
     }
 
     [Test]

Assets/Tests/InputSystem/Plugins/UserTests.cs

@@ -760,6 +760,23 @@ public void Users_CanFindUserPairedToSpecificDevice()
         Assert.That(InputUser.FindUserPairedToDevice(gamepad3), Is.Null);
     }
 
+    [Test]
+    [Category("Users")]
+    public void Users_CanDetectUseOfUnpairedDevice_FromControlThatDoesNotSupportMagnitude()
+    {
+        ++InputUser.listenForUnpairedDeviceActivity;
+
+        var receivedControls = new List<InputControl>();
+        InputUser.onUnpairedDeviceUsed +=
+            control => { receivedControls.Add(control); };
+
+        var mouse = InputSystem.AddDevice<Mouse>();
+
+        Set(mouse.delta, new Vector2(0, 0.234f));
+
+        Assert.That(receivedControls, Is.EquivalentTo(new[] { mouse.delta.y }));
+    }
+
     [Test]
     [Category("Users")]
     public void Users_CanDetectUseOfUnpairedDevice()

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -11,6 +11,8 @@ however, it has to be formatted properly to pass verification tests.
 
 ### Fixed
 
+- Fixed `InputUser.onUnpairedDeviceUser` ignoring input on controls that do not support `EvaluateMagnitude`.
+  * This led to situations, for example, where `PlayerInput` would not initialize a control scheme switch from a `<Mouse>/delta` binding as the delta X and Y axes do not have min&max limits and thus return -1 from `EvaluateMagnitude`.
 - Fixed available processor list not updated right away when changing the action type in the Input Action editor window.
 
 #### Actions
@@ -22,6 +24,7 @@ however, it has to be formatted properly to pass verification tests.
 
 - Removed `timesliceEvents` setting - and made this tied to the update mode instead. We now always time slice when using fixed updates, and not when using dynamic updates.
 - When adding a composite, only ones compatible with the value type of the current action are shown. This will, for example, no longer display a `2D Vector` composite as an option on a floating-point button action.
+- The `InputState.onChange` callback now receives a second argument which is the event (if any) that triggered the state change on the device.
 
 ### Added
 

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

@@ -505,7 +505,9 @@ public static unsafe bool HasValueChangeInEvent(this InputControl control, Input
             // We need to unsubtract those offsets here.
             stateOffset += device.m_StateBlock.byteOffset;
 
-            if (control.m_StateBlock.byteOffset - stateOffset + control.m_StateBlock.alignedSizeInBytes > stateSizeInBytes)
+            // Return null if state is out of range.
+            var controlOffset = (int)control.m_StateBlock.byteOffset - stateOffset;
+            if (controlOffset < 0 || controlOffset + control.m_StateBlock.alignedSizeInBytes > stateSizeInBytes)
                 return null;
 
             return (byte*)statePtr - (int)stateOffset;

Packages/com.unity.inputsystem/InputSystem/Editor/Debugger/InputDeviceDebuggerWindow.cs

@@ -305,7 +305,7 @@ private void OnDeviceChange(InputDevice device, InputDeviceChange change)
             }
         }
 
-        private void OnDeviceStateChange(InputDevice device)
+        private void OnDeviceStateChange(InputDevice device, InputEventPtr eventPtr)
         {
             ////REVIEW: Ideally we would defer the refresh until we repaint. That way, we would not refresh on every single
             ////        state change but rather only once for a repaint. However, for some reason, if we move the refresh

Packages/com.unity.inputsystem/InputSystem/Events/InputEventPtr.cs

@@ -121,6 +121,9 @@ internal double internalTime
 
         public InputEvent* data => m_EventPtr;
 
+        // The stateFormat, stateSizeInBytes, and stateOffset properties are very
+        // useful for debugging.
+
         internal FourCC stateFormat
         {
             get
@@ -145,6 +148,16 @@ internal uint stateSizeInBytes
             }
         }
 
+        internal uint stateOffset
+        {
+            get
+            {
+                if (IsA<DeltaStateEvent>())
+                    return DeltaStateEvent.From(this)->stateOffset;
+                throw new InvalidOperationException("Event must be a DeltaStateEvent but is " + this);
+            }
+        }
+
         public bool IsA<TOtherEvent>()
             where TOtherEvent : struct, IInputEventTypeInfo
         {

Packages/com.unity.inputsystem/InputSystem/InputManager.cs

@@ -39,7 +39,7 @@
 namespace UnityEngine.InputSystem
 {
     using DeviceChangeListener = Action<InputDevice, InputDeviceChange>;
-    using DeviceStateChangeListener = Action<InputDevice>;
+    using DeviceStateChangeListener = Action<InputDevice, InputEventPtr>;
     using LayoutChangeListener = Action<string, InputControlLayoutChange>;
     using EventListener = Action<InputEventPtr, InputDevice>;
     using UpdateListener = Action;
@@ -2844,7 +2844,7 @@ internal unsafe bool UpdateState(InputDevice device, InputUpdateType updateType,
 
             // Notify listeners.
             for (var i = 0; i < m_DeviceStateChangeListeners.length; ++i)
-                m_DeviceStateChangeListeners[i](device);
+                m_DeviceStateChangeListeners[i](device, eventPtr);
 
             // Now that we've committed the new state to memory, if any of the change
             // monitors fired, let the associated actions know.

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs

@@ -1225,7 +1225,7 @@ private static void OnUnpairedDeviceUsed(InputControl control)
 
             // Go through all control schemes and see if there is one usable with the device.
             // If so, switch to it.
-            var controlScheme = InputControlScheme.FindControlSchemeForDevice<ReadOnlyArray<InputControlScheme>>(control.device, player.m_Actions.controlSchemes);
+            var controlScheme = InputControlScheme.FindControlSchemeForDevice(control.device, player.m_Actions.controlSchemes);
             if (controlScheme != null)
             {
                 // First remove the currently paired devices, then pair the device that was used,

Packages/com.unity.inputsystem/InputSystem/Plugins/Users/InputUser.cs

@@ -338,14 +338,16 @@ public static event Action<InputUser, InputUserChange, InputDevice> onChange
         /// To enable detection of the use of unpaired devices, set <see cref="listenForUnpairedDeviceActivity"/> to true.
         /// It is disabled by default.
         ///
-        /// The control passed to the callback is the first control that activity was detected on. The device can be accessed
-        /// through <see cref="InputControl.device"/>. If multiple controls on a device have been actuated, this will simply
-        /// be the first control in <see cref="InputDevice.allControls"/> that is actuated. The system does not spend time
-        /// trying to find the control that has been actuated the strongest. If desired, this can be done manually in the callback.
+        /// The callback is invoked for each non-leaf, non-synthetic, non-noisy control that has been actuated on the device.
+        /// It being restricted to non-leaf controls means that if, say, the stick on a gamepad is actuated in both X and Y
+        /// direction, you will see two calls: one with stick/x and one with stick/y.
+        ///
+        /// The reason that the callback is invoked for each individual control is that pairing often relies on checking
+        /// for specific kinds of interactions. For example, a pairing callback may listen exclusively for button presses.
         ///
         /// Note that whether the use of unpaired devices leads to them getting paired is under the control of the application.
-        /// If the device should be paired, invoke <see cref="PerformPairingWithDevice"/> from the callback. This can also be used
-        /// to further filter the activity, e.g. to only join users when pressing a button.
+        /// If the device should be paired, invoke <see cref="PerformPairingWithDevice"/> from the callback. If you do so,
+        /// no further callbacks will get triggered for other controls that may have been actuated in the same event.
         ///
         /// <example>
         /// <code>
@@ -1613,8 +1615,13 @@ private static void OnDeviceChange(InputDevice device, InputDeviceChange change)
             }
         }
 
-        internal static void OnDeviceStateChange(InputDevice device)
+        private static unsafe void OnDeviceStateChange(InputDevice device, InputEventPtr eventPtr)
         {
+            // Ignore any state change that was triggered internally. This immediately filters out
+            // things such as Pointers resetting deltas, for example.
+            if (!eventPtr.valid)
+                return;
+
             Debug.Assert(s_ListenForUnpairedDeviceActivity != 0,
                 "This should only be called while listening for unpaired device activity");
             if (s_ListenForUnpairedDeviceActivity == 0)
@@ -1629,6 +1636,10 @@ internal static void OnDeviceStateChange(InputDevice device)
 
             Profiler.BeginSample("InputCheckForUnpairedDeviceActivity");
 
+            // Ignore any state change not triggered from a state event.
+            if (!eventPtr.IsA<StateEvent>() && !eventPtr.IsA<DeltaStateEvent>())
+                return;
+
             ////TODO: allow filtering (e.g. by device requirements on user actions)
 
             // Yes, it is so let's find out whether there was actual user activity
@@ -1647,20 +1658,28 @@ internal static void OnDeviceStateChange(InputDevice device)
                 if (control.noisy || control.synthetic)
                     continue;
 
-                ////REVIEW: is this safe?
                 // Ignore non-leaf controls.
                 if (control.children.Count > 0)
                     continue;
 
+                // Ignore controls that aren't part of the event.
+                var statePtr = control.GetStatePtrFromStateEvent(eventPtr);
+                if (statePtr == null)
+                    continue;
+
                 // Check for default state. Cheaper check than magnitude evaluation
                 // which may involve several virtual method calls.
                 if (control.CheckStateIsAtDefault())
                     continue;
 
                 // Ending up here is costly. We now do per-control work that may involve
                 // walking all over the place in the InputControl machinery.
+                //
+                // NOTE: We already know the control has moved away from its default state
+                //       so in case it does not support magnitudes, we assume that the
+                //       control has changed value, too.
                 var magnitude = control.EvaluateMagnitude();
-                if (magnitude > 0)
+                if (magnitude > 0 || magnitude == -1)
                 {
                     // Yes, something was actuated on the device.
                     var deviceHasBeenPaired = false;
@@ -1839,7 +1858,7 @@ private struct OngoingAccountSelection
         private static InlinedArray<Action<InputUser, InputUserChange, InputDevice>> s_OnChange;
         private static InlinedArray<Action<InputControl>> s_OnUnpairedDeviceUsed;
         private static Action<InputDevice, InputDeviceChange> s_OnDeviceChangeDelegate;
-        private static Action<InputDevice> s_OnDeviceStateChangeDelegate;
+        private static Action<InputDevice, InputEventPtr> s_OnDeviceStateChangeDelegate;
         private static bool s_OnDeviceChangeHooked;
         private static bool s_OnDeviceStateChangeHooked;
         private static int s_ListenForUnpairedDeviceActivity;

Packages/com.unity.inputsystem/InputSystem/State/InputState.cs

@@ -35,7 +35,12 @@ public static class InputState
         /// <summary>
         /// Callback that is triggered when the state of an input device changes.
         /// </summary>
-        public static event Action<InputDevice> onChange
+        /// <remarks>
+        /// The first parameter is the device whose state was changed the second parameter is the event
+        /// that triggered the change in state. Note that the latter may be <c>null</c> in case the
+        /// change was performed directly through <see cref="Change"/> rather than through an event.
+        /// </remarks>
+        public static event Action<InputDevice, InputEventPtr> onChange
         {
             add => InputSystem.s_Manager.onDeviceStateChange += value;
             remove => InputSystem.s_Manager.onDeviceStateChange -= value;