Repository navigation
fix: invoke OnNetCodeDisconnect when a hybrid connection closes - #4209
Conversation
A destroyed connection entity keeps only its cleanup components, so the query requiring NetworkId alongside ConnectionState never matched and the disconnect was only reported at shutdown. Read the id from ConnectionState instead, and set it when the connection goes in game.
![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 change caches connection IDs for disconnect reporting after entity cleanup and adds hybrid host/server regression coverage. I examined connection updates, shutdown handling, event subscribers, test lifecycle, and release notes, and found no actionable issues.
Reviewed commit 4a84c3e
🤖 Helpful? 👍/👎
There was a problem hiding this comment.
💡 Harness Review
The change preserves the connection ID for disconnect callbacks after entity destruction. I examined the cleanup flow, transport shutdown behavior, and hybrid test setup; I found no actionable issues.
Reviewed commit 4a84c3e
🤖 Helpful? 👍/👎
Codecov ReportAttention: Patch coverage is @@ Coverage Diff @@
## develop-3.x.x #4209 +/- ##
=================================================
+ Coverage 75.93% 76.25% +0.32%
=================================================
Files 159 159
Lines 27405 27538 +133
=================================================
+ Hits 20809 21000 +191
+ Misses 6596 6538 -58
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes
|
Pull request was converted to draft
There was a problem hiding this comment.
💡 Harness Review
The change preserves connection IDs for hybrid disconnect notifications and allows empty prefab rejection messages. I examined the connection lifecycle, prefab registration guards, and new tests against their setup and teardown behavior, and found no actionable issues.
Reviewed commit 1dcf26e
🤖 Helpful? 👍/👎
Netcode for Entities now puts connections in game automatically by default (NetcodeConfig.AutomaticStateReplication), which stops UnifiedUpdateConnections from seeing new connections: no connect events, no in-session disconnects, and ghosts stream to a client before NGO has synchronized it. Disable it at runtime alongside the other hybrid NetcodeConfig settings.
Purpose of this PR
In a hybrid mode session,
NetworkManager.OnNetCodeDisconnectwas only invoked when the session shut down, not when a connection closed.When N4E destroys a connection entity, only its cleanup components remain.
UnifiedUpdateConnectionsqueried forNetworkIdtogether withConnectionState, so it never matched a closed connection. TheNetworkIdis now stored inConnectionStatewhen the connection goes in game and read from there on disconnect.Jira ticket
N/A
Changelog
NetworkManager.OnNetCodeDisconnectwas not invoked when a connection closed during a hybrid mode session, only when the session shut down.Documentation
Testing & QA (How your changes can be verified during release Playtest)
Functional Testing
Automated tests:
Covered by new automated testsHybridConnectionEventsTests(UnifiedHost and UnifiedServer) checks the connect event fires once the client is synchronized and the server reports a client disconnect while the session is still running.Up-port
Not needed; hybrid mode is NGO 3.x only.
Backports
Not needed.