Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEPM Refactor - REVIEW ONLY - DO NOT MERGE #1915

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

FEPM Refactor - REVIEW ONLY - DO NOT MERGE #1915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d7fdac4
Implement basic support for FEPM (ISX-1842)
timkeo Jan 23, 2024
7371ea8
Refactor InputManager and tests for cleaner init flows (ISX-1842)
timkeo Jan 26, 2024
b1bf0b8
Refactor ProjectWideActions to fix initialization with FEPM (ISX-1840)
timkeo Jan 31, 2024
c8efa00
Refactor various InputSystem members to improved separation of concer…
timkeo Feb 1, 2024
a58a65a
Fix branch after rebasing with develop (ISX-1840)
timkeo Mar 21, 2024
c534e18
Refactor DeferBindingResolution to remove statics (ISX-1840)
timkeo Apr 5, 2024
e042a1f
Refactor GlobalState usages to re-initialize statics (ISX-1840)
timkeo Apr 10, 2024
d1aba5c
Update Plugin classes to fix static usages (ISX-1840)
timkeo Apr 11, 2024
bc2d23e
Misc refactoring to clean up static fields and other small improvemen…
timkeo Apr 18, 2024
bef9515
Address InputSystem refactor PR feedback
timkeo May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 58 additions & 46 deletions Assets/Tests/InputSystem/CoreTests_Actions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5263,56 +5263,68 @@ public class ModificationCases : IEnumerable
[Preserve]
public ModificationCases() {}

private static readonly Modification[] ModificationAppliesToSingleActionMap =
{
Modification.AddBinding,
Modification.RemoveBinding,
Modification.ModifyBinding,
Modification.ApplyBindingOverride,
Modification.AddAction,
Modification.RemoveAction,
Modification.ChangeBindingMask,
Modification.AddDevice,
Modification.RemoveDevice,
Modification.AddDeviceGlobally,
Modification.RemoveDeviceGlobally,
// Excludes: AddMap, RemoveMap
};

private static readonly Modification[] ModificationAppliesToSingletonAction =
{
Modification.AddBinding,
Modification.RemoveBinding,
Modification.ModifyBinding,
Modification.ApplyBindingOverride,
Modification.AddDeviceGlobally,
Modification.RemoveDeviceGlobally,
};

