CHANGE: Enforce started/performed/canceled cycle on button and value actions (#1053).

Author: Rene Damm

Assets/Tests/InputSystem/CoreTests_Actions.cs

@@ -1507,10 +1507,7 @@ public void Actions_WithMultipleBoundControls_CanHandleInteractionsThatTriggerOn
 
             Release(gamepad.buttonNorth);
 
-            var actions = trace.ToArray();
-            Assert.That(actions, Has.Length.EqualTo(1));
-            Assert.That(actions[0].phase, Is.EqualTo(InputActionPhase.Performed));
-            Assert.That(actions[0].control, Is.SameAs(gamepad.buttonNorth));
+            Assert.That(trace, Started(action, gamepad.buttonNorth).AndThen(Performed(action, gamepad.buttonNorth)).AndThen(Canceled(action, gamepad.buttonNorth)));
 
             trace.Clear();
 
@@ -1522,10 +1519,7 @@ public void Actions_WithMultipleBoundControls_CanHandleInteractionsThatTriggerOn
 
             Release(gamepad.buttonNorth);
 
-            actions = trace.ToArray();
-            Assert.That(actions, Has.Length.EqualTo(1));
-            Assert.That(actions[0].phase, Is.EqualTo(InputActionPhase.Performed));
-            Assert.That(actions[0].control, Is.SameAs(gamepad.buttonNorth));
+            Assert.That(trace, Started(action, gamepad.buttonNorth).AndThen(Performed(action, gamepad.buttonNorth)).AndThen(Canceled(action, gamepad.buttonNorth)));
 
             trace.Clear();
 
@@ -1536,10 +1530,7 @@ public void Actions_WithMultipleBoundControls_CanHandleInteractionsThatTriggerOn
 
             Release(keyboard.aKey);
 
-            actions = trace.ToArray();
-            Assert.That(actions, Has.Length.EqualTo(1));
-            Assert.That(actions[0].phase, Is.EqualTo(InputActionPhase.Performed));
-            Assert.That(actions[0].control, Is.SameAs(keyboard.aKey));
+            Assert.That(trace, Started(action, keyboard.aKey).AndThen(Performed(action, keyboard.aKey)).AndThen(Canceled(action, keyboard.aKey)));
 
             trace.Clear();
 
@@ -1558,19 +1549,13 @@ public void Actions_WithMultipleBoundControls_CanHandleInteractionsThatTriggerOn
 
             Release(keyboard.aKey);
 
-            actions = trace.ToArray();
-            Assert.That(actions, Has.Length.EqualTo(1));
-            Assert.That(actions[0].phase, Is.EqualTo(InputActionPhase.Performed));
-            Assert.That(actions[0].control, Is.SameAs(keyboard.aKey));
+            Assert.That(trace, Started(action, keyboard.aKey).AndThen(Performed(action, keyboard.aKey)).AndThen(Canceled(action, keyboard.aKey)));
 
             trace.Clear();
 
             Release(gamepad.buttonNorth);
 
-            actions = trace.ToArray();
-            Assert.That(actions, Has.Length.EqualTo(1));
-            Assert.That(actions[0].phase, Is.EqualTo(InputActionPhase.Performed));
-            Assert.That(actions[0].control, Is.SameAs(gamepad.buttonNorth));
+            Assert.That(trace, Started(action, gamepad.buttonNorth).AndThen(Performed(action, gamepad.buttonNorth)).AndThen(Canceled(action, gamepad.buttonNorth)));
         }
     }
 

Assets/Tests/InputSystem/CoreTests_Actions_Interactions.cs

@@ -4,10 +4,90 @@
 using UnityEngine.InputSystem.Interactions;
 using UnityEngine.InputSystem.LowLevel;
 using UnityEngine.InputSystem.Utilities;
