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 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ UUM-134130 - Fully compliant Compliant requirements: The PR prevents disabling the `m_InputActionAsset` on shutdown if it matches the global `InputSystem.actions`.
⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪ The PR consists of a single logical check added to one method and a corresponding unit test. The scope is small and well-defined.
🏅 Score: 95 The fix correctly addresses the reported issue by checking against the project-wide actions before disabling. The code is clean, and a regression test is included.
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Test State Cleanup The test modifies the static state `InputSystem.s_Manager.actions` (line 111) but does not explicitly reset it to null or its previous value. Although the asset is destroyed (line 119), leaving a reference to a destroyed object in a static manager could potentially affect subsequent tests depending on the test fixture's teardown logic. 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.Shutdown(); Assert.That(asset.enabled, Is.True, "Project-wide actions must remain enabled after provider shutdown."); Object.DestroyImmediate(asset);
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    77.90%   77.91%            
=========================================
  Files          476      482     +6     
  Lines        97613    97780   +167     
=========================================
+ Hits         76048    76187   +139     
- Misses       21565    21593    +28

Flag	Coverage Δ
inputsystem_MacOS_2022.3_project	`75.35% <ø> (-0.04%)`	⬇️
inputsystem_MacOS_6000.0	`5.27% <0.00%> (-0.04%)`	⬇️
inputsystem_MacOS_6000.0_project	`77.26% <100.00%> (-0.03%)`	⬇️
inputsystem_MacOS_6000.3	`5.27% <0.00%> (-0.04%)`	⬇️
inputsystem_MacOS_6000.3_project	`77.26% <100.00%> (-0.03%)`	⬇️
inputsystem_MacOS_6000.4	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_MacOS_6000.4_project	`77.27% <100.00%> (-0.03%)`	⬇️
inputsystem_MacOS_6000.5	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_MacOS_6000.5_project	`77.27% <100.00%> (-0.04%)`	⬇️
inputsystem_MacOS_6000.6	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_MacOS_6000.6_project	`77.27% <100.00%> (-0.04%)`	⬇️
inputsystem_Ubuntu_2022.3_project	`75.15% <ø> (-0.04%)`	⬇️
inputsystem_Ubuntu_6000.0	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_Ubuntu_6000.0_project	`77.06% <100.00%> (-0.03%)`	⬇️
inputsystem_Ubuntu_6000.3	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_Ubuntu_6000.3_project	`77.06% <100.00%> (-0.04%)`	⬇️
inputsystem_Ubuntu_6000.4	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_Ubuntu_6000.4_project	`77.08% <100.00%> (-0.04%)`	⬇️
inputsystem_Ubuntu_6000.5	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_Ubuntu_6000.5_project	`77.07% <100.00%> (-0.04%)`	⬇️
inputsystem_Ubuntu_6000.6	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_Ubuntu_6000.6_project	`77.07% <100.00%> (-0.04%)`	⬇️
inputsystem_Windows_2022.3	`5.30% <ø> (-0.23%)`	⬇️
inputsystem_Windows_2022.3_project	`75.48% <ø> (-0.05%)`	⬇️
inputsystem_Windows_6000.0	`5.27% <0.00%> (-0.04%)`	⬇️
inputsystem_Windows_6000.0_project	`77.39% <100.00%> (-0.04%)`	⬇️
inputsystem_Windows_6000.3	`5.27% <0.00%> (-0.04%)`	⬇️
inputsystem_Windows_6000.3_project	`77.38% <100.00%> (-0.04%)`	⬇️
inputsystem_Windows_6000.4	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_Windows_6000.4_project	`77.39% <100.00%> (-0.04%)`	⬇️
inputsystem_Windows_6000.5	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_Windows_6000.5_project	`77.39% <100.00%> (-0.04%)`	⬇️
inputsystem_Windows_6000.6	`5.28% <0.00%> (-0.04%)`	⬇️
inputsystem_Windows_6000.6_project	`77.39% <100.00%> (-0.04%)`	⬇️

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.91% <100.00%> (+0.20%)`	⬆️
...utSystem/Plugins/InputForUI/InputSystemProvider.cs	`86.64% <100.00%> (+0.03%)`	⬆️

... and 8 files with indirect coverage changes

ℹ️ Need help interpreting these results?

…down

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

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

            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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: Avoid disabling project-wide actions on shutdown#2346

FIX: Avoid disabling project-wide actions on shutdown#2346
AswinRajGopal wants to merge 6 commits intodevelopfrom
uum-134130/fix-project-wide-action-onshutdown

AswinRajGopal commented Feb 17, 2026 •

edited

Loading

Uh oh!

u-pr bot commented Feb 17, 2026

Uh oh!

u-pr bot commented Feb 17, 2026

Uh oh!

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

edited

Loading

Uh oh!

jfreire-unity Mar 6, 2026

Uh oh!

AswinRajGopal Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

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

PR Reviewer Guide 🔍

Uh oh!

u-pr bot commented Feb 17, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AswinRajGopal commented Feb 17, 2026 •

edited

Loading

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

edited

Loading