diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index cdc9fdb43..1c7ad7118 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -7,6 +7,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com). +## [Unreleased] + +### Added +- Added `NetworkManager.IsApproved` flag that is set to `true` a client has been approved.(#2261) + +### Changed + + +### Fixed + +- Fixed `NetworkManager.ApprovalTimeout` will not timeout due to slower client synchronization times as it now uses the added `NetworkManager.IsApproved` flag to determined if the client has been approved or not.(#2261) + + ## [1.1.0] - 2022-10-19 ### Added diff --git a/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs b/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs index b8a21282c..258b64ea0 100644 --- a/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs +++ b/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs @@ -232,6 +232,10 @@ namespace Unity.Netcode.Components private void BuildTransitionStateInfoList() { #if UNITY_EDITOR + if (UnityEditor.EditorApplication.isUpdating) + { + return; + } TransitionStateInfoList = new List(); var animatorController = m_Animator.runtimeAnimatorController as AnimatorController; if (animatorController == null) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index a30b94ed7..20794b49a 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs @@ -366,10 +366,22 @@ namespace Unity.Netcode public bool IsListening { get; internal set; } /// - /// Gets if we are connected as a client + /// When true, the client is connected, approved, and synchronized with + /// the server. /// public bool IsConnectedClient { get; internal set; } + /// + /// Is true when the client has been approved. + /// + /// + /// This only reflects the client's approved status and does not mean the client + /// has finished the connection and synchronization process. The server-host will + /// always be approved upon being starting the + /// + /// + public bool IsApproved { get; internal set; } + /// /// Can be used to determine if the is currently shutting itself down /// @@ -877,6 +889,8 @@ namespace Unity.Netcode return; } + IsApproved = false; + ComponentFactory.SetDefaults(); if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) @@ -1018,7 +1032,6 @@ namespace Unity.Netcode } Initialize(true); - IsServer = true; IsClient = false; IsListening = true; @@ -1031,6 +1044,7 @@ namespace Unity.Netcode SpawnManager.ServerSpawnSceneObjectsOnStartSweep(); OnServerStarted?.Invoke(); + IsApproved = true; return true; } else @@ -1152,6 +1166,7 @@ namespace Unity.Netcode } response.Approved = true; + IsApproved = true; HandleConnectionApproval(ServerClientId, response); } else @@ -1413,6 +1428,7 @@ namespace Unity.Netcode } IsConnectedClient = false; + IsApproved = false; // We need to clean up NetworkObjects before we reset the IsServer // and IsClient properties. This provides consistency of these two @@ -1649,59 +1665,71 @@ namespace Unity.Netcode private IEnumerator ApprovalTimeout(ulong clientId) { - if (IsServer) + var timeStarted = IsServer ? LocalTime.TimeAsFloat : Time.realtimeSinceStartup; + var timedOut = false; + var connectionApproved = false; + var connectionNotApproved = false; + var timeoutMarker = timeStarted + NetworkConfig.ClientConnectionBufferTimeout; + + while (IsListening && !ShutdownInProgress && !timedOut && !connectionApproved) { - NetworkTime timeStarted = LocalTime; + yield return null; + // Check if we timed out + timedOut = timeoutMarker < (IsServer ? LocalTime.TimeAsFloat : Time.realtimeSinceStartup); - //We yield every frame incase a pending client disconnects and someone else gets its connection id - while (IsListening && (LocalTime - timeStarted).Time < NetworkConfig.ClientConnectionBufferTimeout && PendingClients.ContainsKey(clientId)) + if (IsServer) { - yield return null; + // When the client is no longer in the pending clients list and is in the connected clients list + // it has been approved + connectionApproved = !PendingClients.ContainsKey(clientId) && ConnectedClients.ContainsKey(clientId); + + // For the server side, if the client is in neither list then it was declined or the client disconnected + connectionNotApproved = !PendingClients.ContainsKey(clientId) && !ConnectedClients.ContainsKey(clientId); } - - if (!IsListening) + else { - yield break; - } - - if (PendingClients.ContainsKey(clientId) && !ConnectedClients.ContainsKey(clientId)) - { - // Timeout - if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) - { - NetworkLog.LogInfo($"Client {clientId} Handshake Timed Out"); - } - - DisconnectClient(clientId); + connectionApproved = IsApproved; } } - else + + // Exit coroutine if we are no longer listening or a shutdown is in progress (client or server) + if (!IsListening || ShutdownInProgress) { - float timeStarted = Time.realtimeSinceStartup; - //We yield every frame in case a pending client disconnects and someone else gets its connection id - while (IsListening && (Time.realtimeSinceStartup - timeStarted) < NetworkConfig.ClientConnectionBufferTimeout && !IsConnectedClient) - { - yield return null; - } + yield break; + } - if (!IsConnectedClient && NetworkLog.CurrentLogLevel <= LogLevel.Normal) + // If the client timed out or was not approved + if (timedOut || connectionNotApproved) + { + // Timeout + if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) { - // TODO: Possibly add a callback for users to be notified of this condition here? - NetworkLog.LogWarning($"[ApprovalTimeout] Client timed out! You might need to increase the {nameof(NetworkConfig.ClientConnectionBufferTimeout)} duration. Approval Check Start: {timeStarted} | Approval Check Stopped: {Time.realtimeSinceStartup}"); - } - - if (!IsListening) - { - yield break; - } - - if (!IsConnectedClient) - { - // Timeout - if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) + if (timedOut) { - NetworkLog.LogInfo("Server Handshake Timed Out"); + if (IsServer) + { + // Log a warning that the transport detected a connection but then did not receive a follow up connection request message. + // (hacking or something happened to the server's network connection) + NetworkLog.LogWarning($"Server detected a transport connection from Client-{clientId}, but timed out waiting for the connection request message."); + } + else + { + // We only provide informational logging for the client side + NetworkLog.LogInfo("Timed out waiting for the server to approve the connection request."); + } } + else if (connectionNotApproved) + { + NetworkLog.LogInfo($"Client-{clientId} was either denied approval or disconnected while being approved."); + } + } + + if (IsServer) + { + DisconnectClient(clientId); + } + else + { Shutdown(true); } } diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs index 14c4ac3d9..0a685a731 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs @@ -79,6 +79,7 @@ namespace Unity.Netcode networkManager.NetworkTickSystem.Reset(networkManager.NetworkTimeSystem.LocalTime, networkManager.NetworkTimeSystem.ServerTime); networkManager.LocalClient = new NetworkClient() { ClientId = networkManager.LocalClientId }; + networkManager.IsApproved = true; // Only if scene management is disabled do we handle NetworkObject synchronization at this point if (!networkManager.NetworkConfig.EnableSceneManagement) diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index 044a5358a..c2f3dfe55 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs @@ -131,6 +131,18 @@ namespace Unity.Netcode.TestHelpers.Runtime protected bool m_EnableVerboseDebug { get; set; } + /// + /// When set to true, this will bypass the entire + /// wait for clients to connect process. + /// + /// + /// CAUTION: + /// Setting this to true will bypass other helper + /// identification related code, so this should only + /// be used for connection failure oriented testing + /// + protected bool m_BypassConnectionTimeout { get; set; } + /// /// Used to display the various integration test /// stages and can be used to log verbose information @@ -455,31 +467,36 @@ namespace Unity.Netcode.TestHelpers.Runtime // Notification that the server and clients have been started yield return OnStartedServerAndClients(); - // Wait for all clients to connect - yield return WaitForClientsConnectedOrTimeOut(); - AssertOnTimeout($"{nameof(StartServerAndClients)} timed out waiting for all clients to be connected!"); - - if (m_UseHost || m_ServerNetworkManager.IsHost) + // When true, we skip everything else (most likely a connection oriented test) + if (!m_BypassConnectionTimeout) { - // Add the server player instance to all m_ClientSidePlayerNetworkObjects entries - var serverPlayerClones = Object.FindObjectsOfType().Where((c) => c.IsPlayerObject && c.OwnerClientId == m_ServerNetworkManager.LocalClientId); - foreach (var playerNetworkObject in serverPlayerClones) + // Wait for all clients to connect + yield return WaitForClientsConnectedOrTimeOut(); + + AssertOnTimeout($"{nameof(StartServerAndClients)} timed out waiting for all clients to be connected!"); + + if (m_UseHost || m_ServerNetworkManager.IsHost) { - if (!m_PlayerNetworkObjects.ContainsKey(playerNetworkObject.NetworkManager.LocalClientId)) + // Add the server player instance to all m_ClientSidePlayerNetworkObjects entries + var serverPlayerClones = Object.FindObjectsOfType().Where((c) => c.IsPlayerObject && c.OwnerClientId == m_ServerNetworkManager.LocalClientId); + foreach (var playerNetworkObject in serverPlayerClones) { - m_PlayerNetworkObjects.Add(playerNetworkObject.NetworkManager.LocalClientId, new Dictionary()); + if (!m_PlayerNetworkObjects.ContainsKey(playerNetworkObject.NetworkManager.LocalClientId)) + { + m_PlayerNetworkObjects.Add(playerNetworkObject.NetworkManager.LocalClientId, new Dictionary()); + } + m_PlayerNetworkObjects[playerNetworkObject.NetworkManager.LocalClientId].Add(m_ServerNetworkManager.LocalClientId, playerNetworkObject); } - m_PlayerNetworkObjects[playerNetworkObject.NetworkManager.LocalClientId].Add(m_ServerNetworkManager.LocalClientId, playerNetworkObject); } + + ClientNetworkManagerPostStartInit(); + + // Notification that at this time the server and client(s) are instantiated, + // started, and connected on both sides. + yield return OnServerAndClientsConnected(); + + VerboseDebug($"Exiting {nameof(StartServerAndClients)}"); } - - ClientNetworkManagerPostStartInit(); - - // Notification that at this time the server and client(s) are instantiated, - // started, and connected on both sides. - yield return OnServerAndClientsConnected(); - - VerboseDebug($"Exiting {nameof(StartServerAndClients)}"); } } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs index bca5bd827..9f58c5959 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs @@ -1,5 +1,4 @@ using System.Collections; -using System.Linq; using System.Text.RegularExpressions; using NUnit.Framework; using Unity.Netcode.TestHelpers.Runtime; @@ -8,97 +7,95 @@ using UnityEngine.TestTools; namespace Unity.Netcode.RuntimeTests { - [TestFixture(true)] - [TestFixture(false)] + [TestFixture(ApprovalTimedOutTypes.ServerDoesNotRespond)] + [TestFixture(ApprovalTimedOutTypes.ClientDoesNotRequest)] public class ConnectionApprovalTimeoutTests : NetcodeIntegrationTest { protected override int NumberOfClients => 1; - protected override bool CanStartServerAndClients() => false; - - private bool m_UseSceneManagement; - public ConnectionApprovalTimeoutTests(bool useSceneManagement) + public enum ApprovalTimedOutTypes { - m_UseSceneManagement = useSceneManagement; + ClientDoesNotRequest, + ServerDoesNotRespond + } + + private ApprovalTimedOutTypes m_ApprovalFailureType; + + public ConnectionApprovalTimeoutTests(ApprovalTimedOutTypes approvalFailureType) + { + m_ApprovalFailureType = approvalFailureType; } // Must be >= 2 since this is an int value and the test waits for timeout - 1 to try to verify it doesn't // time out early - private const int k_TestTimeoutPeriod = 2; + private const int k_TestTimeoutPeriod = 1; - private void Start() + private Regex m_ExpectedLogMessage; + private LogType m_LogType; + + + protected override IEnumerator OnSetup() { - m_ServerNetworkManager.NetworkConfig.EnableSceneManagement = m_UseSceneManagement; - m_ClientNetworkManagers[0].NetworkConfig.EnableSceneManagement = m_UseSceneManagement; - if (!NetcodeIntegrationTestHelpers.Start(false, m_ServerNetworkManager, m_ClientNetworkManagers)) - { - Debug.LogError("Failed to start instances"); - Assert.Fail("Failed to start instances"); - } + m_BypassConnectionTimeout = true; + return base.OnSetup(); } - [UnityTest] - public IEnumerator WhenClientDoesntRequestApproval_ServerTimesOut() + protected override IEnumerator OnTearDown() { - Start(); - var hook = new MessageCatcher(m_ServerNetworkManager); - m_ServerNetworkManager.MessagingSystem.Hook(hook); ; + m_BypassConnectionTimeout = false; + return base.OnTearDown(); + } + protected override void OnServerAndClientsCreated() + { m_ServerNetworkManager.NetworkConfig.ClientConnectionBufferTimeout = k_TestTimeoutPeriod; m_ServerNetworkManager.LogLevel = LogLevel.Developer; + m_ClientNetworkManagers[0].NetworkConfig.ClientConnectionBufferTimeout = k_TestTimeoutPeriod; m_ClientNetworkManagers[0].LogLevel = LogLevel.Developer; - - yield return new WaitForSeconds(m_ServerNetworkManager.NetworkConfig.ClientConnectionBufferTimeout - 1); - - Assert.AreEqual(0, m_ServerNetworkManager.ConnectedClients.Count); - Assert.AreEqual(1, m_ServerNetworkManager.PendingClients.Count); - - var expectedLogMessage = new Regex($"Client {m_ServerNetworkManager.PendingClients.FirstOrDefault().Key} Handshake Timed Out"); - - NetcodeLogAssert.LogWasNotReceived(LogType.Log, expectedLogMessage); - - yield return new WaitForSeconds(2); - - NetcodeLogAssert.LogWasReceived(LogType.Log, expectedLogMessage); - - Assert.AreEqual(0, m_ServerNetworkManager.ConnectedClients.Count); - Assert.AreEqual(0, m_ServerNetworkManager.PendingClients.Count); + base.OnServerAndClientsCreated(); } - [UnityTest] - public IEnumerator WhenServerDoesntRespondWithApproval_ClientTimesOut() + protected override IEnumerator OnStartedServerAndClients() { - Start(); - - if (m_UseSceneManagement) + if (m_ApprovalFailureType == ApprovalTimedOutTypes.ServerDoesNotRespond) { - var sceneEventHook = new MessageCatcher(m_ClientNetworkManagers[0]); - m_ClientNetworkManagers[0].MessagingSystem.Hook(sceneEventHook); + // We catch (don't process) the incoming approval message to simulate the server not sending the approved message in time + m_ClientNetworkManagers[0].MessagingSystem.Hook(new MessageCatcher(m_ClientNetworkManagers[0])); + m_ExpectedLogMessage = new Regex("Timed out waiting for the server to approve the connection request."); + m_LogType = LogType.Log; } else { - var approvalHook = new MessageCatcher(m_ClientNetworkManagers[0]); - m_ClientNetworkManagers[0].MessagingSystem.Hook(approvalHook); + // We catch (don't process) the incoming connection request message to simulate a transport connection but the client never + // sends (or takes too long to send) the connection request. + m_ServerNetworkManager.MessagingSystem.Hook(new MessageCatcher(m_ServerNetworkManager)); + + // For this test, we know the timed out client will be Client-1 + m_ExpectedLogMessage = new Regex("Server detected a transport connection from Client-1, but timed out waiting for the connection request message."); + m_LogType = LogType.Warning; } + yield return null; + } - m_ClientNetworkManagers[0].NetworkConfig.ClientConnectionBufferTimeout = k_TestTimeoutPeriod; - m_ServerNetworkManager.LogLevel = LogLevel.Developer; - m_ClientNetworkManagers[0].LogLevel = LogLevel.Developer; + [UnityTest] + public IEnumerator ValidateApprovalTimeout() + { + // Delay for half of the wait period + yield return new WaitForSeconds(k_TestTimeoutPeriod * 0.5f); - yield return new WaitForSeconds(m_ClientNetworkManagers[0].NetworkConfig.ClientConnectionBufferTimeout - 1); + // Verify we haven't received the time out message yet + NetcodeLogAssert.LogWasNotReceived(LogType.Log, m_ExpectedLogMessage); - Assert.IsFalse(m_ClientNetworkManagers[0].IsConnectedClient); - Assert.IsTrue(m_ClientNetworkManagers[0].IsListening); + // Wait for 3/4s of the time out period to pass (totaling 1.25x the wait period) + yield return new WaitForSeconds(k_TestTimeoutPeriod * 0.75f); - var expectedLogMessage = new Regex("Server Handshake Timed Out"); - NetcodeLogAssert.LogWasNotReceived(LogType.Log, expectedLogMessage); + // We should have the test relative log message by this time. + NetcodeLogAssert.LogWasReceived(m_LogType, m_ExpectedLogMessage); - yield return new WaitForSeconds(2); - - NetcodeLogAssert.LogWasReceived(LogType.Log, expectedLogMessage); - - Assert.IsFalse(m_ClientNetworkManagers[0].IsConnectedClient); - Assert.IsFalse(m_ClientNetworkManagers[0].IsListening); + // It should only have the host client connected + Assert.AreEqual(1, m_ServerNetworkManager.ConnectedClients.Count, $"Expected only one client when there were {m_ServerNetworkManager.ConnectedClients.Count} clients connected!"); + Assert.AreEqual(0, m_ServerNetworkManager.PendingClients.Count, $"Expected no pending clients when there were {m_ServerNetworkManager.PendingClients.Count} pending clients!"); + Assert.True(!m_ClientNetworkManagers[0].IsApproved, $"Expected the client to not have been approved, but it was!"); } } }