Skip to content

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

Open
romangr wants to merge 4 commits into
rwmt:devfrom
romangr:issue-843-steam-reconnect-replace
Open

Host cannot accept Steam player after reconnect (e.g. Fix & Restart)#958
romangr wants to merge 4 commits into
rwmt:devfrom
romangr:issue-843-steam-reconnect-replace

Conversation

@romangr

@romangr romangr commented Jul 14, 2026

Copy link
Copy Markdown

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 and others added 4 commits June 27, 2026 00:03
…e-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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011htpmJM4qmJhthgQ9djDWT
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011htpmJM4qmJhthgQ9djDWT
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011htpmJM4qmJhthgQ9djDWT
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 <noreply@anthropic.com>
@romangr

romangr commented Jul 14, 2026

Copy link
Copy Markdown
Author

@notfood please take a look

@notfood

notfood commented Jul 27, 2026

Copy link
Copy Markdown
Member

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

romangr commented Jul 27, 2026

Copy link
Copy Markdown
Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants