breaking: Using Events instead of Message for connect and disconnect

Assets/Mirror/Runtime/Messages.cs

@@ -30,8 +30,12 @@ public struct NotReadyMessage : NetworkMessage { }
 
     public struct AddPlayerMessage : NetworkMessage { }
 
+    // todo remove this in 3 months, Obsoleted 2020-11-07
+    [System.Obsolete("This message is never sent over network, Replaced with NetworkServer.OnDisconnected and NetworkClient.OnDisconnected events", true)]
     public struct DisconnectMessage : NetworkMessage { }
 
+    // todo remove this in 3 months, Obsoleted 2020-11-07
+    [System.Obsolete("This message is never sent over network, Replaced with NetworkServer.OnConnected and NetworkClient.OnConnected events", true)]
     public struct ConnectMessage : NetworkMessage { }
 
     public struct SceneMessage : NetworkMessage

Assets/Mirror/Runtime/NetworkClient.cs

@@ -56,6 +56,16 @@ public static class NetworkClient
         /// </summary>
         public static bool isLocalClient => connection is ULocalConnectionToServer;
 
+        /// <summary>
+        /// Called after successfully connected to server
+        /// </summary>
+        public static event ConnectionEvent OnConnected;
+
+        /// <summary>
+        /// Called after disconnected from server
+        /// </summary>
+        public static event ConnectionEvent OnDisconnected;
+
         /// <summary>
         /// Connect client to a NetworkServer instance.
         /// </summary>
@@ -124,8 +134,11 @@ public static void ConnectHost()
         /// </summary>
         public static void ConnectLocalServer()
         {
-            NetworkServer.OnConnected(NetworkServer.localConnection);
-            NetworkServer.localConnection.Send(new ConnectMessage());
+            NetworkServer.OnConnectedInternal(NetworkServer.localConnection);
+
+            Debug.Assert(connection is ULocalConnectionToServer, "Connection should be local connection");
+            // OnConnected should be called with connToServer
+            OnConnected?.Invoke(connection);
         }
 
         /// <summary>
@@ -141,15 +154,15 @@ public static void DisconnectLocalServer()
                 // local connection. should we send a DisconnectMessage here too?
                 // (if we do then we get an Unknown Message ID log)
                 //NetworkServer.localConnection.Send(new DisconnectMessage());
-                NetworkServer.OnDisconnected(NetworkServer.localConnection.connectionId);
+                NetworkServer.OnDisconnectedTransport(NetworkServer.localConnection.connectionId);
             }
         }
 
         static void AddTransportHandlers()
         {
-            Transport.activeTransport.OnClientConnected = OnConnected;
+            Transport.activeTransport.OnClientConnected = OnConnectedTransport;
             Transport.activeTransport.OnClientDataReceived = OnDataReceived;
-            Transport.activeTransport.OnClientDisconnected = OnDisconnected;
+            Transport.activeTransport.OnClientDisconnected = OnDisconnectedTransport;
             Transport.activeTransport.OnClientError = OnError;
         }
 
@@ -158,13 +171,16 @@ static void OnError(Exception exception)
             logger.LogException(exception);
         }
 
-        static void OnDisconnected()
+        /// <summary>
+        /// Handles OnClientDisconnected from transport
+        /// </summary>
+        static void OnDisconnectedTransport()
         {
             connectState = ConnectState.Disconnected;
 
             ClientScene.HandleClientDisconnect(connection);
 
-            connection?.InvokeHandler(new DisconnectMessage(), -1);
+            OnDisconnected?.Invoke(connection);
         }
 
         internal static void OnDataReceived(ArraySegment<byte> data, int channelId)
@@ -176,7 +192,10 @@ internal static void OnDataReceived(ArraySegment<byte> data, int channelId)
             else logger.LogError("Skipped Data message handling because connection is null.");
         }
 
