FIX: (ISXB-1524) Rebinding issues related to incorrect event suppression (WIP)

Assets/Samples/RebindingUI/RebindActionUI.cs

@@ -289,6 +289,7 @@ void CleanUp()
                         UpdateBindingDisplay();
                         CleanUp();
                     })
+                .WithSuppressedActionPropagation()
                 .OnComplete(
                     operation =>
                     {

Assets/Tests/InputSystem/CoreTests_Events.cs

@@ -1216,6 +1216,92 @@ public void Events_CanPreventEventsFromBeingProcessed()
         Assert.That(device.rightTrigger.ReadValue(), Is.EqualTo(0.0).Within(0.00001));
     }
 
+    class SuppressedActionEventData
+    {
+        public bool markNextEventHandled;
+        public int startedCount;
+        public int performedCount;
+        public int canceledCount;
+    }
+
+    [Test]
+    [Category("Events")]
+    public void EventHandledPolicy_ShouldReflectUserSetting()
+    {
+        // Assert default setting
+        Assert.That(InputSystem.inputEventHandledPolicy, Is.EqualTo(InputEventHandledPolicy.SuppressStateUpdates));
+
+        // Assert policy can be changed
+        InputSystem.inputEventHandledPolicy = InputEventHandledPolicy.SuppressActionUpdates;
+        Assert.That(InputSystem.inputEventHandledPolicy, Is.EqualTo(InputEventHandledPolicy.SuppressActionUpdates));
+
+        // Assert policy can be changed back
+        InputSystem.inputEventHandledPolicy = InputEventHandledPolicy.SuppressStateUpdates;
+        Assert.That(InputSystem.inputEventHandledPolicy, Is.EqualTo(InputEventHandledPolicy.SuppressStateUpdates));
+
+        // Assert setting property to an invalid value throws exception and do not have side-effects
+        Assert.Throws<ArgumentOutOfRangeException>(() =>
+            InputSystem.inputEventHandledPolicy = (InputEventHandledPolicy)123456);
+        Assert.That(InputSystem.inputEventHandledPolicy, Is.EqualTo(InputEventHandledPolicy.SuppressStateUpdates));
+    }
+
+    [TestCase(InputEventHandledPolicy.SuppressStateUpdates,
+        new int[] { 0, 0, 1, 1}, new int[] {0, 0, 0, 1})]
+    [TestCase(InputEventHandledPolicy.SuppressActionUpdates,
+        new int[] { 0, 0, 0, 0}, new int[] {0, 0, 0, 0})]
+    [Category("Events")]
+    [Description("ISXB-1524 Events suppressed has side-effects on actions when based on polling")]
+    public void Events_ShouldRespectHandledPolicyUponUpdate(InputEventHandledPolicy policy,
+        int[] expectedProcessed, int[] expectedCancelled) // EDIT
+    {
+        // Update setting to match desired scenario
+        InputSystem.inputEventHandledPolicy = policy;
+
+        // Use a boxed boolean to allow lambda to capture reference.
+        var data = new SuppressedActionEventData();
+
+        InputSystem.onEvent +=
+            (inputEvent, _) =>
+        {
+            // If we mark the event handled, the system should skip it and not
+            // let it go to the device.
+            inputEvent.handled = data.markNextEventHandled;
+        };
+
+        var device = InputSystem.AddDevice<Gamepad>();
+        var action = new InputAction(type: InputActionType.Button, binding: "<Gamepad>/buttonNorth");
+        action.Enable();
+        action.started += _ => ++ data.startedCount;
+        action.performed += _ => ++ data.performedCount;
+        action.canceled += _ => ++ data.canceledCount;
+
+        // Ensure state is updated/initialized
+        InputSystem.QueueStateEvent(device, new GamepadState() { leftStick = new Vector2(0.01f, 0.0f) });
+        InputSystem.Update();
+        Assert.That(data.performedCount, Is.EqualTo(expectedProcessed[0]));
+        Assert.That(data.canceledCount, Is.EqualTo(expectedCancelled[0]));
+
+        // Press button north with event suppression active
+        data.markNextEventHandled = true;
+        InputSystem.QueueStateEvent(device, new GamepadState() { leftStick = new Vector2(0.00f, 0.01f) }.WithButton(GamepadButton.North));
+        InputSystem.Update();
+        Assert.That(data.performedCount, Is.EqualTo(expectedProcessed[1]));
+        Assert.That(data.canceledCount, Is.EqualTo(expectedCancelled[1]));
+
+        // Simulate a periodic reading, this will trigger performed count
+        data.markNextEventHandled = false;
+        InputSystem.QueueStateEvent(device, new GamepadState() { leftStick = new Vector2(0.01f, 0.00f) }.WithButton(GamepadButton.North));
+        InputSystem.Update();
+        Assert.That(data.performedCount, Is.EqualTo(expectedProcessed[2])); // Firing without actual change
+        Assert.That(data.canceledCount, Is.EqualTo(expectedCancelled[2]));
+
+        // Release button north
+        InputSystem.QueueStateEvent(device, new GamepadState() { leftStick = new Vector2(0.00f, 0.01f) });
+        InputSystem.Update();
+        Assert.That(data.performedCount, Is.EqualTo(expectedProcessed[3]));
+        Assert.That(data.canceledCount, Is.EqualTo(expectedCancelled[3]));
+    }
+
     [StructLayout(LayoutKind.Explicit, Size = 2)]
     struct StateWith2Bytes : IInputStateTypeInfo
     {

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -10,6 +10,9 @@ however, it has to be formatted properly to pass verification tests.
 
 ## [Unreleased] - yyyy-mm-dd
 
+### Added
+- Added a new run-time setting `InputSystem.inputEventHandledPolicy` which allows changing how the system processes input events marked as "handled". The new alternative setting (not default) allows for allowing handled events to propagate into state changes but still suppresses action interactions from being processed.
+
 ### Fixed
 - Fixed an analytics event being invoked twice when the Save button in the Actions view was pressed. [ISXB-1378](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1378)
 - Fixed an issue causing a number of errors to be displayed when using `InputTestFixture` in playmode tests with domain reloading disabled on playmode entry. [ISXB-1446](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1446)
@@ -24,6 +27,7 @@ however, it has to be formatted properly to pass verification tests.
 - Fixed Gamepad stick up/down inputs that were not recognized in WebGL. [ISXB-1090](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1090)
 - Fixed PlayerInput component automatically switching away from the default ActionMap set to 'None'.
 - Fixed a console error being shown when targeting visionOS builds in 2022.3.
+- Fixed an issue in `RebindingUISample` that fired actions bound to the same control as the target control in a rebinding process. ISXB-1524.
 
 ## [1.14.0] - 2025-03-20
 

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

@@ -2083,6 +2083,23 @@ public RebindingOperation OnMatchWaitForAnother(float seconds)
                 return this;
             }
 
+            /// <summary>
+            /// Ensures state changes are allowed to propagate during rebinding but suppresses action updates.
+            /// The default behavior is that state changes are also suppressed during rebinding.
+            /// </summary>
+            /// <remarks>
+            /// This is achieved by temporarily setting <see cref="InputSystem.inputEventHandledPolicy"/> to
+            /// <see cref="InputEventHandledPolicy.SuppressActionUpdates" />. This is automatically reverted when
+            /// the rebinding operation completes. If the policy is already set to
+            /// <see cref="InputEventHandledPolicy.SuppressActionUpdates" />, this method has no effect.
+            /// </remarks>
+            /// <returns>Reference to this rebinding operation.</returns>
+            public RebindingOperation WithSuppressedActionPropagation()
+            {
+                m_TargetInputEventHandledPolicy = InputEventHandledPolicy.SuppressActionUpdates;
+                return this;
+            }
+
             /// <summary>
             /// Start the rebinding. This should be invoked after the rebind operation has been fully configured.
             /// </summary>
@@ -2107,6 +2124,9 @@ public RebindingOperation Start()
 
                 m_StartTime = InputState.currentTime;
 
+                m_SavedInputEventHandledPolicy = InputSystem.inputEventHandledPolicy;
+                InputSystem.inputEventHandledPolicy = m_TargetInputEventHandledPolicy;
+
                 if (m_WaitSecondsAfterMatch > 0 || m_Timeout > 0)
                 {
                     HookOnAfterUpdate();
@@ -2606,6 +2626,8 @@ private void ResetAfterMatchCompleted()
 
                 UnhookOnEvent();
                 UnhookOnAfterUpdate();
+
+                InputSystem.inputEventHandledPolicy = m_SavedInputEventHandledPolicy;
             }
 
             private void ThrowIfRebindInProgress()
@@ -2654,6 +2676,8 @@ private string GeneratePathForControl(InputControl control)
             private double m_StartTime;
             private float m_Timeout;
             private float m_WaitSecondsAfterMatch;
+            private InputEventHandledPolicy m_SavedInputEventHandledPolicy;
+            private InputEventHandledPolicy m_TargetInputEventHandledPolicy;
             private InputControlList<InputControl> m_Candidates;
             private Action<RebindingOperation> m_OnComplete;
             private Action<RebindingOperation> m_OnCancel;

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

@@ -1346,6 +1346,7 @@ void IInputStateChangeMonitor.NotifyControlStateChanged(InputControl control, do
             #endif
 
             SplitUpMapAndControlAndBindingIndex(mapControlAndBindingIndex, out var mapIndex, out var controlIndex, out var bindingIndex);
+            // CALLBACK HERE
             ProcessControlStateChange(mapIndex, controlIndex, bindingIndex, time, eventPtr);
         }
 
@@ -1516,6 +1517,10 @@ private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindi
                         }
                     }
 
