fix: network clock adjustment overshoot and per-frame NetworkManager scene search by NoelStephensUnity · Pull Request #4195 · Unity-Technologies/com.unity.netcode.gameobjects

NoelStephensUnity · 2026-10-16T15:55:49Z

Purpose of this PR

Four small, independent fixes, plus a change to the Unified CI job.

Network clock adjustment overshoot: without this fix, a client's clock keeps alternating between running slightly fast and slightly slow on every frame, even after it has synchronized with the server. Interpolated objects then judder (a periodic motion artifact caused by mismatched frame rates): they move in uneven steps instead of smoothly. It is barely noticeable at the default NetworkTimeSystem.AdjustmentRatio, and clearly visible at higher values.

Per-frame NetworkManager scene search: without this fix, UnifiedUpdateConnections searches the whole scene for NetworkManager instances every frame, in every Entities world, even when there is no hybrid session, so the frame time grows with the size of the scene (about 1.5 ms per frame with 10,000 MonoBehaviours). It now only exists in client and server worlds, and only runs in a world created for a hybrid session, where it updates the connections of that world's own NetworkManager.

Hybrid mode in dedicated server builds: Netcode for Entities cannot create a single world host in a dedicated server build, which every hybrid server used. A dedicated server build now creates a server-only world, and a build for only one side logs an error when asked to start the other side.

RTT lookup after a hybrid connection is removed: a client that disconnects could still process a queued TimeSyncMessage after the unified transport has already removed the server connection. This edge case would throw a KeyNotFoundException. GetCurrentRtt now returns 0 for a connection it no longer has, as UnityTransport does.
(The last three are excluded from the changelog since the unified feature set has not yet been released)

Unified CI job: on a b1.large agent, the IL post-processor and compiler processes would hold about 1.5 to 2 GiB due to the required script rebuild. They continued to hold that memory throughout the tests, which could take, cumulatively, up to 50 minutes, with some tests failing on clients not connecting.
This job now runs a "warm-up" headless editor pass first so the project's Library can be built; that editor instance then exits (so the IL post-processor and compiler processes are terminated), and the test runner pass can start and run without that additional overhead. With the warm-up adjustment, the same job passes in about 14 minutes.

Jira ticket

None.

Changelog

Fixed: Issue where a client's clock kept alternating between running slightly fast and slightly slow on every frame, even after it had synchronized with the server. Interpolated objects juddered (a periodic motion artifact caused by mismatched frame rates): they moved in uneven steps instead of smoothly. Barely noticeable at the default NetworkTimeSystem.AdjustmentRatio, and clearly visible at higher values.

Documentation

No documentation changes or additions were necessary.

Testing & QA (How your changes can be verified during release Playtest)

Functional Testing

Automated tests:

Covered by new automated tests: ClientNetworkTimeSystemTests.ConvergedClockAdvancesAtRealRate, UnifiedNetworkTransformTest.EachWorldUpdatesConnectionsForItsOwnNetworkManager, UnifiedUpdateConnectionsTests.NoWorldUpdatesConnectionsWithoutGhostPrefabs

Up-port

Not needed: this targets develop-3.x.x.

Backports

None.

…ing past it Once a client's clock reached the offset it adjusts towards, each frame stepped the full AdjustmentRatio past it and back, so LocalTime and ServerTime alternated between running fast and slow every frame.

…r them every frame UnifiedUpdateConnections called Object.FindObjectsByType<NetworkManager>() on every update, whose cost grows with the number of MonoBehaviours in the scene: about 1.5 ms a call with 10,000 and 14 ms with 50,000. NetworkManager.Instances is filled in Awake and emptied in OnDestroy, and the system visits that.

u-pr · 2026-10-16T15:55:56Z

⚠️ Some settings were ignored in '.pr_agent.toml (repository root)' repo settings

Check the configuration's TOML syntax, permitted settings, and size limit. See the error below.

Error message:
Ignored prohibited settings: github_app.handle_pr_actions. Permitted settings were applied.

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

u-pr

💡 Harness Review

The changes address clock overshoot and repeated scene searches, with a remaining compatibility concern in manager discovery.

_{Reviewed commit 84cbb03}

_{🤖 Helpful? 👍/👎}

UnifiedUpdateConnections now runs only in client and server worlds that hold the NetworkManager they were created for, and updates that NetworkManager alone. This replaces the NetworkManager.Instances list, which a derived NetworkManager declaring its own Awake never entered.

