diff --git a/Assets/Tests/InputSystem/CoreTests_Actions.cs b/Assets/Tests/InputSystem/CoreTests_Actions.cs index c4ae3b7c5d..00e2f5747b 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions.cs @@ -30,6 +30,34 @@ // in terms of complexity. partial class CoreTests { + // ISXB-925: Feature flag values should live with containing settings instance. + [TestCase(InputFeatureNames.kUseReadValueCaching)] + [TestCase(InputFeatureNames.kUseOptimizedControls)] + [TestCase(InputFeatureNames.kParanoidReadValueCachingChecks)] + [TestCase(InputFeatureNames.kDisableUnityRemoteSupport)] + [TestCase(InputFeatureNames.kRunPlayerUpdatesInEditMode)] + #if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS + [TestCase(InputFeatureNames.kUseIMGUIEditorForAssets)] + #endif + public void Settings_ShouldStoreSettingsAndFeatureFlags(string featureName) + { + using (var settings = Scoped.Object(InputSettings.CreateInstance())) + { + InputSystem.settings = settings.value; + + Assert.That(InputSystem.settings.IsFeatureEnabled(featureName), Is.False); + settings.value.SetInternalFeatureFlag(featureName, true); + Assert.That(InputSystem.settings.IsFeatureEnabled(featureName), Is.True); + + using (var other = Scoped.Object(InputSettings.CreateInstance())) + { + InputSystem.settings = other.value; + + Assert.That(InputSystem.settings.IsFeatureEnabled(featureName), Is.False); + } + } + } + [Test] [Category("Actions")] public void Actions_WhenShortcutsDisabled_AllConflictingActionsTrigger() diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index a0fdd96b22..2c39ec0385 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -28,6 +28,7 @@ however, it has to be formatted properly to pass verification tests. - Fixed InputActionReference issues when domain reloads are disabled [ISXB-601](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-601), [ISXB-718](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-718), [ISXB-900](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-900) - Fixed a performance issue with many objects using multiple action maps [ISXB-573](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-573). - Fixed an variable scope shadowing issue causing compilation to fail on Unity 2019 LTS. +- Fixed an issue where changing `InputSettings` instance would not affect associated feature flags. ### Added - Added additional device information when logging the error due to exceeding the maximum number of events processed diff --git a/Packages/com.unity.inputsystem/InputSystem/Controls/InputControl.cs b/Packages/com.unity.inputsystem/InputSystem/Controls/InputControl.cs index a9500197b6..38a6f16b0b 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Controls/InputControl.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Controls/InputControl.cs @@ -929,7 +929,7 @@ public void ApplyParameterChanges() private void SetOptimizedControlDataType() { // setting check need to be inline so we clear optimizations if setting is disabled after the fact - m_OptimizedControlDataType = InputSettings.optimizedControlsFeatureEnabled + m_OptimizedControlDataType = InputSystem.s_Manager.optimizedControlsFeatureEnabled ? CalculateOptimizedControlDataType() : (FourCC)InputStateBlock.kFormatInvalid; } @@ -957,7 +957,7 @@ internal void SetOptimizedControlDataTypeRecursively() [Conditional("UNITY_EDITOR")] internal void EnsureOptimizationTypeHasNotChanged() { - if (!InputSettings.optimizedControlsFeatureEnabled) + if (!InputSystem.s_Manager.optimizedControlsFeatureEnabled) return; var currentOptimizedControlDataType = CalculateOptimizedControlDataType(); @@ -1172,7 +1172,7 @@ public ref readonly TValue value if ( // if feature is disabled we re-evaluate every call - !InputSettings.readValueCachingFeatureEnabled + !InputSystem.s_Manager.readValueCachingFeatureEnabled // if cached value is stale we re-evaluate and clear the flag || m_CachedValueIsStale // if a processor in stack needs to be re-evaluated, but unprocessedValue is still can be cached @@ -1183,7 +1183,7 @@ public ref readonly TValue value m_CachedValueIsStale = false; } #if DEBUG - else if (InputSettings.paranoidReadValueCachingChecksEnabled) + else if (InputSystem.s_Manager.paranoidReadValueCachingChecksEnabled) { var oldUnprocessedValue = m_UnprocessedCachedValue; var newUnprocessedValue = unprocessedValue; @@ -1225,7 +1225,7 @@ internal unsafe ref readonly TValue unprocessedValue if ( // if feature is disabled we re-evaluate every call - !InputSettings.readValueCachingFeatureEnabled + !InputSystem.s_Manager.readValueCachingFeatureEnabled // if cached value is stale we re-evaluate and clear the flag || m_UnprocessedCachedValueIsStale ) @@ -1234,7 +1234,7 @@ internal unsafe ref readonly TValue unprocessedValue m_UnprocessedCachedValueIsStale = false; } #if DEBUG - else if (InputSettings.paranoidReadValueCachingChecksEnabled) + else if (InputSystem.s_Manager.paranoidReadValueCachingChecksEnabled) { var currentUnprocessedValue = ReadUnprocessedValueFromState(currentStatePtr); if (CompareValue(ref currentUnprocessedValue, ref m_UnprocessedCachedValue)) diff --git a/Packages/com.unity.inputsystem/InputSystem/InputManager.cs b/Packages/com.unity.inputsystem/InputSystem/InputManager.cs index 8189aaf430..02dc53bd18 100644 --- a/Packages/com.unity.inputsystem/InputSystem/InputManager.cs +++ b/Packages/com.unity.inputsystem/InputSystem/InputManager.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Runtime.CompilerServices; using System.Text; using Unity.Collections; using UnityEngine.InputSystem.Composites; @@ -2116,6 +2117,33 @@ internal struct AvailableDevice internal IInputRuntime m_Runtime; internal InputMetrics m_Metrics; internal InputSettings m_Settings; + + // Extract as booleans (from m_Settings) because feature check is in the hot path + + private bool m_OptimizedControlsFeatureEnabled; + internal bool optimizedControlsFeatureEnabled + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => m_OptimizedControlsFeatureEnabled; + set => m_OptimizedControlsFeatureEnabled = value; + } + + private bool m_ReadValueCachingFeatureEnabled; + internal bool readValueCachingFeatureEnabled + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => m_ReadValueCachingFeatureEnabled; + set => m_ReadValueCachingFeatureEnabled = value; + } + + private bool m_ParanoidReadValueCachingChecksEnabled; + internal bool paranoidReadValueCachingChecksEnabled + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => m_ParanoidReadValueCachingChecksEnabled; + set => m_ParanoidReadValueCachingChecksEnabled = value; + } + #if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS private InputActionAsset m_Actions; #endif @@ -2644,6 +2672,11 @@ internal void ApplySettings() #if UNITY_EDITOR runPlayerUpdatesInEditMode = m_Settings.IsFeatureEnabled(InputFeatureNames.kRunPlayerUpdatesInEditMode); #endif + + // Extract feature flags into fields since used in hot-path + m_ReadValueCachingFeatureEnabled = m_Settings.IsFeatureEnabled((InputFeatureNames.kUseReadValueCaching)); + m_OptimizedControlsFeatureEnabled = m_Settings.IsFeatureEnabled((InputFeatureNames.kUseOptimizedControls)); + m_ParanoidReadValueCachingChecksEnabled = m_Settings.IsFeatureEnabled((InputFeatureNames.kParanoidReadValueCachingChecks)); } // Cache some values. @@ -3542,7 +3575,7 @@ private void ResetCurrentProcessedEventBytesForDevices() [Conditional("UNITY_EDITOR")] void CheckAllDevicesOptimizedControlsHaveValidState() { - if (!InputSettings.optimizedControlsFeatureEnabled) + if (!InputSystem.s_Manager.m_OptimizedControlsFeatureEnabled) return; foreach (var device in devices) @@ -3732,7 +3765,7 @@ private unsafe void WriteStateChange(InputStateBuffers.DoubleBuffers buffers, in deviceStateSize); } - if (InputSettings.readValueCachingFeatureEnabled) + if (InputSystem.s_Manager.m_ReadValueCachingFeatureEnabled) { // if the buffers have just been flipped, and we're doing a full state update, then the state from the // previous update is now in the back buffer, and we should be comparing to that when checking what diff --git a/Packages/com.unity.inputsystem/InputSystem/InputSettings.cs b/Packages/com.unity.inputsystem/InputSystem/InputSettings.cs index c9cdafa369..e79bd68402 100644 --- a/Packages/com.unity.inputsystem/InputSystem/InputSettings.cs +++ b/Packages/com.unity.inputsystem/InputSystem/InputSettings.cs @@ -717,27 +717,13 @@ public void SetInternalFeatureFlag(string featureName, bool enabled) if (string.IsNullOrEmpty(featureName)) throw new ArgumentNullException(nameof(featureName)); - switch (featureName) - { - case InputFeatureNames.kUseOptimizedControls: - optimizedControlsFeatureEnabled = enabled; - break; - case InputFeatureNames.kUseReadValueCaching: - readValueCachingFeatureEnabled = enabled; - break; - case InputFeatureNames.kParanoidReadValueCachingChecks: - paranoidReadValueCachingChecksEnabled = enabled; - break; - default: - if (m_FeatureFlags == null) - m_FeatureFlags = new HashSet(); - - if (enabled) - m_FeatureFlags.Add(featureName.ToUpperInvariant()); - else - m_FeatureFlags.Remove(featureName.ToUpperInvariant()); - break; - } + if (m_FeatureFlags == null) + m_FeatureFlags = new HashSet(); + + if (enabled) + m_FeatureFlags.Add(featureName.ToUpperInvariant()); + else + m_FeatureFlags.Remove(featureName.ToUpperInvariant()); OnChange(); } @@ -778,11 +764,6 @@ internal bool IsFeatureEnabled(string featureName) return m_FeatureFlags != null && m_FeatureFlags.Contains(featureName.ToUpperInvariant()); } - // Needs a static field because feature check is in the hot path - internal static bool optimizedControlsFeatureEnabled = false; - internal static bool readValueCachingFeatureEnabled; - internal static bool paranoidReadValueCachingChecksEnabled; - internal void OnChange() { if (InputSystem.settings == this)