fix: fixing order of host setup (#832)

new order:
- add player to server collection
- invoke client connected
- invoke server connected

Assets/Mirage/Runtime/NetworkClient.cs

@@ -193,10 +193,14 @@ internal void ConnectHost(NetworkServer server, IDataHandler serverDataHandler)
             // invoke started event after everything is set up, but before peer has connected
             _started.Invoke();
 
-            // client has to connect first or it will miss message in NetworkScenemanager
-            Peer_OnConnected(clientConn);
 
-            server.SetLocalConnection(this, serverConn);
+            // we need add server connection to server's dictionary first
+            // then invoke connected event on client (client has to connect first or it will miss message in NetworkScenemanager)
+            // then invoke connected event on server
+
+            server.AddLocalConnection(this, serverConn);
+            Peer_OnConnected(clientConn);
+            server.InvokeLocalConnected();
         }
 
         void InitializeAuthEvents()

Assets/Mirage/Runtime/NetworkServer.cs

@@ -251,7 +251,7 @@ void ThrowIfSocketIsMissing()
                 throw new InvalidOperationException($"{nameof(SocketFactory)} could not be found for ${nameof(NetworkServer)}");
         }
 
-        private void Update()
+        internal void Update()
         {
             peer?.Update();
             SyncVarSender?.Update();
@@ -345,11 +345,10 @@ public void RemoveConnection(INetworkPlayer player)
         }
 
         /// <summary>
-        /// called by LocalClient to add itself. dont call directly.
+        /// Create Player on Server for hostmode and adds it to collections
+        /// <para>Does not invoke <see cref="Connected"/> event, use <see cref="InvokeLocalConnected"/> instead at the correct time</para>
         /// </summary>
