Add red warning circle when nuke would break alliance #2728
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughAdds frontend support to mark nuke ghosts as targeting allies and render warning-colored range rings; refactors server-side nuke blast counting into utilities and changes alliance-break logic to use aggregated per-player blast counts. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as StructureIconsLayer (client)
participant Util as Util.wouldNukeBreakAlliance
participant Count as Util.computeNukeBlastCounts
participant Map as GameMap
UI->>Util: wouldNukeBreakAlliance(targetTile, magnitude, allyIds, threshold)
Util->>Count: computeNukeBlastCounts(gm, targetTile, magnitude)
Count->>Map: query tiles within radius (weight by distance)
Map-->>Count: tiles + distances
Count-->>Util: per-player blast counts
Util-->>UI: boolean (would break alliances)
UI->>UI: set ghostUnit.targetingAlly and pass to createRange()
Note right of UI: Range rendered with warning style if true
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/client/graphics/layers/StructureDrawingUtils.ts (1)
483-494: Extract hardcoded warning colors to named constants.The warning colors (orange
0xff6b35, red0xff4444) and alpha values are magic numbers. Consider extracting them to named constants at the file level for maintainability and consistency.🔎 Proposed refactor
+// Warning colors when targeting allies with nukes +const ALLY_WARNING_FILL_COLOR = 0xff6b35; // Orange +const ALLY_WARNING_FILL_ALPHA = 0.35; +const ALLY_WARNING_STROKE_COLOR = 0xff4444; // Red +const ALLY_WARNING_STROKE_ALPHA = 0.8; +const ALLY_WARNING_STROKE_WIDTH = 2; + +const DEFAULT_FILL_COLOR = 0xffffff; +const DEFAULT_FILL_ALPHA = 0.2; +const DEFAULT_STROKE_COLOR = 0xffffff; +const DEFAULT_STROKE_ALPHA = 0.5; +const DEFAULT_STROKE_WIDTH = 1; + export class SpriteFactory { // ... rest of class public createRange( type: UnitType, stage: PIXI.Container, pos: { x: number; y: number }, level?: number, targetingAlly: boolean = false, ): PIXI.Container | null { // ... radius calculation ... // Add warning colors (red/orange) when targeting an ally to indicate alliance will break const isNuke = type === UnitType.AtomBomb || type === UnitType.HydrogenBomb; - const fillColor = targetingAlly && isNuke ? 0xff6b35 : 0xffffff; - const fillAlpha = targetingAlly && isNuke ? 0.35 : 0.2; - const strokeColor = targetingAlly && isNuke ? 0xff4444 : 0xffffff; - const strokeAlpha = targetingAlly && isNuke ? 0.8 : 0.5; - const strokeWidth = targetingAlly && isNuke ? 2 : 1; + const fillColor = targetingAlly && isNuke ? ALLY_WARNING_FILL_COLOR : DEFAULT_FILL_COLOR; + const fillAlpha = targetingAlly && isNuke ? ALLY_WARNING_FILL_ALPHA : DEFAULT_FILL_ALPHA; + const strokeColor = targetingAlly && isNuke ? ALLY_WARNING_STROKE_COLOR : DEFAULT_STROKE_COLOR; + const strokeAlpha = targetingAlly && isNuke ? ALLY_WARNING_STROKE_ALPHA : DEFAULT_STROKE_ALPHA; + const strokeWidth = targetingAlly && isNuke ? ALLY_WARNING_STROKE_WIDTH : DEFAULT_STROKE_WIDTH;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/client/graphics/layers/StructureDrawingUtils.tssrc/client/graphics/layers/StructureIconsLayer.ts
🧰 Additional context used
🧠 Learnings (6)
📚 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/StructureIconsLayer.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/StructureIconsLayer.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/StructureIconsLayer.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/StructureIconsLayer.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/graphics/layers/StructureIconsLayer.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/StructureIconsLayer.ts
🧬 Code graph analysis (2)
src/client/graphics/layers/StructureIconsLayer.ts (6)
src/core/game/GameView.ts (4)
myPlayer(724-726)allies(479-483)magnitude(836-838)level(171-173)src/core/game/PlayerImpl.ts (1)
allies(375-377)src/core/game/GameMap.ts (1)
magnitude(167-169)src/core/game/GameImpl.ts (1)
magnitude(864-866)src/client/graphics/layers/BuildMenu.ts (1)
count(380-387)src/core/game/UnitImpl.ts (1)
level(413-415)
src/client/graphics/layers/StructureDrawingUtils.ts (2)
src/core/game/GameView.ts (2)
type(98-100)type(467-469)src/core/game/UnitImpl.ts (1)
type(147-149)
🔇 Additional comments (4)
src/client/graphics/layers/StructureDrawingUtils.ts (1)
458-458: LGTM: Parameter addition preserves backward compatibility.The default value ensures existing calls continue to work without modification.
src/client/graphics/layers/StructureIconsLayer.ts (3)
68-68: LGTM: Clean extension of ghost unit state.The optional field appropriately tracks ally-targeting status for range visualization.
507-536: LGTM: Proper state management for targetingAlly flag.The method correctly:
- Accepts the
targetingAllyparameter- Checks if it changed before recreating the range (lines 512-516)
- Stores it on
ghostUnit(line 523)- Passes it to
createRange()(line 531)
258-299: Fix missing MIRVWarhead exclusion check and extract duplicate alliance-breaking logic.The alliance detection has three issues:
Logic mismatch with NukeExecution: This code only checks AtomBomb and HydrogenBomb types, but does not exclude MIRVWarhead like
NukeExecution.maybeBreakAlliances()does (line 82 in NukeExecution.ts). Add&& nukeType !== UnitType.MIRVWarheadto match the actual game logic.Performance: The BFS runs every 50ms while hovering, with no caching. For Hydrogen Bombs with large blast radius, consider caching results per tile position to avoid repeated scans.
Code duplication: Extract the ally tile-counting logic into a shared utility function both StructureIconsLayer and NukeExecution can use, reducing maintenance burden.
⛔ Skipped due to learnings
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.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.Learnt from: scamiv Repo: openfrontio/OpenFrontIO PR: 2539 File: src/core/game/GameImpl.ts:520-542 Timestamp: 2025-11-29T22:22:37.178Z Learning: In GameImpl.ts, neighborsWithDiag() and forEachNeighborWithDiag() intentionally duplicate coordinate iteration logic. They serve different purposes: forEachNeighborWithDiag() is a zero-allocation hot-path optimization while neighborsWithDiag() is a convenience method that returns an array. Refactoring one to call the other would add callback/closure allocations and indirection overhead, defeating the performance goals.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.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: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.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.
evanpelle
reviewed
Dec 28, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/core/execution/NukeExecution.ts (1)
75-78: Redundant MIRVWarhead check (optional cleanup).The MIRVWarhead type is checked both at the call site (line 137) and inside
maybeBreakAlliances()(lines 75-78). The internal check will never trigger when called from here.This is acceptable as defensive programming - it keeps
maybeBreakAlliances()self-contained and safe if called from elsewhere. If you prefer less duplication, you could remove the outer check and rely on the internal guard.Also applies to: 137-139
src/core/execution/Util.ts (1)
43-73: Early exit pattern is efficient.The
return falseto stop searching once threshold is exceeded is a nice optimization - no need to keep counting after we know the answer.The weight calculation (
d2 <= inner2 ? 1 : 0.5) is duplicated withcomputeNukeBlastCounts. For now this is fine given the simplicity, but if the weighting logic ever changes, you would need to update both places. If you want, you could extract a tiny helper:const calcWeight = (d2: number, inner2: number) => d2 <= inner2 ? 1 : 0.5;Not blocking - just something to consider if the formula gets more complex later.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/client/graphics/layers/StructureIconsLayer.tssrc/core/execution/NukeExecution.tssrc/core/execution/Util.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client/graphics/layers/StructureIconsLayer.ts
🧰 Additional context used
🧠 Learnings (11)
📚 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/execution/Util.tssrc/core/execution/NukeExecution.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/core/execution/Util.tssrc/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/Util.tssrc/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 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.
Applied to files:
src/core/execution/Util.tssrc/core/execution/NukeExecution.ts
📚 Learning: 2025-08-14T14:05:00.867Z
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.
Applied to files:
src/core/execution/Util.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/core/execution/Util.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/execution/Util.tssrc/core/execution/NukeExecution.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/Util.tssrc/core/execution/NukeExecution.ts
📚 Learning: 2025-11-26T22:27:31.844Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/worker/SharedTileRing.ts:55-63
Timestamp: 2025-11-26T22:27:31.844Z
Learning: In SharedTileRing.ts, the ring buffer is sized to width * height (the map dimensions). Combined with the dirty flag deduplication mechanism (each tile can only be enqueued once until the consumer clears its dirty flag), the number of pending entries is naturally bounded by the map size and drained every render frame. Therefore, ring buffer overflow should be extremely rare or effectively impossible, and any theoretical race condition between producer and consumer modifying the read index would only manifest as redundant tile refs being rendered, not memory corruption, which is acceptable.
<!-- </add_learning>
Applied to files:
src/core/execution/NukeExecution.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/execution/NukeExecution.ts
📚 Learning: 2025-11-29T22:22:37.178Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2539
File: src/core/game/GameImpl.ts:520-542
Timestamp: 2025-11-29T22:22:37.178Z
Learning: In GameImpl.ts, neighborsWithDiag() and forEachNeighborWithDiag() intentionally duplicate coordinate iteration logic. They serve different purposes: forEachNeighborWithDiag() is a zero-allocation hot-path optimization while neighborsWithDiag() is a convenience method that returns an array. Refactoring one to call the other would add callback/closure allocations and indirection overhead, defeating the performance goals.
Applied to files:
src/core/execution/NukeExecution.ts
🧬 Code graph analysis (2)
src/core/execution/Util.ts (2)
src/core/game/GameMap.ts (3)
GameMap(6-56)TileRef(3-3)magnitude(172-174)src/core/configuration/Config.ts (1)
NukeMagnitude(62-65)
src/core/execution/NukeExecution.ts (4)
src/core/game/GameImpl.ts (1)
magnitude(870-872)src/core/game/GameMap.ts (1)
magnitude(172-174)src/core/game/GameView.ts (1)
magnitude(840-842)src/core/execution/Util.ts (1)
computeNukeBlastCounts(16-35)
🔇 Additional comments (7)
src/core/execution/NukeExecution.ts (3)
16-16: Good import for shared utility.Clean import of the new
computeNukeBlastCountsfunction. This promotes code reuse between server-side alliance breaking and client-side warning display.
71-88: Clean refactor to use shared blast count utility.The switch from per-tile iteration to the new
computeNukeBlastCountsutility is a good move. The logic is clearer: get weighted counts per player, then check threshold. The magnitude and threshold retrieval from config is correct.
90-114: Alliance breaking loop looks correct.The iteration over
blastCountsentries is clean. Good checks in place:
isPlayer()guard before treating as Player- Null check on
allianceRequestbefore callingreject()- Relation update only for non-self players
One small note:
playerBySmallIDreturnsPlayer | TerraNullius, and you correctly checkisPlayer()before proceeding. Solid work.src/core/execution/Util.ts (4)
5-9: Clean interface design.Simple typed interface with just the required fields. Using
GameMapinstead ofGameis good - it only requires what's needed for the calculation.
16-35: Well-structured blast count computation.The function is pure, focused, and reusable. A few observations:
ownerSmallId > 0correctly filters out unowned tiles (TerraNullius)- Weight logic (1 for inner zone, 0.5 for outer) matches the original implementation
- Returning early with
return truekeeps the search going until all tiles are visited
37-40: Good use of interface extension.Extending
NukeBlastParamswith alliance-specific fields keeps things DRY. TheallySmallIdsas aSet<number>gives O(1) lookup - good choice for the hot path.
48-50: Good early exit for empty ally set.Checking
allySmallIds.size === 0upfront avoids the circleSearch entirely when there are no allies. Clean and efficient.
github-project-automation
bot
moved this from Final Review
to Complete
in OpenFront Release Management
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
When placing a nuke (Atom Bomb or Hydrogen Bomb), the range circle now turns red to warn players when the attack would break an alliance.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
abodcraft1