Host cannot accept Steam player after reconnect (e.g. Fix & Restart)

[romangr]

Fix Steam P2P fast rejoin

Fixing #843

Fast rejoin dropped

When a joiner reconnects before the host has torn down their previous connection (before the Steam P2P timeout fires), the new join packet arrives on the still-open session and 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".
Now a join packet from an already-known remote is treated as a reconnect: the stale player is dropped via SetDisconnected and we fall through to the normal accept path. SetDisconnected is used instead of Close so OnClose → CloseSteamSession doesn't tear down the shared P2P session the new join just arrived on.

Disconnect reasons delivered

Closing the session in OnClose immediately after queueing the goodbye dropped it: SendP2PPacket only queues, and CloseP2PSessionWithUser discards queued unsent packets. Clients never received server-initiated disconnect reasons (wrong password, kick, username taken) and fell back to a generic timeout.
The immediate close is kept for the no-goodbye paths. When a goodbye was sent, CloseP2PSessionWithUser is deferred by 3 seconds via Task.Delay + server.Enqueue so it runs on the server thread after Steam has flushed the packet. The deferred close is skipped 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.

Testing

• Checked that invalid password notification is shown
• Tested that the client successfully connects after fixing the settings diffs

[romangr]

@notfood please take a look

[notfood]

I discussed it with Mibac, a more robust approach is to close any stale connection upon the arrival of a new client sharing the same identification clues. This logic can be applied uniformly, including to non‑Steam clients.

[romangr]

Thanks — happy to go that way. Before I write it, one trade-off I'd rather have your call on, since the "identification clues" differ in how much we can trust them.

Steam gives us a CSteamID at transport level, so "same ID replaces the stale connection" is safe unconditionally. Direct connections only have the username, checked mid-handshake, and it's freely chosen. That leaves two options:

• Replace unconditionally. Simplest, and uniform in the strict sense — but on a passwordless server anyone who knows your name can boot you off, repeatedly.
• Gate it on the old connection being dead. No griefing vector, but it needs liveness tracking we don't have today (ServerPlayer.keepAliveAt is stamped on every keepalive and currently never read, so the signal exists — it's a staleness threshold or a short active probe on collision).

Worth weighing against how much it buys for direct clients: a reconnecting direct client gets its own NetPeer, so it's never confused with the stale one and just proceeds — the only symptom is UsernameAlreadyOnline until the old peer is reaped. Steam is worse off because both connections are keyed on the same remoteId: at SteamP2PNetManager.Tick the old and new are indistinguishable, so the reconnect's join packet hits the else branch and is logged as an error rather than handled, and the client is stuck. Both transports race their own timeout; only Steam loses the new connection to that race.

Either way I'd extract the policy into one PlayerManager helper called from both places, so there's a single place to reason about it.

Separately, one thing I found that keeps the Steam-side code needed regardless: CloseP2PSessionWithUser is load-bearing for the accept prompt. P2PSessionRequest_t only fires when no session exists, and nothing in Tick consults pendingSteam. On master today a kicked Steam player keeps an open session and rejoins with no prompt, gated only by the 1s throttle in OnPreConnect. (The 3s delay before closing is only because CloseP2PSessionWithUser discards queued packets and would eat the goodbye — open to something cleaner.)

Which way do you and Mibac want the username case?

[notfood]

Direct connections should allow for extra validation checks, such as verifying the originating IP address, information we already have and can potentially test against.

Steam's reuse of the connection is acceptable, as the identifier remains unique throughout the process.

[romangr]

Went with the IP check you suggested.

A direct join now replaces the connection holding the username only when it also comes from the same address.

Both paths go through a single PlayerManager.ReplaceStale. Tests cover a direct join from the same address replacing the stale connection, and one from a different address being turned away.

[rautamiekka]

CloseP2PSessionWithUser discards queued unsent

Since this seems to be a Steam thing, should let them know about this, otherwise nothing gets fixed and workarounds keep getting needed.