Address InputSystem refactor PR feedback

- Rename some variables
- Add update some comments
- Other small tweaks.

Author: timkeo

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionRebindingExtensions.cs

@@ -2782,7 +2782,7 @@ internal static DeferBindingResolutionContext DeferBindingResolution()
         }
     }
 
-    internal class DeferBindingResolutionContext : IDisposable
+    internal sealed class DeferBindingResolutionContext : IDisposable
     {
         public int deferredCount => m_DeferredCount;
 
@@ -2793,14 +2793,13 @@ public void Acquire()
 
         public void Release()
         {
-            if (m_DeferredCount > 0)
-                --m_DeferredCount;
-            if (m_DeferredCount == 0)
+            if (m_DeferredCount > 0 && --m_DeferredCount == 0)
                 ExecuteDeferredResolutionOfBindings();
         }
 
         /// <summary>
-        /// Allows usage within using() blocks.
+        /// Allows usage within using() blocks, i.e. we need a "Release" method to match "Acquire", but we also want
+        /// to implement IDisposable so instance are automatically cleaned up when exiting a using() block.
         /// </summary>
         public void Dispose()
         {

Packages/com.unity.inputsystem/InputSystem/InputManager.cs

@@ -61,38 +61,39 @@ internal partial class InputManager : IDisposable
     {
         private InputManager() { }
 
-        public static InputManager CreateAndInitialize(IInputRuntime runtime, InputSettings settings, bool fakeRemove = false)
+        public static InputManager CreateAndInitialize(IInputRuntime runtime, InputSettings settings, bool fakeManagerForRemotingTests = false)
         {
-            var newInst = new InputManager();
+            var newInstance = new InputManager();
 
             // Not directly used by InputManager, but we need  a single instance that's used in a variety of places without a static field
-            newInst.m_DeferBindingResolutionContext = new DeferBindingResolutionContext();
+            newInstance.m_DeferBindingResolutionContext = new DeferBindingResolutionContext();
 
             // If settings object wasn't provided, create a temporary settings object for now
             if (settings == null)
             {
                 settings = ScriptableObject.CreateInstance<InputSettings>();
                 settings.hideFlags = HideFlags.HideAndDontSave;
             }    
-            newInst.m_Settings = settings;
+            newInstance.m_Settings = settings;
 
             #if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
-            newInst.InitializeActions();
+            newInstance.InitializeActions();
             #endif // UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
 
-            newInst.InitializeData();
-            newInst.InstallRuntime(runtime);
+            newInstance.InitializeData();
+            newInstance.InstallRuntime(runtime);
 
-            // Skip if initializing for "Fake Remove" manager (in tests)
-            if (!fakeRemove)
-                newInst.InstallGlobals();
+            // For remoting tests, we need to create a "fake manager" that simulates a remote endpoint.
+            // In this case don't install globals as this will corrupt the "local" manager state.
+            if (!fakeManagerForRemotingTests)
+                newInstance.InstallGlobals();
 
-            newInst.ApplySettings();
+            newInstance.ApplySettings();
 
             #if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
-            newInst.ApplyActions();
+            newInstance.ApplyActions();
             #endif
-            return newInst;
+            return newInstance;
         }
 
 #region Dispose implementation
@@ -3985,7 +3986,7 @@ internal void RestoreStateWithoutDevices(SerializedState state)
             if (state.settings == null)
             {
                 state.settings = ScriptableObject.CreateInstance<InputSettings>();
-                state.settings.hideFlags = HideFlags.HideAndDontSave;
+                state.settings.hideFlags = HideFlags.HideAndDontSave; // Hide from the project Hierarchy and Scene
             }
 
             if (m_Settings != null && m_Settings != state.settings)

Packages/com.unity.inputsystem/InputSystem/InputSystem.cs

@@ -89,7 +89,7 @@ static InputSystem()
 
         [RuntimeInitializeOnLoadMethod(RuntimeInitializeLoadType.BeforeSceneLoad)]
         private static void RuntimeInitialize()
-    {
+        {
             GlobalInitialize(false);
         }
 
@@ -3483,12 +3483,9 @@ private static void SetUpRemotingInternal()
         private static bool ShouldEnableRemoting()
         {
             #if UNITY_INCLUDE_TESTS
-            var isRunningTests = true;
-            #else
-            var isRunningTests = false;
+            return false; // Don't remote while running tests.
             #endif
-            if (isRunningTests)
-                return false; // Don't remote while running tests.
+
             return true;
         }
         #endif //!UNITY_EDITOR
@@ -3517,7 +3514,7 @@ private static void GlobalInitialize(bool calledFromCtor)
             // must initialize via the Runtime call.
 
             if (calledFromCtor || IsDomainReloadDisabledForPlayMode())
-        {
+            {
                 InitializeInEditor(calledFromCtor);
             }
 #else
@@ -3544,7 +3541,7 @@ internal static void EnsureInitialized()
 
 #if UNITY_EDITOR
 
-        // TODO: ISX-1860
+        // ISX-1860 - #ifdef out Domain Reload specific functionality from CoreCLR
         private static InputSystemStateManager s_DomainStateManager;
         internal static InputSystemStateManager domainStateManager => s_DomainStateManager;
 
@@ -3573,8 +3570,8 @@ internal static void InitializeInEditor(bool calledFromCtor, IInputRuntime runti
                 #endif
             }
 
-            var existingSystemObjects = Resources.FindObjectsOfTypeAll<InputSystemStateManager>();
-            if (existingSystemObjects != null && existingSystemObjects.Length > 0)
+            var existingSystemStateManagers = Resources.FindObjectsOfTypeAll<InputSystemStateManager>();
+            if (existingSystemStateManagers != null && existingSystemStateManagers.Length > 0)
             {
                 if (globalReset)
                 {
@@ -3584,7 +3581,7 @@ internal static void InitializeInEditor(bool calledFromCtor, IInputRuntime runti
                     // InputManager state here but we're still waiting from layout registrations
                     // that happen during domain initialization.
 
-                    s_DomainStateManager = existingSystemObjects[0];
+                    s_DomainStateManager = existingSystemStateManagers[0];
                     s_Manager.RestoreStateWithoutDevices(s_DomainStateManager.systemState.managerState);
                     InputDebuggerWindow.ReviveAfterDomainReload();
 
@@ -3765,7 +3762,7 @@ internal static void InitializeInPlayer(IInputRuntime runtime, bool loadSettings
 
 #if UNITY_INCLUDE_TESTS
         //
-        // We cannot define UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONSw with the Test-Framework assembly, and
+        // We cannot define UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS within the Test-Framework assembly, and
         // so this hook is needed; it's called from InputTestStateManager.Reset().
         //
         internal static void TestHook_DisableActions()

Packages/com.unity.inputsystem/InputSystem/InputSystemStateManager.cs

@@ -33,7 +33,7 @@ internal struct InputSystemState
         [NonSerialized] public ISavedState inputUserState;
     }
 
-    // TODO: ISX-1860
+    // ISX-1860 - #ifdef out Domain Reload specific functionality from CoreCLR
 #if UNITY_EDITOR
     /// <summary>
     /// A hidden, internal object we put in the editor to bundle input system state
@@ -45,10 +45,38 @@ internal struct InputSystemState
     /// </remarks>
     internal class InputSystemStateManager : ScriptableObject, ISerializationCallbackReceiver
     {
+        /// <summary>
+        /// References the "core" input state that must survive domain reloads.
+        /// </summary>
         [SerializeField] public InputSystemState systemState;
+
+        /// <summary>
+        /// Triggers Editor restart when enabling NewInput back-ends.
+        /// </summary>
         [SerializeField] public bool newInputBackendsCheckedAsEnabled;
+
+        /// <summary>
+        /// Saves and restores InputSettings across domain reloads
+        /// </summary>
+        /// <remarks>
+        /// InputSettings are serialized to JSON which this string holds.
+        /// </remarks>
         [SerializeField] public string settings;
+
+        /// <summary>
+        /// Timestamp retrieved from InputRuntime.currentTime when exiting EditMode.
+        /// </summary>
+        /// <remarks>
+        /// All input events occurring between exiting EditMode and entering PlayMode are discarded.
+        /// </remarks>
         [SerializeField] public double exitEditModeTime;
+
+        /// <summary>
+        /// Timestamp retrieved from InputRuntime.currentTime when entering PlayMode.
+        /// </summary>
+        /// <remarks>
+        /// All input events occurring between exiting EditMode and entering PlayMode are discarded.
+        /// </remarks>
         [SerializeField] public double enterPlayModeTime;
 
         public void OnBeforeSerialize()

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs

@@ -1209,7 +1209,7 @@ internal struct GlobalState
         private static void InitializeGlobalPlayerState()
         {
             // Touch GlobalState doesn't require Dispose operations
-            s_GlobalState = new GlobalState
+            s_GlobalState = new PlayerInput.GlobalState
             {
                 initPlayerIndex = -1,
                 initSplitScreenIndex = -1