FIX: MakeEscapedJsonString now null-checks inputs. Empty strings also now bypass an allocation. (#2019)

[Input System] MakeEscapedJsonString now null-checks inputs. Empty strings also now bypass an allocation.

This matches older behaviour on null-checking descriptor fields which is needed for certain device implementations.

Author: AlexTyrer

Assets/Tests/InputSystem/CoreTests_Devices.cs

@@ -4162,13 +4162,40 @@ public void Devices_RemovingAndReaddingDevice_DoesNotAllocateMemory()
         recorder.CollectFromAllThreads();
 #endif
 
-        // We expect a single allocation for each call to ReportNewInputDevice when there is one disconnected device
-        //
-        int numberOfRepeats = 2;
-        int numberOfDisconnectedDevices = 1;
-        int numberOfCallsToReportNewInputDevicePerRun = 2;
-        int expectedAllocations = numberOfRepeats * numberOfDisconnectedDevices * numberOfCallsToReportNewInputDevicePerRun;
-        Assert.AreEqual(expectedAllocations, recorder.sampleBlockCount);
+        // No allocations are expected.
+        Assert.AreEqual(0, recorder.sampleBlockCount);
+    }
+
+    // Regression test to cover having null descriptor fields for a device. Some non-desktop gamepad device types do this.
+    [Test]
+    [Category("Devices")]
+    public void Devices_RemovingAndReaddingDeviceWithNullDescriptorFields_DoesNotThrow()
+    {
+        // InputDeviceDescription.ToJson writes empty string fields and not null values, whereas reporting a device via an incomplete description string will fully omit the fields.
+        string description = @"{
+            ""type"": ""Gamepad"",
+            ""product"": ""TestProduct""
+        }";
+
+        var deviceId = runtime.ReportNewInputDevice(description);
+        InputSystem.Update();
+
+        // "Unplug" device.
+        var removeEvent1 = DeviceRemoveEvent.Create(deviceId);
+        InputSystem.QueueEvent(ref removeEvent1);
+        InputSystem.Update();
+
+        // "Plug" it back in.
+        deviceId = runtime.ReportNewInputDevice(description);
+        InputSystem.Update();
+
+        // Repeat that sequence.
+        var removeEvent2 = DeviceRemoveEvent.Create(deviceId);
+        InputSystem.QueueEvent(ref removeEvent2);
+        InputSystem.Update();
+
+        runtime.ReportNewInputDevice(description);
+        InputSystem.Update();
     }
 
     [Test]

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -10,6 +10,9 @@ however, it has to be formatted properly to pass verification tests.
 
 ## [Unreleased] - yyyy-mm-dd
 
+### Fixed
+- Fixed `NullReferenceException` from disconnecting and reconnecting a GXDKGamepad.
+
 ### Added
 - Added the display of the device flag `CanRunInBackground` in device debug view.
 - Added analytics for programmatic `InputAction` setup via `InputActionSetupExtensions` when exiting play-mode.

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

@@ -2562,6 +2562,15 @@ private JsonParser.JsonString MakeEscapedJsonString(string theString)
             // To avoid a very costly escape-skipping character-by-character string comparison in JsonParser.Json.Equals() we
             // reconstruct an escaped string and make an escaped JsonParser.JsonString and use that for the comparison instead.
             //
+            if (string.IsNullOrEmpty(theString))
+            {
+                return new JsonParser.JsonString
+                {
+                    text = string.Empty,    // text should be an empty string and not null for consistency on property comparisons
+                    hasEscapes = false
+                };
+            }
+
             var builder = new StringBuilder();
             var length = theString.Length;
             var hasEscapes = false;