codecov-github-com · 2026-10-16T17:10:20Z

Codecov Report

Attention: Patch coverage is 5.66038% with 50 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...ime/Components/Helpers/UnifiedUpdateConnections.cs	0.00%	37 Missing ⚠️
...cts/Runtime/Components/Helpers/UnifiedBootstrap.cs	0.00%	7 Missing ⚠️
...netcode.gameobjects/Runtime/Core/NetworkManager.cs	0.00%	5 Missing ⚠️
...time/Transports/Unified/UnifiedNetcodeTransport.cs	0.00%	1 Missing ⚠️

@@                Coverage Diff                @@
##           develop-3.x.x    #4195      +/-   ##
=================================================
- Coverage          76.11%   76.03%   -0.09%     
=================================================
  Files                159      159              
  Lines              27333    27344      +11     
=================================================
- Hits               20805    20791      -14     
- Misses              6528     6553      +25

Flag	Coverage Δ
NGOv2_project_testproject_ubuntu	`75.07% <5.66%> (-0.09%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...de.gameobjects/Runtime/Timing/NetworkTimeSystem.cs	`95.09% <100.00%> (+0.04%)`	⬆️
...time/Transports/Unified/UnifiedNetcodeTransport.cs	`0.00% <0.00%> (ø)`
...netcode.gameobjects/Runtime/Core/NetworkManager.cs	`72.37% <0.00%> (-0.28%)`	⬇️
...cts/Runtime/Components/Helpers/UnifiedBootstrap.cs	`0.00% <0.00%> (ø)`
...ime/Components/Helpers/UnifiedUpdateConnections.cs	`0.00% <0.00%> (-23.19%)`	⬇️

Components	Coverage Δ
com.unity.netcode.gameobjects	`76.03% <5.66%> (-0.09%)`	⬇️

ℹ️ Need help interpreting these results?

NoelStephensUnity · 2026-10-16T17:13:41Z

@u-pr please review again

u-pr · 2026-10-16T17:17:23Z

Review Summary

The shift to binding the NetworkManager directly to the hybrid session's NetcodeWorld via the HybridNetworkManager singleton component is a great improvement. It cleanly resolves the previous subclass Awake inheritance limitation while avoiding scene-wide searches and cross-talk across multiple NetworkManager instances. The clock overshoot fix in NetworkTimeSystem and its accompanying tests are also well-designed.

A few observations and suggestions:

Observations & Suggestions

1. Null / Destroyed `NetworkManager` Safety in `UnifiedUpdateConnections`

Location: UnifiedUpdateConnections.cs:56, 94, 115
When using UnityObjectRef<NetworkManager>.Value, if the NetworkManager GameObject is destroyed prior to the ECS World being disposed (e.g., during scene transitions or test teardown), networkManager evaluates to null (or a destroyed Unity object).
Later dereferences (networkManager.IsServer at line 94 and networkManager.ShutdownInProgress at line 115) will throw a NullReferenceException or MissingReferenceException.

Suggestion: Treat networkManager == null as a disconnected/shutting down state:

var networkManager = m_HybridNetworkManagerQuery.GetSingleton<HybridNetworkManager>().Value.Value;
var isShuttingDownOrDisconnected = networkManager == null || networkManager.ShutdownInProgress || !networkManager.IsListening;

(See inline comment for diff context).

2. Missing CHANGELOG Entry for `UnifiedUpdateConnections`

Location: CHANGELOG.md
CHANGELOG.md currently documents the clock adjustment/judder fix, but does not mention the UnifiedUpdateConnections performance improvement (replacing the per-frame FindObjectsByType<NetworkManager>() search with world-scoped manager binding).
Suggestion: Consider adding an entry under ### Fixed for this change as well.

3. Defensive Step Size in `NetworkTimeSystem.Advance`

Location: NetworkTimeSystem.cs:265
If an invalid or negative deltaTimeSec or AdjustmentRatio is ever passed, maximumStep would be negative, causing min > max in Math.Clamp(..., -maximumStep, maximumStep) which throws an ArgumentException.
Suggestion: Have you considered ensuring maximumStep is non-negative, for example:
```
var maximumStep = Math.Max(0.0, deltaTimeSec * AdjustmentRatio);
```

_{🤖 Helpful? 👍/👎}

…uilds Netcode for Entities cannot create a single world host in a dedicated server build, which every hybrid server used. A build for only one side now logs an error when asked to start the side it does not support.

EmandM

A couple of nit-pick comments but this looks fantastic! I really, really like this approach!!

EmandM · 2026-10-16T17:37:41Z

+    /// </summary>
+    internal struct HybridNetworkManager : IComponentData
+    {
+        public UnityObjectRef<NetworkManager> Value;


Oh I LIKE this!!

Though a deep nit-pick, maybe Value isn't the best name here? It's very vague.

Good call, Value didn't say much. Renamed it to NetworkManager, so it is now .NetworkManager.Value

EmandM · 2026-10-16T17:40:34Z

-            m_CurrentLocalTimeOffset += deltaTimeSec * (m_DesiredLocalTimeOffset > m_CurrentLocalTimeOffset ? AdjustmentRatio : -AdjustmentRatio);
-            m_CurrentServerTimeOffset += deltaTimeSec * (m_DesiredServerTimeOffset > m_CurrentServerTimeOffset ? AdjustmentRatio : -AdjustmentRatio);
+            // Stop on the desired offset rather than step past it, or a converged clock alternates between running
+            // fast and slow by the whole ratio every frame, and everything rendered from it judders.


nit: I don't understand this code comment. It wouldn't help me know what's going on here if I ever needed to interact with this code.

That is a fair point. The old comment described the symptom rather than what the code does. It now reads:

Move each offset toward its target by at most AdjustmentRatio of this frame's delta, stopping once it gets there. Stepping the full amount every frame overshot the target, so a synced clock alternated between running fast and slow, and anything driven by it juddered.

…updates The hybrid world outlives its NetworkManager if ShutdownInternal throws before disposing it, so UnifiedUpdateConnections now disconnects every connection and announces none instead of throwing each frame. HybridNetworkManager.Value is renamed to NetworkManager, the clock adjustment comment says what the clamp does, and the unused Unity.Entities using that failed the Standards check is removed.

NoelStephensUnity · 2026-10-16T18:08:44Z

@u-pr please review again.

u-pr · 2026-10-16T18:12:47Z

Review Summary

The recent updates nicely address the previous feedback:

The isShuttingDown check (!networkManager || networkManager.ShutdownInProgress || !networkManager.IsListening) safely guards against dereferencing missing or destroyed NetworkManager instances.
Build-target differentiation in UnifiedBootstrap.CreateWorld properly configures dedicated server (CreateServerWorld) and client-only modes, avoiding invalid single-world host configurations in dedicated server builds.
The updated comments and renaming in HybridNetworkManager clarify intent.

A couple of minor observations and suggestions:

Suggestions

1. Repeated `OnNetCodeDisconnect` Invocations During Extended Shutdown

Location: UnifiedUpdateConnections.cs:119
If the world outlives the NetworkManager over several frames (e.g. if ShutdownInternal throws before world disposal), isShuttingDown remains true on subsequent frames. Because SystemAPI.Query<NetworkId>() matches all entities with NetworkId and does not remove NetworkId, OnNetCodeDisconnect will be repeatedly invoked every frame for the same entities.
Suggestion: Filter the query to only entities that currently have ConnectionState (which is removed in the first pass):
```
foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithAll<ConnectionState>().WithEntityAccess())
```
(See the inline comment for details).

2. Skip Discovery When Shutting Down

Location: UnifiedUpdateConnections.cs:78–88
When isShuttingDown is true, the pending connections loop (lines 89–111) is skipped, but new connections are still queried, allocated (new NetcodeConnection), and inserted into m_NewConnections.
Suggestion: Consider skipping the discovery query when isShuttingDown is true (e.g., guarding with if (!isShuttingDown)), or clearing m_NewConnections on shutdown.

3. Unused Local Variable

Location: UnifiedUpdateConnections.cs:54
var isServer = World.IsServer(); is not referenced within OnUpdate(). Consider removing it to keep the method clean.

_{🤖 Helpful? 👍/👎}

…ssure theory Revert before merge.

A client drains its incoming queue while it disconnects, after the unified transport has removed the server connection, so a queued TimeSyncMessage made GetCurrentRtt throw KeyNotFoundException. It now returns 0 for a connection it no longer has, as UnityTransport does.

… whether 6000.7.0b6 raised memory use Replaces the b1.xlarge test. Revert before merge.

…ed test pass The IL post-processor and compiler processes the editor starts stay resident through the test pass and, on a b1.large agent, leave too little memory for the hybrid tests. The project is now imported and compiled in its own batchmode session that exits first, any build process that outlives it is stopped, and the job log lists the largest processes left before the tests start. Also drops the temporary 6000.7.0b4 editor pin.

The test runner compiles in Release. An editor whose code optimization preference is Debug builds the other configuration in the warm-up, and the test session then recompiles every assembly and restarts the IL post-processor. CI's agents already default to Release, so this is a guard rather than a fix there; it removed the recompile on a machine set to Debug.

…d a negative clock step While its NetworkManager is shutting down, UnifiedUpdateConnections now disconnects only the connections that still have ConnectionState, which it removes, so OnNetCodeDisconnect is not invoked again every frame for the same connections. It also stops collecting new connections while shutting down and drops an unused local. NetworkTimeSystem.Advance no longer throws when the delta time or AdjustmentRatio is negative.

NoelStephensUnity · 2026-10-17T00:36:53Z

🤖 Point 1 was already fixed: a destroyed or missing NetworkManager is treated as shutting down. Point 3 is applied: a negative step no longer reaches Math.Clamp. Point 2 is not taken: the unified feature set has not been released yet, so its fixes stay out of the CHANGELOG.

…created Every NetworkManager that starts a hybrid session creates a UnifiedBootstrap that is then discarded, and its finalizer cleared the static LastCreatedWorld and Instance from the GC finalizer thread. When one ran while another session was starting, Initialize found its new world null, logged "World is null!" and returned false: the NetworkManager had no world, its transport threw a NullReferenceException on start, and the created world was never disposed. The finalizer is removed and Initialize checks the world it created rather than the static.

NoelStephensUnity · 2026-10-17T02:25:04Z

@u-pr please review again.

u-pr · 2026-10-17T02:27:32Z

Review Summary

The latest updates bring substantial stability and correctness improvements:

UnifiedBootstrap GC safety: Removing the ~UnifiedBootstrap() finalizer and capturing the created world in a local variable resolves the race condition where GC finalization cleared static state (LastCreatedWorld and Instance) during session initialization.
Dedicated server & client target handling: UnifiedBootstrap.CreateWorld properly branches on build targets (UNITY_SERVER, UNITY_CLIENT), ensuring dedicated server builds use CreateServerWorld and report clear diagnostics when misconfigured.
Shutdown connection lifecycle: Guarding discovery and connection promotion behind !isShuttingDown avoids queueing new connections while shutting down.
Transport RTT safety: UnifiedNetcodeTransport.GetCurrentRtt safely returns 0 via TryGetValue when a connection has already been removed during shutdown, avoiding KeyNotFoundException.
Clock convergence: Clamping the adjustment step in NetworkTimeSystem.Advance eliminates clock overshoot and visual judder, backed by solid unit tests.

Suggestions

1. Mutually Exclusive Disconnect Queries during Shutdown

Location: UnifiedUpdateConnections.cs:123
If an entity loses its NetworkStreamConnection on the same frame that isShuttingDown evaluates to true, it is matched by the initial disconnect query (line 58, WithNone<NetworkStreamConnection>()). Because the EntityCommandBuffer has not yet been played back, the entity still carries ConnectionState in the EntityManager when the shutdown loop executes (line 123), causing OnNetCodeDisconnect to be invoked twice for that entity in the same frame.
Suggestion: Adding NetworkStreamConnection to the shutdown query (WithAll<ConnectionState, NetworkStreamConnection>()) ensures the shutdown loop only cleans up connections that were still connected, keeping the two cleanup paths strictly disjoint. (See inline comment for code suggestion).

_{🤖 Helpful? 👍/👎}

u-pr · 2026-10-17T02:27:33Z

+            {
+                // Only in-game connections have ConnectionState, and removing it here means each one is only reported once.
+                foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithAll<ConnectionState>().WithEntityAccess())
                {


If a connection loses its NetworkStreamConnection during the same frame that isShuttingDown is true, it matches the disconnect query at line 58 (WithNone<NetworkStreamConnection>). Because commandBuffer.Playback has not occurred yet, the entity still has ConnectionState in the EntityManager, so it will match this query as well and invoke OnNetCodeDisconnect a second time.

Consider requiring NetworkStreamConnection here so the two disconnect queries remain mutually exclusive:

Suggested change

{

foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithAll<ConnectionState, NetworkStreamConnection>().WithEntityAccess())

_{🤖 Helpful? 👍/👎}

…scan

NoelStephensUnity added 3 commits October 16, 2026 07:35

docs: describe the clock adjustment fix by what users saw

84cbb03

NoelStephensUnity requested a review from a team as a code owner October 16, 2026 15:55

u-pr Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs Outdated

u-pr Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread com.unity.netcode.gameobjects/Runtime/Components/Helpers/UnifiedUpdateConnections.cs Outdated

EmandM approved these changes Oct 16, 2026

View reviewed changes

NoelStephensUnity enabled auto-merge (squash) October 16, 2026 18:10

u-pr Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread com.unity.netcode.gameobjects/Runtime/Components/Helpers/UnifiedUpdateConnections.cs Outdated

ci: TEMPORARY run the unified job on b1.xlarge to test the memory pre…

8738af8

…ssure theory Revert before merge.

NoelStephensUnity requested a review from a team as a code owner October 16, 2026 21:03

NoelStephensUnity added 3 commits October 16, 2026 16:50

ci: TEMPORARY run the unified job on b1.large with 6000.7.0b4 to test…

830384e

… whether 6000.7.0b6 raised memory use Replaces the b1.xlarge test. Revert before merge.

NoelStephensUnity disabled auto-merge October 16, 2026 22:58

NoelStephensUnity enabled auto-merge (squash) October 17, 2026 00:18

NoelStephensUnity added 2 commits October 16, 2026 20:31

docs: plainer wording for the comments this PR adds

0abd90c

docs: clearer description of the converged clock test

b46fac8

NoelStephensUnity requested a review from EmandM October 17, 2026 02:11

u-pr Bot reviewed Oct 17, 2026

View reviewed changes

Merge develop-3.x.x into fix/clock-overshoot-and-unified-connections-…

64c6768

…scan

NoelStephensUnity merged commit 44b7386 into develop-3.x.x Oct 17, 2026
26 checks passed

NoelStephensUnity deleted the fix/clock-overshoot-and-unified-connections-scan branch October 17, 2026 04:45

	{
	foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithAll<ConnectionState, NetworkStreamConnection>().WithEntityAccess())

Conversation

NoelStephensUnity commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this PR

Jira ticket

Changelog

Documentation

Testing & QA (How your changes can be verified during release Playtest)

Functional Testing

Up-port

Backports

Uh oh!

u-pr Bot commented Oct 16, 2026

Uh oh!

u-pr Bot left a comment

Choose a reason for hiding this comment

💡 Harness Review

Uh oh!

Uh oh!

codecov-github-com Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

NoelStephensUnity commented Oct 16, 2026

Uh oh!

u-pr Bot commented Oct 16, 2026

Review Summary

Observations & Suggestions

1. Null / Destroyed NetworkManager Safety in UnifiedUpdateConnections

2. Missing CHANGELOG Entry for UnifiedUpdateConnections

3. Defensive Step Size in NetworkTimeSystem.Advance

Uh oh!

Uh oh!

EmandM left a comment

Choose a reason for hiding this comment

Uh oh!

EmandM Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

NoelStephensUnity Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

EmandM Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

NoelStephensUnity Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

NoelStephensUnity commented Oct 16, 2026

Uh oh!

u-pr Bot commented Oct 16, 2026

Review Summary

Suggestions

1. Repeated OnNetCodeDisconnect Invocations During Extended Shutdown

2. Skip Discovery When Shutting Down

3. Unused Local Variable

Uh oh!

Uh oh!

NoelStephensUnity commented Oct 17, 2026

Uh oh!

NoelStephensUnity commented Oct 17, 2026

Uh oh!

u-pr Bot commented Oct 17, 2026

Review Summary

Suggestions

1. Mutually Exclusive Disconnect Queries during Shutdown

Uh oh!

u-pr Bot Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NoelStephensUnity commented Oct 16, 2026 •

edited

Loading

codecov-github-com Bot commented Oct 16, 2026 •

edited

Loading

1. Null / Destroyed `NetworkManager` Safety in `UnifiedUpdateConnections`

2. Missing CHANGELOG Entry for `UnifiedUpdateConnections`

3. Defensive Step Size in `NetworkTimeSystem.Advance`

1. Repeated `OnNetCodeDisconnect` Invocations During Extended Shutdown