Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 32 additions & 1 deletion Assets/Tests/InputSystem/Plugins/InputForUITests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public class InputForUITests : InputTestFixture
readonly List<Event> m_InputForUIEvents = new List<Event>();
private int m_CurrentInputEventToCheck;
InputSystemProvider m_InputSystemProvider;
private bool m_ClearedMockProvider;

private InputActionAsset storedActions;

Expand All @@ -45,6 +46,7 @@ public override void Setup()
{
base.Setup();
m_CurrentInputEventToCheck = 0;
m_ClearedMockProvider = false;

storedActions = InputSystem.actions;

Expand All @@ -59,7 +61,8 @@ public override void Setup()
public override void TearDown()
{
EventProvider.Unsubscribe(InputForUIOnEvent);
EventProvider.ClearMockProvider();
if (!m_ClearedMockProvider)
EventProvider.ClearMockProvider();
m_InputForUIEvents.Clear();

InputSystem.s_Manager.actions = storedActions;
Expand Down Expand Up @@ -92,6 +95,34 @@ public void InputSystemActionAssetIsNotNull()
"Test is invalid since InputSystemProvider actions are not available");
}

[Test]
[Category(kTestCategory)]
public void Shutdown_DoesNotDisableProjectWideActionsAsset()
{
var asset = ScriptableObject.CreateInstance<InputActionAsset>();
var uiMap = new InputActionMap("UI");
uiMap.AddAction("Point", InputActionType.PassThrough, "<Mouse>/position");
uiMap.AddAction("Navigate", InputActionType.PassThrough, "<Gamepad>/leftStick");
uiMap.AddAction("Submit", InputActionType.Button, "<Keyboard>/enter");
uiMap.AddAction("Cancel", InputActionType.Button, "<Keyboard>/escape");
uiMap.AddAction("Click", InputActionType.PassThrough, "<Mouse>/leftButton");
uiMap.AddAction("MiddleClick", InputActionType.PassThrough, "<Mouse>/middleButton");
uiMap.AddAction("RightClick", InputActionType.PassThrough, "<Mouse>/rightButton");
uiMap.AddAction("ScrollWheel", InputActionType.PassThrough, "<Mouse>/scroll");
asset.AddActionMap(uiMap);

InputSystem.s_Manager.actions = asset;

m_InputSystemProvider.Initialize();
Assert.That(asset.enabled, Is.True, "Project-wide actions should be enabled by provider initialization.");

EventProvider.ClearMockProvider();
m_ClearedMockProvider = true;
Assert.That(asset.enabled, Is.True, "Project-wide actions must remain enabled after provider shutdown.");

Object.DestroyImmediate(asset);
}

[Test]
[Category(kTestCategory)]
// Checks that mouse events are ignored when a touch is active.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ void UnregisterActions()
UnregisterAction(ref m_RightClickAction, OnRightClickPerformed);
UnregisterAction(ref m_ScrollWheelAction, OnScrollWheelPerformed);

if (m_InputActionAsset != null)
if (m_InputActionAsset != null && m_InputActionAsset != InputSystem.actions)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually have to disable the actions? I'm not a huge fan of disabling this for a particular asset, like project wide actions. I also think the logic we implemented in the past might not be the most correct.

I believe we should always make sure that the UI action map is enabled when registering the actions. Do you know what's the impact of not disabling this asset for a normal asset?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfreire-unity Thanks for the review, I agree that the current change only addresses the reported bug. I’ll improve the fix so that we explicitly ensure the UI action map is enabled when registering actions, and on unregister we only clean up state owned by the provider instead of disabling shared asset state.

m_InputActionAsset.Disable();
}

Expand Down