Skip to content

fix: remove double x() dereference in MIRV separation point calc #3940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
evanpelle merged 1 commit into from
May 17, 2026
+1 −1
Merged

fix: remove double x() dereference in MIRV separation point calc #3940

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/core/execution/MIRVExecution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export class MirvExecution implements Execution {
});
this.mg.stats().bombLaunch(this.player, this.targetPlayer, UnitType.MIRV);
const x = Math.floor(
(this.mg.x(this.dst) + this.mg.x(this.mg.x(this.nuke.tile()))) / 2,
(this.mg.x(this.dst) + this.mg.x(this.nuke.tile())) / 2,
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add a regression test for the midpoint X fix.

Line 85 fixes the bug, but this src/core/ logic change ships without a test in the provided diff. Please add a deterministic test that exercises MirvExecution.tick() and asserts the separation midpoint uses a single dereference (x = floor((x(dst)+x(nukeTile))/2)), so this regression does not return.

As per coding guidelines, “All changes to src/core/ must include tests”.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/execution/MIRVExecution.ts` at line 85, Add a deterministic unit
test for MirvExecution.tick() that constructs a MirvExecution with a
controllable map/manager mock (mock mg.x to return known integers for the dst
and nuke.tile()), set dst and nuke.tile() values so the midpoint is non-trivial,
call MirvExecution.tick(), and assert the computed separation midpoint X equals
floor((x(dst) + x(nuke.tile())) / 2) (i.e., Math.floor((dstX + nukeTileX) / 2)).
Target the MirvExecution class and its tick() behavior and assert the exact
numeric midpoint to prevent regressions in the single-dereference midpoint
calculation.

);
const y = Math.max(0, this.mg.y(this.dst) - 500) + 50;
this.separateDst = this.mg.ref(x, y);
Expand Down
Loading