FIX: Avoid disabling project-wide actions on shutdown by AswinRajGopal · Pull Request #2346 · Unity-Technologies/InputSystem

AswinRajGopal · 2026-02-17T11:28:56Z

Description

case: https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-134130

Bug:
UI Toolkit now destroys its internal objects when there is no UI left in the scene, so InputSystemProvider.Shutdown() can run during play. During shutdown we unregister the UI actions, but the old code was
managing the whole action asset. With project-wide actions (InputSystem.actions), that meant we could affect input outside UI. With the default internal asset, not cleaning up at all could leave the UI map
enabled longer than intended.

Fix:
Scope the lifecycle to the UI action map only. On registration, InputSystemProvider now makes sure the UI map is enabled. On unregister/shutdown, it disables only that UI map, and only if this provider
enabled it. This avoids changing the enabled state of unrelated maps in InputSystem.actions while still cleaning up the default/internal UI actions correctly.

Testing status & QA

Added new test.
Verified manually using repro project.

Overall Product Risks

Complexity: Low
Halo Effect: Low

Comments to reviewers

Checklist

Before review:

Changelog entry added.
- Explains the change in Changed, Fixed, Added sections.
- For API change contains an example snippet and/or migration example.
- JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
- Jira port for the next release set as "Resolved".
Tests added/changed, if applicable.
- Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
- Performance tests.
- Integration tests.
Docs for new/changed API's.
- Xmldoc cross references are set correctly.
- Added explanation how the API works.
- Usage code examples added.
- The manual is updated, if needed.

During merge:

Commit message for squash-merge is prefixed with one of the list:
- NEW: ___.
- FIX: ___.
- DOCS: ___.
- CHANGE: ___.
- RELEASE: 1.1.0-preview.3.

u-pr · 2026-02-17T11:30:14Z

PR Reviewer Guide 🔍

(Review updated until commit `15e20a1`)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ UUM-134130 - PR Code Verified Compliant requirements: Deleting a `GameObject` with a `UIDocument` component during Play mode must not remove project-wide input actions. The fix should cover the UI Toolkit shutdown path that now runs when no UI is left in the scene. Add or update test coverage for this regression. Requires further human verification: After deleting the `UIDocument` object, the Input Debugger `Actions` list should remain unchanged. Verify the repro steps manually in the Editor, including the stated Windows 11 scenario.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ Small, localized changes with added tests make this fairly quick to review, though the lifecycle edge cases need some careful reasoning.
🏅 Score: 84 The fix is targeted and tested, but it appears to leave stale enabled state behind when switching away from a project-wide action asset at runtime.
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review State leak When the provider enables a previously disabled `UI` map from `InputSystem.actions`, it never marks that map for cleanup because the asset is project-wide. If `InputSystem.actions` is later reassigned or otherwise triggers `OnActionsChange()`, the provider detaches from the old asset without restoring its original disabled state, so the old `UI` map can stay enabled unexpectedly. // For provider-owned assets we are responsible for cleanup on shutdown. // For project-wide actions the play-mode lifecycle manages the asset, so // leave it as-is when the provider goes away. if (m_InputActionAsset != InputSystem.actions) m_ShouldDisableUIActionMapOnUnregister = true;
📜 Additional Context from files Packages/.../InputForUI/InputSystemProvider.cs - Lines 77-127 Provides the necessary context for the Initialize and Shutdown lifecycle of InputSystemProvider, where RegisterActions and UnregisterActions are called, confirming when the UI action map is enabled and disabled. Packages/.../InputForUI/InputSystemProvider.cs - Lines 656-694 Contains SelectInputActionAsset(), which determines whether m_InputActionAsset resolves to InputSystem.actions (project-wide actions) or the default asset, directly validating the condition `m_InputActionAsset != InputSystem.actions` introduced in this PR.
Update review

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

u-pr · 2026-02-17T11:31:38Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category Suggestion Impact

General

Restore global state after test execution

The test modifies the global static state InputSystem.s_Manager.actions without
restoring it, which can cause side effects or failures in subsequent tests. Wrap the
test logic in a try-finally block to ensure that the original state is restored and
the temporary asset is properly destroyed, even if an assertion fails.

Assets/Tests/InputSystem/Plugins/InputForUITests.cs [111-119]