+using UnityEngine.InputSystem.XR;
 using UnityEngine.Scripting;
 
 internal partial class CoreTests
 {
+    [Preserve]
+    class InteractionThatOnlyPerforms : IInputInteraction<float>
+    {
+        public bool stayPerformed;
+
+        public void Process(ref InputInteractionContext context)
+        {
+            if (context.ControlIsActuated())
+            {
+                if (stayPerformed)
+                    context.PerformedAndStayPerformed();
+                else
+                    context.Performed();
+            }
+        }
+
+        public void Reset()
+        {
+        }
+    }
+
+    [Test]
+    [Category("Actions")]
+    public void Actions_StartedAndCanceledAreEnforcedImplicitly()
+    {
+        var gamepad = InputSystem.AddDevice<Gamepad>();
+
+        InputSystem.RegisterInteraction<InteractionThatOnlyPerforms>();
+
+        var action1 = new InputAction(name: "action1", type: InputActionType.Button, binding: "<Gamepad>/buttonSouth", interactions: "interactionThatOnlyPerforms(stayPerformed=true)");
+        var action2 = new InputAction(name: "action2", type: InputActionType.Button, binding: "<Gamepad>/buttonSouth", interactions: "interactionThatOnlyPerforms(stayPerformed=false)");
+        var action3 = new InputAction(name: "action3", type: InputActionType.Button, binding: "<Gamepad>/buttonSouth");
+        var action4 = new InputAction(name: "action4", type: InputActionType.Value, binding: "<Gamepad>/buttonSouth");
+
+        // Pass-Through is special (as always).
+        var action5 = new InputAction(name: "action5", type: InputActionType.PassThrough, binding: "<Gamepad>/buttonSouth");
+        var action6 = new InputAction(name: "action6", type: InputActionType.PassThrough, binding: "<Gamepad>/buttonSouth", interactions: "press");
+
+        action1.Enable();
+        action2.Enable();
+        action3.Enable();
+        action4.Enable();
+        action5.Enable();
+        action6.Enable();
+
+        using (var trace1 = new InputActionTrace(action1))
+        using (var trace2 = new InputActionTrace(action2))
+        using (var trace3 = new InputActionTrace(action3))
+        using (var trace4 = new InputActionTrace(action4))
+        using (var trace5 = new InputActionTrace(action5))
+        using (var trace6 = new InputActionTrace(action6))
+        {
+            Press(gamepad.buttonSouth);
+
+            Assert.That(trace1, Started(action1).AndThen(Performed(action1)));
+            Assert.That(trace2, Started(action2).AndThen(Performed(action2)).AndThen(Canceled(action2)));
+            Assert.That(trace3, Started(action3).AndThen(Performed(action3)));
+            Assert.That(trace4, Started(action4).AndThen(Performed(action4)));
+            Assert.That(trace5, Performed(action5));
+            Assert.That(trace6, Started(action6).AndThen(Performed(action6)));
+
+            trace1.Clear();
+            trace2.Clear();
+            trace3.Clear();
+            trace4.Clear();
+            trace5.Clear();
+            trace6.Clear();
+
+            Release(gamepad.buttonSouth);
+
+            Assert.That(trace1, Is.Empty);
+            Assert.That(trace2, Is.Empty);
+            Assert.That(trace3, Canceled(action3));
+            Assert.That(trace4, Canceled(action4));
+            Assert.That(trace5, Performed(action5)); // Any value change performs.
+            Assert.That(trace6, Canceled(action6));
+        }
+    }
+
     [Test]
     [Category("Actions")]
     public void Actions_WhenTransitionFromOneInteractionToNext_GetCallbacks()
@@ -49,7 +129,7 @@ public void Actions_CanPerformPressInteraction()
         InputSystem.AddDevice<Keyboard>();
 
         // Test all three press behaviors concurrently.
-        var pressOnlyAction = new InputAction("PressOnly", binding: "<Gamepad>/buttonSouth", interactions: "press");
+        var pressOnlyAction = new InputAction("PressOnly", binding: "<Gamepad>/buttonSouth", interactions: "press(behavior=0)");
         pressOnlyAction.AddBinding("<Keyboard>/a");
         var releaseOnlyAction = new InputAction("ReleaseOnly", binding: "<Gamepad>/buttonSouth", interactions: "press(behavior=1)");
         releaseOnlyAction.AddBinding("<Keyboard>/s");
@@ -60,83 +140,50 @@ public void Actions_CanPerformPressInteraction()
         releaseOnlyAction.Enable();
         pressAndReleaseAction.Enable();
 
-        using (var trace = new InputActionTrace())
+        using (var pressOnly = new InputActionTrace(pressOnlyAction))
+        using (var releaseOnly = new InputActionTrace(releaseOnlyAction))
+        using (var pressAndRelease = new InputActionTrace(pressAndReleaseAction))
         {
-            trace.SubscribeToAll();
-
             runtime.currentTime = 1;
             Press(gamepad.buttonSouth);
 
-            var actions = trace.ToArray();
-            Assert.That(actions, Has.Length.EqualTo(5));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(pressOnlyAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Started).And.With.Property("duration")
-                    .EqualTo(0));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(pressOnlyAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Performed).And.With.Property("duration")
-                    .EqualTo(0));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(pressAndReleaseAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Started).And.With.Property("duration")
-                    .EqualTo(0));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(pressAndReleaseAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Performed).And.With.Property("duration")
-                    .EqualTo(0));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(releaseOnlyAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Started).And.With.Property("duration")
-                    .EqualTo(0));
+            Assert.That(pressOnly,
+                Started<PressInteraction>(pressOnlyAction, gamepad.buttonSouth, value: 1.0, time: 1)
+                    .AndThen(Performed<PressInteraction>(pressOnlyAction, gamepad.buttonSouth, time: 1, duration: 0, value: 1.0)));
+            Assert.That(releaseOnly, Started<PressInteraction>(releaseOnlyAction, gamepad.buttonSouth, time: 1, value: 1.0));
+            Assert.That(pressAndRelease,
+                Started<PressInteraction>(pressAndReleaseAction, gamepad.buttonSouth, time: 1, value: 1.0)
+                    .AndThen(Performed<PressInteraction>(pressAndReleaseAction, gamepad.buttonSouth, time: 1, duration: 0, value: 1.0)));
 
