Skip to content

Commit 7371ea8

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 d7fdac4 commit 7371ea8

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
@@ -16,6 +16,8 @@
1616

1717
#if UNITY_EDITOR
1818
using UnityEngine.InputSystem.Editor;
19+
using UnityEngine.Tilemaps;
20+
1921
#endif
2022

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

62135
public TypeTable processors => m_Processors;
63136
public TypeTable interactions => m_Interactions;
64137
public TypeTable composites => m_Composites;
138+
internal IInputRuntime runtime => m_Runtime;
65139

66140
public InputMetrics metrics
67141
{
@@ -91,14 +165,17 @@ public InputSettings settings
91165
{
92166
get
93167
{
94-
Debug.Assert(m_Settings != null);
95168
return m_Settings;
96169
}
97170
set
98171
{
99172
if (value == null)
100173
throw new ArgumentNullException(nameof(value));
101174

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

@@ -1769,45 +1846,6 @@ public void Update(InputUpdateType updateType)
17691846
m_Runtime.Update(updateType);
17701847
}
17711848

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

18121850
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
18131851
// Initialize project-wide actions:
@@ -2113,12 +2151,12 @@ internal struct AvailableDevice
21132151
private bool m_HaveSentStartupAnalytics;
21142152
#endif
21152153

2116-
internal IInputRuntime m_Runtime;
2117-
internal InputMetrics m_Metrics;
2118-
internal InputSettings m_Settings;
2119-
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
2154+
private IInputRuntime m_Runtime;
2155+
private InputMetrics m_Metrics;
2156+
private InputSettings m_Settings;
2157+
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
21202158
private InputActionAsset m_Actions;
2121-
#endif
2159+
#endif
21222160

21232161
#if UNITY_EDITOR
21242162
internal IInputDiagnostics m_Diagnostics;
@@ -2564,6 +2602,8 @@ private void OnBeforeUpdate(InputUpdateType updateType)
25642602
/// </summary>
25652603
internal void ApplySettings()
25662604
{
2605+
Debug.Assert(m_Settings != null);
2606+
25672607
// Sync update mask.
25682608
var newUpdateMask = InputUpdateType.Editor;
25692609
if ((m_UpdateMask & InputUpdateType.BeforeRender) != 0)
@@ -3917,8 +3957,16 @@ internal void RestoreStateWithoutDevices(SerializedState state)
39173957
m_Metrics = state.metrics;
39183958
m_PollingFrequency = state.pollingFrequency;
39193959

3920-
if (m_Settings != null)
3960+
// Cached settings might be null if the ScriptableObject was destroyed; create new default instance in this case.
3961+
if (state.settings == null)
3962+
{
3963+
state.settings = ScriptableObject.CreateInstance<InputSettings>();
3964+
state.settings.hideFlags = HideFlags.HideAndDontSave;
3965+
}
3966+
3967+
if (m_Settings != null && m_Settings != state.settings)
39213968
Object.DestroyImmediate(m_Settings);
3969+
39223970
m_Settings = state.settings;
39233971

39243972
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
@@ -4076,7 +4124,6 @@ private bool RestoreDeviceFromSavedState(ref DeviceState deviceState, InternedSt
40764124

40774125
return true;
40784126
}
4079-
40804127
#endif // UNITY_EDITOR || DEVELOPMENT_BUILD
40814128
}
40824129
}

0 commit comments

Comments
 (0)