FIX: Don't pass events for null devices (for devices which have not b… (#805)

Author: jechter

Assets/Tests/InputSystem/CoreTests_Events.cs

@@ -543,26 +543,23 @@ public void Events_AreProcessedInOrderTheyAreQueuedIn()
 
     [Test]
     [Category("Events")]
-    public void Events_CanQueueAndReceiveEventsAgainstNonExistingDevices()
+    public void Events_WillNotReceiveEventsAgainstNonExistingDevices()
     {
         // Device IDs are looked up only *after* the system shows the event to us.
 
         var receivedCalls = 0;
-        var receivedDeviceId = InputDevice.InvalidDeviceId;
         InputSystem.onEvent +=
             (eventPtr, _) =>
         {
             ++receivedCalls;
-            receivedDeviceId = eventPtr.deviceId;
         };
 
         var inputEvent = DeviceConfigurationEvent.Create(4, 1.0);
         InputSystem.QueueEvent(ref inputEvent);
 
         InputSystem.Update();
 
-        Assert.That(receivedCalls, Is.EqualTo(1));
-        Assert.That(receivedDeviceId, Is.EqualTo(4));
+        Assert.That(receivedCalls, Is.EqualTo(0));
     }
 
     [Test]
@@ -579,7 +576,9 @@ public void Events_HandledFlagIsResetWhenEventIsQueued()
             wasHandled = eventPtr.handled;
         };
 
-        var inputEvent = DeviceConfigurationEvent.Create(4, 1.0);
+        var device = InputSystem.AddDevice<Gamepad>();
+
+        var inputEvent = DeviceConfigurationEvent.Create(device.id, 1.0);
 
         // This should go back to false when we inputEvent goes on the queue.
         // The way the behavior is implemented is a side-effect of how we store

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -11,6 +11,7 @@ however, it has to be formatted properly to pass verification tests.
 
 ### Fixed
 
+- Don't pass events for null devices (for devices which have not been created) to `InputSystem.onEvent` callbacks.
 - Will close debugger input state windows, when the state is no longer valid instead of throwing exceptions.
 
 #### Actions

Packages/com.unity.inputsystem/Documentation~/Events.md

@@ -80,13 +80,13 @@ For example, the following code demonstrates how to read a value for [`Gamepad.l
 
 ```
 InputSystem.onEvent +=
-    eventPtr =>
+    (eventPtr, device) =>
     {
         // Ignore anything that isn't a state event.
         if (!eventPtr.IsA<StateEvent>() && !eventPtr.IsA<DeltaStateEvent>())
             return;
 
-        var gamepad = InputSystem.GetDeviceById(eventPtr.deviceId) as Gamepad;
+        var gamepad = device as Gamepad;
         if (gamepad == null)
         {
             // Event isn't for a gamepad or device ID is no longer valid.

Packages/com.unity.inputsystem/Documentation~/HowDoI.md

@@ -555,7 +555,7 @@ And then make sure you put extra StateEvents for your HMD on the queue right in
 ```C#
 
     InputSystem.onEvent +=
-        eventPtr =>
+        (eventPtr, device) =>
         {
             // Can handle events yourself, for example, and then stop them
             // from further processing by marking them as handled.

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

@@ -2367,25 +2367,6 @@ private unsafe void OnUpdate(InputUpdateType updateType, ref InputEventBuffer ev
                 if (currentEventReadPtr->internalTime <= currentTime)
                     totalEventLag += currentTime - currentEventReadPtr->internalTime;
 
-                // Give listeners a shot at the event.
-                var listenerCount = m_EventListeners.length;
-                if (listenerCount > 0)
-                {
-                    if (device == null)
-                        device = TryGetDeviceById(currentEventReadPtr->deviceId);
-
-                    for (var i = 0; i < listenerCount; ++i)
-                        m_EventListeners[i](new InputEventPtr(currentEventReadPtr), device);
-
-                    // If a listener marks the event as handled, we don't process it further.
-                    if (currentEventReadPtr->handled)
-                    {
-                        eventBuffer.AdvanceToNextEvent(ref currentEventReadPtr, ref currentEventWritePtr,
-                            ref numEventsRetainedInBuffer, ref remainingEventCount, leaveEventInBuffer: false);
-                        continue;
-                    }
-                }
-
                 // Grab device for event. In before-render updates, we already had to
                 // check the device.
                 if (device == null)
@@ -2404,6 +2385,22 @@ private unsafe void OnUpdate(InputUpdateType updateType, ref InputEventBuffer ev
                     continue;
                 }
 
+                // Give listeners a shot at the event.
+                var listenerCount = m_EventListeners.length;
+                if (listenerCount > 0)
+                {
+                    for (var i = 0; i < listenerCount; ++i)
+                        m_EventListeners[i](new InputEventPtr(currentEventReadPtr), device);
+
+                    // If a listener marks the event as handled, we don't process it further.
+                    if (currentEventReadPtr->handled)
+                    {
+                        eventBuffer.AdvanceToNextEvent(ref currentEventReadPtr, ref currentEventWritePtr,
+                            ref numEventsRetainedInBuffer, ref remainingEventCount, leaveEventInBuffer: false);
+                        continue;
+                    }
+                }
+
                 // Process.
                 var currentEventType = currentEventReadPtr->type;
                 var currentEventTimeInternal = currentEventReadPtr->internalTime;

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

@@ -1463,18 +1463,14 @@ public static int FindControls<TControl>(string path, ref InputControlList<TCont
         /// may be postponed to a subsequent frame).
         ///
         /// As the input system reads events from the buffer one by one, it will trigger this
-        /// callback for each event before then proceeding to process the event. However, if
-        /// any of the callbacks sets <see cref="InputEvent.handled"/> to true, the event will
-        /// be skipped and ignored.
+        /// callback for each event which originates from a recognized device, before then proceeding
+        /// to process the event. However, if any of the callbacks sets <see cref="InputEvent.handled"/>
+        /// to true, the event will be skipped and ignored.
         ///
         /// Note that the input system does NOT sort events by timestamps (<see cref="InputEvent.time"/>).
         /// Instead, they are consumed in the order they are produced. This means that they
         /// will also surface on this callback in that order.
         ///
-        /// Note that the callback will be called for any event picked up from the internal
-        /// event buffer. This may include events for devices that have not been created and thus
-        /// may mean that the <see cref="InputDevice"/> parameter to the callback is null.
-        ///
         /// <example>
         /// <code>
         /// // Treat left+right mouse button as middle mouse button.