bugfix: when nuking nearby water, the attacker was not marked as a traitor on launch (#1574)

## Description:

There was a bug where we only checked for betrayals if the target tile
was owned.

## Please complete the following:

- [x] I have added screenshots for all UI updates
- [x] I process any text displayed to the user through translateText()
and I've added it to the en.json file
- [x] I have added relevant tests to the test directory
- [x] I confirm I have thoroughly tested these changes and take full
responsibility for any bugs introduced
- [x] I have read and accepted the CLA aggreement (only required once).

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

evan

src/core/configuration/Config.ts

@@ -152,6 +152,8 @@ export interface Config {
   traitorDefenseDebuff(): number;
   traitorDuration(): number;
   nukeMagnitudes(unitType: UnitType): NukeMagnitude;
+  // Number of tiles destroyed to break an alliance
+  nukeAllianceBreakThreshold(): number;
   defaultNukeSpeed(): number;
   defaultNukeTargetableRange(): number;
   defaultSamRange(): number;

src/core/configuration/DefaultConfig.ts

@@ -805,6 +805,10 @@ export class DefaultConfig implements Config {
     throw new Error(`Unknown nuke type: ${unitType}`);
   }
 
+  nukeAllianceBreakThreshold(): number {
+    return 100;
+  }
+
   defaultNukeSpeed(): number {
     return 6;
   }

src/core/execution/NukeExecution.ts

@@ -64,7 +64,7 @@ export class NukeExecution implements Execution {
     return this.tilesToDestroyCache;
   }
 
-  private breakAlliances(toDestroy: Set<TileRef>) {
+  private maybeBreakAlliances(toDestroy: Set<TileRef>) {
     if (this.nuke === null) {
       throw new Error("Not initialized");
     }
@@ -77,8 +77,12 @@ export class NukeExecution implements Execution {
       }
     }
 
+    const threshold = this.mg.config().nukeAllianceBreakThreshold();
     for (const [other, tilesDestroyed] of attacked) {
-      if (tilesDestroyed > 100 && this.nuke.type() !== UnitType.MIRVWarhead) {
+      if (
+        tilesDestroyed > threshold &&
+        this.nuke.type() !== UnitType.MIRVWarhead
+      ) {
         // Mirv warheads shouldn't break alliances
         const alliance = this.player.allianceWith(other);
         if (alliance !== null) {
@@ -108,6 +112,7 @@ export class NukeExecution implements Execution {
       this.nuke = this.player.buildUnit(this.nukeType, spawn, {
         targetTile: this.dst,
       });
+      this.maybeBreakAlliances(this.tilesToDestroy());
       if (this.mg.hasOwner(this.dst)) {
         const target = this.mg.owner(this.dst);
         if (!target.isPlayer()) {
@@ -120,7 +125,6 @@ export class NukeExecution implements Execution {
             MessageType.NUKE_INBOUND,
             target.id(),
           );
-          this.breakAlliances(this.tilesToDestroy());
         } else if (this.nukeType === UnitType.HydrogenBomb) {
           this.mg.displayIncomingUnit(
             this.nuke.id(),
@@ -129,7 +133,6 @@ export class NukeExecution implements Execution {
             MessageType.HYDROGEN_BOMB_INBOUND,
             target.id(),
           );
-          this.breakAlliances(this.tilesToDestroy());
         }
 
         // Record stats
@@ -198,7 +201,7 @@ export class NukeExecution implements Execution {
 
     const magnitude = this.mg.config().nukeMagnitudes(this.nuke.type());
     const toDestroy = this.tilesToDestroy();
-    this.breakAlliances(toDestroy);
+    this.maybeBreakAlliances(toDestroy);
 
     for (const tile of toDestroy) {
       const owner = this.mg.owner(tile);

tests/core/executions/NukeExecution.test.ts

@@ -12,31 +12,34 @@ import { executeTicks } from "../../util/utils";
 
 let game: Game;
 let player: Player;
+let otherPlayer: Player;
 
 describe("NukeExecution", () => {
   beforeEach(async () => {
-    game = await setup("big_plains", {
+    game = await setup(
-      infiniteGold: true,
+      "big_plains",
-      instantBuild: true,
+      {
-    });
+        infiniteGold: true,
+        instantBuild: true,
+      },
+      [
+        new PlayerInfo("player", PlayerType.Human, "client_id1", "player_id"),
+        new PlayerInfo("other", PlayerType.Human, "client_id2", "other_id"),
+      ],
+    );
 
     (game.config() as TestConfig).nukeMagnitudes = jest.fn(() => ({
       inner: 10,
       outer: 10,
     }));
-    const player_info = new PlayerInfo(
+    (game.config() as TestConfig).nukeAllianceBreakThreshold = jest.fn(() => 5);
-      "player_id",
-      PlayerType.Human,
-      null,
-      "player_id",
-    );
-    game.addPlayer(player_info);
 
     while (game.inSpawnPhase()) {
       game.executeNextTick();
     }
 
     player = game.player("player_id");
+    otherPlayer = game.player("other_id");
   });
 
   test("nuke should destroy buildings and redraw out of range buildings", async () => {
@@ -94,4 +97,29 @@ describe("NukeExecution", () => {
     executeTicks(game, 35);
     expect(nukeExec.getNuke()!.isTargetable()).toBeTruthy();
   });
+
+  test("nuke should break alliances on launch", async () => {
+    const req = player.createAllianceRequest(otherPlayer);
+    req!.accept();
+
+    player.conquer(game.ref(1, 1));
+    player.buildUnit(UnitType.MissileSilo, game.ref(1, 1), {});
+
+    for (let x = 90; x < 99; x++) {
+      for (let y = 90; y < 99; y++) {
+        otherPlayer.conquer(game.ref(x, y));
+      }
+    }
+
+    // Add a nuke targeting just outside the other player's territory.
+    game.addExecution(
+      new NukeExecution(UnitType.AtomBomb, player, game.ref(85, 85), null),
+    );
+
+    game.executeNextTick(); // init
+    game.executeNextTick(); // exec
+
+    expect(player.isTraitor()).toBe(true);
+    expect(player.isAlliedWith(otherPlayer)).toBe(false);
+  });
 });