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

Assets/ActionDebug.cs

@@ -0,0 +1,39 @@
+using UnityEngine;
+using UnityEngine.InputSystem;
+using UnityEngine.InputSystem.LowLevel;
+
+public class ActionDebug : MonoBehaviour
+{
+    public InputActionReference trigger;
+
+    // Start is called once before the first execution of Update after the MonoBehaviour is created
+    void Start()
+    {
+        trigger.action.performed += ActionOnPerformed;
+        trigger.action.canceled += ActionOnCanceled;
+        trigger.action.started += ActionOnStarted;
+        trigger.action.Enable();
+    }
+
+    private void ActionOnStarted(InputAction.CallbackContext obj)
+    {
+        Debug.Log("Action Started");
+    }
+
+    private void ActionOnCanceled(InputAction.CallbackContext obj)
+    {
+        Debug.Log("Action Canceled");
+    }
+
+    private void ActionOnPerformed(InputAction.CallbackContext obj)
+    {
+        Debug.Log("Action Performed");
+    }
+
+    // Update is called once per frame
+    void Update()
+    {
+        if (trigger.action.WasPerformedThisFrame())
+            Debug.Log("Action Performed (Polled Event)");
+    }
+}

Assets/Samples/RebindingUI/RebindActionUI.cs

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

Assets/Samples/RebindingUI/RebindUISampleActions.inputactions

@@ -11,23 +11,35 @@
                     "id": "9d8fcbff-87d1-43ef-857e-931c84d5bd72",
                     "expectedControlType": "Vector2",
                     "processors": "",
-                    "interactions": ""
+                    "interactions": "",
+                    "initialStateCheck": true
                 },
                 {
                     "name": "Look",
                     "type": "Value",
                     "id": "ddab72da-d325-4b4c-a484-abe4f6bdf113",
                     "expectedControlType": "Vector2",
                     "processors": "",
-                    "interactions": ""
+                    "interactions": "",
+                    "initialStateCheck": true
                 },
                 {
                     "name": "Interact",
                     "type": "Button",
                     "id": "2bd60403-0923-469e-a3a4-7338b04f6bbc",
                     "expectedControlType": "",
                     "processors": "",
-                    "interactions": ""
+                    "interactions": "",
+                    "initialStateCheck": false
+                },
+                {
+                    "name": "Test",
+                    "type": "Button",
+                    "id": "a0bbd927-54af-4ba8-be59-8470a31efd79",
+                    "expectedControlType": "",
+                    "processors": "",
+                    "interactions": "",
+                    "initialStateCheck": false
                 }
             ],
             "bindings": [
@@ -140,6 +152,17 @@
                     "action": "Interact",
                     "isComposite": false,
                     "isPartOfComposite": false
+                },
+                {
+                    "name": "",
+                    "id": "60a650d1-dafb-4f35-bfb4-94d9974472fb",
+                    "path": "<Gamepad>/buttonNorth",
+                    "interactions": "",
+                    "processors": "",
+                    "groups": "",
+                    "action": "Test",
+                    "isComposite": false,
+                    "isPartOfComposite": false
                 }
             ]
         }

Assets/Samples/RebindingUI/RebindingUISampleScene.unity

@@ -1096,6 +1096,7 @@ GameObject:
   - component: {fileID: 780148237}
   - component: {fileID: 780148236}
   - component: {fileID: 780148235}
+  - component: {fileID: 780148238}
   m_Layer: 0
   m_Name: EventSystem
   m_TagString: Untagged
@@ -1175,6 +1176,19 @@ Transform:
   m_Children: []
   m_Father: {fileID: 0}
   m_LocalEulerAnglesHint: {x: 0, y: 0, z: 0}
+--- !u!114 &780148238
+MonoBehaviour:
+  m_ObjectHideFlags: 0
+  m_CorrespondingSourceObject: {fileID: 0}
+  m_PrefabInstance: {fileID: 0}
+  m_PrefabAsset: {fileID: 0}
+  m_GameObject: {fileID: 780148234}
+  m_Enabled: 1
+  m_EditorHideFlags: 0
+  m_Script: {fileID: 11500000, guid: 7d36843388d90a04782f5f49339e23c4, type: 3}
+  m_Name: 
+  m_EditorClassIdentifier: 
+  trigger: {fileID: 2345695455583553484, guid: 7dead05c54ca85b4681351aafd8bd03a, type: 3}
 --- !u!1 &861395291
 GameObject:
   m_ObjectHideFlags: 0

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/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);
         }
 
@@ -1537,6 +1538,14 @@ private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindi
                     }
                     else if (!haveInteractionsOnComposite && !isConflictingInput)
                     {
+                        // Skip further notification if event is handled and our policy suppress notifications.
+                        if (eventPtr != null && eventPtr.handled && InputSystem.inputEventHandledPolicy ==
+                            InputEventHandledPolicy.SuppressActionUpdates)
+                        {
+                            return;
+                        }
+
+                        // CALLBACK HERE
                         ProcessDefaultInteraction(ref trigger, actionIndex);
                     }
                 }
@@ -1939,6 +1948,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 +2407,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 +2502,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 +2523,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 +2577,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
+    }
+}