From 1a34321ac82b95753a196624b9b3e529629aad88 Mon Sep 17 00:00:00 2001 From: Navaneeth Prabha <76899241+Navaneethp007@users.noreply.github.com> Date: Wed, 15 Jul 2026 02:51:59 +0530 Subject: [PATCH] Fix transport ships targeting unreachable inland-lake shores (#4577) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves #4555 ## Description: Fixes the bug where transport boats could not be sent to certain targets (e.g. between the arrows on the Four Islands map) when the attacker's territory was far enough away that it approached the target from its inland-lake side. **Root cause:** The transport landing tile was selected purely by distance. `targetTransportTile()` called `SpatialQuery.closestShore()`, which returns the nearest shore owned by the target using a Manhattan-distance BFS and only checks `isShore && isLand && owned` — it never considers whether that shore is reachable by water. Water bodies are tracked as connected components (an inland lake is a separate component from the surrounding ocean), and the *source* selection (`closestShoreByWater()`) correctly requires the attacker to have a shore in the same water component as the destination. So when the nearest owned shore happened to face a disconnected inland lake, the source search found no shore in the lake's component and the transport silently failed — even though the same target had an ocean-facing shore that the attacker's boats could actually reach. **Fix:** Make destination selection reachability-aware so a lake-facing shore is never chosen when a reachable one exists. - Added `SpatialQuery.closestReachableShore(targetOwner, attacker, tile)`: it first collects the set of water components adjacent to the attacker's own shoreline, then returns the nearest target-owned shore whose water component is in that reachable set. Shores that only border a disconnected water body (an inland lake) are skipped. It returns `null` only when the target has no reachable shore at all. - `targetTransportTile()` now takes the attacker and delegates to `closestReachableShore()` instead of `closestShore()`. Its two callers — `canBuildTransportShip()` and `TransportShipExecution.init()` — already have the attacker in hand and pass it through, so both the build-time check and the actual execution agree on the same reachable destination. - `closestShore()` is left unchanged; this adds a reachability-aware variant rather than altering existing behavior. **Testing performed:** Reproduced the bug first on a generated map with an inland lake enclosed by a thick land moat (confirmed the lake resolves to a separate water component and that `closestShoreByWater()` returns `null` for the lake-facing destination while a reachable ocean shore exists). Added unit tests for `closestReachableShore()` (picks the reachable ocean shore; returns `null` when every target shore is in an unreachable water body) and a `canBuildTransportShip()` regression test for the lake scenario. Full suite: 1878 tests pass; `tsc --noEmit`, ESLint, and Prettier are all clean. ## Please complete the following: - [x] I have added screenshots for all UI updates — no UI changes in this PR - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file — no user-facing text added in this PR - [x] I have added relevant tests to the test directory ## Please put your Discord username so you can be contacted if a bug or regression is found: Navaneeth Prabha#0825 --- src/core/execution/TransportShipExecution.ts | 2 +- src/core/game/TransportShipUtils.ts | 12 ++- src/core/pathfinding/spatial/SpatialQuery.ts | 37 ++++++++ tests/core/game/TransportShipUtils.test.ts | 63 ++++++++++++++ tests/core/pathfinding/SpatialQuery.test.ts | 88 +++++++++++++++++++- 5 files changed, 196 insertions(+), 6 deletions(-) create mode 100644 tests/core/game/TransportShipUtils.test.ts diff --git a/src/core/execution/TransportShipExecution.ts b/src/core/execution/TransportShipExecution.ts index 65ceb7ec1..dfd639f64 100644 --- a/src/core/execution/TransportShipExecution.ts +++ b/src/core/execution/TransportShipExecution.ts @@ -106,7 +106,7 @@ export class TransportShipExecution implements Execution { .boatAttackAmount(this.attacker, this.target); this.troops = Math.min(this.troops, this.attacker.troops()); - this.dst = targetTransportTile(this.mg, this.ref); + this.dst = targetTransportTile(this.mg, this.attacker, this.ref); if (this.dst === null) { console.warn( diff --git a/src/core/game/TransportShipUtils.ts b/src/core/game/TransportShipUtils.ts index b53a11688..e7ed5d970 100644 --- a/src/core/game/TransportShipUtils.ts +++ b/src/core/game/TransportShipUtils.ts @@ -13,7 +13,7 @@ export function canBuildTransportShip( return false; } - const dst = targetTransportTile(game, tile); + const dst = targetTransportTile(game, player, tile); if (dst === null) { return false; } @@ -30,9 +30,15 @@ export function canBuildTransportShip( return spatial.closestShoreByWater(player, dst) ?? false; } -export function targetTransportTile(gm: Game, tile: TileRef): TileRef | null { +export function targetTransportTile( + gm: Game, + attacker: Player, + tile: TileRef, +): TileRef | null { const spatial = new SpatialQuery(gm); - return spatial.closestShore(gm.owner(tile), tile); + // Only consider landing shores the attacker can actually reach by water, so a + // shore facing a disconnected inland lake is never chosen as the target. + return spatial.closestReachableShore(gm.owner(tile), attacker, tile); } export function bestShoreDeploymentSource( diff --git a/src/core/pathfinding/spatial/SpatialQuery.ts b/src/core/pathfinding/spatial/SpatialQuery.ts index 8eabff307..c7397cc21 100644 --- a/src/core/pathfinding/spatial/SpatialQuery.ts +++ b/src/core/pathfinding/spatial/SpatialQuery.ts @@ -97,6 +97,43 @@ export class SpatialQuery { return this.bfsNearest(tile, maxDist, isValidTile); } + /** + * Find the closest shore tile owned by `targetOwner` near `tile` that sits on + * a water component reachable from `attacker`'s own shoreline. + * + * Unlike {@link closestShore}, this skips shores that only border a + * disconnected water body (e.g. an inland lake) that the attacker's boats + * could never traverse. Returns null when no reachable target shore exists + * within `maxDist`. + */ + closestReachableShore( + targetOwner: Owner, + attacker: Player, + tile: TileRef, + maxDist: number = 50, + ): TileRef | null { + const gm = this.game; + const targetId = targetOwner.smallID(); + + // Water components adjacent to the attacker's own shoreline. + const reachable = new Set(); + for (const t of attacker.borderTiles()) { + if (!gm.isShore(t) || !gm.isLand(t)) continue; + const component = gm.getWaterComponent(t); + if (component !== null) reachable.add(component); + } + if (reachable.size === 0) return null; + + const isValidTile = (t: TileRef) => { + if (!gm.isShore(t) || !gm.isLand(t)) return false; + if (gm.ownerID(t) !== targetId) return false; + const component = gm.getWaterComponent(t); + return component !== null && reachable.has(component); + }; + + return this.bfsNearest(tile, maxDist, isValidTile); + } + /** * Find closest shore tile by water pathfinding. * Returns null for terra nullius (no borderTiles). diff --git a/tests/core/game/TransportShipUtils.test.ts b/tests/core/game/TransportShipUtils.test.ts new file mode 100644 index 000000000..1620abc1f --- /dev/null +++ b/tests/core/game/TransportShipUtils.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from "vitest"; +import { SpawnExecution } from "../../../src/core/execution/SpawnExecution"; +import { + Game, + Player, + PlayerInfo, + PlayerType, +} from "../../../src/core/game/Game"; +import { TileRef } from "../../../src/core/game/GameMap"; +import { canBuildTransportShip } from "../../../src/core/game/TransportShipUtils"; +import { createGame, L, W } from "../pathfinding/_fixtures"; + +function addPlayer(game: Game, tile: TileRef, id: string = "test"): Player { + const info = new PlayerInfo(id, PlayerType.Human, null, `${id}_id`); + game.addPlayer(info); + game.addExecution(new SpawnExecution("game_id", info, tile)); + game.executeNextTick(); + game.executeNextTick(); + return game.player(info.id); +} + +// Target island (cols 10-24, rows 1-16) with an interior lake +// (cols 15-20, rows 6-11) enclosed by a thick land moat, so the lake is a +// separate water component from the ocean. Attacker island on the left. +function buildLakeMap(): Game { + const width = 26; + const height = 18; + const grid: string[] = new Array(width * height).fill(W); + const set = (x: number, y: number, v: string) => (grid[y * width + x] = v); + const inBox = ( + x: number, + y: number, + x0: number, + x1: number, + y0: number, + y1: number, + ) => x >= x0 && x <= x1 && y >= y0 && y <= y1; + for (let y = 0; y < height; y++) { + for (let x = 0; x < width; x++) { + if (inBox(x, y, 1, 5, 6, 11)) set(x, y, L); // attacker island + if (inBox(x, y, 10, 24, 1, 16)) set(x, y, L); // target island (unowned) + if (inBox(x, y, 15, 20, 6, 11)) set(x, y, W); // carve lake + } + } + return createGame({ width, height, grid }); +} + +describe("canBuildTransportShip", () => { + it("allows a transport toward a reachable ocean shore near a lake click", () => { + const game = buildLakeMap(); + const attacker = addPlayer(game, game.ref(3, 8), "attacker"); + + // Click on the (unowned) target island next to the inland lake. The nearest + // owned shore is lake-facing and unreachable by the attacker's boats, but a + // reachable ocean-facing shore exists — so the boat must still be buildable. + const clickNearLake = game.ref(14, 8); + const src = canBuildTransportShip(game, attacker, clickNearLake); + + expect(src).not.toBe(false); + expect(game.isShore(src as TileRef)).toBe(true); + expect((attacker as Player).smallID()).toBe(game.ownerID(src as TileRef)); + }); +}); diff --git a/tests/core/pathfinding/SpatialQuery.test.ts b/tests/core/pathfinding/SpatialQuery.test.ts index 5c05e4d00..f076b2040 100644 --- a/tests/core/pathfinding/SpatialQuery.test.ts +++ b/tests/core/pathfinding/SpatialQuery.test.ts @@ -12,8 +12,8 @@ import { createGame, L, W } from "./_fixtures"; // Spawns player and **expands territory** via getSpawnTiles (euclidean dist 4) // Ref: src/core/execution/Util.ts -function addPlayer(game: Game, tile: TileRef): Player { - const info = new PlayerInfo("test", PlayerType.Human, null, "test_id"); +function addPlayer(game: Game, tile: TileRef, id: string = "test"): Player { + const info = new PlayerInfo(id, PlayerType.Human, null, `${id}_id`); game.addPlayer(info); game.addExecution(new SpawnExecution("game_id", info, tile)); game.executeNextTick(); @@ -228,4 +228,88 @@ describe("SpatialQuery", () => { expect(game.ownerID(result!)).toBe(player.smallID()); }); }); + + describe("closestReachableShore", () => { + // Target island (cols 10-24, rows 1-16) with an interior lake + // (cols 15-20, rows 6-11) enclosed by a thick land moat, so the lake is a + // separate water component from the ocean. Attacker island on the left. + function buildLakeMap(): Game { + const width = 26; + const height = 18; + const grid: string[] = new Array(width * height).fill(W); + const set = (x: number, y: number, v: string) => + (grid[y * width + x] = v); + const inBox = ( + x: number, + y: number, + x0: number, + x1: number, + y0: number, + y1: number, + ) => x >= x0 && x <= x1 && y >= y0 && y <= y1; + for (let y = 0; y < height; y++) { + for (let x = 0; x < width; x++) { + if (inBox(x, y, 1, 5, 6, 11)) set(x, y, L); // attacker island + if (inBox(x, y, 10, 24, 1, 16)) set(x, y, L); // target island + if (inBox(x, y, 15, 20, 6, 11)) set(x, y, W); // carve lake + } + } + return createGame({ width, height, grid }); + } + + it("skips a lake-facing shore for one the attacker can reach by water", () => { + const game = buildLakeMap(); + const attacker = addPlayer(game, game.ref(3, 8), "attacker"); + const target = addPlayer(game, game.ref(12, 3), "target"); + const spatial = new SpatialQuery(game); + + // Clicking target land next to the lake: the nearest owned shore is + // lake-facing (unreachable), so the reachability-blind closestShore picks + // a shore the attacker's ocean boats can never reach. + const clickNearLake = game.ref(14, 8); + const oldPick = spatial.closestShore( + game.owner(clickNearLake), + clickNearLake, + ); + expect(oldPick).not.toBeNull(); + expect(spatial.closestShoreByWater(attacker, oldPick!)).toBeNull(); + + // The reachability-aware pick must return a shore the attacker can reach. + const result = spatial.closestReachableShore( + target, + attacker, + clickNearLake, + ); + expect(result).not.toBeNull(); + expect(game.isShore(result!)).toBe(true); + expect(game.ownerID(result!)).toBe(target.smallID()); + expect(spatial.closestShoreByWater(attacker, result!)).not.toBeNull(); + }); + + it("returns null when every target shore is in an unreachable water body", () => { + // Land wall (cols 2-11) fully separates left water (cols 0-1) from right + // water (cols 12-13): two disconnected seas. + // prettier-ignore + const game = createGame({ + width: 14, height: 6, grid: [ + W, W, L, L, L, L, L, L, L, L, L, L, W, W, + W, W, L, L, L, L, L, L, L, L, L, L, W, W, + W, W, L, L, L, L, L, L, L, L, L, L, W, W, + W, W, L, L, L, L, L, L, L, L, L, L, W, W, + W, W, L, L, L, L, L, L, L, L, L, L, W, W, + W, W, L, L, L, L, L, L, L, L, L, L, W, W, + ], + }); + const attacker = addPlayer(game, game.ref(3, 2), "attacker"); // left sea + const target = addPlayer(game, game.ref(10, 3), "target"); // right sea + const spatial = new SpatialQuery(game); + + const result = spatial.closestReachableShore( + target, + attacker, + game.ref(10, 3), + ); + expect(result).toBeNull(); + }); + }); });