Skip to content

Destroy incoming nukes when alliance is created #2716

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
plazmaezio wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Destroy incoming nukes when alliance is created #2716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1c8f75
feat: destroy incoming nukes on alliance
plazmaezio Dec 27, 2025
65d31a3
fix: Fixed destroyNukesBetween from Game interface
plazmaezio Dec 27, 2025
45f83e7
fix: Fixed tests for queued nukes
plazmaezio Dec 27, 2025
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
38 changes: 31 additions & 7 deletions src/core/execution/NukeExecution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ export class NukeExecution implements Execution {
this.pathFinder = new ParabolaPathFinder(mg);
}

public target(): Player | TerraNullius {
public target(): Player | TerraNullius | null {
// Not initialized yet -> queued execution
if (!this.mg) return null;

return this.mg.owner(this.dst);
}

Expand All @@ -64,6 +67,25 @@ export class NukeExecution implements Execution {
return this.tilesToDestroyCache;
}

public targetTile(): TileRef {
return this.dst;
}

public isInFlight(): boolean {
return this.nuke !== null;
}

public destroyInFlight(): void {
if (!this.active) return;

if (this.nuke) {
this.nuke.delete(false);
this.nuke = null;
}

this.active = false;
}

/**
* Break alliances with players significantly affected by the nuke strike.
* Uses weighted tile counting (inner=1, outer=0.5).
Expand Down Expand Up @@ -247,12 +269,12 @@ export class NukeExecution implements Execution {
throw new Error("Not initialized");
}

const target = this.target();
const magnitude = this.mg.config().nukeMagnitudes(this.nuke.type());
const toDestroy = this.tilesToDestroy();

const maxTroops = this.target().isPlayer()
? this.mg.config().maxTroops(this.target() as Player)
: 1;
const maxTroops =
target && target.isPlayer() ? this.mg.config().maxTroops(target) : 1;

for (const tile of toDestroy) {
const owner = this.mg.owner(tile);
Expand Down Expand Up @@ -319,9 +341,11 @@ export class NukeExecution implements Execution {
this.nuke.delete(false);

// Record stats
this.mg
.stats()
.bombLand(this.player, this.target(), this.nuke.type() as NukeType);
if (target) {
this.mg
.stats()
.bombLand(this.player, target, this.nuke.type() as NukeType);
}
}

private redrawBuildings(range: number) {
Expand Down
11 changes: 11 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 @@ -773,6 +773,10 @@ export interface Game extends GameMap {
addUpdate(update: GameUpdate): void;
railNetwork(): RailNetwork;
conquerPlayer(conqueror: Player, conquered: Player): void;

destroyNukesBetween(p1: Player, p2: Player): DestroyNukesResult;

executions(): Execution[];
}

export interface PlayerActions {
Expand Down Expand Up @@ -819,6 +823,13 @@ export interface EmojiMessage {
createdAt: Tick;
}

export type DestroyNukesResult = {
inFlight: number;
queued: number;
fromRequestorToRecipient: number;
fromRecipientToRequestor: number;
};

export enum MessageType {
ATTACK_FAILED,
ATTACK_CANCELLED,
Expand Down
186 changes: 185 additions & 1 deletion src/core/game/GameImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { renderNumber } from "../../client/Utils";
import { Config } from "../configuration/Config";
import { NukeExecution } from "../execution/NukeExecution";
import { AllPlayersStats, ClientID, Winner } from "../Schemas";
import { simpleHash } from "../Util";
import { AllianceImpl } from "./AllianceImpl";
Expand All @@ -9,6 +10,7 @@ import {
AllianceRequest,
Cell,
ColoredTeams,
DestroyNukesResult,
Duos,
EmojiMessage,
Execution,
Expand Down Expand Up @@ -277,6 +279,75 @@ export class GameImpl implements Game {
return ar;
}

public destroyNukesBetween(p1: Player, p2: Player): DestroyNukesResult {
let inFlight = 0;
let queued = 0;
let fromRequestorToRecipient = 0;
let fromRecipientToRequestor = 0;

const destroy = (exec: Execution, isQueued: boolean) => {
if (!(exec instanceof NukeExecution)) return;

const launcher = exec.owner();

// queued execution -> target not resolvable yet
const target =
exec.isInFlight() && exec.target()
? exec.target()
: exec instanceof NukeExecution
? exec.targetTile()
: null;

if (!target) return;

let targetOwner: Player | TerraNullius;
if (typeof target === "object" && "isPlayer" in target) {
// target is already a Player or TerraNullius (in-flight nuke)
targetOwner = target as Player | TerraNullius;
} else {
// target is a TileRef (queued nuke)
targetOwner = this.owner(target as TileRef);
}

const isRequestorToRecipient = launcher === p1 && targetOwner === p2;
const isRecipientToRequestor = launcher === p2 && targetOwner === p1;

const isBetween = isRequestorToRecipient || isRecipientToRequestor;

if (!isBetween) {
return;
}

if (isQueued) queued++;
else inFlight++;

if (isRequestorToRecipient) fromRequestorToRecipient++;
else fromRecipientToRequestor++;

exec.destroyInFlight();
};
Comment on lines +288 to +328
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 3, 2026

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Refactor type discrimination logic for clarity and safety.

Lines 294-310 use fragile runtime type checking to distinguish between Player | TerraNullius (in-flight) vs TileRef (queued). The pattern has several issues:

  1. Line 297: Redundant instanceof NukeExecution check (already verified on line 289)
  2. Lines 304-306: Unsafe cast to Player | TerraNullius without verifying the object actually has expected methods
  3. Type discrimination relies on typeof === "object" which is implicit and unclear
🔎 Proposed refactor for explicit type checking 
 const destroy = (exec: Execution, isQueued: boolean) => {
   if (!(exec instanceof NukeExecution)) return;

   const launcher = exec.owner();

-  // queued execution -> target not resolvable yet
-  const target =
-    exec.isInFlight() && exec.target()
-      ? exec.target()
-      : exec instanceof NukeExecution
-        ? exec.targetTile()
-        : null;
-
-  if (!target) return;
-
-  let targetOwner: Player | TerraNullius;
-  if (typeof target === "object" && "isPlayer" in target) {
-    // target is already a Player or TerraNullius (in-flight nuke)
-    targetOwner = target as Player | TerraNullius;
-  } else {
-    // target is a TileRef (queued nuke)
-    targetOwner = this.owner(target as TileRef);
-  }
+  let targetOwner: Player | TerraNullius | null;
+  
+  if (exec.isInFlight()) {
+    // In-flight: target() returns Player | TerraNullius | null
+    targetOwner = exec.target();
+  } else {
+    // Queued: resolve owner from targetTile()
+    targetOwner = this.owner(exec.targetTile());
+  }
+
+  if (!targetOwner) return;

   const isRequestorToRecipient = launcher === p1 && targetOwner === p2;
   const isRecipientToRequestor = launcher === p2 && targetOwner === p1;

   const isBetween = isRequestorToRecipient || isRecipientToRequestor;

   if (!isBetween) {
     return;
   }

   if (isQueued) queued++;
   else inFlight++;

   if (isRequestorToRecipient) fromRequestorToRecipient++;
   else fromRecipientToRequestor++;

   exec.destroyInFlight();
 };


for (const exec of this.execs) {
if (exec instanceof NukeExecution && !exec.isInFlight()) {
// initialized but not launched yet -> queued
destroy(exec, true);
} else {
destroy(exec, false);
}
}

for (const exec of this.unInitExecs) {
destroy(exec, true);
}

return {
inFlight,
queued,
fromRequestorToRecipient,
fromRecipientToRequestor,
};
}

acceptAllianceRequest(request: AllianceRequestImpl) {
this.allianceRequests = this.allianceRequests.filter(
(ar) => ar !== request,
Expand Down Expand Up @@ -311,6 +382,110 @@ export class GameImpl implements Game {
if (recipient.hasEmbargoAgainst(requestor))
recipient.endTemporaryEmbargo(requestor);

const {
inFlight,
queued,
fromRequestorToRecipient,
fromRecipientToRequestor,
} = this.destroyNukesBetween(requestor, recipient);

// Destroy counts available for display messages
this.unInitExecs = this.unInitExecs.filter((e) => e.isActive());

if (fromRequestorToRecipient > 0) {
const requestorMsg = `${fromRequestorToRecipient} nuke${
fromRequestorToRecipient > 1 ? "s" : ""
} launched towards ${recipient.displayName()} ${
fromRequestorToRecipient > 1 ? "were" : "was"
} destroyed due to the alliance`;

const recipientMsg = `${fromRequestorToRecipient} nuke${
fromRequestorToRecipient > 1 ? "s" : ""
} launched by ${requestor.displayName()} towards you ${
fromRequestorToRecipient > 1 ? "were" : "was"
} destroyed due to the alliance`;

this.displayMessage(
requestorMsg,
MessageType.ALLIANCE_ACCEPTED,
requestor.id(),
);
this.displayMessage(
recipientMsg,
MessageType.ALLIANCE_ACCEPTED,
recipient.id(),
);
}

if (fromRecipientToRequestor > 0) {
const requestorMsg = `${fromRecipientToRequestor} nuke${
fromRecipientToRequestor > 1 ? "s" : ""
} launched by ${recipient.displayName()} towards you ${
fromRecipientToRequestor > 1 ? "were" : "was"
} destroyed due to the alliance`;

const recipientMsg = `${fromRecipientToRequestor} nuke${
fromRecipientToRequestor > 1 ? "s" : ""
} launched towards ${requestor.displayName()} ${
fromRecipientToRequestor > 1 ? "were" : "was"
} destroyed due to the alliance`;

this.displayMessage(
requestorMsg,
MessageType.ALLIANCE_ACCEPTED,
requestor.id(),
);
this.displayMessage(
recipientMsg,
MessageType.ALLIANCE_ACCEPTED,
recipient.id(),
);
}

if (inFlight > 0) {
const baseMsg = `${inFlight} nuke${inFlight > 1 ? "s" : ""} in flight ${
inFlight > 1 ? "were" : "was"
} neutralized due to alliance formation with`;

const requestorMsg = `${inFlight} nuke${
inFlight > 1 ? "s" : ""
} in flight ${
inFlight > 1 ? "were" : "was"
} neutralized due to alliance formation with ${recipient.displayName()}`;
const recipientMsg = baseMsg + ` ${requestor.displayName()}`;

this.displayMessage(
requestorMsg,
MessageType.ALLIANCE_ACCEPTED,
requestor.id(),
);
this.displayMessage(
recipientMsg,
MessageType.ALLIANCE_ACCEPTED,
recipient.id(),
);
}

if (queued > 0) {
const baseMsg = `${queued} planned nuke${queued > 1 ? "s" : ""} ${
queued > 1 ? "were" : "was"
} canceled due to alliance formation with`;

const requestorMsg = baseMsg + ` ${recipient.displayName()}`;
const recipientMsg = baseMsg + ` ${requestor.displayName()}`;

this.displayMessage(
requestorMsg,
MessageType.ALLIANCE_ACCEPTED,
requestor.id(),
);
this.displayMessage(
recipientMsg,
MessageType.ALLIANCE_ACCEPTED,
recipient.id(),
);
}
Comment on lines +385 to +487
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Duplicate messages: directional and type-based counters are not mutually exclusive.

The message logic sends multiple notifications for the same nukes:

  1. Lines 395-418: Messages when fromRequestorToRecipient > 0
  2. Lines 420-443: Messages when fromRecipientToRequestor > 0
  3. Lines 445-467: Messages when inFlight > 0
  4. Lines 469-487: Messages when queued > 0

Problem: These conditions are not mutually exclusive. A single in-flight nuke from requestor to recipient increments both fromRequestorToRecipient and inFlight, triggering messages in both blocks 1 and 3.

Example: If player A has 1 in-flight nuke targeting player B when they ally:

  • Player A receives: "1 nuke launched towards B was destroyed" (line 396) AND "1 nuke in flight was neutralized" (line 450)
  • Player B receives: "1 nuke launched by A towards you was destroyed" (line 402) AND "1 nuke in flight was neutralized" (line 455)
🔎 Recommended fix: use mutually exclusive conditions

Consider this approach:

  • If nukes are one-directional (only A→B or only B→A): show directional messages
  • If nukes are mutual (both A→B and B→A): show non-directional summary messages
  • Split in-flight and queued within each category
-    if (fromRequestorToRecipient > 0) {
+    const hasMutualNukes = fromRequestorToRecipient > 0 && fromRecipientToRequestor > 0;
+    
+    if (!hasMutualNukes && fromRequestorToRecipient > 0) {
       // ... directional messages for requestor → recipient
     }

-    if (fromRecipientToRequestor > 0) {
+    if (!hasMutualNukes && fromRecipientToRequestor > 0) {
       // ... directional messages for recipient → requestor
     }

-    if (inFlight > 0) {
+    if (hasMutualNukes && inFlight > 0) {
       // ... non-directional in-flight messages
     }

-    if (queued > 0) {
+    if (hasMutualNukes && queued > 0) {
       // ... non-directional queued messages
     }

Or alternatively, always use directional messages and remove the non-directional blocks entirely.

Committable suggestion skipped: line range outside the PR's diff.


this.addUpdate({
type: GameUpdateType.AllianceRequestReply,
request: request.toUpdate(),
Expand Down Expand Up @@ -357,6 +532,7 @@ export class GameImpl implements Game {
}

executeNextTick(): GameUpdates {
const pending = this.updates;
this.updates = createGameUpdatesMap();
this.execs.forEach((e) => {
if (
Expand Down Expand Up @@ -393,7 +569,15 @@ export class GameImpl implements Game {
});
}
this._ticks++;
return this.updates;

const merged = createGameUpdatesMap();

for (const k in merged) {
merged[k] = [...pending[k], ...this.updates[k]];
}

this.updates = createGameUpdatesMap();
return merged;
}

private hash(): number {
Expand Down
Loading
Loading