FIX: Disconnecting a device while an input is active should not throw (#1427)

Author: andrew-oc

Assets/Tests/InputSystem/CoreTests_Actions_Interactions.cs

@@ -1,5 +1,6 @@
 using System.Linq;
 using NUnit.Framework;
+using NUnit.Framework.Constraints;
 using UnityEngine;
 using UnityEngine.InputSystem;
 using UnityEngine.InputSystem.Interactions;
@@ -793,4 +794,21 @@ public void Actions_ValueIsDefaultWhenActionIsCanceled()
         Assert.That(canceledCount, Is.EqualTo(1));
         Assert.That(canceledValue, Is.EqualTo(0.0));
     }
+
+    // https://fogbugz.unity3d.com/f/cases/1354098/
+    [Test]
+    [Category("Actions")]
+    public void Actions_DoesNotThrowWhenDeviceIsDisconnectedWhileControlIsPressed()
+    {
+        var gamepad = InputSystem.AddDevice<Gamepad>();
+
+        var action = new InputAction("Action",
+            binding: "<Gamepad>/buttonSouth",
+            interactions: "Press,Press"); // this bug occurs when there are multiple interactions on a binding
+        action.Enable();
+
+        Press(gamepad.buttonSouth);
+
+        Assert.That(() => InputSystem.RemoveDevice(gamepad), Throws.Nothing);
+    }
 }

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -26,6 +26,7 @@ however, it has to be formatted properly to pass verification tests.
 - Generic gamepad short display button names where incorrectly mapped on Switch (`A` instead of `B`, etc).
 - Fixed an issue where resetting an action via `InputAction.Reset()` while being in disabled state would prevent the action from being enabled again. ([case 1370732](https://issuetracker.unity3d.com/product/unity/issues/guid/1370732/)).
 - Fixed "Default constructor not found for type UnityEngine.InputSystem.iOS.LowLevel.iOSStepCounter" any other potential exceptions due to classes, methods, fields and properties being stripped when managed stripping setting set to medium or high ([case 1368761](https://issuetracker.unity3d.com/issues/ios-new-input-system-iosstepcounter-crash-on-launch-with-managed-stripping)).
+- Fixed an issue where InvalidOperationExceptions are thrown if an input for an action with multiple interactions is held  while disconnecting the device([case 1354098](https://issuetracker.unity3d.com/issues/input-system-errors-are-thrown-when-disconnecting-controller-while-holding-a-button-with-press-and-release-set-up-separately)).
 - Fixed `action.ReadValue` and others returning invalid data when used from `FixedUpdate` or early update when running in play mode in the editor ([case 1368559](https://issuetracker.unity3d.com/issues/enter-key-is-not-registered-when-using-waspressedthisframe-with-input-system-1-dot-1-1) [case 1367556](https://issuetracker.unity3d.com/issues/input-action-readvalue-always-returns-zero-when-called-from-fixedupdate) [case 1372830](https://issuetracker.unity3d.com/issues/querying-inputs-before-preupdate-dot-newinputupdate-returns-invalid-data-when-running-in-play-mode-in-editor)).
 
 ### Added

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

@@ -1690,6 +1690,9 @@ private void StopTimeout(int mapIndex, int controlIndex, int bindingIndex, int i
         /// <see cref="InputActionPhase.Waiting"/> (default), <see cref="InputActionPhase.Started"/> (if the action is supposed
         /// to be oscillate between started and performed), or <see cref="InputActionPhase.Performed"/> (if the action is
         /// supposed to perform over and over again until canceled).</param>
+        /// <param name="processNextInteractionsOnCancel">Indicates if the system should try and change the phase of other
+        /// interactions on the same action that are already started or performed after cancelling this interaction. This should be
+        /// false when resetting interactions.</param>
         /// <remarks>
         /// Multiple interactions on the same binding can be started concurrently but the
         /// first interaction that starts will get to drive an action until it either cancels
@@ -1705,7 +1708,7 @@ private void StopTimeout(int mapIndex, int controlIndex, int bindingIndex, int i
         /// long and the SlowTapInteraction will get to drive the action next).
         /// </remarks>
         internal void ChangePhaseOfInteraction(InputActionPhase newPhase, ref TriggerState trigger,
-            InputActionPhase phaseAfterPerformed = InputActionPhase.Waiting)
+            InputActionPhase phaseAfterPerformed = InputActionPhase.Waiting, bool processNextInteractionsOnCancel = true)
         {
             var interactionIndex = trigger.interactionIndex;
             var bindingIndex = trigger.bindingIndex;
@@ -1753,6 +1756,9 @@ internal void ChangePhaseOfInteraction(InputActionPhase newPhase, ref TriggerSta
                     if (!ChangePhaseOfAction(newPhase, ref trigger))
                         return;
 
+                    if (processNextInteractionsOnCancel == false)
+                        return;
+
                     var interactionStartIndex = bindingStates[bindingIndex].interactionStartIndex;
                     var numInteractions = bindingStates[bindingIndex].interactionCount;
                     for (var i = 0; i < numInteractions; ++i)
@@ -1942,7 +1948,13 @@ private void ChangePhaseOfActionInternal(int actionIndex, TriggerState* actionSt
             // We need to make sure here that any HaveMagnitude flag we may be carrying over from actionState
             // is handled correctly (case 1239551).
             newState.flags = actionState->flags; // Preserve flags.
-            newState.magnitude = trigger.haveMagnitude ? trigger.magnitude : ComputeMagnitude(trigger.bindingIndex, trigger.controlIndex);
+            if (newPhase != InputActionPhase.Canceled)
+                newState.magnitude = trigger.haveMagnitude
+                    ? trigger.magnitude
+                    : ComputeMagnitude(trigger.bindingIndex, trigger.controlIndex);
+            else
+                newState.magnitude = 0;
+
             newState.phase = newPhase;
             if (newPhase == InputActionPhase.Performed)
             {
@@ -2145,7 +2157,7 @@ private void ResetInteractionStateAndCancelIfNecessary(int mapIndex, int binding
                 {
                     case InputActionPhase.Started:
                     case InputActionPhase.Performed:
-                        ChangePhaseOfInteraction(InputActionPhase.Canceled, ref actionStates[actionIndex]);
+                        ChangePhaseOfInteraction(InputActionPhase.Canceled, ref actionStates[actionIndex], processNextInteractionsOnCancel: false);
                         break;
                 }
             }