feat: Kick player in game #2969
Conversation
|
|
WalkthroughAdds lobby-creator moderation: client UI and modal to kick players, client state to track kicked players, server-side kick-reason propagation, new translations, tests for the kick flow, and a minor error-message translation tweak. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "Lobby Creator"
participant Panel as "PlayerPanel"
participant Modal as "PlayerModerationModal"
participant EventBus as "EventBus"
participant Server as "GameServer"
participant Target as "Target Client"
User->>Panel: open player actions / click Kick
Panel->>Modal: openModeration(target)
Modal->>User: show confirmation dialog
User->>Modal: confirm kick
Modal->>Panel: dispatch local "kicked" event (targetID)
Modal->>EventBus: emit SendKickPlayerIntentEvent(targetID)
EventBus->>Server: deliver kick intent (targetID)
Server->>Server: kickClient(targetID, "lobby_creator")
Server->>Target: send error (reasonKey) and close WebSocket
Panel->>Panel: add targetID to kickedPlayerIDs and hide UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/client/graphics/layers/PlayerPanel.ts`:
- Around line 324-326: renderModeration currently displays the "Kick" button
when clientID !== null but handleKickClick uses a falsy check (const
targetClientID = other.clientID(); if (!targetClientID) return;) which allows
empty string/undefined to show a non-functional button; make the checks
consistent by changing handleKickClick to the same existence test used in
renderModeration (e.g., explicitly compare to null/undefined the same way
renderModeration does) or update renderModeration to use the same falsy check as
handleKickClick so the Kick button is only rendered when other.clientID() passes
the identical truthiness test; update both occurrences referenced
(handleKickClick and the other similar block) to use the same check.
In `@src/server/GameServer.ts`:
- Around line 777-820: kickClient currently only adds to kickedClients when the
client is present in activeClients, so a user who disconnected can still rejoin
after a moderation kick; update kickClient to also add the target clientID to
this.kickedClients when the client is not found (the else branch) — or at
minimum when reason indicates a moderation/lobby kick (reason !==
"duplicate_session") — so that kickedClients always blocks rejoin even if the
WebSocket is already gone; reference the kickClient method, this.activeClients,
and this.kickedClients when making the change.
|
can you make it so a nation takes over? not sure how hard/possible it would be |
I'm not sure if that is a wanted feature. I'm sure it's possible! |
src/server/GameServer.ts
Outdated
| public kickClient(clientID: ClientID): void { | ||
| public kickClient( | ||
| clientID: ClientID, | ||
| reason: KickClientReason = "duplicate_session", |
There was a problem hiding this comment.
instead can this be a string, and the server just passes a translation key
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/GameServer.ts (1)
333-363: Validate kick targets are in this game before blocking rejoin. Right now a lobby creator can send anyclientID; sincekickClientrecords it even if not active, this can block unknown IDs from ever joining. Please verify the target exists inallClients(oractiveClients) before callingkickClient.🔧 Suggested fix
case "kick_player": { // Check if the authenticated client is the lobby creator if (client.clientID !== this.lobbyCreatorID) { this.log.warn(`Only lobby creator can kick players`, { clientID: client.clientID, creatorID: this.lobbyCreatorID, target: clientMsg.intent.target, gameID: this.id, }); return; } // Don't allow lobby creator to kick themselves if (client.clientID === clientMsg.intent.target) { this.log.warn(`Cannot kick yourself`, { clientID: client.clientID, }); return; } + + const targetClient = this.allClients.get(clientMsg.intent.target); + if (!targetClient) { + this.log.warn(`Cannot kick client not in game`, { + clientID: client.clientID, + target: clientMsg.intent.target, + gameID: this.id, + }); + return; + } // Log and execute the kick this.log.info(`Lobby creator initiated kick of player`, { creatorID: client.clientID, target: clientMsg.intent.target, gameID: this.id, kickMethod: "websocket", }); this.kickClient(clientMsg.intent.target, KICK_REASON_LOBBY_CREATOR); return; }
🤖 Fix all issues with AI agents
In `@src/client/graphics/layers/PlayerModerationModal.ts`:
- Around line 53-74: The handler handleKickClick currently proceeds to
confirmation and dispatches a local "kicked" event even if this.eventBus is
null; update handleKickClick to guard for a missing eventBus early (before the
confirm dialog) and return if this.eventBus is falsy to avoid locally marking a
player kicked without sending the SendKickPlayerIntentEvent; ensure
alreadyKicked and dispatchEvent("kicked")/closeModal are only reached after
confirming and successfully emitting the eventBus event.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/server/GameServer.ts`:
- Around line 785-823: kickClient currently adds clientID to kickedClients
before verifying membership, which lets callers mark arbitrary IDs kicked;
change the flow in kickClient so you first check whether the target exists in
this.allClients (or this.activeClients) and only then add to this.kickedClients
and proceed with sending the error/close and removing from activeClients; if the
client is disconnected but present in this.allClients still add to kickedClients
(to block rejoin) but do not attempt to send/close a ws; preserve the existing
log messages (use this.allClients / this.activeClients and client.ws/send/close
references) and avoid adding to kickedClients on unknown IDs.
New modal, shows up after clicking the "Moderation" button |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/GameServer.ts (1)
334-365: Add target existence validation before callingkickClient.The intent handler doesn't validate that the target client exists in
activeClientsbefore passing it tokickClient. AlthoughkickClientitself checksallClientsand logs a warning if not found, validation should happen at the API boundary in the intent handler. This matches the input validation principle: validate all client-provided data (the target ID) before processing.// Validate target exists in active clients const targetClient = this.activeClients.find( (c) => c.clientID === clientMsg.intent.target, ); if (!targetClient) { this.log.warn(`Cannot kick: target not in active clients`, { creatorID: client.clientID, target: clientMsg.intent.target, gameID: this.id, }); return; } this.kickClient( clientMsg.intent.target, KICK_REASON_LOBBY_CREATOR, );
🧹 Nitpick comments (1)
src/server/GameServer.ts (1)
34-35: Type the kick reason with a union instead ofstring.This is a small TS safety win and matches the “typed unions” guideline.
♻️ Suggested change
-const KICK_REASON_DUPLICATE_SESSION = "kick_reason.duplicate_session"; -const KICK_REASON_LOBBY_CREATOR = "kick_reason.lobby_creator"; +const KICK_REASON_DUPLICATE_SESSION = "kick_reason.duplicate_session" as const; +const KICK_REASON_LOBBY_CREATOR = "kick_reason.lobby_creator" as const; +type KickReasonKey = + | typeof KICK_REASON_DUPLICATE_SESSION + | typeof KICK_REASON_LOBBY_CREATOR; public kickClient( clientID: ClientID, - reasonKey: string = KICK_REASON_DUPLICATE_SESSION, + reasonKey: KickReasonKey = KICK_REASON_DUPLICATE_SESSION, ): void {Also applies to: 785-788
| other.type() === PlayerType.Human && | ||
| !!other.clientID(); | ||
|
|
||
| if (!canKick && !alreadyKicked) return html``; |
There was a problem hiding this comment.
maybe we should the moderation panel regardless? and then the moderation panel can have the kick button grayed out if we already kicked the player? Otherwise people maybe get confused why moderation button dissapeared.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/client/graphics/layers/PlayerPanel.ts`:
- Around line 446-469: In PlayerPanel.renderModeration, don't render the
moderation button unless the local player is allowed to kick; add an early
return when canKick is false (and/or when alreadyKicked is true if you want to
hide after kicking) so the button only shows for
creator-only/human/not-self/in-game cases, and remove or repurpose the unused
locals to satisfy ESLint (use canKick and alreadyKicked in the guard or delete
the unused variable). Ensure you update the function renderModeration to return
nothing/empty markup when the guard fails so the moderation UI is not rendered.
MitchZinck
force-pushed
the
feature/2686-kick-player-in-game
branch
from
cd3b1cc to
0d3fc2b
Compare
MitchZinck
force-pushed
the
feature/2686-kick-player-in-game
branch
from
0d3fc2b to
4ea751a
Compare
4 participants
If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #2686
Description:
Screenshot order:
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
mitchfz