fix: adding action to pipe connection so client events can be called …

…on stop (#838)

fix: #837

Assets/Mirage/Runtime/NetworkClient.cs

@@ -172,6 +172,12 @@ private void Peer_OnDisconnected(IConnection conn, DisconnectReason reason)
             Cleanup();
         }
 
+        void OnHostDisconnected()
+        {
+            Player?.MarkAsDisconnected();
+            _disconnected?.Invoke(ClientStoppedReason.RemoteConnectionClosed);
+        }
+
         internal void ConnectHost(NetworkServer server, IDataHandler serverDataHandler)
         {
             logger.Log("Client Connect Host to Server");
@@ -183,7 +189,7 @@ internal void ConnectHost(NetworkServer server, IDataHandler serverDataHandler)
 
             // create local connection objects and connect them
             var dataHandler = new DataHandler();
-            (IConnection clientConn, IConnection serverConn) = PipePeerConnection.Create(dataHandler, serverDataHandler);
+            (IConnection clientConn, IConnection serverConn) = PipePeerConnection.Create(dataHandler, serverDataHandler, OnHostDisconnected, null);
 
             // set up client before connecting to server, server could invoke handlers
             IsLocalClient = true;

Assets/Mirage/Runtime/PipePeerConnection.cs

@@ -27,12 +27,16 @@ public class PipePeerConnection : IConnection
         /// </summary>
         string name;
 
+        Action OnDisconnect;
+
         private PipePeerConnection() { }
 
-        public static (IConnection clientConn, IConnection serverConn) Create(IDataHandler clientHandler, IDataHandler serverHandler)
+        public static (IConnection clientConn, IConnection serverConn) Create(IDataHandler clientHandler, IDataHandler serverHandler, Action ClientOnDisconnect, Action ServerOnDisconnect)
         {
             var client = new PipePeerConnection();
+            client.OnDisconnect = ClientOnDisconnect;
             var server = new PipePeerConnection();
+            server.OnDisconnect = ServerOnDisconnect;
 
             client.otherHandler = serverHandler ?? throw new ArgumentNullException(nameof(serverHandler));
             server.otherHandler = clientHandler ?? throw new ArgumentNullException(nameof(clientHandler));
@@ -61,23 +65,39 @@ public override string ToString()
 
         void IConnection.Disconnect()
         {
+            if (State == ConnectionState.Disconnected)
+                return;
+
             State = ConnectionState.Disconnected;
+            OnDisconnect?.Invoke();
+
+            // tell other connection to also disconnect
+            otherConnection.Disconnect();
         }
 
         INotifyToken IConnection.SendNotify(byte[] packet)
         {
+            if (State == ConnectionState.Disconnected)
+                return default;
+
             receive(packet);
 
             return new PipeNotifyToken();
         }
 
         void IConnection.SendReliable(byte[] message)
         {
+            if (State == ConnectionState.Disconnected)
+                return;
+
             receive(message);
         }
 
         void IConnection.SendUnreliable(byte[] packet)
         {
+            if (State == ConnectionState.Disconnected)
+                return;
+
             receive(packet);
         }
 

Assets/Tests/Common/LocalConnections.cs

@@ -9,7 +9,7 @@ public static (NetworkPlayer serverPlayer, NetworkPlayer clientPlayer) PipedConn
             var clientHandler = new NetworkClient.DataHandler();
             var serverHandler = new NetworkClient.DataHandler();
 
-            (SocketLayer.IConnection clientConn, SocketLayer.IConnection serverConn) = PipePeerConnection.Create(clientHandler, serverHandler);
+            (SocketLayer.IConnection clientConn, SocketLayer.IConnection serverConn) = PipePeerConnection.Create(clientHandler, serverHandler, null, null);
 
             var clientPlayer = new NetworkPlayer(clientConn);
             var serverPlayer = new NetworkPlayer(serverConn);

Assets/Tests/Editor/PipePeerConnectionTest.cs

@@ -35,18 +35,26 @@ public void ExpectData(byte[] expected)
             {
                 handler.Received(1).ReceiveMessage(connection, Arg.Is<ArraySegment<byte>>(x => x.SequenceEqual(expected)));
             }
+            public void ExpectNoData()
+            {
+                handler.DidNotReceiveWithAnyArgs().ReceiveMessage(default, default);
+            }
         }
 
         ConnectionHandler conn1;
         ConnectionHandler conn2;
 
+        Action disconnect1;
+        Action disconnect2;
 
         [SetUp]
         public void Setup()
         {
             IDataHandler handler1 = Substitute.For<IDataHandler>();
             IDataHandler handler2 = Substitute.For<IDataHandler>();
-            (IConnection connection1, IConnection connection2) = PipePeerConnection.Create(handler1, handler2);
+            disconnect1 = Substitute.For<Action>();
+            disconnect2 = Substitute.For<Action>();
+            (IConnection connection1, IConnection connection2) = PipePeerConnection.Create(handler1, handler2, disconnect1, disconnect2);
 
             conn1 = new ConnectionHandler(handler1, connection1);
             conn2 = new ConnectionHandler(handler2, connection2);
@@ -107,40 +115,42 @@ public void ReceivesNotifySentDataMultiple()
         }
 
         [Test]
-        public void DisconnectShouldDoStuff()
-        {
-            Assert.Ignore("NotImplemented");
-            // todo add disconnect actions
-            /*
-            When a pipe connection is disconnected it should:
-            - send state to disconnected
-            - tell its pair to disconnect
-            - invoke peer callbacks via an action so that server/client know about the disconnect
-             */
-
-            //// disconnecting c1 should disconnect both
-            //conn1.Disconnect();
-
-            //var memoryStream = new MemoryStream();
-            //try
-            //{
-            //    await conn1.ReceiveAsync(memoryStream);
-            //    Assert.Fail("Recive Async should have thrown EndOfStreamException");
-            //}
-            //catch (EndOfStreamException)
-            //{
-            //    // good to go
-            //}
-
-            //try
-            //{
-            //    await conn2.ReceiveAsync(memoryStream);
-            //    Assert.Fail("Recive Async should have thrown EndOfStreamException");
-            //}
-            //catch (EndOfStreamException)
-            //{
-            //    // good to go
-            //}
+        public void DisconnectShouldBeCalledOnBothConnections()
+        {
+            conn1.connection.Disconnect();
+
+            Assert.That(conn1.connection.State, Is.EqualTo(ConnectionState.Disconnected));
+            Assert.That(conn2.connection.State, Is.EqualTo(ConnectionState.Disconnected));
+
+            disconnect1.Received(1).Invoke();
+            disconnect2.Received(1).Invoke();
+        }
+
+        [Test]
+        public void NoReceivesUnreliableAfterDisconnect()
+        {
+            conn1.connection.Disconnect();
+            conn1.SendUnreliable(new byte[] { 1, 2, 3, 4 });
+
+            conn2.ExpectNoData();
+        }
+
+        [Test]
+        public void NoReceivesReliableAfterDisconnect()
+        {
+            conn1.connection.Disconnect();
+            conn1.SendReliable(new byte[] { 1, 2, 3, 4 });
+
+            conn2.ExpectNoData();
+        }
+
+        [Test]
+        public void NoReceivesNotifyAfterDisconnect()
+        {
+            conn1.connection.Disconnect();
+            _ = conn1.SendNotify(new byte[] { 1, 2, 3, 4 });
+
+            conn2.ExpectNoData();
         }
 
         [Test]

Assets/Tests/Runtime/Host/HostComponentTests.cs

@@ -64,24 +64,33 @@ public class HostComponentTests : HostSetup<MockComponent>
             Assert.That(component.rpcOwnerArg2, Is.EqualTo("hello"));
         });
 
