Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions com.unity.netcode.gameobjects/Editor/NetworkManagerEditor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -352,18 +352,7 @@ public override void OnInspectorGUI()

if (GUILayout.Button(new GUIContent("Stop " + instanceType, "Stops the " + instanceType + " instance.")))
{
if (m_NetworkManager.IsHost)
{
m_NetworkManager.StopHost();
}
else if (m_NetworkManager.IsServer)
{
m_NetworkManager.StopServer();
}
else if (m_NetworkManager.IsClient)
{
m_NetworkManager.StopClient();
}
m_NetworkManager.Shutdown();
}
}
}
Expand Down
151 changes: 59 additions & 92 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -724,94 +724,6 @@ public SocketTasks StartClient()
return socketTasks;
}

/// <summary>
/// Stops the running server
/// </summary>
public void StopServer()
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo(nameof(StopServer));
}

var disconnectedIds = new HashSet<ulong>();

//Don't know if I have to disconnect the clients. I'm assuming the NetworkTransport does all the cleaning on shtudown. But this way the clients get a disconnect message from server (so long it does't get lost)

// make sure all messages are flushed before transport disconnect clients
if (MessageQueueContainer != null)
{
MessageQueueContainer.ProcessAndFlushMessageQueue(
queueType: MessageQueueContainer.MessageQueueProcessingTypes.Send,
NetworkUpdateStage.PostLateUpdate); // flushing messages in case transport's disconnect
}

foreach (KeyValuePair<ulong, NetworkClient> pair in ConnectedClients)
{
if (!disconnectedIds.Contains(pair.Key))
{
disconnectedIds.Add(pair.Key);

if (pair.Key == NetworkConfig.NetworkTransport.ServerClientId)
{
continue;
}

NetworkConfig.NetworkTransport.DisconnectRemoteClient(pair.Key);
}
}

foreach (KeyValuePair<ulong, PendingClient> pair in PendingClients)
{
if (!disconnectedIds.Contains(pair.Key))
{
disconnectedIds.Add(pair.Key);
if (pair.Key == NetworkConfig.NetworkTransport.ServerClientId)
{
continue;
}

NetworkConfig.NetworkTransport.DisconnectRemoteClient(pair.Key);
}
}

IsServer = false;
Shutdown();
}

/// <summary>
/// Stops the running host
/// </summary>
public void StopHost()
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo(nameof(StopHost));
}

IsServer = false;
IsClient = false;
StopServer();

//We don't stop client since we dont actually have a transport connection to our own host. We just handle host messages directly in the netcode
}

/// <summary>
/// Stops the running client
/// </summary>
public void StopClient()
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo(nameof(StopClient));
}

IsClient = false;
NetworkConfig.NetworkTransport.DisconnectLocalClient();
IsConnectedClient = false;
Shutdown();
}

/// <summary>
/// Starts a Host
/// </summary>
Expand Down Expand Up @@ -940,13 +852,71 @@ private void OnDestroy()
}
}

/// <summary>
/// Globally shuts down the library.
/// Disconnects clients if connected and stops server if running.
/// </summary>
public void Shutdown()
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo(nameof(Shutdown));
}

if (IsServer)
{
// make sure all messages are flushed before transport disconnect clients
if (MessageQueueContainer != null)
{
MessageQueueContainer.ProcessAndFlushMessageQueue(
queueType: MessageQueueContainer.MessageQueueProcessingTypes.Send,
NetworkUpdateStage.PostLateUpdate); // flushing messages in case transport's disconnect
}

var disconnectedIds = new HashSet<ulong>();

//Don't know if I have to disconnect the clients. I'm assuming the NetworkTransport does all the cleaning on shutdown. But this way the clients get a disconnect message from server (so long it does't get lost)

foreach (KeyValuePair<ulong, NetworkClient> pair in ConnectedClients)
{
if (!disconnectedIds.Contains(pair.Key))
{
disconnectedIds.Add(pair.Key);

if (pair.Key == NetworkConfig.NetworkTransport.ServerClientId)
{
continue;
}

NetworkConfig.NetworkTransport.DisconnectRemoteClient(pair.Key);
}
}

foreach (KeyValuePair<ulong, PendingClient> pair in PendingClients)
{
if (!disconnectedIds.Contains(pair.Key))
{
disconnectedIds.Add(pair.Key);
if (pair.Key == NetworkConfig.NetworkTransport.ServerClientId)
{
continue;
}

NetworkConfig.NetworkTransport.DisconnectRemoteClient(pair.Key);
}
}
}

if (IsClient)
{
// Client only, send disconnect to server
NetworkConfig.NetworkTransport.DisconnectLocalClient();
}

IsConnectedClient = false;
IsServer = false;
IsClient = false;

// Unregister INetworkUpdateSystem before shutting down the MessageQueueContainer
this.UnregisterAllNetworkUpdates();

Expand All @@ -969,8 +939,6 @@ public void Shutdown()
NetworkTickSystem = null;
}

IsServer = false;
IsClient = false;
NetworkConfig.NetworkTransport.OnTransportEvent -= HandleRawTransportPoll;

if (SpawnManager != null)
Expand Down Expand Up @@ -1201,8 +1169,7 @@ private void HandleRawTransportPoll(NetworkEvent networkEvent, ulong clientId, N
}
else
{
IsConnectedClient = false;
StopClient();
Shutdown();
}

OnClientDisconnectCallback?.Invoke(clientId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public void UNetCustomChannelRegistrationTest()
Assert.Fail("The UNet transport won't allow registration of a legit user channel");
}

nm.StopServer();
nm.Shutdown();

ut.Channels.Clear();
Expand All @@ -64,7 +63,6 @@ public void UNetCustomChannelRegistrationTest()
Debug.Log(ex.Message);
}

