FIX: handled events still triggering actions when switching PlayerInput

Assets/Tests/InputSystem/CoreTests_Events.cs

@@ -10,6 +10,7 @@
 using UnityEngine;
 using UnityEngine.Scripting;
 using UnityEngine.InputSystem;
+using UnityEngine.InputSystem.Users;
 using UnityEngine.InputSystem.Controls;
 using UnityEngine.InputSystem.Layouts;
 using UnityEngine.InputSystem.LowLevel;
@@ -1256,6 +1257,48 @@ public void Events_CanPreventEventsFromBeingProcessed()
         Assert.That(device.rightTrigger.ReadValue(), Is.EqualTo(0.0).Within(0.00001));
     }
 
+    [Test]
+    [Category("Events")]
+    public void Events_HandledFirstEvent_DoesNotTriggerAction_OnSecondEventSameUpdate()
+    {
+        var gamepad = InputSystem.AddDevice<Gamepad>();
+        var action = new InputAction(type: InputActionType.Button, binding: "<Gamepad>/buttonSouth");
+        action.Enable();
+
+        var performedCount = 0;
+        action.performed += _ => ++performedCount;
+
+        var previousPolicy = InputSystem.s_Manager.inputEventHandledPolicy;
+        InputSystem.s_Manager.inputEventHandledPolicy = InputEventHandledPolicy.SuppressStateUpdates;
+
+        InputUser.listenForUnpairedDeviceActivity++;
+        var handledFirstEventId = 0;
+        Action<InputControl, InputEventPtr> onUnpaired = (control, eventPtr) =>
+        {
+            if (handledFirstEventId == 0)
+            {
+                handledFirstEventId = eventPtr.id;
+                eventPtr.handled = true;
+            }
+        };
+        InputUser.onUnpairedDeviceUsed += onUnpaired;
+
+        try
+        {
+            InputSystem.QueueStateEvent(gamepad, new GamepadState().WithButton(GamepadButton.South));
+            InputSystem.QueueStateEvent(gamepad, new GamepadState().WithButton(GamepadButton.South));
+            InputSystem.Update();
+
+            Assert.That(performedCount, Is.EqualTo(0));
+        }
+        finally
+        {
+            InputUser.onUnpairedDeviceUsed -= onUnpaired;
+            InputUser.listenForUnpairedDeviceActivity--;
+            InputSystem.s_Manager.inputEventHandledPolicy = previousPolicy;
+        }
+    }
+
     [Test]
     [Category("Events")]
     public void EventHandledPolicy_ShouldReflectUserSetting()
@@ -1292,24 +1335,24 @@ class SuppressedActionEventData
     // Step 3: Release button north and stick while no longer being suppressed.
     // Step 4: Press gamepad north and stick.
 
-    // Press event is detected in step 2 (false positive) with default interaction
+    // Press event is suppressed in step 1/2 with default interaction
     [TestCase(InputEventHandledPolicy.SuppressStateUpdates, // policy
         null, // interactions
-        new int[] { 0, 0, 1, 1, 2}, // started
-        new int[] { 0, 0, 1, 1, 2}, // performed
-        new int[] {0, 0, 0, 1, 1})] // cancelled
+        new int[] { 0, 0, 0, 0, 1}, // started
+        new int[] { 0, 0, 0, 0, 1}, // performed
+        new int[] {0, 0, 0, 0, 0})] // cancelled
     // Press event is not detected in step 1/2 with default interaction
     [TestCase(InputEventHandledPolicy.SuppressActionEventNotifications,
         null,
         new int[] { 0, 0, 0, 0, 1},
         new int[] { 0, 0, 0, 0, 1},
         new int[] {0, 0, 0, 1, 1})]
