Skip to content

Commit 1d352e1

Browse files
committed
Fix arrival vs. termination handling to avoid wrong stats and double counting
The new station-aware routing introduced a bug where train arrivals were being double-counted and termination conditions were incorrectly classified as arrivals. Problems fixed: - targetReached() was called twice for successful arrivals: once in getNextTile() when destination was reached, and again in tick() when no tile was returned - Trains removed due to hop limit were counted as successful arrivals - Trains stuck with no routing options were counted as successful arrivals Solution implemented: - Introduced MoveResult union type with explicit cases: "move", "arrived", "hopLimit", "stuck" to clearly distinguish termination conditions - Renamed getNextTile() to getNextStep() and changed return type to MoveResult - Removed targetReached() call from navigation logic to prevent double counting - Updated tick() method to use switch statement on MoveResult for proper handling - Ensured recordTrainArrival() only called for actual destination arrivals - Ensured recordTrainRemovedDueToHopLimit() only called for hop limit terminations - Stuck trains are deleted without recording any arrival statistics This ensures accurate train statistics tracking with the new routing system.
1 parent d6c1dc4 commit 1d352e1

File tree

1 file changed

+38
-18
lines changed

1 file changed

+38
-18
lines changed

src/core/execution/TrainExecution.ts

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ import { RailNetwork } from "../game/RailNetwork";
1111
import { getOrientedRailroad, OrientedRailroad } from "../game/Railroad";
1212
import { TrainStation } from "../game/TrainStation";
1313

14+
type MoveResult =
15+
| { kind: "move"; tile: TileRef }
16+
| { kind: "arrived" }
17+
| { kind: "hopLimit" }
18+
| { kind: "stuck" };
19+
1420
export class TrainExecution implements Execution {
1521
private active = true;
1622
private mg: Game | null = null;
@@ -136,12 +142,24 @@ export class TrainExecution implements Execution {
136142
return;
137143
}
138144

139-
const tile = this.getNextTile();
140-
if (tile) {
141-
this.updateCarsPositions(tile);
142-
} else {
143-
this.targetReached();
144-
this.deleteTrain();
145+
const result = this.getNextStep();
146+
switch (result.kind) {
147+
case "move":
148+
this.updateCarsPositions(result.tile);
149+
break;
150+
case "arrived":
151+
this.targetReached();
152+
this.deleteTrain();
153+
break;
154+
case "hopLimit":
155+
if (this.mg) {
156+
this.mg.recordTrainRemovedDueToHopLimit(this.journeyHopCount);
157+
}
158+
this.deleteTrain();
159+
break;
160+
case "stuck":
161+
this.deleteTrain();
162+
break;
145163
}
146164
}
147165

@@ -262,7 +280,7 @@ export class TrainExecution implements Execution {
262280
);
263281
}
264282

265-
private getNextTile(): TileRef | null {
283+
private getNextStep(): MoveResult {
266284
// If we're at a station, decide where to go next
267285
if (this.isAtStation()) {
268286
// Process arrival if we haven't already for this station visit
@@ -273,18 +291,14 @@ export class TrainExecution implements Execution {
273291

274292
// Check if we've reached the destination
275293
if (this.currentStation === this.destination) {
276-
this.targetReached();
277-
return null;
294+
return { kind: "arrived" };
278295
}
279296

280297
// Check if we've exceeded max hops
281298
if (this.journeyHopCount >= this.maxHops) {
282299
// Give up - we've wandered too long
283-
if (this.mg) {
284-
this.mg.recordTrainRemovedDueToHopLimit(this.journeyHopCount);
285-
}
286300
this.active = false;
287-
return null;
301+
return { kind: "hopLimit" };
288302
}
289303

290304
// Use local greedy routing to choose next station
@@ -295,14 +309,16 @@ export class TrainExecution implements Execution {
295309
);
296310

297311
if (!nextHop) {
298-
// No good options available - stay and wait
299-
return null;
312+
// No good options available - treat as stuck
313+
this.active = false;
314+
return { kind: "stuck" };
300315
}
301316

302317
// Get railroad to next hop
303318
const railroad = getOrientedRailroad(this.currentStation!, nextHop);
304319
if (!railroad) {
305-
return null; // No direct connection
320+
this.active = false;
321+
return { kind: "stuck" }; // No direct connection
306322
}
307323

308324
// Reset arrival flag since we're departing
@@ -339,10 +355,14 @@ export class TrainExecution implements Execution {
339355
this.currentTile = this.currentRailroad.getTiles().length - 1;
340356
}
341357

342-
return this.currentRailroad.getTiles()[this.currentTile];
358+
return {
359+
kind: "move",
360+
tile: this.currentRailroad.getTiles()[this.currentTile],
361+
};
343362
}
344363

345-
return null;
364+
this.active = false;
365+
return { kind: "stuck" };
346366
}
347367

348368
private stationReached() {

0 commit comments

Comments
 (0)