FIX: Removing device causing state monitor corruption (#977).

Author: Rene Damm

.yamato/upm-ci.yml

@@ -48,8 +48,10 @@ platforms:
     - npm install upm-ci-utils@stable -g --registry https://api.bintray.com/npm/unity/unity-npm
     - upm-ci package pack --package-path ./Packages/com.unity.inputsystem/
     - upm-ci package test --package-path ./Packages/com.unity.inputsystem/ -u {{ editor.version }}
-    - {% if platform.installscript %} {{ platform.installscript }} {{ editor.version }} {% endif %} 
-    - upm-ci~/tools/utr/utr --testproject $PWD --editor-location=.Editor --artifacts_path=upm-ci~/test-results/isolation-com.unity.inputsystem.tests --suite=playmode --api-profile=NET_4_6 --stdout-filter=minimal {% if platform.runtime %} --platform {{ platform.runtime }} {% endif %} {% if platform.scripting-backend %} --scripting-backend {{ platform.scripting-backend }} {% endif %} 
+    {% if platform.installscript %}
+    - {{ platform.installscript }} {{ editor.version }}
+    {% endif %}
+    - upm-ci~/tools/utr/utr --testproject . --editor-location=.Editor --artifacts_path=upm-ci~/test-results/isolation-com.unity.inputsystem.tests --suite=playmode --api-profile=NET_4_6 --stdout-filter=minimal {% if platform.runtime %} --platform {{ platform.runtime }} {% endif %} {% if platform.scripting-backend %} --scripting-backend {{ platform.scripting-backend }} {% endif %} 
   triggers:
     branches:
       only:

Assets/Tests/InputSystem/CoreTests_State.cs

@@ -743,6 +743,24 @@ public void State_CanRemoveStateChangeMonitorWithSpecificMonitorIndex()
         Assert.That(receivedMonitorIndex.Value, Is.EqualTo(kRightStick));
     }
 
+    [Test]
+    [Category("State")]
+    public void State_StateChangeMonitorsStayIntactWhenOtherDevicesAreRemoved()
+    {
+        InputSystem.AddDevice<Keyboard>(); // Noise.
+        var gamepad = InputSystem.AddDevice<Gamepad>();
+        var mouse = InputSystem.AddDevice<Mouse>();
+
+        var positionMonitorFired = false;
+        InputState.AddChangeMonitor(mouse.position, (control, d, arg3, arg4) => positionMonitorFired = true);
+
+        InputSystem.RemoveDevice(gamepad);
+
+        Set(mouse.position, new Vector2(123, 234));
+
+        Assert.That(positionMonitorFired);
+    }
+
     // For certain actions, we want to be able to tell whether a specific input arrives in time.
     // For example, we may want to only trigger an action if a specific button was released within
     // a certain amount of time. To support this, the system allows putting timeouts on individual

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -34,6 +34,8 @@ however, it has to be formatted properly to pass verification tests.
 
 - `InputUser` in combination with touchscreens no longer throws `InvalidOperationException` complaining about incorrect state format.
  * In a related change, `InputControlExtensions.GetStatePtrFromStateEvent` now works with touch events, too.
+- Removing a device no longer has the potential of corrupting state change monitors (and thus actions getting triggered) from other devices.
+  * This bug led to input being missed on a device once another device had been removed.
 
 ## [1.0.0-preview.3] - 2019-11-14
 

Packages/com.unity.inputsystem/InputSystem/InputManager.cs

@@ -235,7 +235,11 @@ public event LayoutChangeListener onLayoutChange
         ////TODO: introduce an alternative that consumes events in bulk
         public event EventListener onEvent
         {
-            add => m_EventListeners.AppendWithCapacity(value);
+            add
+            {
+                if (!m_EventListeners.ContainsReference(value))
+                    m_EventListeners.AppendWithCapacity(value);
+            }
             remove
             {
                 var index = m_EventListeners.IndexOf(value);
@@ -249,7 +253,8 @@ public event UpdateListener onBeforeUpdate
             add
             {
                 InstallBeforeUpdateHookIfNecessary();
-                m_BeforeUpdateListeners.AppendWithCapacity(value);
+                if (!m_BeforeUpdateListeners.ContainsReference(value))
+                    m_BeforeUpdateListeners.AppendWithCapacity(value);
             }
             remove
             {
@@ -261,7 +266,11 @@ public event UpdateListener onBeforeUpdate
 
         public event UpdateListener onAfterUpdate
         {
-            add => m_AfterUpdateListeners.AppendWithCapacity(value);
+            add
+            {
+                if (!m_AfterUpdateListeners.ContainsReference(value))
+                    m_AfterUpdateListeners.AppendWithCapacity(value);
+            }
             remove
             {
                 var index = m_AfterUpdateListeners.IndexOf(value);
@@ -272,7 +281,11 @@ public event UpdateListener onAfterUpdate
 
         public event Action onSettingsChange
         {
-            add => m_SettingsChangedListeners.AppendWithCapacity(value);
+            add
+            {
+                if (!m_SettingsChangedListeners.ContainsReference(value))
+                    m_SettingsChangedListeners.AppendWithCapacity(value);
+            }
             remove
             {
                 var index = m_SettingsChangedListeners.IndexOf(value);
@@ -1167,7 +1180,14 @@ public void RemoveDevice(InputDevice device, bool keepOnListOfAvailableDevices =
             // Remove from device array.
             var deviceIndex = device.m_DeviceIndex;
             var deviceId = device.deviceId;
+            if (deviceIndex < m_StateChangeMonitors.LengthSafe())
+            {
+                // m_StateChangeMonitors mirrors layout of m_Devices.
+                var count = m_DevicesCount;
+                ArrayHelpers.EraseAtWithCapacity(m_StateChangeMonitors, ref count, deviceIndex);
+            }
             ArrayHelpers.EraseAtWithCapacity(m_Devices, ref m_DevicesCount, deviceIndex);
+
             m_DevicesById.Remove(deviceId);
 
             if (m_Devices != null)
@@ -1803,6 +1823,7 @@ public void Clear()
             {
                 // We don't actually release memory we've potentially allocated but rather just reset
                 // our count to zero.
+                listeners.Clear(count);
                 signalled.SetLength(0);
             }
         }

Packages/com.unity.inputsystem/InputSystem/Utilities/ArrayHelpers.cs

@@ -708,8 +708,6 @@ public static void MoveSlice<TValue>(TValue[] array, int sourceIndex, int destin
             if (sourceIndex > destinationIndex)
                 Swap(ref sourceIndex, ref destinationIndex);
 
-            var length = array.Length;
-
             while (destinationIndex != sourceIndex)
             {
                 // Swap source and destination slice. Afterwards, the source slice is the right, final