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 `0771421`)

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. When the last UI is destroyed and the UI input provider shuts down, unrelated input state outside UI must remain unchanged. Requires further human verification: The fix should cover the play mode repro where the Input Debugger previously showed project-wide actions disappearing after deleting `UIDocument`.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ Small, focused changes with one new test make this PR quick to review, though the action-lifecycle logic needs careful scrutiny.
🏅 Score: 84 The fix is targeted and tested, but the new ownership logic still leaves one concrete action-state leak for disabled project-wide UI maps.
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review State leak If the project-wide `InputSystem.actions` asset contains a `UI` map that starts disabled, this code enables that map during registration but never marks it for restoration on shutdown or action-asset changes. As a result, the provider can leave the user's project-wide `UI` map enabled after UI is destroyed, which changes project-wide input state outside the lifetime of the UI provider. if (m_UIActionMap != null && !m_UIActionMap.enabled) { // Don't take ownership of the UI map lifecycle for project-wide actions — the user manages those. m_ShouldDisableUIActionMapOnUnregister = m_InputActionAsset != InputSystem.actions; m_UIActionMap.Enable();
📜 Additional Context from files Packages/.../InputForUI/InputSystemProvider.cs - Lines 656-694 Shows the implementation of SelectInputActionAsset, which reveals how m_InputActionAsset is set (either to InputSystem.actions or a default action asset). This is crucial for understanding why m_InputActionAsset != InputSystem.actions is used to determine m_ShouldDisableUIActionMapOnUnregister. Packages/.../InputForUI/InputSystemProvider.cs - Lines 77-127 Shows the Initialize, Shutdown, and OnActionsChange methods of InputSystemProvider, which helps verify the lifecycle flow (specifically when SelectInputActionAsset, RegisterActions, and UnregisterActions are called).
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   138560   +39790     
===========================================
+ Hits         77169   109227   +32058     
- Misses       21601    29333    +7732

Flag	Coverage Δ
inputsystem_MacOS_6000.0	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.0_project	`77.33% <100.00%> (+0.02%)`	⬆️
inputsystem_MacOS_6000.3	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.3_project	`77.32% <100.00%> (+0.05%)`	⬆️
inputsystem_MacOS_6000.4	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.4_project	`77.34% <100.00%> (+0.05%)`	⬆️
inputsystem_MacOS_6000.5	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.5_project	`77.37% <100.00%> (+0.05%)`	⬆️
inputsystem_MacOS_6000.6	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.6_project	`77.37% <100.00%> (+0.05%)`	⬆️
inputsystem_Ubuntu_6000.0	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.0_project	`77.24% <100.00%> (+0.02%)`	⬆️
inputsystem_Ubuntu_6000.3	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.3_project	`77.23% <100.00%> (+0.05%)`	⬆️
inputsystem_Ubuntu_6000.4	`5.34% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.4_project	`77.24% <100.00%> (+0.05%)`	⬆️
inputsystem_Ubuntu_6000.5	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.5_project	`77.28% <100.00%> (+0.05%)`	⬆️
inputsystem_Ubuntu_6000.6	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.6_project	`77.28% <100.00%> (+0.05%)`	⬆️
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.45% <100.00%> (+0.05%)`	⬆️
inputsystem_Windows_6000.4	`5.33% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_6000.4_project	`77.46% <100.00%> (+0.05%)`	⬆️
inputsystem_Windows_6000.5	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_6000.5_project	`77.50% <100.00%> (+0.05%)`	⬆️
inputsystem_Windows_6000.6	`5.32% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_6000.6_project	`77.50% <100.00%> (+0.05%)`	⬆️

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	`96.94% <100.00%> (+0.23%)`	⬆️
.../Runtime/Plugins/InputForUI/InputSystemProvider.cs	`86.72% <100.00%> (ø)`

... and 215 files with indirect coverage changes

ℹ️ Need help interpreting these results?

…down

jfreire-unity · 2026-03-06T08:44:42Z

            UnregisterAction(ref m_ScrollWheelAction, OnScrollWheelPerformed);

-            if (m_InputActionAsset != null)
+            if (m_InputActionAsset != null && m_InputActionAsset != InputSystem.actions)


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?

@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.

…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.

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

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

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 0771421)

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!

jfreire-unity Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

AswinRajGopal Mar 9, 2026

Choose a reason for hiding this comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AswinRajGopal commented Feb 17, 2026 •

edited

Loading

u-pr Bot commented Feb 17, 2026 •

edited

Loading

(Review updated until commit `0771421`)

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