public IEnumerator GetEnumerator()
{
bool ModificationAppliesToSingletonAction(Modification modification)
// NOTE: This executes *outside* of our test fixture during test discovery.

// We cannot directly create the InputAction objects within GetEnumerator() because the underlying
// asset object might be invalid by the time the tests are actually run.
//
// That is, NUnit TestCases are generated once when the Assembly is loaded and will persist until it's unloaded,
// meaning they'll never be recreated without a Domain Reload. However, since InputActionAsset is a ScriptableObject,
// it could be deleted or otherwise invalidated between test case creation and actual test execution.
//
// So, instead we'll create a delegate to create the Actions object as the parameter for each test case, allowing
// the test case to create an Actions object itself when it actually runs.
{
switch (modification)
var actionsFromAsset = new Func<IInputActionCollection2>(() => new DefaultInputActions().asset);
foreach (var value in Enum.GetValues(typeof(Modification)))
{
case Modification.AddBinding:
case Modification.RemoveBinding:
case Modification.ModifyBinding:
case Modification.ApplyBindingOverride:
case Modification.AddDeviceGlobally:
case Modification.RemoveDeviceGlobally:
return true;
yield return new TestCaseData(value, actionsFromAsset);
}
return false;
}

bool ModificationAppliesToSingleActionMap(Modification modification)
{
switch (modification)
var actionMap = new Func<IInputActionCollection2>(CreateMap);
foreach (var value in Enum.GetValues(typeof(Modification)))
{
case Modification.AddMap:
case Modification.RemoveMap:
return false;
if (ModificationAppliesToSingleActionMap.Contains((Modification)value))
yield return new TestCaseData(value, actionMap);
}
return true;
}

// NOTE: This executes *outside* of our test fixture during test discovery.

// Creates a matrix of all permutations of Modifications combined with assets, maps, and singleton actions.
foreach (var func in new Func<IInputActionCollection2>[] { () => new DefaultInputActions().asset, CreateMap, CreateSingletonAction })
{
var singletonMap = new Func<IInputActionCollection2>(CreateSingletonAction);
foreach (var value in Enum.GetValues(typeof(Modification)))
{
var actions = func();
if (actions is InputActionMap map)
{
if (map.m_SingletonAction != null)
{
if (!ModificationAppliesToSingletonAction((Modification)value))
continue;
}
else if (!ModificationAppliesToSingleActionMap((Modification)value))
{
continue;
}
}

yield return new TestCaseData(value, actions);
if (ModificationAppliesToSingletonAction.Contains((Modification)value))
yield return new TestCaseData(value, singletonMap);
}
}
}
Expand Down Expand Up @@ -5343,14 +5355,14 @@ private InputActionMap CreateSingletonAction()
[Test]
[Category("Actions")]
[TestCaseSource(typeof(ModificationCases))]
public void Actions_CanHandleModification(Modification modification, IInputActionCollection2 actions)
public void Actions_CanHandleModification(Modification modification, Func<IInputActionCollection2> getActions)
{
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
// Exclude project-wide actions from this test
InputSystem.actions?.Disable();
InputActionState.DestroyAllActionMapStates(); // Required for `onActionChange` to report correct number of changes
#endif

var actions = getActions();
var gamepad = InputSystem.AddDevice<Gamepad>();

if (modification == Modification.AddDevice || modification == Modification.RemoveDevice)
Expand Down Expand Up @@ -6080,12 +6092,12 @@ public void Actions_AddingSameProcessorTwice_DoesntImpactUIHideState()
InputSystem.RegisterProcessor<ConstantFloat1TestProcessor>();
Assert.That(InputSystem.TryGetProcessor("ConstantFloat1Test"), Is.Not.EqualTo(null));

bool hide = InputSystem.s_Manager.processors.ShouldHideInUI("ConstantFloat1Test");
bool hide = InputSystem.manager.processors.ShouldHideInUI("ConstantFloat1Test");
Assert.That(hide, Is.EqualTo(false));

InputSystem.RegisterProcessor<ConstantFloat1TestProcessor>();
// Check we haven't caused this to alias with itself and cause it to be hidden in the UI
hide = InputSystem.s_Manager.processors.ShouldHideInUI("ConstantFloat1Test");
hide = InputSystem.manager.processors.ShouldHideInUI("ConstantFloat1Test");
Assert.That(hide, Is.EqualTo(false));
}

Expand Down Expand Up @@ -6731,7 +6743,7 @@ public void Actions_RegisteringExistingInteractionUnderNewName_CreatesAlias()
{
InputSystem.RegisterInteraction<HoldInteraction>("TestTest");

Assert.That(InputSystem.s_Manager.interactions.aliases.Contains(new InternedString("TestTest")));
Assert.That(InputSystem.manager.interactions.aliases.Contains(new InternedString("TestTest")));
}

#endif // UNITY_EDITOR
Expand Down Expand Up @@ -9023,7 +9035,7 @@ public void Actions_RegisteringExistingCompositeUnderNewName_CreatesAlias()
{
InputSystem.RegisterBindingComposite<Vector2Composite>("TestTest");

Assert.That(InputSystem.s_Manager.composites.aliases.Contains(new InternedString("TestTest")));
Assert.That(InputSystem.manager.composites.aliases.Contains(new InternedString("TestTest")));
}

#endif // UNITY_EDITOR
Expand Down Expand Up @@ -11103,7 +11115,7 @@ public void Actions_DisablingAllActions_RemovesAllTheirStateMonitors()

