chore: runtime/timing classes moved to unity.netcode.gameobjects.timing namespace - #4150
Open
NoelStephensUnity wants to merge 11 commits into
Open
chore: runtime/timing classes moved to unity.netcode.gameobjects.timing namespace#4150NoelStephensUnity wants to merge 11 commits into
NoelStephensUnity wants to merge 11 commits into
Conversation
Adds the input and the assertions for moving NetworkTime, NetworkTimeSystem and NetworkTickSystem out of Unity.Netcode, ahead of the move itself. Assets/Runtime/DeprecatedTimingUsage.cs names all three in every reference form the editor project already covers, plus a constructor call, and names each one fully qualified at least once so a blocked run can tell "was not rewritten" from "was never referenced". EXPECTED_TYPES becomes EXPECTED_MOVES, grouped by relocation. The old list derived the 3.x name by substituting the namespace prefix, which only works while every move shares one destination; stating the namespace pair once per move keeps a type's two names from drifting and admits a second destination. Both match counts now take a trailing-token boundary, without which Unity.Netcode.NetworkTime also counts every Unity.Netcode.NetworkTimeSystem. --collision-stub is the regression test for why the move exists. It installs an assembly occupying the two names Netcode for Entities would take, and inverts the expectation for exactly those two: a reference that still resolves never reaches the MovedFrom data, so it cannot be migrated. NetworkTickSystem is deliberately absent from the stub and must still migrate, so a pass proves both halves rather than merely failing. The stub lives in a folder ending in '~' and is inert until the flag copies it in. Not run locally: there is no Python on the development machine, so this is verified by /ci apiupdater. The namespace-only form of MovedFrom is also unmeasured until then - every case in the AGENTS.md table moved the assembly too - and AGENTS.md now records that gap along with the one reference form the assertions deliberately do not depend on.
Runtime/Timing moves out of the Unity.Netcode root so that correcting the casing of Netcode for Entities' Unity.NetCode namespace does not collide with it. Two of the names are the collision: NetworkTime and NetworkTimeSystem exist in both SDKs as unrelated types - a time value here, an IComponentData carrying prediction-loop state there - and two assemblies exporting one fully qualified name is CS0433, which no user can work around in source. The three public types carry [MovedFrom(true, "Unity.Netcode", null, null)], so existing scripts are rewritten on upgrade. sourceAssembly is null because Unity.Netcode.Runtime keeps its name and a null argument is read as "unchanged"; the editor relocation passed an assembly name because that one genuinely moved assemblies. The three internal types in the folder carry nothing - the attribute only matters for API the updater has to migrate. Consumers take an import rather than a qualified name at each site. Qualification was the first approach and it is wrong here: .editorconfig sets IDE0001 to error, so a fully qualified name that the simplifier can shorten fails the Standards job. That leaves NGO's own references reading the bare name, which is correct as long as nothing else occupies Unity.Netcode.NetworkTime - if Netcode for Entities takes those names without vacating them, this assembly stops compiling wherever both packages are installed. That is deliberate: it fails early and loudly in our own CI rather than silently in user projects, and the recommendation both halves of this work rest on is that neither SDK keeps those names in the shared root. Files whose declared namespace is exactly Unity.Netcode cannot use an import for the two colliding names at all - the enclosing namespace's members beat both using directives and using aliases (CS0576) - so if the one-sided case ever has to be supported, those five files need qualified names and the rest do not. Sub-namespaces such as Unity.Netcode.Components never walk that far up and are fine either way. NetworkTimeSystem and AnticipationSystem now import Unity.Netcode themselves, and the five Components.NetworkTransform doc references in NetworkTimeSystem are spelled in full, since that prefix was only reachable from inside the root namespace. Compile-checked: runtime (with and without UNITY_EDITOR), editor, runtime tests and editor tests all clean, the last except the known environmental CS0656 on BytePackerTests. The UNIFIED_NETCODE path is not covered - the harness has no Entities or NetCode references - so UnifiedNetcodeTransport was checked by reading it: IRealTimeProvider at line 207 is a type position, so its import is needed.
…mespace xref targets are fully qualified UIDs, so the one in networktime-ticks.md stopped resolving when the type moved. DocFX renders an unresolved xref as its raw text, which reads as a broken link on the published page rather than failing anything in this repo - there is no docfx job here. Nothing else in Documentation~ needs changing. Every code sample that touches these types reaches them through NetworkManager (LocalTime.TimeAsFloat, NetworkTickSystem.Tick), which never names the type and is unaffected; the remaining mentions are prose or links to this same page. The old names in apiupdaterproject/README.md and the CHANGELOG are the 2.x side of before/after tables and are correct as they stand. No upgrade note added. The samples on that page do not need the new import, so a note about it would be advice for a reader the page does not have, and the landed editor relocation set the precedent of updating the affected sample without prose. The CHANGELOG entry covers the move.
The --collision-stub mode was added with the timing relocation but never wired into the job, so /ci apiupdater only ever exercised the default path and the regression test for the reason the move exists could not actually run in CI. Two sequential commands rather than one invocation: each needs its own cold import, since the assertion is meaningless against a Library that already holds rewritten sources. The script removes the stub and restores the 2.x sources on every exit path, so the second run starts from the state the first one did. The on-demand trigger is `pull_request.comment eq "apiupdater"` with no draft exclusion, so this runs on #4150 while it sits in draft. Neither mode needs Netcode for Entities present - the default mode tests NGO's own 2.x to 3.x migration, and the stub mode uses the local stub rather than the real package - so the run does not have to wait on the N4E namespace work.
Netcode for Entities 6.7.0 corrected the casing of its namespace, so it now declares Unity.Netcode.NetworkTimeSystem. The enclosing namespace is searched ahead of any using directive, so once the timing types leave Unity.Netcode the five files that still spell the name bare bind to theirs instead, and Unity.Netcode.Runtime stops compiling in any project that has both packages: 16 errors across 6 files, all of them cascading from the NetworkManager.NetworkTimeSystem property picking up the wrong type. The collision stub is corrected to match what N4E actually shipped. It sub-namespaced NetworkTime into Unity.Netcode.NetcodeTime but left NetworkTimeSystem in the shared root, so only that one name is occupied.
Member
Author
|
/ci apiupdater |
…dundant Unity.Netcode.GameObjects.Timing is nested inside Unity.Netcode, so the root namespace is already in scope there through the enclosing namespace chain. The two imports the move added to AnticipationSystem.cs and NetworkTimeSystem.cs were never needed, and the Unity.Netcode. prefix on the five NetworkTransform doc references shortens to Components. for the same reason - both are IDE0005 and IDE0001, which the Standards job treats as errors. NetworkManagerEventsTests.cs took an import it does not use: its only mention of NetworkTimeSystem is inside a comment. Matches the diff the Standards job produced. Running the tool locally finds nothing on dotnet 10.0.400, which is the version divergence its own failure message warns about, so this was applied from the CI output rather than reproduced here.
Member
Author
|
/ci apiupdater |
The job description still said the stub occupies both NetworkTime and NetworkTimeSystem, which is what the stub looked like before 6013f45 corrected it. N4E 6.7.0 sub-namespaced NetworkTime into Unity.Netcode.NetcodeTime and left NetworkTimeSystem in the shared root, so only one name is occupied and only that one expectation inverts.
NoelStephensUnity
marked this pull request as ready for review
September 7, 2026 03:30
NoelStephensUnity
requested review from
a team,
EmandM,
jabbacakes and
simon-lemay-unity
as code owners
September 7, 2026 03:30
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Harness Review
The runtime namespace migration is consistently applied internally, but the tested upgrade path still leaves a colliding legacy type bound to the other package and has gaps in how that limitation is validated and documented.
Reviewed commit ce53a03
🤖 Helpful? 👍/👎
|
ℹ️ Skipped — a harness review for this commit ( 🤖 Helpful? 👍/👎 |
Two review findings, both accurate. The README still described the stub as occupying both NetworkTime and NetworkTimeSystem, which is what it looked like before 6013f45 corrected it to match N4E 6.7.0. The expectation table was the harmful part: it listed NetworkTime as not-rewritten when it now migrates, so a correct run would read as a failure. The CLI help was already right. The CHANGELOG claimed migration is automatic without qualification. It is not, for exactly the case this move exists to address: when another installed package still declares the name in Unity.Netcode the old reference resolves, never reaches the MovedFrom data and is left alone. The --collision-stub run asserts that as a deliberate negative result, so the test was right and the user-facing sentence was wrong.
DeprecatedTimingUsage.cs reaches NetworkTickSystem through 'using TimeNs = Unity.Netcode;', and the per-type counts cannot see that site: 'TimeNs.NetworkTickSystem' contains neither the old nor the new fully qualified name, so it contributes to neither updated nor stale. The editor runs with -ignoreCompilerErrors, so an updater that left it unresolved would still produce a passing run off the other reference forms. The type alias on the line above is fine - it spells Unity.Netcode.NetworkTime in full, so it already counts toward stale. Separate commit because it cannot be verified here - there is no Python on the machine this was written on, so the next /ci apiupdater run is the first execution. Either outcome is informative: a pass closes the hole, and a failure means the updater does not follow namespace aliases, which is a finding rather than a defect in this assertion.
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop-3.x.x #4150 +/- ##
=================================================
+ Coverage 78.01% 78.18% +0.17%
=================================================
Files 153 153
Lines 26260 26272 +12
=================================================
+ Hits 20486 20542 +56
+ Misses 5774 5730 -44
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes
|
The unified job failed to compile with two CS0104 on NetworkManager.LocalTime and ServerTime: 'NetworkTime' is ambiguous between Unity.Netcode.GameObjects.Timing.NetworkTime and Unity.NetCode.NetworkTime. This is a second, distinct mechanism from the one 6013f45 addressed, and the stub used there could not produce it. That stub modelled N4E after its casing correction, where the colliding names are members of the enclosing Unity.Netcode namespace and therefore win over any import. The N4E that the pinned editor actually bundles is still Unity.NetCode, so both names arrive as imports instead - Unity.NetCode alongside Unity.Netcode.GameObjects.Timing - and two imports offering the same simple name is an ambiguity rather than a silent rebind. Before the move NGO's own NetworkTime was a member of Unity.Netcode, which is why the two imports coexisted for as long as they did. The alias fixes both mechanisms, so this holds whichever casing is installed. NetworkTime and NetworkTimeSystem are the only two names at risk: they are the sole intersection between Runtime/Timing/ and what N4E declares, confirmed against the 6000.7.0a5 bundled package. Only NetworkManager.cs is affected, being the only file that imports both namespaces and names either type in a type position.
Member
Author
|
/ci apiupdater |
Member
Author
|
/harness-review |
There was a problem hiding this comment.
💡 Harness Review
The namespace relocation and its runtime/test call sites are consistently updated, but the public XML documentation for the newly aliased timing properties does not follow the collision-safe type resolution.
Reviewed commit 3ded452
🤖 Helpful? 👍/👎
| /// The local <see cref="NetworkTime"/> | ||
| /// </summary> | ||
| public NetworkTime LocalTime => NetworkTickSystem?.LocalTime ?? default; | ||
| public GameObjectsNetworkTime LocalTime => NetworkTickSystem?.LocalTime ?? default; |
There was a problem hiding this comment.
With UNIFIED_NETCODE, this file imports both Unity.NetCode and the new timing namespace; the preceding comment confirms that both expose NetworkTime and NetworkTimeSystem. The unchanged cref="NetworkTime" comments for this property and ServerTime, plus cref="NetworkTimeSystem" on the property below, are now ambiguous rather than resolving through the enclosing NGO namespace as they did before the move. This produces ambiguous/wrong API-documentation links in the N4E configuration. Point those crefs at GameObjectsNetworkTime / GameObjectsNetworkTimeSystem (or fully qualify the new timing types) alongside the aliases.
🤖 Helpful? 👍/👎
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE:
N4E migration of NetworkTime and NetworkTimeSystem must be merged before taking this project out of draft!
Purpose of this PR
Since NGO will be dependent upon N4E and N4E is going to be updating their root namespace from
Unity.NetCodetoUnity.Netcode, there is a conflict withNetworkTimeandNetworkTimeSystem.This PR migrates NGO's time related classes into
Unity.Netcode.GameObjects.Timing.Jira ticket
MTT-15570
Changelog
Unity.Netcode.GameObjects.Timingnamespace.Documentation
Testing & QA (How your changes can be verified during release Playtest)
Manual testing can be performed by migrating an existing NGO v2.x.x project to NGO v3.x.x.
(Once N4E has made the namespace change)
Functional Testing
Manual testing :
Manual testing doneAutomated tests:
Covered by modified APIUpdater automated testCovered 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
N/A
Backports
N/A