fix: Disconnect event notifications #2390

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -24,6 +24,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Fixed
 
 - Fixed issue where a client, under above average latency and packet loss conditions, could receive multiple NetworkTransform state updates in one frame and when processing the state updates only the last state update would be applied to the transform if interpolation was disabled. (#3614)
+- 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

@@ -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");
@@ -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)
                 {
@@ -520,6 +524,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>
@@ -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)
@@ -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();
 
@@ -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);
 
@@ -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>
@@ -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))
@@ -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)
                 {
@@ -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)}()");

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

@@ -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
 {
@@ -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.
@@ -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.

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

@@ -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;