Add per-player logging and fix connection loss detection#7
Merged
chrisuthe merged 3 commits intochrisuthe:masterfrom Jan 29, 2026
Merged
Conversation
- Add [PlayerName] prefix to all server message log lines (group/update, server/state, server/command) for multi-player diagnostics - Log group ID and name changes in HandleGroupUpdate to help diagnose grouping issues - Simplify SendPlayerStateAckAsync: remove redundant try/catch wrapper (exceptions already handled by SafeFireAndForget) - Clean up comments to match actual protocol behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
Author
|
these seemed like useful things to keep and have in the SDK? |
Trigger HandleConnectionLostAsync from failed sends when WebSocket is closed but receive loop hasn't detected it yet. This fixes players getting stuck in Connected state after server disappears. Also: extract handshake into reusable SendHandshakeAsync, add re-handshake on reconnection, stop time sync on Reconnecting state, and fix ConnectInternalAsync to not transition through Disconnected during AutoReconnect retries. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
7 tasks
This commit addresses critical thread-safety issues identified during code review that could cause duplicate reconnection attempts and unpredictable behavior, especially in multi-player scenarios like multiSendSpin. Race condition fixes: - Remove _isReconnecting field and use e.OldState instead The _isReconnecting field was accessed without synchronization from multiple threads (receive loop, main thread, reconnect loop). Now we check e.OldState == ConnectionState.Reconnecting directly in the event args, which is immutable and thread-safe. - Add Interlocked guard to HandleConnectionLostAsync Both send failure detection and the receive loop could trigger HandleConnectionLostAsync simultaneously when a connection dies. The atomic guard ensures only the first caller proceeds, preventing duplicate reconnection attempts and state machine corruption. Additional improvements: - Add CancellationToken support to PerformReconnectHandshakeAsync Allows graceful cancellation of reconnection handshakes during shutdown. - Remove duplicate _clockSynchronizer.Reset() call HandleServerHello already resets the clock synchronizer when handshake completes, making the reset in PerformReconnectHandshakeAsync redundant. - Revert routine group/update log level to Debug Keep routine state logs at Debug level per CLAUDE.md guidelines. Only actual state changes (group ID/name changes) remain at Information. - Use SafeFireAndForget for TimeSyncLoopAsync Ensures any exceptions from the time sync loop are properly logged instead of being silently discarded with a bare discard assignment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Per-player logging & ACK cleanup
[PlayerName]prefix to all server message log lines (group/update,server/state,server/command) for multi-player diagnosticsHandleGroupUpdateto help diagnose grouping issuesSendPlayerStateAckAsync: remove redundant try/catch (already handled bySafeFireAndForget)Connection loss detection & reconnection improvements
HandleConnectionLostAsyncfrom failed sends — WhenSendMessageAsync/SendBinaryAsyncdetectsWebSocket.State != Openwhile the connection state is stillConnectedorHandshaking, fire-and-forgetHandleConnectionLostAsync(). Fixes players stuck in "Connected" state after server disappears (the receive loop blocks forever onReceiveAsyncwhile TCP hasn't timed out).SendHandshakeAsync()andCreateClientHelloMessage()extracted fromConnectAsync()for reuse in reconnection handshakes.PerformReconnectHandshakeAsync()sends a freshClientHellowhen the WebSocket reconnects (enteringHandshakingstate while_isReconnectingis true). Resets clock synchronizer for the new connection.Reconnectingstate — Prevents "WebSocket is not connected" spam from the time sync loop between disconnection and full teardown.ConnectInternalAsyncretry state — Don't transition throughDisconnectedduringAutoReconnectretries. Let the caller (ConnectAsync/TryReconnectAsync) handle failure, avoiding spurious state change events.Dependency
Test plan
[PlayerName]on all server message handlers🤖 Generated with Claude Code