From 12b06fa0b294dfbb28a7b23e12c79a190770caca Mon Sep 17 00:00:00 2001 From: FloPinguin <25036848+FloPinguin@users.noreply.github.com> Date: Sun, 19 Apr 2026 06:32:21 +0200 Subject: [PATCH] =?UTF-8?q?Pathfinding=20Fixes=20(Water=20Nukes=20/=20Lake?= =?UTF-8?q?s)=20=F0=9F=92=A7=20=20(#3714)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description: Fixes water-pathfinding errors that started appearing after the first water nuke and persisted across the rest of the match. Users reported warships "getting stuck" (stopped moving). image ### Summary - The new `AbstractGraphBuilder.buildClusterConnectionsFromCache` was buggy _(The cached edge costs reused by "clean" clusters were keyed by tile pair without their original `(clusterX, clusterY)`, so a boundary edge could be re-stamped with the wrong cluster and become untraversable by the query-time single-cluster bounded A*. The cache now stores `{ cost, clusterX, clusterY }` and `buildClusterConnectionsFromCache` preserves the original attribution when re-adding the edge.)_ - Warships: `findTargetUnit` now skips trade ships that are not in the warship's water component, avoiding pathfinding to provably unreachable targets. - Warships: On `patrol` `NOT_FOUND`, clear `targetTile` so the warship picks a new target. This is a defensive guard for the rare case where a water nuke splits the component between target selection and pathfinding - without it, the warship retries the same now-unreachable target every tick and spams the log forever. ### Test - Added a Warship test verifying that trade ships in a different water component are not targeted. ## 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 ## Please put your Discord username so you can be contacted if a bug or regression is found: FloPinguin --- src/core/execution/WarshipExecution.ts | 16 +++++++++ .../pathfinding/algorithms/AbstractGraph.ts | 29 ++++++++++++---- tests/Warship.test.ts | 33 +++++++++++++++++++ 3 files changed, 72 insertions(+), 6 deletions(-) diff --git a/src/core/execution/WarshipExecution.ts b/src/core/execution/WarshipExecution.ts index 121e7f2d7..5693152fc 100644 --- a/src/core/execution/WarshipExecution.ts +++ b/src/core/execution/WarshipExecution.ts @@ -83,6 +83,10 @@ export class WarshipExecution implements Execution { const patrolTile = this.warship.patrolTile()!; const patrolRangeSquared = config.warshipPatrolRange() ** 2; + // Lazy: only computed if a TradeShip candidate forces the component check. + // `undefined` = not yet computed; `null` = computed, no component found. + let warshipComponent: number | null | undefined = undefined; + const ships = mg.nearbyUnits( this.warship.tile()!, config.warshipTargettingRange(), @@ -113,6 +117,17 @@ export class WarshipExecution implements Execution { ) { continue; } + + if (warshipComponent === undefined) { + warshipComponent = mg.getWaterComponent(this.warship.tile()); + } + if ( + warshipComponent !== null && + !mg.hasWaterComponent(unit.tile(), warshipComponent) + ) { + continue; + } + if ( mg.euclideanDistSquared(patrolTile, unit.tile()) > patrolRangeSquared ) { @@ -220,6 +235,7 @@ export class WarshipExecution implements Execution { break; case PathStatus.NOT_FOUND: { console.log(`path not found to target`); + this.warship.setTargetTile(undefined); break; } } diff --git a/src/core/pathfinding/algorithms/AbstractGraph.ts b/src/core/pathfinding/algorithms/AbstractGraph.ts index e1cd806c5..4ab24947a 100644 --- a/src/core/pathfinding/algorithms/AbstractGraph.ts +++ b/src/core/pathfinding/algorithms/AbstractGraph.ts @@ -219,7 +219,10 @@ export class AbstractGraphBuilder { // Partial rebuild state private cleanClusters: Set | null = null; - private oldEdgeCosts: Map> | null = null; + private oldEdgeCosts: Map< + number, + Map + > | null = null; constructor( private readonly map: GameMap, @@ -664,8 +667,12 @@ export class AbstractGraphBuilder { this.oldEdgeCosts.set(tileMin, inner); } const existing = inner.get(tileMax); - if (existing === undefined || edge.cost < existing) { - inner.set(tileMax, edge.cost); + if (existing === undefined || edge.cost < existing.cost) { + inner.set(tileMax, { + cost: edge.cost, + clusterX: edge.clusterX, + clusterY: edge.clusterY, + }); } } } @@ -690,9 +697,19 @@ export class AbstractGraphBuilder { const tileMin = Math.min(nodes[i].tile, nodes[j].tile); const tileMax = Math.max(nodes[i].tile, nodes[j].tile); - const cost = oldEdgeCosts.get(tileMin)?.get(tileMax); - if (cost !== undefined) { - this.addOrUpdateEdge(nodes[i].id, nodes[j].id, cost, cx, cy); + const entry = oldEdgeCosts.get(tileMin)?.get(tileMax); + if (entry !== undefined) { + // Preserve the ORIGINAL (clusterX, clusterY) from the old graph. + // The path for a boundary edge between two clusters lives in whichever + // cluster's BFS originally found it; attributing it to `cx,cy` here + // would break query-time single-cluster bounded A*. + this.addOrUpdateEdge( + nodes[i].id, + nodes[j].id, + entry.cost, + entry.clusterX, + entry.clusterY, + ); } } } diff --git a/tests/Warship.test.ts b/tests/Warship.test.ts index ee6c556de..33e0dfc88 100644 --- a/tests/Warship.test.ts +++ b/tests/Warship.test.ts @@ -7,6 +7,7 @@ import { PlayerType, UnitType, } from "../src/core/game/Game"; +import { TileRef } from "../src/core/game/GameMap"; import { setup } from "./util/Setup"; import { executeTicks } from "./util/utils"; @@ -200,6 +201,38 @@ describe("Warship", () => { expect(tradeShip.owner().id()).toBe(player2.id()); }); + test("Warship does not target trade ships in different water components", async () => { + // build port so warship can target trade ships + player1.buildUnit(UnitType.Port, game.ref(coastX, 10), {}); + + const warshipTile = game.ref(coastX + 1, 2); + const tradeShipTile = game.ref(coastX + 1, 12); + + const warship = player1.buildUnit(UnitType.Warship, warshipTile, { + patrolTile: warshipTile, + }); + game.addExecution(new WarshipExecution(warship)); + + const tradeShip = player2.buildUnit(UnitType.TradeShip, tradeShipTile, { + targetUnit: player2.buildUnit(UnitType.Port, game.ref(coastX, 10), {}), + }); + + // Mock different water components + game.getWaterComponent = (tile: TileRef) => { + if (tile === warshipTile) return 1; + return 2; + }; + + game.hasWaterComponent = (tile: TileRef, component: number) => { + return game.getWaterComponent(tile) === component; + }; + + executeTicks(game, 10); + + // Trade ship should not be captured because it's in a different component + expect(tradeShip.owner().id()).toBe(player2.id()); + }); + test("MoveWarshipExecution fails if player is not the owner", async () => { const originalPatrolTile = game.ref(coastX + 1, 10); const warship = player1.buildUnit(