Skip to content

fix: client and server disconnect warnings #3786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 8 commits into
base: develop-2.0.0
Choose a base branch
from
Open

fix: client and server disconnect warnings #3786

wants to merge 8 commits into from

Conversation

@NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Nov 14, 2025 •
edited
Loading

Purpose of this PR

This fix:

  • Prevents the warning message from being logged on the client side when shutting down from the client side and the developer log level is set.
  • Assures that a server or host does not send itself a disconnect message.
  • Separates the transport id clean up between client and host/server upon handling a disconnect event to assure the server/host has access to this transport id when processing a client that disconnected.
  • Defers the complete disconnection, server side, to the end of the frame when disconnecting a client with a reason in order to assure the disconnect reason is sent prior to closing the transport connection.

Jira ticket

MTTB-1739

Changelog

  • Changed: When a server is disconnecting a client with a reason it now defers the complete transport disconnect sequence until the end of the frame after the server's transport has sent all pending outbound messages.
  • Fixed: Issue where a warning message was being logged upon a client disconnecting from a server when the log level is set to developer.
  • Fixed: Issue where the server or host would no longer have access to the transport id to client id table when processing a transport level client disconnect event.

Documentation

  • No documentation changes or additions were necessary.

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

Functional Testing

Manual testing :

Replication Project

image

Validating the Fix

  • Open the project.
  • Start at least 1 MPPM virtual client.
  • Enter play mode.
  • Once the client is connected:
    • (for simplicity) Clear both the editor and virtual client logs.
    • Click the Disconnect button on the client side.
      • No warnings should be logged
      • image
    • Click the Shutdown button on the host side.
      • No warnings should be logged
      • image

Replicating the issue

  • Switch to an earlier version of NGO (i.e. v2.7.0) via package manager--> add package by name
    • Name: com.unity.netcode.gameobjects
    • Version: 2.7.0
  • Repeat the above steps
    • Clicking Disconnect on the client side should generate a warning log on the client and two on the host.
    • image

Automated tests:

  • Covered by existing modified automated tests
  • Covered by new automated tests
    • VerifyNoWarningOnClientDisconnect

Does 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.

Backports

This is a v2.x only issue.

@NoelStephensUnity
fix
145b591 
This fix prevents the warning message from being logged on the client side when shutting down from the client side and the developer log level is set.
This fix also assures that a server or host does not send itself a disconnect message.
This fix separated the transport id clean up between client and host/server upon handling a disconnect event which assures the server/host will have access to this transport id when processing a client that disconnected.
@NoelStephensUnity
update
63c82c0 
Adding change log entries.
@NoelStephensUnity
test
cea8998 
Adding test to check that no warning messages are logged upon a client disconnecting itself from a session.
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review November 16, 2025 19:59
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner November 16, 2025 19:59
@NoelStephensUnity NoelStephensUnity enabled auto-merge (squash) November 16, 2025 19:59
@NoelStephensUnity
refactor and fix
610e2d6 
Fixing several issues with the disconnect flow.
Not removing the transport id until later. The server now only does this within OnClientDisconnectFromServer.
When disconnecting a client with a reason, the client is not completely disconnected until the end of the frame when the server has finished sending all pending messages.
Fixing some missed MessageDeliveryType replacements.
Removing the MockSkippingApproval as it is no longer needed with the fixes in place.
@NoelStephensUnity
test
0c67142 
Did a bunch of cleanup on several connection tests to improve stability and use more recent integration test features.
ClientOnlyConnectionTests reduced the transport connection timeout and increased the timeout helper.
ConnectionApprovalTimeoutTests needed to have the server and client timeout values vary depending upon the test type.
PeerDisconnectCallbackTests needed to trap for the peer disconnecting.
@NoelStephensUnity
Merge branch 'develop-2.0.0' into fix/server-sending-disconnectmessag…
e3a614e 
…e-to-self
@NoelStephensUnity
style
c3d00c1 
Removing whitespaces
@NoelStephensUnity
update
4c0ab7c 
adding one more change log entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@NoelStephensUnity