Skip to content

Commit 4cb86a5

Browse files
ShadauxCatShadauxCat
authored
fix: Fixed ConnectionApprovalTimeout not working on the client side [MTT-3451] (#2164)
* fix: Fixed ConnectionApprovalTimeout not working on the client side * Changelog * standards
1 parent 20ad052 commit 4cb86a5

13 files changed

Lines changed: 495 additions & 98 deletions

‎com.unity.netcode.gameobjects/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
1717
- Fixed issue where `NetworkTransform` was not honoring the InLocalSpace property on the authority side during OnNetworkSpawn. (#2170)
1818
- Implicit conversion of NetworkObjectReference to GameObject will now return null instead of throwing an exception if the referenced object could not be found (i.e., was already despawned) (#2158)
1919
- Fixed warning resulting from a stray NetworkAnimator.meta file (#2153)
20+
- Fixed Connection Approval Timeout not working client side. (#2164)
2021
- Fixed ClientRpcs always reporting in the profiler view as going to all clients, even when limited to a subset of clients by `ClientRpcParams`. (#2144)
2122
- Fixed RPC codegen failing to choose the correct extension methods for `FastBufferReader` and `FastBufferWriter` when the parameters were a generic type (i.e., List<int>) and extensions for multiple instantiations of that type have been defined (i.e., List<int> and List<string>) (#2142)
2223
- Fixed throwing an exception in `OnNetworkUpdate` causing other `OnNetworkUpdate` calls to not be executed. (#1739)

‎com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs‎

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,23 +1623,56 @@ private void SendConnectionRequest()
16231623

16241624
private IEnumerator ApprovalTimeout(ulong clientId)
16251625
{
1626-
NetworkTime timeStarted = LocalTime;
1627-
1628-
//We yield every frame incase a pending client disconnects and someone else gets its connection id
1629-
while ((LocalTime - timeStarted).Time < NetworkConfig.ClientConnectionBufferTimeout && PendingClients.ContainsKey(clientId))
1626+
if (IsServer)
16301627
{
1631-
yield return null;
1632-
}
1628+
NetworkTime timeStarted = LocalTime;
1629+
1630+
//We yield every frame incase a pending client disconnects and someone else gets its connection id
1631+
while (IsListening && (LocalTime - timeStarted).Time < NetworkConfig.ClientConnectionBufferTimeout && PendingClients.ContainsKey(clientId))
1632+
{
1633+
yield return null;
1634+
}
1635+
1636+
if (!IsListening)
1637+
{
1638+
yield break;
1639+
}
1640+
1641+
if (PendingClients.ContainsKey(clientId) && !ConnectedClients.ContainsKey(clientId))
1642+
{
1643+
// Timeout
1644+
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
1645+
{
1646+
NetworkLog.LogInfo($"Client {clientId} Handshake Timed Out");
1647+
}
16331648

1634-
if (PendingClients.ContainsKey(clientId) && !ConnectedClients.ContainsKey(clientId))
1649+
DisconnectClient(clientId);
1650+
}
1651+
}
1652+
else
16351653
{
1636-
// Timeout
1637-
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
1654+
float timeStarted = Time.realtimeSinceStartup;
1655+
1656+
//We yield every frame incase a pending client disconnects and someone else gets its connection id
1657+
while (IsListening && (Time.realtimeSinceStartup - timeStarted) < NetworkConfig.ClientConnectionBufferTimeout && !IsConnectedClient)
1658+
{
1659+
yield return null;
1660+
}
1661+
1662+
if (!IsListening)
16381663
{
1639-
NetworkLog.LogInfo($"Client {clientId} Handshake Timed Out");
1664+
yield break;
16401665
}
16411666

1642-
DisconnectClient(clientId);
1667+
if (!IsConnectedClient)
1668+
{
1669+
// Timeout
1670+
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
1671+
{
1672+
NetworkLog.LogInfo("Server Handshake Timed Out");
1673+
}
1674+
Shutdown(true);
1675+
}
16431676
}
16441677
}
16451678

‎com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
using UnityEngine.SceneManagement;
88
using UnityEngine.TestTools;
99
using System.Runtime.CompilerServices;
10-
10+
using Unity.Netcode.RuntimeTests;
1111
using Object = UnityEngine.Object;
1212

1313
namespace Unity.Netcode.TestHelpers.Runtime
@@ -25,6 +25,8 @@ public abstract class NetcodeIntegrationTest
2525
protected static TimeoutHelper s_GlobalTimeoutHelper = new TimeoutHelper(8.0f);
2626
protected static WaitForSeconds s_DefaultWaitForTick = new WaitForSeconds(1.0f / k_DefaultTickRate);
2727

28+
public NetcodeLogAssert NetcodeLogAssert;
29+
2830
/// <summary>
2931
/// Registered list of all NetworkObjects spawned.
3032
/// Format is as follows:
@@ -207,6 +209,7 @@ public IEnumerator SetUp()
207209
{
208210
VerboseDebug($"Entering {nameof(SetUp)}");
209211

212+
NetcodeLogAssert = new NetcodeLogAssert();
210213
yield return OnSetup();
211214
if (m_NetworkManagerInstatiationMode == NetworkManagerInstatiationMode.AllTests && m_ServerNetworkManager == null ||
212215
m_NetworkManagerInstatiationMode == NetworkManagerInstatiationMode.PerTest)
@@ -596,6 +599,7 @@ public IEnumerator TearDown()
596599
}
597600

598601
VerboseDebug($"Exiting {nameof(TearDown)}");
602+
NetcodeLogAssert.Dispose();
599603
}
600604

601605
/// <summary>

‎com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeLogAssert.cs‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Text.RegularExpressions;
4+
using NUnit.Framework;
5+
using UnityEngine;
6+
7+
namespace Unity.Netcode.RuntimeTests
8+
{
9+
public class NetcodeLogAssert
10+
{
11+
private struct LogData
12+
{
13+
public LogType LogType;
14+
public string Message;
15+
public string StackTrace;
16+
}
17+
18+
private readonly object m_Lock = new object();
19+
private bool m_Disposed;
20+
21+
private List<LogData> AllLogs { get; }
22+
23+
public NetcodeLogAssert()
24+
{
25+
AllLogs = new List<LogData>();
26+
Activate();
27+
}
28+
29+
private void Activate()
30+
{
31+
Application.logMessageReceivedThreaded += AddLog;
32+
}
33+
34+
private void Deactivate()
35+
{
36+
Application.logMessageReceivedThreaded -= AddLog;
37+
}
38+
39+
public void AddLog(string message, string stacktrace, LogType type)
40+
{
41+
lock (m_Lock)
42+
{
43+
var log = new LogData
44+
{
45+
LogType = type,
46+
Message = message,
47+
StackTrace = stacktrace,
48+
};
49+
50+
AllLogs.Add(log);
51+
}
52+
}
53+
54+
public void Dispose()
55+
{
56+
Dispose(true);
57+
GC.SuppressFinalize(this);
58+
}
59+
60+
private void Dispose(bool disposing)
61+
{
62+
if (m_Disposed)
63+
{
64+
return;
65+
}
66+
67+
m_Disposed = true;
68+
69+
if (disposing)
70+
{
71+
Deactivate();
72+
}
73+
}
74+
75+
public void LogWasNotReceived(LogType type, string message)
76+
{
77+
lock (m_Lock)
78+
{
79+
foreach (var logEvent in AllLogs)
80+
{
81+
if (logEvent.LogType == type && message.Equals(logEvent.Message))
82+
{
83+
Assert.Fail($"Unexpected log: [{logEvent.LogType}] {logEvent.Message}");
84+
}
85+
}
86+
}
87+
}
88+
89+
public void LogWasNotReceived(LogType type, Regex messageRegex)
90+
{
91+
lock (m_Lock)
92+
{
93+
foreach (var logEvent in AllLogs)
94+
{
95+
if (logEvent.LogType == type && messageRegex.IsMatch(logEvent.Message))
96+
{
97+
Assert.Fail($"Unexpected log: [{logEvent.LogType}] {logEvent.Message}");
98+
}
99+
}
100+
}
101+
}
102+
103+
public void LogWasReceived(LogType type, string message)
104+
{
105+
lock (m_Lock)
106+
{
107+
var found = false;
108+
foreach (var logEvent in AllLogs)
109+
{
110+
if (logEvent.LogType == type && message.Equals(logEvent.Message))
111+
{
112+
found = true;
113+
break;
114+
}
115+
}
116+
117+
if (!found)
118+
{
119+
Assert.Fail($"Expected log was not received: [{type}] {message}");
120+
}
121+
}
122+
}
123+
124+
public void LogWasReceived(LogType type, Regex messageRegex)
125+
{
126+
lock (m_Lock)
127+
{
128+
var found = false;
129+
foreach (var logEvent in AllLogs)
130+
{
131+
if (logEvent.LogType == type && messageRegex.IsMatch(logEvent.Message))
132+
{
133+
found = true;
134+
}
135+
}
136+
137+
if (!found)
138+
{
139+
Assert.Fail($"Expected log was not received: [{type}] {messageRegex}");
140+
}
141+
}
142+
}
143+
144+
public void Reset()
145+
{
146+
lock (m_Lock)
147+
{
148+
AllLogs.Clear();
149+
}
150+
}
151+
}
152+
}

‎com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeLogAssert.cs.meta‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
using System.Collections;
2+
using System.Linq;
3+
using System.Text.RegularExpressions;
4+
using NUnit.Framework;
5+
using Unity.Netcode.TestHelpers.Runtime;
6+
using UnityEngine;
7+
using UnityEngine.TestTools;
8+
9+
namespace Unity.Netcode.RuntimeTests
10+
{
11+
[TestFixture(true)]
12+
[TestFixture(false)]
13+
public class ConnectionApprovalTimeoutTests : NetcodeIntegrationTest
14+
{
15+
protected override int NumberOfClients => 1;
16+
17+
protected override bool CanStartServerAndClients() => false;
18+
19+
private bool m_UseSceneManagement;
20+
public ConnectionApprovalTimeoutTests(bool useSceneManagement)
21+
{
22+
m_UseSceneManagement = useSceneManagement;
23+
}
24+
25+
// Must be >= 2 since this is an int value and the test waits for timeout - 1 to try to verify it doesn't
26+
// time out early
27+
private const int k_TestTimeoutPeriod = 2;
28+
29+
private void Start()
30+
{
31+
m_ServerNetworkManager.NetworkConfig.EnableSceneManagement = m_UseSceneManagement;
32+
m_ClientNetworkManagers[0].NetworkConfig.EnableSceneManagement = m_UseSceneManagement;
33+
if (!NetcodeIntegrationTestHelpers.Start(false, m_ServerNetworkManager, m_ClientNetworkManagers))
34+
{
35+
Debug.LogError("Failed to start instances");
36+
Assert.Fail("Failed to start instances");
37+
}
38+
}
39+
40+
[UnityTest]
41+
public IEnumerator WhenClientDoesntRequestApproval_ServerTimesOut()
42+
{
43+
Start();
44+
var hook = new MessageCatcher<ConnectionRequestMessage>(m_ServerNetworkManager);
45+
m_ServerNetworkManager.MessagingSystem.Hook(hook); ;
46+
47+
m_ServerNetworkManager.NetworkConfig.ClientConnectionBufferTimeout = k_TestTimeoutPeriod;
48+
m_ServerNetworkManager.LogLevel = LogLevel.Developer;
49+
m_ClientNetworkManagers[0].LogLevel = LogLevel.Developer;
50+
51+
yield return new WaitForSeconds(m_ServerNetworkManager.NetworkConfig.ClientConnectionBufferTimeout - 1);
52+
53+
Assert.AreEqual(0, m_ServerNetworkManager.ConnectedClients.Count);
54+
Assert.AreEqual(1, m_ServerNetworkManager.PendingClients.Count);
55+
56+
var expectedLogMessage = new Regex($"Client {m_ServerNetworkManager.PendingClients.FirstOrDefault().Key} Handshake Timed Out");
57+
58+
NetcodeLogAssert.LogWasNotReceived(LogType.Log, expectedLogMessage);
59+
60+
yield return new WaitForSeconds(2);
61+
62+
NetcodeLogAssert.LogWasReceived(LogType.Log, expectedLogMessage);
63+
64+
Assert.AreEqual(0, m_ServerNetworkManager.ConnectedClients.Count);
65+
Assert.AreEqual(0, m_ServerNetworkManager.PendingClients.Count);
66+
}
67+
68+
[UnityTest]
69+
public IEnumerator WhenServerDoesntRespondWithApproval_ClientTimesOut()
70+
{
71+
Start();
72+
73+
if (m_UseSceneManagement)
74+
{
75+
var sceneEventHook = new MessageCatcher<SceneEventMessage>(m_ClientNetworkManagers[0]);
76+
m_ClientNetworkManagers[0].MessagingSystem.Hook(sceneEventHook);
77+
}
78+
else
79+
{
80+
var approvalHook = new MessageCatcher<ConnectionApprovedMessage>(m_ClientNetworkManagers[0]);
81+
m_ClientNetworkManagers[0].MessagingSystem.Hook(approvalHook);
82+
}
83+
84+
m_ClientNetworkManagers[0].NetworkConfig.ClientConnectionBufferTimeout = k_TestTimeoutPeriod;
85+
m_ServerNetworkManager.LogLevel = LogLevel.Developer;
86+
m_ClientNetworkManagers[0].LogLevel = LogLevel.Developer;
87+
88+
yield return new WaitForSeconds(m_ClientNetworkManagers[0].NetworkConfig.ClientConnectionBufferTimeout - 1);
89+
90+
Assert.IsFalse(m_ClientNetworkManagers[0].IsConnectedClient);
91+
Assert.IsTrue(m_ClientNetworkManagers[0].IsListening);
92+
93+
var expectedLogMessage = new Regex("Server Handshake Timed Out");
94+
NetcodeLogAssert.LogWasNotReceived(LogType.Log, expectedLogMessage);
95+
96+
yield return new WaitForSeconds(2);
97+
98+
NetcodeLogAssert.LogWasReceived(LogType.Log, expectedLogMessage);
99+
100+
Assert.IsFalse(m_ClientNetworkManagers[0].IsConnectedClient);
101+
Assert.IsFalse(m_ClientNetworkManagers[0].IsListening);
102+
}
103+
}
104+
}

‎com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs.meta‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)