-
Notifications
You must be signed in to change notification settings - Fork 331
FIX: Migrate Mono APIs to CoreCLR-compatible APIs #2289
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
base: develop
Are you sure you want to change the base?
Conversation
…blies() - LinkFileGenerator.cs: Add conditional compilation (line 43) - Added using UnityEngine.Assemblies with conditional directive - Maintains backward compatibility with older Unity versions
…ible APIs Add conditional compilation to replace AppDomain.CurrentDomain.GetAssemblies() with CurrentAssemblies.GetLoadedAssemblies() in: - InputManager.cs - InputParameterEditor.cs - CoreTests_Controls.cs - InputTestFixture.cs Maintains backward compatibility with Unity versions prior to 6000.5.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent |
PR Code Suggestions ✨No code suggestions found for the PR. |
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #2289 +/- ##
===========================================
- Coverage 77.95% 77.07% -0.89%
===========================================
Files 477 477
Lines 97416 90097 -7319
===========================================
- Hits 75943 69442 -6501
+ Misses 21473 20655 -818 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 51 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Just double checking if there's anything I should do/run/add for this to become merge:able? :) I also see several jobs failing, but my suspicion it's not related to these changes maybe? I checked the history of some of the failing jobs on other branches and seems they were failing on other branches too not too long ago? But maybe I'm misunderstanding! Appreciate any input and help! :) |
ekcoh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this. Looks good.
|
@MiroBrodlovaUnityGit I think CI failure was unrelated, I started a rerun. Also @jfreire-unity added our QA @Pauliusd01 to the PR in case he would like to check something. Otherwise I think this can land, so feel free to merge @Pauliusd01 once you have approved. |
|
There was some CI issues that were fixed and I updated this PR with latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tried running a couple of samples on 22.3 (mono player/playmode) and latest trunk (mono only player/playmode), did not notice any issues. I wasn't able to build the coreclr version editor on trunk though, kept erroring out. If there's a specific way I need to do it or I need a specific revision then let me know on slack and I'll check that one as well
Purpose of this PR
This PR replaces all instances of analyzer-highlighted
MonoAPI calls withCoreCLR-compatible API calls in com.unity.collections, and is part of Epic SCP-1555.JIRA: https://jira.unity3d.com/browse/SCP-1555
Further Context
Monoto usingCoreCLR.MonoAPI calls withCoreCLR-compatible ones. (here's the list of APIs being replaced, and here's the list of packages that will get updated).MonoAPI calls across all Unity supported packages withCoreCLR-compatible ones.Note: More PRs dealing with SCP-1555 may come in the future. In some cases we cannot replace all
MonoAPI calls withCoreCLRAPI calls yet, as someCoreCLRAPIs are still being worked on/are not yet public. In these cases we suppress warnings for those particular APIs for now.Release Notes
None
Functional Testing status
Running the default job-suite that is run on new PRs. Please do share if there's some steps I've missed that I must do to properly setup the PR and run your CI jobs!
Performance Testing Status
Haven't run any additional performance-tests as this PR should just replace Mono API with equivalent CoreCLR API
Overall Product Risks
Low risk as it just changes Mono API to equivalent CoreCLR API
Comments to Reviewers
If there are any jobs/tests we should run to make sure there are no ripple effects do share and we will kick them off! :) And, if you know of any locations we may have missed updating or any details that we should know about, please do share!