Sab

[scamiv]

Description:

SharedArrayBuffer tile state, backlog-aware tick ingestion, and extended performance instrumentation

Summary

• Introduces a SharedArrayBuffer-backed tile state and ring buffer so the worker can stream tile changes to the main thread without shipping full per-tick tile payloads.
• Makes the worker self-clocked and lets the client apply game updates in backlog-aware slices, reducing UI jank when the simulation falls behind.
• Extends the performance overlay with detailed metrics (backlog, worker vs render throughput, tile update volumes, SAB ring utilization) to debug performance issues.

Feature Set 1 – Shared tile state between worker and main thread

What it does

• TerrainMapLoader (src/core/game/TerrainMapLoader.ts)

  • TerrainMapData now optionally carries sharedStateBuffer and sharedDirtyBuffer.
  • loadTerrainMap(...) can take an optional sharedStateBuffer. If SAB is supported and crossOriginIsolated is true, it creates a SharedArrayBuffer of size width * height * Uint16.
  • Caching is disabled in SAB-capable environments unless we’re explicitly in the non-shared path, so each game instance gets its own mutable state.

• GameMapImpl (src/core/game/GameMap.ts)

  • The state array (per-tile mutable state) can now be backed by an externally supplied buffer (stateBuffer?: ArrayBufferLike), which in practice is a SharedArrayBuffer in SAB mode.
  • Validates that the buffer length matches width * height to avoid misaligned state.

• GameRunner (src/core/GameRunner.ts)

  • createGameRunner(...) takes a sharedStateBuffer and passes it into loadTerrainMap so the worker’s GameMapImpl views the same SharedArrayBuffer as the client.
  • From the game’s perspective, logic and rules are unchanged; only the backing storage for tile state differs.

• ClientGameRunner (src/client/ClientGameRunner.ts)

  • On client startup, loadTerrainMap(...) is called without a sharedStateBuffer. Inside the loader, if SAB is available, the returned TerrainMapData already includes a fresh SharedArrayBuffer.
  • The client then:
    • Detects if SAB is usable (SharedArrayBuffer, Atomics, and crossOriginIsolated).
    • Extracts gameMap.sharedStateBuffer if present.
    • Passes that same SharedArrayBuffer into the worker via WorkerClient and into GameRunner’s createGameRunner(...).
  • GameView (src/core/game/GameView.ts) gets a usesSharedTileState flag so it knows whether packedTileUpdates are “apply this update” or “this tile has already been updated in shared state; just mark it dirty”.

Interaction with the rest of the game

• Simulation logic (Game, GameImpl, Executor, etc.) is unchanged; they still manipulate GameMap through the existing API.
• In SAB mode:
  • Worker and main thread see the same Uint16Array for tile state. The worker writes into it during game.executeNextTick().
  • The main thread no longer re-applies state diffs; it just uses tile refs to know which tiles to redraw.
• In non-SAB environments:
  • Behavior is effectively identical to main: each thread has its own Uint16Array state, and tile updates continue to flow via packedTileUpdates.

Implications

• Performance: Eliminates per-tick copying of huge BigUint64Array tile update arrays from worker → main when SAB is available.
• Memory: One shared tile-state buffer instead of separate copies in worker and main.
• Correctness: Requires that all tile state mutations stay in the worker. The main thread must not mutate the shared state directly (and doesn’t, given current APIs).
• Environment sensitivity: The shared-state path only activates when SharedArrayBuffer + Atomics + crossOriginIsolated === true. Otherwise, the code falls back to the old, cached, non-SAB behavior.

Feature Set 2 – Shared tile update ring buffer (SAB) and dirty flags

What it does

• New ring buffer implementation (src/core/worker/SharedTileRing.ts)

  • SharedTileRingBuffers: three SharedArrayBuffers:
    • header: Int32Array with write index, read index, overflow flag.
    • data: Uint32Array of tile refs.
    • dirty: Uint8Array of per-tile dirty flags.
  • createSharedTileRingBuffers(capacity, numTiles) & createSharedTileRingViews(...) set up both buffers and typed views.
  • pushTileUpdate(...): append a tile ref into the ring, dropping oldest entries and flagging overflow if full.
  • drainTileUpdates(...): pop up to maxItems into an array, advancing the read pointer atomically.

• Worker integration (src/core/worker/Worker.worker.ts)

  • On init:
    • If sharedTileRingHeader/sharedTileRingData are provided, builds a SharedTileRingViews and keeps dirtyFlags from the dirty buffer.
    • Passes a tileUpdateSink to createGameRunner(...):
      • In the deduping path: only pushes tile ref into the ring if dirtyFlags[tile] was 0 (using Atomics.compareExchange).
      • In the simpler path: pushes every tile ref without deduping if only the ring is present.
  • From then on, every tile update in GameRunner can be streamed into the ring instead of being serialized into packedTileUpdates.

• GameRunner integration (src/core/GameRunner.ts)

  • For each tick:
    • Reads updates[GameUpdateType.Tile].
    • If tileUpdateSink exists:
      • Computes tile refs from the TileUpdate bigint and calls tileUpdateSink per tile.
      • Clears updates[GameUpdateType.Tile] and sets packedTileUpdates to an empty BigUint64Array.
    • Otherwise, fills packedTileUpdates exactly as in main.

• Client integration (src/client/ClientGameRunner.ts)

  • On startup:
    • Computes number of tiles from gameMap.gameMap.width() * height().
    • Allocates ring buffers via createSharedTileRingBuffers and passes header + data + dirty to the worker via WorkerClient.
    • Also keeps a local SharedTileRingViews and Uint8Array for dirtyFlags.
  • When processing a batch of game updates per frame:
    • Calls drainTileUpdates(...) with a MAX_TILE_UPDATES_PER_RENDER cap (~100k).
    • Deduplicates tile refs in a Set.
    • Resets dirty flags for each processed tile via Atomics.store(dirtyFlags, ref, 0).
    • Builds a new packedTileUpdates array containing tile refs as BigInt and attaches it to the merged GameUpdateViewData.

• GameView integration (src/core/game/GameView.ts)

  • In update(...), if usesSharedTileState is true:
    • Treats packedTileUpdates entries as tile refs and does not call updateTile.
    • Simply records updatedTiles so render layers can redraw changed tiles.

Interaction with the rest of the game

• The ring is purely a transport optimization between worker and main; the GameUpdateViewData still presents a packedTileUpdates array to GameView.
• Layers that rely on GameView.recentlyUpdatedTiles() continue to work; they just see tile refs drawn from the ring rather than from decoded TileUpdate bigints.
• Worker messaging remains the same shape (game_update messages); the difference is that packedTileUpdates is empty coming out of the worker in SAB mode and is populated on the client side from the ring.

Implications

• Throughput: The worker can emit tile updates at high volume without incurring postMessage serialization costs.
• GC pressure: Moves from “allocate large BigUint64Array on every tick” to “reuse fixed-size SAB ring and small per-frame arrays”.
• Overflow behavior: When the ring fills, oldest tile refs are dropped and an overflow flag is set; the overlay surfaces whether overflow ever occurred, making it visible when the system is under-provisioned.
• Deduping semantics: With dirty flags, each tile ref appears at most once in the ring per “dirty” period; this can slightly reduce redundant redraws on the main thread compared to the naïve per-update approach.

Feature Set 3 – Backlog-aware tick ingestion and self-clocked worker

What it does

