Show rail ghost for initial factory 🚂

[evanpelle]

Problem

Fixes #4284. When you build a factory in an area with no pre-existing factory (e.g. just a city nearby), no rail ghost preview appeared — even though building the factory would lay rail lines connecting it to that city.

Root cause

computeGhostRailPaths in RailNetworkImpl.ts had two factory-hostile assumptions:

1. It bailed out early unless a Factory was already in range (hasUnitNearby(..., UnitType.Factory)).
2. It only matched neighbors that were already train stations (findStation(...) → skipped if null).

But a Factory always becomes a station itself and promotes nearby City/Port/Factory into the rail network (see FactoryExecution). So it needs no pre-existing factory, and its neighbors won't be stations yet on first build. A City/Port only joins the network when a factory already exists (CityExecution/PortExecution) — so their behavior is correctly left unchanged.

Fix

• Skip the "factory must be nearby" gate when the placed unit is itself a Factory.
• For a factory build, pathfind to nearby City/Port/Factory even if they aren't stations yet. City/Port keep connecting only to existing stations.

Tests

Added two cases to RailNetwork.test.ts (factory connects with no pre-existing factory; city still doesn't without one). All 25 tests pass.

Note on scope

As @Katokoda noted on the issue, a fully build-exact preview (neighboring structures also connecting to each other, merging existing networks, etc.) is larger and order-dependent. This PR resolves the reported bug — the initial factory now shows its rail ghost — and leaves the exact-match cascade as a separate follow-up.

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

computeGhostRailPaths in RailNetworkImpl is updated so Factory units bypass the "nearby factory required" early-return guard that still applies to City/Port. Neighbor tile selection is also broadened: when unitType is Factory, pathfinding targets the neighbor unit's tile even if no station exists there yet. Two new tests cover both code paths.

Changes

Factory Ghost Rail Path Fix

Layer / File(s)	Summary
Ghost path gating and neighbor selection src/core/game/RailNetworkImpl.ts, tests/core/game/RailNetwork.test.ts	The early-return guard requiring a nearby factory is now skipped when unitType is Factory. Neighbor tile resolution falls back to the unit's own tile when no station exists and the building unit is a Factory. Two new tests assert Factory path generation and City non-generation under these conditions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• openfrontio/OpenFrontIO#4079: Modifies the same computeGhostRailPaths function in RailNetworkImpl.ts to adjust factory ghost rail snapping eligibility and related tests.

Suggested labels

UI/UX

Poem

🚂 No factory nearby? No problem at all!
The ghost rails now show when you answer the call.
A Factory dreams of tracks yet to be laid,
It snaps to the city — no longer afraid.
Choo choo! 🛤️

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: enabling rail ghost preview for initial factory builds, which is the core bug fix addressed in this PR.
Description check	✅ Passed	The description thoroughly explains the bug, root cause, and fix in relation to the changeset, providing clear context about factory and city/port behavior differences.
Linked Issues check	✅ Passed	The code changes fully address issue #4284 by enabling ghost rail paths for factories without pre-existing factories, fixing the missing ghost preview bug described in the issue.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the reported bug in RailNetworkImpl.ts ghost path logic and adding targeted test coverage, with no unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed due to a network error.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/core/game/RailNetwork.test.ts (1)

380-409: ⚡ Quick win

Use the test setup() helper for these new rail preview cases.

These new tests build the game with local mocks and private (network as any) access. The repo guideline asks tests under tests/** to use tests/util/Setup.ts and exercise core simulation directly, so please rewrite these cases around setup() and real game placement/build-preview behavior instead of mocking RailNetworkImpl internals.

As per coding guidelines, "tests/**/*.test.ts: Use the setup() helper from tests/util/Setup.ts to create test game instances and exercise core simulation directly."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/core/game/RailNetwork.test.ts` around lines 380 - 409, Refactor the
test cases "factory connects to nearby structures with no pre-existing factory"
and "city does not connect to non-station neighbors without a factory" to use
the setup() helper from tests/util/Setup.ts instead of mocking RailNetworkImpl
internals. Replace the local vi.fn() mocks and private (network as any) access
patterns with real game instances created via setup(), and exercise the
computeGhostRailPaths method by setting up actual game state (placing units,
stations, and cities) rather than mocking game.hasUnitNearby, game.nearbyUnits,
stationManager.findStation, pathService.findTilePath, and railGrid. This aligns
with the repo guideline that tests under tests/** should use setup() and test
core simulation behavior directly.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/core/game/RailNetwork.test.ts`:
- Around line 380-409: Refactor the test cases "factory connects to nearby
structures with no pre-existing factory" and "city does not connect to
non-station neighbors without a factory" to use the setup() helper from
tests/util/Setup.ts instead of mocking RailNetworkImpl internals. Replace the
local vi.fn() mocks and private (network as any) access patterns with real game
instances created via setup(), and exercise the computeGhostRailPaths method by
setting up actual game state (placing units, stations, and cities) rather than
mocking game.hasUnitNearby, game.nearbyUnits, stationManager.findStation,
pathService.findTilePath, and railGrid. This aligns with the repo guideline that
tests under tests/** should use setup() and test core simulation behavior
directly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ad25d3d0-b15b-45c3-b413-60582ede7d9e

📥 Commits

Reviewing files that changed from the base of the PR and between 5be72db and 446e5ba.

📒 Files selected for processing (2)

• src/core/game/RailNetworkImpl.ts
• tests/core/game/RailNetwork.test.ts