-InputSystem.s_Manager.actions = asset;
+var originalActions = InputSystem.s_Manager.actions;
+try
+{
+    InputSystem.s_Manager.actions = asset;
 
-m_InputSystemProvider.Initialize();
-Assert.That(asset.enabled, Is.True, "Project-wide actions should be enabled by provider initialization.");
+    m_InputSystemProvider.Initialize();
+    Assert.That(asset.enabled, Is.True, "Project-wide actions should be enabled by provider initialization.");
 
-m_InputSystemProvider.Shutdown();
-Assert.That(asset.enabled, Is.True, "Project-wide actions must remain enabled after provider shutdown.");
+    m_InputSystemProvider.Shutdown();
+    Assert.That(asset.enabled, Is.True, "Project-wide actions must remain enabled after provider shutdown.");
+}
+finally
+{
+    InputSystem.s_Manager.actions = originalActions;
+    Object.DestroyImmediate(asset);
+}
 
-Object.DestroyImmediate(asset);
-

Suggestion importance[1-10]: 8

__

Why: The test modifies the global static state InputSystem.s_Manager.actions. Without a try-finally block, an assertion failure would prevent the restoration of the original state and the destruction of the created asset, potentially causing test pollution and side effects in subsequent tests.

Medium

More suggestions

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

codecov-github-com · 2026-02-17T11:38:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

@@             Coverage Diff             @@
##           develop    #2346      +/-   ##
===========================================
+ Coverage    78.13%   78.83%   +0.70%     
===========================================
  Files          483      760     +277     
  Lines        98770   138592   +39822     
===========================================
+ Hits         77169   109257   +32088     
- Misses       21601    29335    +7734

Flag	Coverage Δ
inputsystem_MacOS_6000.0	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.0_project	`77.34% <100.00%> (+0.04%)`	⬆️
inputsystem_MacOS_6000.3	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.3_project	`77.34% <100.00%> (+0.06%)`	⬆️
inputsystem_MacOS_6000.4	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.4_project	`77.35% <100.00%> (+0.06%)`	⬆️
inputsystem_MacOS_6000.5	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.5_project	`77.38% <100.00%> (+0.06%)`	⬆️
inputsystem_MacOS_6000.6	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.6_project	`77.38% <100.00%> (+0.06%)`	⬆️
inputsystem_Ubuntu_6000.0	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.0_project	`77.25% <100.00%> (+0.04%)`	⬆️
inputsystem_Ubuntu_6000.3	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.3_project	`77.24% <100.00%> (+0.06%)`	⬆️
inputsystem_Ubuntu_6000.4	`5.34% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.4_project	`77.25% <100.00%> (+0.06%)`	⬆️
inputsystem_Ubuntu_6000.5	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.5_project	`77.29% <100.00%> (+0.06%)`	⬆️
inputsystem_Ubuntu_6000.6	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.6_project	`77.29% <100.00%> (+0.07%)`	⬆️
inputsystem_Windows_6000.0	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_6000.0_project	`77.46% <100.00%> (+0.03%)`	⬆️
inputsystem_Windows_6000.3	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_6000.3_project	`77.46% <100.00%> (+0.06%)`	⬆️
inputsystem_Windows_6000.4	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_6000.4_project	`77.47% <100.00%> (+0.06%)`	⬆️
inputsystem_Windows_6000.5	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_6000.5_project	`77.51% <100.00%> (+0.06%)`	⬆️
inputsystem_Windows_6000.6	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_6000.6_project	`77.51% <100.00%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...ssets/Tests/InputSystem/Plugins/InputForUITests.cs	`97.18% <100.00%> (+0.48%)`	⬆️
.../Runtime/Plugins/InputForUI/InputSystemProvider.cs	`87.64% <100.00%> (ø)`

... and 215 files with indirect coverage changes

ℹ️ Need help interpreting these results?

…down

Pauliusd01 · 2026-03-30T10:11:53Z

/test_plan

u-pr · 2026-03-30T10:12:19Z

Pauliusd01 · 2026-03-30T10:41:15Z

I'm not entirely sure what the expected outcome should be. Is it expected that most actions including UI remain active when deleting the UI object? What about this one in the screenshot, what makes it special and why is it the only one that gets removed?

