feat(kick-system): add translated kick reasons with error codes #1815
Conversation
- Add kickReason field to ServerErrorMessage schema with enum values - Display translated kick messages instead of generic "Kicked from game" - Include error codes (KICK_ADMIN, KICK_MULTI_TAB, KICK_LOBBY_CREATOR) for debugging - Update kick handlers in GameServer, PostJoinHandler, and Worker to pass reasons - Modify showErrorModal to handle kick-specific formatting without duplication
|
|
WalkthroughIntroduces kick-reason support end-to-end: schema adds optional kickReason, server kickClient API accepts an optional reason and passes it on at call sites (admin, lobby-creator, multi-tab). Client handles kickReason to show localized messages. English translations add kicked message and reason-specific strings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant Server
participant GameServer
User->>Client: Attempt join / in-game
Client->>Server: WebSocket connect / actions
alt Multi-tab detected
Server->>GameServer: kickClient(existingClient, "kick_multi_tab")
else Admin kicks
Server->>GameServer: kickClient(targetClient, "kick_admin")
else Lobby creator kicks
Server->>GameServer: kickClient(targetClient, "kick_lobby_creator")
end
GameServer-->>Client: {"type":"error","error":"Kicked from game","kickReason":...}
Client->>Client: Translate kicked_message + kicked_reason_* and show modal
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
resources/lang/en.json (1)
548-553: Kick translations LGTM; add a generic fallback keyThe new keys read well and cover all current reasons. For forward-compatibility (mixed client/server versions or future reasons), add a generic fallback message so the UI never shows a raw translation key.
Apply this diff:
- "kicked_reason_multi_tab": "You may be playing on another tab or browser window." + "kicked_reason_multi_tab": "You may be playing on another tab or browser window.", + "kicked_reason_default": "You were removed from the game by the server."src/server/Worker.ts (1)
297-299: Prefer a structured JSON response and echo the reasonMinor API polish: return JSON consistently and include the reason used. Easier to parse by tools and consistent with other endpoints.
- game.kickClient(clientID, "kick_admin"); - res.status(200).send("Player kicked successfully"); + const reason = "kick_admin" as const; + game.kickClient(clientID, reason); + res.status(200).json({ success: true, clientID, reason });src/client/ClientGameRunner.ts (1)
108-121: DRY: extract a small helper to show kick error modalsBoth handlers now share identical logic. Extract a helper to keep things tidy and reduce future drift.
Example helper (outside the selected ranges):
function showKickError(gameID: GameID, clientID: ClientID, kickReason: string) { const reasonKey = `error_modal.kicked_reason_${kickReason.replace("kick_", "")}`; let reasonMessage = translateText(reasonKey); if (reasonMessage === reasonKey) { reasonMessage = translateText("error_modal.kicked_reason_multi_tab"); } showErrorModal( reasonMessage, `Error Code: ${kickReason.toUpperCase()}`, gameID, clientID, true, false, "error_modal.kicked_message", ); }Then replace both in-place blocks with:
showKickError(lobbyConfig.gameID, lobbyConfig.clientID, message.kickReason);I can push the refactor if you want.
Also applies to: 355-369
src/server/GameServer.ts (1)
517-524: Add more signal in the close frame (optional)Not critical, but consider using a policy violation close code and include the reason as the close message for easier client-side triage with no prior “error” frame.
- client.ws.send( + client.ws.send( JSON.stringify({ error: "Kicked from game", kickReason, type: "error", } satisfies ServerErrorMessage), ); - client.ws.close(1000, "Kicked from game"); + // 1008 = Policy Violation (standard). The message is free-form. + client.ws.close(1008, kickReason ?? "Kicked from game");Also applies to: 527-527
src/core/Schemas.ts (1)
468-471: Nit: Document the new field with allowed values and usageA short JSDoc helps future readers understand how this couples with i18n and client UX.
Example:
/** * Optional machine-readable reason for a server kick. * Used by the client to show a localized message. * Allowed values: "kick_admin" | "kick_multi_tab" | "kick_lobby_creator". * When absent, the client should show a generic kicked message. */ kickReason: KickReasonSchema.optional(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (6)
resources/lang/en.json(1 hunks)src/client/ClientGameRunner.ts(2 hunks)src/core/Schemas.ts(1 hunks)src/server/GameServer.ts(3 hunks)src/server/Worker.ts(1 hunks)src/server/worker/websocket/handler/message/PostJoinHandler.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/server/Worker.ts (1)
src/server/GameManager.ts (1)
game(18-20)
src/client/ClientGameRunner.ts (2)
src/client/LangSelector.ts (1)
translateText(246-266)src/client/Utils.ts (1)
translateText(82-137)
src/server/GameServer.ts (4)
src/core/game/GameView.ts (1)
clientID(260-262)src/core/game/PlayerImpl.ts (1)
clientID(194-196)src/core/game/TerraNulliusImpl.ts (1)
clientID(9-11)src/core/Schemas.ts (1)
ClientID(22-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
src/server/worker/websocket/handler/message/PostJoinHandler.ts (1)
80-80: LGTM: correct reason supplied to kickCalling
gs.kickClient(..., "kick_lobby_creator")ensures the client sees the right, localized reason. This aligns with the schema and client handling.src/server/GameServer.ts (1)
178-179: LGTM: conflict kick includes explicit reasonForwarding
"kick_multi_tab"here is correct and matches the client’s translation keys.src/core/Schemas.ts (1)
468-471: No i18n key mismatch — client strips the "kick_" prefix before building the keyVerified: server sends "kick_*" codes, client does message.kickReason.replace("kick_", "") when building
error_modal.kicked_reason_*, and resources/lang/en.json containskicked_reason_*entries — alignment confirmed. No change required.Relevant locations:
- src/core/Schemas.ts — kickReason enum includes "kick_admin", "kick_multi_tab", "kick_lobby_creator" (around line 468)
- src/server/GameServer.ts — kickClient sends the kickReason in the error message (around lines 506–526)
- src/client/ClientGameRunner.ts — builds reasonKey with
.replace("kick_", "")and calls translateText (lines 108–116 and 355–363)- resources/lang/en.json — "kicked_reason_admin", "kicked_reason_lobby_creator", "kicked_reason_multi_tab" (lines 550–552)
| if (message.kickReason) { | ||
| const kickMessage = translateText("error_modal.kicked_message"); | ||
| const reasonKey = `error_modal.kicked_reason_${message.kickReason.replace("kick_", "")}`; | ||
| const reasonMessage = translateText(reasonKey); | ||
|
|
||
| showErrorModal( | ||
| kickMessage, | ||
| `${reasonMessage}\nError Code: ${message.kickReason.toUpperCase()}`, | ||
| lobbyConfig.gameID, | ||
| lobbyConfig.clientID, | ||
| true, | ||
| false, | ||
| "error_modal.connection_error", | ||
| ); |
There was a problem hiding this comment.
Fix import path to avoid duplicate module instances of Utils
The file imports Utils twice with two different relative paths (“./Utils” and “../client/Utils”). This can cause the bundler/loader to create separate module instances, breaking shared caches in translateText and adding bytes.
Update the import at the top of this file:
// before
import { translateText } from "../client/Utils";
// after
import { translateText } from "./Utils";🤖 Prompt for AI Agents
In src/client/ClientGameRunner.ts around lines 108 to 121, the file currently
imports translateText from "../client/Utils" which creates a duplicate module
instance alongside "./Utils"; update the import to use "./Utils" (replace the
"../client/Utils" import) so both imports resolve to the same module instance,
remove the duplicate import if present, and run a quick build to confirm shared
caches (e.g., translateText) behave correctly.
🛠️ Refactor suggestion
Use the “kicked” heading and add a robust translation fallback
Set the modal heading to the translated “kicked” title, and fall back to a safe reason message if the specific translation key is missing (e.g., mixed versions). Also remove the now-unused kickMessage variable.
Apply this diff:
- if (message.kickReason) {
- const kickMessage = translateText("error_modal.kicked_message");
- const reasonKey = `error_modal.kicked_reason_${message.kickReason.replace("kick_", "")}`;
- const reasonMessage = translateText(reasonKey);
-
- showErrorModal(
- kickMessage,
- `${reasonMessage}\nError Code: ${message.kickReason.toUpperCase()}`,
- lobbyConfig.gameID,
- lobbyConfig.clientID,
- true,
- false,
- "error_modal.connection_error",
- );
+ if (message.kickReason) {
+ const reasonKey = `error_modal.kicked_reason_${message.kickReason.replace("kick_", "")}`;
+ let reasonMessage = translateText(reasonKey);
+ if (reasonMessage === reasonKey) {
+ // Fallback if key missing (e.g., mixed versions)
+ reasonMessage = translateText("error_modal.kicked_reason_multi_tab");
+ }
+ showErrorModal(
+ reasonMessage,
+ `Error Code: ${message.kickReason.toUpperCase()}`,
+ lobbyConfig.gameID,
+ lobbyConfig.clientID,
+ true,
+ false,
+ "error_modal.kicked_message",
+ );
} else {
showErrorModal(
message.error,
message.message,
lobbyConfig.gameID,
lobbyConfig.clientID,
true,
false,
"error_modal.connection_error",
);
}Also applies to: 123-132
| if (message.kickReason) { | ||
| const kickMessage = translateText("error_modal.kicked_message"); | ||
| const reasonKey = `error_modal.kicked_reason_${message.kickReason.replace("kick_", "")}`; | ||
| const reasonMessage = translateText(reasonKey); | ||
|
|
||
| showErrorModal( | ||
| kickMessage, | ||
| `${reasonMessage}\nError Code: ${message.kickReason.toUpperCase()}`, | ||
| this.lobby.gameID, | ||
| this.lobby.clientID, | ||
| true, | ||
| false, | ||
| "error_modal.connection_error", | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate logic: apply same heading and fallback in the in-game handler
Mirror the fixes from join flow inside the in-game WebSocket handler to keep UX consistent.
- if (message.type === "error") {
- if (message.kickReason) {
- const kickMessage = translateText("error_modal.kicked_message");
- const reasonKey = `error_modal.kicked_reason_${message.kickReason.replace("kick_", "")}`;
- const reasonMessage = translateText(reasonKey);
-
- showErrorModal(
- kickMessage,
- `${reasonMessage}\nError Code: ${message.kickReason.toUpperCase()}`,
- this.lobby.gameID,
- this.lobby.clientID,
- true,
- false,
- "error_modal.connection_error",
- );
- } else {
+ if (message.type === "error") {
+ if (message.kickReason) {
+ const reasonKey = `error_modal.kicked_reason_${message.kickReason.replace("kick_", "")}`;
+ let reasonMessage = translateText(reasonKey);
+ if (reasonMessage === reasonKey) {
+ reasonMessage = translateText("error_modal.kicked_reason_multi_tab");
+ }
+ showErrorModal(
+ reasonMessage,
+ `Error Code: ${message.kickReason.toUpperCase()}`,
+ this.lobby.gameID,
+ this.lobby.clientID,
+ true,
+ false,
+ "error_modal.kicked_message",
+ );
+ } else {
showErrorModal(
message.error,
message.message,
this.lobby.gameID,
this.lobby.clientID,
true,
false,
"error_modal.connection_error",
);
}
}Also applies to: 370-379
🤖 Prompt for AI Agents
In src/client/ClientGameRunner.ts around lines 355 to 369 (and similarly 370 to
379), the in-game WebSocket handler duplicates old logic for kicked messages
instead of mirroring the join-flow fixes; update the block so the heading and
reason fallback behavior match the join flow: compute kickMessage via
translateText("error_modal.kicked_message"), derive reasonKey by stripping
"kick_" and attempt translateText(reasonKey) with a fallback to
translateText("error_modal.kicked_reason_unknown") (or the same fallback used in
join flow), and call showErrorModal with the same parameters and formatted body
(reasonMessage plus "Error Code: ..." in uppercase) so UX is consistent across
handlers.
| @@ -465,6 +465,7 @@ export const ServerDesyncSchema = z.object({ | |||
|
|
|||
| export const ServerErrorSchema = z.object({ | |||
| error: z.string(), | |||
| kickReason: z.enum(["kick_admin", "kick_multi_tab", "kick_lobby_creator"]).optional(), | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract KickReason into a shared schema and type for reuse
Inline string unions tend to get duplicated across server and client. Centralizing the values improves consistency and reduces drift.
- Add these definitions near other shared schemas:
export const KickReasonValues = ["kick_admin", "kick_multi_tab", "kick_lobby_creator"] as const;
export type KickReason = typeof KickReasonValues[number];
export const KickReasonSchema = z.enum(KickReasonValues);- Then update the field here:
- kickReason: z.enum(["kick_admin", "kick_multi_tab", "kick_lobby_creator"] as const).optional(),
+ kickReason: KickReasonSchema.optional(),This also lets you import KickReason as a type for signatures like kickClient(reason?: KickReason).
🤖 Prompt for AI Agents
In src/core/Schemas.ts around line 468 the kickReason field uses an inline
z.enum which should be centralized; add three shared exports near other shared
schemas: define a const KickReasonValues as the readonly array of
["kick_admin","kick_multi_tab","kick_lobby_creator"], export a type KickReason =
typeof KickReasonValues[number], and export KickReasonSchema =
z.enum(KickReasonValues); then replace the inline z.enum at line 468 with
KickReasonSchema.optional() (and update any imports/usages to use the KickReason
type where needed).
Add as const to z.enum literal array to keep strict types and avoid TS error
Without as const, TypeScript often widens the array to string[], which is not assignable to the tuple type [string, ...string[]] expected by z.enum. This can break type inference or fail compilation.
Apply this minimal fix:
- kickReason: z.enum(["kick_admin", "kick_multi_tab", "kick_lobby_creator"]).optional(),
+ kickReason: z.enum(["kick_admin", "kick_multi_tab", "kick_lobby_creator"] as const).optional(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| kickReason: z.enum(["kick_admin", "kick_multi_tab", "kick_lobby_creator"]).optional(), | |
| kickReason: z.enum(["kick_admin", "kick_multi_tab", "kick_lobby_creator"] as const).optional(), |
🤖 Prompt for AI Agents
In src/core/Schemas.ts around line 468, the z.enum call uses a plain string
array which TypeScript widens to string[] and can cause type errors; update the
enum literal to use a readonly tuple by appending "as const" to the array (e.g.
z.enum(["kick_admin", "kick_multi_tab", "kick_lobby_creator"] as
const).optional()) so the types remain strict and satisfy z.enum's expected
tuple type.
|
|
||
| public kickClient(clientID: ClientID, kickReason?: "kick_admin" | "kick_multi_tab" | "kick_lobby_creator"): void { | ||
| if (this.kickedClients.has(clientID)) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid duplicating the KickReason union; import a shared type
The inline union risks drift if a new reason is added elsewhere. Prefer a single source of truth (Schemas).
Option A (preferred, if exported):
- public kickClient(clientID: ClientID, kickReason?: "kick_admin" | "kick_multi_tab" | "kick_lobby_creator"): void {
+ public kickClient(clientID: ClientID, kickReason?: KickReason): void {And add at top:
import type { KickReason } from "../core/Schemas";Option B (if KickReason type isn’t exported yet): export it from Schemas and use it here. I can open a follow-up PR to centralize this type.
🤖 Prompt for AI Agents
In src/server/GameServer.ts around lines 505 to 507, the method kickClient uses
an inline union type for the kick reason which duplicates the KickReason
definition; replace the inline union with the shared KickReason type from the
Schemas module by importing it at the top of the file (import type { KickReason
} from "../core/Schemas") and update the method signature to use that type; if
KickReason is not exported from Schemas, export it there first (or ask to open a
follow-up PR) then import and use it here.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
Description:
Added translated kick reasons so players see specific context instead of generic "Kicked from game" errors.
Fixes #1654
What's new:
Example:
Please complete the following: