From 237943c047adf990f9aaa9dcc781e050f133db3e Mon Sep 17 00:00:00 2001 From: roman_gr Date: Fri, 26 Jun 2026 23:30:50 +0200 Subject: [PATCH 1/6] #843: clear pending Steam join requests on reject so reconnects re-prompt The P2P session request handler only removed a Steam ID from session.pendingSteam on accept. A rejected (or disconnected-before-accept) player left a stale entry, and the guard then skipped the handler on every reconnect, leaving the host without a prompt and the player stuck "waiting for host to accept". Clear the entry on reject and scope the dedup guard to the prompt enqueue so pendingSteam membership tracks exactly an unanswered prompt. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_011htpmJM4qmJhthgQ9djDWT --- Source/Client/Networking/SteamIntegration.cs | 42 +++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/Source/Client/Networking/SteamIntegration.cs b/Source/Client/Networking/SteamIntegration.cs index 8bfa3f7cc..70d88cbef 100644 --- a/Source/Client/Networking/SteamIntegration.cs +++ b/Source/Client/Networking/SteamIntegration.cs @@ -34,23 +34,37 @@ public static void InitCallbacks() { ServerLog.Log($"Received P2P session request from {req.m_steamIDRemote}"); var session = Multiplayer.session; - if (Multiplayer.LocalServer?.settings.steam == true && !session.pendingSteam.Contains(req.m_steamIDRemote)) + if (Multiplayer.LocalServer?.settings.steam != true) + return; + + var remoteId = req.m_steamIDRemote; + + if (Multiplayer.settings.autoAcceptSteam) + { + SteamNetworking.AcceptP2PSessionWithUser(remoteId); + } + // pendingSteam doubles as the dedup set: an entry exists exactly while a prompt is + // unanswered (both accept and reject clear it), so this skips duplicate prompts + // without a stale entry ever blocking a reconnect permanently (#843). + else if (!session.pendingSteam.Contains(remoteId)) { - if (Multiplayer.settings.autoAcceptSteam) - SteamNetworking.AcceptP2PSessionWithUser(req.m_steamIDRemote); - else + session.pendingSteam.Add(remoteId); + PendingPlayerWindow.EnqueueJoinRequest(remoteId, (joinReq, accepted) => { - session.pendingSteam.Add(req.m_steamIDRemote); - PendingPlayerWindow.EnqueueJoinRequest(req.m_steamIDRemote, (joinReq, accepted) => - { - if(joinReq.steamId.HasValue && accepted) AcceptPlayerJoinRequest(joinReq.steamId.Value); - }); - } - session.knownUsers.Add(req.m_steamIDRemote); - session.NotifyChat(); - - SteamFriends.RequestUserInformation(req.m_steamIDRemote, true); + if (!joinReq.steamId.HasValue) return; + if (accepted) + AcceptPlayerJoinRequest(joinReq.steamId.Value); + else + // Clean up so the player isn't blocked from prompting again on reconnect. + session.pendingSteam.Remove(joinReq.steamId.Value); + }); } + + if (!session.knownUsers.Contains(remoteId)) + session.knownUsers.Add(remoteId); + session.NotifyChat(); + + SteamFriends.RequestUserInformation(remoteId, true); }); friendRchpUpdate = Callback.Create(update => From 67906088fa4098e9c1d3da51e9c487fe3068fa72 Mon Sep 17 00:00:00 2001 From: roman_gr Date: Fri, 26 Jun 2026 23:59:14 +0200 Subject: [PATCH 2/6] #843: close Steam P2P session on disconnect so reconnects re-prompt SteamBaseConn.OnClose previously left the Steam P2P session open (TODO), so after a client left, the host kept a half-open session with their Steam ID. On reconnect Steam reused that session instead of posting a fresh P2PSessionRequest_t, so the host never got a new accept prompt and the client sat on "waiting for host to accept". Free the session via CloseP2PSessionWithUser in OnClose, and also in SteamServerConn's P2P timeout/error path, which tears the connection down through SetDisconnected without going through OnClose. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_011htpmJM4qmJhthgQ9djDWT --- Source/Client/Networking/NetworkingSteam.cs | 22 ++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/Source/Client/Networking/NetworkingSteam.cs b/Source/Client/Networking/NetworkingSteam.cs index 3f9534a61..4d5edb3c3 100644 --- a/Source/Client/Networking/NetworkingSteam.cs +++ b/Source/Client/Networking/NetworkingSteam.cs @@ -45,9 +45,21 @@ public void SendRawSteam(byte[] raw, bool reliable) protected override void OnClose(ServerDisconnectPacket? goodbye) { if (goodbye.HasValue) Send(goodbye.Value); - // TODO this should probably include SteamNetworking.CloseP2PSessionWithUser to free up any leftover - // resources in the Steam API. The API docs are not clear whether the connection is closed instantly, or - // are the queued packets sent. + CloseSteamSession(); + } + + // Frees the underlying Steam P2P session. This is required so that a later reconnect from + // the same user produces a fresh P2PSessionRequest_t (and, on the host, a new accept prompt) + // instead of Steam silently reusing the still-open session, which left the peer stuck and the + // host without a prompt (#843). + // + // Note: a goodbye queued just before this (e.g. a kick reason) is best-effort. SendP2PPacket + // only queues, so closing here may drop it before Steam flushes; the peer then falls back to a + // timeout/generic reason. Freeing the session is worth that trade-off. + protected void CloseSteamSession() + { + ServerLog.Log($"Closing Steam P2P session with {remoteId}"); + SteamNetworking.CloseP2PSessionWithUser(remoteId); } public override string ToString() => $"SteamP2P ({remoteId}:{username})"; @@ -108,6 +120,10 @@ public override void OnKeepAliveArrived(bool idMatched) private void OnDisconnect() { + // The P2P timeout/error path does not go through OnClose, so close the Steam session here + // too. Otherwise the host keeps a half-open session with the departed client and their + // reconnect reuses it without firing a new accept prompt (#843). + CloseSteamSession(); serverPlayer.Server.playerManager.SetDisconnected(this, MpDisconnectReason.ClientLeft); } } From a29d5c9862c2ad147c19a6bcb9bf6b43660395b4 Mon Sep 17 00:00:00 2001 From: roman_gr Date: Mon, 29 Jun 2026 01:07:21 +0200 Subject: [PATCH 3/6] #843: accept Steam reconnect by replacing stale connection in Tick When a joiner reconnects before the host has torn down their previous connection (a fast rejoin, before the Steam P2P timeout fires), their new join packet arrives on the still-open session and previously hit the "shouldn't happen" branch in SteamP2PNetManager.Tick, where it was dropped. The client never resends its join packet, so the joiner stayed stuck on "waiting for host to accept". Treat a join packet from an already-known remote as a reconnect: drop the stale player via SetDisconnected and fall through to the normal accept path. SetDisconnected (not Close) is used deliberately so OnClose -> CloseSteamSession doesn't tear down the shared P2P session the new join just arrived on. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_011htpmJM4qmJhthgQ9djDWT --- Source/Client/Networking/NetworkingSteam.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Source/Client/Networking/NetworkingSteam.cs b/Source/Client/Networking/NetworkingSteam.cs index 4d5edb3c3..a85ef419c 100644 --- a/Source/Client/Networking/NetworkingSteam.cs +++ b/Source/Client/Networking/NetworkingSteam.cs @@ -148,6 +148,22 @@ public void Tick() var player = playerManager.Players .FirstOrDefault(p => p.conn is SteamBaseConn conn && conn.remoteId == packet.remote); + // A join packet from a remote we still consider connected means their previous + // session died and they are reconnecting on a fresh one (e.g. a quick rejoin before + // the old connection timed out). Drop the stale player so the join is accepted below + // instead of being discarded, which would otherwise leave them stuck on "waiting for + // host to accept" (#843). + // + // Use SetDisconnected, not Close/Disconnect: the new join packet just arrived on this + // same Steam P2P session, and Close -> OnClose -> CloseSteamSession would tear that + // shared session down and break the very connection we're about to accept. + if (packet.joinPacket && player != null) + { + ServerLog.Log($"Reconnect from {packet.remote}; replacing stale connection {player.conn}"); + playerManager.SetDisconnected(player.conn, MpDisconnectReason.ClientLeft); + player = null; + } + if (packet.joinPacket && player == null) { ConnectionBase conn = new SteamServerConn(packet.remote, packet.channel); From 0ad8e1a07b8a310ba5321bb81ebb5449369b68c5 Mon Sep 17 00:00:00 2001 From: roman_gr Date: Mon, 13 Jul 2026 15:42:34 +0200 Subject: [PATCH 4/6] #843: delay Steam P2P session close so the goodbye packet can flush Closing the session in OnClose right after queueing the goodbye dropped it: SendP2PPacket only queues, and CloseP2PSessionWithUser discards queued unsent packets. Clients never received server-initiated disconnect reasons over Steam (wrong password, kick, username taken) and fell back to a generic timeout. Keep the immediate close for the no-goodbye paths, but when a goodbye was sent, defer CloseP2PSessionWithUser by 3 seconds via Task.Delay + server.Enqueue so it runs on the server thread after Steam has flushed the packet. Skip the deferred close if a connection with the same remoteId exists in playerManager.Players by then: the old player was already removed, so any match is a fast rejoin riding the same underlying session, and closing would tear it down. Co-Authored-By: Claude Fable 5 --- Source/Client/Networking/NetworkingSteam.cs | 38 ++++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/Source/Client/Networking/NetworkingSteam.cs b/Source/Client/Networking/NetworkingSteam.cs index a85ef419c..d32253f27 100644 --- a/Source/Client/Networking/NetworkingSteam.cs +++ b/Source/Client/Networking/NetworkingSteam.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Threading.Tasks; using Multiplayer.Common; using Multiplayer.Common.Networking.Packet; using Steamworks; @@ -17,6 +18,9 @@ public abstract class SteamBaseConn(CSteamID remoteId, ushort recvChannel, ushor public readonly ushort recvChannel = recvChannel; // currently only for client public readonly ushort sendChannel = sendChannel; // currently only for server + // Time given to Steam to flush a queued goodbye packet before the P2P session is freed (#843). + private static readonly TimeSpan SteamGoodbyeFlushDelay = TimeSpan.FromSeconds(3); + protected override void SendRaw(byte[] raw, bool reliable = true) { byte[] full = new byte[1 + raw.Length]; @@ -42,10 +46,35 @@ public void SendRawSteam(byte[] raw, bool reliable) public abstract void OnError(EP2PSessionError error); + // A goodbye is only ever non-null server-side, and CloseP2PSessionWithUser discards queued unsent + // packets. Closing immediately would drop the just-queued goodbye (e.g. a wrong-password or kick + // reason) before Steam flushes it, so defer the close to let the reliable packet reach the client. protected override void OnClose(ServerDisconnectPacket? goodbye) { - if (goodbye.HasValue) Send(goodbye.Value); - CloseSteamSession(); + if (!goodbye.HasValue) + { + CloseSteamSession(); + return; + } + + Send(goodbye.Value); + + var server = serverPlayer.Server; + var id = remoteId; + + Task.Delay(SteamGoodbyeFlushDelay).ContinueWith(_ => server.Enqueue(() => + { + // A fast reconnect (SteamP2PNetManager.Tick) reuses this same remoteId on a fresh + // connection during the delay window. Closing then would tear that new session down, so + // skip the close if another connection already replaced this one (#843). + if (server.playerManager.Players.Any(p => p.conn is SteamBaseConn c && c != this && c.remoteId == id)) + { + ServerLog.Log($"Skipping delayed Steam P2P session close with {id}; a reconnect replaced it"); + return; + } + + CloseSteamSession(); + })); } // Frees the underlying Steam P2P session. This is required so that a later reconnect from @@ -53,9 +82,8 @@ protected override void OnClose(ServerDisconnectPacket? goodbye) // instead of Steam silently reusing the still-open session, which left the peer stuck and the // host without a prompt (#843). // - // Note: a goodbye queued just before this (e.g. a kick reason) is best-effort. SendP2PPacket - // only queues, so closing here may drop it before Steam flushes; the peer then falls back to a - // timeout/generic reason. Freeing the session is worth that trade-off. + // Called immediately when no goodbye is queued; server-initiated disconnects that queue a goodbye + // defer it (see OnClose) so SendP2PPacket can flush the reason before the session is torn down. protected void CloseSteamSession() { ServerLog.Log($"Closing Steam P2P session with {remoteId}"); From f1e959dc129d103b46ff0a55ed1cba8c2b4e517a Mon Sep 17 00:00:00 2001 From: roman_gr Date: Sat, 1 Aug 2026 19:02:41 +0200 Subject: [PATCH 5/6] #843: share stale connection replacement through PlayerManager The Steam reconnect fix lived entirely in SteamP2PNetManager.Tick, which left no way to apply the same rule to other transports. Review on #958 asked for it to be uniform. Add ConnectionBase.RemoteIdentity, identifying the remote endpoint, and CloseReplacedBy, which closes a connection being superseded. Steam overrides it to close nothing when the replacement shares its P2P session, so the knowledge that a replacement can ride the stale connection's own session stays in SteamBaseConn rather than being asked about from outside. PlayerManager.ReplaceStale then just tells the stale connection to close and forgets the player. Tick now delegates to ReplaceStale. Creating the connection before the stale player is dropped also removes the joinPacket/player == null guard, which no longer had to encode "a reconnect looks like a new connection". Co-Authored-By: Claude Opus 5 --- Source/Client/Networking/NetworkingSteam.cs | 40 ++++++++++----------- Source/Common/Networking/ConnectionBase.cs | 4 +++ Source/Common/PlayerManager.cs | 10 ++++++ 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/Source/Client/Networking/NetworkingSteam.cs b/Source/Client/Networking/NetworkingSteam.cs index d32253f27..1bcf986c0 100644 --- a/Source/Client/Networking/NetworkingSteam.cs +++ b/Source/Client/Networking/NetworkingSteam.cs @@ -21,6 +21,16 @@ public abstract class SteamBaseConn(CSteamID remoteId, ushort recvChannel, ushor // Time given to Steam to flush a queued goodbye packet before the P2P session is freed (#843). private static readonly TimeSpan SteamGoodbyeFlushDelay = TimeSpan.FromSeconds(3); + public override object? RemoteIdentity => remoteId; + + // Steam carries every connection with a peer over one P2P session keyed by their id, so a + // replacement from the same id arrived on this very session and closing would take it down too. + public override void CloseReplacedBy(ConnectionBase replacement, MpDisconnectReason reason) + { + if (replacement is SteamBaseConn conn && conn.remoteId == remoteId) return; + base.CloseReplacedBy(replacement, reason); + } + protected override void SendRaw(byte[] raw, bool reliable = true) { byte[] full = new byte[1 + raw.Length]; @@ -176,26 +186,17 @@ public void Tick() var player = playerManager.Players .FirstOrDefault(p => p.conn is SteamBaseConn conn && conn.remoteId == packet.remote); - // A join packet from a remote we still consider connected means their previous - // session died and they are reconnecting on a fresh one (e.g. a quick rejoin before - // the old connection timed out). Drop the stale player so the join is accepted below - // instead of being discarded, which would otherwise leave them stuck on "waiting for - // host to accept" (#843). - // - // Use SetDisconnected, not Close/Disconnect: the new join packet just arrived on this - // same Steam P2P session, and Close -> OnClose -> CloseSteamSession would tear that - // shared session down and break the very connection we're about to accept. - if (packet.joinPacket && player != null) - { - ServerLog.Log($"Reconnect from {packet.remote}; replacing stale connection {player.conn}"); - playerManager.SetDisconnected(player.conn, MpDisconnectReason.ClientLeft); - player = null; - } - - if (packet.joinPacket && player == null) + if (packet.joinPacket) { ConnectionBase conn = new SteamServerConn(packet.remote, packet.channel); + // A join packet from a remote we still consider connected means their previous + // session died and they are reconnecting on a fresh one (e.g. a quick rejoin before + // the old connection timed out). Without replacing the stale player the join would be + // discarded, leaving them stuck on "waiting for host to accept" (#843). + if (player != null) + playerManager.ReplaceStale(player, conn); + var preConnect = playerManager.OnPreConnect(packet.remote); if (preConnect != null) { @@ -215,14 +216,13 @@ public void Tick() conn.Send(Packets.Server_SteamAccept); } - else if (!packet.joinPacket && player != null) + else if (player != null) { player.HandleReceive(packet.data, packet.reliable); } else { - ServerLog.Error( - $"Received a join packet: {packet.joinPacket} for player: {player} (player should only be null when joinPacket is true)"); + ServerLog.Error($"Received a data packet from {packet.remote}, who has no connection"); } } } diff --git a/Source/Common/Networking/ConnectionBase.cs b/Source/Common/Networking/ConnectionBase.cs index 14ab1b872..b5864d567 100644 --- a/Source/Common/Networking/ConnectionBase.cs +++ b/Source/Common/Networking/ConnectionBase.cs @@ -12,6 +12,8 @@ public abstract class ConnectionBase public virtual int Latency { get; set; } + public virtual object? RemoteIdentity => null; + public ConnectionStateEnum State { get; private set; } public MpConnectionState? StateObj { get; private set; } // If lenient is set, reliable packets without handlers are ignored instead of throwing an exception. @@ -269,6 +271,8 @@ public void Close(MpDisconnectReason reason, byte[]? data = null) OnClose(null); } + public virtual void CloseReplacedBy(ConnectionBase replacement, MpDisconnectReason reason) => Close(reason); + protected abstract void OnClose(ServerDisconnectPacket? goodbye); /// Invoked after a keep alive timer arrives. Only used by the server diff --git a/Source/Common/PlayerManager.cs b/Source/Common/PlayerManager.cs index efa871704..fb8d69fc3 100644 --- a/Source/Common/PlayerManager.cs +++ b/Source/Common/PlayerManager.cs @@ -62,6 +62,16 @@ public ServerPlayer OnConnected(ConnectionBase conn) return conn.serverPlayer; } + // Drops a connection that a newly arrived one has superseded, so the arrival can take its place + // instead of being turned away (#843). + public void ReplaceStale(ServerPlayer stale, ConnectionBase replacement) + { + ServerLog.Log($"Replacing stale connection {stale.conn} with {replacement}"); + + stale.conn.CloseReplacedBy(replacement, MpDisconnectReason.ClientLeft); + SetDisconnected(stale.conn, MpDisconnectReason.ClientLeft); + } + public void SetDisconnected(ConnectionBase conn, MpDisconnectReason reason) { if (conn.State == ConnectionStateEnum.Disconnected) return; From b337db8f549039f0d305ac199db3228bb950be67 Mon Sep 17 00:00:00 2001 From: roman_gr Date: Sat, 1 Aug 2026 19:02:55 +0200 Subject: [PATCH 6/6] #843: let a direct client reclaim its username from the same address A player reconnecting before the server reaped their old peer was turned away with UsernameAlreadyOnline until it timed out. Replacing on a username match alone would let anyone holding your name evict you from a passwordless server, so match on two factors instead: the username says who, and the remote address corroborates the origin. LiteNetConnection reports peer.Address as its RemoteIdentity. A match replaces the stale connection; anything else falls through to the existing rejection, so no case gets worse than before. The address is corroborating evidence only, never an identity on its own: players behind one NAT share it, and a player whose address changed won't match. Steam needs no second factor because a CSteamID is unique per user. Tests cover both directions, standing in for the stale connection with a RecordingConnection at a chosen address. Co-Authored-By: Claude Opus 5 --- Source/Common/Networking/LiteNetConnection.cs | 5 ++ .../Networking/State/ServerJoiningState.cs | 15 ++++-- Source/Tests/Helper/RecordingConnection.cs | 4 ++ Source/Tests/Helper/TestUsernameOnlyState.cs | 27 ++++++++++ Source/Tests/ServerTest.cs | 51 +++++++++++++++++++ 5 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 Source/Tests/Helper/TestUsernameOnlyState.cs diff --git a/Source/Common/Networking/LiteNetConnection.cs b/Source/Common/Networking/LiteNetConnection.cs index b3a8c5d05..323f72cf2 100644 --- a/Source/Common/Networking/LiteNetConnection.cs +++ b/Source/Common/Networking/LiteNetConnection.cs @@ -7,6 +7,11 @@ public class LiteNetConnection(NetPeer peer) : ConnectionBase { public readonly NetPeer peer = peer; + // Only a hint at who the remote is: players behind one NAT share an address, so a housemate + // can match; and a player whose address changed (mobile, VPN, or v4 vs v6 across the two + // NetManagers) won't match their own earlier connection. + public override object? RemoteIdentity => peer.Address; + protected override void SendRaw(byte[] raw, bool reliable) { if (peer.ConnectionState == ConnectionState.Connected) diff --git a/Source/Common/Networking/State/ServerJoiningState.cs b/Source/Common/Networking/State/ServerJoiningState.cs index b2d2a392f..8d3b88e63 100644 --- a/Source/Common/Networking/State/ServerJoiningState.cs +++ b/Source/Common/Networking/State/ServerJoiningState.cs @@ -87,10 +87,19 @@ private void HandleUsername(ClientUsernamePacket packet) return; } - if (Server.GetPlayer(username) != null) + var existing = Server.GetPlayer(username); + if (existing != null) { - Player.Disconnect(MpDisconnectReason.UsernameAlreadyOnline); - return; + // Coming from the same remote as the player already holding this username means it's them + // reconnecting before their old connection was reaped, so give them their name back. Anyone + // else claiming it is turned away as before. + if (existing.conn.RemoteIdentity?.Equals(connection.RemoteIdentity) == true) + Server.playerManager.ReplaceStale(existing, connection); + else + { + Player.Disconnect(MpDisconnectReason.UsernameAlreadyOnline); + return; + } } connection.username = username; diff --git a/Source/Tests/Helper/RecordingConnection.cs b/Source/Tests/Helper/RecordingConnection.cs index e219992a1..8708e39a2 100644 --- a/Source/Tests/Helper/RecordingConnection.cs +++ b/Source/Tests/Helper/RecordingConnection.cs @@ -15,6 +15,10 @@ public RecordingConnection(string username) public override int Latency { get => 0; set { } } + public object? remoteIdentity; + + public override object? RemoteIdentity => remoteIdentity; + protected override void SendRaw(byte[] raw, bool reliable) { if (raw.Length == 0) diff --git a/Source/Tests/Helper/TestUsernameOnlyState.cs b/Source/Tests/Helper/TestUsernameOnlyState.cs new file mode 100644 index 000000000..ca1bde85f --- /dev/null +++ b/Source/Tests/Helper/TestUsernameOnlyState.cs @@ -0,0 +1,27 @@ +using Multiplayer.Common; +using Multiplayer.Common.Networking.Packet; + +namespace Tests; + +// Gets as far as claiming a username and then stays put, so tests have a connection for a later +// client to collide with. +public class TestUsernameOnlyState : AsyncConnectionState +{ + public TestUsernameOnlyState(ConnectionBase connection) : base(connection) + { + } + + // Deliberately registers no packet handlers: handlers accumulate globally per connection state, so + // declaring one here would clash with the state a second client in the same test installs. + protected override async Task RunState() + { + connection.Send(ClientProtocolPacket.Current()); + await TypedPacket(); + + connection.Send(new ClientUsernamePacket(connection.username!)); + await TypedPacket(); + + // Left unanswered, which parks the connection here still holding the username. + await TypedPacket(); + } +} diff --git a/Source/Tests/ServerTest.cs b/Source/Tests/ServerTest.cs index 4cfff8bd5..bc1252e61 100644 --- a/Source/Tests/ServerTest.cs +++ b/Source/Tests/ServerTest.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using System.Net; using LiteNetLib; using Multiplayer.Common; @@ -125,6 +126,56 @@ public void StandaloneJoinWithExistingPlayer_DoesNotStartJoinPoint() Assert.That(server.worldData.CreatingJoinPoint, Is.False); } + [Test] + public void JoinFromSameAddress_ReplacesPlayerHoldingTheUsername() + { + var server = MakeServer(out var port); + var stalePlayer = AddPlayerHoldingTestUsername(server, IPAddress.Loopback); + + ConnectClient(port, typeof(TestUsernameOnlyState)); + + WaitUntil(() => !server.playerManager.Players.Contains(stalePlayer), + "the connection at the same address was not replaced"); + Assert.That(server.playerManager.GetPlayer("test1"), Is.Not.Null); + } + + [Test] + public void JoinFromDifferentAddress_KeepsPlayerHoldingTheUsername() + { + var server = MakeServer(out var port); + var existingPlayer = AddPlayerHoldingTestUsername(server, IPAddress.Parse("10.0.0.1")); + + ConnectClient(port, typeof(TestUsernameOnlyState)); + + // Nothing should displace them, so give the join time to go wrong before checking. + Thread.Sleep(500); + + Assert.That(server.playerManager.Players.Contains(existingPlayer), Is.True); + } + + // Stands in for a player whose client is gone but whose connection the server still holds. + private static ServerPlayer AddPlayerHoldingTestUsername(MultiplayerServer server, IPAddress address) + { + var conn = new RecordingConnection("test1") { remoteIdentity = address }; + conn.ChangeState(ConnectionStateEnum.ServerPlaying); + var player = new ServerPlayer(100, conn); + conn.serverPlayer = player; + server.playerManager.Players.Add(player); + return player; + } + + private static void WaitUntil(Func condition, string message) + { + var timeoutWatch = Stopwatch.StartNew(); + while (!condition()) + { + if (timeoutWatch.ElapsedMilliseconds > 2000) + Assert.Fail($"Timeout: {message}"); + + Thread.Sleep(50); + } + } + private void ConnectClient(int port, Type joiningStateType) { var clientListener = new TestNetListener(joiningStateType);