• Worker overhaul (src/core/worker/Worker.worker.ts)

  • Removes the "heartbeat" message type; the worker no longer depends on the main thread poking it.
  • Introduces processPendingTurns():
    • If a GameRunner exists and hasPendingTurns(), loops through executeNextTick() until there are no pending turns.
    • Guarded by an isProcessingTurns flag to avoid re-entrancy.
  • Calls processPendingTurns():
    • Once after initialization is complete.
    • After every turn message is added via addTurn(...).

• WorkerClient (src/core/worker/WorkerClient.ts) & messages (src/core/worker/WorkerMessages.ts)

  • Drops sendHeartbeat() and the "heartbeat" message type entirely.
  • Extends InitMessage to carry shared ring buffers and shared state buffer to the worker.

• ClientGameRunner tick ingestion (src/client/ClientGameRunner.ts)

  • Previously: each game_update from the worker was applied immediately; render and simulation tick rate were tightly coupled.
  • Now:
    • All GameUpdateViewData are appended to pendingUpdates.
    • processPendingUpdates():
      • Runs in requestAnimationFrame slices.
      • Computes a per-frame time budget (sliceBudgetMs) based on the current backlog (backlogTurns), scaling from 8ms up to 1000ms as backlog grows from ~10 to 500 turns.
      • Consumes up to MAX_TICKS_PER_SLICE (1000) updates or until it runs out of budget.
      • For each consumed update:
        • Tells the transport that a turn is complete.
        • Emits SendHashEvent for server hash checking.
        • Updates backlog metrics (updateBacklogMetrics) based on serverTurnHighWater and the tick number.
        • Saves the game if a Win update is present.
      • Once the queue is drained for this batch:
        • Merges the batch’s non-tile updates and tile updates (via ring or fallback).
        • Applies a single gameView.update(...).
        • Calls renderer.tick().
        • Emits TickMetricsEvent with:
          • Last tick execution time (from worker).
          • Network tick delay (currentTickDelay between server messages).
          • Backlog turns, ticks-per-render, worker ticks/s, render ticks/s.
          • Tile update counts and ring buffer metrics (see Feature Set 4).

• Backlog metrics and events

  • ClientGameRunner.updateBacklogMetrics(processedTick) maintains:
    • backlogTurns = max(0, serverTurnHighWater - lastProcessedTick).
    • backlogGrowing flag (whether backlog increased since last sample).
  • Emits BacklogStatusEvent(backlogTurns, backlogGrowing) whenever metrics update.
  • GameRenderer (src/client/graphics/GameRenderer.ts) listens for BacklogStatusEvent and stores backlogTurns.

Interaction with the rest of the game

• Server/client protocol:
  • ServerMessage handling is unchanged for start, desync, error, turn messages.
  • The client still sends Turn intents to the worker and marks them complete with transport.turnComplete(); this now happens inside the per-frame processing loop rather than immediately on each game_update.
• Replay and singleplayer:
  • Both paths use the same ClientGameRunner and worker; the backlog logic applies uniformly, making heavy replays and high-speed games smoother.

Implications

• Responsiveness: When the client falls behind (e.g., slow machine, heavy game, or replays), it can process multiple simulation ticks per render frame without freezing the UI.
• Smoothness: Rendering is no longer forced to run at worker tick rate; frames are decoupled via batching and ticksPerRender tracking.
• Risk: The “self-clocked” worker must never rely on extra heartbeats for progress; all future features that enqueue work must do it through addTurn or analogous queued work so processPendingTurns() sees it.

Feature Set 4 – Render throttling based on simulation backlog

What it does

• GameRenderer.renderGame() (src/client/graphics/GameRenderer.ts)
  • Before rendering:
    • If backlogTurns > 0, dynamically reduces target FPS from 60 down to 10 as backlog grows (capped at 50 turns).
    • Computes a minimum frame interval from this target; if the last render was too recent, immediately schedules another requestAnimationFrame and skips work for this frame.
  • Tracks lastRenderTime to enforce the throttle.

Interaction with the rest of the game

• This is purely a rendering-side change; all game logic and worker behavior remain the same.
• The performance overlay still receives per-frame durations and layer breakdowns via updateFrameMetrics as before.

Implications

• Catch-up behavior: When simulation is behind, rendering yields some headroom for the worker and main-thread tick ingestion to catch up, at the cost of visual frame rate.
• User perception: The game will feel “slower” visually during extreme backlog, but stays interactive and converges instead of hard-freezing.
• Tuning knobs: BASE_FPS, MIN_FPS, and BACKLOG_MAX_TURNS are the primary tuning constants; they are currently hard-coded.

Feature Set 5 – Extended performance overlay & instrumentation

What it does

• Input events (src/client/InputHandler.ts)

  • TickMetricsEvent gains:
    • backlogTurns
    • ticksPerRender
    • workerTicksPerSecond, renderTicksPerSecond
    • tileUpdatesCount
    • ringBufferUtilization, ringBufferOverflows, ringDrainTime
  • New BacklogStatusEvent(backlogTurns, backlogGrowing) to communicate backlog status to the renderer (and any future listeners).

• Performance overlay (src/client/graphics/layers/PerformanceOverlay.ts)

  • Subscribes to the richer TickMetricsEvent and updates new reactive state fields:
    • Backlog turns and tick throughput (worker/render).
    • Ticks per render frame.
    • Tile updates per render, peak tile updates, total tile updates.
    • Ring buffer utilization (% of capacity touched per render), whether any overflow occurred, and time spent draining the ring.
  • Resets tile metrics when the overlay is cleared.
  • Exposes tile-related metrics in getSnapshot() under a new tiles section.
  • Renders new lines in the overlay UI:
    • Worker ticks/s, render ticks/s, ticks per render.
    • Backlog turns.
    • Tile updates/render (current and peak).
    • Ring buffer utilization and overflow count.
    • Ring drain time in ms.

Interaction with the rest of the game

• The overlay remains entirely optional, controlled by UserSettings: updateTickMetrics is a no-op when overlay is disabled or hidden.
• No gameplay logic depends on these new fields; they are purely diagnostic.

Implications

• Debuggability: Makes it much easier to understand whether bottlenecks are in the worker, main-thread tick ingestion, rendering, or tile update streaming.
• Monitoring: Ring buffer metrics expose whether the SAB pipeline is appropriately sized for the map size and load.

Risks / Edge Cases

• SAB Availability: On environments without SharedArrayBuffer (or without crossOriginIsolated), all the new shared-memory paths automatically fall back to the old behavior. It’s important to test both modes.
• Overflow behavior: If the tile ring overflows regularly (e.g., massive tile churn + too small capacity), some tile updates can be dropped before they’re rendered, leading to temporary visual desyncs. The overflow flag and tile metrics are there to make this visible.
• Concurrency: Shared buffers use Atomics to synchronize indices and dirty flags; correct behavior assumes no direct writes outside of the established push/drain/reset flow.

Testing Suggestions

• Run a normal multiplayer game on a SAB-capable origin (crossOriginIsolated true) and:
  • Verify that the game plays normally and rendering remains correct.
  • Observe the performance overlay to confirm reasonable worker/render tick rates and tile update counts.
• Run on an environment where SAB is disabled:
  • Confirm that games still load and run (fallback path).
• Force heavy load (large map, late-game, or high replay speed) and:
  • Check that backlog metrics increase and then decrease as the client catches up.
  • Confirm that render FPS drops under heavy backlog and recovers later.
  • Watch ring buffer utilization and overflow to ensure the capacity scaling per map size is adequate.

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory
• I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

DISCORD_USERNAME

[coderabbitai]

Walkthrough

Implements shared in-memory tile-ring buffers and threads them through worker and client, replaces heartbeat with explicit pending-turn processing in the worker, adds client-side batched update processing with backlog tracking/metrics, extends tick metrics and performance overlay with tile/ring stats, and adds backlog-aware renderer pacing.