nm.StopServer();
nm.Shutdown();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void MessageHandlerReceivedMessageServerClient()
networkManager.HandleIncomingData(0, NetworkChannel.Internal, new ArraySegment<byte>(messageStream10.GetBuffer(), 0, (int)messageStream10.Length), 0);

// Stop server to trigger full shutdown
networkManager.StopServer();
networkManager.Shutdown();

// Replace the real message handler with a dummy one that just prints a result
networkManager.MessageHandler = new DummyMessageHandler(networkManager);
Expand Down Expand Up @@ -196,7 +196,7 @@ public void MessageHandlerReceivedMessageServerClient()
networkManager.HandleIncomingData(0, NetworkChannel.Internal, new ArraySegment<byte>(messageStream21.GetBuffer(), 0, (int)messageStream21.Length), 0);

// Full cleanup
networkManager.StopClient();
networkManager.Shutdown();

// Ensure no missmatches with expectations
LogAssert.NoUnexpectedReceived();
Expand Down
62 changes: 62 additions & 0 deletions com.unity.netcode.gameobjects/Tests/Editor/StartStopTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
using NUnit.Framework;
using UnityEngine;

namespace Unity.Netcode.EditorTests
{
public class StartStopTests
{
private NetworkManager m_NetworkManager;

[SetUp]
public void Setup()
{
// Create the reusable NetworkManager
m_NetworkManager = new GameObject(nameof(NetworkManager)).AddComponent<NetworkManager>();
var transport = m_NetworkManager.gameObject.AddComponent<DummyTransport>();

m_NetworkManager.NetworkConfig = new NetworkConfig()
{
NetworkTransport = transport
};
}

[Test]
public void TestStopAndRestartForExceptions()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look much better, good job!

nit: The team just had a conversation about this, and still isn't aligned quite yet so feel free to disregard, but I'm a fan of more descriptive test names... e.g. either SUTShouldDoExpectedThingWhenSpecificState or SUT_SpecificState_ExpectedResult so in this case it might be something like NetworkManagerShouldNotThrowExceptionsWhenRestarted, or NetworkManager_StartAfterShutdown_DoesNotThrow

{
m_NetworkManager.StartServer();
m_NetworkManager.Shutdown();
m_NetworkManager.StartServer();
m_NetworkManager.Shutdown();
}

[Test]
public void TestStartupServerState()
{
m_NetworkManager.StartServer();

Assert.True(m_NetworkManager.IsServer);
Assert.False(m_NetworkManager.IsClient);
Assert.False(m_NetworkManager.IsHost);

m_NetworkManager.Shutdown();
}

[Test]
public void TestFlagShutdown()
{
m_NetworkManager.StartServer();
m_NetworkManager.Shutdown();

Assert.False(m_NetworkManager.IsServer);
Assert.False(m_NetworkManager.IsClient);
Assert.False(m_NetworkManager.IsHost);
}

[TearDown]
public void Teardown()
{
// Cleanup
Object.DestroyImmediate(m_NetworkManager.gameObject);
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -198,27 +198,7 @@ private static void StartNetworkManagerMode(NetworkManagerOperatingMode managerM
/// </summary>
private static void StopNetworkManagerMode()
{
switch (CurrentNetworkManagerMode)
{
case NetworkManagerOperatingMode.Host:
{
// Stop the host
NetworkManagerObject.StopHost();
break;
}
case NetworkManagerOperatingMode.Server:
{
// Stop the server
NetworkManagerObject.StopServer();
break;
}
case NetworkManagerOperatingMode.Client:
{
// Stop the client
NetworkManagerObject.StopClient();
break;
}
}
NetworkManagerObject.Shutdown();

Debug.Log($"{CurrentNetworkManagerMode} stopped.");
CurrentNetworkManagerMode = NetworkManagerOperatingMode.None;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public static bool CreateNewClients(int clientCount, out NetworkManager[] client
/// <param name="clientToStop"></param>
public static void StopOneClient(NetworkManager clientToStop)
{
clientToStop.StopClient();
clientToStop.Shutdown();
Object.Destroy(clientToStop.gameObject);
NetworkManagerInstances.Remove(clientToStop);
}
Expand All @@ -109,18 +109,7 @@ public static void Destroy()
// Shutdown the server which forces clients to disconnect
foreach (var networkManager in NetworkManagerInstances)
{
if (networkManager.IsHost)
{
networkManager.StopHost();
}
else if (networkManager.IsServer)
{
networkManager.StopServer();
}
else if (networkManager.IsClient)
{
networkManager.StopClient();
}
networkManager.Shutdown();
}

// Destroy the network manager instances
Expand Down
13 changes: 1 addition & 12 deletions testproject/Assets/Scripts/ExitButtonScript.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,7 @@ public void OnExitScene()
{
if (NetworkManager.Singleton)
{
if (NetworkManager.Singleton.IsHost)
{
NetworkManager.Singleton.StopHost();
}
else if (NetworkManager.Singleton.IsClient)
{
NetworkManager.Singleton.StopClient();
}
else if (NetworkManager.Singleton.IsServer)
{
NetworkManager.Singleton.StopServer();
}
NetworkManager.Singleton.Shutdown();
Destroy(NetworkManager.Singleton.gameObject);
}

Expand Down
4 changes: 2 additions & 2 deletions testproject/Assets/Tests/Runtime/DontDestroyOnLoadTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ public IEnumerator Setup()
[UnityTearDown]
public IEnumerator Teardown()
{
m_ServerNetworkManager.StopHost();
m_ServerNetworkManager.Shutdown();
foreach (var networkManager in m_ClientNetworkManagers)
{
networkManager.StopClient();
networkManager.Shutdown();
}
int nextFrameNumber = Time.frameCount + 4;
yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber);
Expand Down
Loading