From 1528a91154ad390c9af8e015206bc8470fa5bc96 Mon Sep 17 00:00:00 2001
From: scamiv <6170744+scamiv@users.noreply.github.com>
Date: Tue, 18 Nov 2025 19:39:48 +0100
Subject: [PATCH] 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.
---
 src/core/execution/TrainExecution.ts | 56 +++++++++++++++++++---------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/src/core/execution/TrainExecution.ts b/src/core/execution/TrainExecution.ts
index bd89a5d13..501f599b5 100644
--- a/src/core/execution/TrainExecution.ts
+++ b/src/core/execution/TrainExecution.ts
@@ -11,6 +11,12 @@ import { RailNetwork } from "../game/RailNetwork";
 import { getOrientedRailroad, OrientedRailroad } from "../game/Railroad";
 import { TrainStation } from "../game/TrainStation";
 
+type MoveResult =
+  | { kind: "move"; tile: TileRef }
+  | { kind: "arrived" }
+  | { kind: "hopLimit" }
+  | { kind: "stuck" };
+
 export class TrainExecution implements Execution {
   private active = true;
   private mg: Game | null = null;
@@ -136,12 +142,24 @@ export class TrainExecution implements Execution {
       return;
     }
 
-    const tile = this.getNextTile();
-    if (tile) {
-      this.updateCarsPositions(tile);
-    } else {
-      this.targetReached();
-      this.deleteTrain();
+    const result = this.getNextStep();
+    switch (result.kind) {
+      case "move":
+        this.updateCarsPositions(result.tile);
+        break;
+      case "arrived":
+        this.targetReached();
+        this.deleteTrain();
+        break;
+      case "hopLimit":
+        if (this.mg) {
+          this.mg.recordTrainRemovedDueToHopLimit(this.journeyHopCount);
+        }
+        this.deleteTrain();
+        break;
+      case "stuck":
+        this.deleteTrain();
+        break;
     }
   }
 
@@ -262,7 +280,7 @@ export class TrainExecution implements Execution {
     );
   }
 
-  private getNextTile(): TileRef | null {
+  private getNextStep(): MoveResult {
     // If we're at a station, decide where to go next
     if (this.isAtStation()) {
       // Process arrival if we haven't already for this station visit
@@ -273,18 +291,14 @@ export class TrainExecution implements Execution {
 
       // Check if we've reached the destination
       if (this.currentStation === this.destination) {
-        this.targetReached();
-        return null;
+        return { kind: "arrived" };
       }
 
       // Check if we've exceeded max hops
       if (this.journeyHopCount >= this.maxHops) {
         // Give up - we've wandered too long
-        if (this.mg) {
-          this.mg.recordTrainRemovedDueToHopLimit(this.journeyHopCount);
-        }
         this.active = false;
-        return null;
+        return { kind: "hopLimit" };
       }
 
       // Use local greedy routing to choose next station
@@ -295,14 +309,16 @@ export class TrainExecution implements Execution {
       );
 
       if (!nextHop) {
-        // No good options available - stay and wait
-        return null;
+        // No good options available - treat as stuck
+        this.active = false;
+        return { kind: "stuck" };
       }
 
       // Get railroad to next hop
       const railroad = getOrientedRailroad(this.currentStation!, nextHop);
       if (!railroad) {
-        return null; // No direct connection
+        this.active = false;
+        return { kind: "stuck" }; // No direct connection
       }
 
       // Reset arrival flag since we're departing
@@ -339,10 +355,14 @@ export class TrainExecution implements Execution {
         this.currentTile = this.currentRailroad.getTiles().length - 1;
       }
 
-      return this.currentRailroad.getTiles()[this.currentTile];
+      return {
+        kind: "move",
+        tile: this.currentRailroad.getTiles()[this.currentTile],
+      };
     }
 
-    return null;
+    this.active = false;
+    return { kind: "stuck" };
   }
 
   private stationReached() {