Skip to content

Commit 0ceb1b5

Browse files
timkeotimkeo
committed
Refactor InputManager and tests for cleaner init flows (ISX-1842)
- Move Reset/Restore state functionality out of InputSystem to the Test assembly (InputTestStateManager.cs) - Refactor InputManager Init/Dispose to be cleaner and better abstracted: * Adds CreateAndInitialize static method * Replaces Destroy() with IDisposable implementation * InputManager creates "default" InputSettings object if none provided * Runtime, Settings, and Metrics fields now private - Update InitializeInEditor() to incorporate changes - Update and fix tests For the most part, the logic should be mostly preserved. InitializeInEditor() has the biggest (logical) change because Reset() (moved to Tests) contained some actual init calls that needed to be pulled out. However, we *should* be making the same calls in the same order. However, this change does seem to "break" some of the OnScreenTests(); they're now unstable. This will need to be fixed in a later commit.
1 parent bf4dfaf commit 0ceb1b5

File tree

10 files changed

+402
-297
lines changed

10 files changed

+402
-297
lines changed

‎Assets/Tests/InputSystem/CoreTests_Editor.cs‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,19 @@ public static Version ReadVersion()
7171
}
7272
}
7373

74+
private void SimulateDomainReload()
75+
{
76+
// This quite invasively goes into InputSystem internals. Unfortunately, we
77+
// have no proper way of simulating domain reloads ATM. So we directly call various
78+
// internal methods here in a sequence similar to what we'd get during a domain reload.
79+
// Since we're faking it, pass 'true' for calledFromCtor param.
80+
81+
InputSystem.s_SystemObject.OnBeforeSerialize();
82+
InputSystem.s_SystemObject = null;
83+
InputSystem.s_Manager = null; // Do NOT Dispose()! The native memory cannot be freed as it's reference by saved state
84+
InputSystem.InitializeInEditor(true, runtime);
85+
}
86+
7487
[Test]
7588
[Category("Editor")]
7689
public void Editor_PackageVersionAndAssemblyVersionAreTheSame()
@@ -147,11 +160,11 @@ public void Editor_CanSaveAndRestoreState()
147160
}.ToJson());
148161
InputSystem.Update();
149162

150-
InputSystem.SaveAndReset();
163+
m_StateManager.SaveAndReset(false, null);
151164

152165
Assert.That(InputSystem.devices, Has.Count.EqualTo(0));
153166

154-
InputSystem.Restore();
167+
m_StateManager.Restore();
155168

156169
Assert.That(InputSystem.devices,
157170
Has.Exactly(1).With.Property("layout").EqualTo("MyDevice").And.TypeOf<Gamepad>());
@@ -195,11 +208,11 @@ public void Editor_DomainReload_CanRestoreDevicesBuiltWithDynamicallyGeneratedLa
195208

196209
Assert.That(InputSystem.devices, Has.Exactly(1).TypeOf<HID>());
197210

198-
InputSystem.SaveAndReset();
211+
m_StateManager.SaveAndReset(false, null);
199212

200213
Assert.That(InputSystem.devices, Is.Empty);
201214

202-
var state = InputSystem.GetSavedState();
215+
var state = m_StateManager.GetSavedState();
203216
var manager = InputSystem.s_Manager;
204217

205218
manager.m_SavedAvailableDevices = state.managerState.availableDevices;
@@ -209,7 +222,7 @@ public void Editor_DomainReload_CanRestoreDevicesBuiltWithDynamicallyGeneratedLa
209222

210223
Assert.That(InputSystem.devices, Has.Exactly(1).TypeOf<HID>());
211224

212-
InputSystem.Restore();
225+
m_StateManager.Restore();
213226
}
214227

