Merge branch 'develop' into isxb-1070-onscreenstick-memory-leak-fix

Author: ekcoh

Assets/Tests/InputSystem/CoreTests_Actions.cs

@@ -10041,6 +10041,112 @@ public void Actions_CanCreateCompositesWithMultipleBindings()
         Assert.That(value, Is.EqualTo(new Vector2(-1, -1).normalized).Using(Vector2EqualityComparer.Instance));
     }
 
+    // Ensure that https://jira.unity3d.com/browse/ISXB-619 regress
+    [Test]
+    [Category("Actions")]
+    public void Actions_WithCompositeWithMultipleInteractions_Works()
+    {
+        // Will ensure that :
+        // PressRelease AW trigger a tap
+        // Long PressRelease AW trigger a hold
+        // PressRelease AW trigger a tap
+
+        var keyboard = InputSystem.AddDevice<Keyboard>();
+        var action = new InputAction();
+        action.AddCompositeBinding("Dpad", interactions: "tap,hold(duration=2)")
+            .With("Up", "<Keyboard>/w")
+            .With("Down", "<Keyboard>/s")
+            .With("Left", "<Keyboard>/a")
+            .With("Right", "<Keyboard>/d");
+        action.Enable();
+
+        IInputInteraction performedInteraction = null;
+        IInputInteraction canceledInteraction = null;
+        action.performed += ctx =>
+        {
+            performedInteraction = ctx.interaction;
+        };
+        action.canceled += ctx =>
+        {
+            canceledInteraction = ctx.interaction;
+        };
+
+        // PressRelease AW trigger a tap
+        currentTime = 0;
+        InputSystem.QueueStateEvent(keyboard, new KeyboardState(Key.A, Key.W));
+        InputSystem.Update();
+
+        // nothing triggered
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.Null);
+
+        currentTime += InputSystem.settings.defaultTapTime / 2.0f; // half of the tap time to ensure that it performs.
+        InputSystem.QueueStateEvent(keyboard, new KeyboardState());
+        InputSystem.Update();
+
+        // tap should be triggered
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.TypeOf(typeof(TapInteraction)));
+        performedInteraction = null;
+
+        // Long PressRelease AW trigger a hold
+        currentTime += 1;
+        InputSystem.QueueStateEvent(keyboard, new KeyboardState(Key.A, Key.W));
+        InputSystem.Update();
+
+        // tap should be canceled
+        currentTime += InputSystem.settings.defaultTapTime * 4.0;
+        InputSystem.Update();
+
+        Assert.That(canceledInteraction, Is.TypeOf(typeof(TapInteraction)));
+        Assert.That(performedInteraction, Is.Null);
+        canceledInteraction = null;
+
+        // After (defaultTapTime*4 + 2) seconds hold should be performed with duration=2
+        currentTime += 2;
+        InputSystem.Update();
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.TypeOf(typeof(HoldInteraction)));
+        performedInteraction = null;
+
+        // hold should be canceled
+        currentTime += 1;
+        InputSystem.QueueStateEvent(keyboard, new KeyboardState());
+        InputSystem.Update();
+        Assert.That(canceledInteraction, Is.TypeOf(typeof(HoldInteraction)));
+        Assert.That(performedInteraction, Is.Null);
+        canceledInteraction = null;
+
+        // Should be no other remaining events
+        currentTime += 5;
+        InputSystem.Update();
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.Null);
+
+        // PressRelease AW trigger a tap to ensure that is still working
+        currentTime += 1;
+        InputSystem.QueueStateEvent(keyboard, new KeyboardState(Key.A, Key.W));
+        InputSystem.Update();
+
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.Null);
+
+        currentTime += InputSystem.settings.defaultTapTime / 2.0f;
+        InputSystem.QueueStateEvent(keyboard, new KeyboardState());
+        InputSystem.Update();
+
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.TypeOf(typeof(TapInteraction)));
+        performedInteraction = null;
+        canceledInteraction = null;
+
+        // Should be no other remaining events
+        currentTime += 100;
+        InputSystem.Update();
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.Null);
+    }
+
     [Test]
     [Category("Actions")]
     public void Actions_WithMultipleComposites_CancelsIfCompositeIsReleased()

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -11,6 +11,7 @@ however, it has to be formatted properly to pass verification tests.
 ## [Unreleased] - yyyy-mm-dd
 
 ### Fixed