-            trace.Clear();
+            pressOnly.Clear();
+            releaseOnly.Clear();
+            pressAndRelease.Clear();
 
             runtime.currentTime = 2;
             Release(gamepad.buttonSouth);
 
-            actions = trace.ToArray();
-            Assert.That(actions, Has.Length.EqualTo(3));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(releaseOnlyAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Performed).And.With.Property("duration")
-                    .EqualTo(1));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(pressAndReleaseAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Started).And.With.Property("duration")
-                    .EqualTo(0));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(pressAndReleaseAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Performed).And.With.Property("duration")
-                    .EqualTo(0));
+            Assert.That(pressOnly, Canceled<PressInteraction>(pressOnlyAction, gamepad.buttonSouth, value: 0.0, time: 2, duration: 1));
+            Assert.That(releaseOnly,
+                Performed<PressInteraction>(releaseOnlyAction, gamepad.buttonSouth, value: 0.0, time: 2, duration: 1)
+                    .AndThen(Canceled<PressInteraction>(releaseOnlyAction, gamepad.buttonSouth, value: 0.0, time: 2, duration: 1)));
+            Assert.That(pressAndRelease,
+                Performed<PressInteraction>(pressAndReleaseAction, gamepad.buttonSouth, value: 0.0, time: 2, duration: 1)
+                    .AndThen(Canceled<PressInteraction>(pressAndReleaseAction, gamepad.buttonSouth, value: 0.0, time: 2, duration: 1)));
 
-            trace.Clear();
+            pressOnly.Clear();
+            releaseOnly.Clear();
+            pressAndRelease.Clear();
 
             runtime.currentTime = 5;
             Press(gamepad.buttonSouth);
 
-            actions = trace.ToArray();
-            Assert.That(actions, Has.Length.EqualTo(5));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(pressOnlyAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Started).And.With.Property("duration")
-                    .EqualTo(0));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(pressOnlyAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Performed).And.With.Property("duration")
-                    .EqualTo(0));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(pressAndReleaseAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Started).And.With.Property("duration")
-                    .EqualTo(0));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(pressAndReleaseAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Performed).And.With.Property("duration")
-                    .EqualTo(0));
-            Assert.That(actions,
-                Has.Exactly(1).With.Property("action").SameAs(releaseOnlyAction).And.With.Property("phase")
-                    .EqualTo(InputActionPhase.Started).And.With.Property("duration")
-                    .EqualTo(0));
+            Assert.That(pressOnly,
+                Started<PressInteraction>(pressOnlyAction, gamepad.buttonSouth, value: 1.0, time: 5)
+                    .AndThen(Performed<PressInteraction>(pressOnlyAction, gamepad.buttonSouth, time: 5, duration: 0, value: 1.0)));
+            Assert.That(releaseOnly, Started<PressInteraction>(releaseOnlyAction, gamepad.buttonSouth, time: 5, value: 1.0));
+            Assert.That(pressAndRelease,
+                Started<PressInteraction>(pressAndReleaseAction, gamepad.buttonSouth, time: 5, value: 1.0)
+                    .AndThen(Performed<PressInteraction>(pressAndReleaseAction, gamepad.buttonSouth, time: 5, duration: 0, value: 1.0)));
         }
     }
 
