Skip to content

FIX: Fixed Input Actions code generation overwriting user files when the names happened to match. #2159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 2, 2025
Merged

FIX: Fixed Input Actions code generation overwriting user files when the names happened to match. #2159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
31d97ad
don't overwrite user files that happened to be called the same as the…
K-Tone Apr 1, 2025
33d35fd
update changelog
K-Tone Apr 1, 2025
6bfafa4
Merge branch 'develop' into anthony/ixsb-1257-input-actions-overwrite
K-Tone Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ however, it has to be formatted properly to pass verification tests.
- Fixed issue where user was not prompted to save changes when loading a second input actions asset into an already opened editor. [ISXB-1343](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1343)
- Fixed the on hover behaviour of the two plus buttons in the Input Actions Editor window [ISXB-1327](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1327)
- Fixed an issue on macOS which didn't detect up-left DPAD presses for Xbox controllers. [ISXB-810](https://issuetracker.unity3d.com/issues/macos-d-pad-upper-left-corner-is-not-logged-with-the-xbox-controller)
- Fixed Input Actions code generation overwriting user files when the names happened to match. [ISXB-1257](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1257)
- Fixed an issue when providing JoinPlayer with a specific split screen index. [ISXB-897](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-897)

## [1.14.0] - 2025-03-20
Expand Down
12 changes: 12 additions & 0 deletions Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,18 @@ private static void GenerateWrapperCode(AssetImportContext ctx, InputActionAsset
if (!Directory.Exists(dir))
Directory.CreateDirectory(dir);

// Check for the case where the target file already exists.
// If it does, we don't want to overwrite it unless it's been generated by us before.
if (File.Exists(wrapperFilePath))
{
var text = File.ReadAllText(wrapperFilePath);
var autoGeneratedMarker = "This code was auto-generated by com.unity.inputsystem";
Copy link
Collaborator

@ritamerkl ritamerkl Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if we can utilise the string the auto generator uses as well, to avoid that this breaks in the case the string would change. I am not sure how much overhead this would entail, I am comfortable with this solution too.

Copy link
Collaborator Author

@K-Tone K-Tone Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fast review!

This is a good point so I've checked code around just a little bit. Appears, the code generator template is declared as a constant inside a static function. It's used in just two spots, both of which specialise it with some extra data. Regardless of the specialization, it's going to include this particular string that we have right now on the PR. It feels immanent to the code logics that we would include the package name as part of it always.

Looking at it, it feels unlikely it's going to be broken any time soon. We would need to add another code generator. I guess, it's indeed more risk in the form of extra code churn if we were to use (portions) of the actual template used by generator.

Copy link
Collaborator

@ritamerkl ritamerkl Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking, that is fine for me!

if (!text.Contains(autoGeneratedMarker))
{
throw new Exception($"The target file for Input Actions code generation already exists: {wrapperFilePath}. Since it doesn't look to contain Input generated code that we can safely overwrite, we stopped to prevent any data loss. Consider renaming. ");
}
}

var options = new InputActionCodeGenerator.Options
{
sourceAssetPath = ctx.assetPath,
Expand Down