Fix attempt for ISXB-1066 where we still process a hold interaction despite having a more dominant interaction. Test case included.

Author: adrian-koretski-unity3d

Assets/Tests/InputSystem/CoreTests_Actions.cs

@@ -58,6 +58,113 @@ public void Settings_ShouldStoreSettingsAndFeatureFlags(string featureName)
         }
     }
 
+    [Test]
+    [Category("Actions")]
+    public void Actions_WithMultipleBindingsAndMultipleInteractions_Works()
+    {
+        InputSystem.settings.defaultButtonPressPoint = 0.5f;
+
+        var keyboard = InputSystem.AddDevice<Keyboard>();
+
+        var action = new InputAction(interactions: "tap,hold(duration=2)");
+        action.AddBinding("<Keyboard>/w");
+        action.AddBinding("<Keyboard>/a");
+        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 = 0.01;
+        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;
+
+        // Should be no other remaining events
+        currentTime = 10;
+        InputSystem.Update();
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.Null);
+
+
+
+        // PressRelease AW trigger a tap
+        currentTime = 11;
+        InputSystem.QueueStateEvent(keyboard, new KeyboardState(Key.A, Key.W));
+        InputSystem.Update();
+
+        // nothing triggered
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.Null);
+
+        currentTime = 11.01;
+        InputSystem.QueueStateEvent(keyboard, new KeyboardState(Key.A));
+        InputSystem.Update();
+
+        currentTime = 11.02;
+        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;
+
+        // Should be no other remaining events
+        currentTime = 20;
+        InputSystem.Update();
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.Null);
+
+        // PressRelease AW trigger a tap
+        currentTime = 21;
+        InputSystem.QueueStateEvent(keyboard, new KeyboardState(Key.A, Key.W));
+        InputSystem.Update();
+
+        // nothing triggered
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.Null);
+
+        currentTime = 21.01;
+        InputSystem.QueueStateEvent(keyboard, new KeyboardState(Key.W));
+        InputSystem.Update();
+
+        currentTime = 21.02;
+        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;
+
+        // Should be no other remaining events
+        currentTime = 30;
+        InputSystem.Update();
+        Assert.That(canceledInteraction, Is.Null);
+        Assert.That(performedInteraction, Is.Null);
+    }
+
     [Test]
     [Category("Actions")]
     public void Actions_WhenShortcutsDisabled_AllConflictingActionsTrigger()

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

@@ -10,6 +10,7 @@
 using UnityEngine.InputSystem.Utilities;
 
 using ProfilerMarker = Unity.Profiling.ProfilerMarker;
+using UnityEngine.InputSystem.Interactions;
 
 ////TODO: now that we can bind to controls by display name, we need to re-resolve controls when those change (e.g. when the keyboard layout changes)
 
@@ -1510,6 +1511,8 @@ private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindi
                         }
                     }
 
+                    var previousTrigger = trigger;
+                    var previousBindingStatePtr = bindingStatePtr;
                     // 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
@@ -1528,6 +1531,11 @@ private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindi
                     if (interactionCount > 0 && !bindingStatePtr->isPartOfComposite)
                     {
                         ProcessInteractions(ref trigger, bindingStatePtr->interactionStartIndex, interactionCount);
+                        // We still need to process hold interactions. If we don't, then we risk having a danling hold which triggers after all buttons are unpressed. 
+                        if (previousBindingStatePtr != bindingStatePtr)
+                        {
+                            ProcessHoldInteractions(ref previousTrigger, previousBindingStatePtr->interactionStartIndex, previousBindingStatePtr->interactionCount);
+                        }
                     }
                     else if (!haveInteractionsOnComposite && !isConflictingInput)
                     {
@@ -2048,6 +2056,32 @@ private void ProcessInteractions(ref TriggerState trigger, int interactionStartI
             }
         }
 
+        private void ProcessHoldInteractions(ref TriggerState trigger, int interactionStartIndex, int interactionCount)
+        {
+            var context = new InputInteractionContext
+            {
+                m_State = this,
+                m_TriggerState = trigger
+            };
+
+            for (var i = 0; i < interactionCount; ++i)
+            {
+                var index = interactionStartIndex + i;
+                var state = interactionStates[index];
+                var interaction = interactions[index];
+                if (interaction is not HoldInteraction)
+                {
+                    continue;
+                }
+
+                context.m_TriggerState.phase = state.phase;
+                context.m_TriggerState.startTime = state.startTime;
+                context.m_TriggerState.interactionIndex = index;
+
+                interaction.Process(ref context);
+            }
+        }
+
         private void ProcessTimeout(double time, int mapIndex, int controlIndex, int bindingIndex, int interactionIndex)
         {
             Debug.Assert(controlIndex >= 0 && controlIndex < totalControlCount, "Control index out of range");