From 1eb5dc07fa91f81e87931ee82b07ece690959b47 Mon Sep 17 00:00:00 2001 From: Tim Keosababian <36088732+timkeo@users.noreply.github.com> Date: Thu, 23 May 2024 14:00:29 -0700 Subject: [PATCH] Address InputSystem refactor (Neutron) PR feedback - Rename some variables - Add update some comments - Other small tweaks. --- .../Actions/InputActionRebindingExtensions.cs | 9 +++--- .../InputSystem/InputManager.cs | 29 +++++++++--------- .../InputSystem/InputSystem.cs | 21 ++++++------- .../InputSystem/InputSystemStateManager.cs | 30 ++++++++++++++++++- .../Plugins/PlayerInput/PlayerInput.cs | 2 +- 5 files changed, 58 insertions(+), 33 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionRebindingExtensions.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionRebindingExtensions.cs index 6a9af20747..62a02b5cbe 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionRebindingExtensions.cs +++ b/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(); } /// - /// 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. /// public void Dispose() { diff --git a/Packages/com.unity.inputsystem/InputSystem/InputManager.cs b/Packages/com.unity.inputsystem/InputSystem/InputManager.cs index e8268f9502..5ec57297ff 100644 --- a/Packages/com.unity.inputsystem/InputSystem/InputManager.cs +++ b/Packages/com.unity.inputsystem/InputSystem/InputManager.cs @@ -61,12 +61,12 @@ 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) @@ -74,25 +74,26 @@ public static InputManager CreateAndInitialize(IInputRuntime runtime, InputSetti settings = ScriptableObject.CreateInstance(); 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(); - 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) diff --git a/Packages/com.unity.inputsystem/InputSystem/InputSystem.cs b/Packages/com.unity.inputsystem/InputSystem/InputSystem.cs index 634a3516a1..f87b4ce54a 100644 --- a/Packages/com.unity.inputsystem/InputSystem/InputSystem.cs +++ b/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(); - if (existingSystemObjects != null && existingSystemObjects.Length > 0) + var existingSystemStateManagers = Resources.FindObjectsOfTypeAll(); + 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() diff --git a/Packages/com.unity.inputsystem/InputSystem/InputSystemStateManager.cs b/Packages/com.unity.inputsystem/InputSystem/InputSystemStateManager.cs index cab2b50071..0c2decc009 100644 --- a/Packages/com.unity.inputsystem/InputSystem/InputSystemStateManager.cs +++ b/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 /// /// A hidden, internal object we put in the editor to bundle input system state @@ -45,10 +45,38 @@ internal struct InputSystemState /// internal class InputSystemStateManager : ScriptableObject, ISerializationCallbackReceiver { + /// + /// References the "core" input state that must survive domain reloads. + /// [SerializeField] public InputSystemState systemState; + + /// + /// Triggers Editor restart when enabling NewInput back-ends. + /// [SerializeField] public bool newInputBackendsCheckedAsEnabled; + + /// + /// Saves and restores InputSettings across domain reloads + /// + /// + /// InputSettings are serialized to JSON which this string holds. + /// [SerializeField] public string settings; + + /// + /// Timestamp retrieved from InputRuntime.currentTime when exiting EditMode. + /// + /// + /// All input events occurring between exiting EditMode and entering PlayMode are discarded. + /// [SerializeField] public double exitEditModeTime; + + /// + /// Timestamp retrieved from InputRuntime.currentTime when entering PlayMode. + /// + /// + /// All input events occurring between exiting EditMode and entering PlayMode are discarded. + /// [SerializeField] public double enterPlayModeTime; public void OnBeforeSerialize() diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs index 8a511591bb..d3d522aa59 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs +++ b/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