diff --git a/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs b/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs new file mode 100644 index 0000000000..6f25a9488b --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs @@ -0,0 +1,226 @@ +using System; +using System.Collections; +using System.Linq; +using NUnit.Framework; +using UnityEngine; +using UnityEngine.InputSystem; +using UnityEngine.TestTools; +using TouchPhase = UnityEngine.InputSystem.TouchPhase; + +/// +/// Test suite to verify test fixture API published in . +/// +/// +/// This test fixture captures confusion around usage reported in +/// https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637. +/// +internal class InputTestFixtureTests : InputTestFixture +{ + private Keyboard correctlyUsedKeyboard; + + private Keyboard incorrectlyUsedDevice; + private Gamepad incorrectlyUsedGamepad; + private Touchscreen incorrectlyUsedTouchscreen; + + [OneTimeSetUp] + public void UnitySetup() + { + // This is incorrect use since it will add a device to the actual input system since it executes before + // the InputTestFixture.Setup() method. Hence, after Setup() has executed the device is not part of + // the test input system instance. + incorrectlyUsedDevice = InputSystem.AddDevice(); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True); + + incorrectlyUsedGamepad = InputSystem.AddDevice(); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedGamepad), Is.True); + + incorrectlyUsedTouchscreen = InputSystem.AddDevice(); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedTouchscreen), Is.True); + } + + [OneTimeTearDown] + public void UnityTearDown() + { + // Once InputTestFixture.TearDown() has executed, the state stack will have been popped and the correctlyUsedKeyboard + // we added before entering the test fixture should have been restored. + Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedGamepad), Is.True); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedTouchscreen), Is.True); + } + + [SetUp] + public override void Setup() + { + // At this point we are still using the actual system so our device created from UnitySetup() should still + // exist with the context. + Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedGamepad), Is.True); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedTouchscreen), Is.True); + + // This is expected usage pattern, first calling base.Setup() when overriding Setup like this, then + // creating a fake device via the test fixture instance that only lives with the test context. + base.Setup(); + + // Since we have now entered a temporary test state our device created in UnitySetup() will no longer exist + // with this context. + Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.False); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedGamepad), Is.False); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedTouchscreen), Is.False); + } + + [TearDown] + public override void TearDown() + { + // Restore state + base.TearDown(); + + // Our test device should no longer exist with the system since we are back to real instance + Assert.That(InputSystem.devices.Contains(correctlyUsedKeyboard), Is.False); + + Assert.That(InputSystem.devices.Contains(incorrectlyUsedDevice), Is.True); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedGamepad), Is.True); + Assert.That(InputSystem.devices.Contains(incorrectlyUsedTouchscreen), Is.True); + } + + #region Editor playmode tests + + [Test] + public void Press_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext() + { + correctlyUsedKeyboard = InputSystem.AddDevice(); + Press(correctlyUsedKeyboard.spaceKey); + Assert.That(correctlyUsedKeyboard.spaceKey.isPressed, Is.True); + } + + [Test] + public void Press_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice() + { + Assert.Throws(() => Press(incorrectlyUsedDevice.spaceKey)); + } + + [Test] + public void Release_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext() + { + correctlyUsedKeyboard = InputSystem.AddDevice(); + Press(correctlyUsedKeyboard.spaceKey); + Release(correctlyUsedKeyboard.spaceKey); + Assert.That(correctlyUsedKeyboard.spaceKey.isPressed, Is.False); + } + + [Test] + public void Release_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice() + { + Assert.Throws(() => Release(incorrectlyUsedDevice.spaceKey)); + } + + [Test] + public void PressAndRelease_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext() + { + correctlyUsedKeyboard = InputSystem.AddDevice(); + PressAndRelease(correctlyUsedKeyboard.spaceKey); + Assert.That(correctlyUsedKeyboard.spaceKey.isPressed, Is.False); + } + + [Test] + public void PressAndRelease_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice() + { + Assert.Throws(() => PressAndRelease(incorrectlyUsedDevice.spaceKey)); + } + + [Test] + public void Click_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext() + { + correctlyUsedKeyboard = InputSystem.AddDevice(); + Click(correctlyUsedKeyboard.spaceKey); + Assert.That(correctlyUsedKeyboard.spaceKey.isPressed, Is.False); + } + + [Test] + public void Click_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice() + { + Assert.Throws(() => Click(incorrectlyUsedDevice.spaceKey)); + } + + [Test] + public void Move_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext() + { + var gamepad = InputSystem.AddDevice(); + Move(gamepad.leftStick, new Vector2(1.0f, 0.0f)); + Assert.That(gamepad.leftStick.value, Is.EqualTo(new Vector2(1.0f, 0.0f))); + } + + [Test] + public void Move_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice() + { + Assert.Throws(() => Move(incorrectlyUsedGamepad.leftStick, new Vector2(1.0f, 0.0f))); + } + + [Test] + public void Touch_ShouldMutateDeviceState_WithinPlayModeTestFixtureContext() + { + var touchscreen = InputSystem.AddDevice(); + BeginTouch(1, Vector2.zero, screen: touchscreen); + Assert.That(touchscreen.touches.Count, Is.EqualTo(10)); + Assert.That(touchscreen.touches[0].touchId.value, Is.EqualTo(1)); + Assert.That(touchscreen.touches[0].position.value, Is.EqualTo(Vector2.zero)); + Assert.That(touchscreen.touches[0].phase.value, Is.EqualTo(TouchPhase.Began)); + + MoveTouch(1, Vector2.one, screen: touchscreen); + Assert.That(touchscreen.touches.Count, Is.EqualTo(10)); + Assert.That(touchscreen.touches[0].touchId.value, Is.EqualTo(1)); + Assert.That(touchscreen.touches[0].position.value, Is.EqualTo(Vector2.one)); + Assert.That(touchscreen.touches[0].phase.value, Is.EqualTo(TouchPhase.Moved)); + + EndTouch(1, Vector2.zero, screen: touchscreen); + Assert.That(touchscreen.touches.Count, Is.EqualTo(10)); + Assert.That(touchscreen.touches[0].touchId.value, Is.EqualTo(1)); + Assert.That(touchscreen.touches[0].position.value, Is.EqualTo(Vector2.zero)); + Assert.That(touchscreen.touches[0].phase.value, Is.EqualTo(TouchPhase.Ended)); + } + + [Test] + public void Touch_ShouldThrow_WithinPlayModeTestFixtureContextIfInvalidDevice() + { + Assert.Throws(() => BeginTouch(1, Vector2.zero, screen: incorrectlyUsedTouchscreen)); + Assert.Throws(() => MoveTouch(1, Vector2.zero, screen: incorrectlyUsedTouchscreen)); + Assert.Throws(() => EndTouch(1, Vector2.zero, screen: incorrectlyUsedTouchscreen)); + } + + #endregion // Playmode tests + + #region // Edit-mode tests + + [UnityTest] + public IEnumerator Press_ShouldThrow_WithinEditModeTestFixtureContext() + { + correctlyUsedKeyboard = InputSystem.AddDevice(); + Assert.Throws(() => Press(correctlyUsedKeyboard.spaceKey)); + yield break; + } + + [UnityTest] + public IEnumerator Release_ShouldThrow_WithinEditModeTestFixtureContext() + { + correctlyUsedKeyboard = InputSystem.AddDevice(); + Assert.Throws(() => Release(correctlyUsedKeyboard.spaceKey)); + yield break; + } + + [UnityTest] + public IEnumerator PressAndRelease_ShouldThrow_WithinEditModeTestFixtureContext() + { + correctlyUsedKeyboard = InputSystem.AddDevice(); + Assert.Throws(() => PressAndRelease(correctlyUsedKeyboard.spaceKey)); + yield break; + } + + [UnityTest] + public IEnumerator Click_ShouldThrow_WithinEditModeTestFixtureContext() + { + correctlyUsedKeyboard = InputSystem.AddDevice(); + Assert.Throws(() => Click(correctlyUsedKeyboard.spaceKey)); + yield break; + } + + #endregion // Edit-mode tests +} diff --git a/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs.meta b/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs.meta new file mode 100644 index 0000000000..8cb308e72d --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/InputTestFixtureTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: c3455fa1a90b4c5683d9f36ec3ff075a +timeCreated: 1759764564 \ No newline at end of file diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index c321d77d63..b6ed804500 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -12,6 +12,8 @@ however, it has to be formatted properly to pass verification tests. ### Fixed - Fixed warnings being generated on Unity 6.3 (beta). (ISXB-1718). +- Fixed an issue in `DeltaStateEvent.From` where unsafe code would throw exception or crash if internal pointer `currentStatePtr` was `null`. [ISXB-1637](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637). +- Fixed an issue in `InputTestFixture.Set` where attempting to change state of a device not belonging to the test fixture context would result in null pointer exception or crash. [ISXB-1637](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1637). ## [1.15.0] - 2025-10-03 diff --git a/Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs b/Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs index 9002279341..7aa4f90727 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Events/DeltaStateEvent.cs @@ -76,6 +76,8 @@ public static NativeArray From(InputControl control, out InputEventPtr eve if (!device.added) throw new ArgumentException($"Device for control '{control}' has not been added to system", nameof(control)); + if (control.currentStatePtr == null) // Protects statePtr assignment below + throw new ArgumentNullException($"Control '{control}' does not have an associated state"); ref var deviceStateBlock = ref device.m_StateBlock; ref var controlStateBlock = ref control.m_StateBlock; diff --git a/Packages/com.unity.inputsystem/InputSystem/InputManager.cs b/Packages/com.unity.inputsystem/InputSystem/InputManager.cs index 4926f14b08..da89f5f736 100644 --- a/Packages/com.unity.inputsystem/InputSystem/InputManager.cs +++ b/Packages/com.unity.inputsystem/InputSystem/InputManager.cs @@ -1188,6 +1188,11 @@ private void NotifyUsageChanged(InputDevice device) ////TODO: make sure that no device or control with a '/' in the name can creep into the system + internal bool HasDevice(InputDevice device) + { + return device.m_DeviceIndex < m_DevicesCount && ReferenceEquals(m_Devices[device.m_DeviceIndex], device); + } + public InputDevice AddDevice(Type type, string name = null) { if (type == null) diff --git a/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs b/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs index 2727be91be..1cb056770b 100644 --- a/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs +++ b/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestFixture.cs @@ -59,6 +59,17 @@ namespace UnityEngine.InputSystem /// input and device discovery or removal notifications from platform code. This ensures /// that while the test is running, input that may be generated on the machine running /// the test will not infer with it. + /// + /// Be cautious when using NUnit.Framework.OneTimeSetUpAttribute and + /// NUnit.Framework.OneTimeTearDownAttribute in combination with this test fixture. + /// For example, any devices created prior to execution of would be added to the actual + /// Input System instead of the test fixture system and after has executed such devices + /// will no longer be valid. You may of course use these NUnit features, but it is advised to not attempt affecting + /// the Input System under test from those methods since it would affect the real system and not the system + /// under test. + /// + /// This test fixture is designed for play-mode tests and is generally not supported for edit-mode tests. + /// Both [Test] and [UnityTest] are supported, but only in play-mode. /// public class InputTestFixture { @@ -533,6 +544,12 @@ public void Set(InputDevice device, string path, TValue state, double ti /// Note that this parameter will be ignored if the test is a [UnityTest]. Multi-frame /// playmode tests will automatically process input as part of the Unity player loop. /// Value type of the given control. + /// If control is null. + /// If the device associated with has not + /// been added to the system or if the control does not have any associated state. The latter may only + /// happen if attempting to set a control of a device created outside the test context. + /// If attempting to set a control of a test device in an + /// editor assembly. [UnityTest] in editor assemblies is not supported by this test fixture. /// /// /// var gamepad = InputSystem.AddDevice<Gamepad>(); @@ -544,12 +561,14 @@ public void Set(InputControl control, TValue state, double time { if (control == null) throw new ArgumentNullException(nameof(control)); - if (!control.device.added) - throw new ArgumentException( - $"Device of control '{control}' has not been added to the system", nameof(control)); + CheckValidity(control.device, control); if (IsUnityTest()) + { + if (IsEditMode()) + throw new NotSupportedException("InputTestFixture.Set does not support edit mode (editor assembly) [UnityTest]."); queueEventOnly = true; + } void SetUpAndQueueEvent(InputEventPtr eventPtr) { @@ -663,6 +682,7 @@ public void SetTouch(int touchId, TouchPhase phase, Vector2 position, float pres if (screen == null) screen = InputSystem.AddDevice(); } + CheckValidity(screen); InputSystem.QueueStateEvent(screen, new TouchState { @@ -913,6 +933,39 @@ internal void SimulateDomainReload() #endif + private static void CheckValidity(InputDevice device, InputControl control) + { + if (!device.added) + { + throw new ArgumentException( + $"Device '{device}' has not been added to the system", nameof(device)); + } + + // Guards against a device from another scope being used. This is a direct way to evaluate whether + // the device is associated with the current manager state or not since device state isn't consistently + // pushed/popped in the current design. + var manager = InputSystem.s_Manager; + if (manager == null || !manager.HasDevice(device)) + { + throw new ArgumentException($"Control '{control}' does not have any associated state. " + + "Make sure the control or device was added after executing Setup().", nameof(control)); + } + } + + private static void CheckValidity(InputControl control) + { + CheckValidity(control.device, control); + } + + /// + /// Returns true if running inside an Edit Mode test (Editor assembly). + /// Returns false if running in Play Mode. + /// + private static bool IsEditMode() + { + return Application.isEditor && !Application.isPlaying; + } + #if UNITY_EDITOR /// /// Represents an analytics registration event captured by test harness.