Refactor territory rendering to use WebGPU #2916
Draft
+1,447
−752
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replace Canvas 2D-based territory rendering with a WebGPU implementation for improved performance and scalability. Major changes: - Remove TerrainLayer: territory and terrain now rendered together in WebGPU - Add TerritoryWebGLRenderer: new WebGPU-based renderer (1162 lines) - GPU-authoritative state management with compute shaders - Separate uniform buffers for compute and render passes - Efficient incremental updates via scatter/gather compute passes - Full rebuild compute pass for palette/theme changes - Add defense post management: - Track defended state in GameMap (bit 12 in tile state) - Update defended tiles when defense posts are added/removed/moved - Update defended state when tiles change ownership - Extract hover detection into HoverInfo utility: - Centralized logic for player/unit/wilderness detection - Used by PlayerInfoOverlay for cleaner separation of concerns - Canvas architecture changes: - Main canvas now transparent (alpha: true) - WebGPU canvas renders background and territory - Overlay canvas renders UI elements on top - Performance optimizations: - Compute shaders run at simulation rate (tick), not frame rate - Incremental tile updates via pending tiles set - Palette signature tracking to avoid unnecessary rebuilds - Defense posts signature tracking for efficient updates Files changed: - 10 files changed, 1447 insertions(+), 752 deletions(-) - New: TerritoryWebGLRenderer.ts, HoverInfo.ts - Removed: TerrainLayer.ts - Modified: TerritoryLayer.ts (simplified from 710 to 250 lines)
Contributor
WalkthroughThis change introduces WebGPU-based territory rendering, removes the old TerrainLayer implementation, adds a centralized hover information system, implements tile defended state tracking via bit flags, and refactors rendering pipeline coordination between components. Changes
Sequence DiagramssequenceDiagram
participant Renderer as GameRenderer
participant GPU as TerritoryWebGLRenderer
participant Canvas as WebGPU Canvas
participant Theme as Theme
Renderer->>Renderer: Initialize with TerritoryWebGLRenderer
Renderer->>GPU: create(game, theme)
GPU->>GPU: configureContext (WebGPU device, queue)
GPU->>GPU: createGpuResources (textures, buffers, pipelines)
GPU->>Canvas: Obtain canvas element
GPU-->>Renderer: Return renderer instance
loop Per Frame
Renderer->>GPU: tick() - sync state updates
GPU->>GPU: Run compute passes (scatter, clear, update defended)
GPU->>GPU: Upload tile updates and defense posts to GPU
Renderer->>GPU: render() - execute render pass
GPU->>Canvas: Draw territory with colors and overlays
Canvas-->>Renderer: Rendered frame displayed
end
sequenceDiagram
participant Mouse as User Input
participant Overlay as PlayerInfoOverlay
participant HoverInfo as getHoverInfo
participant GameView as GameView
participant TileMap as Tile Map
Mouse->>Overlay: Mouse move to worldCoord
Overlay->>HoverInfo: getHoverInfo(game, worldCoord)
HoverInfo->>GameView: Validate coordinate
HoverInfo->>TileMap: Get tile and owner
alt Owner is Player
HoverInfo->>HoverInfo: Set info.player
else Owner exists, not player, is land
HoverInfo->>GameView: Check hasFallout(tile)
HoverInfo->>HoverInfo: Set isWilderness/isIrradiatedWilderness
else Not land
HoverInfo->>TileMap: Find nearby units (Warship, TradeShip, TransportShip)
HoverInfo->>HoverInfo: Sort by distance, set closest to info.unit
end
HoverInfo-->>Overlay: Return HoverInfo object
Overlay->>Overlay: maybeShow() - display based on info properties
Overlay-->>Mouse: Show player/wilderness/unit overlay
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/client/graphics/layers/PlayerInfoOverlay.ts`:
- Around line 101-115: When loading the async profile in PlayerInfoOverlay,
guard against stale promises by capturing the specific player reference before
awaiting profile() and only assigning the result if the overlay still references
that same player; in the block where you set this.player and call
this.player.profile().then(...), change to save the player in a local const
(e.g. const player = info.player), set this.player = player, then in the .then
callback check if this.player === player before setting this.playerProfile and
calling this.setVisible(true) (or move setVisible earlier as needed) so an older
promise cannot overwrite a newer hover's profile.
In `@src/client/graphics/layers/TerritoryLayer.ts`:
- Around line 85-92: The configureRenderer method currently throws when
TerritoryWebGLRenderer.create returns no renderer; instead, stop throwing and
return early so non-WebGPU clients don't crash: call
TerritoryWebGLRenderer.create(this.game, this.theme), and if renderer is falsy
simply set this.territoryRenderer = null (optionally log reason) and return;
leave renderLayer and GameRenderer.redraw to continue handling the null renderer
case—do not propagate an exception from configureRenderer.
In `@src/client/graphics/layers/TerritoryWebGLRenderer.ts`:
- Around line 150-174: The init() method currently swallows failures; wrap the
adapter/device/context creation and subsequent
configureContext()/createGpuResources() calls in a try/catch and surface
failures by setting a new instance property (e.g. this.initError) and ensuring
this.ready is false on error; specifically, in init() check
adapter/device/context creation and if any are null assign this.initError = new
Error("...") (with descriptive text), set this.ready = false, and return (or
rethrow) — and in the catch block assign this.initError = err, this.ready =
false, and optionally rethrow or emit an event so callers can react; keep
assignments to this.device and this.context only after successful creation and
call configureContext() and createGpuResources() only on success.
In `@src/core/game/GameView.ts`:
- Around line 886-888: The setDefended method is declared to return void but
uses an unnecessary return; remove the "return" keyword so setDefended(ref:
TileRef, value: boolean): void simply calls this._map.setDefended(ref,
value);—locate the method named setDefended in GameView and change the
implementation to a plain call to this._map.setDefended without returning its
value.
🧹 Nitpick comments (3)
src/client/graphics/HoverInfo.ts (1)
12-30: Avoid full unit scan + sort on every hover.You can use the unit grid to fetch nearby ships and pick the closest without
sqrtor sorting.♻️ Suggested refactor
@@ -function euclideanDistWorld( - coord: { x: number; y: number }, - tileRef: TileRef, - game: GameView, -): number { - const x = game.x(tileRef); - const y = game.y(tileRef); - const dx = coord.x - x; - const dy = coord.y - y; - return Math.sqrt(dx * dx + dy * dy); -} - -function distSortUnitWorld(coord: { x: number; y: number }, game: GameView) { - return (a: UnitView, b: UnitView) => { - const distA = euclideanDistWorld(coord, a.tile(), game); - const distB = euclideanDistWorld(coord, b.tile(), game); - return distA - distB; - }; -} @@ if (!game.isLand(tile)) { - const units = game - .units(UnitType.Warship, UnitType.TradeShip, UnitType.TransportShip) - .filter((u) => euclideanDistWorld(worldCoord, u.tile(), game) < 50) - .sort(distSortUnitWorld(worldCoord, game)); - - if (units.length > 0) { - info.unit = units[0]; - } + const nearby = game.nearbyUnits(tile, 50, [ + UnitType.Warship, + UnitType.TradeShip, + UnitType.TransportShip, + ]); + let closest: UnitView | null = null; + let best = Infinity; + for (const { unit, distSquared } of nearby) { + if (distSquared < best) { + best = distSquared; + closest = unit; + } + } + if (closest) { + info.unit = closest; + } }Also applies to: 61-69
src/client/graphics/layers/TerritoryLayer.ts (1)
81-104: Dispose old GPU renderer on redraw.
configureRenderer()replaces the renderer but doesn’t release old GPU resources. Consider adestroy()on the renderer and call it before swapping.🧹 Suggested cleanup hook
- this.territoryRenderer = renderer; + this.territoryRenderer?.destroy?.(); + this.territoryRenderer = renderer;src/client/graphics/layers/TerritoryWebGLRenderer.ts (1)
371-395: Use a larger workgroup for scatter updates.
workgroup_size(1)makes many tiny dispatches. A larger size reduces overhead for large update sets.⚡ Suggested change
-@compute `@workgroup_size`(1) +@compute `@workgroup_size`(64) fn main(`@builtin`(global_invocation_id) globalId: vec3<u32>) { @@ - scatterPass.dispatchWorkgroups(numUpdates); + const workgroups = Math.ceil(numUpdates / 64); + scatterPass.dispatchWorkgroups(workgroups);Also applies to: 1091-1095
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/client/graphics/GameRenderer.tssrc/client/graphics/HoverInfo.tssrc/client/graphics/TransformHandler.tssrc/client/graphics/layers/PlayerInfoOverlay.tssrc/client/graphics/layers/TerrainLayer.tssrc/client/graphics/layers/TerritoryLayer.tssrc/client/graphics/layers/TerritoryWebGLRenderer.tssrc/core/game/GameImpl.tssrc/core/game/GameMap.tssrc/core/game/GameView.ts
💤 Files with no reviewable changes (1)
- src/client/graphics/layers/TerrainLayer.ts
🧰 Additional context used
🧠 Learnings (12)
📓 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.
Learnt from: devalnor
Repo: openfrontio/OpenFrontIO PR: 1248
File: src/client/graphics/layers/TerritoryInfoLayer.ts:20-20
Timestamp: 2025-06-22T21:51:14.990Z
Learning: In TerritoryInfoLayer.ts, the highlightedTerritory field uses both null and undefined intentionally: undefined represents initial state or inactive layer (Ctrl released), while null represents active layer with no territory being highlighted at cursor position. This distinction is important for proper state change detection.
📚 Learning: 2026-01-12T21:37:01.156Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2874
File: src/server/MapLandTiles.ts:7-11
Timestamp: 2026-01-12T21:37:01.156Z
Learning: In this repository's OpenFrontIO deployment, inter-service HTTP calls to the master should target http://localhost:3000 (master at port 3000) as the canonical address. Apply this as the standard for all server-side TypeScript code that communicates with the master. Avoid hardcoding non-master URLs; centralize the master address (e.g., via config or env) when possible, and ensure internal service communication uses localhost:3000 in this architecture.
Applied to files:
src/client/graphics/TransformHandler.tssrc/client/graphics/HoverInfo.tssrc/core/game/GameView.tssrc/client/graphics/GameRenderer.tssrc/core/game/GameMap.tssrc/client/graphics/layers/PlayerInfoOverlay.tssrc/client/graphics/layers/TerritoryLayer.tssrc/core/game/GameImpl.tssrc/client/graphics/layers/TerritoryWebGLRenderer.ts
📚 Learning: 2025-11-26T20:49:29.140Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.140Z
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/GameView.tssrc/core/game/GameMap.tssrc/client/graphics/layers/PlayerInfoOverlay.tssrc/core/game/GameImpl.tssrc/client/graphics/layers/TerritoryWebGLRenderer.ts
📚 Learning: 2026-01-13T20:16:05.535Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2887
File: src/core/execution/NukeExecution.ts:118-122
Timestamp: 2026-01-13T20:16:05.535Z
Learning: In code paths that return a Player-like object, prefer returning a union type (Player | TerraNullius) instead of undefined. When a function may fail to find a player, return TerraNullius for the 'not found' case and a Player for valid IDs, and check .isPlayer() (or equivalent) directly on the result instead of guarding with undefined or optional chaining. This should be enforced in Game, GameImpl, and GameView (and similar accessors) to avoid undefined checks and simplify null-safety handling.
Applied to files:
src/core/game/GameView.tssrc/client/graphics/GameRenderer.tssrc/core/game/GameMap.tssrc/core/game/GameImpl.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.tssrc/client/graphics/layers/TerritoryLayer.tssrc/client/graphics/layers/TerritoryWebGLRenderer.ts
📚 Learning: 2025-12-13T14:58:29.645Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2607
File: src/core/execution/PlayerExecution.ts:271-295
Timestamp: 2025-12-13T14:58:29.645Z
Learning: In src/core/execution/PlayerExecution.ts surroundedBySamePlayer(), the `as Player` cast on `mg.playerBySmallID(scan.enemyId)` is intentional. Since scan.enemyId comes from ownerID() on an owned tile and playerBySmallID() only returns Player or undefined, the cast expresses a known invariant. The maintainers prefer loud failures (runtime errors) over silent masking (early returns with guards) for corrupted game state scenarios at trusted call sites.
Applied to files:
src/core/game/GameMap.tssrc/client/graphics/layers/PlayerInfoOverlay.ts
📚 Learning: 2025-05-21T04:10:33.435Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as a type inferred from a Zod schema that is marked as optional, which means PlayerStats already includes undefined as a possible type (PlayerStats | undefined).
Applied to files:
src/core/game/GameMap.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/PlayerInfoOverlay.ts
📚 Learning: 2025-06-22T21:51:14.990Z
Learnt from: devalnor
Repo: openfrontio/OpenFrontIO PR: 1248
File: src/client/graphics/layers/TerritoryInfoLayer.ts:20-20
Timestamp: 2025-06-22T21:51:14.990Z
Learning: In TerritoryInfoLayer.ts, the highlightedTerritory field uses both null and undefined intentionally: undefined represents initial state or inactive layer (Ctrl released), while null represents active layer with no territory being highlighted at cursor position. This distinction is important for proper state change detection.
Applied to files:
src/client/graphics/layers/PlayerInfoOverlay.tssrc/client/graphics/layers/TerritoryLayer.tssrc/client/graphics/layers/TerritoryWebGLRenderer.ts
📚 Learning: 2026-01-13T20:16:20.098Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2887
File: src/core/execution/NukeExecution.ts:118-122
Timestamp: 2026-01-13T20:16:20.098Z
Learning: In the OpenFrontIO codebase, `playerBySmallID(id: number)` in Game, GameImpl, and GameView explicitly returns `Player | TerraNullius` (or `PlayerView | TerraNullius`), never undefined. The method returns TerraNullius for ID 0 and Player otherwise. Code should check `.isPlayer()` directly without undefined guards or optional chaining.
Applied to files:
src/client/graphics/layers/PlayerInfoOverlay.ts
📚 Learning: 2025-11-29T22:22:37.178Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2539
File: src/core/game/GameImpl.ts:520-542
Timestamp: 2025-11-29T22:22:37.178Z
Learning: In GameImpl.ts, neighborsWithDiag() and forEachNeighborWithDiag() intentionally duplicate coordinate iteration logic. They serve different purposes: forEachNeighborWithDiag() is a zero-allocation hot-path optimization while neighborsWithDiag() is a convenience method that returns an array. Refactoring one to call the other would add callback/closure allocations and indirection overhead, defeating the performance goals.
Applied to files:
src/client/graphics/layers/PlayerInfoOverlay.tssrc/core/game/GameImpl.ts
📚 Learning: 2025-11-26T22:27:31.844Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/worker/SharedTileRing.ts:55-63
Timestamp: 2025-11-26T22:27:31.844Z
Learning: In SharedTileRing.ts, the ring buffer is sized to width * height (the map dimensions). Combined with the dirty flag deduplication mechanism (each tile can only be enqueued once until the consumer clears its dirty flag), the number of pending entries is naturally bounded by the map size and drained every render frame. Therefore, ring buffer overflow should be extremely rare or effectively impossible, and any theoretical race condition between producer and consumer modifying the read index would only manifest as redundant tile refs being rendered, not memory corruption, which is acceptable.
<!-- </add_learning>
Applied to files:
src/client/graphics/layers/TerritoryLayer.tssrc/client/graphics/layers/TerritoryWebGLRenderer.ts
🧬 Code graph analysis (5)
src/client/graphics/HoverInfo.ts (2)
src/core/game/GameView.ts (10)
PlayerView(185-582)UnitView(48-183)GameView(584-951)x(832-834)y(835-837)tile(110-112)owner(113-115)owner(797-799)units(433-437)units(811-818)src/core/game/GameMap.ts (3)
TileRef(3-3)x(131-133)y(135-137)
src/core/game/GameView.ts (2)
src/core/game/GameImpl.ts (1)
ref(877-879)src/core/game/GameMap.ts (2)
ref(120-125)TileRef(3-3)
src/core/game/GameMap.ts (2)
src/core/game/GameImpl.ts (1)
ref(877-879)src/core/game/GameView.ts (1)
ref(826-828)
src/client/graphics/layers/PlayerInfoOverlay.ts (1)
src/client/graphics/HoverInfo.ts (1)
getHoverInfo(32-73)
src/client/graphics/layers/TerritoryWebGLRenderer.ts (4)
src/core/game/GameView.ts (9)
GameView(584-951)width(841-843)height(844-846)tile(110-112)y(835-837)player(757-763)id(86-88)id(461-463)x(832-834)src/core/configuration/Config.ts (1)
Theme(172-197)src/client/Utils.ts (1)
createCanvas(106-118)src/core/game/GameMap.ts (5)
width(143-145)height(146-148)TileRef(3-3)y(135-137)x(131-133)
🪛 Biome (2.1.2)
src/core/game/GameView.ts
[error] 887-888: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🔇 Additional comments (10)
src/client/graphics/TransformHandler.ts (1)
48-50: LGTM!Simple, clean getter that exposes the view offset for external consumers like the WebGPU renderer. Returning a fresh object keeps the internal state safe from external mutation.
src/core/game/GameMap.ts (3)
30-32: LGTM!Interface additions follow the established pattern (
hasFallout/setFallout). Clean and consistent.
82-85: LGTM!Bit 12 fits correctly between the player ID mask (0-11) and fallout bit (13). The state layout is now well-documented with the reserved bit comment.
218-232: Implementation looks correct.
isDefendedandsetDefendedfollow the same pattern as the fallout bit operations.Note:
tileStateView()returns a direct reference to the internal state array. This is likely intentional for the WebGPU renderer to efficiently upload tile state to GPU buffers without copying. Just be aware that external code can mutate this array directly.src/core/game/GameView.ts (1)
883-885: LGTM!Clean forwarding methods consistent with the existing delegation pattern in
GameView.Also applies to: 889-891
src/core/game/GameImpl.ts (2)
931-936: LGTM!Clean proxy methods. Note that
setDefendedhere correctly omits thereturnkeyword (unlikeGameView.ts).
995-997: LGTM!Consistent delegation to the underlying map.
src/client/graphics/GameRenderer.ts (2)
316-317: LGTM!Good comment explaining the architecture change. The main 2D canvas now acts as a transparent overlay while WebGPU handles the background rendering. This layered approach should work well for GPU-accelerated territory rendering.
365-366: LGTM!Using
clearRectinstead offillRectis the right approach for a transparent overlay. The WebGPU territory canvas now provides the background, so this canvas just needs to be cleared before drawing UI layers.src/client/graphics/layers/PlayerInfoOverlay.ts (1)
21-21: Good move to centralize hover logic.Using a shared hover helper keeps the overlay simpler and avoids duplicate rules.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
+101
to
115
| const info = getHoverInfo(this.game, worldCoord); | ||
|
|
||
| const tile = this.game.ref(worldCoord.x, worldCoord.y); | ||
| if (!tile) return; | ||
|
|
||
| const owner = this.game.owner(tile); | ||
|
|
||
| if (owner && owner.isPlayer()) { | ||
| this.player = owner as PlayerView; | ||
| if (info.player) { | ||
| this.player = info.player; | ||
| this.player.profile().then((p) => { | ||
| this.playerProfile = p; | ||
| }); | ||
| this.setVisible(true); | ||
| } else if (owner && !owner.isPlayer() && this.game.isLand(tile)) { | ||
| if (this.game.hasFallout(tile)) { | ||
| this.isIrradiatedWilderness = true; | ||
| } else { | ||
| this.isWilderness = true; | ||
| } | ||
| } else if (info.isWilderness || info.isIrradiatedWilderness) { | ||
| this.isWilderness = info.isWilderness; | ||
| this.isIrradiatedWilderness = info.isIrradiatedWilderness; | ||
| this.setVisible(true); | ||
| } else if (info.unit) { | ||
| this.unit = info.unit; | ||
| this.setVisible(true); |
Contributor
There was a problem hiding this comment.
Guard async profile load from stale hover.
If the mouse moves fast, an older profile() promise can overwrite the current player’s profile.
🛠️ Suggested fix
@@
`@state`()
private playerProfile: PlayerProfile | null = null;
+
+ private profileRequestId = 0;
@@
if (info.player) {
- this.player = info.player;
- this.player.profile().then((p) => {
- this.playerProfile = p;
- });
+ const player = info.player;
+ this.player = player;
+ this.playerProfile = null;
+ const requestId = ++this.profileRequestId;
+ player.profile().then((p) => {
+ if (this.player === player && requestId === this.profileRequestId) {
+ this.playerProfile = p;
+ }
+ });
this.setVisible(true);
} else if (info.isWilderness || info.isIrradiatedWilderness) {🤖 Prompt for AI Agents
In `@src/client/graphics/layers/PlayerInfoOverlay.ts` around lines 101 - 115, When
loading the async profile in PlayerInfoOverlay, guard against stale promises by
capturing the specific player reference before awaiting profile() and only
assigning the result if the overlay still references that same player; in the
block where you set this.player and call this.player.profile().then(...), change
to save the player in a local const (e.g. const player = info.player), set
this.player = player, then in the .then callback check if this.player === player
before setting this.playerProfile and calling this.setVisible(true) (or move
setVisible earlier as needed) so an older promise cannot overwrite a newer
hover's profile.
Comment on lines
+85
to
+92
| private configureRenderer() { | ||
| const { renderer, reason } = TerritoryWebGLRenderer.create( | ||
| this.game, | ||
| this.theme, | ||
| ); | ||
| if (!renderer) { | ||
| throw new Error(reason ?? "WebGPU is required for territory rendering."); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "TerritoryLayer.ts" --type fRepository: openfrontio/OpenFrontIO
Length of output: 111
🏁 Script executed:
cat -n src/client/graphics/layers/TerritoryLayer.ts | head -120 | tail -50Repository: openfrontio/OpenFrontIO
Length of output: 2120
🏁 Script executed:
# Check the TerritoryWebGLRenderer to understand create() method
fd "TerritoryWebGLRenderer" --type fRepository: openfrontio/OpenFrontIO
Length of output: 119
🏁 Script executed:
# Look for fallback rendering patterns in the codebase
rg "renderer.*null|fallback|graceful" --type ts -A 3 -B 3 | head -100Repository: openfrontio/OpenFrontIO
Length of output: 5081
🏁 Script executed:
# Check how other layers handle renderer failures
rg "throw new Error.*required" --type ts | head -20Repository: openfrontio/OpenFrontIO
Length of output: 298
🏁 Script executed:
cat -n src/client/graphics/layers/TerritoryWebGLRenderer.ts | head -150Repository: openfrontio/OpenFrontIO
Length of output: 6427
🏁 Script executed:
# Check how redraw is called and from where
rg "\.redraw\(\)" --type ts -B 5 -A 2Repository: openfrontio/OpenFrontIO
Length of output: 10577
🏁 Script executed:
# Check if there's any initialization/setup that detects WebGPU availability
rg "WebGPU|webgpu|GPUAdapter|requestAdapter" --type ts | head -30Repository: openfrontio/OpenFrontIO
Length of output: 1403
🏁 Script executed:
# Look at the Layer base class to understand the architecture
rg "class.*Layer.*extends" --type ts -A 5 | head -50Repository: openfrontio/OpenFrontIO
Length of output: 49
Remove hard error when WebGPU is unavailable.
The territory renderer is designed to be optional—renderLayer() already handles null gracefully. But configureRenderer() throws when WebGPU is missing, which crashes the entire rendering pipeline. Since GameRenderer.redraw() calls this without catching errors, non-WebGPU clients will fail.
Instead of throwing, return early and leave this.territoryRenderer as null so the existing null check in renderLayer() handles the situation.
Suggested change
private configureRenderer() {
const { renderer, reason } = TerritoryWebGLRenderer.create(
this.game,
this.theme,
);
if (!renderer) {
- throw new Error(reason ?? "WebGPU is required for territory rendering.");
+ return;
}📝 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.
Suggested change
| private configureRenderer() { | |
| const { renderer, reason } = TerritoryWebGLRenderer.create( | |
| this.game, | |
| this.theme, | |
| ); | |
| if (!renderer) { | |
| throw new Error(reason ?? "WebGPU is required for territory rendering."); | |
| } | |
| private configureRenderer() { | |
| const { renderer, reason } = TerritoryWebGLRenderer.create( | |
| this.game, | |
| this.theme, | |
| ); | |
| if (!renderer) { | |
| return; | |
| } |
🤖 Prompt for AI Agents
In `@src/client/graphics/layers/TerritoryLayer.ts` around lines 85 - 92, The
configureRenderer method currently throws when TerritoryWebGLRenderer.create
returns no renderer; instead, stop throwing and return early so non-WebGPU
clients don't crash: call TerritoryWebGLRenderer.create(this.game, this.theme),
and if renderer is falsy simply set this.territoryRenderer = null (optionally
log reason) and return; leave renderLayer and GameRenderer.redraw to continue
handling the null renderer case—do not propagate an exception from
configureRenderer.
Comment on lines
+150
to
+174
| private async init() { | ||
| const nav = globalThis.navigator as any; | ||
| const adapter = await nav.gpu.requestAdapter(); | ||
| if (!adapter) { | ||
| return; | ||
| } | ||
|
|
||
| const device = await adapter.requestDevice(); | ||
| this.device = device; | ||
|
|
||
| const context = this.canvas.getContext("webgpu"); | ||
| if (!context) { | ||
| return; | ||
| } | ||
| this.context = context; | ||
|
|
||
| this.canvasFormat = | ||
| typeof nav.gpu.getPreferredCanvasFormat === "function" | ||
| ? nav.gpu.getPreferredCanvasFormat() | ||
| : "bgra8unorm"; | ||
|
|
||
| this.configureContext(); | ||
| this.createGpuResources(); | ||
| this.ready = true; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/client/graphics/layers/TerritoryWebGLRenderer.ts | head -200Repository: openfrontio/OpenFrontIO
Length of output: 8301
🏁 Script executed:
rg "initPromise|init\(\)" src/client/graphics/layers/TerritoryWebGLRenderer.ts -A 5 -B 2Repository: openfrontio/OpenFrontIO
Length of output: 550
🏁 Script executed:
fd -e ts -e tsx | xargs grep -l "private async init\(\)" | head -5Repository: openfrontio/OpenFrontIO
Length of output: 146
🏁 Script executed:
rg "initPromise|\.ready|isReady" src/client/graphics/layers/TerritoryWebGLRenderer.ts -B 2 -A 2Repository: openfrontio/OpenFrontIO
Length of output: 561
🏁 Script executed:
rg "TerritoryWebGLRenderer|\.create\(" --type ts --type tsx | grep -v node_modules | head -15Repository: openfrontio/OpenFrontIO
Length of output: 94
🏁 Script executed:
rg "\.ready|initPromise" src/client --type ts --type tsx | head -20Repository: openfrontio/OpenFrontIO
Length of output: 94
🏁 Script executed:
rg "TerritoryWebGLRenderer" src/client --type ts -B 2 -A 5 | head -50Repository: openfrontio/OpenFrontIO
Length of output: 3701
🏁 Script executed:
rg "\.ready" src/client --type ts | head -20Repository: openfrontio/OpenFrontIO
Length of output: 1301
🏁 Script executed:
# Look for how the create result is used
rg "TerritoryWebGLCreateResult" src/client --type ts -B 3 -A 10Repository: openfrontio/OpenFrontIO
Length of output: 2429
🏁 Script executed:
# Check the full init method to see if there are any try-catch or error handling
sed -n '150,174p' src/client/graphics/layers/TerritoryWebGLRenderer.tsRepository: openfrontio/OpenFrontIO
Length of output: 671
🏁 Script executed:
# Check if createGpuResources can fail and whether errors are caught
sed -n '185,250p' src/client/graphics/layers/TerritoryWebGLRenderer.tsRepository: openfrontio/OpenFrontIO
Length of output: 2713
🏁 Script executed:
# Check how TerritoryLayer uses the renderer after creation
rg "configureRenderer|territoryRenderer" src/client/graphics/layers/TerritoryLayer.ts -B 2 -A 10Repository: openfrontio/OpenFrontIO
Length of output: 5073
🏁 Script executed:
# Check if initPromise is ever awaited anywhere
rg "await.*initPromise|initPromise.*await" src/client --type tsRepository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Check if there's any error handling or try-catch in the init method or around it
rg "try|catch|throw" src/client/graphics/layers/TerritoryWebGLRenderer.ts -B 2 -A 2Repository: openfrontio/OpenFrontIO
Length of output: 635
🏁 Script executed:
# Check tick and render methods to see what happens when ready is false
sed -n '400,600p' src/client/graphics/layers/TerritoryWebGLRenderer.ts | head -100Repository: openfrontio/OpenFrontIO
Length of output: 2798
🏁 Script executed:
# Check if device.createBuffer, device.createTexture, device.createShaderModule can throw
# Look at how errors are handled in createGpuResources (full method)
sed -n '185,800p' src/client/graphics/layers/TerritoryWebGLRenderer.ts | grep -E "this.device.create|try|catch|throw" -B 2 -A 2Repository: openfrontio/OpenFrontIO
Length of output: 4809
🏁 Script executed:
# Check if createGpuResources completes or if there are failures that could occur
# Let's see what happens after createGpuResources is called
sed -n '170,180p' src/client/graphics/layers/TerritoryWebGLRenderer.tsRepository: openfrontio/OpenFrontIO
Length of output: 344
Surface init failures so the UI can react.
If adapter/device/context creation fails during async initialization, the renderer stays "not ready" with no error reporting. Callers cannot distinguish between "still initializing" and "initialization failed", leaving the map blank with no explanation.
Add error tracking so failures can be diagnosed:
🛠️ Suggested failure reporting
private initPromise: Promise<void> | null = null;
private ready = false;
+ private initFailedReason: string | null = null;
private async init() {
const nav = globalThis.navigator as any;
const adapter = await nav.gpu.requestAdapter();
if (!adapter) {
+ this.initFailedReason = "WebGPU adapter unavailable";
return;
}
const device = await adapter.requestDevice();
this.device = device;
const context = this.canvas.getContext("webgpu");
if (!context) {
+ this.initFailedReason = "WebGPU context unavailable";
return;
}
this.context = context;
this.canvasFormat =
typeof nav.gpu.getPreferredCanvasFormat === "function"
? nav.gpu.getPreferredCanvasFormat()
: "bgra8unorm";
this.configureContext();
this.createGpuResources();
this.ready = true;
}
+ public getInitFailureReason(): string | null {
+ return this.initFailedReason;
+ }🤖 Prompt for AI Agents
In `@src/client/graphics/layers/TerritoryWebGLRenderer.ts` around lines 150 - 174,
The init() method currently swallows failures; wrap the adapter/device/context
creation and subsequent configureContext()/createGpuResources() calls in a
try/catch and surface failures by setting a new instance property (e.g.
this.initError) and ensuring this.ready is false on error; specifically, in
init() check adapter/device/context creation and if any are null assign
this.initError = new Error("...") (with descriptive text), set this.ready =
false, and return (or rethrow) — and in the catch block assign this.initError =
err, this.ready = false, and optionally rethrow or emit an event so callers can
react; keep assignments to this.device and this.context only after successful
creation and call configureContext() and createGpuResources() only on success.
Comment on lines
+886
to
+888
| setDefended(ref: TileRef, value: boolean): void { | ||
| return this._map.setDefended(ref, value); | ||
| } |
Contributor
There was a problem hiding this comment.
Remove the return keyword from the void function.
Static analysis correctly flags this: setDefended has a void return type, but line 887 uses return. The code works but the return is unnecessary and misleading.
Suggested fix
setDefended(ref: TileRef, value: boolean): void {
- return this._map.setDefended(ref, value);
+ this._map.setDefended(ref, value);
}📝 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.
Suggested change
| setDefended(ref: TileRef, value: boolean): void { | |
| return this._map.setDefended(ref, value); | |
| } | |
| setDefended(ref: TileRef, value: boolean): void { | |
| this._map.setDefended(ref, value); | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 887-888: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🤖 Prompt for AI Agents
In `@src/core/game/GameView.ts` around lines 886 - 888, The setDefended method is
declared to return void but uses an unnecessary return; remove the "return"
keyword so setDefended(ref: TileRef, value: boolean): void simply calls
this._map.setDefended(ref, value);—locate the method named setDefended in
GameView and change the implementation to a plain call to this._map.setDefended
without returning its value.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace Canvas 2D-based territory rendering with a WebGPU implementation for improved performance and scalability.
Major changes:
Files changed:
If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #(issue number)
Description:
Describe the PR.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
DISCORD_USERNAME