diff --git a/Assets/Tests/InputSystem/CoreTests_Actions.cs b/Assets/Tests/InputSystem/CoreTests_Actions.cs index 3106ccf80f..ba26bc52b6 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions.cs +++ b/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(); + var action = new InputAction(); + action.AddCompositeBinding("Dpad", interactions: "tap,hold(duration=2)") + .With("Up", "/w") + .With("Down", "/s") + .With("Left", "/a") + .With("Right", "/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() diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 93e28056e0..f285ca5313 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -10,6 +10,9 @@ 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) + ### Changed - Renamed editor Resources directories to PackageResources to fix package validation warnings. diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs index ae66343dfd..3ba1503d49 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs +++ b/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) { diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputBindingResolver.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputBindingResolver.cs index f10eb7c974..8bfc195e9e 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputBindingResolver.cs +++ b/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; + } } }