215228
[Test]
@@ -350,15 +363,15 @@ public void Editor_DomainReload_CanRemoveDevicesDuringDomainReload()
350363
[Category("Editor")]
351364
public void Editor_RestoringStateWillCleanUpEventHooks()
352365
{
353-
InputSystem.SaveAndReset();
366+
m_StateManager.SaveAndReset(false, null);
354367

355368
var receivedOnEvent = 0;
356369
var receivedOnDeviceChange = 0;
357370

358371
InputSystem.onEvent += (e, d) => ++ receivedOnEvent;
359372
InputSystem.onDeviceChange += (c, d) => ++ receivedOnDeviceChange;
360373

361-
InputSystem.Restore();
374+
m_StateManager.Restore();
362375

363376
var device = InputSystem.AddDevice("Gamepad");
364377
InputSystem.QueueStateEvent(device, new GamepadState());
@@ -375,8 +388,8 @@ public void Editor_RestoringStateWillRestoreObjectsOfLayoutBuilder()
375388
var builder = new TestLayoutBuilder {layoutToLoad = "Gamepad"};
376389
InputSystem.RegisterLayoutBuilder(() => builder.DoIt(), "TestLayout");
377390

378-
InputSystem.SaveAndReset();
379-
InputSystem.Restore();
391+
m_StateManager.SaveAndReset(false, null);
392+
m_StateManager.Restore();
380393

381394
var device = InputSystem.AddDevice("TestLayout");
382395

‎Assets/Tests/InputSystem/CoreTests_Remoting.cs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -477,11 +477,7 @@ private class FakeRemote : IDisposable
477477
public FakeRemote()
478478
{
479479
runtime = new InputTestRuntime();
480-
var manager = new InputManager();
481-
manager.m_Settings = ScriptableObject.CreateInstance<InputSettings>();
482-
manager.InitializeData();
483-
manager.InstallRuntime(runtime);
484-
manager.ApplySettings();
480+
var manager = InputManager.CreateAndInitialize(runtime, null, true);
485481

486482
local = new InputRemoting(InputSystem.s_Manager);
487483
remote = new InputRemoting(manager);
@@ -524,8 +520,8 @@ public void Dispose()
524520
}
525521
if (remoteManager != null)
526522
{
527-
Object.Destroy(remoteManager.m_Settings);
528-
remoteManager.Destroy();
523+
Object.Destroy(remoteManager.settings);
524+
remoteManager.Dispose();
529525
}
530526
}
531527
}

‎Assets/Tests/InputSystem/Plugins/HIDTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -708,8 +708,8 @@ public void Devices_HIDDescriptorSurvivesReload()
708708
}.ToJson());
709709
InputSystem.Update();
710710

711-
InputSystem.SaveAndReset();
712-
InputSystem.Restore();
711+
m_StateManager.SaveAndReset(false, null);
712+
m_StateManager.Restore();
713713

714714
var hid = (HID)InputSystem.devices.First(x => x is HID);
715715

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ internal static class InputAnalytics
1717

1818
public static void Initialize(InputManager manager)
1919
{
20-
Debug.Assert(manager.m_Runtime != null);
20+
Debug.Assert(manager.runtime != null);
2121
}
2222

2323
public static void OnStartup(InputManager manager)
@@ -60,8 +60,8 @@ public static void OnStartup(InputManager manager)
6060
data.old_enabled = EditorPlayerSettingHelpers.oldSystemBackendsEnabled;
6161
#endif
6262

63-
manager.m_Runtime.RegisterAnalyticsEvent(kEventStartup, 10, 100);
64-
manager.m_Runtime.SendAnalyticsEvent(kEventStartup, data);
63+
manager.runtime.RegisterAnalyticsEvent(kEventStartup, 10, 100);
64+
manager.runtime.SendAnalyticsEvent(kEventStartup, data);
6565
}
6666

6767
public static void OnShutdown(InputManager manager)
@@ -77,8 +77,8 @@ public static void OnShutdown(InputManager manager)
7777
total_event_processing_time = (float)metrics.totalEventProcessingTime,
7878
};
7979

