Skip to content

Send password rejection before kicking client#1923

Merged
Flamefire merged 2 commits into
Return-To-The-Roots:masterfrom
DevOpsOfChaos:sidequest/direct-ip-wrong-password-message
May 7, 2026
Merged

Send password rejection before kicking client#1923
Flamefire merged 2 commits into
Return-To-The-Roots:masterfrom
DevOpsOfChaos:sidequest/direct-ip-wrong-password-message

Conversation

@DevOpsOfChaos
Copy link
Copy Markdown
Contributor

@DevOpsOfChaos DevOpsOfChaos commented May 1, 2026

Summary

  • send the password rejection response synchronously before kicking the client
  • avoid reporting a generic connection-lost error when joining a password-protected direct-IP game with a wrong password
  • keep the existing WrongPassword client error path unchanged
  • add deterministic client-side regression coverage for the wrong-password response

Root cause

The server queued the password response with sendMsgAsync() and immediately kicked the client afterwards. For wrong passwords this could close the connection before the queued "false" response was sent, so the client only observed a connection loss.

This PR sends the "false" password response synchronously before kicking the player. Successful password responses keep the existing async send path.

The regression test intentionally avoids asserting real socket delivery-vs-disconnect timing, because that would be brittle. Instead, it verifies that receiving GameMessage_Server_Password("false") triggers the existing ClientError::WrongPassword path and stops the client.

Validation

  • Built Test_network locally with Visual Studio 2022 / Debug
  • Ran Test_network.exe --run_test=GameClientTests/ClientReportsWrongPasswordResponse --report_level=short
    • 1 test case passed
    • 9 assertions passed
  • Ran Test_network.exe --report_level=short
    • 14 test cases passed
    • 228 assertions passed
  • Ran git diff --check

Fixes #1057

Flow86
Flow86 previously approved these changes May 1, 2026
Copy link
Copy Markdown
Member

@Flow86 Flow86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was probably a missed refactor thing, I remember that in an early version the KickPlayer flushed the queue of the disconnected player, but now its not doing that.

@Flamefire Flamefire enabled auto-merge May 2, 2026 10:58
auto-merge was automatically disabled May 7, 2026 11:18

Head branch was pushed to by a user without write access

@DevOpsOfChaos DevOpsOfChaos force-pushed the sidequest/direct-ip-wrong-password-message branch from 36fad40 to 00c49eb Compare May 7, 2026 11:18
@DevOpsOfChaos
Copy link
Copy Markdown
Contributor Author

Added deterministic client-side coverage for the wrong-password response path.

I avoided a socket timing test here; the new test verifies that receiving GameMessage_Server_Password("false") triggers ClientError::WrongPassword and stops the client.

Validation:

  • targeted wrong-password test passed
  • full Test_network passed: 14 tests, 228 assertions
  • git diff --check

Copy link
Copy Markdown
Member

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Flamefire Flamefire enabled auto-merge May 7, 2026 11:22
@Flamefire Flamefire merged commit 73f7c72 into Return-To-The-Roots:master May 7, 2026
19 of 20 checks passed
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.

Direct IP and passwords

3 participants