breaking: Using Events instead of Message for connect and disconnect

* Using events is simpler to understand than InvokeHandler * Allows multiple objects to listen for connect events to add message handlers to NetworkClient this change does cause client OnClientConnect in host mode to be called the same BREAKING CHANGE: use events in NetworkServer/Client instead of handlers for Connect/DisconnectMessage
MirrorNetworking · Oct 30, 2020 · 44ad47d · 44ad47d
1 parent 1d6b33c
commit 44ad47d
Show file tree

Hide file tree

Showing 14 changed files with 58 additions and 91 deletions.
diff --git a/Assets/Mirror/Runtime/Messages.cs b/Assets/Mirror/Runtime/Messages.cs
@@ -30,8 +30,10 @@ public struct NotReadyMessage : NetworkMessage { }
 
     public struct AddPlayerMessage : NetworkMessage { }
 
+    [System.Obsolete("this message is never sent over network, replace with an event", true)]
     public struct DisconnectMessage : NetworkMessage { }
 
+    [System.Obsolete("this message is never sent over network, replace with an event", true)]
     public struct ConnectMessage : NetworkMessage { }
 
     public struct SceneMessage : NetworkMessage

diff --git a/Assets/Mirror/Runtime/NetworkClient.cs b/Assets/Mirror/Runtime/NetworkClient.cs
@@ -33,6 +33,8 @@ public static class NetworkClient
         public static NetworkConnection connection { get; internal set; }
 
         internal static ConnectState connectState = ConnectState.None;
+        internal static event ConnectionEvent OnConnectedEvent;
+        internal static event ConnectionEvent OnDisconnectEvent;
 
         /// <summary>
         /// The IP address of the server that this client is connected to.
@@ -125,7 +127,9 @@ public static void ConnectHost()
         public static void ConnectLocalServer()
         {
             NetworkServer.OnConnected(NetworkServer.localConnection);
-            NetworkServer.localConnection.Send(new ConnectMessage());
+
+            Debug.Assert(connection is ULocalConnectionToServer, "Connection should be local connection");
+            OnConnectedEvent?.Invoke(connection);
         }
 
         /// <summary>
@@ -164,7 +168,7 @@ static void OnDisconnected()
 
             ClientScene.HandleClientDisconnect(connection);
 
-            connection?.InvokeHandler(new DisconnectMessage(), -1);
+            OnDisconnectEvent?.Invoke(connection);
         }
 
         internal static void OnDataReceived(ArraySegment<byte> data, int channelId)
@@ -187,7 +191,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);
+                OnConnectedEvent?.Invoke(connection);
             }
             else logger.LogError("Skipped Connect message handling because connection is null.");
         }
@@ -206,7 +210,8 @@ public static void Disconnect()
             {
                 if (isConnected)
                 {
-                    NetworkServer.localConnection.Send(new DisconnectMessage());
+                    Debug.Assert(connection is ULocalConnectionToServer, "Connection should be local connection");
+                    OnDisconnectEvent?.Invoke(connection);
                 }
                 NetworkServer.RemoveLocalConnection();
             }

