From 5374a59d4d99c473e56c15ebfee89c43540f57df Mon Sep 17 00:00:00 2001 From: Adrian Koretski Date: Wed, 20 Nov 2024 15:39:20 -0500 Subject: [PATCH] Fix attempt for ISXB-1066 where we still process a hold interaction despite having a more dominant interaction. Test case included. --- Assets/Tests/InputSystem/CoreTests_Actions.cs | 107 ++++++++++++++++++ .../InputSystem/Actions/InputActionState.cs | 34 ++++++ 2 files changed, 141 insertions(+) diff --git a/Assets/Tests/InputSystem/CoreTests_Actions.cs b/Assets/Tests/InputSystem/CoreTests_Actions.cs index 0aa3d09fbe..8c4af7d972 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions.cs +++ b/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(); + + var action = new InputAction(interactions: "tap,hold(duration=2)"); + action.AddBinding("/w"); + action.AddBinding("/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() diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs index 79cfa1b8cb..d6ce96b29a 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs +++ b/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");