Changes

Cohort / File(s)	Summary
Shared Tile Ring Infrastructure src/core/worker/SharedTileRing.ts	New module providing SharedArrayBuffer-based ring: buffer/view interfaces, header constants, creation helpers, pushTileUpdate and drainTileUpdates using Atomics, plus exported views/buffers types.
Worker Protocol & Client Init src/core/worker/WorkerMessages.ts, src/core/worker/WorkerClient.ts, src/core/worker/Worker.worker.ts	Removed heartbeat type/API; InitMessage now accepts optional SAB fields (sharedTileRingHeader, sharedTileRingData, sharedStateBuffer, sharedDirtyBuffer); WorkerClient forwards SABs; worker initializes SharedTileRingViews when provided, adds turn-drain processing (processPendingTurns) and routes tile updates into the shared ring or sink.
Client Update Batching & Backlog src/client/ClientGameRunner.ts	Added pending updates queue and time-sliced processPendingUpdates, mergeGameUpdates, backlog/lag tracking fields and emission of enriched TickMetricsEvent and BacklogStatusEvent; wired tileRingViews and dirtyFlags through constructor and startup flow.
Metrics & Events src/client/InputHandler.ts	Extended TickMetricsEvent with backlog, worker/render rates, tile/ring metrics; added new BacklogStatusEvent type.
Renderer Frame Pacing src/client/graphics/GameRenderer.ts	Subscribes to BacklogStatusEvent; adds backlogTurns, backlogGrowing, lastRenderTime fields and backlog-aware frame pacing to throttle/skip renders when backlog is high.
Performance Overlay src/client/graphics/layers/PerformanceOverlay.ts	Expanded updateTickMetrics signature and internal state to store/display tile updates, ring buffer utilization/overflows, ring drain time, worker/render rates and aggregated tile stats.
Core Game State & Map Loading src/core/game/GameMap.ts, src/core/game/TerrainMapLoader.ts, src/core/game/GameView.ts, src/core/GameRunner.ts	GameMapImpl can accept external/shared state buffer; TerrainMapLoader/genTerrainFromBin accept/propagate shared buffers and persist sharedStateBuffer/sharedDirtyBuffer; GameView gains usesSharedTileState flag and branches update logic; GameRunner accepts optional tileUpdateSink, routes tile updates to sink when present and exposes hasPendingTurns().

Sequence Diagram(s)

sequenceDiagram
    participant Main as Main Thread
    participant Client as ClientGameRunner
    participant Queue as pendingUpdates
    participant Merge as mergeGameUpdates
    participant View as GameView
    participant TileRing as SharedTileRing
    participant Render as Renderer
    participant Metrics as TickMetricsEvent

    Main->>Client: receive GameUpdate from worker
    Client->>Queue: enqueue update
    Note over Client: per-frame processing (processPendingUpdates, time-sliced)
    Client->>Queue: dequeue batch
    Queue->>Merge: merge updates -> combined update + tile metrics
    Merge->>View: apply merged update (or push shared tile refs)
    alt uses shared tile state
        View->>TileRing: drain/ack tile updates
    end
    Client->>Metrics: emit TickMetricsEvent + BacklogStatusEvent
    Client->>Render: request render if allowed
    Render->>Render: check backlogTurns -> defer or render

Loading

sequenceDiagram
    participant Worker as Worker
    participant GameRun as GameRunner
    participant TileRing as SharedTileRing

    Note over Worker: after init or after processing a turn
    Worker->>Worker: processPendingTurns()
    loop while GameRun.hasPendingTurns()
        Worker->>GameRun: execute turn
        GameRun->>TileRing: pushTileUpdate(tileRef) (Atomics)
        TileRing->>TileRing: write or advance read on overflow
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas needing extra attention:

• src/client/ClientGameRunner.ts — batching, merge logic, time-slice correctness, metric emission.
• src/core/worker/SharedTileRing.ts — Atomics correctness, memory layout, overflow semantics.
• src/core/worker/Worker.worker.ts — processPendingTurns reentrancy and ordering.
• src/core/game/TerrainMapLoader.ts & src/core/game/GameMap.ts — shared buffer creation/validation and caching decisions.
• src/client/graphics/GameRenderer.ts & PerformanceOverlay — frame pacing math and UI metrics consistency.

Possibly related PRs

• Catch up mode #2507 — Direct overlap: client batching/backlog, TickMetrics, worker turn processing, heartbeat removal.
• Add performance stats #2338 — Related telemetry and renderer/overlay performance-metric changes.
• Build bar #2059 — Changes touching InputHandler/TickMetricsEvent and event plumbing.

Suggested labels

Feature - Frontend

Suggested reviewers

• scottanderson

Poem

🧩 Rings of tiles hum across the thread,
Backlog counts the turns ahead,
Batches fold and metrics sing,
Frames breathe easy — no heartbeat ring. 🎶

Pre-merge checks

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'Sab' is extremely vague and does not clearly convey the main changes, which involve SharedArrayBuffer tile state, backlog-aware ingestion, and performance instrumentation.	Use a more descriptive title like 'Add SharedArrayBuffer tile state, backlog-aware ingestion, and performance metrics' to clearly communicate the scope of changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is comprehensive, detailed, and directly related to the changeset, covering all five feature sets with clear explanations of what, why, and how.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (3)

src/client/InputHandler.ts (1)

128-146: Consider an options object for cleaner API.

The TickMetricsEvent constructor now has 10 optional parameters. This works but can be hard to read at call sites. Consider using an options object:

interface TickMetricsData {
  tickExecutionDuration?: number;
  tickDelay?: number;
  backlogTurns?: number;
  // ... etc
}

export class TickMetricsEvent implements GameEvent {
  constructor(public readonly data: Partial<TickMetricsData> = {}) {}
}

This makes call sites clearer and easier to extend. Not blocking, just a suggestion for maintainability.

src/core/GameRunner.ts (1)

183-196: Tile update routing looks correct.

The branch logic cleanly separates the two paths: push to sink when available, otherwise pack into array. One minor note: creating new BigUint64Array() on every tick when using the sink path allocates an empty typed array each time.

Consider reusing a constant empty array:

