feature: game recap #2230
feature: game recap #2230
Conversation
|
|
WalkthroughAdds a recap capture and playback system: new GameRecapCapture, RecapCaptureSurface, RecapFrameStore, RecapCaptureConfig, a LitElement Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as GameRenderer
participant Capture as GameRecapCapture
participant Surface as RecapCaptureSurface
participant Store as RecapFrameStore
Renderer->>Capture: tick()
activate Capture
Capture->>Capture: queueCapture / schedule performCapture
Note over Capture: requestAnimationFrame drives capture window and timing
Capture->>Surface: capture(options)
activate Surface
Surface->>Surface: render selected layers to off-screen canvas
Surface-->>Capture: { blob, width, height, imageBitmap? }
deactivate Surface
Capture->>Store: addFrame(frame)
activate Store
Store->>Store: create objectUrl, compact, notify subscribers
Store-->>Capture: subscribers updated
deactivate Store
deactivate Capture
sequenceDiagram
participant Viewer as GameRecapViewer
participant Store as RecapFrameStore
participant Canvas as HTMLCanvasElement
participant Modal as WinModal
participant Recorder as MediaRecorder
Viewer->>Store: subscribe(callback)
Store-->>Viewer: frames (initial + updates)
Viewer->>Canvas: requestAnimationFrame loop (paint currentIndex)
Note over Viewer: loopPauseMs controls hold between loop cycles
User->>Modal: click export
Modal->>Modal: canExportRecap? (MediaRecorder & captureStream)
Modal->>Recorder: start recording canvas stream
Modal->>Store: getFrames()
Store-->>Modal: RecapFrame[]
loop for each frame
Modal->>Canvas: drawImage(frame.objectUrl or ImageBitmap)
Note right of Modal: timing controlled by targetFps / frame durations
end
Modal->>Recorder: stop()
Recorder-->>Modal: dataavailable -> blob
Modal->>User: trigger download
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (16)
src/client/components/GameRecapViewer.ts (2)
66-81: Hide download when export is unsupportedButton shows even when MediaRecorder/captureStream aren’t available; clicking then no-ops. Gate it from parent.
Apply in GameRecapViewer:
export class GameRecapViewer extends LitElement { + @property({ type: Boolean }) canExport: boolean = false; ... - const showDownloadButton = hasFrames; + const showDownloadButton = hasFrames && this.canExport;And in WinModal render:
- <game-recap-viewer + <game-recap-viewer class="block w-full" .frameStore=${frameStore} + .canExport=${this.canExportRecap} @recap-request-export=${this.onExportRecapWebM} ></game-recap-viewer>
220-241: Sharper rendering: disable image smoothingViewer canvas currently uses browser default smoothing; pixel art/low-scale frames blur. Disable smoothing on the 2D context.
Apply:
const context = this.canvas.getContext("2d"); if (!context) { return; } + context.imageSmoothingEnabled = false;src/client/graphics/recapCapture/RecapCaptureConfig.ts (1)
14-29: PNG + large frames = heavy memory; consider WebP and/or union-typed image configDefaults (1920×1080, maxFrames=900, PNG) can consume hundreds of MiB. Either reduce resolution/frame cap, or prefer WebP/JPEG with quality. Also make image encoding a discriminated union for safer configs.
Example types:
type ImageEncoding = | { type: "png" } | { type: "webp"; quality: number } | { type: "jpeg"; quality: number }; export interface RecapCaptureConfig { ... - imageMimeType: string; - imageQuality?: number; + image: ImageEncoding; ... }And default:
-export const defaultRecapCaptureConfig = { imageMimeType: "image/png", imageQuality: 1, ... } +export const defaultRecapCaptureConfig = { image: { type: "webp", quality: 0.92 }, ... }You can map ImageEncoding to toBlob mime/quality at the call site.
src/client/graphics/recapCapture/RecapCaptureSurface.ts (2)
36-51: Guard constructor for non‑DOM environmentsThis uses document at construction. If imported in SSR/tests without DOM, it will throw before the current document check. Lazily create the canvas or guard earlier.
Apply:
- constructor() { - this.canvas = document.createElement("canvas"); + constructor() { + if (typeof document === "undefined") { + throw new Error("RecapCaptureSurface requires a DOM environment"); + } + this.canvas = document.createElement("canvas"); ... - if (typeof document !== "undefined" && document.body) { - document.body.appendChild(this.canvas); - } + document.body?.appendChild(this.canvas);
84-103: Transform save/restore: keep stack balanced on exceptionsYou already catch per-layer errors; good. Minor: ensure a final restore when transformActive is true even if an exception happens before toggling back.
Wrap layer loop with try/finally for the active transform:
- let transformActive = false; - for (const layer of options.layers) { + let transformActive = false; + try { + for (const layer of options.layers) { ... - } - const worldTransform = this.context.getTransform(); - if (transformActive) { - this.context.restore(); - } + } + } finally { + const worldTransform = this.context.getTransform(); + if (transformActive) this.context.restore(); + }src/client/graphics/layers/WinModal.ts (3)
128-142: Pass export capability to the viewerForward canExportRecap to the viewer so it can hide the button when unsupported.
Apply:
- <game-recap-viewer + <game-recap-viewer class="block w-full" .frameStore=${frameStore} + .canExport=${this.canExportRecap} @recap-request-export=${this.onExportRecapWebM} ></game-recap-viewer>
57-60: Remove empty disconnectedCallback overrideNo custom logic; safe to delete.
- disconnectedCallback(): void { - super.disconnectedCallback(); - }
152-187: Disable button while exporting; consider user feedbackexportingRecap is tracked but not reflected in UI. Expose a prop to the viewer to disable the button or show a spinner.
In WinModal:
- @state() private exportingRecap = false; + @state() private exportingRecap = false;In renderRecapSection:
- .canExport=${this.canExportRecap} + .canExport=${this.canExportRecap} + .disabled=${this.exportingRecap}And in GameRecapViewer, add @Property({type:Boolean}) disabled and set the button disabled class/attr accordingly.
src/client/graphics/GameRenderer.ts (2)
356-359: Avoid stalling the main render loop during captureYou skip a full frame whenever a capture is in progress. Capture runs on an off-screen canvas; stalling here increases visible jank. Let the render proceed unless you’ve observed contention.
Apply:
- if (this.recapCapture?.isCapturing()) { - requestAnimationFrame(() => this.renderGame()); - return; - } + // Keep rendering; recap capture is off-screen.If needed, reduce capture frequency instead (e.g., increase captureEveryNTicks) or run capture on requestIdleCallback when available.
278-295: Consider configurability for capture defaultsHardcoded recapCapture with defaults may be heavy on low-end devices. Allow passing a Partial based on user settings (resolution cap, frame cap).
Example:
- const recapCapture = new GameRecapCapture(game, transformHandler, captureLayers); + const recapCapture = new GameRecapCapture( + game, + transformHandler, + captureLayers, + { targetWidth: 1280, targetHeight: 720, maxFrames: 600 } + );src/client/graphics/recapCapture/GameRecapCapture.ts (4)
365-377: Capture overrides may affect live layers — please confirm the contractsetLayerCaptureOverrides toggles per-layer mode globally for the session. If it also changes the on-screen renderer, consider scoping it per capture:
- Enable before performCapture
- Disable in the finally block of queueCapture
Can you confirm setCaptureRenderEnabled only alters the capture path and not the live renderer? If not, I’ll provide a scoped toggle patch.
428-453: Account for devicePixelRatio and clamp target sizeFor better quality and bounded memory:
- Multiply by window.devicePixelRatio when deriving width/height
- Clamp to a sane max (e.g., 2560×1440) to avoid huge PNGs/WebPs
Example:
- const width = Math.max(1, Math.round(mapWidth * scale)); - const height = Math.max(1, Math.round(mapHeight * scale)); + const dpr = typeof window !== "undefined" ? Math.max(1, Math.round(window.devicePixelRatio || 1)) : 1; + const width = Math.max(1, Math.round(mapWidth * scale * dpr)); + const height = Math.max(1, Math.round(mapHeight * scale * dpr)); + const MAX_W = 2560, MAX_H = 1440; + const clampedW = Math.min(width, MAX_W); + const clampedH = Math.min(height, MAX_H); - if ( - !this.viewport || - this.viewport.width !== width || - this.viewport.height !== height - ) { - this.viewport = { width, height }; - } + if (!this.viewport || this.viewport.width !== clampedW || this.viewport.height !== clampedH) { + this.viewport = { width: clampedW, height: clampedH }; + }
107-110: Expose a simple capability check for UI gatingAdd a public canExport() so the UI can disable the download button on unsupported browsers without trying export.
isCapturing(): boolean { return this.captureInProgress; } + + canExport(): boolean { + return this.resolveExportMimeType() !== null; + }Also applies to: 411-426
287-295: PNG ignores quality; consider WebP or JPEG for smaller blobstoBlob quality is ignored for image/png. Consider image/webp (with quality ~0.85) for much smaller memory footprint and faster export, or make mimeType configurable per environment.
src/client/graphics/recapCapture/RecapFrameStore.ts (2)
37-39: Avoid exposing mutable array references to subscribersReturning and emitting this.frames lets callers mutate your internal state. Return a copy (or a frozen view).
getFrames(): readonly RecapFrame[] { - return this.frames; + return this.frames.slice(); } @@ subscribe(subscriber: Subscriber): () => void { this.subscribers.add(subscriber); - subscriber(this.frames); + subscriber(this.frames.slice()); return () => { this.subscribers.delete(subscriber); }; } @@ private notify() { - for (const subscriber of this.subscribers) { - subscriber(this.frames); - } + const snapshot = this.frames.slice(); + for (const subscriber of this.subscribers) subscriber(snapshot); }Also applies to: 89-95
103-115: Respect per-frame MIME type during serializationUsing a single mimeType argument can lie about content. Prefer frame.blob.type and make the parameter optional.
- async serializeFrames(mimeType: string): Promise<SerializableRecapFrame[]> { + async serializeFrames(mimeType?: string): Promise<SerializableRecapFrame[]> { @@ - serialized.push({ + serialized.push({ tick: frame.tick, capturedAt: frame.capturedAt, - mimeType, + mimeType: mimeType ?? frame.blob.type, dataUrl, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/client/components/GameRecapViewer.ts(1 hunks)src/client/graphics/GameRenderer.ts(8 hunks)src/client/graphics/layers/WinModal.ts(7 hunks)src/client/graphics/recapCapture/GameRecapCapture.ts(1 hunks)src/client/graphics/recapCapture/RecapCaptureConfig.ts(1 hunks)src/client/graphics/recapCapture/RecapCaptureSurface.ts(1 hunks)src/client/graphics/recapCapture/RecapFrameStore.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/client/graphics/recapCapture/RecapCaptureSurface.ts (2)
src/client/graphics/layers/Layer.ts (1)
Layer(1-7)src/client/graphics/TransformHandler.ts (1)
TransformHandler(15-295)
src/client/graphics/recapCapture/GameRecapCapture.ts (6)
src/client/graphics/recapCapture/RecapCaptureConfig.ts (2)
RecapCaptureConfig(1-12)defaultRecapCaptureConfig(14-29)src/client/graphics/recapCapture/RecapCaptureSurface.ts (2)
RecapCaptureSurface(31-163)capture(53-133)src/client/graphics/recapCapture/RecapFrameStore.ts (2)
RecapFrameStore(20-139)RecapFrame(1-9)src/client/graphics/TransformHandler.ts (1)
TransformHandler(15-295)src/client/graphics/layers/Layer.ts (1)
Layer(1-7)src/client/graphics/GameRenderer.ts (1)
tick(408-411)
src/client/components/GameRecapViewer.ts (1)
src/client/graphics/recapCapture/RecapFrameStore.ts (2)
RecapFrameStore(20-139)RecapFrame(1-9)
src/client/graphics/layers/WinModal.ts (1)
src/client/graphics/recapCapture/GameRecapCapture.ts (1)
GameRecapCapture(23-454)
src/client/graphics/GameRenderer.ts (6)
src/client/graphics/layers/TerrainLayer.ts (1)
TerrainLayer(6-77)src/client/graphics/layers/TerritoryLayer.ts (1)
TerritoryLayer(19-575)src/client/graphics/layers/RailroadLayer.ts (1)
RailroadLayer(22-218)src/client/graphics/layers/StructureIconsLayer.ts (1)
StructureIconsLayer(57-616)src/client/graphics/layers/UnitLayer.ts (1)
UnitLayer(30-558)src/client/graphics/recapCapture/GameRecapCapture.ts (1)
GameRecapCapture(23-454)
| ${!hasFrames | ||
| ? html`<div | ||
| class="absolute inset-0 flex items-center justify-center text-sm text-white/70" | ||
| > | ||
| ${"Recap capture warming up"} | ||
| </div>` | ||
| : null} |
There was a problem hiding this comment.
Localize the warming message
Hardcoded text breaks i18n. Use translateText (and add a key in en.json) or accept a property for custom text.
Apply:
+import { translateText } from "../Utils";
...
- ${"Recap capture warming up"}
+ ${translateText("recap.warming")}Also add "recap.warming": "Recap capture warming up" to en.json.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ${!hasFrames | |
| ? html`<div | |
| class="absolute inset-0 flex items-center justify-center text-sm text-white/70" | |
| > | |
| ${"Recap capture warming up"} | |
| </div>` | |
| : null} | |
| ${!hasFrames | |
| ? html`<div | |
| class="absolute inset-0 flex items-center justify-center text-sm text-white/70" | |
| > | |
| ${translateText("recap.warming")} | |
| </div>` | |
| : null} |
🤖 Prompt for AI Agents
In src/client/components/GameRecapViewer.ts around lines 82-88 the "Recap
capture warming up" string is hardcoded which breaks i18n; replace the literal
with a call to translateText('recap.warming') (or accept a prop like warmingText
and default it to translateText('recap.warming')) and add the key
"recap.warming": "Recap capture warming up" to en.json; ensure imports include
translateText if needed and update the component prop types/defaults when using
a prop.
| const handlerInternals = this | ||
| .transformHandler as unknown as TransformSnapshot & { | ||
| _boundingRect?: DOMRect; | ||
| centerAll(fit?: number): void; | ||
| override(x?: number, y?: number, scale?: number): void; | ||
| }; | ||
|
|
||
| const originalTransform = this.snapshotTransform(); | ||
| const originalBoundingRect = handlerInternals._boundingRect; | ||
|
|
||
| const captureRect = | ||
| typeof DOMRect === "function" | ||
| ? new DOMRect(0, 0, this.viewport.width, this.viewport.height) | ||
| : ({ | ||
| x: 0, | ||
| y: 0, | ||
| width: this.viewport.width, | ||
| height: this.viewport.height, | ||
| top: 0, | ||
| right: this.viewport.width, | ||
| bottom: this.viewport.height, | ||
| left: 0, | ||
| } as DOMRect); | ||
| handlerInternals._boundingRect = captureRect; | ||
| this.transformHandler.centerAll(1); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid mutating private TransformHandler internals; add a compositional API
Directly poking _boundingRect and re-centering via centerAll/override is brittle and risks visible camera flicker. Prefer a compositional method on TransformHandler, e.g. withTemporaryViewport(rect, fn) that computes transform for the given rect without mutating live state, or a pure getTransformForViewport(rect) that handleTransform can use.
If you agree, I can draft a minimal change: add TransformHandler.withTemporaryViewport(rect, fn) and change performCapture to call it; no private fields or state leaks. Based on learnings
🤖 Prompt for AI Agents
In src/client/graphics/recapCapture/GameRecapCapture.ts around lines 260-285,
the code mutates private TransformHandler internals (_boundingRect) and calls
centerAll which can cause brittle state leaks and visible camera flicker;
replace this by adding a compositional API on the TransformHandler (preferred:
withTemporaryViewport(rect, fn) that computes and applies a temporary transform
for the duration of fn without mutating live state, or alternatively a pure
getTransformForViewport(rect) that returns the transform to use) and update
performCapture to call that API with a constructed captureRect and perform the
snapshot inside the callback so no private fields are written and the live
transform is never altered or left inconsistent. Ensure original transform is
captured via existing snapshotTransform and restored automatically by the new
API or manually after the callback, and remove direct access to _boundingRect,
centerAll, and override from this file.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/client/graphics/recapCapture/GameRecapCapture.ts (1)
260-284: Direct mutation of private TransformHandler internals remains.This code still directly accesses
_boundingRect, mutates it (line 283), and callscenterAll(), which can cause brittle state leaks and potential camera flicker. The previous review suggested adding a compositional API onTransformHandler(e.g.,withTemporaryViewport(rect, fn)) to avoid exposing private fields.
🧹 Nitpick comments (2)
src/client/graphics/recapCapture/RecapCaptureConfig.ts (1)
1-12: Consider adding runtime validation or branded types for config constraints.The interface allows potentially invalid values (e.g., negative
maxFrames,imageQualityoutside 0–1, emptyexportMimeTypes). While the default config is safe, consumers passingPartial<RecapCaptureConfig>could introduce invalid values. Add validation inGameRecapCaptureconstructor or use TypeScript branded types to constrain ranges at compile time.src/client/graphics/recapCapture/GameRecapCapture.ts (1)
35-44: Validate merged config values after merge.The constructor merges user-supplied config without validating constraints. Invalid values (e.g.,
maxFrames: -1,exportFps: 0) could cause issues downstream. Add validation after line 41.Apply this diff to add basic validation:
constructor( private readonly game: GameView, private readonly transformHandler: TransformHandler, private readonly layers: Layer[], config?: Partial<RecapCaptureConfig>, ) { this.config = { ...defaultRecapCaptureConfig, ...config }; + if (this.config.maxFrames <= 0 || this.config.captureEveryNTicks <= 0) { + throw new Error("RecapCapture: maxFrames and captureEveryNTicks must be positive"); + } + if (this.config.imageQuality !== undefined && (this.config.imageQuality < 0 || this.config.imageQuality > 1)) { + throw new Error("RecapCapture: imageQuality must be between 0 and 1"); + } this.surface = new RecapCaptureSurface(); this.frameStore = new RecapFrameStore(this.config.maxFrames); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/client/graphics/recapCapture/GameRecapCapture.ts(1 hunks)src/client/graphics/recapCapture/RecapCaptureConfig.ts(1 hunks)src/client/graphics/recapCapture/RecapFrameStore.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client/graphics/recapCapture/RecapFrameStore.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/client/graphics/recapCapture/GameRecapCapture.ts (6)
src/client/graphics/recapCapture/RecapCaptureConfig.ts (2)
RecapCaptureConfig(1-12)defaultRecapCaptureConfig(14-29)src/client/graphics/recapCapture/RecapCaptureSurface.ts (1)
RecapCaptureSurface(31-163)src/client/graphics/recapCapture/RecapFrameStore.ts (2)
RecapFrameStore(20-149)RecapFrame(1-9)src/client/graphics/TransformHandler.ts (1)
TransformHandler(15-295)src/client/graphics/layers/Layer.ts (1)
Layer(1-7)src/client/graphics/GameRenderer.ts (1)
tick(408-411)
🔇 Additional comments (9)
src/client/graphics/recapCapture/RecapCaptureConfig.ts (1)
14-29: Default config values look reasonable.The low
exportFps: 10keeps file sizes manageable at the cost of choppiness, which aligns with the PR description noting slow generation. All other defaults are sensible for a 1920×1080 recap.src/client/graphics/recapCapture/GameRecapCapture.ts (8)
1-21: Clean imports and type definitions.The
TransformSnapshotinterface provides type safety for transform snapshotting, andRecapCaptureStatsoffers a clean public API surface.
46-55: State reset now properly implemented.The
start()method now correctly resetslastCaptureTick(line 49) and clears old frames (line 54), addressing the previous review concern about stale state between sessions.
57-67: Proper cleanup and lifecycle handling.The
dispose()andonViewportResize()methods correctly manage resource cleanup and viewport recalculation.
69-79: Clean public API for frame store and stats access.The getter methods provide a straightforward interface for external consumers to access frame data and capture statistics.
81-102: Tick scheduling logic is well-structured.The method correctly guards against stopped state, uninitialized viewport, non-advancing ticks, off-interval ticks, and concurrent captures. The
lastCaptureTickreset instart()ensures proper behavior across game sessions.
104-119: Stop logic correctly handles final frame capture.The
stopCapturing()method properly queues a final capture whether a capture is in-flight (viapendingFinalCapture) or idle (via immediatequeueCapture), ensuring the last frame is recorded.
121-252: WebM export logic is now robust with proper fps validation.The method now guards against invalid
targetFps(lines 125-128), addressing the previous review concern. The single-frame edge case (line 212), blob size validation (lines 247-249), and proper MediaRecorder lifecycle handling all contribute to a solid export implementation.
343-435: Private helper methods are clean and well-implemented.The helpers provide solid implementations with appropriate fallbacks (
requestAnimationFrame/setTimeout), proper resource cleanup (object URL revocation), result caching (MIME type resolution), and boundary validation (viewport sizing).
|
Fixes #1748 (linking to Issue by using the 'fixes' word) |
|
Unfortunately that only works if it's in the original post on the pull request. I've linked it manually instead. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/client/graphics/layers/WinModal.ts (2)
57-59: Remove empty lifecycle override.The
disconnectedCallbackoverride only callssuper.disconnectedCallback()without any additional logic. This can be safely removed.Apply this diff:
- disconnectedCallback(): void { - super.disconnectedCallback(); - } -
152-187: Redundant environment check.Line 166 checks
typeof document === "undefined", but this is redundant since the code at line 171 already usesdocument.createElement()without checking. If we reached this point through the UI, we're guaranteed to be in a browser environment.Apply this diff to simplify:
console.info("[RecapCapture] Manual export requested"); try { const { blob, filename } = await this.recapCapture.exportAsWebM(); - if (typeof document === "undefined") { - console.warn("Recap export is unavailable in this environment"); - return; - } const url = URL.createObjectURL(blob); const anchor = document.createElement("a");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/client/graphics/GameRenderer.ts(8 hunks)src/client/graphics/layers/WinModal.ts(7 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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/layers/WinModal.tssrc/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/layers/WinModal.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/client/graphics/GameRenderer.ts
🧬 Code graph analysis (2)
src/client/graphics/layers/WinModal.ts (1)
src/client/graphics/recapCapture/GameRecapCapture.ts (1)
GameRecapCapture(23-436)
src/client/graphics/GameRenderer.ts (6)
src/client/graphics/layers/TerrainLayer.ts (1)
TerrainLayer(6-77)src/client/graphics/layers/TerritoryLayer.ts (1)
TerritoryLayer(25-637)src/client/graphics/layers/RailroadLayer.ts (1)
RailroadLayer(22-218)src/client/graphics/layers/StructureIconsLayer.ts (1)
StructureIconsLayer(57-627)src/client/graphics/layers/UnitLayer.ts (1)
UnitLayer(30-558)src/client/graphics/recapCapture/GameRecapCapture.ts (1)
GameRecapCapture(23-436)
| const recapCapture = new GameRecapCapture( | ||
| game, | ||
| transformHandler, | ||
| captureLayers, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify recap capture disposal.
GameRecapCapture has a dispose() method that cleans up resources (surface, frameStore), but I don't see it being called anywhere in the GameRenderer lifecycle. Consider adding disposal logic when the renderer is destroyed to prevent memory leaks.
Run the following to check if disposal is handled elsewhere:
🏁 Script executed:
#!/bin/bash
# Search for recapCapture disposal calls
rg -n 'recapCapture.*dispose' --type ts -C3
# Check for GameRenderer cleanup/destroy methods
rg -n 'class GameRenderer' --type ts -A50 | rg -P '(dispose|destroy|cleanup|teardown)\s*\('Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for GameRenderer class structure and any cleanup patterns
rg -n 'class GameRenderer' --type ts -A100 | head -120
# Search for dispose method in GameRecapCapture
rg -n 'dispose\s*\(' --type ts -B5 -A10 | grep -A15 'GameRecapCapture'
# Check if recapCapture is stored as a field in GameRenderer
rg -n 'this\.recapCapture' --type ts -C2Length of output: 12022
Add disposal cleanup to GameRenderer lifecycle.
recapCapture is stored as a private field and started in initialize() (line 328), but GameRecapCapture.dispose() is never called. This leaks the off-screen surface and frame storage. Add a cleanup method to GameRenderer that calls this.recapCapture?.dispose() and invoke it when the renderer is destroyed.
🤖 Prompt for AI Agents
In src/client/graphics/GameRenderer.ts around lines 287–291, the
GameRecapCapture instance created and stored on the renderer is started in
initialize() (around line 328) but never disposed; add a cleanup step by
implementing a GameRenderer.dispose() or destroy() method that calls
this.recapCapture?.dispose() and sets this.recapCapture = undefined/null, then
ensure that method is invoked whenever the renderer is torn down (e.g., in
existing destroy/unmount logic or where the renderer is removed) so the
off-screen surface and frames are released.
4 participants
Description:
Adds a timelapse recap of the entire game to the win modal. Frames are stored during gameplay and displayed in a loop at the end (or death).
There's also a "download" button in the recap viewer that creates a
.webmof the recap, but it takes a while to generate the video in an acceptable quality, so maybe it should be removed again. I also didn't have a good icon at hand for that button, so I've added an emoji instead.Screencast.From.2025-10-18.12-09-48.webm
The downloaded video:
openfront-recap-1760782396447.webm
Another limitation is that I'm not including all layers. Some of them could be included but make the output a bit messier (transport ships etc.), others would be way more complicated because they render as HTML (such as the NameLayer).
Please complete the following:
not sure what tests are needed here if any, happy for input
Please put your Discord username so you can be contacted if a bug or regression is found:
newyearnewphil / [CYN] super mario