FIX: Input System actions not auto-saving in Project Settings when both related windows were open

[josepmariapujol-unity]

Description

This PR fixes the problem of input system actions not being saved when ProjectSettings window is opened together with InputSystem Actions window.

Related post on forums

JIRA: UUM-134035

Before:

Before.mov

After:

After.mov

Testing status & QA

Double-check what is shown in the video works.

Overall Product Risks

Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any.

• Complexity: 1
• Halo Effect: 1

Comments to reviewers

Please describe any additional information such as what to focus on, or historical info for the 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]

PR Reviewer Guide 🔍

(Review updated until commit 5c2eab5)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 UUM-134035 - Partially compliant Compliant requirements: Newly created Action Maps should be saved as sub-assets when closing the Project Settings window after editing Project-wide Input Actions. Non-compliant requirements: Behavior should match the expected auto-save behavior seen in versions where the issue is not reproducible (e.g., 1.17.0). Requires further human verification: Verify via the provided repro steps that closing the Project Settings window now persists newly created Action Maps as sub-assets (including the “both windows open” scenario mentioned in the PR description). Verify expected behavior when InputEditorUserSettings.autoSaveInputActionAssets is disabled (no unintended saving).

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ Small diff, but it changes save-gating behavior and needs quick validation for regressions around the auto-save setting and window/focus interactions.