+                    // Check if we should suppress interaction processing
+                    var suppressInteractionProcessing = (eventPtr != null) && eventPtr.handled &&
+                        InputSystem.inputEventHandledPolicy == InputEventHandledPolicy.SuppressActionUpdates;
+
                     // Check if we have multiple concurrent actuations on the same action. This may lead us
                     // to ignore certain inputs (e.g. when we get an input of lesser magnitude while already having
                     // one of higher magnitude) or may even lead us to switch to processing a different binding
@@ -1537,6 +1542,7 @@ private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindi
                     }
                     else if (!haveInteractionsOnComposite && !isConflictingInput)
                     {
+                        //if (!suppressInteractionProcessing)
                         ProcessDefaultInteraction(ref trigger, actionIndex);
                     }
                 }
@@ -1939,6 +1945,7 @@ private void ProcessDefaultInteraction(ref TriggerState trigger, int actionIndex
                         var threshold = controls[trigger.controlIndex] is ButtonControl button ? button.pressPointOrDefault : ButtonControl.s_GlobalDefaultButtonPressPoint;
                         if (actuation >= threshold)
                         {
+                            // CALLBACK HERE
                             ChangePhaseOfAction(InputActionPhase.Performed, ref trigger,
                                 phaseAfterPerformedOrCanceled: InputActionPhase.Performed);
                         }
@@ -2397,6 +2404,7 @@ private bool ChangePhaseOfAction(InputActionPhase newPhase, ref TriggerState tri
                 }
                 else if (actionState->phase != newPhase || newPhase == InputActionPhase.Performed) // We allow Performed to trigger repeatedly.
                 {
+                    // CALLBACK HERE
                     ChangePhaseOfActionInternal(actionIndex, actionState, newPhase, ref trigger,
                         isDisablingAction: newPhase == InputActionPhase.Canceled && phaseAfterPerformedOrCanceled == InputActionPhase.Disabled);
                     if (!actionState->inProcessing)
@@ -2491,6 +2499,9 @@ private void ChangePhaseOfActionInternal(int actionIndex, TriggerState* actionSt
                 newState.startTime = newState.time;
             *actionState = newState;
 
+            //if (InputSystem.inputEventHandledPolicy == InputEventHandledPolicy.SuppressNotifications)
+            //    return;
+
             // Let listeners know.
             var map = maps[trigger.mapIndex];
             Debug.Assert(actionIndex >= mapIndices[trigger.mapIndex].actionStartIndex,
@@ -2509,6 +2520,7 @@ private void ChangePhaseOfActionInternal(int actionIndex, TriggerState* actionSt
                 case InputActionPhase.Performed:
                 {
                     Debug.Assert(trigger.controlIndex != -1, "Must have control to perform an action");
+                    // CALLBACK HERE
                     CallActionListeners(actionIndex, map, newPhase, ref action.m_OnPerformed, "performed");
                     break;
                 }
@@ -2562,6 +2574,7 @@ private void CallActionListeners(int actionIndex, InputActionMap actionMap, Inpu
             }
 
             // Run callbacks (if any) directly on action.
+            // CALLBACK INVOKED HERE
             DelegateHelpers.InvokeCallbacksSafe(ref listeners, context, callbackName, action);
 
             // Run callbacks (if any) on action map.

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

@@ -0,0 +1,19 @@
+namespace UnityEngine.InputSystem.LowLevel
+{
+    /// <summary>
+    /// Policy defining how the Input System will react to <see cref="InputEvent"/> instances marked as
+    /// <see cref="InputEvent.handled"/> (Or marked handled via <see cref="InputEventPtr.handled"/>).
+    /// </summary>
+    public enum InputEventHandledPolicy
+    {
+        /// <summary>
+        /// Input events will be discarded directly and not propagate for state changes.
+        /// </summary>
+        SuppressStateUpdates,
+
+        /// <summary>
+        /// Input events will be processed for state updates but will not trigger interaction nor phase updates.
+        /// </summary>
+        SuppressActionUpdates
+    }
+}

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

@@ -210,6 +210,23 @@ public float pollingFrequency
             }
         }
 
+        internal InputEventHandledPolicy inputEventHandledPolicy
+        {
+            get => m_InputEventHandledPolicy;
+            set
+            {
+                switch (value)
+                {
+                    case InputEventHandledPolicy.SuppressActionUpdates:
+                    case InputEventHandledPolicy.SuppressStateUpdates:
+                        m_InputEventHandledPolicy = value;
+                        break;
+                    default:
+                        throw new ArgumentOutOfRangeException("value");
+                }
+            }
+        }
+
         public event DeviceChangeListener onDeviceChange
         {
             add => m_DeviceChangeListeners.AddCallback(value);
@@ -1891,6 +1908,9 @@ internal void InitializeData()
             // Default polling frequency is 60 Hz.
             m_PollingFrequency = 60;
 
+            // Default input event handled policy.
+            m_InputEventHandledPolicy = InputEventHandledPolicy.SuppressStateUpdates;
+
             // Register layouts.
             // NOTE: Base layouts must be registered before their derived layouts
             //       for the detection of base layouts to work.
@@ -2142,6 +2162,7 @@ internal struct AvailableDevice
         // Used by EditorInputControlLayoutCache to determine whether its state is outdated.
         internal int m_LayoutRegistrationVersion;
         private float m_PollingFrequency;
+        private InputEventHandledPolicy m_InputEventHandledPolicy;
 
         internal InputControlLayout.Collection m_Layouts;
         private TypeTable m_Processors;
@@ -3456,7 +3477,8 @@ private unsafe void OnUpdate(InputUpdateType updateType, ref InputEventBuffer ev
                             new InputEventPtr(currentEventReadPtr), device, k_InputOnEventMarker, "InputSystem.onEvent");
 
                         // If a listener marks the event as handled, we don't process it further.
-                        if (currentEventReadPtr->handled)
+                        if (m_InputEventHandledPolicy == InputEventHandledPolicy.SuppressStateUpdates &&
+                            currentEventReadPtr->handled)
                         {
                             m_InputEventStream.Advance(false);
                             continue;
@@ -4048,6 +4070,7 @@ internal struct SerializedState
         {
             public int layoutRegistrationVersion;
             public float pollingFrequency;
+            public InputEventHandledPolicy inputEventHandledPolicy;
             public DeviceState[] devices;
             public AvailableDevice[] availableDevices;
             public InputStateBuffers buffers;
@@ -4093,6 +4116,7 @@ internal SerializedState SaveState()
             {
                 layoutRegistrationVersion = m_LayoutRegistrationVersion,
                 pollingFrequency = m_PollingFrequency,
+                inputEventHandledPolicy =  m_InputEventHandledPolicy,
                 devices = deviceArray,
                 availableDevices = m_AvailableDevices?.Take(m_AvailableDeviceCount).ToArray(),
                 buffers = m_StateBuffers,
@@ -4119,6 +4143,7 @@ internal void RestoreStateWithoutDevices(SerializedState state)
             scrollDeltaBehavior = state.scrollDeltaBehavior;
             m_Metrics = state.metrics;
             m_PollingFrequency = state.pollingFrequency;
+            m_InputEventHandledPolicy = state.inputEventHandledPolicy;
 
             if (m_Settings != null)
                 Object.DestroyImmediate(m_Settings);

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

@@ -382,7 +382,7 @@ internal unsafe void FireStateChangeNotifications(int deviceIndex, double intern
 
             // Call IStateChangeMonitor.NotifyControlStateChange for every monitor that is in
             // signalled state.
-            eventPtr->handled = false;
+            var previouslyHandled = eventPtr->handled;
             for (var i = 0; i < signals.length; ++i)
             {
                 if (!signals.TestBit(i))
@@ -403,8 +403,9 @@ internal unsafe void FireStateChangeNotifications(int deviceIndex, double intern
 
                 // If the monitor signalled that it has processed the state change, reset all signalled
                 // state monitors in the same group. This is what causes "SHIFT+B" to prevent "B" from
-                // also triggering.
-                if (eventPtr->handled)
+                // also triggering. Note that we skip this if it was already marked handled before notifying
+                // monitors.
+                if (!previouslyHandled && eventPtr->handled)
                 {
                     var groupIndex = listeners[i].groupIndex;
                     for (var n = i + 1; n < signals.length; ++n)
@@ -420,11 +421,12 @@ internal unsafe void FireStateChangeNotifications(int deviceIndex, double intern
                         if (listeners[n].groupIndex == groupIndex && listeners[n].monitor == listener.monitor)
                             signals.ClearBit(n);
                     }
+                }
 
-                    // Need to reset it back to false as we may have more signalled state monitors that
-                    // aren't in the same group (i.e. have independent inputs).
+                // Need to reset it back to false as we may have more signalled state monitors that
+                // aren't in the same group (i.e. have independent inputs).
+                if (eventPtr->handled)
                     eventPtr->handled = false;
-                }
 
                 signals.ClearBit(i);
             }