Skip to content

Fix/4527 sam range upgrade #4802

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

Open
JB940 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix/4527 sam range upgrade #4802

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
12003a0
fix SAM recheck cached incoming nukes
JB940 Jul 30, 2026
e4b7a7b
run pretty (before PR this time)
JB940 Jul 30, 2026
f455b16
fix failing build. Update test per Rabbit request. Remove useless che…
JB940 Jul 31, 2026
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
6 changes: 6 additions & 0 deletions src/core/execution/SAMLauncherExecution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@ class SAMTargetingSystem {
}

public getValidTargets(ticks: number): Target[] {
// Clear the entire cache, we might be able to shoot other nukes further away too.
if (this.sam.needsSamRangeRecheck()) {
this.precomputedNukes.clear();
this.sam.setSamRangeRecheck(false);
}

const samTile = this.sam.tile();
const range = this.mg.config().samRange(this.sam.level());
const rangeSquared = range * range;
Expand Down
4 changes: 4 additions & 0 deletions src/core/game/Game.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,10 @@ export interface Unit {
isInCooldown(): boolean;
missileTimerQueue(): number[];

// SAMs
needsSamRangeRecheck(): boolean | undefined;
setSamRangeRecheck(samRangeRecheck: boolean): void;

// Trade Ships
setSafeFromPirates(): void; // Only for trade ships
isSafeFromPirates(): boolean; // Only for trade ships
Expand Down
1 change: 1 addition & 0 deletions src/core/game/GameUpdates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ export interface UnitUpdate {
health?: number;
underConstruction?: boolean;
missileTimerQueue: number[];
recheckSAMRange: boolean;
level: number;
hasTrainStation: boolean;
trainType?: TrainType; // Only for trains
Expand Down
22 changes: 22 additions & 0 deletions src/core/game/UnitImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export class UnitImpl implements Unit {
private _troops: number;
// Number of missiles in cooldown, if empty all missiles are ready.
private _missileTimerQueue: number[] = [];
//Needs to recheck range on upgrade
private _recheckSAMRange: boolean = false;
Comment on lines +39 to +40

@evanpelle evanpelle Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

@JB940 JB940 Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 evanpelle Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

@JB940 JB940 Aug 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've spent some time thinking about it, I really like a range that increases to its new level over 5 seconds or so is not just fanciest but also best. Players I know are used to quickly upgrading a SAM to try to intercept, and expect it work, then get upset when it doesn't. I don't think we want spam upgrade to be a cheese mechanic, but I think players expect them to be able to intercept missiles from far away.

I like the idea of making it time based, let's say straight up missile_reload_time, or maybe divided by two.
if players have to guess and it just suddenly jumps up in range without visual indicators, I don't think it's bad but why not make it look cool and provide visual clarity about what's happening at the same time, is my thought. For people trying to fire it can just show max range. Unless that's an issue with the renderer.

private _hasTrainStation: boolean = false;
private _level: number = 1;
private _targetable: boolean = true;
Expand Down Expand Up @@ -150,6 +152,7 @@ export class UnitImpl implements Unit {
targetUnitId: this._targetUnit?.id() ?? undefined,
targetTile: this.targetTile() ?? undefined,
missileTimerQueue: this._missileTimerQueue,
recheckSAMRange: this._recheckSAMRange,
level: this.level(),
hasTrainStation: this._hasTrainStation,
trainType: this._trainType,
Expand Down Expand Up @@ -626,6 +629,10 @@ export class UnitImpl implements Unit {
this._level++;
if ([UnitType.MissileSilo, UnitType.SAMLauncher].includes(this.type())) {
this._missileTimerQueue.push(this.mg.ticks());
//inner if to reduce if checks
if (this.type() === UnitType.SAMLauncher) {
this._recheckSAMRange = true;
}
}
this.mg.addUpdate(this.toUpdate());
}
Expand All @@ -634,6 +641,10 @@ export class UnitImpl implements Unit {
this._level--;
if ([UnitType.MissileSilo, UnitType.SAMLauncher].includes(this.type())) {
this._missileTimerQueue.pop();
//inner if to reduce if checks
if (this.type() === UnitType.SAMLauncher) {
this._recheckSAMRange = true;
}
}
if (this._level <= 0) {
this.delete(true, destroyer);
Expand All @@ -646,6 +657,17 @@ export class UnitImpl implements Unit {
return this._trainType;
}

needsSamRangeRecheck(): boolean | undefined {
return this._recheckSAMRange;
}

setSamRangeRecheck(samRangeRecheck: boolean): void {
if (this._recheckSAMRange !== samRangeRecheck) {
this._recheckSAMRange = samRangeRecheck;
this.mg.addUpdate(this.toUpdate());
}
}

isLoaded(): boolean | undefined {
return this._loaded;
}
Expand Down
38 changes: 38 additions & 0 deletions tests/core/executions/SAMLauncherExecution.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,4 +444,42 @@ describe("SAM", () => {
expect(nuke.reachedTarget()).toBeFalsy();
expect(nuke.wasDestroyedByEnemy()).toBeTruthy();
});

test("leveling up a SAM launcher should recheck range and intercept a nuke previously out of range", async () => {
const sam = defender.buildUnit(UnitType.SAMLauncher, game.ref(1, 1), {});
const execution = new SAMLauncherExecution(defender, game.ref(1, 1), sam);
game.addExecution(execution);

// Level 1 SAM range squared is 15^2 = 225.
// Nuke trajectory is at distance ~25 from SAM (out of level 1 range).
const nuke = attacker.buildUnit(UnitType.AtomBomb, game.ref(50, 1), {
targetTile: game.ref(25, 1),
trajectory: [
{ tile: game.ref(50, 1), targetable: true },
{ tile: game.ref(45, 1), targetable: true },
{ tile: game.ref(40, 1), targetable: true },
{ tile: game.ref(35, 1), targetable: true },
{ tile: game.ref(30, 1), targetable: true },
{ tile: game.ref(25, 1), targetable: true },
],
});

// Run ticks at level 1: nuke should be marked as unreachable / out of range.
executeTicks(game, 2);
expect(nuke.isActive()).toBeTruthy();
expect(nuke.wasDestroyedByEnemy()).toBeFalsy();

// Level up SAM launcher: and sets recheck flag.
sam.increaseLevel();
sam.reloadMissile();
expect(sam.needsSamRangeRecheck()).toBeTruthy();

//Custom test config range always returns 20 no matter SAM level, mock 26 for range increase.
vi.spyOn(game.config(), "samRange").mockReturnValue(26);

// Run next ticks: cache should be cleared and nuke intercepted under new range.
executeTicks(game, 6);
expect(nuke.reachedTarget()).toBeFalsy();
expect(nuke.wasDestroyedByEnemy()).toBeTruthy();
});
});
Loading