From 0fbb056a08f2619ab3de37fa9fa15b6ad8b060dc Mon Sep 17 00:00:00 2001 From: Hurdan1 Date: Sun, 26 Jul 2026 12:48:43 -0500 Subject: [PATCH 1/5] Batch move_warship intents to stay under MAX_INTENT_SIZE Large fleet selections produced a single move_warship intent frame larger than the server's 2000-byte cap, which kicks the client. Split fleet orders client-side into batches that each fit in one intent, and share the MAX_INTENT_SIZE constant between client and server. --- src/client/Transport.ts | 13 +++--- src/core/Schemas.ts | 25 +++++++++++ src/server/ClientMsgRateLimiter.ts | 3 +- tests/MoveWarshipIntentBatching.test.ts | 56 +++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 tests/MoveWarshipIntentBatching.test.ts diff --git a/src/client/Transport.ts b/src/client/Transport.ts index 2ad958ab11..dc476db2cd 100644 --- a/src/client/Transport.ts +++ b/src/client/Transport.ts @@ -12,6 +12,7 @@ import { import { TileRef } from "../core/game/GameMap"; import { AllPlayersStats, + batchMoveWarshipUnitIds, ClientHashMessage, ClientIntentMessage, ClientJoinMessage, @@ -649,11 +650,13 @@ export class Transport { } private onMoveWarshipEvent(event: MoveWarshipIntentEvent) { - this.sendIntent({ - type: "move_warship", - unitIds: event.unitIds, - tile: event.tile, - }); + for (const unitIds of batchMoveWarshipUnitIds(event.unitIds, event.tile)) { + this.sendIntent({ + type: "move_warship", + unitIds, + tile: event.tile, + }); + } } private onSendDeleteUnitIntent(event: SendDeleteUnitIntentEvent) { diff --git a/src/core/Schemas.ts b/src/core/Schemas.ts index f47dfa1961..b06b7d9037 100644 --- a/src/core/Schemas.ts +++ b/src/core/Schemas.ts @@ -538,6 +538,31 @@ export const MoveWarshipIntentSchema = z.object({ tile: z.number(), }); +// Client messages larger than this get the client kicked by ClientMsgRateLimiter. +export const MAX_INTENT_SIZE = 2000; + +export function batchMoveWarshipUnitIds( + unitIds: readonly number[], + tile: number, +): number[][] { + const overhead = JSON.stringify({ + type: "intent", + intent: { type: "move_warship", unitIds: [], tile }, + } satisfies ClientIntentMessage).length; + const batches: number[][] = []; + let size = overhead; + for (const unitId of unitIds) { + const unitIdSize = String(unitId).length + 1; + if (batches.length === 0 || size + unitIdSize > MAX_INTENT_SIZE) { + batches.push([]); + size = overhead; + } + batches[batches.length - 1].push(unitId); + size += unitIdSize; + } + return batches; +} + export const DeleteUnitIntentSchema = z.object({ type: z.literal("delete_unit"), unitId: z.number(), diff --git a/src/server/ClientMsgRateLimiter.ts b/src/server/ClientMsgRateLimiter.ts index 96b408092f..6c0dbee42e 100644 --- a/src/server/ClientMsgRateLimiter.ts +++ b/src/server/ClientMsgRateLimiter.ts @@ -1,9 +1,8 @@ import { RateLimiter } from "limiter"; -import { ClientID } from "../core/Schemas"; +import { ClientID, MAX_INTENT_SIZE } from "../core/Schemas"; const INTENTS_PER_SECOND = 10; const INTENTS_PER_MINUTE = 150; -const MAX_INTENT_SIZE = 2000; const TOTAL_BYTES = 5 * 1024 * 1024; // 5MB per client export type RateLimitResult = "ok" | "limit" | "kick"; diff --git a/tests/MoveWarshipIntentBatching.test.ts b/tests/MoveWarshipIntentBatching.test.ts new file mode 100644 index 0000000000..12b4e7c91e --- /dev/null +++ b/tests/MoveWarshipIntentBatching.test.ts @@ -0,0 +1,56 @@ +import { + batchMoveWarshipUnitIds, + ClientIntentMessage, + ClientMessageSchema, + MAX_INTENT_SIZE, +} from "../src/core/Schemas"; +import { replacer } from "../src/core/Util"; + +const TILE = 250_000; + +function frame(unitIds: number[], tile: number): string { + return JSON.stringify( + { + type: "intent", + intent: { type: "move_warship", unitIds, tile }, + } satisfies ClientIntentMessage, + replacer, + ); +} + +describe("batchMoveWarshipUnitIds", () => { + test("sends a small fleet as a single intent", () => { + const unitIds = [10000, 10003, 10006]; + expect(batchMoveWarshipUnitIds(unitIds, TILE)).toEqual([unitIds]); + }); + + test("returns no batches for an empty selection", () => { + expect(batchMoveWarshipUnitIds([], TILE)).toEqual([]); + }); + + test.each([1, 400, 4000])( + "keeps every batch under the server cap (%i warships)", + (count) => { + const unitIds = Array.from({ length: count }, (_, i) => 900_000 + i * 3); + const batches = batchMoveWarshipUnitIds(unitIds, TILE); + + for (const batch of batches) { + expect(batch.length).toBeGreaterThan(0); + expect( + Buffer.byteLength(frame(batch, TILE), "utf8"), + ).toBeLessThanOrEqual(MAX_INTENT_SIZE); + } + expect(batches.flat()).toEqual(unitIds); + }, + ); + + test("every batch is a valid client intent message", () => { + const unitIds = Array.from({ length: 450 }, (_, i) => 10_000 + i * 3); + + for (const batch of batchMoveWarshipUnitIds(unitIds, TILE)) { + expect( + ClientMessageSchema.safeParse(JSON.parse(frame(batch, TILE))).success, + ).toBe(true); + } + }); +}); From 0f4ffabe70255befa5d47fd10fbfa87fdc10c357 Mon Sep 17 00:00:00 2001 From: Hurdan1 Date: Sun, 26 Jul 2026 14:00:27 -0500 Subject: [PATCH 2/5] Bound unitIds in the schema and translate the too_much_data kick reason The issue lists bounding the array as an alternative to batching; 1000 sits above the 964 ids a single batch can physically hold. The kick reason had no en.json entry, so players saw the raw translation key. --- resources/lang/en.json | 3 ++- src/core/Schemas.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/resources/lang/en.json b/resources/lang/en.json index 6c89617f8b..09f5ce3dcc 100644 --- a/resources/lang/en.json +++ b/resources/lang/en.json @@ -890,7 +890,8 @@ "admin": "Kicked by an admin", "duplicate_session": "Kicked from game (you may have been playing on another tab)", "host_left": "The host has left the lobby.", - "lobby_creator": "Kicked by lobby creator" + "lobby_creator": "Kicked by lobby creator", + "too_much_data": "Kicked for sending too much data" }, "lang": { "en": "English", diff --git a/src/core/Schemas.ts b/src/core/Schemas.ts index b06b7d9037..3fd0319e6b 100644 --- a/src/core/Schemas.ts +++ b/src/core/Schemas.ts @@ -534,7 +534,7 @@ export const CancelBoatIntentSchema = z.object({ export const MoveWarshipIntentSchema = z.object({ type: z.literal("move_warship"), - unitIds: z.array(z.number().int()).nonempty(), + unitIds: z.array(z.number().int()).nonempty().max(1000), tile: z.number(), }); From 4b824042b68174aea611fa386fe9fef43133a1dc Mon Sep 17 00:00:00 2001 From: Hurdan1 Date: Sun, 26 Jul 2026 14:29:08 -0500 Subject: [PATCH 3/5] Pace batched move_warship intents at a fixed interval Batches were emitted in one synchronous burst, so an order producing more than ten batches exceeded the server's per-second intent budget and the excess was silently dropped. Send the first batch immediately and the rest at 150ms spacing. Singleplayer uses no spacing since LocalServer has no rate limiter. --- src/client/PacedSender.ts | 35 ++++++++++++ src/client/Transport.ts | 24 +++++++-- tests/client/PacedSender.test.ts | 91 ++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 5 deletions(-) create mode 100644 src/client/PacedSender.ts create mode 100644 tests/client/PacedSender.test.ts diff --git a/src/client/PacedSender.ts b/src/client/PacedSender.ts new file mode 100644 index 0000000000..a0592e2cea --- /dev/null +++ b/src/client/PacedSender.ts @@ -0,0 +1,35 @@ +/** + * Sends queued work at a fixed minimum spacing. + * + * The server drops intents past its per-second budget, so a burst of batches + * has to be spread out rather than emitted all at once. + */ +export class PacedSender { + private readonly queue: (() => void)[] = []; + private timer: ReturnType | null = null; + + constructor(private readonly intervalMs: number) {} + + push(send: () => void): void { + this.queue.push(send); + if (this.timer === null) this.next(); + } + + clear(): void { + this.queue.length = 0; + if (this.timer !== null) { + clearTimeout(this.timer); + this.timer = null; + } + } + + private next(): void { + const send = this.queue.shift(); + if (send === undefined) { + this.timer = null; + return; + } + send(); + this.timer = setTimeout(() => this.next(), this.intervalMs); + } +} diff --git a/src/client/Transport.ts b/src/client/Transport.ts index dc476db2cd..8a9d921532 100644 --- a/src/client/Transport.ts +++ b/src/client/Transport.ts @@ -33,9 +33,15 @@ import { getPlayToken } from "./Auth"; import { LobbyConfig } from "./ClientGameRunner"; import { showInGameAlert } from "./InGameModal"; import { LocalServer } from "./LocalServer"; +import { PacedSender } from "./PacedSender"; import { translateText } from "./Utils"; import { PlayerView } from "./view"; +// Spacing between batched move_warship intents. The server allows 10 intents +// per second across all types; this stays clear of that with room for the +// player's other actions. +const WARSHIP_BATCH_INTERVAL_MS = 150; + export class PauseGameIntentEvent implements GameEvent { constructor(public readonly paused: boolean) {} } @@ -203,6 +209,7 @@ export class Transport { private onmessage: (msg: ServerMessage) => void; private pingInterval: number | null = null; + private readonly warshipBatches: PacedSender; public readonly isLocal: boolean; constructor( @@ -214,6 +221,10 @@ export class Transport { this.isLocal = lobbyConfig.gameRecord !== undefined || lobbyConfig.gameStartInfo?.config.gameType === GameType.Singleplayer; + // The local server has no rate limiter, so only remote games need spacing. + this.warshipBatches = new PacedSender( + this.isLocal ? 0 : WARSHIP_BATCH_INTERVAL_MS, + ); this.eventBus.on(SendAllianceRequestIntentEvent, (e) => this.onSendAllianceRequest(e), @@ -442,6 +453,7 @@ export class Transport { } leaveGame() { + this.warshipBatches.clear(); if (this.isLocal) { this.localServer.endGame(); return; @@ -651,11 +663,13 @@ export class Transport { private onMoveWarshipEvent(event: MoveWarshipIntentEvent) { for (const unitIds of batchMoveWarshipUnitIds(event.unitIds, event.tile)) { - this.sendIntent({ - type: "move_warship", - unitIds, - tile: event.tile, - }); + this.warshipBatches.push(() => + this.sendIntent({ + type: "move_warship", + unitIds, + tile: event.tile, + }), + ); } } diff --git a/tests/client/PacedSender.test.ts b/tests/client/PacedSender.test.ts new file mode 100644 index 0000000000..4b90653b0d --- /dev/null +++ b/tests/client/PacedSender.test.ts @@ -0,0 +1,91 @@ +import { PacedSender } from "../../src/client/PacedSender"; + +const INTERVAL = 150; + +describe("PacedSender", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + test("sends the first item immediately", () => { + const sender = new PacedSender(INTERVAL); + const sent: number[] = []; + + sender.push(() => sent.push(0)); + + expect(sent).toEqual([0]); + }); + + test("holds the rest until their turn", () => { + const sender = new PacedSender(INTERVAL); + const sent: number[] = []; + + for (let i = 0; i < 4; i++) sender.push(() => sent.push(i)); + + expect(sent).toEqual([0]); + vi.advanceTimersByTime(INTERVAL); + expect(sent).toEqual([0, 1]); + vi.advanceTimersByTime(INTERVAL * 2); + expect(sent).toEqual([0, 1, 2, 3]); + }); + + test("delivers a whole fleet order in order", () => { + const sender = new PacedSender(INTERVAL); + const sent: number[] = []; + const batches = 15; + + for (let i = 0; i < batches; i++) sender.push(() => sent.push(i)); + vi.advanceTimersByTime(INTERVAL * batches); + + expect(sent).toEqual(Array.from({ length: batches }, (_, i) => i)); + }); + + test("never sends faster than the interval", () => { + const sender = new PacedSender(INTERVAL); + const times: number[] = []; + + for (let i = 0; i < 15; i++) sender.push(() => times.push(Date.now())); + vi.advanceTimersByTime(INTERVAL * 15); + + for (let i = 1; i < times.length; i++) { + expect(times[i] - times[i - 1]).toBeGreaterThanOrEqual(INTERVAL); + } + }); + + test("orders queued later still go out behind the current one", () => { + const sender = new PacedSender(INTERVAL); + const sent: string[] = []; + + sender.push(() => sent.push("first")); + sender.push(() => sent.push("second")); + expect(sent).toEqual(["first"]); + + vi.advanceTimersByTime(INTERVAL); + expect(sent).toEqual(["first", "second"]); + }); + + test("clear() drops anything still queued", () => { + const sender = new PacedSender(INTERVAL); + const sent: number[] = []; + + for (let i = 0; i < 10; i++) sender.push(() => sent.push(i)); + sender.clear(); + vi.advanceTimersByTime(INTERVAL * 20); + + expect(sent).toEqual([0]); + }); + + test("an interval of zero still preserves order", () => { + const sender = new PacedSender(0); + const sent: number[] = []; + + for (let i = 0; i < 5; i++) sender.push(() => sent.push(i)); + vi.runAllTimers(); + + expect(sent).toEqual([0, 1, 2, 3, 4]); + }); +}); From 72c74d4b95c0008dca196d5024b99d30df254ac7 Mon Sep 17 00:00:00 2001 From: Hurdan1 Date: Sun, 26 Jul 2026 14:46:20 -0500 Subject: [PATCH 4/5] Keep paced batches queued until the socket accepts them PacedSender dequeued before sending, so a batch whose send landed while the socket was reconnecting was lost permanently. sendIntent now reports whether the message went out, and the batch stays at the front of the queue until it does. --- src/client/PacedSender.ts | 10 +++-- src/client/Transport.ts | 15 +++---- tests/client/PacedSender.test.ts | 67 +++++++++++++++++++++++++------- 3 files changed, 68 insertions(+), 24 deletions(-) diff --git a/src/client/PacedSender.ts b/src/client/PacedSender.ts index a0592e2cea..5935673fe6 100644 --- a/src/client/PacedSender.ts +++ b/src/client/PacedSender.ts @@ -5,12 +5,12 @@ * has to be spread out rather than emitted all at once. */ export class PacedSender { - private readonly queue: (() => void)[] = []; + private readonly queue: (() => boolean)[] = []; private timer: ReturnType | null = null; constructor(private readonly intervalMs: number) {} - push(send: () => void): void { + push(send: () => boolean): void { this.queue.push(send); if (this.timer === null) this.next(); } @@ -24,12 +24,14 @@ export class PacedSender { } private next(): void { - const send = this.queue.shift(); + const send = this.queue[0]; if (send === undefined) { this.timer = null; return; } - send(); + // Keep the item queued if it could not be sent, so a reconnect does not + // lose it. leaveGame() clears the queue when the game is over. + if (send()) this.queue.shift(); this.timer = setTimeout(() => this.next(), this.intervalMs); } } diff --git a/src/client/Transport.ts b/src/client/Transport.ts index 8a9d921532..5f7a5f0edb 100644 --- a/src/client/Transport.ts +++ b/src/client/Transport.ts @@ -698,20 +698,21 @@ export class Transport { this.sendIntent({ type: "toggle_game_start_timer" }); } - private sendIntent(intent: Intent) { + private sendIntent(intent: Intent): boolean { if (this.isLocal || this.socket?.readyState === WebSocket.OPEN) { const msg = { type: "intent", intent: intent, } satisfies ClientIntentMessage; this.sendMsg(msg); - } else { - console.log( - "WebSocket is not open. Current state:", - this.socket?.readyState, - ); - console.log("attempting reconnect"); + return true; } + console.log( + "WebSocket is not open. Current state:", + this.socket?.readyState, + ); + console.log("attempting reconnect"); + return false; } private sendMsg(msg: ClientMessage) { diff --git a/tests/client/PacedSender.test.ts b/tests/client/PacedSender.test.ts index 4b90653b0d..9b1423b3a2 100644 --- a/tests/client/PacedSender.test.ts +++ b/tests/client/PacedSender.test.ts @@ -11,11 +11,19 @@ describe("PacedSender", () => { vi.useRealTimers(); }); + // Mirrors Transport.sendIntent, which reports whether the socket took it. + function record(sent: number[], id: number) { + return () => { + sent.push(id); + return true; + }; + } + test("sends the first item immediately", () => { const sender = new PacedSender(INTERVAL); const sent: number[] = []; - sender.push(() => sent.push(0)); + sender.push(record(sent, 0)); expect(sent).toEqual([0]); }); @@ -24,7 +32,7 @@ describe("PacedSender", () => { const sender = new PacedSender(INTERVAL); const sent: number[] = []; - for (let i = 0; i < 4; i++) sender.push(() => sent.push(i)); + for (let i = 0; i < 4; i++) sender.push(record(sent, i)); expect(sent).toEqual([0]); vi.advanceTimersByTime(INTERVAL); @@ -38,7 +46,7 @@ describe("PacedSender", () => { const sent: number[] = []; const batches = 15; - for (let i = 0; i < batches; i++) sender.push(() => sent.push(i)); + for (let i = 0; i < batches; i++) sender.push(record(sent, i)); vi.advanceTimersByTime(INTERVAL * batches); expect(sent).toEqual(Array.from({ length: batches }, (_, i) => i)); @@ -48,7 +56,12 @@ describe("PacedSender", () => { const sender = new PacedSender(INTERVAL); const times: number[] = []; - for (let i = 0; i < 15; i++) sender.push(() => times.push(Date.now())); + for (let i = 0; i < 15; i++) { + sender.push(() => { + times.push(Date.now()); + return true; + }); + } vi.advanceTimersByTime(INTERVAL * 15); for (let i = 1; i < times.length; i++) { @@ -56,23 +69,51 @@ describe("PacedSender", () => { } }); - test("orders queued later still go out behind the current one", () => { + test("keeps unsent batches queued while disconnected", () => { const sender = new PacedSender(INTERVAL); - const sent: string[] = []; + const sent: number[] = []; + let connected = false; + + for (let i = 0; i < 3; i++) { + sender.push(() => { + if (!connected) return false; + sent.push(i); + return true; + }); + } - sender.push(() => sent.push("first")); - sender.push(() => sent.push("second")); - expect(sent).toEqual(["first"]); + vi.advanceTimersByTime(INTERVAL * 10); + expect(sent).toEqual([]); - vi.advanceTimersByTime(INTERVAL); - expect(sent).toEqual(["first", "second"]); + connected = true; + vi.advanceTimersByTime(INTERVAL * 3); + expect(sent).toEqual([0, 1, 2]); + }); + + test("retries the failed batch before the ones behind it", () => { + const sender = new PacedSender(INTERVAL); + const order: number[] = []; + let failFirst = true; + + sender.push(() => { + if (failFirst) return false; + order.push(0); + return true; + }); + sender.push(record(order, 1)); + + expect(order).toEqual([]); + failFirst = false; + vi.advanceTimersByTime(INTERVAL * 2); + + expect(order).toEqual([0, 1]); }); test("clear() drops anything still queued", () => { const sender = new PacedSender(INTERVAL); const sent: number[] = []; - for (let i = 0; i < 10; i++) sender.push(() => sent.push(i)); + for (let i = 0; i < 10; i++) sender.push(record(sent, i)); sender.clear(); vi.advanceTimersByTime(INTERVAL * 20); @@ -83,7 +124,7 @@ describe("PacedSender", () => { const sender = new PacedSender(0); const sent: number[] = []; - for (let i = 0; i < 5; i++) sender.push(() => sent.push(i)); + for (let i = 0; i < 5; i++) sender.push(record(sent, i)); vi.runAllTimers(); expect(sent).toEqual([0, 1, 2, 3, 4]); From 7768d8a86a062b4e612abfb62cc89bfce7c0cf86 Mon Sep 17 00:00:00 2001 From: Hurdan1 Date: Sun, 26 Jul 2026 15:03:02 -0500 Subject: [PATCH 5/5] Only send intents once the server has acknowledged the join An open socket does not mean the server has processed our join or rejoin. Worker.ts handles messages asynchronously and discards anything other than join/rejoin until that completes, so a paced batch sent right after reconnect could be dropped while sendIntent reported success. Track whether the server has replied on this connection and gate intent sends on it. --- src/client/Transport.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/client/Transport.ts b/src/client/Transport.ts index 5f7a5f0edb..ace1374245 100644 --- a/src/client/Transport.ts +++ b/src/client/Transport.ts @@ -210,6 +210,9 @@ export class Transport { private pingInterval: number | null = null; private readonly warshipBatches: PacedSender; + // The server discards intents until it has processed our join, and an open + // socket says nothing about that. Its first message does. + private joined = false; public readonly isLocal: boolean; constructor( @@ -363,6 +366,7 @@ export class Transport { // the desktop app://openfront origin), not window.location.host. const workerPath = ClientEnv.workerPath(this.lobbyConfig.gameID); this.socket = new WebSocket(`${ClientEnv.serverWsBase()}/${workerPath}`); + this.joined = false; this.onconnect = onconnect; this.onmessage = onmessage; this.socket.onopen = () => { @@ -383,6 +387,7 @@ export class Transport { onconnect(); }; this.socket.onmessage = (event: MessageEvent) => { + this.joined = true; try { const parsed = JSON.parse(event.data); const result = ServerMessageSchema.safeParse(parsed); @@ -699,7 +704,10 @@ export class Transport { } private sendIntent(intent: Intent): boolean { - if (this.isLocal || this.socket?.readyState === WebSocket.OPEN) { + if ( + this.isLocal || + (this.joined && this.socket?.readyState === WebSocket.OPEN) + ) { const msg = { type: "intent", intent: intent,