FIX: ISXB-1674 - Input actions asset not converted correctly when upgrading from 1.14.1 (Regression) #2244

AswinRajGopal · 2025-09-25T11:03:56Z

Description

Bug: https://jira.unity3d.com/browse/ISXB-1674
Port: 1.14.X

Preserve original tokens and only touch the enum values. That means processor names, order, legacy processors, and formatting survive unchanged so the editor no longer collapses or marks them “Obsolete”.
Avoid whole string rewrites, which previously caused mismatches between what the editor expected and what was written back and only assign a new processor string if something actually changed.

Testing status & QA

Verified manually with the attached repro project.

Overall Product Risks

Complexity: 0
Halo Effect: 0

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.

After merge:

Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

codecov-github-com · 2025-09-25T11:30:29Z

Codecov Report

Attention: Patch coverage is 92.06349% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...UITKAssetEditor/Views/NameAndParametersListView.cs	25.00%	3 Missing ⚠️
...nputsystem/InputSystem/Actions/InputActionAsset.cs	96.77%	1 Missing ⚠️
...tsystem/InputSystem/Utilities/NameAndParameters.cs	87.50%	1 Missing ⚠️

@@ Coverage Diff @@ ## develop #2244 +/- ## =========================================== + Coverage 68.14% 76.74% +8.60%  =========================================== Files 367 465 +98 Lines 53685 87951 +34266 =========================================== + Hits 36584 67502 +30918  - Misses 17101 20449 +3348

Flag	Coverage Δ
inputsystem_MacOS_2021.3	`5.91% <0.00%> (?)`
inputsystem_MacOS_2021.3_project	`78.04% <10.25%> (?)`
inputsystem_MacOS_2022.3	`5.37% <0.00%> (?)`
inputsystem_MacOS_2022.3_project	`74.58% <9.30%> (?)`
inputsystem_MacOS_6000.0	`5.18% <0.00%> (?)`
inputsystem_MacOS_6000.0_project	`76.55% <92.06%> (?)`
inputsystem_MacOS_6000.2	`5.18% <0.00%> (?)`
inputsystem_MacOS_6000.2_project	`76.55% <92.06%> (?)`
inputsystem_MacOS_6000.3	`5.18% <0.00%> (?)`
inputsystem_MacOS_6000.3_project	`76.55% <92.06%> (?)`
inputsystem_MacOS_6000.4	`5.18% <0.00%> (?)`
inputsystem_MacOS_6000.4_project	`76.55% <92.06%> (?)`
inputsystem_Ubuntu_2021.3	`5.91% <0.00%> (?)`
inputsystem_Ubuntu_2021.3_project	`77.95% <10.25%> (?)`
inputsystem_Ubuntu_2022.3	`5.37% <0.00%> (?)`
inputsystem_Ubuntu_2022.3_project	`74.38% <9.30%> (?)`
inputsystem_Ubuntu_6000.0	`5.19% <0.00%> (?)`
inputsystem_Ubuntu_6000.0_project	`76.36% <92.06%> (?)`
inputsystem_Ubuntu_6000.2	`5.19% <0.00%> (?)`
inputsystem_Ubuntu_6000.2_project	`76.36% <92.06%> (?)`
inputsystem_Ubuntu_6000.3	`5.19% <0.00%> (?)`
inputsystem_Ubuntu_6000.3_project	`76.36% <92.06%> (?)`
inputsystem_Ubuntu_6000.4	`5.19% <0.00%> (?)`
inputsystem_Ubuntu_6000.4_project	`76.36% <92.06%> (?)`
inputsystem_Windows_2021.3	`5.91% <0.00%> (?)`
inputsystem_Windows_2021.3_project	`78.18% <10.25%> (?)`
inputsystem_Windows_2022.3	`5.37% <0.00%> (?)`
inputsystem_Windows_2022.3_project	`74.72% <9.30%> (?)`
inputsystem_Windows_6000.0	`5.18% <0.00%> (?)`
inputsystem_Windows_6000.0_project	`76.69% <92.06%> (?)`
inputsystem_Windows_6000.2	`5.18% <0.00%> (?)`
inputsystem_Windows_6000.2_project	`76.69% <92.06%> (?)`
inputsystem_Windows_6000.3	`5.18% <0.00%> (?)`
inputsystem_Windows_6000.3_project	`76.69% <92.06%> (?)`
inputsystem_Windows_6000.4	`5.18% <0.00%> (?)`
inputsystem_Windows_6000.4_project	`76.69% <92.06%> (?)`

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

Files with missing lines	Coverage Δ
...ests/InputSystem.Editor/CustomProcessorEnumTest.cs	`85.93% <100.00%> (ø)`
...nputsystem/InputSystem/Actions/InputActionAsset.cs	`84.91% <96.77%> (+10.82%)`	⬆️
...tsystem/InputSystem/Utilities/NameAndParameters.cs	`92.59% <87.50%> (-0.56%)`	⬇️
...UITKAssetEditor/Views/NameAndParametersListView.cs	`65.38% <25.00%> (-0.98%)`	⬇️

... and 98 files with indirect coverage changes

🚀 New features to boost your workflow:

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

ekcoh · 2025-10-02T08:34:38Z

I noticed this PR title was incorrect so I changed it to remove that failure.

ekcoh

Thanks for looking into this, my main comment at this review round is to add inline comments explaining why we need to do the different parts since it is not clear to me from the code. I think this is important to make sure its maintainable.

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

ekcoh · 2025-10-03T08:32:50Z

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

+ if (mapJson.actions == null || mapJson.actions.Length == 0)
+ continue;
+
+ for (var ai = 0; ai < mapJson.actions.Length; ++ai)


It looks like 'ai' isn't really used so why not e.g foreach instead?

the iterator actionJson is immutable as the ReadActionJson is a struct so we use for loop instead of foreach.

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

ekcoh · 2025-10-03T08:35:33Z

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

+ if (tokens.Count == 0)
+ continue;
+
+ var parsed = new List<NameAndParameters>(tokens.Count);


Also this list may be moved to outer scope to avoid allocating on every iteration.

ekcoh · 2025-10-03T08:35:45Z

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

+ foreach (var t in tokens)
+ parsed.Add(NameAndParameters.Parse(t));
+
+ var rebuiltTokens = new List<string>(tokens.Count);


same with this list

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

LeoUnity

There are a few things you mention in the PR description that I don't understand why they are needed:

Preserve original tokens and only touch the enum values

Formatting survive unchanged
I don't understand why this would be a requirement, I believe that if the user uses our editor to edit the asset that was written manually and has custom formatting that will overwrite the users formatting. Trying to parse json with string replaces, regex is a losing battle, the only way this won't be brittle is by parsing it, converting the data and serializing it back to json.

Avoid whole string rewrites, which previously caused mismatches between what the editor expected
As above, I don't understand why do we want to avoid string rewrites, my current thinking is that we didnt catch this for 2 reasons, we are not reusing the parsing and serializing funcitons that is used elsewhere, and we didnt test this case.

I believe the proper way of moving forward here is that we use the exact same code to parse and serialize json for this type that is used elsewhere in the editor, if we try to do anything different we are creating an opportunity for a bug to be hidden here.

Assets/Tests/InputSystem.Editor/CustomProcessorEnumTest.cs

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

Assets/Tests/InputSystem.Editor/CustomProcessorEnumTest.cs

This reverts commit 00b37a5.

This reverts commit a260324.

Assets/Tests/InputSystem.Editor/CustomProcessorEnumTest.cs

ekcoh

Same comment as @LeoUnity, let's complement the test by making sure it parses correctly (logically) and then we can conclude this I believe.

Packages/com.unity.inputsystem/InputSystem/Utilities/NameAndParameters.cs

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

Packages/com.unity.inputsystem/InputSystem/Utilities/NameAndParameters.cs

.../com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs

LeoUnity

Missing an automated test case that reproduces the issue we are trying to fix here.

Assets/Tests/InputSystem.Editor/CustomProcessorEnumTest.cs

… ListView is editor only.

Assets/Tests/InputSystem.Editor/CustomProcessorEnumTest.cs

… processors and robust way to assert them.

Update the parser for migration

a260324

ekcoh changed the title ~~Regression Fix: ISXB-1674 - Input actions asset not converted correctly when upgrading from 1.14.1~~ FIX: ISXB-1674 - Input actions asset not converted correctly when upgrading from 1.14.1 (Regression) Oct 2, 2025

Added a test to cover the package migration.

00b37a5

AswinRajGopal requested review from LeoUnity and ekcoh October 2, 2025 12:24

ekcoh requested changes Oct 3, 2025

View reviewed changes

LeoUnity reviewed Oct 3, 2025

View reviewed changes

AswinRajGopal added 5 commits October 3, 2025 21:44

Revert "Added a test to cover the package migration."

d637adf

This reverts commit 00b37a5.

Revert "Update the parser for migration"

4e76813

This reverts commit a260324.

The the processors are split by , instead of ; it was typo

a8ba533

Robust handling of migration logic

ff05b1d

Added test to cover the migration

829c4aa

AswinRajGopal requested review from LeoUnity and ekcoh October 6, 2025 09:09

LeoUnity reviewed Oct 6, 2025

View reviewed changes

Assets/Tests/InputSystem.Editor/CustomProcessorEnumTest.cs Outdated Show resolved Hide resolved

ekcoh requested changes Oct 6, 2025

View reviewed changes

ekcoh reviewed Oct 6, 2025

View reviewed changes

Packages/com.unity.inputsystem/InputSystem/Utilities/NameAndParameters.cs Show resolved Hide resolved

ekcoh reviewed Oct 6, 2025

View reviewed changes

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs Outdated Show resolved Hide resolved

ekcoh reviewed Oct 7, 2025

View reviewed changes

Packages/com.unity.inputsystem/InputSystem/Utilities/NameAndParameters.cs Outdated Show resolved Hide resolved

ekcoh reviewed Oct 7, 2025

View reviewed changes

.../com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs Outdated Show resolved Hide resolved

AswinRajGopal added 3 commits October 7, 2025 17:12

Update test

9467adf

Address PR feeback.

3e490f1

Address PR comments.

a31d971

LeoUnity requested changes Oct 7, 2025

View reviewed changes

Assets/Tests/InputSystem.Editor/CustomProcessorEnumTest.cs Show resolved Hide resolved

AswinRajGopal added 2 commits October 8, 2025 00:24

Update the test to include more than one processors in actionmap.

817ff0e

Removed redundant call

2914bf7

AswinRajGopal requested review from LeoUnity and ekcoh October 7, 2025 19:04

Revert changes and moved the function changes to NameAndParameters as…

6daabc9

… ListView is editor only.

LeoUnity reviewed Oct 8, 2025

View reviewed changes

Assets/Tests/InputSystem.Editor/CustomProcessorEnumTest.cs Outdated Show resolved Hide resolved

Bring back the test which verifies the migration along with processed…

6aca9db

… processors and robust way to assert them.

AswinRajGopal requested a review from LeoUnity October 10, 2025 07:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

FIX: ISXB-1674 - Input actions asset not converted correctly when upgrading from 1.14.1 (Regression) #2244

FIX: ISXB-1674 - Input actions asset not converted correctly when upgrading from 1.14.1 (Regression) #2244

AswinRajGopal commented Sep 25, 2025

codecov-github-com bot commented Sep 25, 2025 •

edited

Loading

ekcoh commented Oct 2, 2025

ekcoh left a comment

Uh oh!

ekcoh Oct 3, 2025

AswinRajGopal Oct 7, 2025

Uh oh!

ekcoh Oct 3, 2025

ekcoh Oct 3, 2025

Uh oh!

Uh oh!

Uh oh!

LeoUnity left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ekcoh left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LeoUnity left a comment

Uh oh!

Uh oh!

Labels

3 participants

FIX: ISXB-1674 - Input actions asset not converted correctly when upgrading from 1.14.1 (Regression) #2244

Are you sure you want to change the base?

FIX: ISXB-1674 - Input actions asset not converted correctly when upgrading from 1.14.1 (Regression) #2244

Conversation

AswinRajGopal commented Sep 25, 2025

Description

Testing status & QA

Overall Product Risks

Comments to reviewers

Checklist

codecov-github-com bot commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

ekcoh commented Oct 2, 2025

ekcoh left a comment

Choose a reason for hiding this comment

Uh oh!

ekcoh Oct 3, 2025

Choose a reason for hiding this comment

AswinRajGopal Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

ekcoh Oct 3, 2025

Choose a reason for hiding this comment

ekcoh Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LeoUnity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ekcoh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LeoUnity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Labels

3 participants

codecov-github-com bot commented Sep 25, 2025 •

edited

Loading