mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-21 21:04:14 +00:00
8cc6c2c2aa
## Description: Train spawning hot-path optimization (trade destination selection) ## Summary This PR reduces per-tick overhead in train spawning by removing temporary allocations and reducing work in the destination-selection path. The change focuses on `Cluster` trade destination lookup and how `TrainStationExecution` picks a destination. ## What changed ### 1) Maintain a “trade-capable” station subset per cluster `src/core/game/TrainStation.ts` - `Cluster` now maintains: - `stations`: all stations in the cluster (unchanged) - `tradeStations`: maintained subset of stations that can act as trade endpoints (`City` or `Port`) - `tradeStations` is kept in sync in: - `addStation()` - `removeStation()` - `clear()` Impact: - Trade queries no longer scan every station in the cluster; they only scan `tradeStations`. ### 2) Add cheap eligibility helpers `src/core/game/TrainStation.ts` - `hasAnyTradeDestination(player)`: - Fast early-exit check: returns as soon as it finds any eligible trade destination. - `randomTradeDestination(player, random)`: - Picks a random eligible trade destination directly without materializing an intermediate `Set`. ### 3) Use reservoir sampling for single-pass random choice `src/core/game/TrainStation.ts` `Cluster.randomTradeDestination()` uses reservoir sampling: - Iterates `tradeStations` once. - Maintains a running count of eligible stations (`eligibleSeen`). - Replaces the selected station with probability `1/eligibleSeen`. Properties: - Uniform selection among eligible stations. - One pass instead of “count then pick by index” (two pass). - Allocation-free. - Returns `null` when no eligible destination exists. ### 4) Update train spawning to avoid temporary sets `src/core/execution/TrainStationExecution.ts` - Previously: `spawnTrain()` called `cluster.availableForTrade()` and then `random.randFromSet(...)`. - This built a new `Set` on the hot path. - Now: - Early-exit via `cluster.hasAnyTradeDestination(owner)`. - Destination via `cluster.randomTradeDestination(owner, random)`. Net effect: - Less per-tick work and no per-spawn temporary `Set` allocations. ## Why this helps Train spawning happens frequently and can become a hot path in large games / large rail clusters. Avoiding repeated allocations and reducing work inside `tick()` helps keep frame/update time predictable. ## notes - Trade rules are unchanged (`tradeAvailable(player)` still gates eligibility). - Destination selection remains random-uniform over eligible `City`/`Port` stations that satisfy `tradeAvailable(player)`. - `TrainStationExecution` now avoids calling `spawnTrain()` entirely when `spawnTrains` is falsy (it was already guarded inside). ## Please complete the following: - [ ] I have added screenshots for all UI updates - [ ] I process any text displayed to the user through translateText() and I've added it to the en.json file - [ ] I have added relevant tests to the test directory - [ ] 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: DISCORD_USERNAME
71 lines
2.0 KiB
TypeScript
71 lines
2.0 KiB
TypeScript
import { vi, type Mocked } from "vitest";
|
|
import { UnitType } from "../../../src/core/game/Game";
|
|
import { Cluster, TrainStation } from "../../../src/core/game/TrainStation";
|
|
|
|
const createMockStation = (id: string): Mocked<TrainStation> => {
|
|
return {
|
|
id,
|
|
unit: {
|
|
type: vi.fn(() => UnitType.City),
|
|
} as any,
|
|
setCluster: vi.fn(),
|
|
getCluster: vi.fn(() => null),
|
|
} as any;
|
|
};
|
|
|
|
describe("Cluster tests", () => {
|
|
let cluster: Cluster;
|
|
let stationA: Mocked<TrainStation>;
|
|
let stationB: Mocked<TrainStation>;
|
|
let stationC: Mocked<TrainStation>;
|
|
|
|
beforeEach(() => {
|
|
cluster = new Cluster();
|
|
stationA = createMockStation("A");
|
|
stationB = createMockStation("B");
|
|
stationC = createMockStation("C");
|
|
});
|
|
|
|
test("addStation adds a station and sets cluster", () => {
|
|
cluster.addStation(stationA);
|
|
|
|
expect(cluster.has(stationA)).toBe(true);
|
|
expect(stationA.setCluster).toHaveBeenCalledWith(cluster);
|
|
});
|
|
|
|
test("removeStation removes station from cluster", () => {
|
|
cluster.addStation(stationA);
|
|
cluster.removeStation(stationA);
|
|
|
|
expect(cluster.has(stationA)).toBe(false);
|
|
});
|
|
|
|
test("addStations adds multiple stations and sets cluster", () => {
|
|
const set = new Set([stationA, stationB]);
|
|
|
|
cluster.addStations(set);
|
|
|
|
expect(cluster.has(stationA)).toBe(true);
|
|
expect(cluster.has(stationB)).toBe(true);
|
|
expect(stationA.setCluster).toHaveBeenCalledWith(cluster);
|
|
expect(stationB.setCluster).toHaveBeenCalledWith(cluster);
|
|
});
|
|
|
|
test("merge combines stations from another cluster", () => {
|
|
const otherCluster = new Cluster();
|
|
otherCluster.addStation(stationB);
|
|
otherCluster.addStation(stationC);
|
|
|
|
cluster.addStation(stationA);
|
|
cluster.merge(otherCluster);
|
|
|
|
expect(cluster.has(stationA)).toBe(true);
|
|
expect(cluster.has(stationB)).toBe(true);
|
|
expect(cluster.has(stationC)).toBe(true);
|
|
});
|
|
|
|
test("has returns false for non-member stations", () => {
|
|
expect(cluster.has(stationA)).toBe(false);
|
|
});
|
|
});
|