fix: Disconnect event notifications #2390 (#3551)

* update

This update is a work in progress resolution to the issue where disconnect event notifications were too vague and did not provide any way to know what the cause of the disconnect was.

* Update

Adding some final adjustments and (hopefully) improving the disconnect event messaging upon a local client or server shutting down.

* fix

When using NotMeRpc, make sure we only send to connected clients and not the service to avoid infinite spamming of the RPC.

* update

Adding additional comments and disconnect event specific messages for UnityTransport.

* style - PVP

Missed a parameter in the XML-API

* test - fix

Fixing some minor test issues after the disconnect notification updates.

* test

Updating the multi-client test.

* update

Making adjustments based on peer review of this branch's PR.

* update

Adding changelog entries.
Trying a possible new "tag" unreleased format.

* update

Trying out just marking it with a temporary release date.

* update

Continuing work on suggested updates to disconnect event notifications.

* update

Adjustments for Emma's suggestions/findings.

* update

Some more minor adjustments based on PR feedback.

* update

Making some minor changelog entry modifications based on recent updates.

* update

Some additional adjustments for readability and clarity.

* update

Simplifying the GetDisconnectEventMessage entry.

* update

bumping up to v2.6.0 due to API addition.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -28,6 +28,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Multiple disconnect events from the same transport will no longer disconnect the host. (#3707)
 - Distributed authority clients no longer send themselves in the `ClientIds` list when sending a `ChangeOwnershipMessage`. (#3687)
 - Made a variety of small performance improvements. (#3683)
+- Fixed issue where the disconnect event and provided message was too generic to know why the disconnect occurred. (#3551)
 
 ### Security
 

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

@@ -375,10 +375,14 @@ private ulong GetServerTransportId()
         {
             if (NetworkManager != null)
             {
-                var transport = NetworkManager.NetworkConfig.NetworkTransport;
-                if (transport != null)
+                if (Transport == null && NetworkManager.NetworkConfig.NetworkTransport != null)
                 {
-                    return transport.ServerClientId;
+                    Transport = NetworkManager.NetworkConfig.NetworkTransport;
+                }
+
+                if (Transport)
+                {
+                    return Transport.ServerClientId;
                 }
 
                 throw new NullReferenceException($"The transport in the active {nameof(NetworkConfig)} is null");
@@ -419,7 +423,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)
                 {
@@ -530,6 +534,27 @@ internal void DataEventHandler(ulong transportClientId, ref ArraySegment<byte> p
 #endif
         }
 
+        private void GenerateDisconnectInformation(ulong clientId, ulong transportClientId, string reason = null)
+        {
+            var header = $"[Disconnect Event][Client-{clientId}][TransportClientId-{transportClientId}]";
+            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>
@@ -557,6 +582,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();
 
@@ -1397,9 +1430,28 @@ internal void DisconnectClient(ulong clientId, string reason = null)
                 return;
             }
 
+            if (!string.IsNullOrEmpty(reason))
+            {
+                var disconnectReason = new DisconnectReasonMessage
+                {
+                    Reason = reason
+                };
+                SendMessage(ref disconnectReason, NetworkDelivery.Reliable, clientId);
+            }
+
+            Transport.ClosingRemoteConnection();
+            var transportId = ClientIdToTransportId(clientId);
+            if (transportId.Item2)
+            {
+                GenerateDisconnectInformation(clientId, transportId.Item1, reason);
+            }
+
             HandleConnectionDisconnect(clientId, reason);
         }
 
+        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>
@@ -1421,24 +1473,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.Item1, $"{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))
@@ -1481,7 +1544,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)
                 {
@@ -1512,7 +1575,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)}()");

com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs

@@ -2,15 +2,15 @@
 using System.Collections.Generic;
 using Unity.Collections;
 using System.Linq;
+using Unity.Netcode.Components;
+using Unity.Netcode.Runtime;
 using UnityEngine;
 #if UNITY_EDITOR
 using UnityEditor;
 using PackageInfo = UnityEditor.PackageManager.PackageInfo;
 #endif
 using UnityEngine.SceneManagement;
 using Debug = UnityEngine.Debug;
-using Unity.Netcode.Components;
-using Unity.Netcode.Runtime;
 
 namespace Unity.Netcode
 {
@@ -613,6 +613,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.
@@ -1494,7 +1504,7 @@ public ulong GetClientIdFromTransportId(ulong transportId)
         /// 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.

com.unity.netcode.gameobjects/Runtime/Messaging/RpcTargets/NotMeRpcTarget.cs

@@ -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))
                     {
                         continue;
                     }

com.unity.netcode.gameobjects/Runtime/Transports/NetworkTransport.cs

@@ -164,6 +164,104 @@ internal NetworkTopologyTypes CurrentTopology()
         {
             return OnCurrentTopology();
         }
+
+        /// <summary>
+        /// The Netcode for GameObjects standardized disconnection event types.
+        /// </summary>
+        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 = GetDisconnectEventMessage(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 GetDisconnectEventMessage(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>

com.unity.netcode.gameobjects/Runtime/Transports/SinglePlayer/SinglePlayerTransport.cs

@@ -32,7 +32,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;