Skip to content
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.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;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

low
For better maintainability and consistency with the rest of the test class, consider using the public InputSystem.actions property instead of the internal s_Manager field. This aligns with the access pattern used in the Setup method.

Suggested change
InputSystem.s_Manager.actions = asset;
InputSystem.actions = asset;

🤖 Helpful? 👍/👎

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, this should be done. InputSystem.actions is the public API but tests have access to all kinds of internal APIs so let's avoid it.


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.");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium
Based on the implementation in InputSystemProvider.UnregisterActions, this assertion will likely fail.

If the provider was responsible for enabling the "UI" map (which it will be here since the asset is newly created and its maps are disabled by default), it will disable that map during shutdown. Since no other maps are enabled in this test asset, asset.enabled will return false after the provider is cleared.

🤖 Helpful? 👍/👎


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 @@ -17,6 +17,8 @@ internal class InputSystemProvider : IEventProviderImpl

DefaultInputActions m_DefaultInputActions;
InputActionAsset m_InputActionAsset;
InputActionMap m_UIActionMap;
bool m_ShouldDisableUIActionMapOnUnregister;

// Note that these are plain action references instead since InputActionReference do
// not provide any value when this integration doesn't have any UI. If this integration
Expand Down Expand Up @@ -636,14 +638,11 @@ void RegisterActions()
m_RightClickAction = FindActionAndRegisterCallback(Actions.RightClickAction, OnRightClickPerformed);
m_ScrollWheelAction = FindActionAndRegisterCallback(Actions.ScrollWheelAction, OnScrollWheelPerformed);

// When adding new actions, don't forget to add them to UnregisterActions
if (InputSystem.actions == null)
{
// If we've not loaded a user-created set of actions, just enable the UI actions from our defaults.
m_InputActionAsset.FindActionMap("UI", true).Enable();
}
else
m_InputActionAsset.Enable();
// Only touch the UI map so we don't change the enabled state of unrelated maps.
m_UIActionMap = m_InputActionAsset?.FindActionMap("UI", true);
m_ShouldDisableUIActionMapOnUnregister = m_UIActionMap != null && !m_UIActionMap.enabled;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high
This call will throw an ArgumentException if the m_InputActionAsset does not contain a map named "UI". This is a regression for scenarios where a user provides a custom InputActionAsset (via InputSystem.actions) that uses different map names (e.g., "Default", "GameUI", etc.).

Additionally, if the user's UI actions are located in a map with a different name, only enabling the "UI" map will result in the integration failing to work because the relevant actions will remain disabled. The previous implementation was safer as it enabled the entire asset when a user-provided asset was detected.

Suggested change
m_ShouldDisableUIActionMapOnUnregister = m_UIActionMap != null && !m_UIActionMap.enabled;
m_UIActionMap = m_InputActionAsset?.FindActionMap("UI", false);

🤖 Helpful? 👍/👎

if (m_ShouldDisableUIActionMapOnUnregister)
m_UIActionMap.Enable();
}

void UnregisterAction(ref InputAction action, Action<InputAction.CallbackContext> callback = null)
Expand All @@ -664,8 +663,11 @@ void UnregisterActions()
UnregisterAction(ref m_RightClickAction, OnRightClickPerformed);
UnregisterAction(ref m_ScrollWheelAction, OnScrollWheelPerformed);

if (m_InputActionAsset != null)
m_InputActionAsset.Disable();
if (m_ShouldDisableUIActionMapOnUnregister && m_UIActionMap != null)
m_UIActionMap.Disable();

m_UIActionMap = null;
m_ShouldDisableUIActionMapOnUnregister = false;
}

void SelectInputActionAsset()
Expand Down
Loading