AswinRajGopal · 2026-03-30T11:04:32Z

I'm not entirely sure what the expected outcome should be. Is it expected that most actions including UI remain active when deleting the UI object? What about this one in the screenshot, what makes it special and why is it the only one that gets removed?

Hi @Pauliusd01 nextPreviousAction (no Asset) is the exception because it is not part of the project asset. The code creates it temporarily just for this UI provider and removes it again when the provider goes away. That is why it is the only one that disappears. @jfreire-unity Please correct me if I'm wrong.

jfreire-unity · 2026-03-30T12:47:36Z

Sorry I'll try to have a look at this in the next couple of days. Need to "load the context" of this area to make sure we're not introducing any other regression

jfreire-unity · 2026-03-30T12:47:52Z

/crc

u-pr

The review has identified several significant issues that need to be addressed. The current implementation introduces a regression risk with a potential ArgumentException when custom assets are used. Additionally, there is a logic discrepancy in the fix itself that results in disabling portions of the project-wide action asset, which will likely cause the newly added test case to fail. Some minor improvements to test code consistency and maintenance are also recommended.

_{🤖 Helpful? 👍/👎}

AswinRajGopal · 2026-04-13T08:53:50Z

Sorry I'll try to have a look at this in the next couple of days. Need to "load the context" of this area to make sure we're not introducing any other regression

Hi @jfreire-unity could you please review this PR? or can you suggest another reviewers from the team to move it forward? it is been a while on my name. Thanks in advance :)

jfreire-unity

As mentioned in Slack, have a look at the U-PR suggestions to see if they make any sense. And feel free to look if we need to extend the added test to look into the action map that's being disabled.

AswinRajGopal · 2026-05-01T12:30:53Z

Currently working on U-PR comments,

…pr comments.

…down

u-pr · 2026-05-22T07:37:08Z

Persistent review updated to latest commit 0771421

u-pr · 2026-05-22T07:39:31Z

PR Code Suggestions ✨

No code suggestions found for the PR.

ekcoh · 2026-05-26T11:09:55Z

@AswinRajGopal It seems unclear whether the third u-pr feedback comment has been resolved? It has been marked resolved without a comment, but seems referenced code has not changed (as it have for the other 3). I would suggest adding a comment on that u-pr comment why it's not valid or not addressed or whether is incorrect. Then I would update the u-pr review to get a new review round based on current commits.

u-pr · 2026-05-26T16:14:21Z

Persistent review updated to latest commit 0acc808

Pauliusd01 · 2026-05-27T03:45:26Z

FYI I am only free from next week

…map enablement tracking

…own). For project-wide actions the play-mode lifecycle owns the asset, so the provider enables the UI map on init but leaves it as-is when shutting down.

u-pr · 2026-05-28T05:08:17Z

Persistent review updated to latest commit 2b5eadf

u-pr · 2026-05-28T05:22:46Z

Persistent review updated to latest commit 15e20a1

AswinRajGopal · 2026-05-29T04:53:57Z

/crc

u-pr

This review identifies a critical state/event leak issue in the test suite where calling Initialize() twice on the same provider leads to duplicate global event subscriptions.

_{🤖 Helpful? 👍/👎}

…et in tests.

AswinRajGopal · 2026-05-29T05:19:44Z

/ci prv

AswinRajGopal added 2 commits February 17, 2026 16:52

Added test to cover the fix.

c562118

Fix: Avoid disabling project-wide actions on shutdown

0ed73d8

AswinRajGopal requested a review from Pauliusd01 February 17, 2026 11:29

AswinRajGopal removed the request for review from Pauliusd01 February 17, 2026 11:43

AswinRajGopal added 2 commits March 2, 2026 13:44

Merge branch 'develop' into uum-134130/fix-project-wide-action-onshut…

e917c04

…down

Fix test double shutdown call causing issues.

61807cc

AswinRajGopal requested review from Pauliusd01 and jfreire-unity March 3, 2026 11:01

jfreire-unity reviewed Mar 6, 2026

View reviewed changes

Comment thread Packages/com.unity.inputsystem/InputSystem/Plugins/InputForUI/InputSystemProvider.cs Outdated