-        [UnityTest]
-        public IEnumerator DisconnectHostTest() => UniTask.ToCoroutine(async () =>
+        [Test]
+        public void StopHostTest()
         {
-            // set local connection
-            Assert.That(server.LocalClientActive, Is.True);
-            Assert.That(server.Players, Has.Count.EqualTo(1));
-
             server.Stop();
 
-            // wait for messages to get dispatched
-            await AsyncUtil.WaitUntilWithTimeout(() => !server.LocalClientActive);
-
             // state cleared?
             Assert.That(server.Players, Is.Empty);
             Assert.That(server.Active, Is.False);
             Assert.That(server.LocalPlayer, Is.Null);
             Assert.That(server.LocalClientActive, Is.False);
-        });
+        }
+
+        [Test]
+        public void StoppingHostShouldCallDisconnectedOnLocalClient()
+        {
+            int invoked = 0;
+            client.Disconnected.AddListener((reason) =>
+            {
+                Assert.That(reason, Is.EqualTo(ClientStoppedReason.RemoteConnectionClosed));
+                invoked++;
+            });
+
+            server.Stop();
+
+            // state cleared?
+            Assert.That(invoked, Is.EqualTo(1));
+        }
 
         [UnityTest]
         public IEnumerator ClientSceneChangedOnReconnect() => UniTask.ToCoroutine(async () =>