Skip to content

Commit 6610131

Browse files
FIX: Avoid disabling project-wide actions on shutdown (#2346)
Co-authored-by: Rita Merkl <127492464+ritamerkl@users.noreply.github.com>
1 parent 38d27d4 commit 6610131

3 files changed

Lines changed: 115 additions & 9 deletions

File tree

Assets/Tests/InputSystem/Plugins/InputForUITests.cs

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public class InputForUITests : InputTestFixture
3737
readonly List<Event> m_InputForUIEvents = new List<Event>();
3838
private int m_CurrentInputEventToCheck;
3939
InputSystemProvider m_InputSystemProvider;
40+
private bool m_ClearedMockProvider;
4041

4142
private InputActionAsset storedActions;
4243

@@ -45,6 +46,7 @@ public override void Setup()
4546
{
4647
base.Setup();
4748
m_CurrentInputEventToCheck = 0;
49+
m_ClearedMockProvider = false;
4850

4951
storedActions = InputSystem.actions;
5052

@@ -59,9 +61,11 @@ public override void Setup()
5961
public override void TearDown()
6062
{
6163
EventProvider.Unsubscribe(InputForUIOnEvent);
62-
EventProvider.ClearMockProvider();
64+
if (!m_ClearedMockProvider)
65+
EventProvider.ClearMockProvider();
6366
m_InputForUIEvents.Clear();
6467

68+
// InputSystem.actions setter throws in play mode, so we use the internal manager property here.
6569
InputSystem.manager.actions = storedActions;
6670

6771
#if UNITY_EDITOR
@@ -92,6 +96,98 @@ public void InputSystemActionAssetIsNotNull()
9296
"Test is invalid since InputSystemProvider actions are not available");
9397
}
9498

99+
// Creates a minimal project-wide asset recognised by SelectInputActionAsset().
100+
// At least one action is required: InputActionMap.enabled is m_EnabledActionsCount > 0,
101+
// so an empty map can never report as enabled.
102+
static InputActionAsset CreateProjectWideAssetWithUIMap(out InputActionMap uiMap)
103+
{
104+
var asset = ScriptableObject.CreateInstance<InputActionAsset>();
105+
uiMap = new InputActionMap("UI");
106+
uiMap.AddAction("Point", InputActionType.PassThrough, "<Mouse>/position");
107+
asset.AddActionMap(uiMap);
108+
return asset;
109+
}
110+
111+
[Test]
112+
[Category(kTestCategory)]
113+
public void Shutdown_DoesNotDisableProjectWideActionsAsset()
114+
{
115+
var asset = CreateProjectWideAssetWithUIMap(out var uiMap);
116+
117+
// A non-UI map the user has enabled — provider must never touch it.
118+
var gameplayMap = new InputActionMap("Gameplay");
119+
gameplayMap.AddAction("Jump", InputActionType.Button, "<Keyboard>/space");
120+
asset.AddActionMap(gameplayMap);
121+
gameplayMap.Enable(); // Enable after all maps are added; modifying the asset while any map is enabled is not allowed.
122+
123+
// InputSystem.actions setter throws in play mode, so we use the internal manager property here.
124+
InputSystem.manager.actions = asset;
125+
try
126+
{
127+
m_InputSystemProvider.OnActionsChange();
128+
Assert.That(uiMap.enabled, Is.True, "UI action map should be enabled by provider initialization.");
129+
Assert.That(gameplayMap.enabled, Is.True, "Provider must not change enabled state of non-UI maps.");
130+
131+
EventProvider.ClearMockProvider();
132+
m_ClearedMockProvider = true;
133+
134+
Assert.That(uiMap.enabled, Is.True, "Provider must not disable the UI map in a project-wide asset on shutdown.");
135+
Assert.That(gameplayMap.enabled, Is.True, "Provider must not disable non-UI maps on shutdown.");
136+
}
137+
finally
138+
{
139+
Object.DestroyImmediate(asset);
140+
}
141+
}
142+
143+
[Test]
144+
[Category(kTestCategory)]
145+
public void Shutdown_DoesNotDisableProjectWideUIMap_WhenAlreadyEnabledBeforeInit()
146+
{
147+
var asset = CreateProjectWideAssetWithUIMap(out var uiMap);
148+
uiMap.Enable(); // User had the UI map enabled before the provider started.
149+
150+
// InputSystem.actions setter throws in play mode, so we use the internal manager property here.
151+
InputSystem.manager.actions = asset;
152+
try
153+
{
154+
m_InputSystemProvider.OnActionsChange();
155+
Assert.That(uiMap.enabled, Is.True, "UI action map should remain enabled after provider initialization.");
156+
157+
EventProvider.ClearMockProvider();
158+
m_ClearedMockProvider = true;
159+
160+
// The provider did not enable the map, so it must not disable it on shutdown.
161+
Assert.That(uiMap.enabled, Is.True, "UI action map must remain enabled after provider shutdown when the user had it enabled before initialization.");
162+
}
163+
finally
164+
{
165+
Object.DestroyImmediate(asset);
166+
}
167+
}
168+
169+
[Test]
170+
[Category(kTestCategory)]
171+
public void Shutdown_DisablesUIActionMap_ForProviderOwnedAsset()
172+
{
173+
InputActionMap capturedUIMap = null;
174+
InputSystemProvider.SetOnRegisterActions(asset =>
175+
capturedUIMap = asset?.FindActionMap("UI", false));
176+
177+
// Remove project-wide actions so the provider falls back to its own internal default asset.
178+
// InputSystem.actions setter throws in play mode, so we use the internal manager property here.
179+
InputSystem.manager.actions = null;
180+
m_InputSystemProvider.OnActionsChange();
181+
InputSystemProvider.SetOnRegisterActions(null);
182+
183+
Assert.That(capturedUIMap, Is.Not.Null, "Provider should have a UI action map in its internal default asset.");
184+
Assert.That(capturedUIMap.enabled, Is.True, "UI action map should be enabled by provider initialization.");
185+
186+
EventProvider.ClearMockProvider();
187+
m_ClearedMockProvider = true;
188+
Assert.That(capturedUIMap.enabled, Is.False, "UI action map should be disabled after provider shutdown for provider-owned assets.");
189+
}
190+
95191
[Test]
96192
[Category(kTestCategory)]
97193
// Checks that mouse events are ignored when a touch is active.

Packages/com.unity.inputsystem/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
99

1010
### Fixed
1111

12+
- Fixed `InputSystemProvider` disabling project-wide actions on shutdown when UI Toolkit destroys its objects mid-play. The provider now scopes its lifecycle to the UI action map only and does not disable project-wide actions [UUM-134130](https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-134130)
1213
- Fixed `InputActionRebindingExtensions.GetBindingDisplayString(InputAction, InputBinding, ...)` returning an empty string for composite bindings when the binding mask filters by group [UUM-141423](https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-141423)
1314
- Fixed `InputEventPtr.handled` not preventing actions from triggering when switching devices. The default event handled policy has been changed from `SuppressStateUpdates` (now deprecated) to `SuppressActionEventNotifications`, which keeps device state synchronized while suppressing action callbacks for handled events. [ISXB-1097](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1097)
1415
- Fixed all `InputAction.WasXxxThisFrame()` and `WasXxxThisDynamicUpdate()` polling APIs to use per-action suppression tracking instead of a map-wide flag. Previously, when multiple events arrived in the same frame with mixed handled/unhandled states, the last event's suppression state could incorrectly affect all actions in the map. [ISXB-1097](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1097)

Packages/com.unity.inputsystem/InputSystem/Runtime/Plugins/InputForUI/InputSystemProvider.cs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ internal class InputSystemProvider : IEventProviderImpl
1717

1818
DefaultInputActions m_DefaultInputActions;
1919
InputActionAsset m_InputActionAsset;
20+
InputActionMap m_UIActionMap;
21+
bool m_ShouldDisableUIActionMapOnUnregister;
2022

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

639-
// When adding new actions, don't forget to add them to UnregisterActions
640-
if (InputSystem.actions == null)
641+
// Only touch the UI map so we don't change the enabled state of unrelated maps.
642+
m_UIActionMap = m_InputActionAsset?.FindActionMap("UI", false);
643+
if (m_UIActionMap != null && !m_UIActionMap.enabled)
641644
{
642-
// If we've not loaded a user-created set of actions, just enable the UI actions from our defaults.
643-
m_InputActionAsset.FindActionMap("UI", true).Enable();
645+
m_UIActionMap.Enable();
646+
647+
// For provider-owned assets we are responsible for cleanup on shutdown.
648+
// For project-wide actions the play-mode lifecycle manages the asset, so
649+
// leave it as-is when the provider goes away.
650+
if (m_InputActionAsset != InputSystem.actions)
651+
m_ShouldDisableUIActionMapOnUnregister = true;
644652
}
645-
else
646-
m_InputActionAsset.Enable();
647653
}
648654

649655
void UnregisterAction(ref InputAction action, Action<InputAction.CallbackContext> callback = null)
@@ -664,8 +670,11 @@ void UnregisterActions()
664670
UnregisterAction(ref m_RightClickAction, OnRightClickPerformed);
665671
UnregisterAction(ref m_ScrollWheelAction, OnScrollWheelPerformed);
666672

667-
if (m_InputActionAsset != null)
668-
m_InputActionAsset.Disable();
673+
if (m_ShouldDisableUIActionMapOnUnregister && m_UIActionMap != null)
674+
m_UIActionMap.Disable();
675+
676+
m_UIActionMap = null;
677+
m_ShouldDisableUIActionMapOnUnregister = false;
669678
}
670679

671680
void SelectInputActionAsset()

0 commit comments

Comments
 (0)