@@ -513,15 +560,15 @@ public void Actions_ValueIsDefaultWhenActionIsCanceled()
         var gamepad = InputSystem.AddDevice<Gamepad>();
 
         var action = new InputAction("test", binding: "<Gamepad>/leftTrigger", interactions: "CancelingTest");
-        int performedCount = 0;
+        var performedCount = 0;
         float performedValue = -1;
         action.performed += ctx =>
         {
             performedValue = ctx.ReadValue<float>();
             performedCount++;
         };
 
-        int canceledCount = 0;
+        var canceledCount = 0;
         float canceledValue = -1;
         action.canceled += ctx =>
         {

Assets/Tests/InputSystem/CoreTests_Devices.cs

@@ -24,6 +24,8 @@
 using Vector2 = UnityEngine.Vector2;
 using Vector3 = UnityEngine.Vector3;
 
+////TODO: set display name from product string only for layout builders (and give them control); for other layouts, let the layout dictate the display name
+
 ////TODO: test that device re-creation doesn't lose flags and such
 
 partial class CoreTests

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -11,6 +11,21 @@ however, it has to be formatted properly to pass verification tests.
 
 ### Changed
 
+- We've changed the rules that govern how action phases have to progress:
+  * __This is a breaking change!__
+    - The primary effect is additional callbacks getting triggered.
+  * __Before__:
+    - There were no enforced rules about how an action would go through `InputAction.started`, `InputAction.performed`, and `InputAction.canceled`. Which of the callbacks were triggered and in what order depended on a number of factors, the biggest influencer of which were the different interactions that could be applied to actions (like `Press` or `Hold`).
+    - This made for unpredictable and frequently surprising results. In addition, it led to bugs where, for [example](https://issuetracker.unity3d.com/issues/input-system-ui-becomes-unresponsive-after-the-first-ui-button-press), adding a `Press` interaction to the `Click` action of `InputSystemUIInputModule` would cause the click state to get stuck because the click action would never cancel.
+  * __Now__:
+    - The system will now *always* trigger `InputAction.started` first. If this is not done explicitly, it happens implicitly.
+    - Likewise, the system will now *always* trigger `InputAction.canceled` before going back to waiting state. Like with `InputAction.started`, if this isn't done explicitly, it will happen implicitly. This implies that `InputAction.canceled` no longer signifies an action getting aborted because it stopped after it started but before it performed. It now simply means "the action has ended" whether it actually got performed or not.
+    - In-between `InputAction.started` and `InputAction.canceled`, `InputAction.performed` may be triggered arbitrary many times (including not at all).
+  * While late in the cycle for 1.0, we've opted to make this change now in order to fix a range of bugs and problems we've observed that people encountered because of the previous behavior of the system.
+- Related to the change above, the behavior of `PressInteraction` has been tweaked and now is the following:
+  * `Press Only`: Starts and immediately performs when pressed, then stays performed and cancels when button is released.
+  * `Release Only`: Starts when button is pressed and then performs and immediately cancels when the button is released.
+  * `Press And Release`: Starts and immediately performs when button is pressed, then stays performed and performs again and immediately cancels when button is released.
 - `Vector2Composite` now has a `mode` parameter which can be used to choose between `DigitalNormalized` (the default), `Digital` (same as `DigitalNormalized` but does not normalize the resulting vector), and `Analog` (uses float input values as is).
   * `Vector2Composite.normalize` has been deprecated. Note that it will not work together with `Analog`. The parameter will be removed in the future.
 

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

@@ -314,9 +314,21 @@ public static string GetDisplayName(string interaction)
             if (interactionType == null)
                 return interaction;
 
+            return GetDisplayName(interactionType);
+        }
+
+        public static string GetDisplayName(Type interactionType)
+        {
+            if (interactionType == null)
+                throw new ArgumentNullException(nameof(interactionType));
+
             var displayNameAttribute = interactionType.GetCustomAttribute<DisplayNameAttribute>();
             if (displayNameAttribute == null)
-                return interaction;
+            {
+                if (interactionType.Name.EndsWith("Interaction"))
+                    return interactionType.Name.Substring(0, interactionType.Name.Length - "Interaction".Length);
+                return interactionType.Name;
+            }
 
             return displayNameAttribute.DisplayName;
         }