diff --git a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs index 7520525508..0eac75309d 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs @@ -599,6 +599,12 @@ public void StopServer() var disconnectedIds = new HashSet(); //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 RPCs are flushed before transport disconnect clients + if (RpcQueueContainer != null) + { + RpcQueueContainer.ProcessAndFlushRpcQueue(queueType: RpcQueueContainer.RpcQueueProcessingTypes.Send, NetworkUpdateStage.PostLateUpdate); // flushing messages in case transport's disconnect + } + foreach (KeyValuePair pair in ConnectedClients) { if (!disconnectedIds.Contains(pair.Key)) diff --git a/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs index ff161eab4a..8a19ba9e1f 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs @@ -68,11 +68,15 @@ public NetworkObject GetLocalPlayerObject() } /// - /// Returns the player object with a given clientId or null if one does not exist + /// Returns the player object with a given clientId or null if one does not exist. This is only valid server side. /// /// The player object with a given clientId or null if one does not exist public NetworkObject GetPlayerNetworkObject(ulong clientId) { + if (!NetworkManager.IsServer && NetworkManager.LocalClientId != clientId) + { + throw new NotServerException("Only the server can find player objects from other clients."); + } if (NetworkManager.ConnectedClients.TryGetValue(clientId, out NetworkClient networkClient)) { return networkClient.PlayerObject; diff --git a/com.unity.multiplayer.mlapi/Tests/Runtime/MultiInstance/MultiInstanceHelpers.cs b/com.unity.multiplayer.mlapi/Tests/Runtime/MultiInstance/MultiInstanceHelpers.cs index a1d159504e..570474d34a 100644 --- a/com.unity.multiplayer.mlapi/Tests/Runtime/MultiInstance/MultiInstanceHelpers.cs +++ b/com.unity.multiplayer.mlapi/Tests/Runtime/MultiInstance/MultiInstanceHelpers.cs @@ -26,17 +26,20 @@ public static class MultiInstanceHelpers /// The clients NetworkManagers public static bool Create(int clientCount, out NetworkManager server, out NetworkManager[] clients) { - clients = new NetworkManager[clientCount]; + NetworkManagerInstances = new List(); + + CreateNewClients(clientCount, out clients); - for (int i = 0; i < clientCount; i++) { // Create gameObject - var go = new GameObject("NetworkManager - Client - " + i); + var go = new GameObject("NetworkManager - Server"); + // Create networkManager component - clients[i] = go.AddComponent(); + server = go.AddComponent(); + NetworkManagerInstances.Insert(0, server); // Set the NetworkConfig - clients[i].NetworkConfig = new NetworkConfig() + server.NetworkConfig = new NetworkConfig() { // Set the current scene to prevent unexpected log messages which would trigger a failure RegisteredScenes = new List() { SceneManager.GetActiveScene().name }, @@ -45,18 +48,28 @@ public static bool Create(int clientCount, out NetworkManager server, out Networ }; } - NetworkManagerInstances = new List(clients); + return true; + } + + /// + /// Used to add a client to the already existing list of clients + /// + /// The amount of clients + /// + /// + public static bool CreateNewClients(int clientCount, out NetworkManager[] clients) + { + clients = new NetworkManager[clientCount]; + for (int i = 0; i < clientCount; i++) { // Create gameObject - var go = new GameObject("NetworkManager - Server"); - + var go = new GameObject("NetworkManager - Client - " + i); // Create networkManager component - server = go.AddComponent(); - NetworkManagerInstances.Insert(0, server); + clients[i] = go.AddComponent(); // Set the NetworkConfig - server.NetworkConfig = new NetworkConfig() + clients[i].NetworkConfig = new NetworkConfig() { // Set the current scene to prevent unexpected log messages which would trigger a failure RegisteredScenes = new List() { SceneManager.GetActiveScene().name }, @@ -65,9 +78,21 @@ public static bool Create(int clientCount, out NetworkManager server, out Networ }; } + NetworkManagerInstances.AddRange(clients); return true; } + /// + /// Stops one single client and makes sure to cleanup any static variables in this helper + /// + /// + public static void StopOneClient(NetworkManager clientToStop) + { + clientToStop.StopClient(); + Object.Destroy(clientToStop.gameObject); + NetworkManagerInstances.Remove(clientToStop); + } + /// /// Should always be invoked when finished with a single unit test /// (i.e. during TearDown) @@ -112,7 +137,7 @@ public static bool Start(bool host, NetworkManager server, NetworkManager[] clie } else { - server.StartClient(); + server.StartServer(); } for (int i = 0; i < clients.Length; i++) diff --git a/com.unity.multiplayer.mlapi/Tests/Runtime/NetworkSpawnManagerTests.cs b/com.unity.multiplayer.mlapi/Tests/Runtime/NetworkSpawnManagerTests.cs new file mode 100644 index 0000000000..bc868e3429 --- /dev/null +++ b/com.unity.multiplayer.mlapi/Tests/Runtime/NetworkSpawnManagerTests.cs @@ -0,0 +1,201 @@ +using System; +using System.Collections; +using MLAPI.Exceptions; +using NUnit.Framework; +using UnityEngine; +using UnityEngine.TestTools; +using Object = System.Object; + +namespace MLAPI.RuntimeTests +{ + public class NetworkSpawnManagerTests + { + private NetworkManager m_ServerNetworkManager; + private NetworkManager[] m_ClientNetworkManagers; + private GameObject m_PlayerPrefab; + private int m_OriginalTargetFrameRate; + + private ulong serverSideClientId => m_ServerNetworkManager.ServerClientId; + private ulong clientSideClientId => m_ClientNetworkManagers[0].LocalClientId; + private ulong otherClientSideClientId => m_ClientNetworkManagers[1].LocalClientId; + + [UnitySetUp] + public IEnumerator Setup() + { + // Just always track the current target frame rate (will be re-applied upon TearDown) + m_OriginalTargetFrameRate = Application.targetFrameRate; + + // Since we use frame count as a metric, we need to assure it runs at a "common update rate" + // between platforms (i.e. Ubuntu seems to run at much higher FPS when set to -1) + if (Application.targetFrameRate < 0 || Application.targetFrameRate > 120) + { + Application.targetFrameRate = 120; + } + + // Create multiple NetworkManager instances + if (!MultiInstanceHelpers.Create(2, out NetworkManager server, out NetworkManager[] clients)) + { + Debug.LogError("Failed to create instances"); + Assert.Fail("Failed to create instances"); + } + + m_ServerNetworkManager = server; + m_ClientNetworkManagers = clients; + + // Create playerPrefab + m_PlayerPrefab = new GameObject("Player"); + NetworkObject networkObject = m_PlayerPrefab.AddComponent(); + + // Make it a prefab + MultiInstanceHelpers.MakeNetworkedObjectTestPrefab(networkObject); + + // Set the player prefab + server.NetworkConfig.PlayerPrefab = m_PlayerPrefab; + + for (int i = 0; i < clients.Length; i++) + { + clients[i].NetworkConfig.PlayerPrefab = m_PlayerPrefab; + } + + // Start the instances + if (!MultiInstanceHelpers.Start(true, server, clients)) + { + Debug.LogError("Failed to start instances"); + Assert.Fail("Failed to start instances"); + } + + // Wait for connection on client side + for (int i = 0; i < clients.Length; i++) + { + yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.WaitForClientConnected(clients[i])); + } + + // Wait for connection on server side + yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.WaitForClientsConnectedToServer(server, clientCount: 3)); + } + + [Test] + public void TestServerCanAccessItsOwnPlayer() + { + // server can access its own player + var serverSideServerPlayerObject = m_ServerNetworkManager.SpawnManager.GetPlayerNetworkObject(serverSideClientId); + Assert.NotNull(serverSideServerPlayerObject); + Assert.AreEqual(serverSideClientId, serverSideServerPlayerObject.OwnerClientId); + } + + [Test] + public void TestServerCanAccessOtherPlayers() + { + // server can access other players + var serverSideClientPlayerObject = m_ServerNetworkManager.SpawnManager.GetPlayerNetworkObject(clientSideClientId); + Assert.NotNull(serverSideClientPlayerObject); + Assert.AreEqual(clientSideClientId, serverSideClientPlayerObject.OwnerClientId); + + var serverSideOtherClientPlayerObject = m_ServerNetworkManager.SpawnManager.GetPlayerNetworkObject(otherClientSideClientId); + Assert.NotNull(serverSideOtherClientPlayerObject); + Assert.AreEqual(otherClientSideClientId, serverSideOtherClientPlayerObject.OwnerClientId); + } + + [Test] + public void TestClientCantAccessServerPlayer() + { + // client can't access server player + Assert.Throws(() => + { + m_ClientNetworkManagers[0].SpawnManager.GetPlayerNetworkObject(serverSideClientId); + }); + } + + [Test] + public void TestClientCanAccessOwnPlayer() + { + // client can access own player + var clientSideClientPlayerObject = m_ClientNetworkManagers[0].SpawnManager.GetPlayerNetworkObject(clientSideClientId); + Assert.NotNull(clientSideClientPlayerObject); + Assert.AreEqual(clientSideClientId, clientSideClientPlayerObject.OwnerClientId); + } + + [Test] + public void TestClientCantAccessOtherPlayer() + { + // client can't access other player + Assert.Throws(() => + { + m_ClientNetworkManagers[0].SpawnManager.GetPlayerNetworkObject(otherClientSideClientId); + }); + } + + [Test] + public void TestServerGetsNullValueIfInvalidId() + { + // server gets null value if invalid id + var nullPlayer = m_ServerNetworkManager.SpawnManager.GetPlayerNetworkObject(9999); + Assert.Null(nullPlayer); + } + + [Test] + public void TestServerCanUseGetLocalPlayerObject() + { + // test server can use GetLocalPlayerObject + var serverSideServerPlayerObject = m_ServerNetworkManager.SpawnManager.GetLocalPlayerObject(); + Assert.NotNull(serverSideServerPlayerObject); + Assert.AreEqual(serverSideClientId, serverSideServerPlayerObject.OwnerClientId); + } + + [Test] + public void TestClientCanUseGetLocalPlayerObject() + { + // test client can use GetLocalPlayerObject + var clientSideClientPlayerObject = m_ClientNetworkManagers[0].SpawnManager.GetLocalPlayerObject(); + Assert.NotNull(clientSideClientPlayerObject); + Assert.AreEqual(clientSideClientId, clientSideClientPlayerObject.OwnerClientId); + } + + [UnityTest] + public IEnumerator TestConnectAndDisconnect() + { + // test when client connects, player object is now available + + // connect new client + if (!MultiInstanceHelpers.CreateNewClients(1, out NetworkManager[] clients)) + { + Debug.LogError("Failed to create instances"); + Assert.Fail("Failed to create instances"); + } + var newClientNetworkManager = clients[0]; + newClientNetworkManager.NetworkConfig.PlayerPrefab = m_PlayerPrefab; + newClientNetworkManager.StartClient(); + yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.WaitForClientConnected(newClientNetworkManager)); + yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.WaitForCondition(() => m_ServerNetworkManager.ConnectedClients.ContainsKey(newClientNetworkManager.LocalClientId))); + var newClientLocalClientId = newClientNetworkManager.LocalClientId; + + // test new client can get that itself locally + var newPlayerObject = newClientNetworkManager.SpawnManager.GetLocalPlayerObject(); + Assert.NotNull(newPlayerObject); + Assert.AreEqual(newClientLocalClientId, newPlayerObject.OwnerClientId); + // test server can get that new client locally + var serverSideNewClientPlayer = m_ServerNetworkManager.SpawnManager.GetPlayerNetworkObject(newClientLocalClientId); + Assert.NotNull(serverSideNewClientPlayer); + Assert.AreEqual(newClientLocalClientId, serverSideNewClientPlayer.OwnerClientId); + + // test when client disconnects, player object no longer available. + var nbConnectedClients = m_ServerNetworkManager.ConnectedClients.Count; + MultiInstanceHelpers.StopOneClient(newClientNetworkManager); + yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.WaitForCondition(() => m_ServerNetworkManager.ConnectedClients.Count == nbConnectedClients - 1)); + serverSideNewClientPlayer = m_ServerNetworkManager.SpawnManager.GetPlayerNetworkObject(newClientLocalClientId); + Assert.Null(serverSideNewClientPlayer); + } + + [UnityTearDown] + public IEnumerator Teardown() + { + // Shutdown and clean up both of our NetworkManager instances + MultiInstanceHelpers.Destroy(); + UnityEngine.Object.Destroy(m_PlayerPrefab); + + // Set the application's target frame rate back to its original value + Application.targetFrameRate = m_OriginalTargetFrameRate; + yield return new WaitForSeconds(0); // wait for next frame so everything is destroyed, so following tests can execute from clean environment + } + } +} diff --git a/com.unity.multiplayer.mlapi/Tests/Runtime/NetworkSpawnManagerTests.cs.meta b/com.unity.multiplayer.mlapi/Tests/Runtime/NetworkSpawnManagerTests.cs.meta new file mode 100644 index 0000000000..88c0a11cbd --- /dev/null +++ b/com.unity.multiplayer.mlapi/Tests/Runtime/NetworkSpawnManagerTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 173bed02aed54db4a4f056c245a67393 +timeCreated: 1621449221 \ No newline at end of file diff --git a/com.unity.multiplayer.mlapi/Tests/Runtime/Transport/SIPTransport.cs b/com.unity.multiplayer.mlapi/Tests/Runtime/Transport/SIPTransport.cs index 6cbd492efc..9465613e25 100644 --- a/com.unity.multiplayer.mlapi/Tests/Runtime/Transport/SIPTransport.cs +++ b/com.unity.multiplayer.mlapi/Tests/Runtime/Transport/SIPTransport.cs @@ -29,13 +29,14 @@ private class Peer public Queue IncomingBuffer = new Queue(); } - private readonly Dictionary m_Clients = new Dictionary(); + private readonly Dictionary m_Peers = new Dictionary(); private ulong m_ClientsCounter = 1; private static Peer s_Server; private Peer m_LocalConnection; public override ulong ServerClientId => 0; + public ulong LocalClientId; public override void DisconnectLocalClient() { @@ -53,12 +54,12 @@ public override void DisconnectLocalClient() if (s_Server != null && m_LocalConnection != null) { // Remove the connection - s_Server.Transport.m_Clients.Remove(m_LocalConnection.ConnectionId); + s_Server.Transport.m_Peers.Remove(m_LocalConnection.ConnectionId); } if (m_LocalConnection.ConnectionId == ServerClientId) { - s_Server = null; + StopServer(); } // Remove the local connection @@ -69,10 +70,10 @@ public override void DisconnectLocalClient() // Called by server public override void DisconnectRemoteClient(ulong clientId) { - if (m_Clients.ContainsKey(clientId)) + if (m_Peers.ContainsKey(clientId)) { // Inject disconnect into remote - m_Clients[clientId].IncomingBuffer.Enqueue(new Event + m_Peers[clientId].IncomingBuffer.Enqueue(new Event { Type = NetworkEvent.Disconnect, Channel = NetworkChannel.Internal, @@ -90,10 +91,10 @@ public override void DisconnectRemoteClient(ulong clientId) }); // Remove the local connection on remote - m_Clients[clientId].Transport.m_LocalConnection = null; + m_Peers[clientId].Transport.m_LocalConnection = null; // Remove connection on server - m_Clients.Remove(clientId); + m_Peers.Remove(clientId); } } @@ -107,25 +108,31 @@ public override void Init() { } + private void StopServer() + { + s_Server = null; + m_Peers.Remove(ServerClientId); + } + public override void Shutdown() { // Inject disconnects to all the remotes - foreach (KeyValuePair pair in m_Clients) + foreach (KeyValuePair onePeer in m_Peers) { - pair.Value.IncomingBuffer.Enqueue(new Event + onePeer.Value.IncomingBuffer.Enqueue(new Event { Type = NetworkEvent.Disconnect, Channel = NetworkChannel.Internal, - ConnectionId = pair.Key, + ConnectionId = LocalClientId, Data = new ArraySegment() }); } - + if (m_LocalConnection != null && m_LocalConnection.ConnectionId == ServerClientId) { - s_Server = null; + StopServer(); } - + // TODO: Cleanup } @@ -148,6 +155,7 @@ public override SocketTasks StartClient() // Generate an Id for the server that represents this client ulong serverConnectionId = ++s_Server.Transport.m_ClientsCounter; + LocalClientId = serverConnectionId; // Create local connection m_LocalConnection = new Peer() @@ -158,10 +166,10 @@ public override SocketTasks StartClient() }; // Add the server as a local connection - m_Clients.Add(ServerClientId, s_Server); + m_Peers.Add(ServerClientId, s_Server); // Add local connection as a connection on the server - s_Server.Transport.m_Clients.Add(serverConnectionId, m_LocalConnection); + s_Server.Transport.m_Peers.Add(serverConnectionId, m_LocalConnection); // Sends a connect message to the server s_Server.Transport.m_LocalConnection.IncomingBuffer.Enqueue(new Event() @@ -209,6 +217,8 @@ public override SocketTasks StartServer() // Set the local connection as the server s_Server = m_LocalConnection; + m_Peers.Add(ServerClientId, s_Server); + return SocketTask.Done.AsTasks(); } @@ -221,7 +231,12 @@ public override void Send(ulong clientId, ArraySegment data, NetworkChanne byte[] copy = new byte[data.Count]; Buffer.BlockCopy(data.Array, data.Offset, copy, 0, data.Count); - m_Clients[clientId].IncomingBuffer.Enqueue(new Event + if (!m_Peers.ContainsKey(clientId)) + { + throw new KeyNotFoundException($"peer id {clientId} not in peer list"); + } + + m_Peers[clientId].IncomingBuffer.Enqueue(new Event { Type = NetworkEvent.Data, ConnectionId = m_LocalConnection.ConnectionId,