Made intercepting multiple warheads actually increase the cool down by the required amount #4638
Made intercepting multiple warheads actually increase the cool down by the required amount #4638TKTK123456 wants to merge 6 commits into
Conversation
…ultipul MIRV warheads
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughMIRV interception eligibility now uses the SAM’s level-based Manhattan range. Later intercepted warheads may trigger additional SAM launches when the launcher is not cooling down, while all intercepted warheads are deleted and statistics count launches that occurred. ChangesMIRV SAM interception
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/execution/SAMLauncherExecution.ts (1)
323-351: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTrack exact interceptions and break properly when on cooldown.
A
returninsideforEachacts like acontinuestatement, failing to stop the loop when the SAM is on cooldown. Additionally, stats and messages usemirvWarheadTargets.lengthbefore verifying how many warheads were actually intercepted.Using a standard
forloop lets usbreaksecurely when the cooldown is reached and track the exactinterceptedCount. It also removes the need forthis.sam !== nullchecks since we avoid a closure block.🛠️ Proposed fix
- // Message - this.mg.displayMessage( - "events_display.mirv_warheads_intercepted", - MessageType.SAM_HIT, - samOwner.id(), - undefined, - { count: mirvWarheadTargets.length }, - ); - - mirvWarheadTargets.forEach(({ unit: u }, i) => { - if (this.sam !== null && i > 0) { - if (this.sam.isInCooldown()) { - return; - } - this.sam.launch(); - } - - // Delete warheads - u.delete(); - }); - - // Record stats - this.mg - .stats() - .bombIntercept( - samOwner, - UnitType.MIRVWarhead, - mirvWarheadTargets.length, - ); + let interceptedCount = 0; + + for (let i = 0; i < mirvWarheadTargets.length; i++) { + const u = mirvWarheadTargets[i].unit; + if (i > 0) { + if (this.sam.isInCooldown()) { + break; + } + this.sam.launch(); + } + u.delete(); + interceptedCount++; + } + + // Message + this.mg.displayMessage( + "events_display.mirv_warheads_intercepted", + MessageType.SAM_HIT, + samOwner.id(), + undefined, + { count: interceptedCount }, + ); + + // Record stats + this.mg + .stats() + .bombIntercept( + samOwner, + UnitType.MIRVWarhead, + interceptedCount, + );🤖 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/SAMLauncherExecution.ts` around lines 323 - 351, Replace the mirvWarheadTargets.forEach callback with a standard loop that stops with break when this.sam.isInCooldown() is reached, while preserving SAM launch behavior. Track the number actually deleted in an interceptedCount variable, and use that count for the interception message and bombIntercept stats instead of mirvWarheadTargets.length; structure the loop so redundant this.sam !== null checks are unnecessary.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/core/execution/SAMLauncherExecution.ts`:
- Around line 274-306: Update the MIRV warhead target collection in the
execution method to use const, remove the redundant level-based filter, and
delete the console.log debug statement. Leave target selection to the
interception loop’s existing cooldown-aware break behavior.
---
Outside diff comments:
In `@src/core/execution/SAMLauncherExecution.ts`:
- Around line 323-351: Replace the mirvWarheadTargets.forEach callback with a
standard loop that stops with break when this.sam.isInCooldown() is reached,
while preserving SAM launch behavior. Track the number actually deleted in an
interceptedCount variable, and use that count for the interception message and
bombIntercept stats instead of mirvWarheadTargets.length; structure the loop so
redundant this.sam !== null checks are unnecessary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7391fd5b-f626-4406-b294-27349225e6fa
📒 Files selected for processing (1)
src/core/execution/SAMLauncherExecution.ts
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/execution/SAMLauncherExecution.ts (1)
113-117: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a regression test for the expanded detection range. The current 199-distance case is still inside the old 2× range (300), so it does not cover this change. Use a target beyond 300 and within 600.
🤖 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/SAMLauncherExecution.ts` around lines 113 - 117, Add a regression test covering the expanded detection range used by SAMLauncherExecution, placing the target at a distance greater than 300 and no more than 600 from the SAM tile so the test fails with the former 2× range but passes with the current 4× range.Source: Coding guidelines
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@src/core/execution/SAMLauncherExecution.ts`:
- Around line 113-117: Add a regression test covering the expanded detection
range used by SAMLauncherExecution, placing the target at a distance greater
than 300 and no more than 600 from the SAM tile so the test fails with the
former 2× range but passes with the current 4× range.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33e4f4bc-f08c-410e-ab46-12d5ade0941e
📒 Files selected for processing (1)
src/core/execution/SAMLauncherExecution.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/execution/SAMLauncherExecution.ts (1)
318-346: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix the interception loop and the stat count.
The
forEachloop usesreturn, which skips to the next item instead of breaking the loop. This means the code skips launching missiles when on cooldown, but still iterates and skips deleting the remaining warheads. Also, the stats and message usemirvWarheadTargets.length, which reports the total targets found instead of the ones actually intercepted and deleted.Using a cleaner
for...ofloop withbreakstops the iteration correctly. We can then use the actual intercepted count for the stats and message.♻️ Proposed fix
- // Message - this.mg.displayMessage( - "events_display.mirv_warheads_intercepted", - MessageType.SAM_HIT, - samOwner.id(), - undefined, - { count: mirvWarheadTargets.length }, - ); - - mirvWarheadTargets.forEach(({ unit: u }, i) => { - if (this.sam !== null && i > 0) { - if (this.sam.isInCooldown()) { - return; - } - this.sam.launch(); - } - - // Delete warheads - u.delete(); - }); - - // Record stats - this.mg - .stats() - .bombIntercept( - samOwner, - UnitType.MIRVWarhead, - mirvWarheadTargets.length, - ); + let interceptedCount = 0; + + for (const { unit: u } of mirvWarheadTargets) { + if (interceptedCount > 0) { + if (this.sam.isInCooldown()) { + break; + } + this.sam.launch(); + } + + u.delete(); + interceptedCount++; + } + + // Message + this.mg.displayMessage( + "events_display.mirv_warheads_intercepted", + MessageType.SAM_HIT, + samOwner.id(), + undefined, + { count: interceptedCount }, + ); + + // Record stats + this.mg + .stats() + .bombIntercept( + samOwner, + UnitType.MIRVWarhead, + interceptedCount, + );🤖 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/SAMLauncherExecution.ts` around lines 318 - 346, Update the MIRV interception loop in the SAM execution flow to use a breakable iteration, such as for...of, so cooldown stops processing additional warheads rather than merely skipping one callback. Track the number actually launched and deleted, then use that intercepted count instead of mirvWarheadTargets.length in both displayMessage and bombIntercept.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@src/core/execution/SAMLauncherExecution.ts`:
- Around line 318-346: Update the MIRV interception loop in the SAM execution
flow to use a breakable iteration, such as for...of, so cooldown stops
processing additional warheads rather than merely skipping one callback. Track
the number actually launched and deleted, then use that intercepted count
instead of mirvWarheadTargets.length in both displayMessage and bombIntercept.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 526cf52d-f057-4a1e-b0f8-1a2829ed46bc
📒 Files selected for processing (1)
src/core/execution/SAMLauncherExecution.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/execution/SAMLauncherExecution.ts (2)
297-300: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftKeep the core range check integer-only and add deterministic tests.
samRange()insrc/core/configuration/Config.tsuses division, so this new MIRV comparison relies on floating-point math insrc/core. Prefer an integer-scaled comparison or integer range helper. Add tests for inside, outside, and boundary targets, plus the new multi-warhead cooldown behavior.🤖 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/SAMLauncherExecution.ts` around lines 297 - 300, Update the MIRV range comparison in SAMLauncherExecution to use integer-only arithmetic or an existing integer range helper instead of the floating-point samRange() result. Add deterministic tests covering inside-range, outside-range, and exact-boundary targets, along with the new multi-warhead cooldown behavior.Source: Coding guidelines
326-335: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlways delete the current warhead.
returnonly exits theforEachcallback. When the SAM is already in cooldown,u.delete()is skipped, leaving that warhead active and eligible for processing again.🐛 Proposed fix
mirvWarheadTargets.forEach(({ unit: u }, i) => { if (this.sam !== null && i > 0) { - if (this.sam.isInCooldown()) { - return; + if (!this.sam.isInCooldown()) { + this.sam.launch(); } - this.sam.launch(); } // Delete warheads u.delete();🤖 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/SAMLauncherExecution.ts` around lines 326 - 335, Update the forEach callback in the mirvWarheadTargets processing flow so a SAM cooldown does not bypass the current warhead cleanup. Preserve the cooldown early exit for launching, but ensure u.delete() executes for every warhead, including those encountered while this.sam.isInCooldown() is true.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@src/core/execution/SAMLauncherExecution.ts`:
- Around line 297-300: Update the MIRV range comparison in SAMLauncherExecution
to use integer-only arithmetic or an existing integer range helper instead of
the floating-point samRange() result. Add deterministic tests covering
inside-range, outside-range, and exact-boundary targets, along with the new
multi-warhead cooldown behavior.
- Around line 326-335: Update the forEach callback in the mirvWarheadTargets
processing flow so a SAM cooldown does not bypass the current warhead cleanup.
Preserve the cooldown early exit for launching, but ensure u.delete() executes
for every warhead, including those encountered while this.sam.isInCooldown() is
true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b619acb6-e879-4728-893d-0530860a2646
📒 Files selected for processing (1)
src/core/execution/SAMLauncherExecution.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/core/execution/SAMLauncherExecution.ts (3)
297-300: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd tests for the new MIRV range boundary.
Please add or confirm tests covering targets inside, exactly at, and outside
samRange(this.sam.level()). Changes undersrc/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/SAMLauncherExecution.ts` around lines 297 - 300, Add tests for the MIRV range logic around the samRange(this.sam.level()) boundary in SAMLauncherExecution, covering targets inside the range, exactly at the boundary, and outside it. Place the tests in the established test suite for this execution flow and preserve the expected behavior for each case.Source: Coding guidelines
325-345: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCount the initial SAM launch in interception statistics.
this.sam.launch()already runs before the loop, butamountOfWarheadsstarts at zero and counts only later launches.bombIntercepttherefore undercounts every non-empty MIRV interception by one.Proposed fix
-let amountOfWarheads = 0; +let amountOfWarheads = 1; // initial launch above🤖 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/SAMLauncherExecution.ts` around lines 325 - 345, Update the MIRV interception accounting around amountOfWarheads and bombIntercept so the initial this.sam.launch() is included in the recorded count. Initialize or increment amountOfWarheads to account for that first launch while preserving the existing cooldown handling and subsequent-warhead loop behavior.
325-337: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlways delete intercepted warheads, even during cooldown.
When
isInCooldown()is true, the callback returns beforeu.delete(). The warhead remains active and can be intercepted again on a later tick. Skip only the additional launch; keep deletion unconditional.Proposed fix
let amountOfWarheads = 0; mirvWarheadTargets.forEach(({ unit: u }, i) => { - if (this.sam !== null && i > 0) { - if (this.sam.isInCooldown()) { - return; - } + if (this.sam !== null && i > 0 && !this.sam.isInCooldown()) { amountOfWarheads++; this.sam.launch(); } - // Delete warheads u.delete(); });🤖 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/SAMLauncherExecution.ts` around lines 325 - 337, Update the mirvWarheadTargets callback so cooldown only prevents the additional this.sam.launch() and amountOfWarheads increment, not the subsequent u.delete(). Remove or restructure the early return from the isInCooldown() branch, while preserving the existing first-warhead behavior and ensuring every intercepted warhead is deleted.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@src/core/execution/SAMLauncherExecution.ts`:
- Around line 297-300: Add tests for the MIRV range logic around the
samRange(this.sam.level()) boundary in SAMLauncherExecution, covering targets
inside the range, exactly at the boundary, and outside it. Place the tests in
the established test suite for this execution flow and preserve the expected
behavior for each case.
- Around line 325-345: Update the MIRV interception accounting around
amountOfWarheads and bombIntercept so the initial this.sam.launch() is included
in the recorded count. Initialize or increment amountOfWarheads to account for
that first launch while preserving the existing cooldown handling and
subsequent-warhead loop behavior.
- Around line 325-337: Update the mirvWarheadTargets callback so cooldown only
prevents the additional this.sam.launch() and amountOfWarheads increment, not
the subsequent u.delete(). Remove or restructure the early return from the
isInCooldown() branch, while preserving the existing first-warhead behavior and
ensuring every intercepted warhead is deleted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7eb24434-fe62-4750-9cd1-262e911b126c
📒 Files selected for processing (1)
src/core/execution/SAMLauncherExecution.ts
There was a problem hiding this comment.
I thought we were planning on making mirv warheads have the same logic as regular nukes anyways?
There was a problem hiding this comment.
It wasn't working, unless do you mean that we change there movement speed? (I would advise not unless we want them to be launched directly from the silo)
There was a problem hiding this comment.
yeah we can change their movement stpeed
There was a problem hiding this comment.
if we change the movement speed then we need to have the MIRV warheads launch directly from the silo
2 participants
Add approved & assigned issue number here:
Resolves #4508
Description:
Made intercepting multiple warheads actually increase the cool down by the required amount (doesn't perfectly resolve the issue but its as good as I could get it to be)
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
tktk1234567