+const EMPTY_TILE_UPDATES = new BigUint64Array(0);
+
 // Many tiles are updated; either publish them via a shared sink or pack
 // them into the view data.
 let packedTileUpdates: BigUint64Array;
 const tileUpdates = updates[GameUpdateType.Tile];
 if (this.tileUpdateSink !== undefined) {
   for (const u of tileUpdates) {
     const tileRef = Number(u.update >> 16n) as TileRef;
     this.tileUpdateSink(tileRef);
   }
-  packedTileUpdates = new BigUint64Array();
+  packedTileUpdates = EMPTY_TILE_UPDATES;
 } else {

src/client/ClientGameRunner.ts (1)

670-676: Type casting loses safety.

The as unknown as any[] pattern bypasses TypeScript's type system. Consider a more explicit approach:

 for (const key in gu.updates) {
   const type = Number(key) as GameUpdateType;
-  // We don't care about the specific update subtype here; just treat it
-  // as an array we can concatenate.
-  const updatesForType = gu.updates[type] as unknown as any[];
-  (combinedUpdates[type] as unknown as any[]).push(...updatesForType);
+  const source = gu.updates[type];
+  const target = combinedUpdates[type];
+  for (let i = 0; i < source.length; i++) {
+    target.push(source[i]);
+  }
 }

This avoids the double cast and spread operator overhead on large arrays.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d8c1c2 and c1bc4a7.

📒 Files selected for processing (12)

• src/client/ClientGameRunner.ts (11 hunks)
• src/client/InputHandler.ts (1 hunks)
• src/client/graphics/GameRenderer.ts (4 hunks)
• src/client/graphics/layers/PerformanceOverlay.ts (6 hunks)
• src/core/GameRunner.ts (5 hunks)
• src/core/game/GameMap.ts (2 hunks)
• src/core/game/GameView.ts (3 hunks)
• src/core/game/TerrainMapLoader.ts (4 hunks)
• src/core/worker/SharedTileRing.ts (1 hunks)
• src/core/worker/Worker.worker.ts (4 hunks)
• src/core/worker/WorkerClient.ts (3 hunks)
• src/core/worker/WorkerMessages.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (6)

📚 Learning: 2025-10-20T20:15:28.858Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.

Applied to files:

• src/client/InputHandler.ts
• src/core/GameRunner.ts
• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-10-21T20:06:04.823Z

Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.

Applied to files:

• src/core/game/TerrainMapLoader.ts

📚 Learning: 2025-10-08T17:14:49.369Z

Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: In `ClientGameRunner.ts`, the `myPlayer` field is always set when `shouldPreventWindowClose()` is called, so the null check in that method is sufficient without needing to fetch it again from `gameView.playerByClientID()`.

Applied to files:

• src/core/GameRunner.ts
• src/client/ClientGameRunner.ts

📚 Learning: 2025-06-09T02:20:43.637Z

Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.

Applied to files:

• src/client/ClientGameRunner.ts
• src/client/graphics/GameRenderer.ts

📚 Learning: 2025-08-12T00:31:50.144Z

Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.

Applied to files:

• src/client/ClientGameRunner.ts

📚 Learning: 2025-10-18T11:00:57.142Z

Learnt from: NewYearNewPhil
Repo: openfrontio/OpenFrontIO PR: 2230
File: src/client/graphics/GameRenderer.ts:269-277
Timestamp: 2025-10-18T11:00:57.142Z
Learning: In src/client/graphics/GameRenderer.ts, the GameRecapCapture implementation does not use setCaptureRenderEnabled on layers. Instead, it uses RecapCaptureSurface.capture() to render capture layers (TerrainLayer, TerritoryLayer, RailroadLayer, StructureIconsLayer, UnitLayer) directly to an off-screen canvas without requiring layer-level capture mode methods.

Applied to files:

• src/client/graphics/GameRenderer.ts

🧬 Code graph analysis (7)

src/client/InputHandler.ts (1)

src/core/EventBus.ts (1)

• GameEvent (1-1)

src/core/game/TerrainMapLoader.ts (2)

src/core/game/GameImpl.ts (2)

• map (197-199)
• miniMap (200-202)

src/client/TerrainMapFileLoader.ts (1)

• terrainMapFileLoader (4-4)

src/core/worker/SharedTileRing.ts (1)

src/core/game/GameMap.ts (1)

• TileRef (3-3)

src/core/worker/Worker.worker.ts (3)

src/core/worker/SharedTileRing.ts (3)

• SharedTileRingViews (9-14)
• createSharedTileRingViews (31-43)
• pushTileUpdate (45-64)

src/core/GameRunner.ts (1)

• createGameRunner (35-95)

src/core/game/GameMap.ts (1)

• TileRef (3-3)

src/core/GameRunner.ts (3)

src/core/game/GameView.ts (1)

• tile (106-108)

src/core/game/UnitImpl.ts (1)

• tile (176-178)

src/core/game/GameMap.ts (1)

• TileRef (3-3)

src/core/game/GameMap.ts (3)

src/core/game/GameView.ts (2)

• width (701-703)
• height (704-706)

src/core/game/GameImpl.ts (2)

• width (807-809)
• height (810-812)

src/client/graphics/TransformHandler.ts (1)

• width (48-50)

src/client/graphics/GameRenderer.ts (1)

src/client/InputHandler.ts (1)

• BacklogStatusEvent (148-153)

🔇 Additional comments (23)

src/core/game/GameMap.ts (1)

78-103: Clean addition of shared state buffer support.

The optional stateBuffer parameter keeps backward compatibility. The length validation is correct, and creating a Uint16Array view on the shared buffer is the right approach.

One small note: since this buffer can be shared across threads (via SharedArrayBuffer), callers must ensure proper synchronization when reading/writing state. The current design assumes the worker owns writes and the main thread reads, which should be fine.

src/core/worker/WorkerMessages.ts (1)

34-42: Clean message type extension.

The optional SharedArrayBuffer fields allow the worker to operate in either shared-memory mode or fallback mode. Good design for progressive enhancement.

src/core/worker/WorkerClient.ts (2)

23-29: Good constructor extension for shared buffer support.

The optional parameters keep backward compatibility. Storing them as private fields and forwarding to the worker init message is clean.

---

72-81: Initialization payload correctly wires shared buffers.

The optional chaining on sharedTileRingBuffers?.header handles the undefined case properly.

src/client/InputHandler.ts (1)

148-153: Simple and clear event class.

BacklogStatusEvent is straightforward with required fields. Good.

src/client/graphics/GameRenderer.ts (2)

356-378: Good backlog-aware frame pacing implementation.

The logic reduces render FPS linearly from 60 to 10 as backlog grows from 0 to 50 turns. This allows the simulation to catch up while still showing progress. The early return with requestAnimationFrame keeps the loop alive.

One edge case: if lastRenderTime is 0 (first frame), the check on line 368 ensures we skip throttling, which is correct.

---

316-321: Event subscription correctly updates state.

The BacklogStatusEvent handler updates internal state that drives the frame pacing logic.

src/core/worker/Worker.worker.ts (2)

44-65: Solid reentrancy guard for turn processing.

The isProcessingTurns flag prevents recursive calls. The try/finally ensures the flag is reset even on error.

---

90-99: Clever conditional for tile update sink.

The three-way ternary handles:

1. Ring + dirty flags: uses atomic compare-exchange for deduplication
2. Ring only: direct push (no dedup)
3. No ring: undefined (fallback mode)

The Atomics.compareExchange(dirtyFlags!, tile, 0, 1) === 0 pattern is correct for single-writer "mark if not already marked" logic.

src/client/graphics/layers/PerformanceOverlay.ts (3)

545-548: Verify overflow accumulation logic.

Line 547 does this.ringBufferOverflows += ringBufferOverflows. This assumes each call passes an increment (0 or 1), not a cumulative count. Make sure the caller (TickMetricsEvent producer) sends delta values, not totals. Otherwise the displayed count will grow incorrectly.

---

440-469: Good use of Lit reactive state for new metrics.

All new fields use @state() decorator, which triggers re-renders when values change. Clean approach.

---

326-333: Reset properly clears new tile metrics.

All new fields are zeroed, maintaining consistency with the reset behavior.

src/core/GameRunner.ts (2)

40-48: LGTM! Clean extension of the factory function.

The new optional parameters tileUpdateSink and sharedStateBuffer are well-placed and properly forwarded. This allows callers to opt-in to shared memory features without breaking existing usage.

---

292-295: LGTM! Simple and clear method.

The hasPendingTurns() method provides a clean way to check if there are queued turns to process.

src/core/game/TerrainMapLoader.ts (2)

73-80: stateBuffer only passed for Normal map size.

The stateBuffer is passed to genTerrainFromBin only for GameMapSize.Normal (line 78), not for GameMapSize.Compact (line 80). Verify this is intentional.

---

115-132: LGTM! Clean parameter addition.

The optional stateBuffer parameter is properly forwarded to GameMapImpl.

src/client/ClientGameRunner.ts (3)

173-203: LGTM! Good setup for shared buffer infrastructure.

The cross-origin isolation check and conditional buffer creation is well-structured. The ring capacity scaling with world size (numTiles) is a sensible choice.

---

500-569: Solid frame-pacing logic with adaptive budget.

The dynamic slice budget based on backlog size is a good approach for catching up while keeping UI responsive. The constants are well-documented.

One small note: the array compaction at lines 562-569 could be optimized by using a deque or circular buffer pattern instead of slice(), but for typical backlog sizes this should be fine.

---

745-756: LGTM! Clean backlog tracking.

The metrics update and event emission for backlog status is straightforward and well-structured.

src/core/worker/SharedTileRing.ts (4)

3-14: LGTM! Clean interface definitions.

The separation between raw SharedArrayBuffer handles and typed Views is a good pattern - it allows the buffers to be transferred between workers while views are created locally.

---

66-85: LGTM! Drain implementation is straightforward.

The drain loop correctly advances the read pointer atomically after processing. The maxItems limit prevents unbounded draining.

---

21-29: I'll help you verify the review comment about BacklogStatusEvent. Let me search the codebase to understand the current state of this event definition and its imports.
<function_calls>

shell
#!/bin/bash

Search for BacklogStatusEvent definition and usage

rg -n "BacklogStatusEvent" --type=ts -A3 -B3


</function_calls>

Now let me check the ClientGameRunner.ts file specifically at the mentioned import line:
<function_calls>

shell
#!/bin/bash

Find ClientGameRunner.ts and check imports around line 39

fd -n "ClientGameRunner.ts" --type=f | head -1 | xargs -I {} sh -c 'echo "File: {}"; head -50 {} | tail -15'


</function_calls>

Let me also check the InputHandler file to see what it exports:
<function_calls>

shell
#!/bin/bash

Find InputHandler file and check exports

fd -n "InputHandler" --type=f -e ts | head -5


</function_calls>

---

45-64: Since the repository clone is failing, I'll analyze the concern based on the code snippet provided and my understanding of concurrent ring buffer patterns.

Producer-side overflow handling does create a race condition in concurrent scenarios.

Analyzing the code snippet:

1. Lines 52-54: Read both write and read pointers atomically (independent loads)
2. Lines 58-59: If buffer is full, the producer advances the read pointer by storing to TILE_RING_HEADER_READ_INDEX
3. Line 60: Producer writes the value
4. Line 61: Producer advances write pointer

The race condition described is valid: if the consumer is simultaneously reading and advancing read, and the producer also advances read during overflow, they can interfere with each other's view of the buffer state. This is a known issue with producer-side overflow handling in lock-free ring buffers.

However, without access to:

• How pushTileUpdate is actually called (single producer vs. multiple producers)
• The consumer implementation
• Any documentation or tests
• Whether there are additional synchronization mechanisms in place

I cannot definitively determine whether this is an actual problem or if the SPSC assumption is correctly maintained in practice.

---

Producer-side overflow handling can cause entries to be skipped if producer and consumer race during overflow, as described. However, verification requires examining actual usage patterns, consumer implementation, and any synchronization guarantees in the codebase.

Without access to the repository to verify the usage patterns and consumer side implementation, I cannot confirm whether the SPSC model is enforced or if alternative synchronization exists. The theoretical concern is valid, but practical impact depends on usage context.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Missing translations for new metric labels.

The new performance lines use hardcoded English strings like "Worker ticks/s:", "Backlog turns:", etc. Other labels in this file use translateText() for localization.

Consider adding translation keys:

        <div class="performance-line">
-         Worker ticks/s:
+         ${translateText("performance_overlay.worker_ticks")}
          <span>${this.workerTicksPerSecond.toFixed(1)}</span>
        </div>

This keeps consistency with the rest of the overlay.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In src/client/graphics/layers/PerformanceOverlay.ts around lines 707 to 737,
several new performance label strings are hardcoded in English (e.g., "Worker
ticks/s:", "Render ticks/s:", "Ticks per render:", "Backlog turns:", "Tile
updates/render:", "Ring buffer:", "Ring drain time:") instead of using the
existing translateText() localization helper; replace each hardcoded label with
a call to translateText() using appropriate translation keys (add new keys to
the i18n resource files if needed), keep any punctuation/formatting the same,
and ensure the translated strings are used consistently with surrounding labels
so the overlay supports localization.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

src/client/graphics/layers/PerformanceOverlay.ts (1)

707-737: Missing translations for new metric labels.

These new performance lines use hardcoded English strings instead of translateText(), breaking localization consistency with the rest of the overlay.

As noted in the previous review, wrap each label with translateText():

         <div class="performance-line">
-          Worker ticks/s:
+          ${translateText("performance_overlay.worker_ticks_per_second")}
           <span>${this.workerTicksPerSecond.toFixed(1)}</span>
         </div>

Apply similar changes to all new labels: "Render ticks/s:", "Ticks per render:", "Backlog turns:", "Tile updates/render:", "Ring buffer:", and "Ring drain time:". Remember to add the corresponding keys to en.json.

Based on learnings, translation keys should be added to en.json only; other locale files will be updated by dedicated translation PRs.

🧹 Nitpick comments (1)

src/client/graphics/layers/PerformanceOverlay.ts (1)

326-332: Consider resetting all new metrics for consistency.

The reset clears tile/ring metrics but not backlogTurns, ticksPerRender, workerTicksPerSecond, and renderTicksPerSecond. For consistency, consider resetting these as well:

     // reset tile metrics
     this.tileUpdatesPerRender = 0;
     this.tileUpdatesPeak = 0;
     this.ringBufferUtilization = 0;
     this.ringBufferOverflows = 0;
     this.ringDrainTime = 0;
     this.totalTilesUpdated = 0;
+
+    // reset tick/backlog metrics
+    this.backlogTurns = 0;
+    this.ticksPerRender = 0;
+    this.workerTicksPerSecond = 0;
+    this.renderTicksPerSecond = 0;

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c1bc4a7 and 1e3d2e1.

📒 Files selected for processing (2)

• src/client/graphics/layers/PerformanceOverlay.ts (6 hunks)
• src/core/game/TerrainMapLoader.ts (4 hunks)

🧰 Additional context used

🧠 Learnings (5)

📚 Learning: 2025-06-02T14:27:37.609Z

Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-08-16T10:52:08.292Z

Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-10-20T20:15:28.858Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-11-26T20:49:29.110Z

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.110Z
Learning: In GameView.ts, when usesSharedTileState is true (SAB mode), packedTileUpdates contains unpacked tile references as BigInt(tileRef) only, because all tile state lives in the shared Uint16Array. In non-SAB mode, packedTileUpdates contains packed TileUpdate bigints in the format (tileRef << 16n | state), which must be decoded via updateTile(tu). Therefore, Number(tu) is correct in SAB mode and shifting right by 16 bits would be wrong.

Applied to files:

• src/core/game/TerrainMapLoader.ts

📚 Learning: 2025-10-21T20:06:04.823Z

Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.

Applied to files:

• src/core/game/TerrainMapLoader.ts

🧬 Code graph analysis (1)

src/core/game/TerrainMapLoader.ts (1)

src/client/TerrainMapFileLoader.ts (1)

• terrainMapFileLoader (4-4)

🔇 Additional comments (11)

src/core/game/TerrainMapLoader.ts (7)

9-10: LGTM! Clean type extension.

Adding optional SharedArrayBuffer fields to the type definition is backwards compatible and clearly supports the new SAB-based tile state sharing.

---

42-55: Clarify the caching strategy.

The caching logic introduces three boolean flags (useCache, canUseSharedBuffers, shouldUseCache) to determine when to cache. The behavior is:

• Cache is skipped when sharedStateBuffer is provided (expected)
• Cache is also skipped when canUseSharedBuffers is true, even if no buffer was provided

This means when SharedArrayBuffer is available, each call without a provided buffer will create a new SharedArrayBuffer instead of reusing a cached one. If this prevents inappropriate state sharing across game instances, it's correct—but if callers are expected to reuse the same map multiple times, this could be inefficient.

Add a comment explaining why caching is disabled when canUseSharedBuffers is true, or verify this behavior is intentional:

// Don't use cache if we can create SharedArrayBuffer but none was provided
// This prevents sharing state buffers across different game instances
const shouldUseCache = useCache && !canUseSharedBuffers;

If callers should reuse the same SAB-backed map, consider documenting that they must pass the same sharedStateBuffer on subsequent calls.

---

59-67: LGTM! StateBuffer creation is well-structured.

The logic cleanly handles three cases: use provided buffer, create new SAB if possible, or use undefined. The size calculation correctly allocates 2 bytes per tile for Uint16 state.

---

71-75: LGTM! Proper stateBuffer forwarding.

The stateBuffer is correctly passed only to the main game map when mapSize === Normal. The 4x and mini maps don't receive shared state buffers, which is appropriate since they're likely used only for preview/display purposes.

---

95-109: LGTM! Cache consistency is now correct.

The result object properly includes sharedStateBuffer when a SharedArrayBuffer was used, and the cache write (lines 107-109) is now gated by shouldUseCache, matching the cache read logic. This addresses the previous issue where cache was written but not read consistently.

The typeof SharedArrayBuffer !== "undefined" check on line 100 is necessary for environments where SharedArrayBuffer might not be defined at all.

---

116-116: LGTM! Type choice is appropriately flexible.

Using ArrayBufferLike for the stateBuffer parameter allows both ArrayBuffer and SharedArrayBuffer, which gives flexibility while remaining type-safe. The optional parameter maintains backwards compatibility.

---

129-129: LGTM! Clean parameter forwarding.

The stateBuffer is correctly forwarded to the GameMapImpl constructor without unnecessary transformation.

src/client/graphics/layers/PerformanceOverlay.ts (4)

232-243: LGTM! Event forwarding is clean.

The expanded TickMetricsEvent parameters are correctly forwarded to updateTickMetrics.

---

440-468: LGTM! State declarations follow conventions.

The new metric fields are properly declared with @state() decorators and appropriate types.

---

470-481: LGTM! Signature is backward-compatible.

All new parameters are optional, maintaining compatibility while extending functionality.

---

584-591: LGTM! Snapshot includes all tile metrics.

The new tiles block properly captures all expanded tile and ring buffer metrics for JSON export.

[scamiv]

Atomic-SAB Architecture

Shared Tile State (sharedStateBuffer: SharedArrayBuffer → GameMapImpl.state: Uint16Array)
Authoritative per-tile game state (owner, fallout, etc.) - worker writes, main reads directly

Ring Header (SharedTileRingBuffers.header: SharedArrayBuffer → Int32Array[3])
Write index, read index, overflow flag - atomics coordinate producer/consumer access

Ring Data (SharedTileRingBuffers.data: SharedArrayBuffer → Uint32Array(capacity))
Queue of TileRefs needing redraw - worker enqueues, main drains as dirty hints for rendering

Dirty Flags (SharedTileRingBuffers.dirty: SharedArrayBuffer → Uint8Array(numTiles))
Per-tile dirty bitmap (0=clean, 1=dirty) - ensures each tile enqueued at most once while dirty; cleared when main processes tile

[scamiv]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (3)

src/client/graphics/layers/PerformanceOverlay.ts (2)

545-548: Overflow tracking acknowledged as temporary debug flag.

Per past review discussion, this is intentionally a flag (not a counter) for debugging purposes and will be removed later.

---

707-737: Missing translations for new metric labels.

Already flagged in previous review - these hardcoded English strings should use translateText() for consistency with other labels in the overlay.

src/client/ClientGameRunner.ts (1)

706-716: Overflow flag read but not reset - acknowledged as debug feature.

Per past review discussion, the overflow flag being read but not cleared is intentional for debugging purposes.

🧹 Nitpick comments (6)

src/client/graphics/GameRenderer.ts (1)

356-378: Backlog-aware frame throttling looks correct.

The logic scales target FPS linearly from 60 to 10 as backlog grows from 0 to 50 turns. When backlogTurns > 0, rendering is skipped if the frame interval hasn't elapsed. Once backlog clears, rendering resumes at full speed.

One small note: BACKLOG_MAX_TURNS = 50 is a magic number. Consider extracting this to a config constant if it needs tuning later.

src/core/worker/Worker.worker.ts (1)

90-99: Simplify: second branch is unreachable dead code.

Looking at lines 73-83: when sharedTileRing is created, dirtyFlags is always set from sharedTileRing.dirtyFlags. So if sharedTileRing is truthy, dirtyFlags is also truthy. The second branch (lines 96-97) can never execute.

          gameRunner = createGameRunner(
            message.gameStartInfo,
            message.clientID,
            mapLoader,
            gameUpdate,
-           sharedTileRing && dirtyFlags
-             ? (tile: TileRef) => {
-                 if (Atomics.compareExchange(dirtyFlags!, tile, 0, 1) === 0) {
-                   pushTileUpdate(sharedTileRing!, tile);
-                 }
-               }
-             : sharedTileRing
-               ? (tile: TileRef) => pushTileUpdate(sharedTileRing!, tile)
-               : undefined,
+           sharedTileRing
+             ? (tile: TileRef) => {
+                 if (Atomics.compareExchange(dirtyFlags!, tile, 0, 1) === 0) {
+                   pushTileUpdate(sharedTileRing!, tile);
+                 }
+               }
+             : undefined,
            message.sharedStateBuffer,
          )

src/client/graphics/layers/PerformanceOverlay.ts (1)

470-481: Consider using an options object for cleaner signature.

With 10 optional parameters, the function signature is getting long. An options object would make call sites more readable and self-documenting.

-  updateTickMetrics(
-    tickExecutionDuration?: number,
-    tickDelay?: number,
-    backlogTurns?: number,
-    ticksPerRender?: number,
-    workerTicksPerSecond?: number,
-    renderTicksPerSecond?: number,
-    tileUpdatesCount?: number,
-    ringBufferUtilization?: number,
-    ringBufferOverflows?: number,
-    ringDrainTime?: number,
-  ) {
+  updateTickMetrics(opts: {
+    tickExecutionDuration?: number;
+    tickDelay?: number;
+    backlogTurns?: number;
+    ticksPerRender?: number;
+    workerTicksPerSecond?: number;
+    renderTicksPerSecond?: number;
+    tileUpdatesCount?: number;
+    ringBufferUtilization?: number;
+    ringBufferOverflows?: number;
+    ringDrainTime?: number;
+  }) {
+    const {
+      tickExecutionDuration,
+      tickDelay,
+      backlogTurns,
+      // ... etc
+    } = opts;

src/core/worker/SharedTileRing.ts (1)

66-85: Drain implementation is correct, but consider pre-sizing output array.

The drain logic is sound. However, using out.push() in a tight loop can trigger array reallocations. For performance with large drains, consider pre-computing count and using array index assignment:

 export function drainTileUpdates(
   views: SharedTileRingViews,
   maxItems: number,
   out: TileRef[],
 ): void {
   const { header, buffer, capacity } = views;
 
   let read = Atomics.load(header, TILE_RING_HEADER_READ_INDEX);
   const write = Atomics.load(header, TILE_RING_HEADER_WRITE_INDEX);
 
-  let count = 0;
-
-  while (read !== write && count < maxItems) {
-    out.push(buffer[read]);
-    read = (read + 1) % capacity;
-    count++;
-  }
+  // Pre-compute available count
+  const available = (write - read + capacity) % capacity;
+  const toRead = Math.min(available, maxItems);
+  const startIndex = out.length;
+  out.length = startIndex + toRead;
+  
+  for (let i = 0; i < toRead; i++) {
+    out[startIndex + i] = buffer[read];
+    read = (read + 1) % capacity;
+  }
 
   Atomics.store(header, TILE_RING_HEADER_READ_INDEX, read);
 }

src/client/ClientGameRunner.ts (2)

571-625: Rendering only occurs when pending queue is fully drained.

The condition on line 572 (pendingStart >= pendingUpdates.length) means rendering happens only after ALL pending updates are processed. During heavy backlog, this delays visual feedback until catch-up is complete.

Consider rendering after each time-slice while still showing stale state, or rendering periodically even with pending updates:

-      // Only update view and render when ALL processing is complete
-      if (
-        this.pendingStart >= this.pendingUpdates.length &&
-        batch.length > 0 &&
-        lastTick !== undefined
-      ) {
+      // Render after each slice to maintain visual responsiveness
+      if (batch.length > 0 && lastTick !== undefined) {

This would provide smoother visual feedback during catch-up, showing intermediate game states rather than waiting for full sync.

---

669-677: Type assertion to any[] bypasses type safety.

The cast to unknown as any[] works but loses compile-time checks. Consider a typed helper:

function appendUpdates<T>(target: T[], source: T[]): void {
  target.push(...source);
}

Then use it with explicit type for each GameUpdateType. Low priority since this is internal.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d8c1c2 and 1e3d2e1.

📒 Files selected for processing (12)

• src/client/ClientGameRunner.ts (11 hunks)
• src/client/InputHandler.ts (1 hunks)
• src/client/graphics/GameRenderer.ts (4 hunks)
• src/client/graphics/layers/PerformanceOverlay.ts (6 hunks)
• src/core/GameRunner.ts (5 hunks)
• src/core/game/GameMap.ts (2 hunks)
• src/core/game/GameView.ts (3 hunks)
• src/core/game/TerrainMapLoader.ts (4 hunks)
• src/core/worker/SharedTileRing.ts (1 hunks)
• src/core/worker/Worker.worker.ts (4 hunks)
• src/core/worker/WorkerClient.ts (3 hunks)
• src/core/worker/WorkerMessages.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (16)

📚 Learning: 2025-11-26T20:49:29.110Z

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.110Z
Learning: In GameView.ts, when usesSharedTileState is true (SAB mode), packedTileUpdates contains unpacked tile references as BigInt(tileRef) only, because all tile state lives in the shared Uint16Array. In non-SAB mode, packedTileUpdates contains packed TileUpdate bigints in the format (tileRef << 16n | state), which must be decoded via updateTile(tu). Therefore, Number(tu) is correct in SAB mode and shifting right by 16 bits would be wrong.

Applied to files:

• src/core/worker/WorkerClient.ts
• src/core/GameRunner.ts
• src/core/worker/SharedTileRing.ts
• src/core/game/GameMap.ts
• src/core/worker/Worker.worker.ts
• src/client/ClientGameRunner.ts
• src/core/game/GameView.ts
• src/core/game/TerrainMapLoader.ts

📚 Learning: 2025-10-20T20:15:28.858Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.

Applied to files:

• src/client/InputHandler.ts
• src/core/GameRunner.ts
• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-10-08T17:14:49.369Z

Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: In `ClientGameRunner.ts`, the `myPlayer` field is always set when `shouldPreventWindowClose()` is called, so the null check in that method is sufficient without needing to fetch it again from `gameView.playerByClientID()`.

Applied to files:

• src/core/GameRunner.ts
• src/client/ClientGameRunner.ts

📚 Learning: 2025-06-02T14:27:37.609Z

Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-08-16T10:52:08.292Z

Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-10-20T11:02:16.969Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-10-27T09:47:26.395Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-10-18T17:54:01.311Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.311Z
Learning: In src/core/execution/FakeHumanExecution.ts, MIRVs and ground attacks should not be mutually exclusive. The considerMIRV() method should not short-circuit maybeAttack() - both actions can occur in the same tick.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-06-22T05:48:19.241Z

Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 786
File: src/client/TerritoryPatternsModal.ts:337-338
Timestamp: 2025-06-22T05:48:19.241Z
Learning: In src/client/TerritoryPatternsModal.ts, the bit shifting operators (<<) used in coordinate calculations with decoder.getScale() are intentional and should not be changed to multiplication. The user scottanderson confirmed this is functioning as intended.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts
• src/core/game/GameView.ts

📚 Learning: 2025-08-12T00:31:50.144Z

Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts
• src/client/ClientGameRunner.ts

📚 Learning: 2025-10-08T17:14:49.369Z

Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.

Applied to files:

• src/client/graphics/layers/PerformanceOverlay.ts

📚 Learning: 2025-10-18T11:00:57.142Z

Learnt from: NewYearNewPhil
Repo: openfrontio/OpenFrontIO PR: 2230
File: src/client/graphics/GameRenderer.ts:269-277
Timestamp: 2025-10-18T11:00:57.142Z
Learning: In src/client/graphics/GameRenderer.ts, the GameRecapCapture implementation does not use setCaptureRenderEnabled on layers. Instead, it uses RecapCaptureSurface.capture() to render capture layers (TerrainLayer, TerritoryLayer, RailroadLayer, StructureIconsLayer, UnitLayer) directly to an off-screen canvas without requiring layer-level capture mode methods.

Applied to files:

• src/client/graphics/GameRenderer.ts

📚 Learning: 2025-06-09T02:20:43.637Z

Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.

Applied to files:

• src/client/graphics/GameRenderer.ts
• src/client/ClientGameRunner.ts

📚 Learning: 2025-08-29T16:16:11.309Z

Learnt from: BrewedCoffee
Repo: openfrontio/OpenFrontIO PR: 1957
File: src/core/execution/PlayerExecution.ts:40-52
Timestamp: 2025-08-29T16:16:11.309Z
Learning: In OpenFrontIO PlayerExecution.ts, when Defense Posts are captured due to tile ownership changes, the intended behavior is to first call u.decreaseLevel() to downgrade them, then still transfer them to the capturing player via captureUnit(). This is not a bug - Defense Posts should be both downgraded and captured, not destroyed outright.

Applied to files:

• src/core/game/GameView.ts

📚 Learning: 2025-10-26T15:37:07.732Z

Learnt from: GlacialDrift
Repo: openfrontio/OpenFrontIO PR: 2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.

Applied to files:

• src/core/game/GameView.ts

📚 Learning: 2025-10-21T20:06:04.823Z

Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.

Applied to files:

• src/core/game/TerrainMapLoader.ts

🧬 Code graph analysis (7)

src/core/worker/WorkerClient.ts (1)

src/core/worker/SharedTileRing.ts (1)

• SharedTileRingBuffers (3-7)

src/client/InputHandler.ts (1)

src/core/EventBus.ts (1)

• GameEvent (1-1)

src/core/GameRunner.ts (5)

src/core/game/GameView.ts (2)

• tile (106-108)
• config (668-670)

src/core/game/UnitImpl.ts (1)

• tile (176-178)

src/core/game/GameMap.ts (1)

• TileRef (3-3)

src/core/game/GameImpl.ts (1)

• config (336-338)

src/core/configuration/ConfigLoader.ts (1)

• getConfig (11-27)

src/core/worker/SharedTileRing.ts (1)

src/core/game/GameMap.ts (1)

• TileRef (3-3)

src/client/graphics/GameRenderer.ts (1)

src/client/InputHandler.ts (1)

• BacklogStatusEvent (148-153)

src/core/game/GameMap.ts (3)

src/core/game/GameView.ts (2)

• width (701-703)
• height (704-706)

src/core/game/GameImpl.ts (2)

• width (807-809)
• height (810-812)

src/client/graphics/TransformHandler.ts (1)

• width (48-50)

src/core/worker/Worker.worker.ts (4)

src/core/worker/SharedTileRing.ts (3)

• SharedTileRingViews (9-14)
• createSharedTileRingViews (31-43)
• pushTileUpdate (45-64)

src/core/worker/WorkerMessages.ts (2)

• MainThreadMessage (115-122)
• InitializedMessage (50-52)

src/core/GameRunner.ts (1)

• createGameRunner (35-95)

src/core/game/GameMap.ts (1)

• TileRef (3-3)

🔇 Additional comments (24)

src/core/game/GameView.ts (2)

475-487: LGTM! Clean SAB flag wiring.

The optional usesSharedTileState parameter with a false default keeps backward compatibility. The flag is stored and used to branch tile update handling correctly.

---

516-525: Correct branching for SAB vs non-SAB tile updates.

Based on learnings, in SAB mode packedTileUpdates contains unpacked tile refs (BigInt(tileRef)), so Number(tu) is correct. In non-SAB mode, it contains packed values requiring updateTile(tu) to decode. The logic is sound.

src/core/game/GameMap.ts (1)

78-103: Well-structured external buffer support with proper validation.

The constructor:

1. Validates stateBuffer length matches width * height
2. Throws a clear error with dimensions if mismatch
3. Falls back to internal allocation when no buffer provided

This enables shared memory without breaking existing code paths.

src/core/worker/WorkerClient.ts (2)

23-29: Clean constructor extension for shared buffers.

Optional parameters for sharedTileRingBuffers, sharedStateBuffer, and sharedDirtyBuffer keep the API backward compatible while enabling SAB mode when available.

---

72-81: Initialization payload correctly passes shared buffers.

The init message extracts header/data from sharedTileRingBuffers and includes state/dirty buffers. The optional chaining handles the non-SAB fallback case.

src/core/worker/WorkerMessages.ts (1)

34-42: Clean InitMessage extension for shared buffers.

Optional SharedArrayBuffer fields allow gradual adoption of SAB mode while keeping the message contract backward compatible.

src/client/graphics/GameRenderer.ts (1)

298-300: Good state additions for backlog-aware rendering.

These fields track backlog state from events and enable frame pacing decisions.

src/client/InputHandler.ts (2)

128-146: TickMetricsEvent extended with useful diagnostics.

The new optional fields for backlog, throughput, and ring buffer stats are well-documented with comments. Optional parameters keep backward compatibility.

---

148-153: Clean BacklogStatusEvent definition.

Simple event with backlogTurns count and backlogGrowing flag. Matches usage in GameRenderer.

src/core/worker/Worker.worker.ts (2)

25-27: Module-level state for turn processing and shared ring.

These globals track the processing state and shared buffer views. Clean initialization.

---

44-65: Verification confirms: executeNextTick() is synchronous.

The web search confirms that executeNextTick() is a synchronous, single-tick game-loop method that runs the deterministic per-tick simulation and returns updates—not a Promise. This means the while loop correctly processes pending turns without await statements, and the reentrancy protection with isProcessingTurns flag combined with the try-finally cleanup is appropriate.

src/core/GameRunner.ts (3)

40-48: Clean API extension for shared buffer support.

The optional parameters tileUpdateSink and sharedStateBuffer are well-typed and properly threaded through to dependent functions. Good use of optional parameters to maintain backward compatibility.

---

183-196: Tile update branching logic is correct.

The extraction of tileRef via Number(u.update >> 16n) correctly unpacks the upper bits from the packed format (tileRef << 16n | state). When using a shared sink, returning an empty BigUint64Array is the right approach since tile state lives in the shared buffer.

---

292-295: Simple and clear pending turns check.

Clean implementation. The method name clearly conveys its purpose.

src/client/graphics/layers/PerformanceOverlay.ts (2)

231-243: Clean event forwarding for new metrics.

All new metric fields are properly extracted from the event and passed to updateTickMetrics.

---

440-468: State properties well-structured.

All new metrics use the @state() decorator correctly, and defaults are sensible. Good organization.

src/core/game/TerrainMapLoader.ts (3)

42-67: Cache and shared buffer logic is well-structured.

The caching bypass when SAB is available and the nullish coalescing for stateBuffer are correctly implemented. Previous review concerns have been addressed.

---

69-76: Verify: Compact maps intentionally skip shared state buffer.

Only normal-sized maps receive the stateBuffer. Compact maps (4x variants) do not get shared state. Please confirm this is the intended behavior - if compact maps should also support SAB mode, the buffer would need to be sized appropriately and passed here as well.

---

95-109: Result construction handles buffer types safely.

Good defensive check with instanceof SharedArrayBuffer before assignment. The caching logic correctly applies only when SAB path is not in use.

src/core/worker/SharedTileRing.ts (2)

3-19: Clean interface definitions for shared ring buffer.

Good use of typed interfaces over class hierarchy. The separation of Buffers (raw SharedArrayBuffer) from Views (typed array views) is a clean pattern for cross-thread data sharing.

---

21-43: Factory functions are straightforward.

Clean separation between buffer allocation and view creation. This allows buffers to be transferred to workers while views are created on each side.

src/client/ClientGameRunner.ts (3)

173-203: Shared buffer initialization is well-structured.

Clean feature detection for SAB/Atomics/crossOriginIsolated. Ring capacity sizing to numTiles provides a reasonable upper bound for tile updates.

---

258-272: Instance fields for batched processing are well-organized.

Using pendingStart as a cursor index into pendingUpdates is a good pattern to avoid costly array shifts. The separation of worker/render tick counters enables accurate throughput metrics.

---

745-756: Backlog tracking is straightforward.

Clean calculation of backlog turns and emission of status events. The backlogGrowing flag provides useful trend information for downstream throttling.

[evanpelle]

I wonder if there's a way to abstract the data transfer behind an interface and then we can pop in SharedArrayBufferTransfer or PostMessageTransfer. this way we can avoid the if(sabEnabled) else ... occurrances

[evanpelle]

I think this should be an interface, it's too easy to mix up arguments with this many parameters.

[scamiv]

most of these can/should be removed after evaluation

[evanpelle]

can all the buffer logic be moved to another file? ClientGameRunner is getting pretty large.

[scamiv]

I do believe that, in the current logic, this belong here. I would consider Refactoring this outside of scope.

[evanpelle]

Is this data relevant to InputHandler?

[scamiv]

Semantically, no, but it does extend TickMetricsEvent which was here before.

Most of this is only for the perf overlay and can be removed after testing.

[evanpelle]

I don't think this belongs in InputHandler either

[evanpelle]

I think this logic should be refactored into a function so it can be reused.

[evanpelle]

could we group these into an interface/object?

[evanpelle]

Would it be possible to break this into multiple PRs? I can't really review a PR this large effectively

[evanpelle]

The territory tiles don't seem to render when i tried running this locally