Skip to content

Local train routing instead of global pathfinding #2473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Local train routing instead of global pathfinding #2473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
51c603a
feat: Implement local train routing with discovery and greedy path se…
scamiv Nov 18, 2025
fda1d39
removed journey source and replaced it with recent stations during j…
scamiv Nov 18, 2025
d6c1dc4
Enhance train routing logic and memory management
scamiv Nov 18, 2025
1d352e1
Fix arrival vs. termination handling to avoid wrong stats and double …
scamiv Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
247 changes: 194 additions & 53 deletions src/core/execution/TrainExecution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -20,9 +26,17 @@ export class TrainExecution implements Execution {
private currentTile: number = 0;
private spacing = 2;
private usedTiles: TileRef[] = []; // used for cars behind
private stations: TrainStation[] = [];
private currentRailroad: OrientedRailroad | null = null;
private currentStation: TrainStation | null = null;
private speed: number = 2;
// Journey tracking for organic route discovery - simplified to immediate neighbors only
private hasProcessedArrival: boolean = false;
private journeyHopCount: number = 0;

// Local greedy routing properties
private recentStations: TrainStation[] = []; // Recently visited stations (for loop prevention)
private maxHops: number = 50; // Maximum hops before giving up
private recentMemorySize: number = 50; // How many recent stations to remember

constructor(
private railNetwork: RailNetwork,
Expand All @@ -36,27 +50,81 @@ export class TrainExecution implements Execution {
return this.player;
}

/**
* Share journey information with a station for organic route discovery
*/
public shareJourneyInfo(): {
routeInformation: Array<{
destination: TrainStation;
nextHop: TrainStation | null;
distance: number;
}>;
} {
const routeInformation: Array<{
destination: TrainStation;
nextHop: TrainStation | null;
distance: number;
}> = [];

// Derive routing info from recentStations array
// recentStations = [oldest, ..., previous, current]
const immediatePrevious =
this.recentStations.length > 1
? this.recentStations[this.recentStations.length - 2]
: null;

// Find the start index for sharing journey information
// Only share information about stations visited since the last time we passed through the current station
let startIndex = 0;
const currentStation = this.recentStations[this.recentStations.length - 1];

// Look for the last occurrence of current station before the current visit
for (let i = this.recentStations.length - 2; i >= 0; i--) {
if (this.recentStations[i] === currentStation) {
// Found the last previous visit to this station, start sharing from after that visit
startIndex = i + 1;
break;
}
}

// Only share routes to stations we visited since our last visit to this station (not including current)
for (let i = startIndex; i < this.recentStations.length - 1; i++) {
const destination = this.recentStations[i];
// For reverse routing: to reach any destination, go through the station we came from
const nextHop = immediatePrevious;
// Distance from current station to this destination
const distance = this.recentStations.length - 1 - i;

routeInformation.push({
destination,
nextHop,
distance,
});
}

return {
routeInformation,
};
}
Comment on lines +53 to +108
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Guard against empty recentStations to prevent undefined access.

At the first station (source), stationReached() is called before recentStations is populated (line 331 pushes nextHop after arrival processing completes). This means:

  • Line 79: this.recentStations[this.recentStations.length - 1] evaluates to undefined when recentStations is empty.
  • Line 83: Comparing undefined === currentStation will always be false, but accessing properties of undefined later could cause runtime errors.

Additionally, the comment on line 70 states "recentStations = [oldest, ..., previous, current]", but the current station is not actually in the array when this method is called—only after line 331 runs.

To fix, consider one of these approaches:

Option 1 (minimal fix): Guard at the start of shareJourneyInfo:

 public shareJourneyInfo(): {
   routeInformation: Array<{
     destination: TrainStation;
     nextHop: TrainStation | null;
     distance: number;
   }>;
 } {
+  // Nothing to share if we haven't built up a journey yet
+  if (this.recentStations.length === 0) {
+    return { routeInformation: [] };
+  }
+
   const routeInformation: Array<{
     destination: TrainStation;
     nextHop: TrainStation | null;
     distance: number;
   }> = [];

Option 2 (clearer semantics): Initialize recentStations with the source in init() (line 125) so it always contains at least the starting point:

   this.currentStation = this.source;
+  this.recentStations.push(this.source);

Then update the comment on line 70 to clarify that recentStations contains stations we've visited (including current), not stations we're heading to.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Share journey information with a station for organic route discovery
*/
public shareJourneyInfo(): {
routeInformation: Array<{
destination: TrainStation;
nextHop: TrainStation | null;
distance: number;
}>;
} {
const routeInformation: Array<{
destination: TrainStation;
nextHop: TrainStation | null;
distance: number;
}> = [];
// Derive routing info from recentStations array
// recentStations = [oldest, ..., previous, current]
const immediatePrevious =
this.recentStations.length > 1
? this.recentStations[this.recentStations.length - 2]
: null;
// Find the start index for sharing journey information
// Only share information about stations visited since the last time we passed through the current station
let startIndex = 0;
const currentStation = this.recentStations[this.recentStations.length - 1];
// Look for the last occurrence of current station before the current visit
for (let i = this.recentStations.length - 2; i >= 0; i--) {
if (this.recentStations[i] === currentStation) {
// Found the last previous visit to this station, start sharing from after that visit
startIndex = i + 1;
break;
}
}
// Only share routes to stations we visited since our last visit to this station (not including current)
for (let i = startIndex; i < this.recentStations.length - 1; i++) {
const destination = this.recentStations[i];
// For reverse routing: to reach any destination, go through the station we came from
const nextHop = immediatePrevious;
// Distance from current station to this destination
const distance = this.recentStations.length - 1 - i;
routeInformation.push({
destination,
nextHop,
distance,
});
}
return {
routeInformation,
};
}
/**
* Share journey information with a station for organic route discovery
*/
public shareJourneyInfo(): {
routeInformation: Array<{
destination: TrainStation;
nextHop: TrainStation | null;
distance: number;
}>;
} {
// Nothing to share if we haven't built up a journey yet
if (this.recentStations.length === 0) {
return { routeInformation: [] };
}
const routeInformation: Array<{
destination: TrainStation;
nextHop: TrainStation | null;
distance: number;
}> = [];
// Derive routing info from recentStations array
// recentStations = [oldest, ..., previous, current]
const immediatePrevious =
this.recentStations.length > 1
? this.recentStations[this.recentStations.length - 2]
: null;
// Find the start index for sharing journey information
// Only share information about stations visited since the last time we passed through the current station
let startIndex = 0;
const currentStation = this.recentStations[this.recentStations.length - 1];
// Look for the last occurrence of current station before the current visit
for (let i = this.recentStations.length - 2; i >= 0; i--) {
if (this.recentStations[i] === currentStation) {
// Found the last previous visit to this station, start sharing from after that visit
startIndex = i + 1;
break;
}
}
// Only share routes to stations we visited since our last visit to this station (not including current)
for (let i = startIndex; i < this.recentStations.length - 1; i++) {
const destination = this.recentStations[i];
// For reverse routing: to reach any destination, go through the station we came from
const nextHop = immediatePrevious;
// Distance from current station to this destination
const distance = this.recentStations.length - 1 - i;
routeInformation.push({
destination,
nextHop,
distance,
});
}
return {
routeInformation,
};
}
🤖 Prompt for AI Agents 
In src/core/execution/TrainExecution.ts around lines 53-108, shareJourneyInfo
can access this.recentStations[this.recentStations.length - 1] when
recentStations is empty causing undefined access; add an early guard that
returns an empty routeInformation array when recentStations.length === 0, and
adjust the comment near line 70 to accurately describe the contents (e.g.,
"recentStations = [oldest, ..., previous, current] when non-empty; may be empty
at start"); alternatively, ensure recentStations is initialized with the source
in init() so it is never empty — implement one of these fixes and update the
comment accordingly.


init(mg: Game, ticks: number): void {
this.mg = mg;
const stations = this.railNetwork.findStationsPath(
this.source,
this.destination,
);
if (!stations || stations.length <= 1) {

// Validate that source and destination are active
if (!this.source.isActive() || !this.destination.isActive()) {
this.active = false;
return;
}

this.stations = stations;
const railroad = getOrientedRailroad(this.stations[0], this.stations[1]);
if (railroad) {
this.currentRailroad = railroad;
} else {
// If source and destination are the same, we're already there
if (this.source === this.destination) {
this.active = false;
return;
}

const spawn = this.player.canBuild(UnitType.Train, this.stations[0].tile());
this.currentStation = this.source;

const spawn = this.player.canBuild(UnitType.Train, this.source.tile());
if (spawn === false) {
console.warn(`cannot build train`);
this.active = false;
Expand All @@ -74,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;
}
}

Expand All @@ -98,6 +178,12 @@ export class TrainExecution implements Execution {
if (this.train === null) {
return;
}

// Record train arrival statistics
if (this.mg) {
this.mg.recordTrainArrival(this.journeyHopCount);
}

this.train.setReachedTarget();
this.cars.forEach((car: Unit) => {
car.setReachedTarget();
Expand Down Expand Up @@ -140,11 +226,7 @@ export class TrainExecution implements Execution {
}

private activeSourceOrDestination(): boolean {
return (
this.stations.length > 1 &&
this.stations[1].isActive() &&
this.stations[0].isActive()
);
return this.source.isActive() && this.destination.isActive();
}

/**
Expand Down Expand Up @@ -187,49 +269,108 @@ export class TrainExecution implements Execution {
}
}

private nextStation() {
if (this.stations.length > 2) {
this.stations.shift();
const railRoad = getOrientedRailroad(this.stations[0], this.stations[1]);
if (railRoad) {
this.currentRailroad = railRoad;
return true;
}
}
return false;
}
private isAtStation(): boolean {
if (!this.train || !this.currentStation || !this.mg) return false;

private canTradeWithDestination() {
// Check if train is at the current station's tile
const trainTile = this.train.tile();
return (
this.stations.length > 1 && this.stations[1].tradeAvailable(this.player)
this.mg.x(trainTile) === this.mg.x(this.currentStation.tile()) &&
this.mg.y(trainTile) === this.mg.y(this.currentStation.tile())
);
}

private getNextTile(): TileRef | null {
if (this.currentRailroad === null || !this.canTradeWithDestination()) {
return 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
if (!this.hasProcessedArrival) {
this.stationReached(); // Handle arrival at current station
this.hasProcessedArrival = true;
}

// Check if we've reached the destination
if (this.currentStation === this.destination) {
return { kind: "arrived" };
}

// Check if we've exceeded max hops
if (this.journeyHopCount >= this.maxHops) {
// Give up - we've wandered too long
this.active = false;
return { kind: "hopLimit" };
}

// Use local greedy routing to choose next station
const nextHop = this.currentStation!.chooseNextStation(
this.destination,
this.recentStations,
this.player,
);

if (!nextHop) {
// 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) {
this.active = false;
return { kind: "stuck" }; // No direct connection
}

// Reset arrival flag since we're departing
this.hasProcessedArrival = false;

// Notify current station that train is departing
this.currentStation!.onTrainDepartureFromStation(this);

// Update recent stations memory for loop prevention
this.recentStations.push(nextHop);
if (this.recentStations.length > this.recentMemorySize) {
this.recentStations.shift(); // Remove oldest
}

// Update journey tracking - remember where we came from BEFORE changing currentStation
// This should happen after arrival processing but before departure
this.journeyHopCount++;

this.currentStation = nextHop;
this.currentRailroad = railroad;
this.currentTile = 0;
}
Comment on lines +283 to 343
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Consider pushing currentStation instead of nextHop to recentStations for clearer semantics.

Currently, line 331 pushes nextHop to recentStations after choosing it. This creates a timing mismatch:

  • recentStations contains "stations we've chosen to go to" rather than "stations we've arrived at."
  • On the first call at the source, recentStations is empty, so chooseNextStation (line 305) has no history to work with.
  • After choosing nextHop and pushing it, the next call to chooseNextStation at nextHop will see nextHop in recentStations and avoid it.

While this technically prevents immediate backtracking, the semantics are confusing. The array is supposed to represent visited stations (as implied by the comment on line 70 and its use in shareJourneyInfo), but it actually contains stations we're heading to.

Suggested fix: Push currentStation to recentStations during arrival processing, before choosing the next hop:

     // Process arrival if we haven't already for this station visit
     if (!this.hasProcessedArrival) {
       this.stationReached(); // Handle arrival at current station
       this.hasProcessedArrival = true;
+      
+      // Record this station as visited for loop prevention
+      this.recentStations.push(this.currentStation!);
+      if (this.recentStations.length > this.recentMemorySize) {
+        this.recentStations.shift();
+      }
     }

     // Check if we've reached the destination
     if (this.currentStation === this.destination) {
       return { kind: "arrived" };
     }
     
     ...
     
     // Notify current station that train is departing
     this.currentStation!.onTrainDepartureFromStation(this);

-    // Update recent stations memory for loop prevention
-    this.recentStations.push(nextHop);
-    if (this.recentStations.length > this.recentMemorySize) {
-      this.recentStations.shift(); // Remove oldest
-    }

     // Update journey tracking - remember where we came from BEFORE changing currentStation

This ensures recentStations truly represents visited stations and fixes the shareJourneyInfo bug (from the previous comment) in a semantically correct way.

Alternatively, if you prefer the current timing, update the comments to clarify that recentStations contains "stations we've committed to visit or have visited," and ensure shareJourneyInfo accounts for the timing difference.

🤖 Prompt for AI Agents 
In src/core/execution/TrainExecution.ts around lines 283-343, recentStations is
being updated by pushing nextHop after selecting it, so the array actually
represents "stations we're heading to" rather than stations we've visited; to
fix this, push the currentStation into recentStations during arrival processing
(before calling chooseNextStation), then enforce the recentStations length trim
there as well, and remove the later push of nextHop; this preserves semantics
that recentStations lists visited stations and keeps chooseNextStation aware of
the immediate past; optionally update comments and adjust shareJourneyInfo if it
relied on the previous timing.

this.saveTraversedTiles(this.currentTile, this.speed);
this.currentTile = this.currentTile + this.speed;
const leftOver = this.currentTile - this.currentRailroad.getTiles().length;
if (leftOver >= 0) {
// Station reached, pick the next station
this.stationReached();
if (!this.nextStation()) {
return null; // Destination reached (or no valid connection)

// Follow current railroad
if (
this.currentRailroad &&
this.currentTile < this.currentRailroad.getTiles().length
) {
this.saveTraversedTiles(this.currentTile, this.speed);
this.currentTile += this.speed;

if (this.currentTile >= this.currentRailroad.getTiles().length) {
// We've reached the next station
this.currentTile = this.currentRailroad.getTiles().length - 1;
}
this.currentTile = leftOver;
this.saveTraversedTiles(0, leftOver);

return {
kind: "move",
tile: this.currentRailroad.getTiles()[this.currentTile],
};
}
return this.currentRailroad.getTiles()[this.currentTile];

this.active = false;
return { kind: "stuck" };
}

private stationReached() {
if (this.mg === null || this.player === null) {
if (this.mg === null || this.player === null || !this.currentStation) {
throw new Error("Not initialized");
}
this.stations[1].onTrainStop(this);
return;

this.currentStation.onTrainStop(this);
}

isActive(): boolean {
Expand Down
4 changes: 4 additions & 0 deletions src/core/execution/TrainStationExecution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ export class TrainStationExecution implements Execution {
this.active = false;
return;
}

// Handle periodic routing broadcasts
this.station.tick();

this.spawnTrain(this.station, ticks);
}

Expand Down
2 changes: 2 additions & 0 deletions src/core/game/Game.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,8 @@ export interface Game extends GameMap {
): Array<{ unit: Unit; distSquared: number }>;

addExecution(...exec: Execution[]): void;
recordTrainArrival(steps: number): void;
recordTrainRemovedDueToHopLimit(steps: number): void;
displayMessage(
message: string,
type: MessageType,
Expand Down
Loading
Loading