80-
manager.m_Runtime.RegisterAnalyticsEvent(kEventShutdown, 10, 100);
81-
manager.m_Runtime.SendAnalyticsEvent(kEventShutdown, data);
80+
manager.runtime.RegisterAnalyticsEvent(kEventShutdown, 10, 100);
81+
manager.runtime.SendAnalyticsEvent(kEventShutdown, data);
8282
}
8383

8484
/// <summary>

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

Lines changed: 95 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
#if UNITY_EDITOR
1717
using UnityEngine.InputSystem.Editor;
18+
using UnityEngine.Tilemaps;
19+
1820
#endif
1921

2022
#if UNITY_EDITOR
@@ -54,13 +56,85 @@ namespace UnityEngine.InputSystem
5456
///
5557
/// Manages devices, layouts, and event processing.
5658
/// </remarks>
57-
internal partial class InputManager
59+
internal partial class InputManager : IDisposable
5860
{
61+
private InputManager() { }
62+
63+
public static InputManager CreateAndInitialize(IInputRuntime runtime, InputSettings settings, bool fakeRemove = false)
64+
{
65+
var newInst = new InputManager();
66+
67+
// If settings object wasn't provided, create a temporary settings object for now
68+
if (settings == null)
69+
{
70+
settings = ScriptableObject.CreateInstance<InputSettings>();
71+
settings.hideFlags = HideFlags.HideAndDontSave;
72+
}
73+
newInst.m_Settings = settings;
74+
75+
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
76+
newInst.InitializeActions();
77+
#endif // UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
78+
79+
newInst.InitializeData();
80+
newInst.InstallRuntime(runtime);
81+
82+
// Skip if initializing for "Fake Remove" manager (in tests)
83+
if (!fakeRemove)
84+
newInst.InstallGlobals();
85+
86+
newInst.ApplySettings();
87+
88+
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
89+
newInst.ApplyActions();
90+
#endif
91+
return newInst;
92+
}
93+
94+
#region Dispose implementation
95+
protected virtual void Dispose(bool disposing)
96+
{
97+
if (!disposedValue)
98+
{
99+
if (disposing)
100+
{
101+
// Notify devices are being removed but don't actually removed them; no point when disposing
102+
for (var i = 0; i < m_DevicesCount; ++i)
103+
m_Devices[i].NotifyRemoved();
104+
105+
m_StateBuffers.FreeAll();
106+
UninstallGlobals();
107+
108+
// If we're still holding the "temporary" settings object make sure to delete it
109+
if (m_Settings != null && m_Settings.hideFlags == HideFlags.HideAndDontSave)
110+
Object.DestroyImmediate(m_Settings);
111+
112+
// Project-wide Actions are never temporary so we do not destroy them.
113+
}
114+
115+
disposedValue = true;
116+
}
117+
}
118+
119+
~InputManager()
120+
{
121+
Dispose(false);
122+
}
123+
124+
public void Dispose()
125+
{
126+
Dispose(true);
127+
GC.SuppressFinalize(this);
128+
}
129+
private bool disposedValue;
130+
#endregion
131+
59132
public ReadOnlyArray<InputDevice> devices => new ReadOnlyArray<InputDevice>(m_Devices, 0, m_DevicesCount);
60133

61134
public TypeTable processors => m_Processors;
62135
public TypeTable interactions => m_Interactions;
63136
public TypeTable composites => m_Composites;
137+
internal IInputRuntime runtime => m_Runtime;
64138

65139
public InputMetrics metrics
66140
{
@@ -90,14 +164,17 @@ public InputSettings settings
90164
{
91165
get
92166
{
93-
Debug.Assert(m_Settings != null);
94167
return m_Settings;
95168
}
96169
set
97170
{
98171
if (value == null)
99172
throw new ArgumentNullException(nameof(value));
100173

174+
// Delete the "temporary" settings if necessary
175+
if (m_Settings != null && m_Settings.hideFlags == HideFlags.HideAndDontSave)
176+
ScriptableObject.DestroyImmediate(m_Settings);
177+
101178
if (m_Settings == value)
102179
return;
103180

@@ -1768,45 +1845,6 @@ public void Update(InputUpdateType updateType)
17681845
m_Runtime.Update(updateType);
17691846
}
17701847

1771-
internal void Initialize(IInputRuntime runtime, InputSettings settings)
1772-
{
1773-
Debug.Assert(settings != null);
1774-
1775-
m_Settings = settings;
1776-
1777-
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
1778-
InitializeActions();
1779-
#endif // UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
1780-
InitializeData();
1781-
InstallRuntime(runtime);
1782-
InstallGlobals();
1783-
1784-
ApplySettings();
1785-
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
1786-
ApplyActions();
1787-
#endif
1788-
}
1789-
1790-
internal void Destroy()
1791-
{
1792-
// There isn't really much of a point in removing devices but we still
1793-
// want to clear out any global state they may be keeping. So just tell
1794-
// the devices that they got removed without actually removing them.
1795-
for (var i = 0; i < m_DevicesCount; ++i)
1796-
m_Devices[i].NotifyRemoved();
1797-
1798-
// Free all state memory.
1799-
m_StateBuffers.FreeAll();
1800-
1801-
// Uninstall globals.
1802-
UninstallGlobals();
1803-
1804-
// Destroy settings if they are temporary.
1805-
if (m_Settings != null && m_Settings.hideFlags == HideFlags.HideAndDontSave)
1806-
Object.DestroyImmediate(m_Settings);
1807-
1808-
// Project-wide Actions are never temporary so we do not destroy them.
1809-
}
18101848

18111849
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
18121850
// Initialize project-wide actions:
@@ -2112,12 +2150,12 @@ internal struct AvailableDevice
21122150
private bool m_HaveSentStartupAnalytics;
21132151
#endif
21142152

2115-
internal IInputRuntime m_Runtime;
2116-
internal InputMetrics m_Metrics;
2117-
internal InputSettings m_Settings;
2118-
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
2153+
private IInputRuntime m_Runtime;
2154+
private InputMetrics m_Metrics;
2155+
private InputSettings m_Settings;
2156+
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
21192157
private InputActionAsset m_Actions;
2120-
#endif
2158+
#endif
21212159

21222160
#if UNITY_EDITOR
21232161
internal IInputDiagnostics m_Diagnostics;
@@ -2563,6 +2601,8 @@ private void OnBeforeUpdate(InputUpdateType updateType)
25632601
/// </summary>
25642602
internal void ApplySettings()
25652603
{
2604+
Debug.Assert(m_Settings != null);
2605+
25662606
// Sync update mask.
25672607
var newUpdateMask = InputUpdateType.Editor;
25682608
if ((m_UpdateMask & InputUpdateType.BeforeRender) != 0)
@@ -3865,8 +3905,16 @@ internal void RestoreStateWithoutDevices(SerializedState state)
38653905
m_Metrics = state.metrics;
38663906
m_PollingFrequency = state.pollingFrequency;
38673907

3868-
if (m_Settings != null)
3908+
// Cached settings might be null if the ScriptableObject was destroyed; create new default instance in this case.
3909+
if (state.settings == null)
3910+
{
3911+
state.settings = ScriptableObject.CreateInstance<InputSettings>();
3912+
state.settings.hideFlags = HideFlags.HideAndDontSave;
3913+
}
3914+
3915+
if (m_Settings != null && m_Settings != state.settings)
38693916
Object.DestroyImmediate(m_Settings);
3917+
38703918
m_Settings = state.settings;
38713919

38723920
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
@@ -4024,7 +4072,6 @@ private bool RestoreDeviceFromSavedState(ref DeviceState deviceState, InternedSt
40244072

40254073
return true;
40264074
}
4027-
40284075
#endif // UNITY_EDITOR || DEVELOPMENT_BUILD
40294076
}
40304077
}

0 commit comments

Comments
 (0)