-    // Press event is detected in step 2 (false positive) with explicit press interaction
+    // Press event is suppressed in step 1/2 with explicit press interaction
     [TestCase(InputEventHandledPolicy.SuppressStateUpdates,
         "press",
-        new int[] { 0, 0, 1, 1, 2},
-        new int[] { 0, 0, 1, 1, 2},
-        new int[] {0, 0, 0, 1, 1})]
+        new int[] { 0, 0, 0, 0, 1},
+        new int[] { 0, 0, 0, 0, 1},
+        new int[] {0, 0, 0, 0, 0})]
     // Press event is not detected in step 1/2 (false positive) with explicit press interaction
     [TestCase(InputEventHandledPolicy.SuppressActionEventNotifications,
         "press",
@@ -1396,7 +1439,7 @@ public void Events_ShouldRespectHandledPolicyUponUpdateAndSuppressedPressTransit
         Assert.That(action.WasPressedThisFrame, Is.EqualTo(performedThisFrame));
         releasedThisFrame = expectedCancelled[2] - expectedCancelled[1] > 0;
         Assert.That(action.WasReleasedThisFrame, Is.EqualTo(releasedThisFrame));
-        Assert.That(action.IsPressed, Is.True); // Note: This is not an event and hence not suppressed
+        Assert.That(action.IsPressed, Is.EqualTo(seesControlChangesUnderSuppression)); // Note: This is not an event and hence not suppressed
 
         Assert.That(Gamepad.current.buttonNorth.wasPressedThisFrame, Is.EqualTo(!seesControlChangesUnderSuppression));
         Assert.That(Gamepad.current.buttonNorth.wasReleasedThisFrame, Is.False);

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -21,6 +21,7 @@ however, it has to be formatted properly to pass verification tests.
 - Align title font size with toolbar style in `Input Action` window.
 - Updated Action Properties headers to use colors consistent with GameObject component headers.
 - Fixed misaligned Virtual Cursor when changing resolution [ISXB-1119](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1119)
+- Fixed handled input events from unpaired devices still triggering actions on the same physical press [ISXB-1097](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1097)
 
 ### Added
 

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

@@ -1436,6 +1436,9 @@ private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindi
             Debug.Assert(controlIndex >= 0 && controlIndex < totalControlCount, "Control index out of range");
             Debug.Assert(bindingIndex >= 0 && bindingIndex < totalBindingCount, "Binding index out of range");
 
+            if (InputSystem.s_Manager.ShouldSuppressActionsForDevice(controls[controlIndex].device))
+                return;
+
             using (InputActionRebindingExtensions.DeferBindingResolution())
             {
                 // Callbacks can do pretty much anything and thus trigger arbitrary state/configuration

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

@@ -2236,6 +2236,7 @@ internal struct AvailableDevice
         internal CallbackArray<InputDeviceCommandDelegate> m_DeviceCommandCallbacks;
         private CallbackArray<LayoutChangeListener> m_LayoutChangeListeners;
         private CallbackArray<EventListener> m_EventListeners;
+        private uint[] m_SuppressActionsForDeviceUpdate;
         private CallbackArray<UpdateListener> m_BeforeUpdateListeners;
         private CallbackArray<UpdateListener> m_AfterUpdateListeners;
         private CallbackArray<Action> m_SettingsChangedListeners;
@@ -3454,24 +3455,31 @@ private unsafe void OnUpdate(InputUpdateType updateType, ref InputEventBuffer ev
                         }
                     }
 
+                    var currentEventPtr = new InputEventPtr(currentEventReadPtr);
+
                     // Give listeners a shot at the event.
                     // NOTE: We call listeners also for events where the device is disabled. This is crucial for code
                     //       such as TouchSimulation that disables the originating devices and then uses its events to
                     //       create simulated events from.
                     if (m_EventListeners.length > 0)
                     {
-                        DelegateHelpers.InvokeCallbacksSafe(ref m_EventListeners,
-                            new InputEventPtr(currentEventReadPtr), device, k_InputOnEventMarker, "InputSystem.onEvent");
-
-                        // If a listener marks the event as handled, we don't process it further.
-                        if (m_InputEventHandledPolicy == InputEventHandledPolicy.SuppressStateUpdates &&
-                            currentEventReadPtr->handled)
+                        if (DelegateHelpers.InvokeCallbacksSafeUntilHandled(ref m_EventListeners, currentEventPtr, device, k_InputOnEventMarker, "InputSystem.onEvent",
+                            m_InputEventHandledPolicy == InputEventHandledPolicy.SuppressStateUpdates))
                         {
+                            currentEventReadPtr->handled = true;
+                            SuppressActionsForDevice(device);
                             m_InputEventStream.Advance(false);
                             continue;
                         }
                     }
 
+                    if (m_InputEventHandledPolicy == InputEventHandledPolicy.SuppressStateUpdates && currentEventPtr.handled)
+                    {
+                        SuppressActionsForDevice(device);
+                        m_InputEventStream.Advance(false);
+                        continue;
+                    }
+
                     // Update metrics.
                     if (currentEventTimeInternal <= currentTime)
                         totalEventLag += currentTime - currentEventTimeInternal;
@@ -3484,8 +3492,6 @@ private unsafe void OnUpdate(InputUpdateType updateType, ref InputEventBuffer ev
                         case StateEvent.Type:
                         case DeltaStateEvent.Type:
 
-                            var eventPtr = new InputEventPtr(currentEventReadPtr);
-
                             // Ignore the event if the last state update we received for the device was
                             // newer than this state event is. We don't allow devices to go back in time.
                             //
@@ -3496,7 +3502,7 @@ private unsafe void OnUpdate(InputUpdateType updateType, ref InputEventBuffer ev
                             //       increasing timestamps across all such streams.
                             var deviceIsStateCallbackReceiver = device.hasStateCallbacks;
                             if (currentEventTimeInternal < device.m_LastUpdateTimeInternal &&
-                                !(deviceIsStateCallbackReceiver && device.stateBlock.format != eventPtr.stateFormat))
+                                !(deviceIsStateCallbackReceiver && device.stateBlock.format != currentEventPtr.stateFormat))
                             {
 #if UNITY_EDITOR
                                 m_Diagnostics?.OnEventTimestampOutdated(new InputEventPtr(currentEventReadPtr), device);
@@ -3520,38 +3526,38 @@ private unsafe void OnUpdate(InputUpdateType updateType, ref InputEventBuffer ev
                                 m_ShouldMakeCurrentlyUpdatingDeviceCurrent = true;
                                 // NOTE: We leave it to the device to make sure the event has the right format. This allows the
                                 //       device to handle multiple different incoming formats.
-                                ((IInputStateCallbackReceiver)device).OnStateEvent(eventPtr);
+                                ((IInputStateCallbackReceiver)device).OnStateEvent(currentEventPtr);
 
                                 haveChangedStateOtherThanNoise = m_ShouldMakeCurrentlyUpdatingDeviceCurrent;
                             }
                             else
                             {
                                 // If the state format doesn't match, ignore the event.
-                                if (device.stateBlock.format != eventPtr.stateFormat)
+                                if (device.stateBlock.format != currentEventPtr.stateFormat)
                                 {
 #if UNITY_EDITOR
                                     m_Diagnostics?.OnEventFormatMismatch(currentEventReadPtr, device);
 #endif
                                     break;
                                 }
 
-                                haveChangedStateOtherThanNoise = UpdateState(device, eventPtr, updateType);
+                                haveChangedStateOtherThanNoise = UpdateState(device, currentEventPtr, updateType);
                             }
 
-                            totalEventBytesProcessed += eventPtr.sizeInBytes;
+                            totalEventBytesProcessed += currentEventPtr.sizeInBytes;
 
-                            device.m_CurrentProcessedEventBytesOnUpdate += eventPtr.sizeInBytes;
+                            device.m_CurrentProcessedEventBytesOnUpdate += currentEventPtr.sizeInBytes;
 
                             // Update timestamp on device.
                             // NOTE: We do this here and not in UpdateState() so that InputState.Change() will *NOT* change timestamps.
                             //       Only events should. If running play mode updates in editor, we want to defer to the play mode
                             //       callbacks to set the last update time to avoid dropping events only processed by the editor state.
-                            if (device.m_LastUpdateTimeInternal <= eventPtr.internalTime
+                            if (device.m_LastUpdateTimeInternal <= currentEventPtr.internalTime
 #if UNITY_EDITOR
                                 && !(updateType == InputUpdateType.Editor && runPlayerUpdatesInEditMode)
 #endif
                             )
-                                device.m_LastUpdateTimeInternal = eventPtr.internalTime;
+                                device.m_LastUpdateTimeInternal = currentEventPtr.internalTime;
 
                             // Make device current. Again, only do this when receiving events.
                             if (haveChangedStateOtherThanNoise)
@@ -3890,6 +3896,31 @@ private void InvokeAfterUpdateCallback(InputUpdateType updateType)
 
         private bool m_ShouldMakeCurrentlyUpdatingDeviceCurrent;
 
+        private void SuppressActionsForDevice(InputDevice device)
+        {
+            if (m_InputEventHandledPolicy != InputEventHandledPolicy.SuppressStateUpdates || device == null)
+                return;
+
+            var deviceIndex = device.m_DeviceIndex;
+            if (m_SuppressActionsForDeviceUpdate == null || deviceIndex >= m_SuppressActionsForDeviceUpdate.Length)
+                Array.Resize(ref m_SuppressActionsForDeviceUpdate, deviceIndex + 1);
+            m_SuppressActionsForDeviceUpdate[deviceIndex] = InputUpdate.s_UpdateStepCount;
+        }
+
+        internal bool ShouldSuppressActionsForDevice(InputDevice device)
+        {
+            if (m_InputEventHandledPolicy != InputEventHandledPolicy.SuppressStateUpdates || device == null)
+                return false;
+
+            var deviceIndex = device.m_DeviceIndex;
+            if (m_SuppressActionsForDeviceUpdate == null || deviceIndex >= m_SuppressActionsForDeviceUpdate.Length)
+                return false;
+
+            var lastUpdate = (int)m_SuppressActionsForDeviceUpdate[deviceIndex];
+            var currentUpdate = (int)InputUpdate.s_UpdateStepCount;
+            return lastUpdate == currentUpdate || lastUpdate + 1 == currentUpdate;
+        }
+
         // This is a dirty hot fix to expose entropy from device back to input manager to make a choice if we want to make device current or not.
         // A proper fix would be to change IInputStateCallbackReceiver.OnStateEvent to return bool to make device current or not.
         internal void DontMakeCurrentlyUpdatingDeviceCurrent()

Packages/com.unity.inputsystem/InputSystem/Utilities/DelegateHelpers.cs

@@ -1,5 +1,6 @@
 using System;
 using Unity.Profiling;
+using UnityEngine.InputSystem.LowLevel;
 
 namespace UnityEngine.InputSystem.Utilities
 {
@@ -83,6 +84,45 @@ public static void InvokeCallbacksSafe<TValue1, TValue2>(ref CallbackArray<Actio
             marker.End();
         }
 
+        public static bool InvokeCallbacksSafeUntilHandled(ref CallbackArray<Action<InputEventPtr, InputDevice>> callbacks,
+            InputEventPtr eventPtr, InputDevice device, ProfilerMarker marker, string callbackName, bool stopOnHandled)
+        {
+            if (callbacks.length == 0)
+                return false;
+
+            marker.Begin();
+            callbacks.LockForChanges();
+            var handled = false;
+            try
+            {
+                for (var i = 0; i < callbacks.length; ++i)
+                {
+                    try
+                    {
+                        callbacks[i](eventPtr, device);
+                    }
+                    catch (Exception exception)
+                    {
+                        Debug.LogException(exception);
+                        Debug.LogError($"{exception.GetType().Name} while executing '{callbackName}' callbacks");
+                    }
+
+                    if (stopOnHandled && eventPtr.handled)
+                    {
+                        handled = true;
+                        break;
+                    }
+                }
+            }
+            finally
+            {
+                callbacks.UnlockForChanges();
+                marker.End();
+            }
+
+            return handled;
+        }
+
         public static bool InvokeCallbacksSafe_AnyCallbackReturnsTrue<TValue1, TValue2>(ref CallbackArray<Func<TValue1, TValue2, bool>> callbacks,
             TValue1 argument1, TValue2 argument2, string callbackName, object context = null)
         {