diff --git a/Source/Common/Networking/LiteNetConnection.cs b/Source/Common/Networking/LiteNetConnection.cs
index b3a8c5d05..aa9fdc201 100644
--- a/Source/Common/Networking/LiteNetConnection.cs
+++ b/Source/Common/Networking/LiteNetConnection.cs
@@ -7,12 +7,16 @@ public class LiteNetConnection(NetPeer peer) : ConnectionBase
{
public readonly NetPeer peer = peer;
+ ///
+ /// Sends without first checking the peer's transport state. That state changes on LiteNetLib's
+ /// own thread, so it can go stale between approving the send
+ /// against the protocol state and the packet arriving here; a check on this side can never be
+ /// race-free. LiteNetLib discards sends to a peer that has gone away, silently and without
+ /// throwing, so there is nothing left for one to do.
+ ///
protected override void SendRaw(byte[] raw, bool reliable)
{
- if (peer.ConnectionState == ConnectionState.Connected)
- peer.Send(raw, reliable ? DeliveryMethod.ReliableOrdered : DeliveryMethod.Unreliable);
- else
- ServerLog.Error($"SendRaw() called with invalid connection state ({peer}): {peer.ConnectionState}");
+ peer.Send(raw, reliable ? DeliveryMethod.ReliableOrdered : DeliveryMethod.Unreliable);
}
protected override void OnClose(ServerDisconnectPacket? goodbye)
diff --git a/Source/Common/Networking/MpConnectionState.cs b/Source/Common/Networking/MpConnectionState.cs
index 8551f48eb..ba8a62be7 100644
--- a/Source/Common/Networking/MpConnectionState.cs
+++ b/Source/Common/Networking/MpConnectionState.cs
@@ -43,6 +43,19 @@ public static ConnectionStateEnum GetStateEnumOf(MpConnectionState state)
? null
: (MpConnectionState)Activator.CreateInstance(StateImpls[(int)state], conn);
+ ///
+ /// Removes the handlers registered for one state, so a different implementation can take its
+ /// place. is additive and rejects a second handler for the
+ /// same packet, which suits registering each state once at start-up but leaves no way to
+ /// replace one afterwards.
+ ///
+ public static void ClearImplementation(ConnectionStateEnum state)
+ {
+ StateImpls[(int)state] = null!;
+ for (var packetId = 0; packetId < packetHandlers.GetLength(1); packetId++)
+ packetHandlers[(int)state, packetId] = null;
+ }
+
public static void SetImplementation(ConnectionStateEnum state, Type type)
{
if (!type.IsSubclassOf(typeof(MpConnectionState))) return;
diff --git a/Source/Tests/Helper/TestNetListener.cs b/Source/Tests/Helper/TestNetListener.cs
index 1eebdf695..30c8d790d 100644
--- a/Source/Tests/Helper/TestNetListener.cs
+++ b/Source/Tests/Helper/TestNetListener.cs
@@ -15,6 +15,9 @@ public void OnPeerConnected(NetPeer peer)
conn = new LiteNetConnection(peer);
conn.username = "test1";
+ // The registry is process-wide and additive, so a registration from an earlier test has to
+ // be removed before this one can take its place.
+ MpConnectionState.ClearImplementation(ConnectionStateEnum.ClientJoining);
MpConnectionState.SetImplementation(ConnectionStateEnum.ClientJoining, joiningStateType);
conn.ChangeState(ConnectionStateEnum.ClientJoining);
diff --git a/Source/Tests/ServerTest.cs b/Source/Tests/ServerTest.cs
index 4cfff8bd5..a746bf4a4 100644
--- a/Source/Tests/ServerTest.cs
+++ b/Source/Tests/ServerTest.cs
@@ -31,17 +31,10 @@ public void Test()
var server = MakeServer(out var port);
ConnectClient(port, typeof(TestJoiningState));
- var timeoutWatch = Stopwatch.StartNew();
- while (true)
- {
- if (server.InitDataState == InitDataState.Complete && server.playerManager.Players.Count == 0)
- break; // Success
-
- if (timeoutWatch.ElapsedMilliseconds > 2000)
- Assert.Fail("Timeout");
-
- Thread.Sleep(50);
- }
+ WaitUntil(
+ nameof(Test),
+ () => server.InitDataState == InitDataState.Complete && server.playerManager.Players.Count == 0,
+ () => $"InitDataState={server.InitDataState} Players={server.playerManager.Players.Count}");
}
[Test]
@@ -83,17 +76,22 @@ public void LoadingStateHandlesKeepAliveWhileWaitingForJoinPoint()
ConnectClient(port, typeof(TestLoadingKeepAliveState));
- var timeoutWatch = Stopwatch.StartNew();
- while (true)
- {
- if (server.playerManager.Players.Count == 0)
- break;
-
- if (timeoutWatch.ElapsedMilliseconds > 2000)
- Assert.Fail("Timeout");
-
- Thread.Sleep(50);
- }
+ // Players.Count == 0 already holds before the client connects, so waiting on it alone proves
+ // nothing. Observe the client arrive and then leave: both conditions start out false.
+ WaitUntil(
+ $"{nameof(LoadingStateHandlesKeepAliveWhileWaitingForJoinPoint)}.joined",
+ () => server.playerManager.Players.Count == 1,
+ () => $"Players={server.playerManager.Players.Count} " +
+ $"CreatingJoinPoint={server.worldData.CreatingJoinPoint}");
+
+ WaitUntil(
+ $"{nameof(LoadingStateHandlesKeepAliveWhileWaitingForJoinPoint)}.left",
+ () => server.playerManager.Players.Count == 0,
+ () => $"Players={server.playerManager.Players.Count} " +
+ $"CreatingJoinPoint={server.worldData.CreatingJoinPoint}");
+
+ // The loading state was blocked on this join point, so reaching here means it completed.
+ Assert.That(server.worldData.CreatingJoinPoint, Is.False);
}
[Test]
@@ -110,21 +108,69 @@ public void StandaloneJoinWithExistingPlayer_DoesNotStartJoinPoint()
ConnectClient(port, typeof(TestJoiningState));
- var timeoutWatch = Stopwatch.StartNew();
+ // The seeded player above already satisfies Players.Count == 1, so waiting on that alone
+ // would pass without the client ever joining. Wait for it to arrive, then to leave.
+ WaitUntil(
+ $"{nameof(StandaloneJoinWithExistingPlayer_DoesNotStartJoinPoint)}.joined",
+ () => server.playerManager.Players.Count == 2,
+ () => $"Players={server.playerManager.Players.Count} " +
+ $"CreatingJoinPoint={server.worldData.CreatingJoinPoint}");
+
+ WaitUntil(
+ $"{nameof(StandaloneJoinWithExistingPlayer_DoesNotStartJoinPoint)}.left",
+ () => server.playerManager.Players.Count == 1,
+ () => $"Players={server.playerManager.Players.Count} " +
+ $"CreatingJoinPoint={server.worldData.CreatingJoinPoint}");
+
+ Assert.That(server.worldData.CreatingJoinPoint, Is.False);
+ }
+
+ ///
+ /// Polls until holds, or fails with what was actually observed.
+ ///
+ /// These tests drive a real server on a real socket, so how long the condition takes depends on the
+ /// machine. "Timeout" on its own says only that something did not happen; it does not say which half
+ /// of a compound condition was still false, nor how close to the budget the run came. Both are needed
+ /// to choose a defensible budget rather than a superstitious one.
+ ///
+ /// Every wait emits one measurement line, on success as well as failure, so a repeated run yields the
+ /// distribution instead of a single anecdote. Written through TestContext.Progress because NUnit only
+ /// surfaces captured Console output for tests that fail, and the successful runs are the interesting
+ /// ones here.
+ ///
+ private static void WaitUntil(string label, Func condition, Func describeState,
+ int timeoutMs = 5000)
+ {
+ var watch = Stopwatch.StartNew();
+ var polls = 0;
+
while (true)
{
- if (server.playerManager.Players.Count == 1)
- break;
+ polls++;
+
+ if (condition())
+ {
+ Report(label, "ok", watch.ElapsedMilliseconds, polls, describeState());
+ return;
+ }
- if (timeoutWatch.ElapsedMilliseconds > 2000)
- Assert.Fail("Timeout");
+ if (watch.ElapsedMilliseconds > timeoutMs)
+ {
+ var state = describeState();
+ Report(label, "timeout", watch.ElapsedMilliseconds, polls, state);
+ Assert.Fail($"{label} timed out after {watch.ElapsedMilliseconds} ms " +
+ $"({polls} polls, budget {timeoutMs} ms); observed {state}");
+ }
Thread.Sleep(50);
}
-
- Assert.That(server.worldData.CreatingJoinPoint, Is.False);
}
+ /// One greppable line per wait. Prefixed so a harness can pick it out of ordinary test output.
+ private static void Report(string label, string outcome, long elapsedMs, int polls, string state) =>
+ TestContext.Progress.WriteLine(
+ $"##WAIT## label={label} outcome={outcome} elapsed_ms={elapsedMs} polls={polls} state=[{state}]");
+
private void ConnectClient(int port, Type joiningStateType)
{
var clientListener = new TestNetListener(joiningStateType);