diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index c16b945d34..fadcbba64c 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -13,6 +13,9 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Changed +- The `NetworkManager` functions `GetTransportIdFromClientId` and `GetClientIdFromTransportId` will now return `ulong.MaxValue` when the clientId or transportId do not exist. (#3707) +- Improved performance of the NetworkVariable. (#3683) +- Improved performance around the NetworkBehaviour component. (#3687) ### Deprecated @@ -22,6 +25,9 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Multiple disconnect events from the same transport will no longer disconnect the host. (#3707) +- Distributed authority clients no longer send themselves in the `ClientIds` list when sending a `ChangeOwnershipMessage`. (#3687) +- Made a variety of small performance improvements. (#3683) ### Security diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index 5123c4b2c1..ec3eaf59a9 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -1,12 +1,14 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Runtime.CompilerServices; using Unity.Collections; using Unity.Collections.LowLevel.Unsafe; using Unity.Profiling; using UnityEngine; +using Debug = UnityEngine.Debug; using Object = UnityEngine.Object; namespace Unity.Netcode @@ -322,16 +324,16 @@ internal void RemovePendingClient(ulong clientId) private ulong m_NextClientId = 1; [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal ulong TransportIdToClientId(ulong transportId) + internal (ulong, bool) TransportIdToClientId(ulong transportId) { if (transportId == GetServerTransportId()) { - return NetworkManager.ServerClientId; + return (NetworkManager.ServerClientId, true); } if (TransportIdToClientIdMap.TryGetValue(transportId, out var clientId)) { - return clientId; + return (clientId, true); } if (NetworkLog.CurrentLogLevel == LogLevel.Developer) @@ -339,20 +341,20 @@ internal ulong TransportIdToClientId(ulong transportId) NetworkLog.LogWarning($"Trying to get the NGO client ID map for the transport ID ({transportId}) but did not find the map entry! Returning default transport ID value."); } - return default; + return (default, false); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal ulong ClientIdToTransportId(ulong clientId) + internal (ulong, bool) ClientIdToTransportId(ulong clientId) { if (clientId == NetworkManager.ServerClientId) { - return GetServerTransportId(); + return (GetServerTransportId(), true); } if (ClientIdToTransportIdMap.TryGetValue(clientId, out var transportClientId)) { - return transportClientId; + return (transportClientId, true); } if (NetworkLog.CurrentLogLevel == LogLevel.Developer) @@ -360,7 +362,7 @@ internal ulong ClientIdToTransportId(ulong clientId) NetworkLog.LogWarning($"Trying to get the transport client ID map for the NGO client ID ({clientId}) but did not find the map entry! Returning default transport ID value."); } - return default; + return (0, false); } /// @@ -389,19 +391,24 @@ private ulong GetServerTransportId() /// Handles cleaning up the transport id/client id tables after receiving a disconnect event from transport /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal ulong TransportIdCleanUp(ulong transportId) + internal (ulong, bool) TransportIdCleanUp(ulong transportId) { // This check is for clients that attempted to connect but failed. // When this happens, the client will not have an entry within the m_TransportIdToClientIdMap or m_ClientIdToTransportIdMap lookup tables so we exit early and just return 0 to be used for the disconnect event. if (!LocalClient.IsServer && !TransportIdToClientIdMap.ContainsKey(transportId)) { - return NetworkManager.LocalClientId; + return (NetworkManager.LocalClientId, true); + } + + var (clientId, isConnectedClient) = TransportIdToClientId(transportId); + if (!isConnectedClient) + { + return (default, false); } - var clientId = TransportIdToClientId(transportId); TransportIdToClientIdMap.Remove(transportId); ClientIdToTransportIdMap.Remove(clientId); - return clientId; + return (clientId, true); } internal void PollAndHandleNetworkEvents() @@ -512,8 +519,11 @@ internal void DataEventHandler(ulong transportClientId, ref ArraySegment p #if DEVELOPMENT_BUILD || UNITY_EDITOR s_HandleIncomingData.Begin(); #endif - var clientId = TransportIdToClientId(transportClientId); - MessageManager.HandleIncomingData(clientId, payload, receiveTime); + var (clientId, isConnectedClient) = TransportIdToClientId(transportClientId); + if (isConnectedClient) + { + MessageManager.HandleIncomingData(clientId, payload, receiveTime); + } #if DEVELOPMENT_BUILD || UNITY_EDITOR s_HandleIncomingData.End(); @@ -525,10 +535,16 @@ internal void DataEventHandler(ulong transportClientId, ref ArraySegment p /// internal void DisconnectEventHandler(ulong transportClientId) { + var (clientId, wasConnectedClient) = TransportIdCleanUp(transportClientId); + if (!wasConnectedClient) + { + return; + } + #if DEVELOPMENT_BUILD || UNITY_EDITOR s_TransportDisconnect.Begin(); #endif - var clientId = TransportIdCleanUp(transportClientId); + if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) { NetworkLog.LogInfo($"Disconnect Event From {clientId}"); @@ -749,7 +765,15 @@ internal void ProcessPendingApprovals() { try { - HandleConnectionApproval(senderId, response); + if (response.Approved) + { + HandleConnectionApproval(senderId, response.CreatePlayerObject, response.PlayerPrefabHash, response.Position, response.Rotation); + } + else + { + // If the connection wasn't approved, disconnect the pending connection. + HandleConnectionDisconnect(senderId, response.Reason); + } senders ??= new List(); senders.Add(senderId); @@ -776,171 +800,188 @@ internal void ProcessPendingApprovals() /// internal bool MockSkippingApproval; + + /// + /// Server Side: Handles the denial of a client who sent a connection request + /// + /// + /// This will send a if a reason is given + /// + private void HandleConnectionDisconnect(ulong ownerClientId, string reason = "") + { + if (!string.IsNullOrEmpty(reason)) + { + var disconnectReason = new DisconnectReasonMessage + { + Reason = reason + }; + SendMessage(ref disconnectReason, NetworkDelivery.Reliable, ownerClientId); + } + + DisconnectRemoteClient(ownerClientId); + } + /// /// Server Side: Handles the approval of a client /// /// /// This will spawn the player prefab as well as start client synchronization if is enabled /// - internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.ConnectionApprovalResponse response) + internal void HandleConnectionApproval(ulong ownerClientId, bool createPlayerObject, uint? playerPrefabHash = null, Vector3? playerPosition = null, Quaternion? playerRotation = null) { - LocalClient.IsApproved = response.Approved; - if (response.Approved) + if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) { - if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) - { - NetworkLog.LogInfo($"[Server-Side] Pending Client-{ownerClientId} connection approved!"); - } - // The client was approved, stop the server-side approval time out coroutine - RemovePendingClient(ownerClientId); + NetworkLog.LogInfo($"[Server-Side] Pending Client-{ownerClientId} connection approved!"); + } + // The client was approved, stop the server-side approval time out coroutine + RemovePendingClient(ownerClientId); - var client = AddClient(ownerClientId); + var client = AddClient(ownerClientId); - // Server-side spawning (only if there is a prefab hash or player prefab provided) - if (!NetworkManager.DistributedAuthorityMode && response.CreatePlayerObject && (response.PlayerPrefabHash.HasValue || NetworkManager.NetworkConfig.PlayerPrefab != null)) - { - var playerObject = response.PlayerPrefabHash.HasValue ? NetworkManager.SpawnManager.GetNetworkObjectToSpawn(response.PlayerPrefabHash.Value, ownerClientId, response.Position ?? null, response.Rotation ?? null) - : NetworkManager.SpawnManager.GetNetworkObjectToSpawn(NetworkManager.NetworkConfig.PlayerPrefab.GetComponent().GlobalObjectIdHash, ownerClientId, response.Position ?? null, response.Rotation ?? null); - - // Spawn the player NetworkObject locally - NetworkManager.SpawnManager.SpawnNetworkObjectLocally( - playerObject, - NetworkManager.SpawnManager.GetNetworkObjectId(), - sceneObject: false, - playerObject: true, - ownerClientId, - destroyWithScene: false); - - client.AssignPlayerObject(ref playerObject); - } + // Set the local settings of the server once the host client has been added. + if (ownerClientId == NetworkManager.ServerClientId) + { + LocalClient = client; + LocalClient.IsConnected = true; + LocalClient.IsApproved = true; + } - // Server doesn't send itself the connection approved message - if (ownerClientId != NetworkManager.ServerClientId) - { - var message = new ConnectionApprovedMessage - { - OwnerClientId = ownerClientId, - NetworkTick = NetworkManager.LocalTime.Tick, - IsDistributedAuthority = NetworkManager.DistributedAuthorityMode, - ConnectedClientIds = new NativeArray(ConnectedClientIds.Count, Allocator.Temp) - }; + // Server-side spawning (only if there is a prefab hash or player prefab provided) + if (!NetworkManager.DistributedAuthorityMode && createPlayerObject && (playerPrefabHash.HasValue || NetworkManager.NetworkConfig.PlayerPrefab != null)) + { + var playerObject = playerPrefabHash.HasValue ? NetworkManager.SpawnManager.GetNetworkObjectToSpawn(playerPrefabHash.Value, ownerClientId, playerPosition, playerRotation) + : NetworkManager.SpawnManager.GetNetworkObjectToSpawn(NetworkManager.NetworkConfig.PlayerPrefab.GetComponent().GlobalObjectIdHash, ownerClientId, playerPosition, playerRotation); - var i = 0; - foreach (var clientId in ConnectedClientIds) - { - message.ConnectedClientIds[i] = clientId; - ++i; - } + // Spawn the player NetworkObject locally + NetworkManager.SpawnManager.SpawnNetworkObjectLocally( + playerObject, + NetworkManager.SpawnManager.GetNetworkObjectId(), + sceneObject: false, + playerObject: true, + ownerClientId, + destroyWithScene: false); - if (!NetworkManager.NetworkConfig.EnableSceneManagement) - { - // Update the observed spawned NetworkObjects for the newly connected player when scene management is disabled - NetworkManager.SpawnManager.UpdateObservedNetworkObjects(ownerClientId); - if (NetworkManager.SpawnManager.SpawnedObjectsList.Count != 0) - { - message.SpawnedObjectsList = NetworkManager.SpawnManager.SpawnedObjectsList; - } - } + client.AssignPlayerObject(ref playerObject); + } - message.MessageVersions = new NativeArray(MessageManager.MessageHandlers.Length, Allocator.Temp); - for (int index = 0; index < MessageManager.MessageHandlers.Length; index++) - { - if (MessageManager.MessageTypes[index] != null) - { - var type = MessageManager.MessageTypes[index]; - message.MessageVersions[index] = new MessageVersionData - { - Hash = XXHash.Hash32(type.FullName), - Version = MessageManager.GetLocalVersion(type) - }; - } - } - if (!MockSkippingApproval) - { - SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, ownerClientId); - } - else - { - NetworkLog.LogInfo("Mocking server not responding with connection approved..."); - } - message.MessageVersions.Dispose(); - message.ConnectedClientIds.Dispose(); - if (MockSkippingApproval) - { - return; - } + if (ownerClientId == NetworkManager.ServerClientId || !NetworkManager.NetworkConfig.EnableSceneManagement) + { + // Update the observed spawned NetworkObjects always for the server + // Update for a newly connected player only when scene management is disabled + NetworkManager.SpawnManager.UpdateObservedNetworkObjects(ownerClientId); + } - // If scene management is disabled, then we are done and notify the local host-server the client is connected - if (!NetworkManager.NetworkConfig.EnableSceneManagement) - { - NetworkManager.ConnectedClients[ownerClientId].IsConnected = true; - InvokeOnClientConnectedCallback(ownerClientId); - if (LocalClient.IsHost) - { - InvokeOnPeerConnectedCallback(ownerClientId); - } - NetworkManager.SpawnManager.DistributeNetworkObjects(ownerClientId); + // Server doesn't send itself the connection approved message + if (ownerClientId != NetworkManager.ServerClientId) + { + if (MockSkippingApproval) + { + NetworkLog.LogInfo("Mocking server not responding with connection approved..."); + return; + } - } - else // Otherwise, let NetworkSceneManager handle the initial scene and NetworkObject synchronization + SendConnectionApprovedMessage(ownerClientId); + + // If scene management is disabled, then we are done and notify the local host-server the client is connected + if (!NetworkManager.NetworkConfig.EnableSceneManagement) + { + NetworkManager.ConnectedClients[ownerClientId].IsConnected = true; + InvokeOnClientConnectedCallback(ownerClientId); + if (LocalClient.IsHost) { - if (NetworkManager.DistributedAuthorityMode && NetworkManager.LocalClient.IsSessionOwner) - { - NetworkManager.SceneManager.SynchronizeNetworkObjects(ownerClientId); - } - else if (!NetworkManager.DistributedAuthorityMode) - { - NetworkManager.SceneManager.SynchronizeNetworkObjects(ownerClientId); - } + InvokeOnPeerConnectedCallback(ownerClientId); } + NetworkManager.SpawnManager.DistributeNetworkObjects(ownerClientId); + } - else // Server just adds itself as an observer to all spawned NetworkObjects + else // Otherwise, let NetworkSceneManager handle the initial scene and NetworkObject synchronization { - LocalClient = client; - NetworkManager.SpawnManager.UpdateObservedNetworkObjects(ownerClientId); - LocalClient.IsConnected = true; - // If running mock service, then set the instance as the default session owner - if (NetworkManager.DistributedAuthorityMode && NetworkManager.DAHost) + if (NetworkManager.DistributedAuthorityMode && NetworkManager.LocalClient.IsSessionOwner) { - NetworkManager.SetSessionOwner(NetworkManager.LocalClientId); - NetworkManager.SceneManager.InitializeScenesLoaded(); + NetworkManager.SceneManager.SynchronizeNetworkObjects(ownerClientId); } - - if (NetworkManager.DistributedAuthorityMode && NetworkManager.AutoSpawnPlayerPrefabClientSide) + else if (!NetworkManager.DistributedAuthorityMode) { - CreateAndSpawnPlayer(ownerClientId); + NetworkManager.SceneManager.SynchronizeNetworkObjects(ownerClientId); } } - - // Exit early if no player object was spawned - if (!response.CreatePlayerObject || (response.PlayerPrefabHash == null && NetworkManager.NetworkConfig.PlayerPrefab == null)) + } + else // Server just adds itself as an observer to all spawned NetworkObjects + { + // If running mock service, then set the instance as the default session owner + if (NetworkManager.DistributedAuthorityMode && NetworkManager.DAHost) { - return; + NetworkManager.SetSessionOwner(NetworkManager.LocalClientId); + NetworkManager.SceneManager.InitializeScenesLoaded(); } - // Players are always spawned by their respective client, exit early. (DAHost mode anyway, CMB Service will never spawn player prefab) - if (NetworkManager.DistributedAuthorityMode) + if (NetworkManager.DistributedAuthorityMode && NetworkManager.AutoSpawnPlayerPrefabClientSide) { - return; + CreateAndSpawnPlayer(ownerClientId); } - // Separating this into a contained function call for potential further future separation of when this notification is sent. - ApprovedPlayerSpawn(ownerClientId, response.PlayerPrefabHash ?? NetworkManager.NetworkConfig.PlayerPrefab.GetComponent().GlobalObjectIdHash); } - else + + // Exit early if no player object was spawned + if (!createPlayerObject || (playerPrefabHash == null && NetworkManager.NetworkConfig.PlayerPrefab == null)) + { + return; + } + + // Players are always spawned by their respective client, exit early. (DAHost mode anyway, CMB Service will never spawn player prefab) + if (NetworkManager.DistributedAuthorityMode) + { + return; + } + + // Separating this into a contained function call for potential further future separation of when this notification is sent. + ApprovedPlayerSpawn(ownerClientId, playerPrefabHash ?? NetworkManager.NetworkConfig.PlayerPrefab.GetComponent().GlobalObjectIdHash); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void SendConnectionApprovedMessage(ulong approvedClientId) + { + var message = new ConnectionApprovedMessage + { + OwnerClientId = approvedClientId, + NetworkTick = NetworkManager.LocalTime.Tick, + IsDistributedAuthority = NetworkManager.DistributedAuthorityMode, + ConnectedClientIds = new NativeArray(ConnectedClientIds.Count, Allocator.Temp) + }; + + // Do a no-memory allocation copy of the current list of connected clients + for (int i = 0; i < ConnectedClientIds.Count; i++) { - if (!string.IsNullOrEmpty(response.Reason)) + message.ConnectedClientIds[i] = ConnectedClientIds[i]; + } + + // Send existing spawned objects when scene management is disabled + if (!NetworkManager.NetworkConfig.EnableSceneManagement && NetworkManager.SpawnManager.SpawnedObjectsList.Count != 0) + { + message.SpawnedObjectsList = NetworkManager.SpawnManager.SpawnedObjectsList; + } + + // Calculate and collate the most recent message versions that can be used based on the newest supported version that the client sent and the server understands. + message.MessageVersions = new NativeArray(MessageManager.MessageHandlers.Length, Allocator.Temp); + for (int index = 0; index < MessageManager.MessageHandlers.Length; index++) + { + if (MessageManager.MessageTypes[index] != null) { - var disconnectReason = new DisconnectReasonMessage + var type = MessageManager.MessageTypes[index]; + message.MessageVersions[index] = new MessageVersionData { - Reason = response.Reason + Hash = XXHash.Hash32(type.FullName), + Version = MessageManager.GetLocalVersion(type) }; - SendMessage(ref disconnectReason, NetworkDelivery.Reliable, ownerClientId); - MessageManager.ProcessSendQueues(); } - DisconnectRemoteClient(ownerClientId); } + + SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, approvedClientId); + + message.MessageVersions.Dispose(); + message.ConnectedClientIds.Dispose(); } + /// /// Client-Side Spawning in distributed authority mode uses this to spawn the player. /// @@ -1041,6 +1082,7 @@ internal NetworkClient AddClient(ulong clientId) NetworkManager.MessageManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, NetworkManager.CurrentSessionOwner); } } + if (!ConnectedClientIds.Contains(clientId)) { ConnectedClientIds.Add(clientId); @@ -1115,7 +1157,6 @@ internal void OnClientDisconnectFromServer(ulong clientId) // If we are shutting down and this is the server or host disconnecting, then ignore // clean up as everything that needs to be destroyed will be during shutdown. - if (NetworkManager.ShutdownInProgress && clientId == NetworkManager.ServerClientId) { return; @@ -1290,44 +1331,14 @@ internal void OnClientDisconnectFromServer(ulong clientId) MessageManager?.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, ConnectedClientIds); // Used for testing/validation purposes only -#if ENABLE_DAHOST_AUTOPROMOTE_SESSION_OWNER - if (NetworkManager.DistributedAuthorityMode && !NetworkManager.ShutdownInProgress && NetworkManager.IsListening) - { - var newSessionOwner = NetworkManager.LocalClientId; - if (ConnectedClientIds.Count > 1) - { - var lowestRTT = ulong.MaxValue; - var unityTransport = NetworkManager.NetworkConfig.NetworkTransport as Transports.UTP.UnityTransport; - - foreach (var identifier in ConnectedClientIds) - { - if (identifier == NetworkManager.LocalClientId) - { - continue; - } - var rtt = unityTransport.GetCurrentRtt(identifier); - if (rtt < lowestRTT) - { - newSessionOwner = identifier; - lowestRTT = rtt; - } - } - } - - var sessionOwnerMessage = new SessionOwnerMessage() - { - SessionOwner = newSessionOwner, - }; - MessageManager?.SendMessage(ref sessionOwnerMessage, NetworkDelivery.ReliableFragmentedSequenced, ConnectedClientIds); - NetworkManager.SetSessionOwner(newSessionOwner); - } -#endif + // Promote a new session owner when the ENABLE_DAHOST_AUTOPROMOTE_SESSION_OWNER scripting define is set + DaHostPromoteSessionOwner(); } // If the client ID transport map exists - if (ClientIdToTransportIdMap.ContainsKey(clientId)) + var (transportId, idExists) = ClientIdToTransportId(clientId); + if (idExists) { - var transportId = ClientIdToTransportId(clientId); NetworkManager.NetworkConfig.NetworkTransport.DisconnectRemoteClient(transportId); InvokeOnClientDisconnectCallback(clientId); @@ -1386,16 +1397,7 @@ internal void DisconnectClient(ulong clientId, string reason = null) return; } - if (!string.IsNullOrEmpty(reason)) - { - var disconnectReason = new DisconnectReasonMessage - { - Reason = reason - }; - SendMessage(ref disconnectReason, NetworkDelivery.Reliable, clientId); - } - - DisconnectRemoteClient(clientId); + HandleConnectionDisconnect(clientId, reason); } /// @@ -1612,5 +1614,42 @@ internal int SendMessage(ref T message, NetworkDelivery delivery, ulong clien return MessageManager.SendMessage(ref message, delivery, clientId); } + + [Conditional("ENABLE_DAHOST_AUTOPROMOTE_SESSION_OWNER")] + private void DaHostPromoteSessionOwner() + { + if (NetworkManager.DistributedAuthorityMode && !NetworkManager.ShutdownInProgress && NetworkManager.IsListening) + { + return; + } + + var newSessionOwner = NetworkManager.LocalClientId; + if (ConnectedClientIds.Count > 1) + { + var lowestRTT = ulong.MaxValue; + var unityTransport = NetworkManager.NetworkConfig.NetworkTransport as Transports.UTP.UnityTransport; + + foreach (var identifier in ConnectedClientIds) + { + if (identifier == NetworkManager.LocalClientId) + { + continue; + } + var rtt = unityTransport.GetCurrentRtt(identifier); + if (rtt < lowestRTT) + { + newSessionOwner = identifier; + lowestRTT = rtt; + } + } + } + + var sessionOwnerMessage = new SessionOwnerMessage() + { + SessionOwner = newSessionOwner, + }; + MessageManager?.SendMessage(ref sessionOwnerMessage, NetworkDelivery.ReliableFragmentedSequenced, ConnectedClientIds); + NetworkManager.SetSessionOwner(newSessionOwner); + } } } diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index 5fe071ce0e..77b095e3ab 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs @@ -1441,18 +1441,13 @@ private void HostServerInitialize() } } - response.Approved = true; - ConnectionManager.HandleConnectionApproval(ServerClientId, response); + ConnectionManager.HandleConnectionApproval(ServerClientId, response.CreatePlayerObject, response.PlayerPrefabHash, response.Position, response.Rotation); } else { - var response = new ConnectionApprovalResponse - { - Approved = true, - // Distributed authority always returns true since the client side handles spawning (whether automatically or manually) - CreatePlayerObject = DistributedAuthorityMode || NetworkConfig.PlayerPrefab != null, - }; - ConnectionManager.HandleConnectionApproval(ServerClientId, response); + // Distributed authority always tries to create the player object since the client side handles spawning (whether automatically or manually) + var createPlayerObject = DistributedAuthorityMode || NetworkConfig.PlayerPrefab != null; + ConnectionManager.HandleConnectionApproval(ServerClientId, createPlayerObject); } SpawnManager.ServerSpawnSceneObjectsOnStartSweep(); @@ -1473,15 +1468,27 @@ private void HostServerInitialize() /// Get the TransportId from the associated ClientId. /// /// The ClientId to get the TransportId from - /// The TransportId associated with the given ClientId - public ulong GetTransportIdFromClientId(ulong clientId) => ConnectionManager.ClientIdToTransportId(clientId); + /// + /// The TransportId associated with the given ClientId if the given clientId is valid; otherwise + /// + public ulong GetTransportIdFromClientId(ulong clientId) + { + var (id, success) = ConnectionManager.ClientIdToTransportId(clientId); + return success ? id : ulong.MaxValue; + } /// /// Get the ClientId from the associated TransportId. /// /// The TransportId to get the ClientId from - /// The ClientId from the associated TransportId - public ulong GetClientIdFromTransportId(ulong transportId) => ConnectionManager.TransportIdToClientId(transportId); + /// + /// The ClientId from the associated TransportId if the given transportId is valid; otherwise + /// + public ulong GetClientIdFromTransportId(ulong transportId) + { + var (id, success) = ConnectionManager.TransportIdToClientId(transportId); + return success ? id : ulong.MaxValue; + } /// /// Disconnects the remote client. diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/DefaultMessageSender.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/DefaultMessageSender.cs index 021ad0c604..873762fee8 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/DefaultMessageSender.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/DefaultMessageSender.cs @@ -19,8 +19,19 @@ public DefaultMessageSender(NetworkManager manager) public void Send(ulong clientId, NetworkDelivery delivery, FastBufferWriter batchData) { var sendBuffer = batchData.ToTempByteArray(); + var (transportId, clientExists) = m_ConnectionManager.ClientIdToTransportId(clientId); - m_NetworkTransport.Send(m_ConnectionManager.ClientIdToTransportId(clientId), sendBuffer, delivery); + if (!clientExists) + { + if (m_ConnectionManager.NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogWarning("Trying to send a message to a client who doesn't have a transport connection"); + } + + return; + } + + m_NetworkTransport.Send(transportId, sendBuffer, delivery); } } } diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionRequestMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionRequestMessage.cs index 11aa4f35bc..3b2692ed1e 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionRequestMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionRequestMessage.cs @@ -210,12 +210,16 @@ public void Handle(ref NetworkContext context) } else { - var response = new NetworkManager.ConnectionApprovalResponse + var createPlayerObject = networkManager.NetworkConfig.PlayerPrefab != null; + + // DAHost only: + // Never create the player object on the server if AutoSpawnPlayerPrefabClientSide is set. + if (networkManager.DistributedAuthorityMode && networkManager.AutoSpawnPlayerPrefabClientSide) { - Approved = true, - CreatePlayerObject = networkManager.DistributedAuthorityMode && networkManager.AutoSpawnPlayerPrefabClientSide ? false : networkManager.NetworkConfig.PlayerPrefab != null - }; - networkManager.ConnectionManager.HandleConnectionApproval(senderId, response); + createPlayerObject = false; + } + + networkManager.ConnectionManager.HandleConnectionApproval(senderId, createPlayerObject); } } } diff --git a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs index e6bc0ca957..6a3873404c 100644 --- a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs +++ b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs @@ -909,7 +909,11 @@ private void SendBatchedMessages(SendTarget sendTarget, BatchedSendQueue queue) var mtu = 0; if (m_NetworkManager) { - var ngoClientId = m_NetworkManager.ConnectionManager.TransportIdToClientId(sendTarget.ClientId); + var (ngoClientId, isConnectedClient) = m_NetworkManager.ConnectionManager.TransportIdToClientId(sendTarget.ClientId); + if (!isConnectedClient) + { + return; + } mtu = m_NetworkManager.GetPeerMTU(ngoClientId); } @@ -1321,7 +1325,7 @@ public override ulong GetCurrentRtt(ulong clientId) if (m_NetworkManager != null) { - var transportId = m_NetworkManager.ConnectionManager.ClientIdToTransportId(clientId); + var (transportId, _) = m_NetworkManager.ConnectionManager.ClientIdToTransportId(clientId); var rtt = ExtractRtt(ParseClientId(transportId)); if (rtt > 0) @@ -1347,13 +1351,14 @@ public NetworkEndpoint GetEndpoint(ulong clientId) { if (m_Driver.IsCreated && m_NetworkManager != null && m_NetworkManager.IsListening) { - var transportId = m_NetworkManager.ConnectionManager.ClientIdToTransportId(clientId); + var (transportId, connectionExists) = m_NetworkManager.ConnectionManager.ClientIdToTransportId(clientId); var networkConnection = ParseClientId(transportId); - if (m_Driver.GetConnectionState(networkConnection) == NetworkConnection.State.Connected) + if (connectionExists && m_Driver.GetConnectionState(networkConnection) == NetworkConnection.State.Connected) { return m_Driver.GetRemoteEndpoint(networkConnection); } } + return new NetworkEndpoint(); } @@ -1447,10 +1452,18 @@ public override void Send(ulong clientId, ArraySegment payload, NetworkDel // If the message is sent reliably, then we're over capacity and we can't // provide any reliability guarantees anymore. Disconnect the client since at // this point they're bound to become desynchronized. + if (m_NetworkManager != null) + { + var (ngoClientId, isConnectedClient) = m_NetworkManager.ConnectionManager.TransportIdToClientId(clientId); + if (isConnectedClient) + { + clientId = ngoClientId; + } + + } - var ngoClientId = m_NetworkManager?.ConnectionManager.TransportIdToClientId(clientId) ?? clientId; Debug.LogError($"Couldn't add payload of size {payload.Count} to reliable send queue. " + - $"Closing connection {ngoClientId} as reliability guarantees can't be maintained."); + $"Closing connection {clientId} as reliability guarantees can't be maintained."); if (clientId == m_ServerClientId) { diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs index 522d5e80ac..e7ad3595e2 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs @@ -108,16 +108,24 @@ private void OnConnectionEvent(NetworkManager networkManager, ConnectionEventDat /// private bool TransportIdCleanedUp() { - if (m_ServerNetworkManager.ConnectionManager.TransportIdToClientId(m_TransportClientId) == m_ClientId) + var (clientId, isConnected) = m_ServerNetworkManager.ConnectionManager.TransportIdToClientId(m_TransportClientId); + if (isConnected) { return false; } - if (m_ServerNetworkManager.ConnectionManager.ClientIdToTransportId(m_ClientId) == m_TransportClientId) + if (clientId == m_ClientId) { return false; } - return true; + + var (transportId, connectionExists) = m_ServerNetworkManager.ConnectionManager.ClientIdToTransportId(m_ClientId); + if (connectionExists) + { + return false; + } + + return transportId != m_TransportClientId; } /// @@ -145,7 +153,9 @@ public IEnumerator ClientPlayerDisconnected([Values] ClientDisconnectType client var serverSideClientPlayer = m_ServerNetworkManager.ConnectionManager.ConnectedClients[m_ClientId].PlayerObject; - m_TransportClientId = m_ServerNetworkManager.ConnectionManager.ClientIdToTransportId(m_ClientId); + bool connectionExists; + (m_TransportClientId, connectionExists) = m_ServerNetworkManager.ConnectionManager.ClientIdToTransportId(m_ClientId); + Assert.IsTrue(connectionExists); if (clientDisconnectType == ClientDisconnectType.ServerDisconnectsClient) { diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs index 0756d94f38..6419b02c70 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs @@ -484,6 +484,17 @@ protected void SetDistributedAuthorityProperties(NetworkManager networkManager) /// protected virtual bool m_EnableTimeTravel => false; + /// + /// When true, and will use a + /// as the on the created server and/or clients. + /// When false, a is used. + /// + /// + /// This defaults to, and is required to be true when is true. + /// will not work with the component. + /// + protected virtual bool m_UseMockTransport => m_EnableTimeTravel; + /// /// If this is false, SetUp will call OnInlineSetUp instead of OnSetUp. /// This is a performance advantage when not using the coroutine functionality, as a coroutine that @@ -638,7 +649,7 @@ public IEnumerator SetUp() VerboseDebugLog.Clear(); VerboseDebug($"Entering {nameof(SetUp)}"); NetcodeLogAssert = new NetcodeLogAssert(); - if (m_EnableTimeTravel) + if (m_UseMockTransport) { if (m_NetworkManagerInstatiationMode == NetworkManagerInstatiationMode.AllTests) { @@ -648,8 +659,11 @@ public IEnumerator SetUp() { MockTransport.Reset(); } + } - // Setup the frames per tick for time travel advance to next tick + // Setup the frames per tick for time travel advance to next tick + if (m_EnableTimeTravel) + { ConfigureFramesPerTick(); } @@ -781,7 +795,7 @@ protected void CreateServerAndClients(int numberOfClients) } // Create multiple NetworkManager instances - if (!NetcodeIntegrationTestHelpers.Create(numberOfClients, out NetworkManager server, out NetworkManager[] clients, m_TargetFrameRate, m_CreateServerFirst, m_EnableTimeTravel, m_UseCmbService)) + if (!NetcodeIntegrationTestHelpers.Create(numberOfClients, out NetworkManager server, out NetworkManager[] clients, m_TargetFrameRate, m_CreateServerFirst, m_UseMockTransport, m_UseCmbService)) { Debug.LogError("Failed to create instances"); Assert.Fail("Failed to create instances"); @@ -872,7 +886,7 @@ protected virtual bool ShouldWaitForNewClientToConnect(NetworkManager networkMan /// The newly created . protected NetworkManager CreateNewClient() { - var networkManager = NetcodeIntegrationTestHelpers.CreateNewClient(m_ClientNetworkManagers.Length, m_EnableTimeTravel, m_UseCmbService); + var networkManager = NetcodeIntegrationTestHelpers.CreateNewClient(m_ClientNetworkManagers.Length, m_UseMockTransport, m_UseCmbService); networkManager.NetworkConfig.PlayerPrefab = m_PlayerPrefab; SetDistributedAuthorityProperties(networkManager); @@ -981,7 +995,7 @@ private bool AllPlayerObjectClonesSpawned(NetworkManager joinedClient) /// protected void CreateAndStartNewClientWithTimeTravel() { - var networkManager = NetcodeIntegrationTestHelpers.CreateNewClient(m_ClientNetworkManagers.Length, m_EnableTimeTravel); + var networkManager = NetcodeIntegrationTestHelpers.CreateNewClient(m_ClientNetworkManagers.Length, m_UseMockTransport); networkManager.NetworkConfig.PlayerPrefab = m_PlayerPrefab; SetDistributedAuthorityProperties(networkManager); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs index 92fb80e265..c8e40ae256 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs @@ -327,7 +327,18 @@ public static bool Create(int clientCount, out NetworkManager server, out Networ return true; } - internal static NetworkManager CreateNewClient(int identifier, bool mockTransport = false, bool useCmbService = false) + /// + /// Creates a new and configures it for use in a multi instance setting. + /// + /// The ClientId representation that is used in the name of the NetworkManager + /// + /// When true, the client is created with a ; otherwise a is added + /// + /// + /// Whether to configure the client to run against a hosted build of the CMB Service. Only applies if mockTransport is set to false. + /// + /// The newly created component. + public static NetworkManager CreateNewClient(int identifier, bool mockTransport = false, bool useCmbService = false) { // Create gameObject var go = new GameObject("NetworkManager - Client - " + identifier); @@ -351,7 +362,7 @@ internal static NetworkManager CreateNewClient(int identifier, bool mockTranspor /// Output array containing the created NetworkManager instances /// When true, uses mock transport for testing, otherwise uses real transport. Default value is false /// If true, each client will be created with transport configured to connect to a locally hosted da service - /// Returns if the clients were successfully created and configured, otherwise . + /// Returns true if the clients were successfully created and configured, otherwise false. public static bool CreateNewClients(int clientCount, out NetworkManager[] clients, bool useMockTransport = false, bool useCmbService = false) { clients = new NetworkManager[clientCount]; diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/TransportTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/TransportTests.cs new file mode 100644 index 0000000000..c5a7b6c039 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/TransportTests.cs @@ -0,0 +1,47 @@ +using System.Collections; +using NUnit.Framework; +using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine.TestTools; + +namespace Unity.Netcode.RuntimeTests +{ + [TestFixture(HostOrServer.Server)] + [TestFixture(HostOrServer.Host)] + internal class TransportTests : NetcodeIntegrationTest + { + protected override int NumberOfClients => 2; + + protected override bool m_UseMockTransport => true; + + public TransportTests(HostOrServer hostOrServer) : base(hostOrServer) { } + + [UnityTest] + public IEnumerator MultipleDisconnectEventsNoop() + { + var clientToDisconnect = GetNonAuthorityNetworkManager(0); + var clientTransport = clientToDisconnect.NetworkConfig.NetworkTransport; + + var otherClient = GetNonAuthorityNetworkManager(1); + + // Send multiple disconnect events + clientTransport.DisconnectLocalClient(); + clientTransport.DisconnectLocalClient(); + + // completely stop and clean up the client + yield return StopOneClient(clientToDisconnect); + + var expectedConnectedClients = m_UseHost ? NumberOfClients : NumberOfClients - 1; + yield return WaitForConditionOrTimeOut(() => otherClient.ConnectedClientsIds.Count == expectedConnectedClients); + AssertOnTimeout($"Incorrect number of connected clients. Expected: {expectedConnectedClients}, have: {otherClient.ConnectedClientsIds.Count}"); + + // Start a new client to ensure everything is still working + yield return CreateAndStartNewClient(); + + var newExpectedClients = m_UseHost ? NumberOfClients + 1 : NumberOfClients; + yield return WaitForConditionOrTimeOut(() => otherClient.ConnectedClientsIds.Count == newExpectedClients); + AssertOnTimeout($"Incorrect number of connected clients. Expected: {newExpectedClients}, have: {otherClient.ConnectedClientsIds.Count}"); + + + } + } +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/TransportTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/TransportTests.cs.meta new file mode 100644 index 0000000000..e715992bef --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/TransportTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: ec3892f3f62743de89b76b1d9a729afd +timeCreated: 1759788373 \ No newline at end of file