FIX: ISXB-1127 composite incorrectly returning IsPressed

Assets/Tests/InputSystem/CoreTests_Actions.cs

@@ -12405,99 +12405,4 @@ public void Actions_ActionMapDisabledDuringOnAfterSerialization()
         Assert.That(map.enabled, Is.True);
         Assert.That(map.FindAction("MyAction", true).enabled, Is.True);
     }
-
-    // ResetDevice wasn't properly clearly Composite key state, i.e. BindingState.pressTime
-    // https://jira.unity3d.com/browse/ISXB-746
-    [Test]
-    [TestCase(false)]
-    [TestCase(true)]
-    [Category("Actions")]
-    public void Actions_CompositeBindingResetWhenResetDeviceCalledWhileExecutingAction(bool useTwoModifierComposite)
-    {
-        var keyboard = InputSystem.AddDevice<Keyboard>();
-        bool actionPerformed;
-
-        // Enables "Modifier must be pressed first" behavior on all Composite Bindings
-        InputSystem.settings.shortcutKeysConsumeInput = true;
-
-        const string modifier1 = "<Keyboard>/shift";
-        const string modifier2 = "<Keyboard>/ctrl";
-        const string key = "<Keyboard>/F1";
-
-        var map = new InputActionMap();
-        var resetAction = map.AddAction("resetAction");
-
-        if (!useTwoModifierComposite)
-        {
-            resetAction.AddCompositeBinding("OneModifier")
-                .With("Modifier", modifier1)
-                .With("Binding", key);
-        }
-        else
-        {
-            resetAction.AddCompositeBinding("TwoModifiers")
-                .With("Modifier1", modifier1)
-                .With("Modifier2", modifier2)
-                .With("Binding", key);
-        }
-
-        resetAction.performed += (InputAction.CallbackContext ctx) =>
-        {
-            // Disable the Keyboard while action is being performed.
-            // This simulates an "OnFocusLost" event occurring while processing the Action, e.g. when switching primary displays or moving the main window
-            actionPerformed = true;
-            InputSystem.s_Manager.EnableOrDisableDevice(keyboard.device, false, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);
-        };
-
-        map.Enable();
-
-        actionPerformed = false;
-        Press(keyboard.leftShiftKey);
-        Press(keyboard.leftCtrlKey);
-        Press(keyboard.f1Key);
-
-        Assert.IsTrue(actionPerformed);
-
-        // Re enable the Keyboard (before keys are released) and execute Action again
-        InputSystem.s_Manager.EnableOrDisableDevice(keyboard.device, true, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);
-
-        actionPerformed = false;
-        Release(keyboard.leftShiftKey);
-        Release(keyboard.leftCtrlKey);
-        Release(keyboard.f1Key);
-
-        Press(keyboard.leftCtrlKey);
-        Press(keyboard.leftShiftKey);
-        Press(keyboard.f1Key);
-
-        Assert.IsTrue(actionPerformed);
-
-        actionPerformed = false;
-        Release(keyboard.leftCtrlKey);
-        Release(keyboard.leftShiftKey);
-        Release(keyboard.f1Key);
-
-        // Re enable the Keyboard (after keys are released) and execute Action one more time
-        InputSystem.s_Manager.EnableOrDisableDevice(keyboard.device, true, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);
-
-        Press(keyboard.leftCtrlKey);
-        Press(keyboard.leftShiftKey);
-        Press(keyboard.f1Key);
-
-        Assert.IsTrue(actionPerformed);
-
-        actionPerformed = false;
-        Press(keyboard.leftShiftKey);
-        Press(keyboard.leftCtrlKey);
-        Press(keyboard.f1Key);
-
-        // Re enable the Keyboard (before keys are released) and verify Action isn't triggered when Key pressed first
-        InputSystem.s_Manager.EnableOrDisableDevice(keyboard.device, true, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);
-
-        Press(keyboard.f1Key);
-        Press(keyboard.leftCtrlKey);
-        Press(keyboard.leftShiftKey);
-
-        Assert.IsFalse(actionPerformed);
-    }
 }

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -12,6 +12,7 @@ however, it has to be formatted properly to pass verification tests.
 
 ### Fixed
 - Fixed an issue causing the Action context menu to not show on right click when right clicking an action in the Input Action Editor [ISXB-1134](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1134).
+- Reverted changes from 0ddd534d8 (ISXB-746) which introduced a regression [ISXB-1127](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1127).
 - Fixed `ArgumentNullException: Value cannot be null.` during the migration of Project-wide Input Actions from `InputManager.asset` to `InputSystem_Actions.inputactions` asset which lead do the lost of the configuration [ISXB-1105](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1105)
 - Fixed pointerId staying the same when simultaneously releasing and then pressing in the same frame on mobile using touch. [ISXB-1006](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-845)
 

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

@@ -1476,43 +1476,37 @@ private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindi
                     // If the binding is part of a composite, check for interactions on the composite
                     // itself and give them a first shot at processing the value change.
                     var haveInteractionsOnComposite = false;
