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 4 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.01% 78.25% +0.24%
=================================================
Files 153 154 +1
Lines 26260 26543 +283
=================================================
+ Hits 20486 20771 +285
+ Misses 5774 5772 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 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.
| { | ||
| "name": "com.unity.netcode", | ||
| "expression": "1.10.1", | ||
| "define": "UNIFIED_NETCODE" |
There was a problem hiding this comment.
Should we remove UNIFIED_NETCODE from other places and keep only UNIFIED_NETCODE_7_0_0? I saw other places kept it
|
|
||
| ### Added | ||
|
|
||
| - Netcode for Entities (`com.unity.netcode` 6.7.0) is now a dependency, so installing Netcode for GameObjects also brings it, and its own dependencies (Entities, Burst, Collections), into the project. |
There was a problem hiding this comment.
| - Netcode for Entities (`com.unity.netcode` 6.7.0) is now a dependency, so installing Netcode for GameObjects also brings it, and its own dependencies (Entities, Burst, Collections), into the project. | |
| - Netcode for Entities (`com.unity.netcode` 7.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. |
| "com.unity.nuget.mono-cecil": "1.11.4", | ||
| "com.unity.transport": "2.6.0" | ||
| "com.unity.transport": "2.6.0", | ||
| "com.unity.netcode": "6.7.0" |
There was a problem hiding this comment.
So as N4Ev7.0.0 landed properly and will be included in 6000.7.0b1 shouldn't we declare dependency on 7.0.0?
As for transport we should depend on 6.7.0 (their version of core-package bundled with the editor) as this is the version in our minimal supported editor and we may try to lower this in the future (which is allowed) but safer to start with latest
| "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?
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.5.0 to match it.
Jira ticket
MTT-14623
Changelog
NA
Documentation
Testing & QA (How your changes can be verified during release Playtest)
Functional Testing
Manual testing :
Manual testing doneAutomated tests:
Covered by existing automated testsCovered by new automated testsDoes the change require QA team to:
Review automated tests?Execute manual tests?Provide feedback about the PR?If any boxes above are checked the QA team will be automatically added as a PR reviewer.
Up-port
No up-port is required.
Backports
No back-port is required.