Skip to content

fix: Disconnect event notifications #2390 #3551

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.

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

Draft
wants to merge 11 commits into
base: develop-2.0.0
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,23 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Added

- Added disconnection event notification handling capabilities where `NetworkTransport` derived custom transports can set the current disconnect event type (`NetworkTransport.DisconnectEvents`) that, if implemented, will provide more details on why the transport disconnected. (#3551)
- Added protected method `NetworkTransport.SetDisconnectEvent` that a `NetworkTransport` derived custom transport can use to provide the disconnect event type that occurred. (#3551)
- Added protected virtual method `NetworkTransport.OnGetDisconnectEventMessage` that, when overridden, a `NetworkTransport` derived custom transport can use to provide a customized extended message for each `NetworkTransport.DisconnectEvents` value. (#3551)

### Fixed

- Fixed issue where the disconnect event and provided message was too generic to know why the disconnect occurred. (#3551)
- Fixed issue where `SendTo.NotMe` could cause an RPC to be delivered to the sender when connected to a live distributed authority session. (#3551)

### Changed

- Changed `UnityTransport` now handles setting the current disconnect notification type, via internal `UnityTransportNotificationHandler` class, while also providing extended informational messages for each disconnect event type. (#3551)

## [2.5.0] - 2025-08-01

### Added

- Added serializer for `Pose` (#3546)
- Added methods `GetDefaultNetworkSettings` and `GetDefaultPipelineConfigurations` to `UnityTransport`. These can be used to retrieve the default settings and pipeline stages that are used by `UnityTransport`. This is useful when providing a custom driver constructor through `UnityTransport.s_DriverConstructor`, since it allows reusing or tuning the existing configuration instead of trying to recreate it. This means a transport with a custom driver can now easily benefit from most of the features of `UnityTransport`, like integration with the Network Simulator and Network Profiler from the multiplayer tools package. (#3501)
- Added mappings between `ClientId` and `TransportId`. (#3516)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,10 +373,14 @@ private ulong GetServerTransportId()
{
if (NetworkManager != null)
{
var transport = NetworkManager.NetworkConfig.NetworkTransport;
if (transport != null)
if (Transport == null && NetworkManager.NetworkConfig.NetworkTransport != null)
{
Transport = NetworkManager.NetworkConfig.NetworkTransport;
}

if (Transport)
{
return transport.ServerClientId;
return Transport.ServerClientId;
}

throw new NullReferenceException($"The transport in the active {nameof(NetworkConfig)} is null");
Expand Down Expand Up @@ -412,7 +416,7 @@ internal void PollAndHandleNetworkEvents()
NetworkEvent networkEvent;
do
{
networkEvent = NetworkManager.NetworkConfig.NetworkTransport.PollEvent(out ulong transportClientId, out ArraySegment<byte> payload, out float receiveTime);
networkEvent = Transport.PollEvent(out ulong transportClientId, out ArraySegment<byte> payload, out float receiveTime);
HandleNetworkEvent(networkEvent, transportClientId, payload, receiveTime);
if (networkEvent == NetworkEvent.Disconnect || networkEvent == NetworkEvent.TransportFailure)
{
Expand Down Expand Up @@ -520,6 +524,27 @@ internal void DataEventHandler(ulong transportClientId, ref ArraySegment<byte> p
#endif
}

private void GenerateDisconnectInformation(ulong clientId, ulong transportClientId, string reason = null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
private void GenerateDisconnectInformation(ulong clientId, ulong transportClientId, string reason = null)
private void GenerateDisconnectInformation(ulong clientId, string reason = null)

It looks like transportClientId isn't being used here

{
var header = $"[Disconnect Event][Client-{clientId}]";
var existingDisconnectReason = DisconnectReason;

var defaultMessage = Transport.DisconnectEventMessage;
if (reason != null)
{
defaultMessage = $"{reason} {defaultMessage}";
}
// Just go ahead and set this whether client or server so any subscriptions to a disconnect event can check the DisconnectReason
// to determine why the client disconnected
DisconnectReason = $"{header}[{Transport.DisconnectEvent}] {defaultMessage}";
DisconnectReason = $"{DisconnectReason}\n{existingDisconnectReason}";

if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo($"{DisconnectReason}");
}
}

/// <summary>
/// Handles a <see cref="NetworkEvent.Disconnect"/> event.
/// </summary>
Expand All @@ -528,11 +553,8 @@ internal void DisconnectEventHandler(ulong transportClientId)
#if DEVELOPMENT_BUILD || UNITY_EDITOR
s_TransportDisconnect.Begin();
#endif
var clientId = TransportIdCleanUp(transportClientId);
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo($"Disconnect Event From {clientId}");
}
var clientId = TransportIdToClientId(transportClientId);
TransportIdCleanUp(transportClientId);

// If we are a client and we have gotten the ServerClientId back, then use our assigned local id as the client that was
// disconnected (either the user disconnected or the server disconnected, but the client that disconnected is the LocalClientId)
Expand All @@ -541,6 +563,14 @@ internal void DisconnectEventHandler(ulong transportClientId)
clientId = NetworkManager.LocalClientId;
}

// If the disconnect is due to the transport being shutdown and we have received a notification
// from transport that we have disconnected, then we are a client that has shutdown the NetworkManager
// and there is no need to generate any disconnect information as all of that should already be set at this point.
if (Transport.DisconnectEvent != NetworkTransport.DisconnectEvents.TransportShutdown)
{
GenerateDisconnectInformation(clientId, transportClientId);
}

// Process the incoming message queue so that we get everything from the server disconnecting us or, if we are the server, so we got everything from that client.
MessageManager.ProcessIncomingMessageQueue();

Expand Down Expand Up @@ -1327,7 +1357,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
if (ClientIdToTransportIdMap.ContainsKey(clientId))
{
var transportId = ClientIdToTransportId(clientId);
NetworkManager.NetworkConfig.NetworkTransport.DisconnectRemoteClient(transportId);
Transport.DisconnectRemoteClient(transportId);

InvokeOnClientDisconnectCallback(clientId);

Expand Down Expand Up @@ -1394,9 +1424,14 @@ internal void DisconnectClient(ulong clientId, string reason = null)
SendMessage(ref disconnectReason, NetworkDelivery.Reliable, clientId);
}

Transport.ClosingRemoteConnection();
GenerateDisconnectInformation(clientId, ClientIdToTransportId(clientId), reason);
DisconnectRemoteClient(clientId);
}

internal NetworkTransport Transport;
internal NetworkTransport.DisconnectEvents DisconnectEvent => Transport ? Transport.DisconnectEvent : NetworkTransport.DisconnectEvents.Disconnected;

/// <summary>
/// Should be invoked when starting a server-host or client
/// </summary>
Expand All @@ -1418,24 +1453,35 @@ internal void Initialize(NetworkManager networkManager)
NetworkManager = networkManager;
MessageManager = networkManager.MessageManager;

NetworkManager.NetworkConfig.NetworkTransport.NetworkMetrics = NetworkManager.MetricsManager.NetworkMetrics;

NetworkManager.NetworkConfig.NetworkTransport.OnTransportEvent += HandleNetworkEvent;
NetworkManager.NetworkConfig.NetworkTransport.Initialize(networkManager);
Transport = NetworkManager.NetworkConfig.NetworkTransport;
if (Transport)
{
Transport.NetworkMetrics = NetworkManager.MetricsManager.NetworkMetrics;
Transport.OnTransportEvent += HandleNetworkEvent;
Transport.Initialize(networkManager);
}
}

/// <summary>
/// Should be called when shutting down the NetworkManager
/// </summary>
internal void Shutdown()
{
if (Transport && IsListening)
{
Transport.ShuttingDown();
var clientId = NetworkManager ? NetworkManager.LocalClientId : NetworkManager.ServerClientId;
var transportId = ClientIdToTransportId(clientId);
GenerateDisconnectInformation(clientId, transportId, $"{nameof(NetworkConnectionManager)} was shutdown.");
}

if (LocalClient.IsServer)
{
// Build a list of all client ids to be disconnected
var disconnectedIds = new HashSet<ulong>();

//Don't know if I have to disconnect the clients. I'm assuming the NetworkTransport does all the cleaning on shutdown. But this way the clients get a disconnect message from server (so long it does't get lost)
var serverTransportId = NetworkManager.NetworkConfig.NetworkTransport.ServerClientId;
var serverTransportId = GetServerTransportId();
foreach (KeyValuePair<ulong, NetworkClient> pair in ConnectedClients)
{
if (!disconnectedIds.Contains(pair.Key))
Expand Down Expand Up @@ -1478,7 +1524,7 @@ internal void Shutdown()
// Client only, send disconnect and if transport throws and exception, log the exception and continue the shutdown sequence (or forever be shutting down)
try
{
NetworkManager.NetworkConfig.NetworkTransport.DisconnectLocalClient();
Transport?.DisconnectLocalClient();
}
catch (Exception ex)
{
Expand Down Expand Up @@ -1509,7 +1555,6 @@ internal void Shutdown()
if (transport != null)
{
transport.Shutdown();

if (NetworkManager.LogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo($"{nameof(NetworkConnectionManager)}.{nameof(Shutdown)}() -> {nameof(IsListening)} && {nameof(NetworkManager.NetworkConfig.NetworkTransport)} != null -> {nameof(NetworkTransport)}.{nameof(NetworkTransport.Shutdown)}()");
Expand Down
14 changes: 12 additions & 2 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
using System.Collections.Generic;
using Unity.Collections;
using System.Linq;
using Unity.Netcode.Components;
using UnityEngine;
#if UNITY_EDITOR
using UnityEditor;
using PackageInfo = UnityEditor.PackageManager.PackageInfo;
#endif
using UnityEngine.SceneManagement;
using Debug = UnityEngine.Debug;
using Unity.Netcode.Components;

namespace Unity.Netcode
{
Expand Down Expand Up @@ -611,6 +611,16 @@ public ulong LocalClientId
/// </summary>
public string DisconnectReason => ConnectionManager.DisconnectReason;

/// <summary>
/// If supported by the <see cref="NetworkTransport"/>, this <see cref="NetworkTransport.DisconnectEvents"/> property will be set for each disconnect event.
/// If not supported, then this remain as the default <see cref="Networking.Transport.Error.DisconnectReason"/> value.
/// </summary>
/// <remarks>
/// A server/host will receive notifications for remote clients disconnecting and will update this <see cref="Networking.Transport.Error.DisconnectReason"/> property
/// upon each disconnect event.<br />
/// </remarks>
public NetworkTransport.DisconnectEvents DisconnectEvent => ConnectionManager.DisconnectEvent;

/// <summary>
/// Is true when a server or host is listening for connections.
/// Is true when a client is connecting or connected to a network session.
Expand Down Expand Up @@ -1485,7 +1495,7 @@ private void HostServerInitialize()
/// Disconnects the remote client.
/// </summary>
/// <param name="clientId">The ClientId to disconnect</param>
public void DisconnectClient(ulong clientId) => ConnectionManager.DisconnectClient(clientId);
public void DisconnectClient(ulong clientId) => ConnectionManager.DisconnectClient(clientId, $"Client-{clientId} disconnected by server.");

/// <summary>
/// Disconnects the remote client.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,11 @@ internal override void Send(NetworkBehaviour behaviour, ref RpcMessage message,
{
continue;
}
// In distributed authority mode, we send to target id 0 (which would be a DAHost) via the group
if (clientId == NetworkManager.ServerClientId && !m_NetworkManager.DistributedAuthorityMode)
// In distributed authority mode, we send to target id 0 (which would be a DAHost).
// We only add when there is a "DAHost" by
// - excluding the server id when using client-server (i.e. !m_NetworkManager.DistributedAuthorityMode )
// - excluding if connected to the CMB backend service (i.e. we don't want to send to service as it will broadcast it back)
if (clientId == NetworkManager.ServerClientId && (!m_NetworkManager.DistributedAuthorityMode || m_NetworkManager.CMBServiceConnection))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seeing as we can't reproduce this error I think it's maybe best to remove the changes in this file 🙃

{
continue;
}
Expand Down
101 changes: 101 additions & 0 deletions com.unity.netcode.gameobjects/Runtime/Transports/NetworkTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,107 @@ internal NetworkTopologyTypes CurrentTopology()
{
return OnCurrentTopology();
}

/// <summary>
/// The Netcode for GameObjects standardized disconnection event types.
/// </summary>
/// <remarks>
/// <see cref="AddDisconnectEventMap"/> provides you with the ability to register the transport's disconnect event types with the local equivalent.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The AddDisconnectEventMap function is defined in UnityTransport rather than here in NetworkTransport. From this code doc it looks like you've intended for it to live in NetworkTransport?

/// </remarks>
public enum DisconnectEvents
{
/// <summary>
/// If transport has mapped its disconnect events, this event signifies that the transport closed the connection due to a locally invoked shutdown.
/// </summary>
TransportShutdown,
/// <summary>
/// If transport has mapped its disconnect events, this event signifies a graceful disconnect.
/// </summary>
Disconnected,
/// <summary>
/// If transport has mapped its disconnect events, this event signifies that the transport's connection to the endpoint has timed out and the connection was closed.
/// </summary>
ProtocolTimeout,
/// <summary>
/// If transport has mapped its disconnect events, this event signifies that the disconnect is due to the maximum number of failed connection attempts has been reached.
/// </summary>
MaxConnectionAttempts,
/// <summary>
/// If transport has mapped its disconnect events, this event signifies that the remote endpoint closed the connection.
/// </summary>
ClosedByRemote,
/// <summary>
/// If transport has mapped its disconnect events, this event signifies the local transport closed the incoming remote endpoint connection.
/// </summary>
ClosedRemoteConnection,
/// <summary>
/// If transport has mapped its disconnect events, this event signifies that the connection was closed due to an authentication failure.
/// </summary>
AuthenticationFailure,
/// <summary>
/// If transport has mapped its disconnect events, this event signifies that a lower-level (unkown) transport error occurred.
/// </summary>
ProtocolError,
}

/// <summary>
/// If the transport has implemented disconnection event mapping, then this will be set to the most recent disconnection event.
/// </summary>
public DisconnectEvents DisconnectEvent { get; private set; }

/// <summary>
/// If the transport has implemented disconnection event mapping and disconnection event message mapping, then this will contain
/// the transport specific message associated with the disconnect event type.
/// </summary>
public string DisconnectEventMessage { get; private set; }

/// <summary>
/// This should be invoked by the <see cref="NetworkTransport"/> derived class when a transport level disconnect event occurs.<br />
/// It is up to the <see cref="NetworkTransport"/> derived class to create a map between the transport's disconnect events and the
/// pre-defined <see cref="DisconnectEvents"/> enum values.
/// </summary>
/// <param name="disconnectEvent">The <see cref="DisconnectEvents"/> type to set.</param>
/// <param name="message">An optional message override.</param>
protected void SetDisconnectEvent(DisconnectEvents disconnectEvent, string message = null)
{
DisconnectEvent = disconnectEvent;
DisconnectEventMessage = string.Empty;

if (message != null)
{
DisconnectEventMessage = message;
}
else
{
DisconnectEventMessage = OnGetDisconnectEventMessage(disconnectEvent);
}
}

/// <summary>
/// Override this method to provide additional information about the disconnection event.
/// </summary>
/// <param name="disconnectEvent">The disconnect event to get from the <see cref="NetworkTransport"/> derived class.</param>
/// <returns><see cref="string.Empty"/> as a default or if overridden the <see cref="string"/> returned.</returns>
protected virtual string OnGetDisconnectEventMessage(DisconnectEvents disconnectEvent)
{
return string.Empty;
}

/// <summary>
/// Invoked when the local <see cref="NetworkManager"/> forces the transport to close a remote connection.
/// </summary>
internal void ClosingRemoteConnection()
{
SetDisconnectEvent(DisconnectEvents.ClosedRemoteConnection);
}

/// <summary>
/// Invoked just before the transport is shutdown.
/// </summary>
internal void ShuttingDown()
{
SetDisconnectEvent(DisconnectEvents.TransportShutdown);
}
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ private struct MessageData

private static Dictionary<ulong, Queue<MessageData>> s_MessageQueue = new Dictionary<ulong, Queue<MessageData>>();

private bool m_Initialized;
private ulong m_TransportId = 0;
private NetworkManager m_NetworkManager;

Expand Down
Loading