-        static void OnConnected()
+        /// <summary>
+        /// Handles OnClientConnected from transport
+        /// </summary>
+        internal static void OnConnectedTransport()
         {
             if (connection != null)
             {
@@ -187,7 +206,7 @@ static void OnConnected()
                 // thus we should set the connected state before calling the handler
                 connectState = ConnectState.Connected;
                 NetworkTime.UpdateClient();
-                connection.InvokeHandler(new ConnectMessage(), -1);
+                OnConnected?.Invoke(connection);
             }
             else logger.LogError("Skipped Connect message handling because connection is null.");
         }
@@ -206,7 +225,8 @@ public static void Disconnect()
             {
                 if (isConnected)
                 {
-                    NetworkServer.localConnection.Send(new DisconnectMessage());
+                    Debug.Assert(connection is ULocalConnectionToServer, "Connection should be local connection");
+                    OnDisconnected?.Invoke(connection);
                 }
                 NetworkServer.RemoveLocalConnection();
             }
@@ -374,7 +394,11 @@ public static void Shutdown()
             // we do NOT call Transport.Shutdown, because someone only called
             // NetworkClient.Shutdown. we can't assume that the server is
             // supposed to be shut down too!
-            Transport.activeTransport.ClientDisconnect();
+            Transport.activeTransport?.ClientDisconnect();
+
+            // clear event listeners
+            OnConnected = null;
+            OnDisconnected = null;
         }
     }
 }

Assets/Mirror/Runtime/NetworkConnection.cs

@@ -203,6 +203,8 @@ internal bool InvokeHandler(int msgType, NetworkReader reader, int channelId)
         /// <typeparam name="T">The message type to unregister.</typeparam>
         /// <param name="msg">The message object to process.</param>
         /// <returns>Returns true if the handler was successfully invoked</returns>
