FIX: IndexOutOfRangeException when enabling InputActionMap. (ISXB-1767) (#2296)

Co-authored-by: Darren-Kelly-Unity <118264423+Darren-Kelly-Unity@users.noreply.github.com>
Co-authored-by: Darren Kelly <darren.kelly@unity3d.com>

Author: ekcoh

Assets/Tests/InputSystem/CoreTests_Actions.cs

@@ -12581,4 +12581,67 @@ public void Actions_ActionMapDisabledDuringOnAfterSerialization()
         Assert.That(map.enabled, Is.True);
         Assert.That(map.FindAction("MyAction", true).enabled, Is.True);
     }
+
+    // Verifies that a multi-control-scheme project with a disconnected device that attempts to enable the associated
+    // actions via an action cancellation callback doesn't result in IndexOutOfBoundsException, and that the binding
+    // will be restored upon device reconnect. We use the same bindings as the associated ISXB issue to make sure
+    // we test the exact same scenario.
+    [Test, Description("https://jira.unity3d.com/browse/ISXB-1767")]
+    public void Actions_CanHandleDeviceDisconnectWithControlSchemesAndReconnect()
+    {
+        int started = 0;
+        int performed = 0;
+        int canceled = 0;
+
+        // Create an input action asset object.
+        var actions = ScriptableObject.CreateInstance<InputActionAsset>();
+
+        // These control schemes are critical to this test. Without them the exception won't happen.
+        var keyboardScheme = actions.AddControlScheme("Keyboard").WithRequiredDevice<Keyboard>();
+        var gamepadScheme = actions.AddControlScheme("Gamepad").WithRequiredDevice<Gamepad>();
+
+        // Create a single action map since its sufficient for the scenario.
+        var map = actions.AddActionMap("map");
+
+        var action = map.AddAction(name: "Toggle", InputActionType.Button);
+        action.AddBinding("<Gamepad>/leftTrigger");
+        action.started += context => ++ started;
+        action.performed += context => ++ performed;
+        action.canceled += (context) =>
+        {
+            // In reported issue, map state is changed from cancellation callback.
+            map.Disable();
+            map.Enable();
+
+            // This is not part of the bug reported in ISXB-1767 but extends the test coverage since
+            // it makes sure Disable() is safe after logically skipped Enable().
+            map.Disable();
+            map.Enable();
+
+            ++canceled;
+        };
+
+        // Add a keyboard and a gamepad.
+        var keyboard = InputSystem.AddDevice<Keyboard>();
+        var gamepad = InputSystem.AddDevice<Gamepad>();
+
+        // Enable the map, press (and hold) the left trigger and assert action is firing.
+        map.Enable();
+        Press(gamepad.leftTrigger, queueEventOnly: true);
+        InputSystem.Update();
+        Assert.That(started, Is.EqualTo(1));
+
+        // Remove the gamepad device. This is consistent with event queue based removal (not kept on list).
+        InputSystem.RemoveDevice(gamepad);
+        InputSystem.Update();
+        Assert.That(canceled, Is.EqualTo(1));
+
+        // Reconnect the disconnected gamepad
+        InputSystem.AddDevice(gamepad);
+
+        // Interact again
+        Press(gamepad.leftTrigger, queueEventOnly: true);
+        InputSystem.Update();
+        Assert.That(started, Is.EqualTo(2));
+    }
 }

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -21,6 +21,7 @@ however, it has to be formatted properly to pass verification tests.
 - Align title font size with toolbar style in `Input Action` window.
 - Updated Action Properties headers to use colors consistent with GameObject component headers.
 - Fixed misaligned Virtual Cursor when changing resolution [ISXB-1119](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1119)
+- Fixed an issue where `IndexOutOfRangeException` was thrown from `InputManagerStateMonitors.AddStateChangeMonitor` when attempting to enable an action map from within an `InputAction.cancel` callback when using control schemes. (ISXB-1767).
 
 ### Added
 

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

@@ -1153,6 +1153,13 @@ private void EnableControls(int mapIndex, int controlStartIndex, int numControls
                 if (IsControlEnabled(controlIndex))
                     continue;
 
+                // We might end up here if an action map is enabled from e.g. an event processing callback such as
+                // InputAction.cancel event handler (ISXB-1767). In this case we must skip controls associated with
+                // a device that is not connected to the system (Have deviceIndex < 0). We check this here to not
+                // cause side effects if aborting later in the call-chain.
+                if (!controls[controlIndex].device.added)
+                    continue;
+
                 var bindingIndex = controlIndexToBindingIndex[controlIndex];
                 var mapControlAndBindingIndex = ToCombinedMapAndControlAndBindingIndex(mapIndex, controlIndex, bindingIndex);
 
@@ -1474,8 +1481,18 @@ private void ProcessControlStateChange(int mapIndex, int controlIndex, int bindi
                     if (m_OnBeforeUpdateHooked)
                         bindingStatePtr->initialStateCheckPending = false;
 
-                    // Store magnitude. We do this once and then only read it from here.
                     var control = controls[controlIndex];
+
+                    // We might end up here if an action map is enabled from e.g. an event processing callback such as
+                    // InputAction.cancel event handler (ISXB-1767). In this case we must skip controls associated with
+                    // a device that is not connected to the system (Have deviceIndex < 0). We check this here to not
+                    // cause side effects if aborting later in the call-chain.
+                    if (control == null || !controls[controlIndex].device.added)
+                    {
+                        return;
+                    }
+
+                    // Store magnitude. We do this once and then only read it from here.
                     trigger.magnitude = control.CheckStateIsAtDefault() ? 0f : control.magnitude;
                     controlMagnitudes[controlIndex] = trigger.magnitude;