Skip to content

fix: invoke OnNetCodeDisconnect when a hybrid connection closes - #4209

Open
NoelStephensUnity wants to merge 8 commits into
develop-3.x.xfrom
fix/hybrid-disconnect-events
Open

NoelStephensUnity wants to merge 8 commits into
develop-3.x.xfrom
fix/hybrid-disconnect-events

Conversation

@NoelStephensUnity

Copy link
Copy Markdown
Member

Purpose of this PR

In a hybrid mode session, NetworkManager.OnNetCodeDisconnect was only invoked when the session shut down, not when a connection closed.

When N4E destroys a connection entity, only its cleanup components remain. UnifiedUpdateConnections queried for NetworkId together with ConnectionState, so it never matched a closed connection. The NetworkId is now stored in ConnectionState when the connection goes in game and read from there on disconnect.

Jira ticket

N/A

Changelog

  • Fixed: Issue where NetworkManager.OnNetCodeDisconnect was not invoked when a connection closed during a hybrid mode session, only when the session shut down.

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

HybridConnectionEventsTests (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.

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.
@NoelStephensUnity
NoelStephensUnity requested a review from a team as a code owner October 22, 2026 16:51

@u-pr u-pr Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Harness Review

Ship it

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? 👍/👎

@NoelStephensUnity
NoelStephensUnity marked this pull request as draft October 22, 2026 17:01
@NoelStephensUnity
NoelStephensUnity marked this pull request as ready for review October 22, 2026 17:01

@u-pr u-pr Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Harness Review

Ship it

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-github-com

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

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ime/Components/Helpers/UnifiedUpdateConnections.cs 0.00% 2 Missing ⚠️
@@                Coverage Diff                @@
##           develop-3.x.x    #4209      +/-   ##
=================================================
+ Coverage          76.03%   76.05%   +0.02%     
=================================================
  Files                159      159              
  Lines              27344    27341       -3     
=================================================
+ Hits               20791    20795       +4     
+ Misses              6553     6546       -7     
Flag Coverage Δ
NGOv2_project_testproject_ubuntu 75.11% <0.00%> (+0.02%) ⬆️

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

Files with missing lines Coverage Δ
...ime/Components/Helpers/UnifiedUpdateConnections.cs 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

Components Coverage Δ
com.unity.netcode.gameobjects 76.05% <82.14%> (+0.02%) ⬆️

ℹ️ Need help interpreting these results?

@EmandM
EmandM enabled auto-merge (squash) October 26, 2026 21:29

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants