| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
The relocation metadata covers the unconditional public editor API, but the new validation harness has a configuration-specific coverage gap and cannot be run reliably from paths containing spaces.
Reviewed commit dcf9a26
🤖 Helpful? 👍/👎
Sorry, something went wrong.
| ) | ||
|
|
||
| Write-Host 'Running the editor (this imports the project and runs the API updater)...' | ||
| $process = Start-Process -FilePath $unity -ArgumentList $unityArgs -PassThru -Wait -NoNewWindow |
There was a problem hiding this comment.
Start-Process joins this array into one command line without preserving quotes around individual values. If the checkout is under a path such as C:\Users\Jane Doe\src, both $projectPath and $logFile are split at the space, so Unity receives an invalid -projectPath/-logFile and the documented local test cannot run. Quote/escape those two values when building the argument string, or use ProcessStartInfo.ArgumentList to pass them as distinct arguments.
🤖 Helpful? 👍/👎
Sorry, something went wrong.
There was a problem hiding this comment.
@u-pr This is fixed and should no longer be a problem.
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good! Quoting "$projectPath" and "$logFile" ensures the paths won't be split on spaces when passed to Start-Process.
🤖 Helpful? 👍/👎
Sorry, something went wrong.
| # Every 2.x type the sources reference, and what the updater is expected to turn it into. | ||
| # Frozen: this is the public editor API of develop-2.0.0, which is released and will not change. | ||
| # Extend it by hand if a public editor type is ever relocated again within 3.x. | ||
| $expected = @( |
There was a problem hiding this comment.
This expected list has no NetworkObjectOwnership entry, and the fixture never references it. That class is a public old editor type whenever BYPASS_DEFAULT_ENUM_DRAWER && MULTIPLAYER_SERVICES_SDK_INSTALLED is enabled, so the test currently cannot verify the [MovedFrom] just added for that supported compilation mode; a regression there will still report success. Add a fixture/configuration that enables those symbols and include the type in the expected results.
🤖 Helpful? 👍/👎
Sorry, something went wrong.
There was a problem hiding this comment.
@u-pr Can you re-verify this? I remove that legacy script in NetworkObjectEditor.cs so it should not be of concern now.
Sorry, something went wrong.
There was a problem hiding this comment.
Verified. Since NetworkObjectOwnership has been removed from NetworkObjectEditor.cs rather than migrated to the new namespace with a [MovedFrom] attribute, it no longer needs coverage in DeprecatedApiUsage.cs or the $expected list in run-upgrade-test.ps1. The concern is resolved.
🤖 Helpful? 👍/👎
Sorry, something went wrong.
Removed obsolete custom property drawer for NetworkObject.OwnershipStatus.
…com/Unity-Technologies/com.unity.netcode.gameobjects into chore/editor-namespace-auto-upgrade
| @@ -0,0 +1,78 @@ | |||
| # This .gitignore file should be placed at the root of your Unity project directory | |||
There was a problem hiding this comment.
I don't think we need this?
Sorry, something went wrong.
There was a problem hiding this comment.
It helps keep it clean from "noise changes".
It is the same as the testproject one.
Sorry, something went wrong.
| @@ -0,0 +1,173 @@ | |||
| <# | |||
There was a problem hiding this comment.
This is a powershell script so it can't be run from Ubuntu or MacOS. It's also pretty opaque claude written code. A python script (@michalChrobot already has a few python scripts) would probably be preferred. Not a blocker, just a concern
Sorry, something went wrong.
There was a problem hiding this comment.
We can make that something more generic.
Sorry, something went wrong.
| @@ -0,0 +1,121 @@ | |||
| # API updater upgrade-path project | |||
There was a problem hiding this comment.
This README has too much info. The README from this project doesn't need info on how the upgrade was implemented, it just needs info on how it works.
If you want this project to be used also for future upgrades then this README shouldn't have any references to the 2.x -> 3.x upgrade specifically, otherwise future us might think we can pull this project out once that upgrade is released.
Sorry, something went wrong.
There was a problem hiding this comment.
This project and the tests associated with it will not be pertinent once v2.x.x becomes deprecated.
But I can remove the extra info...leaving it only provides the context as to why it is there in the first place.
Sorry, something went wrong.
| { | ||
| "disableProjectUpdate": false, | ||
| "dependencies": { | ||
| "com.unity.netcode.gameobjects": "file:../../com.unity.netcode.gameobjects", |
There was a problem hiding this comment.
It might be nice to have N4E in here so we can be sure the upgrade doesn't introduce a conflict with N4E
Sorry, something went wrong.
There was a problem hiding this comment.
This was tested extensively locally. Nothing wrapped in UNIFIED_NETCODE is impacted by the namespace migration since anything that does reference NGO editor classes(etc) already has the change applied and no user has any script that could cause conflicts.
This validates without N4E an NGO users upgrading from v2.x to v3.x will automatically have everything that needs to be updated...updated.
Sorry, something went wrong.
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop-3.x.x #4142 +/- ##
==============================================
Coverage 78.00% 78.00%
==============================================
Files 153 153
Lines 26260 26260
==============================================
Hits 20483 20483
Misses 5777 5777
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Purpose of this PR
This PR handles the auto-upgrade namespace change from Unity.Netcode.Editor to Unity.Netcode.GameObjects.Editor.
It also includes a job that can be manually triggered to validate the auto-upgrade or the job itself could be added to nightly or the like.
Jira ticket
MTT-15570
Changelog
NA
Documentation
Testing & QA (How your changes can be verified during release Playtest)
Functional Testing
Manual testing :
Automated tests:
Does the change require QA team to:
If any boxes above are checked the QA team will be automatically added as a PR reviewer.
Up-port
None
Backports
None