From 15519b95c8fc79a8d9b166c034ab02f5be0c72cd Mon Sep 17 00:00:00 2001
From: evanpelle <evanpelle@gmail.com>
Date: Wed, 28 May 2025 17:26:13 -0700
Subject: [PATCH] fix warship targetting range (#938)

## Description:
A warship refactor caused a regressions where warships could attack at
any distance. Also refactored & simplified the trade ship logic.

## Please complete the following:

- [x] I have added screenshots for all UI updates
- [x] I confirm I have thoroughly tested these changes and take full
responsibility for any bugs introduced
- [x] I understand that submitting code with bugs that could have been
caught through manual testing blocks releases and new features for all
contributors

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

<DISCORD USERNAME>
---
 src/core/execution/WarshipExecution.ts | 59 +++++++++++++++++---------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/src/core/execution/WarshipExecution.ts b/src/core/execution/WarshipExecution.ts
index f5400c74d..43233bd91 100644
--- a/src/core/execution/WarshipExecution.ts
+++ b/src/core/execution/WarshipExecution.ts
@@ -79,28 +79,45 @@ export class WarshipExecution implements Execution {
     const hasPort = this.warship.owner().units(UnitType.Port).length > 0;
     const patrolRangeSquared = this.mg.config().warshipPatrolRange() ** 2;
 
-    const ships = this.mg
-      .nearbyUnits(
-        this.warship.patrolTile()!,
-        this.mg.config().warshipTargettingRange(),
-        [UnitType.TransportShip, UnitType.Warship, UnitType.TradeShip],
-      )
-      .filter(
-        ({ unit }) =>
-          unit.owner() !== this.warship.owner() &&
-          unit !== this.warship &&
-          !unit.owner().isFriendly(this.warship.owner()) &&
-          !this.alreadySentShell.has(unit) &&
-          (unit.type() !== UnitType.TradeShip ||
-            (hasPort &&
-              this.mg.euclideanDistSquared(this.warship.tile(), unit.tile()) <=
-                patrolRangeSquared &&
-              unit.targetUnit()?.owner() !== this.warship.owner() &&
-              !unit.targetUnit()?.owner().isFriendly(this.warship.owner()) &&
-              unit.isSafeFromPirates() !== true)),
-      );
+    const ships = this.mg.nearbyUnits(
+      this.warship.tile()!,
+      this.mg.config().warshipTargettingRange(),
+      [UnitType.TransportShip, UnitType.Warship, UnitType.TradeShip],
+    );
+    const potentialTargets: { unit: Unit; distSquared: number }[] = [];
+    for (const { unit, distSquared } of ships) {
+      if (
+        unit.owner() === this.warship.owner() ||
+        unit === this.warship ||
+        unit.owner().isFriendly(this.warship.owner()) ||
+        this.alreadySentShell.has(unit)
+      ) {
+        continue;
+      }
+      if (unit.type() === UnitType.TradeShip) {
+        if (
+          !hasPort ||
+          unit.isSafeFromPirates() ||
+          unit.targetUnit()?.owner() === this.warship.owner() || // trade ship is coming to my port
+          unit.targetUnit()?.owner().isFriendly(this.warship.owner()) // trade ship is coming to my ally
+        ) {
+          continue;
+        }
+        if (
+          this.mg.euclideanDistSquared(
+            this.warship.patrolTile()!,
+            unit.tile(),
+          ) > patrolRangeSquared
+        ) {
+          // Prevent warship from chasing trade ship that is too far away from
+          // the patrol tile to prevent warships from wandering around the map.
+          continue;
+        }
+      }
+      potentialTargets.push({ unit: unit, distSquared });
+    }
 
-    return ships.sort((a, b) => {
+    return potentialTargets.sort((a, b) => {
       const { unit: unitA, distSquared: distA } = a;
       const { unit: unitB, distSquared: distB } = b;