diff --git a/Assets/Tests/InputSystem/CoreTests_Devices.cs b/Assets/Tests/InputSystem/CoreTests_Devices.cs index ed90efe605..5b92d32755 100644 --- a/Assets/Tests/InputSystem/CoreTests_Devices.cs +++ b/Assets/Tests/InputSystem/CoreTests_Devices.cs @@ -4384,6 +4384,28 @@ public void Devices_CanSetPollingFrequency() Assert.That(InputSystem.pollingFrequency, Is.EqualTo(120).Within(0.000001)); } + [Test] + [Category("Devices")] + public void Devices_CannotSetPollingFrequencyToNonFiniteValue() + { + // A plain `value <= 0` guard misses NaN (which compares false against everything) and + // +Infinity, leaving pollingFrequency stuck at that value on every read. (ISX-2837) + InputSystem.pollingFrequency = 120; + + Assert.That(() => InputSystem.pollingFrequency = float.NaN, Throws.ArgumentException); + + // The native backend does accept +Infinity, as "poll continuously". This public API + // deliberately does not - it pins a core, and is far more likely to be a `1f/0f` accident + // at the call site than a deliberate choice. + Assert.That(() => InputSystem.pollingFrequency = float.PositiveInfinity, Throws.ArgumentException); + Assert.That(() => InputSystem.pollingFrequency = float.NegativeInfinity, Throws.ArgumentException); + Assert.That(() => InputSystem.pollingFrequency = 0f, Throws.ArgumentException); + Assert.That(() => InputSystem.pollingFrequency = -1f, Throws.ArgumentException); + + // A rejected assignment must leave the previous value untouched. + Assert.That(InputSystem.pollingFrequency, Is.EqualTo(120).Within(0.000001)); + } + #if UNITY_INPUT_SYSTEM_PLATFORM_POLLING_FREQUENCY [Test] [Category("Devices")] diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 10e1e71015..8da969470a 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed +- Fixed `InputSystem.pollingFrequency` accepting non-finite values. Both `NaN` and `+Infinity` slipped past the `value <= 0` guard - `NaN` left the property returning `NaN` on every read and disabled background polling, while `+Infinity` set the backend to poll continuously and occupy a CPU core. Since both are almost always the result of an arithmetic accident at the call site (`0f/0f` and `1f/0f` respectively), they are now rejected with an `ArgumentException`. [ISX-2837](https://jira.unity3d.com/browse/ISX-2837) - Fixed the "Supported Devices" list in the Input System Package Settings sitting flush against the panel edge with no left/right margin, unlike the surrounding fields; it is now inset to line up with the other settings controls [UUM-150207](https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-150207) - Fixed the Inspector help button for a selected `.inputactions` asset ("Open Reference for Input Action Importer") opening a missing documentation page; it now links to the Action Assets manual page [UUM-149518](https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-149518) - Fixed an `OverflowException` when creating a control scheme (or other named item) whose all-numeric name exceeds `Int32.MaxValue`, which previously discarded the entered name and fell back to the default [UUM-145766](https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-145766) diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/InputManager.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/InputManager.cs index 994538ecc2..eb4b4e4922 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/InputManager.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/InputManager.cs @@ -277,9 +277,17 @@ public float pollingFrequency set { - ////REVIEW: allow setting to zero to turn off polling altogether? - if (value <= 0) - throw new ArgumentException("Polling frequency must be greater than zero", "value"); + // Non-finite values almost always arrive by accident - `0f/0f` gives NaN and `1f/0f` + // gives +Infinity - and both slip past a plain `value <= 0` check. NaN gets cached + // and read back forever. +Infinity means continuous polling in the native backend, + // which leaves the polling thread nothing to wait on and pins a core. Neither is + // something a public setter should accept. Zero is likewise supported by the native + // backend, where it disables polling outright, but is not exposed here - silently + // stopping all polled-device input from a frequency setter is a footgun rather than + // a discoverable off switch. + // (float.IsFinite is unavailable on netstandard2.0.) + if (value <= 0 || float.IsNaN(value) || float.IsInfinity(value)) + throw new ArgumentException("Polling frequency must be a finite value greater than zero", "value"); #if UNITY_INPUT_SYSTEM_PLATFORM_POLLING_FREQUENCY m_Runtime.pollingFrequency = value; diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/InputSystem.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/InputSystem.cs index fce3a22117..c9cbece292 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/InputSystem.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/InputSystem.cs @@ -1337,7 +1337,13 @@ public static event InputDeviceFindControlLayoutDelegate onFindLayoutForDevice /// /// Also note that the polling frequency applies to all devices that are polled. It is not possible /// to set polling frequency on a per-device basis. + /// + /// Only finite values greater than zero are accepted. Non-finite values are rejected rather than + /// passed through, as they are almost always the result of an arithmetic accident at the call site + /// (0f/0f yields NaN, 1f/0f yields +Infinity) and would otherwise leave + /// polling in an unusable state. /// + /// Value is not a finite number greater than zero. public static float pollingFrequency { get => s_Manager.pollingFrequency;