mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-07-23 13:20:46 +00:00
**Approved and assigned issue:** #4643 Resolves #4643 ## Description `TradeShipExecution` currently calls `WaterPathFinder.next()` and then performs a second one-shot `findPath()` for the same destination when it records a motion plan. `next()` has already calculated and cached the traversal route, so the second call repeats the expensive water-path calculation solely to recreate the route for the client. This change lets `PathFinderStepper` return a copy of the active path after `next()` advances it. Numeric paths remain `Uint32Array`s, matching the compact representation already used by the stepper and supported by motion-plan packing. `WaterPathFinder.pathForTraversal()` retains the existing second water-graph freshness check. If the graph refresh replaces the stepper between `next()` and motion-plan recording, it falls back to the original one-shot query. Otherwise it returns the cached remainder. The method also normalizes the result to begin at the tile returned by `next()`, so `TradeShipExecution` does not need to mutate the path. `findPath()` remains a stateless one-shot query. The change is limited to TradeShip and the minimal pathfinder support; it includes no TransportShip changes. ## Performance Current upstream `main` (`da2e0918`) was compared with the same commit plus only this change. Each variant was run once per replay on macOS arm64 with CPU profiling disabled. The replay intents were identical between variants, and the current-client final hash and hash tick matched for every pair. | Game ID | Workload | Whole replay | TradeShip execution | Throughput | | --- | --- | ---: | ---: | ---: | | `DWmULj4H` | Antarctica, TradeShip-heavy | 68.960 s → 60.271 s (**-12.60%**) | 18.981 s → 11.844 s (**-37.60%**) | 446 → 510 ticks/s | | `efA7EZv9` | Australia | 14.660 s → 14.346 s (**-2.14%**) | 1.216 s → 0.920 s (**-24.34%**) | 441 → 450 ticks/s | | `waxvMCue` | Svalmel | 16.666 s → 16.537 s (**-0.77%**) | 1.169 s → 0.931 s (**-20.36%**) | 551 → 555 ticks/s | Matched current-client final hashes: - `DWmULj4H`: `49212092378184090` at tick 31,060 - `efA7EZv9`: `82334231069422620` at tick 6,760 - `waxvMCue`: `34697926985435590` at tick 9,480 ## Validation - `npm run build-prod` - Full Node 24 coverage suite: 174 test files and 2,091 tests passed - Full ESLint check - Full Prettier check - Current-main baseline/patched replay hashes matched for all three benchmark games ## Checklist - [x] No UI updates; screenshots are not applicable. - [x] No user-facing text was added or changed. - [x] Relevant tests were added. ## LLM disclosure This change, PR description, and benchmark analysis were authored by **GPT-5.6 Sol**, an LLM, under human direction.
175 lines
5.4 KiB
TypeScript
175 lines
5.4 KiB
TypeScript
import { TradeShipExecution } from "../../../src/core/execution/TradeShipExecution";
|
|
import { Game, MessageType, Player, Unit } from "../../../src/core/game/Game";
|
|
import { PathStatus } from "../../../src/core/pathfinding/types";
|
|
import { setup } from "../../util/Setup";
|
|
|
|
describe("TradeShipExecution", () => {
|
|
let game: Game;
|
|
let origOwner: Player;
|
|
let dstOwner: Player;
|
|
let pirate: Player;
|
|
let srcPort: Unit;
|
|
let piratePort: Unit;
|
|
let piratePort2: Unit;
|
|
let tradeShip: Unit;
|
|
let dstPort: Unit;
|
|
let tradeShipExecution: TradeShipExecution;
|
|
|
|
beforeEach(async () => {
|
|
// Mock Game, Player, Unit, and required methods
|
|
|
|
game = await setup("ocean_and_land", {
|
|
infiniteGold: true,
|
|
instantBuild: true,
|
|
});
|
|
game.displayMessage = vi.fn();
|
|
origOwner = {
|
|
canBuild: vi.fn(() => true),
|
|
buildUnit: vi.fn((type, spawn, opts) => tradeShip),
|
|
displayName: vi.fn(() => "Origin"),
|
|
addGold: vi.fn(),
|
|
units: vi.fn(() => [dstPort]),
|
|
unitCount: vi.fn(() => 1),
|
|
id: vi.fn(() => 1),
|
|
clientID: vi.fn(() => 1),
|
|
canTrade: vi.fn(() => true),
|
|
} as any;
|
|
|
|
dstOwner = {
|
|
id: vi.fn(() => 2),
|
|
addGold: vi.fn(),
|
|
displayName: vi.fn(() => "Destination"),
|
|
units: vi.fn(() => [dstPort]),
|
|
unitCount: vi.fn(() => 1),
|
|
clientID: vi.fn(() => 2),
|
|
canTrade: vi.fn(() => true),
|
|
} as any;
|
|
|
|
pirate = {
|
|
id: vi.fn(() => 3),
|
|
addGold: vi.fn(),
|
|
displayName: vi.fn(() => "Destination"),
|
|
units: vi.fn(() => [piratePort, piratePort2]),
|
|
unitCount: vi.fn(() => 2),
|
|
canTrade: vi.fn(() => true),
|
|
} as any;
|
|
|
|
piratePort = {
|
|
id: vi.fn(() => 201),
|
|
tile: vi.fn(() => 56),
|
|
owner: vi.fn(() => pirate),
|
|
isActive: vi.fn(() => true),
|
|
isUnderConstruction: vi.fn(() => false),
|
|
isMarkedForDeletion: vi.fn(() => false),
|
|
} as any;
|
|
|
|
piratePort2 = {
|
|
id: vi.fn(() => 202),
|
|
tile: vi.fn(() => 75),
|
|
owner: vi.fn(() => pirate),
|
|
isActive: vi.fn(() => true),
|
|
isUnderConstruction: vi.fn(() => false),
|
|
isMarkedForDeletion: vi.fn(() => false),
|
|
} as any;
|
|
|
|
srcPort = {
|
|
id: vi.fn(() => 101),
|
|
tile: vi.fn(() => 10),
|
|
owner: vi.fn(() => origOwner),
|
|
isActive: vi.fn(() => true),
|
|
isUnderConstruction: vi.fn(() => false),
|
|
isMarkedForDeletion: vi.fn(() => false),
|
|
} as any;
|
|
|
|
dstPort = {
|
|
id: vi.fn(() => 102),
|
|
tile: vi.fn(() => 100),
|
|
owner: vi.fn(() => dstOwner),
|
|
isActive: vi.fn(() => true),
|
|
isUnderConstruction: vi.fn(() => false),
|
|
isMarkedForDeletion: vi.fn(() => false),
|
|
} as any;
|
|
|
|
tradeShip = {
|
|
isActive: vi.fn(() => true),
|
|
owner: vi.fn(() => origOwner),
|
|
id: vi.fn(() => 123),
|
|
move: vi.fn(),
|
|
setTargetUnit: vi.fn(),
|
|
setSafeFromPirates: vi.fn(),
|
|
touch: vi.fn(),
|
|
delete: vi.fn(),
|
|
tile: vi.fn(() => 32),
|
|
} as any;
|
|
|
|
tradeShipExecution = new TradeShipExecution(origOwner, srcPort, dstPort);
|
|
tradeShipExecution.init(game, 0);
|
|
tradeShipExecution["pathFinder"] = {
|
|
next: vi.fn(() => ({ status: PathStatus.NEXT, node: 32 })),
|
|
findPath: vi.fn((from: number) => [from]),
|
|
pathForTraversal: vi.fn(() => [32]),
|
|
} as any;
|
|
tradeShipExecution["tradeShip"] = tradeShip;
|
|
});
|
|
|
|
it("should initialize and tick without errors", () => {
|
|
const pathFinder = tradeShipExecution["pathFinder"];
|
|
tradeShipExecution.tick(1);
|
|
expect(tradeShipExecution.isActive()).toBe(true);
|
|
expect(pathFinder.pathForTraversal).toHaveBeenCalledOnce();
|
|
expect(pathFinder.findPath).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("should deactivate if tradeShip is not active", () => {
|
|
tradeShip.isActive = vi.fn(() => false);
|
|
tradeShipExecution.tick(1);
|
|
expect(tradeShipExecution.isActive()).toBe(false);
|
|
});
|
|
|
|
it("should delete ship if port owner changes to current owner", () => {
|
|
dstPort.owner = vi.fn(() => origOwner);
|
|
tradeShipExecution.tick(1);
|
|
expect(tradeShip.delete).toHaveBeenCalledWith(false);
|
|
expect(tradeShipExecution.isActive()).toBe(false);
|
|
});
|
|
|
|
it("should pick another port if ship is captured", () => {
|
|
tradeShip.owner = vi.fn(() => pirate);
|
|
tradeShipExecution.tick(1);
|
|
expect(tradeShip.setTargetUnit).toHaveBeenCalledWith(piratePort);
|
|
});
|
|
|
|
it("should notify the original owner when the ship is captured", () => {
|
|
tradeShip.owner = vi.fn(() => pirate);
|
|
tradeShipExecution.tick(1);
|
|
expect(game.displayMessage).toHaveBeenCalledWith(
|
|
"events_display.trade_ship_captured",
|
|
MessageType.UNIT_DESTROYED,
|
|
origOwner.id(),
|
|
undefined,
|
|
{ name: pirate.displayName() },
|
|
tradeShip.id(),
|
|
);
|
|
});
|
|
|
|
it("should only notify the original owner once across ticks", () => {
|
|
tradeShip.owner = vi.fn(() => pirate);
|
|
tradeShipExecution.tick(1);
|
|
tradeShipExecution.tick(2);
|
|
expect(game.displayMessage).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("should complete trade and award gold", () => {
|
|
tradeShipExecution["pathFinder"] = {
|
|
next: vi.fn(() => ({ status: PathStatus.COMPLETE, node: 32 })),
|
|
findPath: vi.fn((from: number) => [from]),
|
|
pathForTraversal: vi.fn(() => [32]),
|
|
} as any;
|
|
tradeShipExecution.tick(1);
|
|
expect(tradeShip.delete).toHaveBeenCalledWith(false);
|
|
expect(tradeShipExecution.isActive()).toBe(false);
|
|
expect(origOwner.addGold).toHaveBeenCalled();
|
|
expect(dstOwner.addGold).toHaveBeenCalled();
|
|
});
|
|
});
|