-        /// <param name="client">The local client</param>
-        /// <param name="connection">The connection to the client</param>
-        internal void SetLocalConnection(INetworkClient client, IConnection connection)
+        internal void AddLocalConnection(INetworkClient client, IConnection connection)
         {
             if (LocalPlayer != null)
             {
@@ -360,7 +359,23 @@ internal void SetLocalConnection(INetworkClient client, IConnection connection)
             LocalPlayer = player;
             LocalClient = client;
 
-            ConnectionAccepted(player);
+            if (logger.LogEnabled()) logger.Log("Server accepted Local client:" + player);
+
+            // add connection
+            AddConnection(player);
+        }
+
+        /// <summary>
+        /// Invokes the Connected event using the local player
+        /// <para>this should be done after the clients version has been invoked</para>
+        /// </summary>
+        internal void InvokeLocalConnected()
+        {
+            if (LocalPlayer == null)
+            {
+                throw new InvalidOperationException("Local Connection does not exist");
+            }
+            Connected?.Invoke(LocalPlayer);
         }
 
         /// <summary>

Assets/Tests/Common/TestSocketFactory.cs

@@ -126,7 +126,7 @@ public struct Packet
 
     public class TestSocketFactory : SocketFactory
     {
-        EndPoint serverEndpoint = Substitute.For<EndPoint>();
+        public EndPoint serverEndpoint = Substitute.For<EndPoint>();
 
         public override ISocket CreateClientSocket()
         {

Assets/Tests/Runtime/ClientServer/ClientServerSetup.cs

@@ -10,7 +10,6 @@
 
 namespace Mirage.Tests.Runtime.ClientServer
 {
-    // set's up a client and a server
     public class ClientServerSetup<T> where T : NetworkBehaviour
     {
 

Assets/Tests/Runtime/Host/HostSetup.cs

@@ -1,12 +1,12 @@
 using System.Collections;
 using Cysharp.Threading.Tasks;
+using Mirage.SocketLayer;
 using NUnit.Framework;
 using UnityEngine;
 using UnityEngine.TestTools;
 
 namespace Mirage.Tests.Runtime.Host
 {
-    // set's up a host
     public class HostSetup<T> where T : NetworkBehaviour
     {
 
@@ -24,6 +24,8 @@ public class HostSetup<T> where T : NetworkBehaviour
         protected T component;
 
         protected virtual bool AutoStartServer => true;
+        protected virtual Config ServerConfig => null;
+        protected virtual Config ClientConfig => null;
 
         public virtual void ExtraSetup() { }
 
@@ -43,6 +45,10 @@ public class HostSetup<T> where T : NetworkBehaviour
             manager.Server = networkManagerGo.GetComponent<NetworkServer>();
             server = manager.Server;
             client = manager.Client;
+
+            if (ServerConfig != null) server.PeerConfig = ServerConfig;
+            if (ClientConfig != null) client.PeerConfig = ClientConfig;
+
             sceneManager.Client = client;
             sceneManager.Server = server;
             serverObjectManager.Server = server;
@@ -102,6 +108,15 @@ void Started()
             ExtraTearDown();
         });
 
+        public void DoUpdate(int updateCount = 1)
+        {
+            for (int i = 0; i < updateCount; i++)
+            {
+                server.Update();
+                client.Update();
+            }
+        }
+
         #endregion
     }
 }

Assets/Tests/Runtime/Host/NetworkServerMaxConnectionTests.cs

@@ -0,0 +1,44 @@
+using Mirage.SocketLayer;
+using NUnit.Framework;
+using UnityEngine;
+using Object = UnityEngine.Object;
+
+namespace Mirage.Tests.Runtime.Host
+{
+    public class NetworkServerMaxConnectionTests : HostSetup<MockComponent>
+    {
+        protected override Config ServerConfig => new Config
+        {
+            // max is 0 becuase host connection isn't included for peer
+            MaxConnections = 0
+        };
+
+        [Test]
+        public void DisconnectsConnectionsOverMax()
+        {
+            var secondGO = new GameObject();
+            NetworkClient secondClient = secondGO.AddComponent<NetworkClient>();
+            TestSocketFactory socketFactory = networkManagerGo.GetComponent<TestSocketFactory>();
+
+            secondClient.SocketFactory = socketFactory;
+
+            int disconnectedCalled = 0;
+            secondClient.Disconnected.AddListener(reason =>
+            {
+                disconnectedCalled++;
+                Assert.That(reason, Is.EqualTo(ClientStoppedReason.ServerFull));
+            });
+            secondClient.Connect("localhost");
+
+            // updates both so that connect and reject message is received 
+            DoUpdate();
+            secondClient.Update();
+
+            Assert.That(server.Players, Has.Count.EqualTo(1));
+            // also check if client was disconnected (this will confirm it was rejected
+            Assert.That(disconnectedCalled, Is.EqualTo(1));
+
+            Object.Destroy(secondGO);
+        }
+    }
+}

Assets/Tests/Runtime/Host/NetworkServerNoAutoStartTest.cs

@@ -0,0 +1,19 @@
+using System;
+using NUnit.Framework;
+
+namespace Mirage.Tests.Runtime.Host
+{
+    public class NetworkServerNoAutoStartTest : HostSetup<MockComponent>
+    {
+        protected override bool AutoStartServer => false;
+
+        [Test]
+        public void InvokeLocalConnectedThrowsIfLocalClientIsNull()
+        {
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                server.InvokeLocalConnected();
+            });
+        }
+    }
+}

Assets/Tests/Runtime/Host/NetworkServerTest.cs

@@ -1,45 +1,55 @@
 using System;
+using System.Collections.Generic;
 using NUnit.Framework;
-using UnityEngine;
-using Object = UnityEngine.Object;
 
 namespace Mirage.Tests.Runtime.Host
 {
-
     [TestFixture]
     public class NetworkServerTest : HostSetup<MockComponent>
     {
-        [Test]
-        public void MaxConnectionsTest()
-        {
-            var secondGO = new GameObject();
-            NetworkClient secondClient = secondGO.AddComponent<NetworkClient>();
-            TestSocketFactory socketFactory = networkManagerGo.GetComponent<TestSocketFactory>();
-
-            secondClient.SocketFactory = socketFactory;
+        readonly List<INetworkPlayer> serverConnectedCalls = new List<INetworkPlayer>();
+        readonly List<INetworkPlayer> clientConnectedCalls = new List<INetworkPlayer>();
 
-            secondClient.Connect("localhost");
+        public override void ExtraSetup()
+        {
+            serverConnectedCalls.Clear();
+            clientConnectedCalls.Clear();
 
-            Assert.That(server.Players, Has.Count.EqualTo(1));
+            server.Connected.AddListener(player => serverConnectedCalls.Add(player));
+            client.Connected.AddListener(player => clientConnectedCalls.Add(player));
+        }
 
-            Object.Destroy(secondGO);
+        [Test]
+        public void ConnectedEventIsCalledOnceForServer()
+        {
+            Assert.That(serverConnectedCalls, Has.Count.EqualTo(1));
+            Assert.That(serverConnectedCalls[0].Connection, Is.TypeOf<PipePeerConnection>());
+        }
+        [Test]
+        public void ConnectedEventIsCalledOnceForClient()
+        {
+            Assert.That(clientConnectedCalls, Has.Count.EqualTo(1));
+            Assert.That(clientConnectedCalls[0].Connection, Is.TypeOf<PipePeerConnection>());
         }
 
+
         [Test]
         public void LocalClientActiveTest()
         {
             Assert.That(server.LocalClientActive, Is.True);
         }
 
         [Test]
-        public void SetLocalConnectionExceptionTest()
+        public void AddLocalConnectionExceptionTest()
         {
             Assert.Throws<InvalidOperationException>(() =>
             {
-                server.SetLocalConnection(null, null);
+                server.AddLocalConnection(null, null);
             });
         }
 
+
+
         [Test]
         public void StartedNotNullTest()
         {

Assets/Tests/Runtime/NetworkIdentityCallbackTests.cs

@@ -68,7 +68,8 @@ public void AddAllReadyServerConnectionsToObservers()
             server.Players.Add(player2);
 
             // add a host connection
-            server.SetLocalConnection(client, Substitute.For<SocketLayer.IConnection>());
+            server.AddLocalConnection(client, Substitute.For<SocketLayer.IConnection>());
+            server.InvokeLocalConnected();
             server.LocalPlayer.IsReady = true;
 
             // call OnStartServer so that observers dict is created