From b98453a3f57dd5b7ab875387fefd6f4a9d8b1e62 Mon Sep 17 00:00:00 2001 From: Alex Tyrer Date: Mon, 30 Sep 2024 17:10:38 +0100 Subject: [PATCH 1/2] [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. --- Assets/Tests/InputSystem/CoreTests_Devices.cs | 41 +++++++++++++++---- Packages/com.unity.inputsystem/CHANGELOG.md | 3 ++ .../InputSystem/InputManager.cs | 9 ++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/Assets/Tests/InputSystem/CoreTests_Devices.cs b/Assets/Tests/InputSystem/CoreTests_Devices.cs index 62b5a9232f..3dd8c69193 100644 --- a/Assets/Tests/InputSystem/CoreTests_Devices.cs +++ b/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] diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index d06a397cfb..4aa5bc38b7 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/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. diff --git a/Packages/com.unity.inputsystem/InputSystem/InputManager.cs b/Packages/com.unity.inputsystem/InputSystem/InputManager.cs index 989ec623cf..10e3c17fb8 100644 --- a/Packages/com.unity.inputsystem/InputSystem/InputManager.cs +++ b/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; From 336bfd5364f72479276ba21980977806b1b75ec1 Mon Sep 17 00:00:00 2001 From: Alex Tyrer Date: Tue, 1 Oct 2024 09:58:59 +0100 Subject: [PATCH 2/2] Fix formatting in CoreTests_Devices.cs (indentation) --- Assets/Tests/InputSystem/CoreTests_Devices.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Assets/Tests/InputSystem/CoreTests_Devices.cs b/Assets/Tests/InputSystem/CoreTests_Devices.cs index 3dd8c69193..bc4ec5a82b 100644 --- a/Assets/Tests/InputSystem/CoreTests_Devices.cs +++ b/Assets/Tests/InputSystem/CoreTests_Devices.cs @@ -4181,7 +4181,7 @@ public void Devices_RemovingAndReaddingDeviceWithNullDescriptorFields_DoesNotThr InputSystem.Update(); // "Unplug" device. - var removeEvent1 = DeviceRemoveEvent.Create(deviceId); + var removeEvent1 = DeviceRemoveEvent.Create(deviceId); InputSystem.QueueEvent(ref removeEvent1); InputSystem.Update();