Fow test 3 #2772
Fow test 3 #2772
Conversation
…me.ts that where present in the previously shared files)
WalkthroughAdds a Fog of War feature: core FogOfWarManager, client FogOfWarLayer, renderer and UI wiring, input guards to block actions on fully fogged tiles, and multiple UI/UX changes (leaderboard, radial menus, overlays, unit/UI layers). Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Player (Client)
participant Runner as ClientGameRunner
participant Game as GameImpl
participant Vision as FogOfWarLayer
participant Renderer as GameRenderer
Client->>Runner: click tile / unit
Runner->>Game: read gameMode
alt GameMode == FogOfWar
Runner->>Vision: getFogValueAt(tileIdx)
alt fogValue >= 1.0
Runner-->>Client: block action (early return)
else
Runner->>Game: dispatch action (move/attack/etc.)
Game->>Renderer: state update -> render
end
else
Runner->>Game: dispatch action (no fog checks)
end
sequenceDiagram
participant Game as GameImpl
participant FoWMgr as FogOfWarManager
participant Vision as FogOfWarLayer
participant Layers as Render Layers (Unit/Name/UI)
Game->>FoWMgr: initialize() (on FogOfWar)
loop per-player tick
Game->>FoWMgr: updateVisibility(player)
FoWMgr->>Vision: provide visible tiles / marks
end
Vision->>Layers: expose getFogValueAt(tile)
Layers->>Layers: render normally or apply alpha/hide based on fogValue
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (4)
src/client/graphics/layers/PlayerInfoOverlay.ts-86-98 (1)
86-98: Unused properties:seenPlayersandnameLayerare declared but never used.
seenPlayersis populated at line 149 but never readnameLayeris declared but has no usage in this fileRemove unused code or implement the intended functionality.
🔎 Remove unused code
- // Track which players have been seen (visibility memory) - private seenPlayers: Set<string> = new Set(); - - /** - * Referência à camada de Fog of War para verificação de visibilidade - */ @property({ type: Object }) public fogOfWarLayer: FogOfWarLayer | null = null; // Reference to FogOfWarLayer for visibility checking - - @property({ type: Object }) - public nameLayer: any = null; // Reference to NameLayer for visibility checkingAnd remove line 149:
- // Mark player as seen - this.seenPlayers.add(player.id());Committable suggestion skipped: line range outside the PR's diff.
src/client/graphics/layers/Leaderboard.ts-168-170 (1)
168-170: Strict visibility check may hide players unexpectedly.
fogValue === 0means only tiles with zero fog show players. If a player's name is at fog 0.01 (nearly visible), they won't appear. Consider using a threshold likefogValue < 0.1for consistency with other fog checks that use< 0.8.src/client/graphics/layers/NameLayer.ts-206-211 (1)
206-211: Dead code: ally elimination check is unreachable.Line 119 already returns early if
!render.player.isAlive(). This check for eliminated allies will never execute for dead players.src/client/graphics/GameRenderer.ts-296-315 (1)
296-315: Duplicate wiring and redundant lookups.
leaderboard.fogOfWarLayeris set twice (lines 298-300 and 313-315)mainRadialMenu.setFogOfWarLayeris called twice (lines 257 and 308-310)fogOfWarLayerRefis found from thelayersarray, butfogOfWarLayervariable is already in scope from line 216Remove the duplicates and use the existing variable:
- // Set reference to FogOfWarLayer for Leaderboard - const fogOfWarLayerRef = layers.find(layer => layer.constructor.name === 'FogOfWarLayer') as any; - if (fogOfWarLayerRef && leaderboard) { - leaderboard.fogOfWarLayer = fogOfWarLayerRef; - } - - // Set reference to FogOfWarLayer for PlayerInfoOverlay - if (fogOfWarLayerRef && playerInfo) { - playerInfo.fogOfWarLayer = fogOfWarLayerRef; - } - - // Set reference to FogOfWarLayer for MainRadialMenu - if (fogOfWarLayerRef && mainRadialMenu) { - mainRadialMenu.setFogOfWarLayer(fogOfWarLayerRef); - } - - // Set reference to FogOfWarLayer for Leaderboard - if (fogOfWarLayerRef && leaderboard) { - leaderboard.fogOfWarLayer = fogOfWarLayerRef; - } + // Set FogOfWarLayer references + leaderboard.fogOfWarLayer = fogOfWarLayer; + playerInfo.fogOfWarLayer = fogOfWarLayer;Committable suggestion skipped: line range outside the PR's diff.
🧹 Nitpick comments (18)
src/client/ClientGameRunner.ts (2)
504-518: Extract repeated fog check logic into a helper method.This same fog-checking pattern appears three times (lines 504-518, 617-631, 652-666). Extract it to reduce duplication and make future changes easier.
🔎 Extract helper method
+ private isTileFullyFogged(tile: TileRef): boolean { + if (this.gameView.config().gameConfig().gameMode !== GameMode.FogOfWar) { + return false; + } + const fogOfWarLayer = this.renderer.getFogOfWarLayer(); + if (!fogOfWarLayer) { + return false; + } + const tileX = this.gameView.x(tile); + const tileY = this.gameView.y(tile); + const idx = tileY * this.gameView.width() + tileX; + return fogOfWarLayer.getFogValueAt(idx) >= 1.0; + }Then replace the three duplicated blocks with:
if (this.isTileFullyFogged(tile)) { return; }
507-507: Avoid runtime type check for method existence.Using
typeof fogOfWarLayer.getFogValueAt === 'function'suggests uncertain typing. IfFogOfWarLayeris properly typed, this check is not needed. If the layer might be missing the method, the type definition should reflect that with an optional method.src/client/graphics/layers/UILayer.ts (2)
52-54: Use English for code comments.The JSDoc comment is in Portuguese. Keep comments in English for consistency across the codebase.
🔎 Translate comment
- /** - * @param fogOfWarLayer Referência opcional à camada de Fog of War para controlar visibilidade de elementos de interface - */ + /** + * @param fogOfWarLayer Optional reference to the Fog of War layer for controlling UI element visibility + */
378-385: Extract fixed structures list to a constant.This list of fixed structure types is defined inline. Consider extracting it to a module-level constant for reuse and clarity.
🔎 Extract constant
+const FIXED_STRUCTURES = [ + UnitType.City, + UnitType.Factory, + UnitType.Port, + UnitType.DefensePost, + UnitType.MissileSilo, + UnitType.SAMLauncher, +] as const; + // In createLoadingBar: - const fixedStructures = [ - UnitType.City, - UnitType.Factory, - UnitType.Port, - UnitType.DefensePost, - UnitType.MissileSilo, - UnitType.SAMLauncher - ]; - - if (fixedStructures.includes(unit.type()) && ... + if (FIXED_STRUCTURES.includes(unit.type()) && ...src/client/graphics/layers/PlayerInfoOverlay.ts (1)
90-97: Use English for comments and avoidanytype.Portuguese comments should be in English. Also,
nameLayer: anyloses type safety. If this property is needed, use the proper type.src/core/game/FogOfWar.ts (2)
4-11: Remove duplicate docblock.There are two consecutive docblocks (lines 4-7 and 8-11) saying essentially the same thing. Keep only one.
🔎 Remove duplicate
-/** - * Classe responsável por gerenciar o sistema de Fog of War (nevoeiro de guerra) - * no jogo. Controla quais tiles são visíveis para cada jogador. - */ /** - * Gerencia o sistema de Fog of War (nevoeiro de guerra) no jogo. - * Controla quais tiles visíveis para cada jogador no modo Fog of War. + * Manages the Fog of War system in the game. + * Controls which tiles are visible for each player in Fog of War mode. */
131-162: Redundant iteration: Cities are already included inplayer.units().You iterate all units at line 131, then iterate cities separately at line 152. Since cities are units, they're already processed in the first loop. You can combine the logic:
🔎 Combine unit iteration
const units = player.units(); for (const unit of units) { const unitTile = unit.tile(); visibleTiles.add(unitTile); const neighbors = this.game.map().neighbors(unitTile); for (const neighbor of neighbors) { visibleTiles.add(neighbor); } - if (unit.type() === "Warship" || unit.type() === "Transport") { - const extendedView = this.game.map().circleSearch(unitTile, 3); - for (const tile of extendedView) { - visibleTiles.add(tile); - } + // Extended vision for certain unit types + let visionRadius = 0; + if (unit.type() === UnitType.Warship || unit.type() === UnitType.TransportShip) { + visionRadius = 3; + } else if (unit.type() === UnitType.City) { + visionRadius = 2; + } + + if (visionRadius > 0) { + const extendedView = this.game.map().circleSearch(unitTile, visionRadius); + for (const tile of extendedView) { + visibleTiles.add(tile); + } } } - - // Adiciona tiles das cidades do jogador - const cities = player.units(UnitType.City); - for (const city of cities) { - const cityTile = city.tile(); - visibleTiles.add(cityTile); - - // Cities have greater vision - const cityView = this.game.map().circleSearch(cityTile, 2); - for (const tile of cityView) { - visibleTiles.add(tile); - } - }src/client/graphics/layers/MainRadialMenu.ts (3)
102-108: Empty if block does nothing - remove or implement.This if block checks for FogOfWar mode but has no logic inside, only a comment. Either remove it or implement the intended behavior.
🔎 Remove empty block
- // In Fog of War mode, allow the radial menu in all areas - // Filtering which buttons to show will be done in rootMenuElement - if (this.game.config().gameConfig().gameMode === GameMode.FogOfWar) { - // The logic for which buttons to show at each fog level - // is handled in rootMenuElement, so we allow the menu in all areas - }
35-38: Use proper types instead ofany.
fogOfWarLayer: anyand the method parameters lose type safety. Import and use theFogOfWarLayertype.🔎 Add proper typing
+import { FogOfWarLayer } from "./FogOfWarLayer"; + @customElement("main-radial-menu") export class MainRadialMenu extends LitElement implements Layer { // ... - private fogOfWarLayer: any = null; + private fogOfWarLayer: FogOfWarLayer | null = null; - public setFogOfWarLayer(fogLayer: any) { + public setFogOfWarLayer(fogLayer: FogOfWarLayer | null) { this.fogOfWarLayer = fogLayer; }Also applies to: 78-87
84-87: Remove unused method stub.
setNameLayerhas an empty body and no implementation. Remove it if not needed.🔎 Remove unused method
- // Method to set the reference to NameLayer (not used in this implementation but added for consistency) - public setNameLayer(nameLayer: any) { - // Not used in this implementation - }src/core/game/GameImpl.ts (1)
395-398: Performance: Fog updates run every tick for every player.
updateFogOfWaris called for every player on every tick. If fog state rarely changes, consider updating less frequently (e.g., every 10 ticks) or only when player territory/units change.src/client/graphics/layers/UnitLayer.ts (1)
654-660: Complex alpha logic may produce unexpected results.When
targetableis false andfogEffectAlphais less than 1.0, the alpha becomesMath.min(0.5, fogEffectAlpha). But iffogEffectAlphais 0.1 (from fog), the unit will render at 0.1 alpha - nearly invisible. IffogEffectAlphais 0 (not visible), context is still saved but nothing visible is drawn. Consider simplifying:
- If unit is not visible (
fogEffectAlpha === 0), skip rendering entirely.- Otherwise, apply the appropriate alpha.
src/client/graphics/layers/NameLayer.ts (1)
132-182: Significant code duplication in visibility checks.The visibility check logic (lines 134-150 and 165-181) is nearly identical. Extract into a helper function:
private checkVisibilityConditions(render: RenderInfo): boolean { const baseSize = Math.max(1, Math.floor(render.player.nameLocation().size)); const size = this.transformHandler.scale * baseSize; const isOnScreen = render.location ? this.transformHandler.isOnScreen(render.location) : false; const maxZoomScale = 17; return this.isVisible && size >= 7 && !(this.transformHandler.scale > maxZoomScale && size > 100) && isOnScreen; }src/client/graphics/layers/RadialMenuElements.ts (1)
800-821: Redundant cast toanywhen fogOfWarLayer is already in params.Line 812-813 casts
paramstoanyto accessfogOfWarLayer, butfogOfWarLayeris already defined inMenuElementParams(line 46). Remove the unnecessary cast:- if ((params as any).fogOfWarLayer) { - const fogOfWarLayer = (params as any).fogOfWarLayer; + if (params.fogOfWarLayer) { + const fogOfWarLayer = params.fogOfWarLayer;src/client/graphics/layers/Leaderboard.ts (1)
494-519: Eye icon view action has redundant mode check.Lines 500-501 check for FogOfWar mode again inside the click handler, but this entire block is only rendered when
isFogOfWarMode && isPlayerEliminated(line 494). The inner check is redundant.- if (this.game?.config().gameConfig().gameMode === GameMode.FogOfWar) { - const myPlayer = this.game.myPlayer(); - if (myPlayer && !myPlayer.isAlive()) { + const myPlayer = this.game?.myPlayer(); + if (myPlayer && !myPlayer.isAlive()) {src/client/graphics/layers/FogOfWarLayer.ts (3)
196-232: Border vision uses await in a loop - potential performance issue.
applyBorderVisioncallsawait player.borderTiles()for each allied player inside a loop. IfborderTiles()is slow, this serializes the calls. Consider usingPromise.all()for parallel fetching:const borderPromises = alliedPlayers.map(async (player) => { try { const borderTiles = await player.borderTiles(); return { playerId: player.id(), borderTiles: borderTiles.borderTiles }; } catch (e) { console.warn("Failed to get border tiles for player", player.id(), e); return null; } }); const borderData = (await Promise.all(borderPromises)).filter(Boolean);
436-471: Method namecanUnitActivateVisionhas inverted logic.The method returns
truewhenvisibilityRatio < 0.75, meaning the unit can activate vision when less than 75% of its area is already visible. The name suggests it checks if a unit can activate, but the threshold logic is inverted from the comment "reveal at least 25%".Consider renaming to
shouldUnitProvideVisionor clarifying the comment:- // Method to check if a unit can reveal at least 25% of its vision field + // Returns true if more than 25% of the unit's vision field is still hidden (worth revealing) private canUnitActivateVision(centerTile: number, radius: number): boolean {
538-541: Empty method body.
updateMobileUnitFogVisibility()has only a comment and no implementation. If it's intentionally empty, remove it or add a TODO comment explaining what it will do.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
resources/lang/en.jsonsrc/client/ClientGameRunner.tssrc/client/SinglePlayerModal.tssrc/client/graphics/GameRenderer.tssrc/client/graphics/layers/FogOfWarLayer.tssrc/client/graphics/layers/Leaderboard.tssrc/client/graphics/layers/MainRadialMenu.tssrc/client/graphics/layers/NameLayer.tssrc/client/graphics/layers/PlayerInfoOverlay.tssrc/client/graphics/layers/RadialMenuElements.tssrc/client/graphics/layers/UILayer.tssrc/client/graphics/layers/UnitLayer.tssrc/core/game/FogOfWar.tssrc/core/game/GameImpl.ts
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1655
File: resources/maps/giantworldmap/manifest.json:165-174
Timestamp: 2025-07-31T12:03:08.052Z
Learning: In OpenFrontIO draft PRs, flag names in map manifest files may be placeholders that will be updated to match the actual SVG flag file names when the assets are received. The final naming depends on the actual flag SVG files provided, not code naming conventions.
📚 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/SinglePlayerModal.tssrc/client/ClientGameRunner.tssrc/client/graphics/layers/PlayerInfoOverlay.tssrc/client/graphics/layers/MainRadialMenu.tssrc/client/graphics/layers/Leaderboard.tssrc/client/graphics/layers/RadialMenuElements.tssrc/client/graphics/GameRenderer.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/client/SinglePlayerModal.tssrc/client/ClientGameRunner.tssrc/core/game/GameImpl.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/SinglePlayerModal.tssrc/client/ClientGameRunner.tssrc/client/graphics/layers/PlayerInfoOverlay.tssrc/core/game/GameImpl.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/SinglePlayerModal.tssrc/client/ClientGameRunner.tssrc/client/graphics/layers/Leaderboard.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/client/SinglePlayerModal.tssrc/client/graphics/layers/UILayer.tssrc/client/graphics/layers/UnitLayer.tssrc/core/game/GameImpl.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/client/ClientGameRunner.tssrc/client/graphics/layers/PlayerInfoOverlay.tssrc/client/graphics/layers/Leaderboard.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/client/ClientGameRunner.tssrc/client/graphics/layers/PlayerInfoOverlay.tssrc/core/game/GameImpl.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/ClientGameRunner.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/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/layers/NameLayer.tssrc/client/graphics/layers/Leaderboard.tssrc/client/graphics/layers/FogOfWarLayer.tssrc/client/graphics/GameRenderer.ts
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/client/graphics/layers/MainRadialMenu.tssrc/client/graphics/layers/RadialMenuElements.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/PastelThemeDark.ts:53-53
Timestamp: 2025-06-10T09:56:44.473Z
Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.
Applied to files:
src/client/graphics/layers/UILayer.tssrc/client/graphics/layers/UnitLayer.ts
📚 Learning: 2025-12-26T22:21:21.904Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2689
File: src/client/PublicLobby.ts:245-245
Timestamp: 2025-12-26T22:21:21.904Z
Learning: In public lobbies with HumansVsNations mode in src/client/PublicLobby.ts, maxPlayers represents only human player slots (already halved in DefaultConfig.ts). The nation NPCs are added automatically server-side and don't count toward maxPlayers. Therefore, getTeamSize correctly returns maxPlayers directly for HumansVsNations to display the proper team size (e.g., maxPlayers=5 yields "5 Humans vs 5 Nations").
Applied to files:
src/client/graphics/layers/Leaderboard.tssrc/core/game/GameImpl.ts
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
Repo: openfrontio/OpenFrontIO PR: 977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
Applied to files:
src/client/graphics/layers/Leaderboard.ts
📚 Learning: 2025-11-21T22:30:12.246Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2493
File: src/core/game/TrainStation.ts:79-80
Timestamp: 2025-11-21T22:30:12.246Z
Learning: In src/core/game/TrainStation.ts and RailNetworkImpl.ts, the railroad network maintains a system invariant of at most one railroad between any two stations. RailNetworkImpl.connect is private and only called from connectToNearbyStations during initial station connection, which iterates over unique nearby units without duplicates. The Map<TrainStation, Railroad> structure in railroadByNeighbor correctly reflects this 1-to-1 relationship and should not be flagged as potentially allowing silent overwrites.
Applied to files:
src/core/game/GameImpl.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/GameImpl.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:
resources/lang/en.json
📚 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:
resources/lang/en.json
📚 Learning: 2025-08-27T08:12:19.610Z
Learnt from: mokizzz
Repo: openfrontio/OpenFrontIO PR: 1940
File: resources/lang/en.json:763-766
Timestamp: 2025-08-27T08:12:19.610Z
Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.
Applied to files:
resources/lang/en.json
🧬 Code graph analysis (8)
src/client/SinglePlayerModal.ts (2)
src/client/Utils.ts (1)
translateText(92-147)src/client/LangSelector.ts (1)
translateText(258-278)
src/client/ClientGameRunner.ts (2)
src/core/game/UnitImpl.ts (1)
tile(178-180)src/core/game/GameView.ts (1)
tile(110-112)
src/client/graphics/layers/NameLayer.ts (3)
src/core/game/GameView.ts (3)
GameView(584-917)myPlayer(728-730)player(732-738)src/client/graphics/TransformHandler.ts (2)
TransformHandler(15-333)isOnScreen(142-150)src/client/graphics/layers/Leaderboard.ts (1)
render(381-549)
src/core/game/FogOfWar.ts (4)
src/core/game/GameMap.ts (2)
TileRef(3-3)GameMap(6-56)src/core/game/Game.ts (1)
Game(678-774)src/core/game/GameImpl.ts (4)
player(494-500)ref(842-844)neighbors(899-901)units(238-240)src/core/game/PlayerImpl.ts (1)
tiles(283-285)
src/client/graphics/layers/UILayer.ts (1)
src/client/graphics/layers/FogOfWarLayer.ts (1)
FogOfWarLayer(15-619)
src/client/graphics/layers/UnitLayer.ts (1)
src/client/graphics/layers/FogOfWarLayer.ts (1)
FogOfWarLayer(15-619)
src/core/game/GameImpl.ts (2)
src/core/game/FogOfWar.ts (1)
FogOfWarManager(12-193)src/core/game/GameMap.ts (1)
TileRef(3-3)
src/client/graphics/layers/RadialMenuElements.ts (3)
src/core/game/GameView.ts (3)
nameLocation(439-441)isLand(828-830)unit(794-796)src/core/game/GameImpl.ts (1)
isLand(869-871)src/core/game/GameMap.ts (1)
isLand(154-156)
🪛 GitHub Actions: 🧪 CI
src/client/SinglePlayerModal.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/SinglePlayerModal.ts' to fix.
src/client/ClientGameRunner.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/ClientGameRunner.ts' to fix.
src/client/graphics/layers/PlayerInfoOverlay.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/graphics/layers/PlayerInfoOverlay.ts' to fix.
src/client/graphics/layers/NameLayer.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/graphics/layers/NameLayer.ts' to fix.
src/core/game/FogOfWar.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/core/game/FogOfWar.ts' to fix.
src/client/graphics/layers/MainRadialMenu.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/graphics/layers/MainRadialMenu.ts' to fix.
src/client/graphics/layers/UILayer.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/graphics/layers/UILayer.ts' to fix.
src/client/graphics/layers/UnitLayer.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/graphics/layers/UnitLayer.ts' to fix.
src/client/graphics/layers/Leaderboard.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/graphics/layers/Leaderboard.ts' to fix.
src/core/game/GameImpl.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/core/game/GameImpl.ts' to fix.
src/client/graphics/layers/FogOfWarLayer.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/graphics/layers/FogOfWarLayer.ts' to fix.
src/client/graphics/layers/RadialMenuElements.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/graphics/layers/RadialMenuElements.ts' to fix.
src/client/graphics/GameRenderer.ts
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write src/client/graphics/GameRenderer.ts' to fix.
resources/lang/en.json
[error] 1-1: Prettier formatting issues detected. Run 'npx prettier --write resources/lang/en.json' to fix.
🪛 GitHub Actions: 🚀 Deploy
src/client/SinglePlayerModal.ts
[error] 194-194: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 197-197: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 206-206: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 513-513: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
src/client/ClientGameRunner.ts
[error] 505-505: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 618-618: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 653-653: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
src/client/graphics/layers/NameLayer.ts
[error] 124-124: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 242-242: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
src/client/graphics/layers/MainRadialMenu.ts
[error] 105-105: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
src/client/graphics/layers/Leaderboard.ts
[error] 102-102: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 130-130: TS2531: Object is possibly 'null'.
[error] 137-137: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 146-146: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 207-207: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
src/core/game/GameImpl.ts
[error] 106-106: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 160-160: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
src/client/graphics/layers/FogOfWarLayer.ts
[error] 106-106: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 121-121: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
🪛 GitHub Check: 🔍 ESLint
src/client/graphics/layers/FogOfWarLayer.ts
[failure] 586-586:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator.
[failure] 552-552:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator.
[failure] 487-487:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator.
[failure] 474-474:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator.
[failure] 342-342:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator.
[failure] 270-270:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator.
[failure] 152-152:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator.
[failure] 132-132:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator.
🪛 GitHub Check: 🔬 Test
src/client/graphics/layers/RadialMenuElements.ts
[failure] 805-805: tests/client/graphics/RadialMenuElements.test.ts > RadialMenuElements > rootMenuElement > should handle ally menu correctly
TypeError: params.game.config(...).gameConfig is not a function
❯ Object.subMenu src/client/graphics/layers/RadialMenuElements.ts:805:30
❯ tests/client/graphics/RadialMenuElements.test.ts:345:39
[failure] 805-805: tests/client/graphics/RadialMenuElements.test.ts > RadialMenuElements > rootMenuElement > should include info menu in both cases
TypeError: params.game.config(...).gameConfig is not a function
❯ Object.subMenu src/client/graphics/layers/RadialMenuElements.ts:805:30
❯ tests/client/graphics/RadialMenuElements.test.ts:331:39
[failure] 805-805: tests/client/graphics/RadialMenuElements.test.ts > RadialMenuElements > rootMenuElement > should show attack and boat menu on enemy territory
TypeError: params.game.config(...).gameConfig is not a function
❯ Object.subMenu src/client/graphics/layers/RadialMenuElements.ts:805:30
❯ tests/client/graphics/RadialMenuElements.test.ts:320:39
[failure] 805-805: tests/client/graphics/RadialMenuElements.test.ts > RadialMenuElements > rootMenuElement > should show build and delete menu on own territory
TypeError: params.game.config(...).gameConfig is not a function
❯ Object.subMenu src/client/graphics/layers/RadialMenuElements.ts:805:30
❯ tests/client/graphics/RadialMenuElements.test.ts:303:39
[failure] 805-805: tests/radialMenuElements.spec.ts > RadialMenuElements ally break > action calls handleBreakAlliance and closes menu
TypeError: params.game.config is not a function
❯ Object.subMenu src/client/graphics/layers/RadialMenuElements.ts:805:21
❯ tests/radialMenuElements.spec.ts:90:35
[failure] 805-805: tests/radialMenuElements.spec.ts > RadialMenuElements ally break > shows break option with correct color when allied
TypeError: params.game.config is not a function
❯ Object.subMenu src/client/graphics/layers/RadialMenuElements.ts:805:21
❯ tests/radialMenuElements.spec.ts:81:35
🔇 Additional comments (15)
src/client/SinglePlayerModal.ts (1)
509-516: Good addition: Auto-enable random spawn for Fog of War mode.This makes sense since Fog of War typically needs random spawning for fairness. One small note: the checkbox UI for random spawn won't reflect this forced state visually until the next render.
src/client/graphics/layers/PlayerInfoOverlay.ts (1)
164-181: Visibility check requires exactly zero fog - may be too strict.The check
fogValue === 0means player info only shows when there is absolutely no fog. Consider using a small tolerance (e.g.,fogValue < 0.1) to handle floating-point precision and provide a smoother user experience.src/core/game/GameImpl.ts (1)
160-160: Good: FogOfWar shares the FFA player-adding path.This correctly treats FogOfWar as a variant of FFA for team assignment purposes.
src/core/game/FogOfWar.ts (1)
143-148: Bug: Comparing enum to string literal will always fail.
unit.type()returns aUnitTypeenum value, not a string. The comparisonunit.type() === "Warship"will always befalse. Use the enum values instead.🔎 Use enum values for comparison
- if (unit.type() === "Warship" || unit.type() === "Transport") { + if (unit.type() === UnitType.Warship || unit.type() === UnitType.TransportShip) {Note: Also verify if "Transport" should be
UnitType.TransportShipor another unit type.⛔ Skipped due to learnings
Learnt from: scottanderson Repo: openfrontio/OpenFrontIO PR: 784 File: src/core/game/StatsImpl.ts:143-159 Timestamp: 2025-05-18T23:36:12.847Z Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.Learnt from: scottanderson Repo: openfrontio/OpenFrontIO PR: 784 File: src/core/game/StatsImpl.ts:143-159 Timestamp: 2025-05-18T23:36:12.847Z Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.Learnt from: Dimitrije-V Repo: openfrontio/OpenFrontIO PR: 1814 File: src/client/graphics/layers/PlayerActionHandler.ts:0-0 Timestamp: 2025-08-14T14:05:00.867Z Learning: In TypeScript, when dealing with union types like NukeType (derived from UnitType), it's better to accept the broader type (UnitType) as a parameter and use runtime type checking rather than trying to narrow the parameter type at compile time. The approach of keeping flexible input parameters with runtime validation followed by properly typed state/storage is more practical than over-constraining function signatures.src/client/graphics/layers/UnitLayer.ts (1)
55-66: Good addition of FogOfWarLayer dependency.The optional
fogOfWarLayerparameter allows fog-aware rendering without breaking existing code. The JSDoc comment is in Portuguese though - consider using English for consistency with the rest of the codebase.src/client/graphics/layers/RadialMenuElements.ts (1)
124-148: Good helper function for fog visibility check.The
isPlayerVisibleInFoghelper centralizes fog visibility logic. This is the right approach.However, note that
GameMode.FogOfWardoes not exist (same TypeScript error as other files). This needs fixing.resources/lang/en.json (3)
355-359: New Fog of War game mode key added.The new
game_mode.fog_of_warkey looks good. Make sure the correspondingGameModeenum in TypeScript includes this value with a matching name.
605-611: New leaderboard keys for Fog of War mode.Good additions for the FoW leaderboard features:
local_mode,global_modefor mode toggleeliminated,viewfor spectatingswitch_to_global,switch_to_localfor toggle buttonsThese align with the Leaderboard.ts changes.
279-281: Verify that public lobby labels still work.The team labels changed from
"{team_count} of 2 (Duos)"to"of 2 (Duos)". Verify that the UI code formatting these strings still produces readable output without the team_count prefix.src/client/graphics/layers/Leaderboard.ts (3)
38-41: Good: New event type for viewing player vision.The
ViewPlayerVisionEventclass follows the existing event pattern in the codebase. Simple and clean.
47-50: Good: Proper typing for fogOfWarLayer.Unlike other files, this one uses the proper
FogOfWarLayer | nulltype. The JSDoc is in Portuguese though - consider English for consistency.
112-120: Good defensive coding for setObservedPlayer.Checking
typeof this.fogOfWarLayer.setObservedPlayer === 'function'before calling is good practice when the layer reference might be of different types or mocked.src/client/graphics/GameRenderer.ts (1)
347-361: Good: Event handling for ViewPlayerVisionEvent.The event handler correctly finds and updates both the FogOfWarLayer and Leaderboard when switching player vision. However, use
instanceofinstead of string comparison for reliability.src/client/graphics/layers/FogOfWarLayer.ts (1)
1-5: New FogOfWarLayer implements core fog rendering.The layer structure follows the existing pattern with
shouldTransform(),init(),redraw(),tick(), andrenderLayer(). Good composition-based design.src/client/graphics/layers/NameLayer.ts (1)
124-124: Critical:GameMode.FogOfWardoes not exist.The GameMode enum in
src/core/game/Game.tsonly defines:
FFA = "Free For All"Team = "Team"Add
FogOfWarto the enum or change line 124 to use an existing GameMode value.⛔ Skipped due to learnings
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.Learnt from: Ble4Ch Repo: openfrontio/OpenFrontIO PR: 1063 File: src/core/configuration/PastelThemeDark.ts:53-53 Timestamp: 2025-06-10T09:56:44.473Z Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.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.
| getFogOfWarLayer() { | ||
| return this.layers.find(layer => layer.constructor.name === 'FogOfWarLayer') as any; | ||
| } |
There was a problem hiding this comment.
Fragile class name comparison will break with minification.
Using layer.constructor.name === 'FogOfWarLayer' will fail when code is minified (class names get mangled). Use instanceof instead:
getFogOfWarLayer() {
- return this.layers.find(layer => layer.constructor.name === 'FogOfWarLayer') as any;
+ return this.layers.find(layer => layer instanceof FogOfWarLayer) as FogOfWarLayer | undefined;
}Same issue on line 297, 351, and 357.
📝 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.
| getFogOfWarLayer() { | |
| return this.layers.find(layer => layer.constructor.name === 'FogOfWarLayer') as any; | |
| } | |
| getFogOfWarLayer() { | |
| return this.layers.find(layer => layer instanceof FogOfWarLayer) as FogOfWarLayer | undefined; | |
| } |
🤖 Prompt for AI Agents
In src/client/graphics/GameRenderer.ts around lines 460-462 (and also address
the similar fragile checks at lines ~297, ~351, and ~357), the code compares
layer.constructor.name to 'FogOfWarLayer' which will break when minified;
replace these string-based class name checks with direct instanceof checks
against the FogOfWarLayer class (e.g., use layer instanceof FogOfWarLayer) so
the runtime type check remains robust after minification; ensure you import or
reference FogOfWarLayer in this file and update each occurrence accordingly.
| private lastVisionUpdate = 0; | ||
| private readonly VISION_INTERVAL = 100; | ||
|
|
||
| private _observedPlayer: any = null; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use proper typing instead of any for _observedPlayer.
- private _observedPlayer: any = null;
+ private _observedPlayer: PlayerView | null = null;This requires importing PlayerView from GameView.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/client/graphics/layers/FogOfWarLayer.ts around line 36, the field
declaration uses the unsafe type "any" for _observedPlayer; replace it with the
proper PlayerView type by importing PlayerView from the GameView module and
updating the declaration to use PlayerView | null (or the appropriate nullable
PlayerView type used across the codebase). Ensure the import statement is added
at the top of the file and adjust any usages of _observedPlayer to satisfy the
stricter type (e.g., null checks or optional chaining) so the file compiles
without any implicit any.
| async tick() { | ||
| if (this.game.config().gameConfig().gameMode !== GameMode.FogOfWar) return; | ||
| const now = performance.now(); | ||
|
|
||
| if (now - this.lastVisionUpdate >= this.VISION_INTERVAL) { | ||
| await this.updateVision(); | ||
| this.lastVisionUpdate = now; | ||
| } | ||
|
|
||
| this.handleAllianceChanges(); | ||
| this.applyPermanentTerritory(); | ||
| this.updateMobileUnitFogVisibility(); | ||
| if (this.hasDirtyChunks()) this.renderFog(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for the Layer interface definition
rg -A 10 "interface Layer" --type tsRepository: openfrontio/OpenFrontIO
Length of output: 506
🏁 Script executed:
# Also check if there are implementations of tick() method to see if async is used elsewhere
rg "tick\s*\(\s*\)" --type ts -A 2 | head -50Repository: openfrontio/OpenFrontIO
Length of output: 2593
🏁 Script executed:
# Look for where tick() is called in the rendering loop
rg "\.tick\(\)" --type ts -B 2 -A 2Repository: openfrontio/OpenFrontIO
Length of output: 1274
🏁 Script executed:
# Check AdTimer implementation
cat -n src/client/graphics/layers/AdTimer.ts | head -40Repository: openfrontio/OpenFrontIO
Length of output: 905
🏁 Script executed:
# See if there are any comments or special handling around async tick methods
rg "async tick" --type ts -B 3 -A 10Repository: openfrontio/OpenFrontIO
Length of output: 3990
Fix type violation: async tick() does not match Layer interface.
The Layer interface defines tick?: () => void (synchronous), but FogOfWarLayer.tick() is declared as async, returning Promise<void>. The GameRenderer calls tick() without awaiting:
this.layers.forEach((l) => l.tick?.());This means updateVision() executes as a fire-and-forget promise. Either make tick() synchronous, or update the Layer interface and GameRenderer to handle async tick() methods. Note: AdTimer, MainRadialMenu, and PlayerPanel have the same issue.
🧰 Tools
🪛 GitHub Actions: 🚀 Deploy
[error] 106-106: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
🤖 Prompt for AI Agents
In src/client/graphics/layers/FogOfWarLayer.ts around lines 105–118: the method
is declared async but Layer.tick is synchronous, causing fire-and-forget
behavior when GameRenderer calls layers.forEach(l => l.tick?.()); either make
tick synchronous or make the interface and renderer support async ticks. Fix
option A (recommended minimal change): remove async from FogOfWarLayer.tick,
ensure updateVision() is called without await (or refactor updateVision to
expose a synchronous fast-path) so tick returns void; repeat for AdTimer,
MainRadialMenu, and PlayerPanel. Fix option B (broader change): update the Layer
interface to tick?: () => Promise<void>, change GameRenderer to await ticks
(e.g., iterate with for..of and await l.tick?.() or collect promises and await
Promise.all) and update all layer implementations to return Promise<void>;
choose one option and apply consistently across the codebase.
| redraw() { this.markAllDirty(); this.renderFog(); } | ||
|
|
||
| async tick() { | ||
| if (this.game.config().gameConfig().gameMode !== GameMode.FogOfWar) return; |
There was a problem hiding this comment.
Critical: GameMode.FogOfWar does not exist.
Same issue as other files. This enum value must be added to the GameMode enum or use the correct existing value.
Also applies to: 121-121
🧰 Tools
🪛 GitHub Actions: 🚀 Deploy
[error] 106-106: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
🤖 Prompt for AI Agents
In src/client/graphics/layers/FogOfWarLayer.ts around lines 106 and 121, the
code references GameMode.FogOfWar which does not exist; either add a FogOfWar
member to the GameMode enum where it is defined or change these comparisons to
the correct existing enum value. Locate the GameMode enum declaration, add a
FogOfWar entry (matching the project's naming/serialization style) and update
any imports/types if necessary; alternatively, replace GameMode.FogOfWar in this
file with the correct existing enum constant used for fog-of-war gameplay across
the codebase.
| // This ensures that only areas with active vision sources remain visible | ||
| this.visionBuffer.fill(255); | ||
|
|
||
| const player = this._observedPlayer || this.game.myPlayer(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
ESLint: Use nullish coalescing ?? instead of ||.
The || operator can incorrectly fall through on falsy values like 0 or "". Use ?? for null/undefined checks:
- const player = this._observedPlayer || this.game.myPlayer();
+ const player = this._observedPlayer ?? this.game.myPlayer();
- const type = unit.type?.() || "";
+ const type = unit.type?.() ?? "";Apply this pattern to all flagged lines.
Also applies to: 152-152, 270-270, 342-342, 474-474, 487-487, 552-552, 586-586
🧰 Tools
🪛 GitHub Check: 🔍 ESLint
[failure] 132-132:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator.
🤖 Prompt for AI Agents
In src/client/graphics/layers/FogOfWarLayer.ts around lines 132, 152, 270, 342,
474, 487, 552, and 586, replace uses of the logical OR operator (||) used for
defaulting with the nullish coalescing operator (??) so that only null or
undefined trigger the fallback; update each expression like "a || b" to "a ?? b"
(or equivalent) at the specified lines to avoid incorrectly falling through on
falsy values like 0 or "".
| <div | ||
| class="option-card ${this.gameMode === GameMode.FogOfWar | ||
| ? "selected" | ||
| : ""}" | ||
| @click=${() => this.handleGameModeSelection(GameMode.FogOfWar)} | ||
| > | ||
| <div class="option-card-title"> | ||
| ${translateText("game_mode.fog_of_war")} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Build blocker: GameMode.FogOfWar is not defined in the enum.
The pipeline fails with TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'. You need to add FogOfWar to the GameMode enum in src/core/game/Game.ts before this code can compile.
🔎 Add FogOfWar to GameMode enum
In src/core/game/Game.ts, add FogOfWar to the GameMode enum:
export enum GameMode {
FFA = "FFA",
Team = "Team",
FogOfWar = "FogOfWar", // Add this line
}🧰 Tools
🪛 GitHub Actions: 🚀 Deploy
[error] 194-194: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
[error] 197-197: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
🤖 Prompt for AI Agents
In src/client/SinglePlayerModal.ts around lines 193-202 the code references
GameMode.FogOfWar which doesn't exist; open src/core/game/Game.ts and add a
FogOfWar member to the exported GameMode enum (e.g., FogOfWar = "FogOfWar") so
the property exists and TypeScript compiles; save and run the build/tests to
verify the pipeline error is resolved.
| class="option-card ${this.teamCount === o | ||
| ? "selected" | ||
| : ""}" | ||
| : ""} | ||
| @click=${() => this.handleTeamCountSelection(o)} |
There was a problem hiding this comment.
Syntax error: Missing closing quote on class attribute.
Line 231 has a missing closing quote for the class attribute, which will break the template rendering.
🔎 Fix the class attribute
<div
class="option-card ${this.teamCount === o
? "selected"
- : ""}
+ : ""}"
@click=${() => this.handleTeamCountSelection(o)}
>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/client/SinglePlayerModal.ts around lines 229 to 232, the template has a
missing closing quote on the class attribute which breaks rendering; fix it by
closing the class attribute string after the conditional interpolation (e.g.,
ensure the class attribute ends with the closing quote before the @click), and
verify spacing so the @click binding is outside the quoted attribute.
| public initialize(): void { | ||
| // Para cada jogador, cria um conjunto vazio de tiles explorados | ||
| for (const player of this.game.players()) { | ||
| this.exploredTiles.set(player.id(), new Set<TileRef>()); | ||
| } | ||
| } |
There was a problem hiding this comment.
New players added after initialization won't have fog tracking.
initialize() creates entries only for players existing at initialization time. If players are added later (e.g., bots joining), they won't have an explored tiles set. Consider lazy initialization in markAsExplored or isVisible.
🔎 Add lazy initialization
public markAsExplored(player: Player, tiles: Set<TileRef> | TileRef[]): void {
const playerId = player.id();
- const exploredSet = this.exploredTiles.get(playerId);
-
- if (!exploredSet) {
- console.warn(`No explored tiles set found for player ${playerId}`);
- return;
+ let exploredSet = this.exploredTiles.get(playerId);
+ if (!exploredSet) {
+ exploredSet = new Set<TileRef>();
+ this.exploredTiles.set(playerId, exploredSet);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/core/game/FogOfWar.ts around lines 20 to 25, initialize() only creates
exploredTiles entries for players present at startup so players added later
won’t have fog tracking; modify the code to lazily initialize an entry whenever
you access exploredTiles (e.g., in markAsExplored and isVisible) by checking if
exploredTiles.has(player.id()) and if not creating and setting a new
Set<TileRef>() before using it, or alternatively hook into the player-add event
to create the Set when a player is added.
| // Inicializa o sistema de Fog of War se estiver no modo correto | ||
| if (_config.gameConfig().gameMode === GameMode.FogOfWar) { | ||
| this.fogOfWarManager = new FogOfWarManager(this); | ||
| this.fogOfWarManager.initialize(); | ||
| } |
There was a problem hiding this comment.
Initialization timing issue: No players exist when initialize() is called.
FogOfWarManager.initialize() is called in the constructor before addPlayers(). The initialize() method iterates over game.players() which is empty at this point, so no player entries are created.
🔎 Move initialization after players are added
// Inicializa o sistema de Fog of War se estiver no modo correto
if (_config.gameConfig().gameMode === GameMode.FogOfWar) {
this.fogOfWarManager = new FogOfWarManager(this);
- this.fogOfWarManager.initialize();
}
if (_config.gameConfig().gameMode === GameMode.Team) {
this.populateTeams();
}
this.addPlayers();
+
+ // Initialize fog after players are added
+ if (this.fogOfWarManager) {
+ this.fogOfWarManager.initialize();
+ }
}📝 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.
| // Inicializa o sistema de Fog of War se estiver no modo correto | |
| if (_config.gameConfig().gameMode === GameMode.FogOfWar) { | |
| this.fogOfWarManager = new FogOfWarManager(this); | |
| this.fogOfWarManager.initialize(); | |
| } | |
| // Inicializa o sistema de Fog of War se estiver no modo correto | |
| if (_config.gameConfig().gameMode === GameMode.FogOfWar) { | |
| this.fogOfWarManager = new FogOfWarManager(this); | |
| } | |
| if (_config.gameConfig().gameMode === GameMode.Team) { | |
| this.populateTeams(); | |
| } | |
| this.addPlayers(); | |
| // Initialize fog after players are added | |
| if (this.fogOfWarManager) { | |
| this.fogOfWarManager.initialize(); | |
| } |
🧰 Tools
🪛 GitHub Actions: 🚀 Deploy
[error] 106-106: TS2339: Property 'FogOfWar' does not exist on type 'typeof GameMode'.
🤖 Prompt for AI Agents
In src/core/game/GameImpl.ts around lines 105-109, FogOfWarManager.initialize()
is being called in the constructor before any players are added, so
game.players() is empty and no player fog entries are created; move the
FogOfWarManager initialization out of the constructor and call
fogOfWarManager.initialize() after players are added (for example at the end of
addPlayers() or immediately after addPlayers() is invoked), or defer
initialization by storing the manager instance in the constructor and invoking
initialize() once player setup completes so that initialize() can iterate
non-empty game.players().
| getVisibleTiles(player: Player): Set<TileRef> { | ||
| if (!this.fogOfWarManager) { | ||
| // If not in Fog of War mode, returns all tiles | ||
| const allTiles = new Set<TileRef>(); | ||
| this.map().forEachTile(tile => allTiles.add(tile)); | ||
| return allTiles; | ||
| } | ||
| return this.fogOfWarManager.getVisibleTiles(player); | ||
| } |
There was a problem hiding this comment.
Expensive fallback: Creating a Set of all tiles on every call.
When not in FogOfWar mode, getVisibleTiles iterates over every tile to build a new Set. This is O(n) for map size on every call. Consider caching this or returning a sentinel value indicating "all visible".
🔎 Return null or use a cached set
getVisibleTiles(player: Player): Set<TileRef> | null {
if (!this.fogOfWarManager) {
- // If not in Fog of War mode, returns all tiles
- const allTiles = new Set<TileRef>();
- this.map().forEachTile(tile => allTiles.add(tile));
- return allTiles;
+ // If not in Fog of War mode, return null to indicate all tiles are visible
+ return null;
}
return this.fogOfWarManager.getVisibleTiles(player);
}Callers should check for null to mean "everything is visible".
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/core/game/GameImpl.ts around lines 1016 to 1024 the fallback path builds
a new Set of all tiles on every call which is O(n) per invocation; instead
either return a sentinel (e.g., null or a documented ALL_VISIBLE value) so
callers treat null as “everything visible”, or cache a single immutable Set of
all tiles and return that to avoid repeated allocations — implement a
lazily-initialized cachedAllTiles field (and invalidate/update it when the map
changes) or change the method contract to allow null/ALL_VISIBLE and update all
callers accordingly.
(based on files shared on 2 Jan 2026. Misses files like Game.ts that where present in the previously shared files https://github.com/openfrontio/OpenFrontIO/pull/2415/changes. Deliberately removed the language files besides the en.json one here, as the languages are maintained from Crowdin. Rest is as shared by @MEePALAa)
DRAFT MEANT AS TEST
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
DISCORD_USERNAME