diff --git a/Assets/Mirror/Runtime/NetworkConnection.cs b/Assets/Mirror/Runtime/NetworkConnection.cs
@@ -202,6 +202,7 @@ 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 can we remove this function? does anyone need to invoke a message handler locally
         public bool InvokeHandler<T>(T msg, int channelId) where T : NetworkMessage
         {
             using (PooledNetworkWriter writer = NetworkWriterPool.GetWriter())

diff --git a/Assets/Mirror/Runtime/NetworkManager.cs b/Assets/Mirror/Runtime/NetworkManager.cs
@@ -719,8 +719,8 @@ bool InitializeSingleton()
 
         void RegisterServerMessages()
         {
-            NetworkServer.RegisterHandler<ConnectMessage>(OnServerConnectInternal, false);
-            NetworkServer.RegisterHandler<DisconnectMessage>(OnServerDisconnectInternal, false);
+            NetworkServer.OnConnectedEvent += OnServerConnectInternal;
+            NetworkServer.OnDisconnectEvent += OnServerDisconnectInternal;
             NetworkServer.RegisterHandler<AddPlayerMessage>(OnServerAddPlayerInternal);
             NetworkServer.RegisterHandler<ErrorMessage>(OnServerErrorInternal, false);
 
@@ -730,8 +730,8 @@ void RegisterServerMessages()
 
         void RegisterClientMessages()
         {
-            NetworkClient.RegisterHandler<ConnectMessage>(OnClientConnectInternal, false);
-            NetworkClient.RegisterHandler<DisconnectMessage>(OnClientDisconnectInternal, false);
+            NetworkClient.OnConnectedEvent += OnClientConnectInternal;
+            NetworkClient.OnDisconnectEvent += OnClientDisconnectInternal;
             NetworkClient.RegisterHandler<NotReadyMessage>(OnClientNotReadyMessageInternal);
             NetworkClient.RegisterHandler<ErrorMessage>(OnClientErrorInternal, false);
             NetworkClient.RegisterHandler<SceneMessage>(OnClientSceneInternal, false);
@@ -1118,7 +1118,7 @@ public Transform GetStartPosition()
 
         #region Server Internal Message Handlers
 
-        void OnServerConnectInternal(NetworkConnection conn, ConnectMessage connectMsg)
+        void OnServerConnectInternal(NetworkConnection conn)
         {
             logger.Log("NetworkManager.OnServerConnectInternal");
 
@@ -1152,7 +1152,7 @@ void OnServerAuthenticated(NetworkConnection conn)
             OnServerConnect(conn);
         }
 
-        void OnServerDisconnectInternal(NetworkConnection conn, DisconnectMessage msg)
+        void OnServerDisconnectInternal(NetworkConnection conn)
         {
             logger.Log("NetworkManager.OnServerDisconnectInternal");
             OnServerDisconnect(conn);
@@ -1199,7 +1199,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");
 
@@ -1237,7 +1237,7 @@ void OnClientAuthenticated(NetworkConnection conn)
             }
         }
 
-        void OnClientDisconnectInternal(NetworkConnection conn, DisconnectMessage msg)
+        void OnClientDisconnectInternal(NetworkConnection conn)
         {
             logger.Log("NetworkManager.OnClientDisconnectInternal");
             OnClientDisconnect(conn);

diff --git a/Assets/Mirror/Runtime/NetworkServer.cs b/Assets/Mirror/Runtime/NetworkServer.cs
@@ -72,6 +72,15 @@ 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 OnConnectedEvent;
+        /// <summary>
+        /// Called after connection disconnects from server
+        /// </summary>
+        public static event ConnectionEvent OnDisconnectEvent;
+
         /// <summary>
         /// This shuts down the server and disconnects all clients.
         /// </summary>
@@ -535,7 +544,7 @@ internal static void OnConnected(NetworkConnectionToClient conn)
 
             // add connection and invoke connected event
             AddConnection(conn);
-            conn.InvokeHandler(new ConnectMessage(), -1);
+            OnConnectedEvent?.Invoke(conn);
         }
 
         internal static void OnDisconnected(int connectionId)
@@ -554,7 +563,8 @@ internal static void OnDisconnected(int connectionId)
 
         static void OnDisconnected(NetworkConnection conn)
         {
-            conn.InvokeHandler(new DisconnectMessage(), -1);
+            // todo change to event instead of message
+            OnDisconnectEvent?.Invoke(conn);
             if (logger.LogEnabled()) logger.Log("Server lost client:" + conn);
         }
 

diff --git a/Assets/Mirror/Runtime/UNetwork.cs b/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
     {

diff --git a/Assets/Mirror/Tests/Editor/BuiltInMessages.cs b/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(() =>
-            {
-                MessagePacker.Unpack<ConnectMessage>(arr);
-            });
-        }
-
-        [Test]
-        public void DisconnectMessage()
-        {
-            // try setting value with constructor
-            DisconnectMessage message = new DisconnectMessage();
-            byte[] arr = MessagePackerTest.PackToByteArray(message);
-            Assert.DoesNotThrow(() =>
-            {
-                MessagePacker.Unpack<DisconnectMessage>(arr);
-            });
-        }
-
         [Test]
         public void ErrorMessage()
         {

diff --git a/Assets/Mirror/Tests/Editor/MessagePackerTest.cs b/Assets/Mirror/Tests/Editor/MessagePackerTest.cs
@@ -32,16 +32,19 @@ public void TestPacking()
             Assert.That(unpacked.sceneOperation, Is.EqualTo(SceneOperation.LoadAdditive));
         }
 
+        struct Message1 : NetworkMessage { }
+        struct Message2 : NetworkMessage { }
+
         [Test]
         public void UnpackWrongMessage()
         {
-            ConnectMessage message = new ConnectMessage();
+            Message1 message = new Message1();
 
             byte[] data = PackToByteArray(message);
 
             Assert.Throws<FormatException>(() =>
             {
-                DisconnectMessage unpacked = MessagePacker.Unpack<DisconnectMessage>(data);
+                Message2 unpacked = MessagePacker.Unpack<Message2>(data);
             });
         }
 

diff --git a/Assets/Mirror/Tests/Editor/NetworkBehaviourTests.cs b/Assets/Mirror/Tests/Editor/NetworkBehaviourTests.cs
@@ -300,8 +300,6 @@ public void SendCommandInternal()
             // we need to start a server and connect a client in order to be
             // able to send commands
             // message handlers
-            NetworkServer.RegisterHandler<ConnectMessage>((conn, msg) => { }, false);
-            NetworkServer.RegisterHandler<DisconnectMessage>((conn, msg) => { }, false);
             NetworkServer.RegisterHandler<ErrorMessage>((conn, msg) => { }, false);
             NetworkServer.RegisterHandler<SpawnMessage>((conn, msg) => { }, false);
             NetworkServer.Listen(1);
@@ -413,8 +411,6 @@ public void SendRPCInternal()
             // we need to start a server and connect a client in order to be
             // able to send commands
             // message handlers
-            NetworkServer.RegisterHandler<ConnectMessage>((conn, msg) => { }, false);
-            NetworkServer.RegisterHandler<DisconnectMessage>((conn, msg) => { }, false);
             NetworkServer.RegisterHandler<ErrorMessage>((conn, msg) => { }, false);
             NetworkServer.RegisterHandler<SpawnMessage>((conn, msg) => { }, false);
             NetworkServer.Listen(1);
@@ -499,8 +495,6 @@ public void SendTargetRPCInternal()
             // we need to start a server and connect a client in order to be
             // able to send commands
             // message handlers
-            NetworkServer.RegisterHandler<ConnectMessage>((conn, msg) => { }, false);
-            NetworkServer.RegisterHandler<DisconnectMessage>((conn, msg) => { }, false);
             NetworkServer.RegisterHandler<ErrorMessage>((conn, msg) => { }, false);
             NetworkServer.RegisterHandler<SpawnMessage>((conn, msg) => { }, false);
             NetworkServer.Listen(1);

diff --git a/Assets/Mirror/Tests/Editor/NetworkClientTests.cs b/Assets/Mirror/Tests/Editor/NetworkClientTests.cs
@@ -1,4 +1,4 @@
-using System;
+using System;
 using NUnit.Framework;
 using UnityEngine;
 
@@ -17,13 +17,8 @@ public void SetUp()
             Transport.activeTransport = transportGO.AddComponent<MemoryTransport>();
 
             // we need a server to connect to
-            NetworkServer.RegisterHandler<ConnectMessage>((conn, msg) => { }, false);
-            NetworkServer.RegisterHandler<DisconnectMessage>((conn, msg) => { }, false);
             NetworkServer.RegisterHandler<ErrorMessage>((conn, msg) => { }, false);
             NetworkServer.Listen(10);
-
-            // setup client handlers too
-            NetworkClient.RegisterHandler<ConnectMessage>(msg => { }, false);
         }
 
         [TearDown]

diff --git a/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs b/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs
@@ -845,7 +845,6 @@ public void OnStartServerInHostModeSetsIsClientTrue()
             // call client connect so that internals are set up
             // (it won't actually successfully connect)
             // -> also set up connectmessage handler to avoid unhandled msg error
-            NetworkClient.RegisterHandler<ConnectMessage>(msg => { }, false);
             NetworkClient.Connect("localhost");
 
             // manually invoke transport.OnConnected so that NetworkClient.active is set to true

diff --git a/Assets/Mirror/Tests/Editor/NetworkManagerStopHostOnServerDisconnectTest.cs b/Assets/Mirror/Tests/Editor/NetworkManagerStopHostOnServerDisconnectTest.cs
@@ -1,4 +1,5 @@
-using NUnit.Framework;
+using System;
+using NUnit.Framework;
 using UnityEngine;
 
 namespace Mirror.Tests
@@ -13,19 +14,26 @@ class NetworkManagerOnServerDisconnect : NetworkManager
     public class NetworkManagerStopHostOnServerDisconnectTest
     {
         GameObject gameObject;
+        GameObject playerPrefab;
         NetworkManagerOnServerDisconnect manager;
 
         [SetUp]
         public void SetUp()
         {
             gameObject = new GameObject();
             manager = gameObject.AddComponent<NetworkManagerOnServerDisconnect>();
+            playerPrefab = new GameObject("player");
+            NetworkIdentity id = playerPrefab.AddComponent<NetworkIdentity>();
+            id.assetId = Guid.NewGuid();
+            id.sceneId = 0;
+            manager.playerPrefab = playerPrefab;
         }
 
         [TearDown]
         public void TearDown()
         {
             GameObject.DestroyImmediate(gameObject);
+            GameObject.DestroyImmediate(playerPrefab);
         }
 
         // test to prevent https://github.com/vis2k/Mirror/issues/1515