Repository navigation
feat: hybrid NetcodeConfig defaults - #4144
Conversation
When Netcode for Entities is installed and a registered network prefab carries a GhostObject, the project's NetCodeConfig is now aligned with what NGO needs rather than leaving it to the user to discover. The two settings hybrid mode cannot run without (single world hosting, and automatic bootstrapping disabled since NetworkManager owns world creation) are corrected whenever they drift. The Netcode for Entities tick rates are driven from NetworkConfig.TickRate so that ghost transform updates land on the same interval as everything else. The snapshot, interpolation and transport values tuned against the 2000 instance stress test are applied once and then left alone, so a user's own edits survive; Project Settings > Multiplayer > Netcode for GameObjects can restore them. The config is never created here. Netcode for Entities already creates one unconditionally from its own InitializeOnLoadMethod, and creating a second lands the project in its multiple-config error path. Also: - UnifiedIsConfiguredCorrectly now validates EnableClientServerBootstrap and warns when the two tick rates diverge. Its two log messages were missing their string interpolation prefixes. - m_TempStreamSize returns to the Netcode for Entities default of 8192. GhostSendSystem takes max(TempStreamInitialSize, dataStream.Capacity), and capacity is DefaultSnapshotPacketSize, so 4192 had no effect. - Unity.Netcode.Editor.Tests gains the UNIFIED_NETCODE version define. Without it any hybrid editor test compiles away and reports zero cases rather than failing. - Adds a measurement fixture for sizing DefaultSnapshotPacketSize. It is marked Explicit so its 24 cases stay out of the suites.
![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 new hybrid configuration flow establishes the intended defaults and exposes them in settings, but several reachable editor states prevent it from preserving or aligning those defaults correctly.
Reviewed commit 49e605e
🤖 Helpful? 👍/👎
Fixing some things u-pr caught and adding a test to validate the update.
|
Is it possible to add a call to action to the console log that the user can click on the directly open the relevant page? |
I was actually thinking that we should make that a dialog box that explains what was updated and where to reset back to the defaults. The console log could easily be missed.
This would be a change to the NetcodeConfig property drawer in N4E. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop-3.x.x #4144 +/- ##
=================================================
+ Coverage 78.26% 78.36% +0.09%
=================================================
Files 153 154 +1
Lines 26359 26630 +271
=================================================
+ Hits 20631 20868 +237
- Misses 5728 5762 +34
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes
|
Ah ok ok! Yea, a helpbox with some Call-to-action would make more sense.
Yea, we could look at the workflow a bit closer |
EmandM
left a comment
EmandM
left a comment
There was a problem hiding this comment.
Big concern here with using AssetDatabase.FindAssets.
- It's very slow
- The pattern will change and/or go away as the editor moves towards importing on demand.
Relevant assets should register themselves in OnEnable, that way behaviour will be automatically triggered when relevant rather than doing an expensive search over many irrelevant things.
| /// snapshot metrics singleton for a fixed sample window. Results are emitted as "PKTSZ|" log lines. | ||
| /// </summary> | ||
| [TestFixture(HostOrServer.UnifiedHost)] | ||
| [Explicit("Measurement harness, not a regression test. The 24 auto-expanded cases take ~162s, so it only runs when selected by name: -testFilter \".*UnifiedSnapshotPacketSizeMeasurement.*\"")] |
There was a problem hiding this comment.
Can we not add tests that we have to explictly run. We're going to forget they exist and never run them
There was a problem hiding this comment.
The explicit attribute:
Measurement harness, not a regression test.
It is just a tool for measurement purposes. If you would like me to remove this tool I can do that.
(I can keep a copy of this somewhere else so I can run this to verify the snapshot sizes)
There was a problem hiding this comment.
What is it measuring? When do we need to be measuring? Might be worth writing some internal docs explaining that the tool exists and what it does?
There was a problem hiding this comment.
Measurement harness (not a pass/fail behaviour test) used to determine bandwidth consumption based on the
NetCode.GhostSendSystemData.DefaultSnapshotPacketSizewhen running in hybrid mode.
It measures what the total size of the snapshot that is consumed by hybrid prefab transform synchronization. It is a metric that can be used against non-hybrid instances to determine the bandwidth savings.
Pull request was converted to draft
There was a problem hiding this comment.
💡 Harness Review
The one-shot defaults flow and runtime tick alignment are coherent for hybrid sessions, but the editor initialization also changes the shared Netcode for Entities configuration in projects that never use hybrid prefabs.
Reviewed commit f12c046
🤖 Helpful? 👍/👎
None of this script exists anymore. |
…netcodeconfig-defaults
Netcode for GameObjects no longer writes anything into the NetcodeConfig asset. NetworkManager writes the in-memory global config immediately before the world is created from it, and Netcode for Entities seeds its world singletons from that. Two values are driven, both measured rather than assumed: - The tick rates. NGO's own send queues are flushed by a system in N4E's SimulationSystemGroup, so a simulation rate below NetworkConfig.TickRate starves NGO's outbound traffic. N4E defaults to 60 and NGO to 30. - The snapshot packet size, behind the new NetworkConfig.AutoConfigureSnapshotSize. N4E's default of one MTU round-robins above roughly 230 GhostObjects: at 512 instances it holds a 13.51Hz effective update rate against a 30Hz tick, where 4096 holds 30.00Hz with no saturated snapshots. Below that count the cap is never reached, so it costs nothing. UnifiedBootstrap no longer delegates to base.Initialize. Entities selects the most derived ICustomBootstrap, so this type is the only one it instantiates under UNIFIED_NETCODE and N4E's own Initialize is never reached on its own. Calling it only gave N4E a chance to create worlds this bootstrap would then reject, which is why EnableClientServerBootstrap had to be configured at all. It no longer does. Removed with it: the editor applier, the Project Settings section, the applied-defaults version marker, and the interpolation, despawn reservation and transport queue values. Those were tuned against a 2000 instance stress test and are either inside N4E's own recommended ranges or inert below the snapshot cap. The FailValidation and ClientDoesNotRequest hybrid test cases are removed pending a separate fix: the server schedules its disconnect reason into N4E's outgoing RPC buffer and then tears the connection down in the same call, before RpcSystem transmits it.
The polled disconnect reason check belongs with the fix for the defect it found, not with the NetcodeConfig changes. Only the removal of the FailValidation hybrid fixture stays here.
Drops a stray blank line that was the only remaining change in NetcodeForGameObjectsProjectSettings, so that file leaves the diff. Trims the comments that carried mechanism or measurements the PR description already covers, keeping the reason each value or omission exists.
| return true; | ||
| } | ||
|
|
||
| // NetworkManager owns world creation here, so base.Initialize is deliberately not called: it would |
There was a problem hiding this comment.
Context for the removed base.Initialize call, since the reason is not visible from the diff.
Entities selects the most derived ICustomBootstrap (DefaultWorldInitialization.CreateBootStrap), so UnifiedBootstrap is the only one it instantiates whenever UNIFIED_NETCODE is set, and Netcode for Entities' own ClientServerBootstrap.Initialize is never reached on its own. That method's entire body is:
if (!DetermineIfBootstrappingEnabled())
return false;
CreateDefaultClientServerWorlds();
return true;DetermineIfBootstrappingEnabled() is the only reader of EnableClientServerBootstrap. So calling the base was Netcode for GameObjects asking Netcode for Entities to create worlds and then rejecting them when it did — which is the only reason that setting had to be configured at all. Not calling it removes the requirement rather than working around it.
Verified with a control: with EnableAutomaticBootstrap set (the Netcode for Entities default), the UnifiedHost case passes without the base call and fails with it.
The body is identical in 7.0.0 and 7.1.0.
|
@u-pr please review again |
SummaryThis updated revision is a significant improvement over earlier iterations. By eliminating the editor-time asset searching ( Highlights
Suggestions
🤖 Helpful? 👍/👎 |
…netcodeconfig-defaults # Conflicts: # com.unity.netcode.gameobjects/CHANGELOG.md # com.unity.netcode.gameobjects/Tests/Runtime/Unified.meta
…ts' into feat/hybrid-netcodeconfig-defaults # Conflicts: # com.unity.netcode.gameobjects/CHANGELOG.md
…netcodeconfig-defaults # Conflicts: # com.unity.netcode.gameobjects/CHANGELOG.md
Updated comments for clarity and accuracy regarding hybrid mode settings and tick rate application in HybridNetcodeDefaults.
Updated summary and remarks in UnifiedApplyNetcodeConfig method to clarify the application of NetcodeConfig settings during runtime.
EmandM
left a comment
EmandM
left a comment
There was a problem hiding this comment.
I'd love to make that default snapshot size configurable rather than a constant, but this is a great first step!
5 participants
Purpose of this PR
This PR addresses Netcode for GameObjects' hybrid mode operation requirements for the two NetcodeConfig settings, disabling auto bootstrap and tick rate frquency, while also providing users with an optional snapshot size configuration of 4096 bytes (more info about these below).
Jira ticket
MTT-15571
PR Scope:
Required runtime NetcodeConfig adjustments:
Tick rates follow
NetworkConfig.TickRate. Netcode for GameObjects' own send queues are flushed by a system in Netcode for Entities'SimulationSystemGroup, so a simulation rate belowNetworkConfig.TickRatestarves outbound traffic. Netcode for Entities defaults to 60 and Netcode for GameObjects to 30.UnifiedBootstrapno longer delegates tobase.Initialize. Entities selects the most derivedICustomBootstrap, so this type is the only one it instantiates underUNIFIED_NETCODEand Netcode for Entities' ownInitializeis never reached on its own. Calling it only gave Netcode for Entities a chance to create worlds this bootstrap would then reject, which is whyEnableClientServerBootstraphad to be configured at all. It no longer does.Optional runtime NetcodeConfig adjustments:
Snapshot packet size, behind the new
NetworkConfig.AutoConfigureSnapshotSize. Netcode for Entities defaults to a single MTU, which round-robins above roughly 230 GhostObjects. Measured at 512 instances against a 30Hz tick rate:Below that count the cap is never reached, so it costs nothing. Users can disable this option and configure it themselves in their project's
NetcodeConfig.This property is placed into its own section within the
NetworkManagerinspector view for easier discoverability:Test harness fix (blocks #4186):
m_AllPrefabsAsHybridis restored after every test case. A fixture that takes itsHostOrServeras a method parameter sets the flag from the test body, butSetUpruns before that. After anyUnified*case the flag stayed set, so the next case created a hybrid prefab nobody asked for and the server start sweep rejected it as an unregisteredNetworkObject. The flag is captured when the fixture is constructed and restored inTearDown(restored, not cleared: a[TestFixture(HostOrServer.UnifiedHost)]sets it in its constructor and needs it for every case in the fixture).This only reproduces where
UNIFIED_NETCODEis compiled and the ordinary suite runs, which is why CI has not shown it until now. It accounts for the 96NetworkVariableTestsfailures on #4186.Out of scope:
UnifiedSnapshotPacketSizeMeasurementis removed (an[Explicit]measurement harness that CI never selects, so it only added review surface; it moves to the NGO Examples project).testproject'sNetcodeConfigis left in place (every value in it is now overwritten at session start or already equal to the Netcode for Entities default. The asset itself is removed in chore: enable unified netcode from N4E 7.2.0, only for hybrid prefab sessions #4186).Issues found, but were not fixed for this pass and require a follow-up PR
The server schedules its disconnect reason into Netcode for Entities' outgoing RPC buffer and then tears the connection down in the same call, before
RpcSystemtransmits it, so the client never receives the reason.The
FailValidationandClientDoesNotRequesthybrid prefab test cases are excluded until that is fixed.Changelog
NetworkConfig.TickRatewhen a session carryingGhostObjectprefabs is started.NetworkConfig.AutoConfigureSnapshotSize, enabled by default, which sets the Netcode for Entities snapshot packet size to 4096 bytes when a session carryingGhostObjectprefabs is started.NetcodeConfigvalidation messages were not interpolated.NetcodeConfig.EnableClientServerBootstrapwas set toDisableAutomaticBootstrap.Documentation
Testing & QA (How your changes can be verified during release Playtest)
Functional Testing
Manual testing :
Manual testing doneGhostObject.NetcodeConfigasset: it is unchanged on disk.NetworkManagertick rate for the Netcode for Entities tick rates, and a snapshot packet size of 4096 unlessNetworkConfig.AutoConfigureSnapshotSizeis disabled.Automated 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?Up-port
None
Backports
None