FIX: ISXB-925 Fixed an issue where changing InputSettings instance fo…

…r the Input System wouldn't affect feature flags. (#1954) * FIX: ISXB-925 Fixed an issue where changing InputSettings instance for the Input System wouldn't affect feature flags.
Unity-Technologies · Jun 26, 2024 · f9d7240 · f9d7240
1 parent 29cde6c
commit f9d7240
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 34 deletions.
diff --git 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<InputSettings>()))
+        {
+            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<InputSettings>()))
+            {
+                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
@@ -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
@@ -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
@@ -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
@@ -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<string>();
-
-                    if (enabled)
-                        m_FeatureFlags.Add(featureName.ToUpperInvariant());
-                    else
-                        m_FeatureFlags.Remove(featureName.ToUpperInvariant());
-                    break;
-            }
+            if (m_FeatureFlags == null)
+                m_FeatureFlags = new HashSet<string>();
+
+            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)