chore: netcode for entities 7.0.0 as a dependency - #4158
chore: netcode for entities 7.0.0 as a dependency#4158NoelStephensUnity wants to merge 8 commits into
Conversation
… a define N4E becomes a hard dependency at 7.0.0, and com.unity.transport moves to 6.5.0 to match it. N4E 7.0.0 ships its unified API - GhostObject, GhostBehaviour, GhostField - behind NETCODE_GAMEOBJECT_BRIDGE_EXPERIMENTAL, which leaves those types internal in the configuration NGO ships. A public type cannot derive from an internal one, so the unified path can no longer key off the package being present: the UNIFIED_NETCODE versionDefine is removed from the four assembly definitions and the symbol is set by the project instead, alongside N4E's own define. NetworkObjectBridge is internal for the same reason, and UnifiedBootstrap raises an #error when only one of the two defines is set. With N4E present the two packages share the Unity.Netcode namespace, which breaks three things outside the unified gates: - N4E's Netcode facade type shadows the namespace, so 22 XML doc crefs and one nameof that qualified with Netcode. stopped resolving. - N4E declares an INetworkStreamDriverConstructor in the shared root that the enclosing namespace finds ahead of NGO's, so the one implementation in the editor tests is fully qualified. - N4E's source generator errors on an assembly that does not reference N4E, so the editor test assembly now references it. Multiplayer Services 2.3.1 does not compile against N4E 7.0.0 and no fix is published, so it is dropped from the test project manifests. Validation moves to trunk, the only editor that has dropped N4E as a core package and can resolve the 7.0.0 request rather than its bundled 6.7.0. The unified work is preserved and still compiles with both defines set: the N4E 6.7.0 spellings are updated, and GhostObject.ApplyPostTransformMatrixScale, which N4E removed in its scale rework, is reimplemented over the new PostTransformMatrix model. The unified CI job sets the defines itself and is no longer a PR check. Also disables the serialization tests that fail under CoreCLR so the package is not ejected from the editor manifest batch. Parts of this are taken from #4156.
There was a problem hiding this comment.
💡 Harness Review
The change updates the package dependency surface and isolates the optional unified bridge behind project-level defines; I examined the runtime namespace/API migration, scale bridge call paths, assembly references, manifests, and CI gating.
Reviewed commit c78f679
🤖 Helpful? 👍/👎
Three things the standards job caught, since it opens the project, runs the formatter with --fix and then fails on any diff at all: - The fully qualified INetworkStreamDriverConstructor is simplified by the formatter (IDE0001), so write it the way the formatter wants. It still resolves to NGO's, since the lookup walks out to Unity.Netcode.Transports before reaching the shared root. - N4E 7.0.0 renamed the serialized GlobalNetCodeConfig field to GlobalNetcodeConfig, which the editor rewrites in the test project's settings asset the first time it loads it. - validation_editors.minimal is back on a released editor. The standards job opens the project with it, and trunk rewrites ProjectVersion.txt out from under the cleanliness check. Nothing there needs N4E present: a released editor bundles 6.7.0 as a built-in package, so the 7.0.0 request is dropped silently and the project builds with no N4E at all - verified on 6000.7.0a6, which resolves and compiles clean with N4E absent from packages-lock, PackageCache and ScriptAssemblies alike.
The formatter and the compiler want opposite things here. With N4E present the bare INetworkStreamDriverConstructor binds to N4E's copy in the shared Unity.Netcode root, which the enclosing namespace reaches before the using directive, and the class then fails to implement it. With N4E absent - which is what the standards job's editor resolves - any qualification is redundant and IDE0001 strips it back off, so the last two attempts each traded one failure for the other. A file-scope using alias satisfies both. Its target cannot be shortened, since at file scope there is no enclosing namespace to shorten it against; the use site is a single identifier with no qualified form; and IDE0005 cannot drop the alias because removing it stops the name resolving at all.
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop-3.x.x #4158 +/- ##
=================================================
+ Coverage 78.18% 78.21% +0.02%
=================================================
Files 153 154 +1
Lines 26272 26543 +271
=================================================
+ Hits 20542 20761 +219
- Misses 5730 5782 +52
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes
|
…r version The 7.0.0 dependency could not be satisfied anywhere. Every editor available today - 6000.7.0a6 and current trunk alike - lists com.unity.netcode 6.7.0 as mustBeBundled in its manifest, so UPM drops a 7.0.0 request silently: no assemblies, no packages-lock entry, no warning. upm-pvp then fails hard with "depends on com.unity.netcode, which is not in the lock file", which is what took out Validate on all three platforms and Package Test on both editors. Dependency versions are minimums, so asking for 6.7.0 resolves the bundled package today and picks up 7.0.0 by itself once an editor manifest carries it. The transport bump goes back to 2.6.0: 6.5.0 was never published (the 6.x line is editor-bundled only, latest published is 2.7.4), so it fails PVP-160-1, which the Validate job explicitly requires - and N4E asks for 6.5.0 itself, so resolution is unaffected either way. With the dependency resolving everywhere, nothing needs a trunk editor and validation_editors goes back to what develop had. Since either version can now be what resolves, the unified path compiles against both, gated on a UNIFIED_NETCODE_7_0_0 versionDefine keyed to the resolved package version. The 6.7.0 spellings are aliased to the 7.0.0 names so the use sites read the same either way, except N4E's Netcode class: inside namespace Unity.Netcode that name binds to the enclosing namespace before any file-scope alias is considered, so it is aliased as EntitiesNetcode in both. GhostObjectScaleExtensions is 7.0.0-only, since 6.7.0 still has the method it stands in for. That also makes the dependency real rather than dropped, which has consequences worth knowing: N4E's assemblies now build in every project, its ClientServerBootstrap becomes the Entities bootstrap with a non-zero AutoConnectPort, and it writes a NetcodeConfig into the project when its settings asset does not resolve one - which is why the GlobalNetCodeConfig field goes back to the 6.7.0 spelling here. Verified on 6000.7.0a6 against both versions: shipping (unified off), unified against bundled 6.7.0, and unified against 7.0.0 all compile with zero errors, and the standards check is clean.
| "com.unity.netcode.gameobjects": "file:../../com.unity.netcode.gameobjects", | ||
| "com.unity.package-validation-suite": "0.49.0-preview", | ||
| "com.unity.services.authentication": "3.7.4", | ||
| "com.unity.services.multiplayer": "2.3.1", |
There was a problem hiding this comment.
Depending if we will merge this PR first (then we keep this removed) or they release their package first (then we keep it but update it)
| "com.unity.mathematics": "1.4.0", | ||
| "com.unity.multiplayer.tools": "2.2.11", | ||
| "com.unity.netcode": "6.7.0", | ||
| "com.unity.netcode": "7.0.0", |
There was a problem hiding this comment.
Do we actually still need manifest-unified?
|
Check the configuration's TOML syntax, permitted settings, and size limit. See the error below. Error message: Configuration content:# This is the configuration for the u-pr bot
# https://github.cds.internal.unity3d.com/unity/u-pr
# For configuration of this file:
# https://developer.portal.internal.unity.com/catalog/default/component/u-pr/docs/configuration/automatic_runs/
[github_app]
handle_pr_actions = ['opened', 'ready_for_review'] # PR events that auto-run pr_commands
pr_commands = ["/harness-review"] # what runs on those events
reviewer_commands = ["/harness-review"] # what runs when @u-pr is added as reviewer
|
Parts of this are taken from #4156.
Purpose of this PR
N4E becomes a hard dependency at 7.0.0, and com.unity.transport moves to 6.7.0 to match it. We update our CI to match the actual minimal supported editor.
We had to disable the serialization tests that would otherwise fail under CoreCLR in the editor later (as we don't have CI for it yet here)
Jira ticket
MTT-14623
Changelog
com.unity.netcode7.0.0) is now a hard dependency, so installing Netcode for GameObjects also brings it, and its own dependencies (Entities, Burst, Collections), into the project.Documentation
Testing & QA (How your changes can be verified during release Playtest)
Green Ci should validate this properly
Up-port
No up-port is required.
Backports
No back-port is required.