// Not the most elegant test as we reach into internals here but with the
// current API, it's not possible to enumerate monitors from outside.
Assert.That(InputSystem.s_Manager.m_StateChangeMonitors,
Assert.That(InputSystem.manager.m_StateChangeMonitors,
Has.All.Matches(
(InputManager.StateChangeMonitorsForDevice x) => x.memoryRegions.All(r => r.sizeInBits == 0)));
}
Expand Down Expand Up @@ -12123,7 +12135,7 @@ public void Actions_CompositeBindingResetWhenResetDeviceCalledWhileExecutingActi
// Disable the Keyboard while action is being performed.
// This simulates an "OnFocusLost" event occurring while processing the Action, e.g. when switching primary displays or moving the main window
actionPerformed = true;
InputSystem.s_Manager.EnableOrDisableDevice(keyboard.device, false, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);
InputSystem.manager.EnableOrDisableDevice(keyboard.device, false, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);
};

map.Enable();
Expand All @@ -12136,7 +12148,7 @@ public void Actions_CompositeBindingResetWhenResetDeviceCalledWhileExecutingActi
Assert.IsTrue(actionPerformed);

// Re enable the Keyboard (before keys are released) and execute Action again
InputSystem.s_Manager.EnableOrDisableDevice(keyboard.device, true, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);
InputSystem.manager.EnableOrDisableDevice(keyboard.device, true, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);

actionPerformed = false;
Release(keyboard.leftShiftKey);
Expand All @@ -12155,7 +12167,7 @@ public void Actions_CompositeBindingResetWhenResetDeviceCalledWhileExecutingActi
Release(keyboard.f1Key);

// Re enable the Keyboard (after keys are released) and execute Action one more time
InputSystem.s_Manager.EnableOrDisableDevice(keyboard.device, true, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);
InputSystem.manager.EnableOrDisableDevice(keyboard.device, true, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);

Press(keyboard.leftCtrlKey);
Press(keyboard.leftShiftKey);
Expand All @@ -12169,7 +12181,7 @@ public void Actions_CompositeBindingResetWhenResetDeviceCalledWhileExecutingActi
Press(keyboard.f1Key);

// Re enable the Keyboard (before keys are released) and verify Action isn't triggered when Key pressed first
InputSystem.s_Manager.EnableOrDisableDevice(keyboard.device, true, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);
InputSystem.manager.EnableOrDisableDevice(keyboard.device, true, InputManager.DeviceDisableScope.TemporaryWhilePlayerIsInBackground);

Press(keyboard.f1Key);
Press(keyboard.leftCtrlKey);
Expand Down
10 changes: 5 additions & 5 deletions Assets/Tests/InputSystem/CoreTests_Devices.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,11 +572,11 @@ public void Devices_AddingDeviceThatUsesBeforeRenderUpdates_CausesBeforeRenderUp

InputSystem.RegisterLayout(deviceJson);

Assert.That(InputSystem.s_Manager.updateMask & InputUpdateType.BeforeRender, Is.EqualTo((InputUpdateType)0));
Assert.That(InputSystem.manager.updateMask & InputUpdateType.BeforeRender, Is.EqualTo((InputUpdateType)0));

InputSystem.AddDevice("CustomGamepad");

Assert.That(InputSystem.s_Manager.updateMask & InputUpdateType.BeforeRender, Is.EqualTo(InputUpdateType.BeforeRender));
Assert.That(InputSystem.manager.updateMask & InputUpdateType.BeforeRender, Is.EqualTo(InputUpdateType.BeforeRender));
}

[Test]
Expand All @@ -596,15 +596,15 @@ public void Devices_RemovingLastDeviceThatUsesBeforeRenderUpdates_CausesBeforeRe
var device1 = InputSystem.AddDevice("CustomGamepad");
var device2 = InputSystem.AddDevice("CustomGamepad");

Assert.That(InputSystem.s_Manager.updateMask & InputUpdateType.BeforeRender, Is.EqualTo(InputUpdateType.BeforeRender));
Assert.That(InputSystem.manager.updateMask & InputUpdateType.BeforeRender, Is.EqualTo(InputUpdateType.BeforeRender));

InputSystem.RemoveDevice(device1);

Assert.That(InputSystem.s_Manager.updateMask & InputUpdateType.BeforeRender, Is.EqualTo(InputUpdateType.BeforeRender));
Assert.That(InputSystem.manager.updateMask & InputUpdateType.BeforeRender, Is.EqualTo(InputUpdateType.BeforeRender));

InputSystem.RemoveDevice(device2);

Assert.That(InputSystem.s_Manager.updateMask & InputUpdateType.BeforeRender, Is.EqualTo((InputUpdateType)0));
Assert.That(InputSystem.manager.updateMask & InputUpdateType.BeforeRender, Is.EqualTo((InputUpdateType)0));
}

private class TestDeviceReceivingAddAndRemoveNotification : Mouse
Expand Down
40 changes: 21 additions & 19 deletions Assets/Tests/InputSystem/CoreTests_Editor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ public void Editor_CanSaveAndRestoreState()
}.ToJson());
InputSystem.Update();

InputSystem.SaveAndReset();
m_StateManager.SaveAndReset(false, null);

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

InputSystem.Restore();
m_StateManager.Restore();

Assert.That(InputSystem.devices,
Has.Exactly(1).With.Property("layout").EqualTo("MyDevice").And.TypeOf<Gamepad>());
Expand All @@ -165,6 +165,7 @@ public void Editor_CanSaveAndRestoreState()
Assert.That(unsupportedDevices[0].interfaceName, Is.EqualTo("Test"));
}

#if !ENABLE_CORECLR
// onFindLayoutForDevice allows dynamically injecting new layouts into the system that
// are custom-tailored at runtime for the discovered device. Make sure that our domain
// reload can restore these.
Expand Down Expand Up @@ -195,12 +196,12 @@ public void Editor_DomainReload_CanRestoreDevicesBuiltWithDynamicallyGeneratedLa

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

InputSystem.SaveAndReset();
m_StateManager.SaveAndReset(false, null);

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

var state = InputSystem.GetSavedState();
var manager = InputSystem.s_Manager;
var state = m_StateManager.GetSavedState();
var manager = InputSystem.manager;

manager.m_SavedAvailableDevices = state.managerState.availableDevices;
manager.m_SavedDeviceStates = state.managerState.devices;
Expand All @@ -209,7 +210,7 @@ public void Editor_DomainReload_CanRestoreDevicesBuiltWithDynamicallyGeneratedLa

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

InputSystem.Restore();
m_StateManager.Restore();
}

[Test]
Expand All @@ -219,7 +220,7 @@ public void Editor_DomainReload_PreservesUsagesOnDevices()
var device = InputSystem.AddDevice<Gamepad>();
InputSystem.SetDeviceUsage(device, CommonUsages.LeftHand);

SimulateDomainReload();
InputSystem.TestHook_SimulateDomainReload(runtime);

var newDevice = InputSystem.devices[0];

Expand All @@ -239,7 +240,7 @@ public void Editor_DomainReload_PreservesEnabledState()

Assert.That(device.enabled, Is.False);

SimulateDomainReload();
InputSystem.TestHook_SimulateDomainReload(runtime);

var newDevice = InputSystem.devices[0];

Expand All @@ -252,7 +253,7 @@ public void Editor_DomainReload_InputSystemInitializationCausesDevicesToBeRecrea
{
InputSystem.AddDevice<Gamepad>();

SimulateDomainReload();
InputSystem.TestHook_SimulateDomainReload(runtime);

Assert.That(InputSystem.devices, Has.Count.EqualTo(1));
Assert.That(InputSystem.devices[0], Is.TypeOf<Gamepad>());
Expand Down Expand Up @@ -289,7 +290,7 @@ public void Editor_DomainReload_CustomDevicesAreRestoredAsLayoutsBecomeAvailable
InputSystem.RegisterLayout(kLayout);
InputSystem.AddDevice("CustomDevice");

SimulateDomainReload();
InputSystem.TestHook_SimulateDomainReload(runtime);

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

Expand All @@ -310,7 +311,7 @@ public void Editor_DomainReload_RetainsUnsupportedDevices()
});
InputSystem.Update();

