From 42b837868b74abc4f9f8258c312ed49e9fcdbe69 Mon Sep 17 00:00:00 2001 From: evanpelle Date: Wed, 28 May 2025 11:17:37 -0700 Subject: [PATCH] Revert tradeship path caching (#927) ## Description: The path caching was causing bugs, sometimes they wouldn't get removed, sometimes they would go in circles. Deleting the cache so we can deploy v23. ## 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: --- src/core/execution/TradeShipExecution.ts | 126 ++++++----------------- src/core/game/Game.ts | 4 - src/core/game/UnitImpl.ts | 8 -- 3 files changed, 31 insertions(+), 107 deletions(-) diff --git a/src/core/execution/TradeShipExecution.ts b/src/core/execution/TradeShipExecution.ts index 927140cb4..bed5803dc 100644 --- a/src/core/execution/TradeShipExecution.ts +++ b/src/core/execution/TradeShipExecution.ts @@ -10,18 +10,17 @@ import { UnitType, } from "../game/Game"; import { TileRef } from "../game/GameMap"; -import { AStar, PathFindResultType } from "../pathfinding/AStar"; -import { MiniAStar } from "../pathfinding/MiniAStar"; +import { PathFindResultType } from "../pathfinding/AStar"; +import { PathFinder } from "../pathfinding/PathFinding"; import { distSortUnit } from "../Util"; export class TradeShipExecution implements Execution { private active = true; - private mg: Game | null = null; - private origOwner: Player | null = null; - private tradeShip: Unit | null = null; + private mg: Game; + private origOwner: Player; + private tradeShip: Unit; private wasCaptured = false; - private tilesTraveled = 0; - private aStar: AStar | null = null; + private pathFinder: PathFinder; constructor( private _owner: PlayerID, @@ -32,12 +31,10 @@ export class TradeShipExecution implements Execution { init(mg: Game, ticks: number): void { this.mg = mg; this.origOwner = mg.player(this._owner); + this.pathFinder = PathFinder.Mini(mg, 2500); } tick(ticks: number): void { - if (this.mg === null || this.origOwner === null) { - throw new Error("Not initialized"); - } if (this.tradeShip === null) { const spawn = this.origOwner.canBuild( UnitType.TradeShip, @@ -52,9 +49,6 @@ export class TradeShipExecution implements Execution { targetUnit: this._dstPort, lastSetSafeFromPirates: ticks, }); - - // Record stats - this.mg.stats().boatSendTrade(this.origOwner, this._dstPort.owner()); } if (!this.tradeShip.isActive()) { @@ -100,105 +94,52 @@ export class TradeShipExecution implements Execution { } } - const cachedNextTile = this._dstPort.cacheGet(this.tradeShip.tile()); - if (cachedNextTile !== undefined) { - this.moveTradeShip(cachedNextTile); - return; - } + const result = this.pathFinder.nextTile( + this.tradeShip.tile(), + this._dstPort.tile(), + ); - this.computeNewPath(); - } - - private fillCachePath(port: Unit, path: TileRef[]): void { - if (path.length < 2) { - throw new Error("path must have at least 2 points"); - } - for (let i = 0; i < path.length - 1; i++) { - if (port.cacheGet(path[i]) !== undefined) { - continue; - } - const from = path[i]; - const to = path[i + 1]; - port.cachePut(from, to); - } - } - - private moveTradeShip(nextTile?: TileRef): void { - if (nextTile === undefined) { - throw new Error("missing tile"); - } - - if (nextTile === this._dstPort.tile()) { - this.complete(); - return; - } - // Update safeFromPirates status - if (this.mg!.isWater(nextTile) && this.mg!.isShoreline(nextTile)) { - this.tradeShip!.setSafeFromPirates(); - } - this.tradeShip!.move(nextTile); - this.tilesTraveled++; - } - - private computeNewPath(): void { - if (this.aStar === null) { - this.aStar = new MiniAStar( - this.mg!, - this.mg!.miniMap(), - this.tradeShip!.tile(), - this._dstPort.tile(), - 2500, - 20, - ); - } - - switch (this.aStar.compute()) { + switch (result.type) { + case PathFindResultType.Completed: + this.complete(); + break; case PathFindResultType.Pending: // Fire unit event to rerender. - this.tradeShip!.touch(); + this.tradeShip.move(this.tradeShip.tile()); break; - case PathFindResultType.Completed: { - const fullPath = this.aStar.reconstructPath(); - if (fullPath === null || fullPath.length === 0) { - throw new Error("missing path"); + case PathFindResultType.NextTile: + // Update safeFromPirates status + if (this.mg.isWater(result.tile) && this.mg.isShoreline(result.tile)) { + this.tradeShip.setSafeFromPirates(); } - this.fillCachePath(this._dstPort, fullPath); - if (!this.wasCaptured) { - this.fillCachePath(this.srcPort, fullPath.slice().reverse()); - } - this.moveTradeShip(fullPath.shift()); + this.tradeShip.move(result.tile); break; - } case PathFindResultType.PathNotFound: - consolex.warn("trade ship cannot find route"); - this.tradeShip?.delete(false); + consolex.warn("captured trade ship cannot find route"); + if (this.tradeShip.isActive()) { + this.tradeShip.delete(false); + } this.active = false; break; - default: - throw new Error("unexpected path finding compute result"); } } private complete() { - if (this.mg === null || this.origOwner === null) { - throw new Error("Not initialized"); - } - if (this.tradeShip === null) return; this.active = false; this.tradeShip.delete(false); - const gold = this.mg.config().tradeShipGold(this.tilesTraveled); + const gold = this.mg + .config() + .tradeShipGold( + this.mg.manhattanDist(this.srcPort.tile(), this._dstPort.tile()), + ); if (this.wasCaptured) { - const player = this.tradeShip.owner(); - player.addGold(gold); + this.tradeShip.owner().addGold(gold); this.mg.displayMessage( `Received ${renderNumber(gold)} gold from ship captured from ${this.origOwner.displayName()}`, MessageType.SUCCESS, this.tradeShip.owner().id(), ); - - // Record stats - this.mg.stats().boatCapturedTrade(player, this.origOwner, gold); } else { this.srcPort.owner().addGold(gold); this._dstPort.owner().addGold(gold); @@ -212,11 +153,6 @@ export class TradeShipExecution implements Execution { MessageType.SUCCESS, this.srcPort.owner().id(), ); - - // Record stats - this.mg - .stats() - .boatArriveTrade(this.srcPort.owner(), this._dstPort.owner(), gold); } return; } diff --git a/src/core/game/Game.ts b/src/core/game/Game.ts index 77e9d94ff..eb52a6b10 100644 --- a/src/core/game/Game.ts +++ b/src/core/game/Game.ts @@ -399,10 +399,6 @@ export interface Unit { // Warships setPatrolTile(tile: TileRef): void; patrolTile(): TileRef | undefined; - - // Ports - cachePut(from: TileRef, to: TileRef): void; - cacheGet(from: TileRef): TileRef | undefined; } export interface TerraNullius { diff --git a/src/core/game/UnitImpl.ts b/src/core/game/UnitImpl.ts index de77f066e..2b41b46b9 100644 --- a/src/core/game/UnitImpl.ts +++ b/src/core/game/UnitImpl.ts @@ -27,7 +27,6 @@ export class UnitImpl implements Unit { private _lastOwner: PlayerImpl | null = null; private _troops: number; private _cooldownStartTick: Tick | null = null; - private _pathCache: Map = new Map(); private _patrolTile: TileRef | undefined; constructor( private _type: UnitType, @@ -84,13 +83,6 @@ export class UnitImpl implements Unit { return this._targetTile; } - cachePut(from: TileRef, to: TileRef): void { - this._pathCache.set(from, to); - } - cacheGet(from: TileRef): TileRef | undefined { - return this._pathCache.get(from); - } - id() { return this._id; }