AswinRajGopal added 2 commits March 23, 2026 15:20

Fix InputForUI action lifecycle to only manage the UI action map.

b087ccd

Merge branch 'develop' into uum-134130/fix-project-wide-action-onshut…

9cdf098

…down

AswinRajGopal requested a review from jfreire-unity March 25, 2026 09:38

Pauliusd01 approved these changes Mar 30, 2026

View reviewed changes

u-pr Bot reviewed Mar 30, 2026

View reviewed changes

jfreire-unity requested changes Apr 14, 2026

View reviewed changes

AswinRajGopal added 2 commits May 20, 2026 02:05

Scope UI action map lifecycle to provider-owned assets and address u-…

7c52dce

…pr comments.

Merge branch 'develop' into uum-134130/fix-project-wide-action-onshut…

0771421

…down

AswinRajGopal requested a review from jfreire-unity May 22, 2026 05:45

Address PR comments and add changelog.

17450d1

AswinRajGopal requested a review from Pauliusd01 May 23, 2026 18:42

jfreire-unity approved these changes May 26, 2026

View reviewed changes

track UI action map enablement independent of asset ownership

0acc808

AswinRajGopal added 5 commits May 27, 2026 10:09

Fix; New Tests; - Correct and extend provider shutdown tests for per-…

20e7f4e

…map enablement tracking

Missed change due to test failure locally.

93e82fb

Fix test failures

c9fbf6a

For provider-owned assets the cleanup still applies (disable on shutd…

dc11f07

…own). For project-wide actions the play-mode lifecycle owns the asset, so the provider enables the UI map on init but leaves it as-is when shutting down.

Update test action asset.

2b5eadf

Clearing the callback in teardown to avoid test pollution.

15e20a1

The teardown cleanup is unnecessary.

3775eb7

AswinRajGopal requested a review from ekcoh May 28, 2026 15:20

u-pr Bot reviewed May 29, 2026

View reviewed changes

Comment thread Assets/Tests/InputSystem/Plugins/InputForUITests.cs Outdated

Use OnActionsChange() instead of Initialize() to re-select action ass…

1478b5b

…et in tests.

Conversation

AswinRajGopal commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing status & QA

Overall Product Risks

Comments to reviewers

Checklist

Uh oh!

u-pr Bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 15e20a1)

Uh oh!

u-pr Bot commented Feb 17, 2026 • edited by AswinRajGopal Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

codecov-github-com Bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Pauliusd01 commented Mar 30, 2026

Uh oh!

u-pr Bot commented Mar 30, 2026 • edited by Pauliusd01 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Plan

Summary of Changes

Risk Assessment

Functional Testing

Regression Testing

Edge Cases

Uh oh!

Pauliusd01 commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AswinRajGopal commented Mar 30, 2026

Uh oh!

jfreire-unity commented Mar 30, 2026

Uh oh!

jfreire-unity commented Mar 30, 2026

Uh oh!

u-pr Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AswinRajGopal commented Apr 13, 2026

Uh oh!

jfreire-unity left a comment

Choose a reason for hiding this comment

Uh oh!

AswinRajGopal commented May 1, 2026

Uh oh!

u-pr Bot commented May 22, 2026

Uh oh!

u-pr Bot commented May 22, 2026

PR Code Suggestions ✨

Uh oh!

ekcoh commented May 26, 2026

Uh oh!

u-pr Bot commented May 26, 2026

Uh oh!

Pauliusd01 commented May 27, 2026

Uh oh!

u-pr Bot commented May 28, 2026

Uh oh!

u-pr Bot commented May 28, 2026

Uh oh!

AswinRajGopal commented May 29, 2026

Uh oh!

u-pr Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AswinRajGopal commented May 29, 2026

Uh oh!

AswinRajGopal commented Feb 17, 2026 •

edited

Loading

u-pr Bot commented Feb 17, 2026 •

edited

Loading

(Review updated until commit `15e20a1`)

u-pr Bot commented Feb 17, 2026 •

edited by AswinRajGopal

Loading

codecov-github-com Bot commented Feb 17, 2026 •

edited

Loading

u-pr Bot commented Mar 30, 2026 •

edited by Pauliusd01

Loading

Pauliusd01 commented Mar 30, 2026 •

edited

Loading