SimulateDomainReload();
InputSystem.TestHook_SimulateDomainReload(runtime);

Assert.That(InputSystem.GetUnsupportedDevices(), Has.Count.EqualTo(1));
Assert.That(InputSystem.GetUnsupportedDevices()[0].interfaceName, Is.EqualTo("SomethingUnknown"));
Expand Down Expand Up @@ -345,20 +346,21 @@ public void Editor_DomainReload_CanRemoveDevicesDuringDomainReload()
Assert.That(InputSystem.devices, Has.Count.EqualTo(1));
Assert.That(InputSystem.devices[0], Is.AssignableTo<Keyboard>());
}
#endif // !ENABLE_CORECLR

[Test]
[Category("Editor")]
public void Editor_RestoringStateWillCleanUpEventHooks()
{
InputSystem.SaveAndReset();
m_StateManager.SaveAndReset(false, null);

var receivedOnEvent = 0;
var receivedOnDeviceChange = 0;

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

InputSystem.Restore();
m_StateManager.Restore();

var device = InputSystem.AddDevice("Gamepad");
InputSystem.QueueStateEvent(device, new GamepadState());
Expand All @@ -375,8 +377,8 @@ public void Editor_RestoringStateWillRestoreObjectsOfLayoutBuilder()
var builder = new TestLayoutBuilder {layoutToLoad = "Gamepad"};
InputSystem.RegisterLayoutBuilder(() => builder.DoIt(), "TestLayout");

InputSystem.SaveAndReset();
InputSystem.Restore();
m_StateManager.SaveAndReset(false, null);
m_StateManager.Restore();

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

Expand Down Expand Up @@ -2504,7 +2506,7 @@ public void TODO_Editor_SettingsModifiedInPlayMode_AreRestoredWhenReEnteringEdit
[Category("Editor")]
public void Editor_AlwaysKeepsEditorUpdatesEnabled()
{
Assert.That(InputSystem.s_Manager.updateMask & InputUpdateType.Editor, Is.EqualTo(InputUpdateType.Editor));
Assert.That(InputSystem.manager.updateMask & InputUpdateType.Editor, Is.EqualTo(InputUpdateType.Editor));
}

[Test]
Expand Down Expand Up @@ -2926,15 +2928,15 @@ public void Editor_LeavingPlayMode_DestroysAllActionStates()
action.Enable();

Assert.That(InputActionState.s_GlobalState.globalList.length, Is.EqualTo(1));
Assert.That(InputSystem.s_Manager.m_StateChangeMonitors.Length, Is.GreaterThan(0));
Assert.That(InputSystem.s_Manager.m_StateChangeMonitors[0].count, Is.EqualTo(1));
Assert.That(InputSystem.manager.m_StateChangeMonitors.Length, Is.GreaterThan(0));
Assert.That(InputSystem.manager.m_StateChangeMonitors[0].count, Is.EqualTo(1));

// Exit play mode.
InputSystem.OnPlayModeChange(PlayModeStateChange.ExitingPlayMode);
InputSystem.OnPlayModeChange(PlayModeStateChange.EnteredEditMode);

Assert.That(InputActionState.s_GlobalState.globalList.length, Is.Zero);
Assert.That(InputSystem.s_Manager.m_StateChangeMonitors[0].listeners[0].control, Is.Null); // Won't get removed, just cleared.
Assert.That(InputSystem.manager.m_StateChangeMonitors[0].listeners[0].control, Is.Null); // Won't get removed, just cleared.
}

[Test]
Expand Down
Loading
Loading