+- Fixed Multiple interactions could breaks on Composite Binding. [ISXB-619](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-619)
 - Fixed memory leak when the OnScreenStick component was destroyed [ISXB-1070](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1070). Contribution by [LukeUnityDev](https://github.com/LukeUnityDev).
 
 ### Changed

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

@@ -2215,7 +2215,7 @@ internal void ChangePhaseOfInteraction(InputActionPhase newPhase, ref TriggerSta
 
             // See if it affects the phase of an associated action.
             var actionIndex = bindingStates[bindingIndex].actionIndex; // We already had to tap this array and entry in ProcessControlStateChange.
-            if (actionIndex != -1)
+            if (actionIndex != kInvalidIndex)
             {
                 if (actionStates[actionIndex].phase == InputActionPhase.Waiting)
                 {
@@ -2313,12 +2313,14 @@ internal void ChangePhaseOfInteraction(InputActionPhase newPhase, ref TriggerSta
             // Exception: if it was performed and we're to remain in started state, set the interaction
             //            to started. Note that for that phase transition, there are no callbacks being
             //            triggered (i.e. we don't call 'started' every time after 'performed').
-            if (newPhase == InputActionPhase.Performed && actionStates[actionIndex].interactionIndex != trigger.interactionIndex)
-            {
-                // We performed but we're not the interaction driving the action. We want to stay performed to make
-                // sure that if the interaction that is currently driving the action cancels, we get to perform
-                // the action. If we go back to waiting here, then the system can't tell that there's another interaction
-                // ready to perform (in fact, that has already performed).
+            if (newPhase == InputActionPhase.Performed &&
+                actionIndex != kInvalidIndex && !actionStates[actionIndex].isPerformed &&
+                actionStates[actionIndex].interactionIndex != trigger.interactionIndex)
+            {
+                // If the action was not already performed and we performed but we're not the interaction driving the action.
+                // We want to stay performed to make sure that if the interaction that is currently driving the action
+                // cancels, we get to perform the action. If we go back to waiting here, then the system can't tell
+                // that there's another interaction ready to perform (in fact, that has already performed).
             }
             else if (newPhase == InputActionPhase.Performed && phaseAfterPerformed != InputActionPhase.Waiting)
             {

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

@@ -320,25 +320,37 @@ public unsafe void AddActionMap(InputActionMap actionMap)
                             }
 
                             // Instantiate interactions.
-                            var interactionString = unresolvedBinding.effectiveInteractions;
-                            if (!string.IsNullOrEmpty(interactionString))
+                            if (isPartOfComposite)
                             {
-                                // Add interactions from binding.
-                                firstInteractionIndex = InstantiateWithParameters(InputInteraction.s_Interactions, interactionString,
-                                    ref interactions, ref totalInteractionCount, actionMap, ref unresolvedBinding);
-                                if (firstInteractionIndex != InputActionState.kInvalidIndex)
-                                    numInteractions = totalInteractionCount - firstInteractionIndex;
+                                // Composite's part use composite interactions
+                                if (currentCompositeBindingIndex != InputActionState.kInvalidIndex)
+                                {
+                                    firstInteractionIndex = bindingStatesPtr[currentCompositeBindingIndex].interactionStartIndex;
+                                    numInteractions = bindingStatesPtr[currentCompositeBindingIndex].interactionCount;
+                                }
                             }
-                            if (!string.IsNullOrEmpty(action.m_Interactions))
+                            else
                             {
-                                // Add interactions from action.
-                                var index = InstantiateWithParameters(InputInteraction.s_Interactions, action.m_Interactions,
-                                    ref interactions, ref totalInteractionCount, actionMap, ref unresolvedBinding);
-                                if (index != InputActionState.kInvalidIndex)
+                                var interactionString = unresolvedBinding.effectiveInteractions;
+                                if (!string.IsNullOrEmpty(interactionString))
+                                {
+                                    // Add interactions from binding.
+                                    firstInteractionIndex = InstantiateWithParameters(InputInteraction.s_Interactions, interactionString,
+                                        ref interactions, ref totalInteractionCount, actionMap, ref unresolvedBinding);
+                                    if (firstInteractionIndex != InputActionState.kInvalidIndex)
+                                        numInteractions = totalInteractionCount - firstInteractionIndex;
+                                }
+                                if (!string.IsNullOrEmpty(action.m_Interactions))
                                 {
-                                    if (firstInteractionIndex == InputActionState.kInvalidIndex)
-                                        firstInteractionIndex = index;
-                                    numInteractions += totalInteractionCount - index;
+                                    // Add interactions from action.
+                                    var index = InstantiateWithParameters(InputInteraction.s_Interactions, action.m_Interactions,
+                                        ref interactions, ref totalInteractionCount, actionMap, ref unresolvedBinding);
+                                    if (index != InputActionState.kInvalidIndex)
+                                    {
+                                        if (firstInteractionIndex == InputActionState.kInvalidIndex)
+                                            firstInteractionIndex = index;
+                                        numInteractions += totalInteractionCount - index;
+                                    }
                                 }
                             }