-                    var compositeAlreadyTriggered = false;
                     if (bindingStatePtr->isPartOfComposite)
                     {
                         var compositeBindingIndex = bindingStatePtr->compositeOrCompositeBindingIndex;
                         var compositeBindingPtr = &bindingStates[compositeBindingIndex];
 
-                        // If the composite has already been triggered from the very same event set a flag so it isn't triggered again.
+                        // If the composite has already been triggered from the very same event, ignore it.
                         // Example: KeyboardState change that includes both A and W key state changes and we're looking
                         //          at a WASD composite binding. There's a state change monitor on both the A and the W
                         //          key and thus the manager will notify us individually of both changes. However, we
                         //          want to perform the action only once.
-                        // NOTE: Do NOT ignore this Event, we still need finish processing the individual button states.
-                        if (!ShouldIgnoreInputOnCompositeBinding(compositeBindingPtr, eventPtr))
-                        {
-                            // Update magnitude for composite.
-                            var compositeIndex = bindingStates[compositeBindingIndex].compositeOrCompositeBindingIndex;
-                            var compositeContext = new InputBindingCompositeContext
-                            {
-                                m_State = this,
-                                m_BindingIndex = compositeBindingIndex
-                            };
-                            trigger.magnitude = composites[compositeIndex].EvaluateMagnitude(ref compositeContext);
-                            memory.compositeMagnitudes[compositeIndex] = trigger.magnitude;
+                        if (ShouldIgnoreInputOnCompositeBinding(compositeBindingPtr, eventPtr))
+                            return;
 
-                            // Run through interactions on composite.
-                            var interactionCountOnComposite = compositeBindingPtr->interactionCount;
-                            if (interactionCountOnComposite > 0)
-                            {
-                                haveInteractionsOnComposite = true;
-                                ProcessInteractions(ref trigger,
-                                    compositeBindingPtr->interactionStartIndex,
-                                    interactionCountOnComposite);
-                            }
-                        }
-                        else
+                        // Update magnitude for composite.
+                        var compositeIndex = bindingStates[compositeBindingIndex].compositeOrCompositeBindingIndex;
+                        var compositeContext = new InputBindingCompositeContext
+                        {
+                            m_State = this,
+                            m_BindingIndex = compositeBindingIndex
+                        };
+                        trigger.magnitude = composites[compositeIndex].EvaluateMagnitude(ref compositeContext);
+                        memory.compositeMagnitudes[compositeIndex] = trigger.magnitude;
+
+                        // Run through interactions on composite.
+                        var interactionCountOnComposite = compositeBindingPtr->interactionCount;
+                        if (interactionCountOnComposite > 0)
                         {
-                            compositeAlreadyTriggered = true;
+                            haveInteractionsOnComposite = true;
+                            ProcessInteractions(ref trigger,
+                                compositeBindingPtr->interactionStartIndex,
+                                interactionCountOnComposite);
                         }
                     }
 
@@ -1521,31 +1515,21 @@ private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindi
                     // one of higher magnitude) or may even lead us to switch to processing a different binding
                     // (e.g. when an input of previously greater magnitude has now fallen below the level of another
                     // ongoing input with now higher magnitude).
-                    //
-                    // If Composite has already been triggered, skip this step; it's unnecessary and could also
-                    // cause a processing issue if we switch to another binding.
-                    var isConflictingInput = false;
-                    if (!compositeAlreadyTriggered)
-                    {
-                        isConflictingInput = IsConflictingInput(ref trigger, actionIndex);
-                        bindingStatePtr = &bindingStates[trigger.bindingIndex]; // IsConflictingInput may switch us to a different binding.
-                    }
+                    var isConflictingInput = IsConflictingInput(ref trigger, actionIndex);
+                    bindingStatePtr = &bindingStates[trigger.bindingIndex]; // IsConflictingInput may switch us to a different binding.
 
                     // Process button presses/releases.
-                    // We MUST execute this processing even if Composite has already been triggered to ensure button states
-                    // are properly updated (ISXB-746)
                     if (!isConflictingInput)
                         ProcessButtonState(ref trigger, actionIndex, bindingStatePtr);
 
                     // If we have interactions, let them do all the processing. The presence of an interaction
                     // essentially bypasses the default phase progression logic of an action.
-                    // Interactions are skipped if compositeAlreadyTriggered is set.
                     var interactionCount = bindingStatePtr->interactionCount;
                     if (interactionCount > 0 && !bindingStatePtr->isPartOfComposite)
                     {
                         ProcessInteractions(ref trigger, bindingStatePtr->interactionStartIndex, interactionCount);
                     }
-                    else if (!haveInteractionsOnComposite && !isConflictingInput && !compositeAlreadyTriggered)
+                    else if (!haveInteractionsOnComposite && !isConflictingInput)
                     {
                         ProcessDefaultInteraction(ref trigger, actionIndex);
                     }