+        // todo remove this in 3 months, Obsoleted 2020-11-07
+        [Obsolete("InvokeHandler is only used to invoke messages locally and will be removed soon, instead call the methods directly. see https://github.com/vis2k/Mirror/pull/2397")]
         public bool InvokeHandler<T>(T msg, int channelId)
             where T : struct, NetworkMessage
         {

Assets/Mirror/Runtime/NetworkManager.cs

@@ -1,10 +1,10 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using kcp2k;
 using UnityEngine;
 using UnityEngine.SceneManagement;
 using UnityEngine.Serialization;
-using kcp2k;
 
 namespace Mirror
 {
@@ -623,6 +623,8 @@ public void StopServer()
 
             logger.Log("NetworkManager StopServer");
             isNetworkActive = false;
+            NetworkServer.OnConnected -= OnServerConnectInternal;
+            NetworkServer.OnDisconnected -= OnServerDisconnectInternal;
             NetworkServer.Shutdown();
 
             // set offline mode BEFORE changing scene so that FinishStartScene
@@ -656,6 +658,8 @@ public void StopClient()
             isNetworkActive = false;
 
             // shutdown client
+            NetworkClient.OnConnected -= OnClientConnectInternal;
+            NetworkClient.OnDisconnected -= OnClientDisconnectInternal;
             NetworkClient.Disconnect();
             NetworkClient.Shutdown();
 
@@ -750,8 +754,8 @@ bool InitializeSingleton()
 
         void RegisterServerMessages()
         {
-            NetworkServer.RegisterHandler<ConnectMessage>(OnServerConnectInternal, false);
-            NetworkServer.RegisterHandler<DisconnectMessage>(OnServerDisconnectInternal, false);
+            NetworkServer.OnConnected += OnServerConnectInternal;
+            NetworkServer.OnDisconnected += OnServerDisconnectInternal;
             NetworkServer.RegisterHandler<AddPlayerMessage>(OnServerAddPlayerInternal);
             NetworkServer.RegisterHandler<ErrorMessage>(OnServerErrorInternal, false);
 
@@ -761,8 +765,8 @@ void RegisterServerMessages()
 
         void RegisterClientMessages()
         {
-            NetworkClient.RegisterHandler<ConnectMessage>(OnClientConnectInternal, false);
-            NetworkClient.RegisterHandler<DisconnectMessage>(OnClientDisconnectInternal, false);
+            NetworkClient.OnConnected += OnClientConnectInternal;
+            NetworkClient.OnDisconnected += OnClientDisconnectInternal;
             NetworkClient.RegisterHandler<NotReadyMessage>(OnClientNotReadyMessageInternal);
             NetworkClient.RegisterHandler<ErrorMessage>(OnClientErrorInternal, false);
             NetworkClient.RegisterHandler<SceneMessage>(OnClientSceneInternal, false);
@@ -1149,7 +1153,7 @@ public Transform GetStartPosition()
 
         #region Server Internal Message Handlers
 
-        void OnServerConnectInternal(NetworkConnection conn, ConnectMessage connectMsg)
+        void OnServerConnectInternal(NetworkConnection conn)
         {
             logger.Log("NetworkManager.OnServerConnectInternal");
 
@@ -1183,7 +1187,7 @@ void OnServerAuthenticated(NetworkConnection conn)
             OnServerConnect(conn);
         }
 
-        void OnServerDisconnectInternal(NetworkConnection conn, DisconnectMessage msg)
+        void OnServerDisconnectInternal(NetworkConnection conn)
         {
             logger.Log("NetworkManager.OnServerDisconnectInternal");
             OnServerDisconnect(conn);
@@ -1230,7 +1234,7 @@ void OnServerErrorInternal(NetworkConnection conn, ErrorMessage msg)
 
         #region Client Internal Message Handlers
 
-        void OnClientConnectInternal(NetworkConnection conn, ConnectMessage message)
+        void OnClientConnectInternal(NetworkConnection conn)
         {
             logger.Log("NetworkManager.OnClientConnectInternal");
 
@@ -1268,7 +1272,7 @@ void OnClientAuthenticated(NetworkConnection conn)
             }
         }
 
-        void OnClientDisconnectInternal(NetworkConnection conn, DisconnectMessage msg)
+        void OnClientDisconnectInternal(NetworkConnection conn)
         {
             logger.Log("NetworkManager.OnClientDisconnectInternal");
             OnClientDisconnect(conn);

Assets/Mirror/Runtime/NetworkServer.cs

@@ -72,6 +72,16 @@ public static class NetworkServer
         /// </summary>
         public static float disconnectInactiveTimeout = 60f;
 
+        /// <summary>
+        /// Called after new connection is set up on server
+        /// </summary>
+        public static event ConnectionEvent OnConnected;
+
+        /// <summary>
+        /// Called after connection disconnects from server
+        /// </summary>
+        public static event ConnectionEvent OnDisconnected;
+
         /// <summary>
         /// This shuts down the server and disconnects all clients.
         /// </summary>
@@ -98,6 +108,10 @@ public static void Shutdown()
 
             CleanupNetworkIdentities();
             NetworkIdentity.ResetNextNetworkId();
+
+            // clear event listeners
+            OnConnected = null;
+            OnDisconnected = null;
         }
 
         static void CleanupNetworkIdentities()
@@ -427,7 +441,7 @@ public static void DisconnectAllConnections()
                 conn.Disconnect();
                 // call OnDisconnected unless local player in host mode
                 if (conn.connectionId != NetworkConnection.LocalConnectionId)
-                    OnDisconnected(conn);
+                    OnDisconnectedInternal(conn);
             }
             connections.Clear();
         }
@@ -489,13 +503,16 @@ static void CheckForInactiveConnections()
 
         static void AddTransportHandlers()
         {
-            Transport.activeTransport.OnServerConnected = OnConnected;
+            Transport.activeTransport.OnServerConnected = OnConnectedTransport;
             Transport.activeTransport.OnServerDataReceived = OnDataReceived;
-            Transport.activeTransport.OnServerDisconnected = OnDisconnected;
+            Transport.activeTransport.OnServerDisconnected = OnDisconnectedTransport;
             Transport.activeTransport.OnServerError = OnError;
         }
 
-        static void OnConnected(int connectionId)
+        /// <summary>
+        /// Handles OnServerConnected from transport
+        /// </summary>
+        static void OnConnectedTransport(int connectionId)
         {
             if (logger.LogEnabled()) logger.Log("Server accepted client:" + connectionId);
 
@@ -524,7 +541,7 @@ static void OnConnected(int connectionId)
             {
                 // add connection
                 NetworkConnectionToClient conn = new NetworkConnectionToClient(connectionId);
-                OnConnected(conn);
+                OnConnectedInternal(conn);
             }
             else
             {
@@ -534,16 +551,22 @@ static void OnConnected(int connectionId)
             }
         }
 
-        internal static void OnConnected(NetworkConnectionToClient conn)
+        /// <summary>
+        /// Handles OnConnectedTransport and host connection
+        /// </summary>
+        internal static void OnConnectedInternal(NetworkConnectionToClient conn)
         {
             if (logger.LogEnabled()) logger.Log("Server accepted client:" + conn);
 
             // add connection and invoke connected event
             AddConnection(conn);
-            conn.InvokeHandler(new ConnectMessage(), -1);
+            OnConnected?.Invoke(conn);
         }
 
-        internal static void OnDisconnected(int connectionId)
+        /// <summary>
+        /// Handles OnServerDisconnected from transport
+        /// </summary>
+        internal static void OnDisconnectedTransport(int connectionId)
         {
             if (logger.LogEnabled()) logger.Log("Server disconnect client:" + connectionId);
 
@@ -553,13 +576,16 @@ internal static void OnDisconnected(int connectionId)
                 RemoveConnection(connectionId);
                 if (logger.LogEnabled()) logger.Log("Server lost client:" + connectionId);
 
-                OnDisconnected(conn);
+                OnDisconnectedInternal(conn);
             }
         }
 
-        static void OnDisconnected(NetworkConnection conn)
+        /// <summary>
+        /// Handles OnDisconnectedTransport and host connection
+        /// </summary>
+        static void OnDisconnectedInternal(NetworkConnection conn)
         {
-            conn.InvokeHandler(new DisconnectMessage(), -1);
+            OnDisconnected?.Invoke(conn);
             if (logger.LogEnabled()) logger.Log("Server lost client:" + conn);
         }
 

Assets/Mirror/Runtime/UNetwork.cs

@@ -15,6 +15,10 @@ namespace Mirror
     // Handles requests to unspawn objects on the client
     public delegate void UnSpawnDelegate(GameObject spawned);
 
+
+    public delegate void ConnectionEvent(NetworkConnection conn);
+
+
     // invoke type for Cmd/Rpc
     public enum MirrorInvokeType
     {

Assets/Mirror/Tests/Common/MemoryTransport.cs

@@ -124,20 +124,12 @@ public override void ServerSend(int connectionId, int channelId, ArraySegment<by
 
         public override bool ServerDisconnect(int connectionId)
         {
-            // clear all pending messages that we may have received.
-            // over the wire, we wouldn't receive any more pending messages
-            // ether after calling disconnect.
-            serverIncoming.Clear();
-
             // add client disconnected message with connectionId
             clientIncoming.Enqueue(new Message(connectionId, EventType.Disconnected, null));
 
             // add server disconnected message with connectionId
             serverIncoming.Enqueue(new Message(connectionId, EventType.Disconnected, null));
 
-            // not active anymore
-            serverActive = false;
-
             return false;
         }
 

Assets/Mirror/Tests/Editor/BuiltInMessages.cs

@@ -30,30 +30,6 @@ public void CommandMessage()
                     Is.EqualTo(message.payload.Array[message.payload.Offset + i]));
         }
 
-        [Test]
-        public void ConnectMessage()
-        {
-            // try setting value with constructor
-            ConnectMessage message = new ConnectMessage();
-            byte[] arr = MessagePackerTest.PackToByteArray(message);
-            Assert.DoesNotThrow(() =>
-            {
-                MessagePackerTest.UnpackFromByteArray<ConnectMessage>(arr);
-            });
-        }
-
-        [Test]
-        public void DisconnectMessage()
-        {
-            // try setting value with constructor
-            DisconnectMessage message = new DisconnectMessage();
-            byte[] arr = MessagePackerTest.PackToByteArray(message);
-            Assert.DoesNotThrow(() =>
-            {
-                MessagePackerTest.UnpackFromByteArray<DisconnectMessage>(arr);
-            });
-        }
-
         [Test]
         public void ErrorMessage()
         {