From 678ac68b58798816658d29be649bdaf18ad70794 Mon Sep 17 00:00:00 2001 From: vis2k Date: Thu, 26 Mar 2020 20:54:53 +0100 Subject: [PATCH] fix: #1515 - StopHost now invokes OnServerDisconnected for the host client too (#1601) * fix: #1515 - StopHost now invokes OnServerDisconnected for the host client too * avoid NullReferenceException when calling StopHost without StartHost * test WIP * test --- Assets/Mirror/Runtime/NetworkClient.cs | 17 ++++++++ Assets/Mirror/Runtime/NetworkManager.cs | 9 ++++ Assets/Mirror/Runtime/NetworkServer.cs | 2 +- ...ManagerStopHostOnServerDisconnectedTest.cs | 43 +++++++++++++++++++ ...erStopHostOnServerDisconnectedTest.cs.meta | 11 +++++ .../Mirror/Tests/Editor/NetworkManagerTest.cs | 4 +- 6 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 Assets/Mirror/Tests/Editor/NetworkManagerStopHostOnServerDisconnectedTest.cs create mode 100644 Assets/Mirror/Tests/Editor/NetworkManagerStopHostOnServerDisconnectedTest.cs.meta diff --git a/Assets/Mirror/Runtime/NetworkClient.cs b/Assets/Mirror/Runtime/NetworkClient.cs index 986878c32..dcf7c3dbf 100644 --- a/Assets/Mirror/Runtime/NetworkClient.cs +++ b/Assets/Mirror/Runtime/NetworkClient.cs @@ -124,6 +124,23 @@ internal static void ConnectLocalServer() NetworkServer.localConnection.Send(new ConnectMessage()); } + /// + /// disconnect host mode. this is needed to call DisconnectMessage for + /// the host client too. + /// + internal static void DisconnectLocalServer() + { + // only if host connection is running + if (NetworkServer.localConnection != null) + { + // TODO ConnectLocalServer manually sends a ConnectMessage to the + // 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); + } + } + static void InitializeTransportHandlers() { Transport.activeTransport.OnClientConnected.AddListener(OnConnected); diff --git a/Assets/Mirror/Runtime/NetworkManager.cs b/Assets/Mirror/Runtime/NetworkManager.cs index 9d3456dc7..631268c97 100644 --- a/Assets/Mirror/Runtime/NetworkManager.cs +++ b/Assets/Mirror/Runtime/NetworkManager.cs @@ -547,6 +547,15 @@ void StartHostClient() public void StopHost() { OnStopHost(); + + // TODO try to move DisconnectLocalServer into StopClient(), and + // then call StopClient() before StopServer(). needs testing!. + + // DisconnectLocalServer needs to be called so that the host client + // receives a DisconnectMessage too. + // fixes: https://github.com/vis2k/Mirror/issues/1515 + NetworkClient.DisconnectLocalServer(); + StopServer(); StopClient(); } diff --git a/Assets/Mirror/Runtime/NetworkServer.cs b/Assets/Mirror/Runtime/NetworkServer.cs index 9b7aa8b16..886ec1cb6 100644 --- a/Assets/Mirror/Runtime/NetworkServer.cs +++ b/Assets/Mirror/Runtime/NetworkServer.cs @@ -473,7 +473,7 @@ internal static void OnConnected(NetworkConnectionToClient conn) conn.InvokeHandler(new ConnectMessage(), -1); } - static void OnDisconnected(int connectionId) + internal static void OnDisconnected(int connectionId) { if (LogFilter.Debug) Debug.Log("Server disconnect client:" + connectionId); diff --git a/Assets/Mirror/Tests/Editor/NetworkManagerStopHostOnServerDisconnectedTest.cs b/Assets/Mirror/Tests/Editor/NetworkManagerStopHostOnServerDisconnectedTest.cs new file mode 100644 index 000000000..06212c3f7 --- /dev/null +++ b/Assets/Mirror/Tests/Editor/NetworkManagerStopHostOnServerDisconnectedTest.cs @@ -0,0 +1,43 @@ +using NUnit.Framework; +using UnityEngine; + +namespace Mirror.Tests +{ + class NetworkManagerOnServerDisconnect : NetworkManager + { + public int called; + public override void OnServerDisconnect(NetworkConnection conn) { ++called; } + } + + [TestFixture] + public class NetworkManagerStopHostOnServerDisconnectedTest + { + GameObject gameObject; + NetworkManagerOnServerDisconnect manager; + + [SetUp] + public void SetUp() + { + gameObject = new GameObject(); + manager = gameObject.AddComponent(); + } + + [TearDown] + public void TearDown() + { + GameObject.DestroyImmediate(gameObject); + } + + // test to prevent https://github.com/vis2k/Mirror/issues/1515 + [Test] + public void StopHostCallsOnServerDisconnectForHostClient() + { + // OnServerDisconnect is always called when a client disconnects. + // it should also be called for the host client when we stop the host + Assert.That(manager.called, Is.EqualTo(0)); + manager.StartHost(); + manager.StopHost(); + Assert.That(manager.called, Is.EqualTo(1)); + } + } +} diff --git a/Assets/Mirror/Tests/Editor/NetworkManagerStopHostOnServerDisconnectedTest.cs.meta b/Assets/Mirror/Tests/Editor/NetworkManagerStopHostOnServerDisconnectedTest.cs.meta new file mode 100644 index 000000000..b21084477 --- /dev/null +++ b/Assets/Mirror/Tests/Editor/NetworkManagerStopHostOnServerDisconnectedTest.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2f583081473a64b92b971678e571382a +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Mirror/Tests/Editor/NetworkManagerTest.cs b/Assets/Mirror/Tests/Editor/NetworkManagerTest.cs index 0794d7ca3..b898b2a3b 100644 --- a/Assets/Mirror/Tests/Editor/NetworkManagerTest.cs +++ b/Assets/Mirror/Tests/Editor/NetworkManagerTest.cs @@ -114,7 +114,7 @@ public void RegisterStartPositionTest() public void UnRegisterStartPositionTest() { Assert.That(NetworkManager.startPositions.Count , Is.Zero); - + NetworkManager.RegisterStartPosition(gameObject.transform); Assert.That(NetworkManager.startPositions.Count , Is.EqualTo(1)); Assert.That(NetworkManager.startPositions, Has.Member(gameObject.transform)); @@ -127,7 +127,7 @@ public void UnRegisterStartPositionTest() public void GetStartPositionTest() { Assert.That(NetworkManager.startPositions.Count , Is.Zero); - + NetworkManager.RegisterStartPosition(gameObject.transform); Assert.That(NetworkManager.startPositions.Count , Is.EqualTo(1)); Assert.That(NetworkManager.startPositions, Has.Member(gameObject.transform));