Fix/4527 sam range upgrade

[JB940]

Before opening a PR: discuss new features on Discord first, and file bugs or small improvements as issues. You must be assigned to an approved issue — unsolicited PRs will be auto-closed.

Add approved & assigned issue number here:

Resolves #4527

Description:

Fixes a bug where a SAM wouldn't shoot something after a range upgrade in the detection range since it got cached.
Has a new flag when SAM gets upgraded, clears cache in the next cycle in SAMExecution.

Please complete the following:

• I have added relevant tests to the test directory

Please put your Discord username so you can be contacted if a bug or regression is found:

JB940

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 77d56a6b-78c8-4371-b083-d9e75b6fb558

📥 Commits

Reviewing files that changed from the base of the PR and between e4b7a7b and f455b16.

📒 Files selected for processing (2)

• tests/core/executions/SAMLauncherExecution.test.ts
• tests/util/viewStubs.ts

---

Walkthrough

SAM launchers now mark range changes after level updates, publish that state, clear cached target results, and reevaluate nukes against the updated range.

Changes

SAM range recheck

Layer / File(s)	Summary
Recheck state and update payload src/core/game/Game.ts, src/core/game/GameUpdates.ts, src/core/game/UnitImpl.ts, tests/util/viewStubs.ts	Adds SAM range recheck accessors, stores and serializes the flag, and updates unit stubs.
Level change trigger src/core/game/UnitImpl.ts	Marks SAMs for range rechecking when their level increases or decreases.
Target cache invalidation and validation src/core/execution/SAMLauncherExecution.ts, tests/core/executions/SAMLauncherExecution.test.ts	Clears cached interception results before scanning and tests upgraded and downgraded SAM behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• openfrontio/OpenFrontIO#4638: Updates SAM range and eligibility logic in the same execution path.
• openfrontio/OpenFrontIO#4753: Modifies the SAM targeting cache used by this change.
• openfrontio/OpenFrontIO#4770: Changes SAM targeting and interception behavior in the same system.

Suggested reviewers: evanpelle

Poem

A SAM recalls its changing sight,
Clears old guesses, scans anew.
A nuke once beyond its reach
Now meets a wider range.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the SAM range upgrade bug fix and includes the related issue number.
Description check	✅ Passed	The description explains the cached-range bug, the cache reset fix, and the added tests.
Linked Issues check	✅ Passed	The changes implement issue #4527 by resetting cached unreachable targets after SAM range upgrades and testing upgraded and downgraded behavior.
Out of Scope Changes check	✅ Passed	All code and test changes directly support the SAM range upgrade fix described in issue #4527.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Fix failing CI checks

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/core/game/UnitImpl.ts (1)

644-647: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add regression coverage for SAM downgrades.

The new test covers increaseLevel(), but this changed decreaseLevel() branch also invalidates cached range results. Add a case that verifies the flag is set and stale targeting is rechecked after a downgrade. As per coding guidelines, core simulation changes should 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/game/UnitImpl.ts` around lines 644 - 647, Add regression coverage
for the SAMLauncher downgrade path in decreaseLevel(), verifying that
_recheckSAMRange is set after decreasing the unit level and that subsequent
targeting recalculates range rather than using stale cached results. Keep the
existing increaseLevel() coverage and follow the established core simulation
test patterns.

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.

Nitpick comments:
In `@src/core/game/UnitImpl.ts`:
- Around line 644-647: Add regression coverage for the SAMLauncher downgrade
path in decreaseLevel(), verifying that _recheckSAMRange is set after decreasing
the unit level and that subsequent targeting recalculates range rather than
using stale cached results. Keep the existing increaseLevel() coverage and
follow the established core simulation test patterns.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e2788c7-4f8b-4455-a33d-6cd5efaf505c

📥 Commits

Reviewing files that changed from the base of the PR and between 8b095d2 and e4b7a7b.

📒 Files selected for processing (5)

• src/core/execution/SAMLauncherExecution.ts
• src/core/game/Game.ts
• src/core/game/GameUpdates.ts
• src/core/game/UnitImpl.ts
• tests/core/executions/SAMLauncherExecution.test.ts

[evanpelle]

Fixes a bug where a SAM wouldn't shoot something after a range upgrade in the detection range since it got cached.
Has a new flag when SAM gets upgraded, clears cache in the next cycle in SAMExecution.

Could this be used to cheese? For example if a hydro is heading toward a level 4 SAM, you could quickly upgrade it before the hydro hits?

[evanpelle]

since this is SAM specific, can we create a SAMState type like WarshipState

[JB940]

I will make the SAMState thing. I just copied it from another unit I saw so there might be more that would need a State class/file.

And yes, I mentioned this as a problem in the issue. You can just spam level up and catch everything with this. I personally suggest making a range that increases over 9 seconds (missile reload time to be exact)

Code on MAIN just caches any missiles and doesn't reupdate, so it's just a magic variable causing an unintended side effect. I'm down with making it official too, but as it stands it's not based on anything like Sam level or range, just a hidden variable nobody knows.

Since it just got assigned without comment I just decided to fix the unintended side effect but I'm super happy to change the functionality to an increase in range over time

[evanpelle]

I think the current behavior is good, we should make it more clear in the code though. maybe a timer or something for how long the new range takes affect. Or maybe any inflight missiles still use the old range. something like that.