🏅 Score: 72 The change is minimal and likely fixes the reported scenario, but it appears to remove honoring the user auto-save setting, which could introduce an unintended behavior change.

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Behavior Change The early-return no longer checks InputEditorUserSettings.autoSaveInputActionAssets, which may cause assets to be saved even when auto-save is disabled; confirm that this is intended for the Project Settings workflow and doesn’t regress user expectations elsewhere. private void ValidateAndSaveAsset(InputActionAsset asset) { // This code should be cleaned up once we migrate the InputControl stuff from ImGUI completely. // Since at that point it stops being a separate window that steals focus. // (See case ISXB-1713) if (m_View.IsControlSchemeViewActive()) { return; }

Update review

---

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

[u-pr]

PR Code Suggestions ✨

Latest suggestions up to 5c2eab5

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Preserve the auto-save toggle This change makes ValidateAndSaveAsset save even when the user has disabled auto-save, which is a behavior regression and can lead to unintended asset writes. Keep honoring InputEditorUserSettings.autoSaveInputActionAssets while still short-circuiting when the control scheme view is active. Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorSettingsProvider.cs [181-184] -if (m_View.IsControlSchemeViewActive()) +if (!InputEditorUserSettings.autoSaveInputActionAssets || m_View.IsControlSchemeViewActive()) { return; } Suggestion importance[1-10]: 8 __ Why: The PR removes the check for InputEditorUserSettings.autoSaveInputActionAssets, which means the editor will now auto-save even if the user has explicitly disabled that setting. This is a regression in user-facing behavior, as user preferences should be respected.	Medium
More suggestions

---

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

---

Previous suggestions

Suggestions up to commit 7e2617c

Category	Suggestion	Impact
Possible issue	Guard against null path/asset Add guards for asset == null and string.IsNullOrEmpty(path) to avoid NullReferenceException (e.g., when the asset is deleted/unloaded while the UI is open). Also avoid calling AssetDatabase.GetAssetPath in the comparison when path is null/empty and compare using StringComparison.Ordinal for consistent behavior. Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs [295-302] internal static bool Save(string path, InputActionAsset asset) { + if (string.IsNullOrEmpty(path) || asset == null) + return false; + var projectWideActions = InputSystem.actions; - if (projectWideActions != null && path == AssetDatabase.GetAssetPath(projectWideActions)) + var projectWidePath = projectWideActions != null ? AssetDatabase.GetAssetPath(projectWideActions) : null; + if (!string.IsNullOrEmpty(projectWidePath) && string.Equals(path, projectWidePath, StringComparison.Ordinal)) ProjectWideActionsAsset.Verify(asset); return InputActionAssetManager.SaveAsset(path, asset.ToJson()); } Suggestion importance[1-10]: 6 __ Why: The suggestion adds necessary null guards for asset and path in the internal static Save method. This prevents a potential NullReferenceException when calling asset.ToJson() and improves the method's robustness for general usage.	Low

Suggestions up to commit 826e5a2

Category	Suggestion	Impact
Possible issue	Allow manual saves unconditionally The control-schemes view guard makes sense for auto-save (focus-stealing), but it will also block an explicit user-initiated save. Remove the IsControlSchemeViewActive() early return here so the user can always save from the toolbar. Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorSettingsProvider.cs [190-199] private void OnSaveAssetRequested() { var asset = GetAsset(); if (asset == null) return; - if (m_View != null && m_View.IsControlSchemeViewActive()) - return; + ProjectWideActionsAsset.Verify(asset); EditorHelpers.SaveAsset(AssetDatabase.GetAssetPath(asset), asset.ToJson()); } Suggestion importance[1-10]: 7 __ Why: The m_View.IsControlSchemeViewActive() check is a workaround for ImGUI focus issues during auto-save. Including it in OnSaveAssetRequested (triggered by a manual user action) can lead to a confusing UX where the Save button click is ignored without any feedback.	Medium

[u-pr]

Persistent review updated to latest commit 7e2617c

[codecov-github-com]

Codecov Report

Attention: Patch coverage is 46.15385% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...Editor/UITKAssetEditor/InputActionsEditorWindow.cs	50.00%	6 Missing ⚠️
...KAssetEditor/InputActionsEditorSettingsProvider.cs	0.00%	1 Missing ⚠️

@@             Coverage Diff             @@
##           develop    #2362      +/-   ##
===========================================
- Coverage    77.90%   77.89%   -0.02%     
===========================================
  Files          476      476              
  Lines        97613    97643      +30     
===========================================
+ Hits         76048    76055       +7     
- Misses       21565    21588      +23     

Flag	Coverage Δ
inputsystem_MacOS_2022.3	5.52% <0.00%> (-0.01%)	⬇️
inputsystem_MacOS_2022.3_project	75.38% <0.00%> (-0.02%)	⬇️
inputsystem_MacOS_6000.0	5.30% <0.00%> (-0.01%)	⬇️
inputsystem_MacOS_6000.0_project	77.29% <46.15%> (-0.01%)	⬇️
inputsystem_MacOS_6000.3	5.30% <0.00%> (-0.01%)	⬇️
inputsystem_MacOS_6000.3_project	77.28% <46.15%> (-0.01%)	⬇️
inputsystem_MacOS_6000.4	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_MacOS_6000.4_project	77.30% <46.15%> (-0.01%)	⬇️
inputsystem_MacOS_6000.5	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_MacOS_6000.5_project	77.29% <46.15%> (-0.01%)	⬇️
inputsystem_MacOS_6000.6	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_MacOS_6000.6_project	77.29% <46.15%> (-0.01%)	⬇️
inputsystem_Ubuntu_2022.3_project	75.18% <0.00%> (-0.02%)	⬇️
inputsystem_Ubuntu_6000.0	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_Ubuntu_6000.0_project	77.09% <46.15%> (+<0.01%)	⬆️
inputsystem_Ubuntu_6000.3	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_Ubuntu_6000.3_project	77.09% <46.15%> (-0.02%)	⬇️
inputsystem_Ubuntu_6000.4	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_Ubuntu_6000.4_project	77.10% <46.15%> (-0.02%)	⬇️
inputsystem_Ubuntu_6000.5	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_Ubuntu_6000.5_project	77.09% <46.15%> (-0.02%)	⬇️
inputsystem_Ubuntu_6000.6	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_Ubuntu_6000.6_project	77.09% <46.15%> (-0.02%)	⬇️
inputsystem_Windows_2022.3	5.52% <0.00%> (-0.01%)	⬇️
inputsystem_Windows_2022.3_project	75.51% <0.00%> (-0.02%)	⬇️
inputsystem_Windows_6000.0	5.30% <0.00%> (-0.01%)	⬇️
inputsystem_Windows_6000.0_project	77.42% <46.15%> (-0.01%)	⬇️
inputsystem_Windows_6000.3	5.30% <0.00%> (-0.01%)	⬇️
inputsystem_Windows_6000.3_project	77.41% <46.15%> (-0.01%)	⬇️
inputsystem_Windows_6000.4	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_Windows_6000.4_project	77.42% <46.15%> (-0.01%)	⬇️
inputsystem_Windows_6000.5	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_Windows_6000.5_project	77.42% <46.15%> (-0.01%)	⬇️
inputsystem_Windows_6000.6	5.31% <0.00%> (-0.01%)	⬇️
inputsystem_Windows_6000.6_project	77.42% <46.15%> (-0.01%)	⬇️

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

Files with missing lines	Coverage Δ
...KAssetEditor/InputActionsEditorSettingsProvider.cs	0.00% <0.00%> (ø)
...Editor/UITKAssetEditor/InputActionsEditorWindow.cs	54.52% <50.00%> (-0.13%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[josepmariapujol-unity]

This could be probably the wrong approach, since I have just realized that it is a regression from PR

[u-pr]

Persistent review updated to latest commit 5c2eab5

[MorganHoarau]

It's a minor change and I don't want to block the minor change but I have a few interrogations.

[MorganHoarau]

Double checking my understanding of the repo, so currently tested scenario is:

1. Open Project settings > Input System project wide input actions window (PWIA)
2. Open the InputActionAsset used in PWIA
3. Add new ActionMap to the PWIA window
4. Exit PWIA focus
   Expected: InputActionAsset reflect the change from the PWIA auto save.

What would happen given the opposite scenario?

1. Open Project settings > Input System project wide input actions window (PWIA)
2. Open the InputActionAsset used in PWIA
3. Disable auto save in the InputActionAsset window
4. Add new ActionMap to the InputAction window
5. Without saving, focus PWIA window. (Alt: make a small change (add a control to an existing input action))
6. Focus back InputAction window

Expected: I assume that this should do nothing as the instance of PWIA is not dirty and skip auto save, but might be worth a quick try.

Alt Expected:
My assumption is that the PWIA is now auto saving and gaining priority over any unsaved work you do in the other window. This sounds like expected behaviour, but:

• could that count as a regression for some users?
• should we update documentation to reflect the expected behaviour?
• should InputActionAsset used in PWIA be readonly? (with maybe a helpbox to align with PlayerInput behaviour?)