Skip to content

NEW: Added Regenerate Precompiled Layouts for internal FastXXX layouts to QA menu. #2258

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

Jump to bottom
Merged
merged 5 commits into from
Oct 13, 2025
Merged

NEW: Added Regenerate Precompiled Layouts for internal FastXXX layouts to QA menu. #2258

merged 5 commits into from
Oct 13, 2025

Conversation

@ekcoh
Copy link
Collaborator

@ekcoh ekcoh commented Oct 10, 2025 •
edited
Loading

Description

Changes:

  • Added an asmdef for editor assembly in Assets to support internal development tools.
  • Added an internal script regenerating precompiled layouts for Keyboard, Mouse, Touchscreen in their correct place in source tree.
  • Gave internal visibility to non-shipped assembly.

How to use:

  • QA Tools > Regenerate Precompiled Layouts

RegeneratePrecompiled

Testing status & QA

Tried regenerating precompiled layouts.

Overall Product Risks

Minmal

  • Complexity: Small
  • Halo Effect: Small

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.

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.

@u-pr-agent
Copy link
Contributor

u-pr-agent bot commented Oct 10, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

The PR is small and adds a simple editor tool, but its core logic appears to be flawed, requiring careful review.
🏅 Score: 60

The PR introduces a useful internal tool, but its core functionality of regenerating files is not correctly implemented, as it doesn't write the changes to disk.
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Flawed Update Logic

The logic for updating the layout file seems incorrect. The script should first read the existing content, then generate the new code, and only if they differ, write the new code to the file. The current implementation reads the file after generating the code and is missing the step to write the changes to disk (e.g., using File.WriteAllText).

 
    var code = InputLayoutCodeGenerator.GenerateCodeFileForDeviceLayout(layoutName, filePath, prefix: "Fast");
    var existingContent = File.ReadAllText(filePath);
    if (string.Compare(existingContent, code, StringComparison.InvariantCulture) == 0)
    {
        Debug.Log($"Skipped updating precompiled layout: '{filePath}'. No difference.");
        return;
    }
    Debug.Log($"Updated precompiled layout: '{filePath}'. {Path.GetFullPath(filePath)}");
    Debug.Log(code);
}

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

u-pr-agent[bot]
u-pr-agent bot reviewed Oct 10, 2025
View reviewed changes
Packages/com.unity.inputsystem/InputSystem/AssemblyInfo.cs
[assembly: InternalsVisibleTo("Unity.InputSystem.IntegrationTests")]
[assembly: InternalsVisibleTo("Unity.InputSystem.ForUI")] // To avoid minor bump
[assembly: InternalsVisibleTo("Unity.AI.Assistant.Editor")]
[assembly: InternalsVisibleTo("Unity.InputSystem.EditorDevelopmentTools")]
Copy link
Contributor

@u-pr-agent u-pr-agent bot Oct 10, 2025

Choose a reason for hiding this comment

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

Suggestion: The new script RegeneratePrecompiledLayouts.cs is in Assets/Editor, which compiles into the Assembly-CSharp-Editor assembly. However, InternalsVisibleTo is being granted to Unity.InputSystem.EditorDevelopmentTools. To allow the new script to access internal APIs from the Input System, the attribute should target the assembly the script is actually in. [possible issue, importance: 9]

Suggested change
[assembly: InternalsVisibleTo("Unity.InputSystem.EditorDevelopmentTools")]
[assembly: InternalsVisibleTo("Assembly-CSharp-Editor")]

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

Copy link
Collaborator Author

@ekcoh ekcoh Oct 10, 2025

Choose a reason for hiding this comment

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

Should probably just move the folder out of Editor

@codecov-git.colasdn.top
Copy link

codecov-git.colasdn.top bot commented Oct 10, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 29 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
Assets/Tools/RegeneratePrecompiledLayouts.cs 0.00% 29 Missing ⚠️ 
@@             Coverage Diff             @@
##           develop    #2258      +/-   ##
===========================================
- Coverage    76.70%   76.63%   -0.07%     
===========================================
  Files          465      467       +2     
  Lines        87919    88039     +120     
===========================================
+ Hits         67442    67473      +31     
- Misses       20477    20566      +89
Flag Coverage Δ
inputsystem_MacOS_2021.3 5.91% <ø> (-0.01%) ⬇️
inputsystem_MacOS_2022.3 5.37% <ø> (-0.01%) ⬇️
inputsystem_MacOS_2022.3_project 74.52% <0.00%> (-0.07%) ⬇️
inputsystem_MacOS_6000.0 5.18% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.0_project 76.44% <0.00%> (-0.07%) ⬇️
inputsystem_MacOS_6000.2 5.18% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.2_project 76.44% <0.00%> (-0.07%) ⬇️
inputsystem_MacOS_6000.3 5.19% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.3_project 76.44% <0.00%> (-0.07%) ⬇️
inputsystem_MacOS_6000.4 5.19% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.4_project 76.44% <0.00%> (-0.07%) ⬇️
inputsystem_Ubuntu_2021.3 5.91% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_2022.3 5.38% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_2022.3_project 74.33% <0.00%> (-0.07%) ⬇️
inputsystem_Ubuntu_6000.0 5.19% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.0_project 76.25% <0.00%> (-0.07%) ⬇️
inputsystem_Ubuntu_6000.2 5.19% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.2_project 76.25% <0.00%> (-0.07%) ⬇️
inputsystem_Ubuntu_6000.3 5.19% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.3_project 76.25% <0.00%> (-0.07%) ⬇️
inputsystem_Ubuntu_6000.4 5.19% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.4_project 76.25% <0.00%> (-0.07%) ⬇️
inputsystem_Windows_2021.3 5.91% <ø> (-0.01%) ⬇️
inputsystem_Windows_2022.3 5.37% <ø> (-0.01%) ⬇️
inputsystem_Windows_2022.3_project 74.66% <0.00%> (-0.07%) ⬇️
inputsystem_Windows_6000.0 5.18% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.0_project 76.58% <0.00%> (-0.07%) ⬇️
inputsystem_Windows_6000.2 5.18% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.2_project 76.58% <0.00%> (-0.07%) ⬇️
inputsystem_Windows_6000.3 5.19% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.3_project 76.58% <0.00%> (-0.07%) ⬇️
inputsystem_Windows_6000.4 5.19% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.4_project 76.58% <0.00%> (-0.07%) ⬇️

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

Files with missing lines Coverage Δ
Assets/Tools/AddScenesToBuild.cs 0.00% <ø> (ø)
Assets/Tools/RegeneratePrecompiledLayouts.cs 0.00% <0.00%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Pauliusd01
Copy link
Collaborator

Pauliusd01 commented Oct 10, 2025 •
edited
Loading

I get unauthorized access errors (I changed the package version and tried to regenerate):

image

@ekcoh ekcoh merged commit 14d3e18 into develop Oct 13, 2025
113 of 115 checks passed
@ekcoh ekcoh deleted the precompiled-layouts-menu-item branch October 13, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Pauliusd01 Pauliusd01 Pauliusd01 approved these changes

@jfreire-unity jfreire-unity jfreire-unity approved these changes

+1 more reviewer

@u-pr-agent u-pr-agent[bot] u-pr-agent[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ekcoh @Pauliusd01 @jfreire-unity