feat: Nuke Trajectory SAM intercept prediction #2541
Conversation
WalkthroughEnhances nuke trajectory and SAM radius visualization by tracking bomb targetability segments, computing SAM interception points, and rendering multi-state trajectory previews. SAMRadiusLayer updated to use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/core/execution/NukeExecution.ts (1)
191-193: Extract duplicatetargetRangeSquaredcalculation.The same logic appears in both
getTrajectory()(lines 191-193) andupdateNukeTargetable()(lines 222-224). Consider a small helper method to keep this DRY:+ private getTargetRangeSquared(): number { + return this.mg.config().defaultNukeInvulnerability() + ? this.mg.config().defaultNukeTargetableRange() ** 2 + : Number.MAX_VALUE; + } + private getTrajectory(target: TileRef): TrajectoryTile[] { const trajectoryTiles: TrajectoryTile[] = []; - const targetRangeSquared = this.mg.config().defaultNukeInvulnerability() - ? this.mg.config().defaultNukeTargetableRange() ** 2 - : Number.MAX_VALUE; + const targetRangeSquared = this.getTargetRangeSquared();Then use the same helper in
updateNukeTargetable().Also applies to: 222-224
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts (3)
258-260: DuplicatetargetRangeSquaredlogic across files.This calculation mirrors what's in
NukeExecution.ts. For now, the duplication is acceptable since preview layer runs on client and execution runs on game logic, but keep them in sync if the formula changes.One option: move this to a shared helper on
Configlikeconfig().nukeTargetRangeSquared()to avoid drift.
302-316: Minor: cache SAM range squared outside inner loop.
r ** 2is computed for each SAM on every trajectory point iteration. Consider caching it per SAM before the loop:+ // Pre-compute range squared for each SAM + const samRangesSquared = samLaunchers.map((sam) => { + const r = this.game.config().samRange(sam.level()); + return { sam, rangeSquared: r * r }; + }); // Check trajectory for (let i = 0; i < this.trajectoryPoints.length; i++) { const tile = this.trajectoryPoints[i]; - for (const sam of samLaunchers) { - const samTile = sam.tile(); - const r = this.game.config().samRange(sam.level()); - if (this.game.euclideanDistSquared(tile, samTile) <= r ** 2) { + for (const { sam, rangeSquared } of samRangesSquared) { + if (this.game.euclideanDistSquared(tile, sam.tile()) <= rangeSquared) { this.targetedIndex = i; break; } }This is minor since you noted performance is acceptable in testing.
25-25: Unused field:samLaunchersMap is never populated.Since
updateSAMs()is commented out, this Map stays empty. The code on lines 293-300 queriesgame.units(UnitType.SAMLauncher)directly instead. Either remove the field or enableupdateSAMs()for the intended caching behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts(7 hunks)src/client/graphics/layers/SAMRadiusLayer.ts(5 hunks)src/core/configuration/Config.ts(1 hunks)src/core/configuration/DefaultConfig.ts(1 hunks)src/core/execution/NukeExecution.ts(2 hunks)src/core/game/GameView.ts(1 hunks)tests/util/TestConfig.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
📚 Learning: 2025-10-08T17:14:49.369Z
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: In `ClientGameRunner.ts`, the `myPlayer` field is always set when `shouldPreventWindowClose()` is called, so the null check in that method is sufficient without needing to fetch it again from `gameView.playerByClientID()`.
Applied to files:
src/core/game/GameView.ts
📚 Learning: 2025-06-02T01:42:57.407Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1000
File: src/core/configuration/Config.ts:83-83
Timestamp: 2025-06-02T01:42:57.407Z
Learning: PlayerView implements the Player interface, so methods accepting Player parameters can also accept PlayerView arguments without issue.
Applied to files:
src/core/game/GameView.ts
📚 Learning: 2025-08-24T11:13:08.495Z
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.tssrc/core/configuration/Config.tssrc/core/configuration/DefaultConfig.tssrc/core/execution/NukeExecution.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/NukeTrajectoryPreviewLayer.tssrc/client/graphics/layers/SAMRadiusLayer.ts
📚 Learning: 2025-11-26T20:49:29.110Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.110Z
Learning: In GameView.ts, when usesSharedTileState is true (SAB mode), packedTileUpdates contains unpacked tile references as BigInt(tileRef) only, because all tile state lives in the shared Uint16Array. In non-SAB mode, packedTileUpdates contains packed TileUpdate bigints in the format (tileRef << 16n | state), which must be decoded via updateTile(tu). Therefore, Number(tu) is correct in SAB mode and shifting right by 16 bits would be wrong.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.tssrc/core/configuration/DefaultConfig.tssrc/core/execution/NukeExecution.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/core/configuration/Config.tssrc/core/configuration/DefaultConfig.tstests/util/TestConfig.tssrc/core/execution/NukeExecution.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.
Applied to files:
src/core/configuration/Config.tssrc/core/execution/NukeExecution.ts
📚 Learning: 2025-11-12T23:11:34.445Z
Learnt from: MaxHT0x
Repo: openfrontio/OpenFrontIO PR: 2262
File: src/core/configuration/DefaultConfig.ts:806-832
Timestamp: 2025-11-12T23:11:34.445Z
Learning: In src/core/configuration/DefaultConfig.ts, JSDoc documentation for configuration methods should not be added inline, as it was requested that documentation be placed elsewhere in the codebase.
Applied to files:
src/core/configuration/DefaultConfig.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/core/execution/NukeExecution.ts
📚 Learning: 2025-05-18T23:36:12.847Z
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.
Applied to files:
src/core/execution/NukeExecution.ts
📚 Learning: 2025-08-23T07:48:19.060Z
Learnt from: ElMelchizedek
Repo: openfrontio/OpenFrontIO PR: 1876
File: src/core/execution/FakeHumanExecution.ts:470-473
Timestamp: 2025-08-23T07:48:19.060Z
Learning: In FakeHumanExecution.ts DefensePost placement logic, returning -Infinity from structureSpawnTileValue when no sampled border tiles neighbor enemies is intentional. The logic samples up to 50 border tiles as a heuristic - if none are adjacent to enemies, it assumes DefensePost placement is unnecessary and aborts the entire placement attempt rather than continuing to evaluate individual tiles.
Applied to files:
src/core/execution/NukeExecution.ts
🧬 Code graph analysis (3)
src/core/game/GameView.ts (1)
src/core/game/GameImpl.ts (1)
player(472-478)
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts (3)
src/core/game/GameMap.ts (1)
TileRef(3-3)src/core/game/GameView.ts (1)
unit(673-675)src/core/execution/NukeExecution.ts (1)
owner(327-329)
src/client/graphics/layers/SAMRadiusLayer.ts (1)
src/core/game/GameView.ts (2)
tile(106-108)PlayerView(181-457)
🔇 Additional comments (11)
src/core/game/GameView.ts (1)
607-609: Clean helper method for local player identification.This is a nice, simple utility. The optional chaining on
myPlayer()handles the null case correctly — when there's no local player, comparison yieldsfalseas expected.src/client/graphics/layers/SAMRadiusLayer.ts (3)
184-191: Constants exposed for easy adjustment — nice touch.These color and width constants at the top of the method make future visual tweaks straightforward. Clear naming makes intent obvious.
263-264: Correct value-based comparison using smallID.Comparing
PlayerViewobjects bysmallID()instead of reference equality is the right call — different instances representing the same player now compare correctly.
335-345: Color logic handles edge cases well.The cascade from self → friend → enemy is clear. When
myPlayer()is null, the optional chaining ensures we fall through to enemy color, which is sensible behavior for spectators or disconnected states.tests/util/TestConfig.ts (1)
45-47: Test config aligns with default behavior.Returns
trueto matchDefaultConfig, keeping test behavior consistent with production. Tests can subclass and override if they need to test the disabled invulnerability path.src/core/configuration/Config.ts (1)
173-174: Interface addition is clean and well-placed.Good placement alongside other nuke config methods. The comment "If nuke can ever be invulnerable" is brief and clear.
src/core/configuration/DefaultConfig.ts (1)
923-925: Simple implementation, correctly placed.Returns
trueto enable nuke invulnerability by default. Matches the interface and sits nicely with other nuke config methods.src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts (4)
338-355: Good: color and dash constants at top for easy adjustment.The PR notes visuals are non-final. Having these constants grouped at the method start makes future tweaking simple. Currently some pairs are identical (
untargetableAndUntargetedLineColorequalstargetableAndUntargetedLineColor), which is fine if they may diverge later.
367-378: Nice use of inner helper for outline + stroke drawing.The
outlineAndStrokeclosure captures the current state and avoids repeating the two-pass draw logic. Clean approach.
265-289: Correct handling of overlapping spawn/target ranges.The break on lines 277-278 ensures short trajectories where spawn and target ranges overlap don't produce invalid bounds. The
[-1, -1]default is then used in drawing to skip the untargetable segment rendering.
388-455: Drawing logic correctly mirrors trajectory calculation.The rendering loop and the SAM detection loop both skip the untargetable segment (lines 314-315 skip during detection, and drawing handles bounds transitions). This ensures
targetedIndexis never inside the untargetable zone, keeping the visual consistent with game logic.
|
Looks nice |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts (3)
334-334: Consider clearer sentinel value for targetedIndex.Using
this.trajectoryPoints.lengthas a sentinel to indicate "no SAM intercept" is correct but implicit. A named constant or -1 would make the intent clearer when checking this value elsewhere.Example using -1:
// Find the point where SAM can intercept - this.targetedIndex = this.trajectoryPoints.length; + this.targetedIndex = -1; // Check trajectory for (let i = 0; i < this.trajectoryPoints.length; i++) { const tile = this.trajectoryPoints[i]; for (const [, sam] of this.enemySAMLaunchers.entries()) { const samTile = sam.tile(); const r = this.game.config().samRange(sam.level()); if (this.game.euclideanDistSquared(tile, samTile) <= r ** 2) { this.targetedIndex = i; break; } } - if (this.targetedIndex !== this.trajectoryPoints.length) break; + if (this.targetedIndex !== -1) break; // Jump over untargetable segment if (i === this.untargetableSegmentBounds[0]) i = this.untargetableSegmentBounds[1] - 1; }Then update rendering checks accordingly.
371-393: Move rendering constants to class level.The PR description states "Color/outline constants exposed at the start of relevant files" but these are currently defined inside the method. Moving them to class-level private readonly fields improves maintainability and makes them easier to adjust.
Move constants to the top of the class:
export class NukeTrajectoryPreviewLayer implements Layer { // Rendering constants for trajectory visualization private readonly UNTARGETED_OUTLINE_COLOR = "rgba(140, 140, 140, 1)"; private readonly TARGETED_OUTLINE_COLOR = "rgba(150, 90, 90, 1)"; private readonly TARGETED_LOCATION_COLOR = "rgba(255, 0, 0, 1)"; private readonly UNTARGETABLE_UNTARGETED_LINE_COLOR = "rgba(255, 255, 255, 1)"; private readonly TARGETABLE_UNTARGETED_LINE_COLOR = "rgba(255, 255, 255, 1)"; private readonly UNTARGETABLE_TARGETED_LINE_COLOR = "rgba(255, 80, 80, 1)"; private readonly TARGETABLE_TARGETED_LINE_COLOR = "rgba(255, 80, 80, 1)"; private readonly OUTLINE_EXTRA_WIDTH = 1.5; private readonly LINE_WIDTH = 1.25; private readonly UNTARGETABLE_UNTARGETED_LINE_DASH = [2, 6]; private readonly TARGETABLE_UNTARGETED_LINE_DASH = [8, 4]; private readonly UNTARGETABLE_TARGETED_LINE_DASH = [2, 6]; private readonly TARGETABLE_TARGETED_LINE_DASH = [8, 4]; // ... rest of classThen reference them in the method using
this.CONSTANT_NAME.
442-469: Duplicate color/dash assignment logic.Lines 442-450 and 461-469 contain similar logic for setting
currentOutlineColor,currentLineColor, andcurrentLineDashbased on whetheri >= this.targetedIndex. This duplication could be extracted into a small helper function.Example helper:
const setColorsForSegment = (isUntargetable: boolean) => { if (i >= this.targetedIndex) { currentOutlineColor = targetedOutlineColor; currentLineColor = isUntargetable ? untargetableAndTargetedLineColor : targetableAndTargetedLineColor; currentLineDash = isUntargetable ? untargetableAndTargetedLineDash : targetableAndTargetedLineDash; } else { currentOutlineColor = untargetedOutlineColor; currentLineColor = isUntargetable ? untargetableAndUntargetedLineColor : targetableAndUntargetedLineColor; currentLineDash = isUntargetable ? untargetableAndUntargetedLineDash : targetableAndUntargetedLineDash; } };Then use
setColorsForSegment(true)at line 442 andsetColorsForSegment(false)at line 461.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts(7 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
📚 Learning: 2025-08-24T11:13:08.495Z
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.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/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-11-26T20:49:29.110Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.110Z
Learning: In GameView.ts, when usesSharedTileState is true (SAB mode), packedTileUpdates contains unpacked tile references as BigInt(tileRef) only, because all tile state lives in the shared Uint16Array. In non-SAB mode, packedTileUpdates contains packed TileUpdate bigints in the format (tileRef << 16n | state), which must be decoded via updateTile(tu). Therefore, Number(tu) is correct in SAB mode and shifting right by 16 bits would be wrong.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.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/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
🧬 Code graph analysis (1)
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts (2)
src/core/game/GameMap.ts (1)
TileRef(3-3)src/core/game/GameView.ts (3)
UnitView(44-179)unit(673-675)PlayerView(181-457)
🔇 Additional comments (3)
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts (3)
19-27: LGTM! Clear state tracking for SAM interception.The new fields appropriately track untargetable zones, SAM intercept points, spawn tile caching, and enemy SAM launchers. The types are clean and the comment explains the caching strategy well.
58-61: Good - SAM updates now enabled.The
updateSAMs()call is now active, addressing the previous review feedback. This ensures SAM tracking stays current each tick.
90-90: Due to repository access issues, I cannot directly verify theisMyPlayer()method in the codebase. However, the review comment's verification request is valid and necessary. Here is the rewritten comment:
Verify
isMyPlayer()method exists in GameView.The code uses
this.game.isMyPlayer(unit.owner())but this method was not visible in the provided GameView snippets. The PR summary indicates "adds a small GameView helper for identifying the local player," suggesting it was added in this PR. Please confirm the method exists and check its implementation to ensure it correctly identifies the local player.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts(7 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
📚 Learning: 2025-08-24T11:13:08.495Z
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.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/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-18T17:54:01.311Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.311Z
Learning: In src/core/execution/FakeHumanExecution.ts, MIRVs and ground attacks should not be mutually exclusive. The considerMIRV() method should not short-circuit maybeAttack() - both actions can occur in the same tick.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-08T17:14:49.369Z
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-08-23T07:48:19.060Z
Learnt from: ElMelchizedek
Repo: openfrontio/OpenFrontIO PR: 1876
File: src/core/execution/FakeHumanExecution.ts:470-473
Timestamp: 2025-08-23T07:48:19.060Z
Learning: In FakeHumanExecution.ts DefensePost placement logic, returning -Infinity from structureSpawnTileValue when no sampled border tiles neighbor enemies is intentional. The logic samples up to 50 border tiles as a heuristic - if none are adjacent to enemies, it assumes DefensePost placement is unnecessary and aborts the entire placement attempt rather than continuing to evaluate individual tiles.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-11-26T22:27:31.808Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/worker/SharedTileRing.ts:55-63
Timestamp: 2025-11-26T22:27:31.808Z
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/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-17T19:16:57.699Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:711-723
Timestamp: 2025-10-17T19:16:57.699Z
Learning: In the OpenFrontIO game, MIRVs (Multiple Independently targetable Reentry Vehicles) have an area of effect with a radius of 1500 tiles around the aim point. To maximize efficiency, bots should aim at the center of the target player's territory rather than selecting random tiles. This differs from the initial understanding that MIRVs covered an entire player's territory regardless of aim point.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-11-26T20:49:29.110Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.110Z
Learning: In GameView.ts, when usesSharedTileState is true (SAB mode), packedTileUpdates contains unpacked tile references as BigInt(tileRef) only, because all tile state lives in the shared Uint16Array. In non-SAB mode, packedTileUpdates contains packed TileUpdate bigints in the format (tileRef << 16n | state), which must be decoded via updateTile(tu). Therefore, Number(tu) is correct in SAB mode and shifting right by 16 bits would be wrong.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.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/NukeTrajectoryPreviewLayer.ts
🔇 Additional comments (1)
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts (1)
27-27: Verify initial SAM tracking on game load.
enemySAMLaunchersstarts empty andupdateSAMs()only processes unit updates. If a player joins mid-game or reloads the page, existing enemy SAMs won't be tracked until they're updated (e.g., level up, ownership change). This could cause incorrect intercept predictions initially.If initial units aren't sent as updates, add initialization in
init()or firsttick():private initializeSAMs() { if (this.enemySAMLaunchers.size > 0) return; // Already initialized const myPlayer = this.game.myPlayer(); if (!myPlayer) return; for (const player of this.game.players()) { if (this.game.isMyPlayer(player.id()) || myPlayer.isFriendly(player.id())) { continue; } for (const sam of player.units(UnitType.SAMLauncher)) { this.enemySAMLaunchers.set(sam.id(), sam); } } }Also applies to: 72-99
Removes SAM tracking in favor of nearbyUnits() call. Unsure if perf has any noticeable change (might slightly worsen) but simplifies logic enough and scales better with more SAMs on map.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/client/graphics/layers/SAMRadiusLayer.ts (1)
184-191: Consider extracting color constants to class-level.The color and styling constants (lines 184-191) are defined inside the
drawCirclesUnion()method. Since these define the visual appearance of SAM radii and are referenced in the PR description as "exposed near the top," consider moving them to class-level constants for easier customization and consistency with the stated design goal.Apply this diff:
export class SAMRadiusLayer implements Layer { private readonly canvas: HTMLCanvasElement; private readonly context: CanvasRenderingContext2D; private readonly samLaunchers: Map<number, number> = new Map(); + + // SAM radius visual styling + private readonly OUTLINE_COLOR = "rgba(0, 0, 0, 1)"; + private readonly LINE_COLOR_SELF = "rgba(0, 255, 0, 1)"; + private readonly LINE_COLOR_ENEMY = "rgba(255, 0, 0, 1)"; + private readonly LINE_COLOR_FRIEND = "rgba(255, 255, 0, 1)"; + private readonly EXTRA_OUTLINE_WIDTH = 1; + private readonly LINE_WIDTH = 2; + private readonly LINE_DASH = [12, 6]; + private needsRedraw = true;Then reference these constants in the method instead of defining them inline.
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts (1)
302-327: Extract visual constants to class level for easier customization.Similar to
SAMRadiusLayer, the color, width, and dash constants (lines 302-327) define the visual appearance and are mentioned in the PR description as being "exposed near the top" for easy tweaking. Moving these to class-level constants would align with that goal and improve maintainability.Apply this diff:
export class NukeTrajectoryPreviewLayer implements Layer { + // Trajectory visual styling + private readonly UNTARGETED_OUTLINE_COLOR = "rgba(140, 140, 140, 1)"; + private readonly TARGETED_OUTLINE_COLOR = "rgba(150, 90, 90, 1)"; + private readonly SYMBOL_OUTLINE_COLOR = "rgba(0, 0, 0, 1)"; + private readonly TARGETED_LOCATION_COLOR = "rgba(255, 0, 0, 1)"; + private readonly UNTARGETABLE_LINE_COLOR = "rgba(255, 255, 255, 1)"; + private readonly TARGETABLE_LINE_COLOR = "rgba(255, 255, 255, 1)"; + private readonly TARGETED_LINE_COLOR = "rgba(255, 80, 80, 1)"; + private readonly OUTLINE_EXTRA_WIDTH = 1.5; + private readonly LINE_WIDTH = 1.25; + private readonly X_LINE_WIDTH = 2; + private readonly X_SIZE = 6; + private readonly UNTARGETABLE_DASH = [2, 6]; + private readonly TARGETABLE_DASH = [8, 4]; + // Trajectory preview state private mousePos = { x: 0, y: 0 };Then reference these constants throughout the rendering method.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts(4 hunks)src/client/graphics/layers/SAMRadiusLayer.ts(5 hunks)src/core/game/GameView.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
📚 Learning: 2025-08-24T11:13:08.495Z
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.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/NukeTrajectoryPreviewLayer.tssrc/client/graphics/layers/SAMRadiusLayer.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/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-18T17:54:01.311Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.311Z
Learning: In src/core/execution/FakeHumanExecution.ts, MIRVs and ground attacks should not be mutually exclusive. The considerMIRV() method should not short-circuit maybeAttack() - both actions can occur in the same tick.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-08T17:14:49.369Z
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-08-23T07:48:19.060Z
Learnt from: ElMelchizedek
Repo: openfrontio/OpenFrontIO PR: 1876
File: src/core/execution/FakeHumanExecution.ts:470-473
Timestamp: 2025-08-23T07:48:19.060Z
Learning: In FakeHumanExecution.ts DefensePost placement logic, returning -Infinity from structureSpawnTileValue when no sampled border tiles neighbor enemies is intentional. The logic samples up to 50 border tiles as a heuristic - if none are adjacent to enemies, it assumes DefensePost placement is unnecessary and aborts the entire placement attempt rather than continuing to evaluate individual tiles.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-11-26T22:27:31.808Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/worker/SharedTileRing.ts:55-63
Timestamp: 2025-11-26T22:27:31.808Z
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/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-10-17T19:16:57.699Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:711-723
Timestamp: 2025-10-17T19:16:57.699Z
Learning: In the OpenFrontIO game, MIRVs (Multiple Independently targetable Reentry Vehicles) have an area of effect with a radius of 1500 tiles around the aim point. To maximize efficiency, bots should aim at the center of the target player's territory rather than selecting random tiles. This differs from the initial understanding that MIRVs covered an entire player's territory regardless of aim point.
Applied to files:
src/client/graphics/layers/NukeTrajectoryPreviewLayer.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/graphics/layers/NukeTrajectoryPreviewLayer.tssrc/core/game/GameView.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/NukeTrajectoryPreviewLayer.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/NukeTrajectoryPreviewLayer.ts
📚 Learning: 2025-06-02T01:42:57.407Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1000
File: src/core/configuration/Config.ts:83-83
Timestamp: 2025-06-02T01:42:57.407Z
Learning: PlayerView implements the Player interface, so methods accepting Player parameters can also accept PlayerView arguments without issue.
Applied to files:
src/core/game/GameView.ts
🧬 Code graph analysis (2)
src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts (2)
src/core/game/GameMap.ts (1)
TileRef(3-3)src/core/game/GameView.ts (4)
tile(106-108)targetTile(130-132)x(688-690)y(691-693)
src/client/graphics/layers/SAMRadiusLayer.ts (1)
src/core/game/GameView.ts (1)
PlayerView(181-463)
🔇 Additional comments (7)
src/core/game/GameView.ts (1)
389-391: LGTM! Clean implementation of player identity check.The
isMe()method correctly uses optional chaining to handle the null case and compares bysmallID(), which aligns with the ownership comparison pattern used throughout the codebase.src/client/graphics/layers/SAMRadiusLayer.ts (1)
335-345: Rendering logic correctly implements per-owner styling.The outline and inline stroke rendering with conditional colors based on
isMe()andisFriendly()correctly visualizes SAM ownership, making it easy to distinguish self/friend/enemy coverage.src/client/graphics/layers/NukeTrajectoryPreviewLayer.ts (5)
18-19: State variables correctly track trajectory segments.The
untargetableSegmentBoundstuple andtargetedIndexclearly represent the trajectory state needed for multi-phase rendering. The initialization to[-1, -1]and-1properly signals "not found."
256-281: SAM interception logic efficiently detects threats.The SAM detection correctly:
- Uses
nearbyUnits()withmaxSamRange()for efficient spatial querying- Filters friendly SAMs before checking ranges
- Compares squared distances to avoid sqrt
- Skips the untargetable segment to focus on vulnerable trajectory portions
336-367: Inline helper functions are acceptable here.The inline helper functions (
outlineAndStroke,drawUntargetableCircle,drawTargetedX) use closure over thecurrent*state variables, which keeps the code concise. Extracting these to methods would require passing many parameters, so the current approach is pragmatic for single-use helpers.
377-424: Rendering state machine correctly visualizes trajectory phases.The loop handles state transitions at key indices:
- Untargetable segment boundaries → draw circles and switch dash patterns
- SAM intercept point → draw X and switch to "targeted" colors
The approach of stroking accumulated path before each transition, drawing a marker, then starting a new path segment with updated styling correctly produces the multi-state visual.
236-242: Handle overlapping range edge case correctly.When spawn and target ranges overlap (lines 236-241), the code breaks without setting bounds, leaving them at
[-1, -1]. This correctly represents "always targetable". Verify that the rendering logic (lines 387-413) gracefully handles this case where both bounds are -1 instead of attempting array access that could fail.
3 participants
Description:
Overhaul to nuke previews:
Colors/outlines aren't meant to be final, and there are constants to play around with at the start of both files.
I also kept the naive implementation since the performance cost isn't too high from my testing and any improvements I can think of are small and require a bigger rewrite.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
bibizu