Skip to content
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.

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

FIX: Avoid problems with CI and reimports #2091

Merged
merged 7 commits into from
Dec 20, 2024
Merged

Conversation

ekcoh
Copy link
Collaborator

@ekcoh ekcoh commented Dec 17, 2024

Description

FIX: Solves problem with CI and reimports issues by EditorApplication.delayCall which leads to CI build errors "Error: Scripts are compiling" when attempting to run a batch mode project via UTR when source code associated with an .inputactions asset is out of sync due to asset or code generator update.

Testing status & QA

Have tested various ways to go around the problem EditorApplication.update, AssetDatabase.ImportAsset etc but went with AssetDatabase.ImportAsset which is a solution that goes straight against what previous inline comment suggested. Discussed solution with Asset Database team and no solution is good since this should use a source generator (Roslyn) for this kind of feature, but cannot be reliably setup in current Unity version. Hence this solution.

Recommendation for testing is to also ensure this solution works with regular editor use (non-batch mode). Also proven by the following PR which has intentionally outdated test source asset and sample source asset: #2060

The linked PR should be closed if/when this lands.

Overall Product Risks

  • Complexity: Small to medium
  • Halo Effect: Medium due to many corner cases and unknown side effects

Comments to reviewers

Focus on corner cases and potential race conditions.

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.

…ilding Player because scripts are compiling" if a source generated .inputactions asset is out of sync with its generated source code.
@ekcoh ekcoh marked this pull request as ready for review December 17, 2024 15:57
@ekcoh ekcoh requested a review from bmalrat December 17, 2024 20:19
Copy link
Collaborator

@ritamerkl ritamerkl left a comment

Choose a reason for hiding this comment

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

Provisory approving, when the testing is done and no issues found

Copy link
Collaborator

@Pauliusd01 Pauliusd01 left a comment

Choose a reason for hiding this comment

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

LGTM, things I tried checking:

  • Running most of our testing scenes in the main repo (player/playmode)
  • Making changes to these scenes and then running them again (player/playmode)
  • Doing above while also throwing in a reimport before running them
  • Doing the above again while having their input actions assets generate a C# file option on

I did not notice any issues with the testing scenes or using/saving my changes whether I had the C# option on or whether the assets were reimported recently

@ekcoh ekcoh merged commit db232e3 into develop Dec 20, 2024
75 of 77 checks passed
@ekcoh ekcoh deleted the isxb-1300-importer-issue branch December 20, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants