mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-21 08:20:50 +00:00
Fix railroad glowing green for non-snapping structures (#4281)
## Problem When placing a building near a railroad, the railroad glows green to show the building would snap to it. This should only apply to **City**, **Port**, and **Factory** — but missile silos, SAMs, and defense posts (which cannot be placed on railroads) were also triggering the green highlight. ## Root cause The core's `overlappingRailroads()` populated snap tiles for *every* buildable type. In v31 the green highlight didn't leak because the client renderer (`RailroadLayer.ts`) gated it with a `SNAPPABLE_STRUCTURES = [Port, City, Factory]` allowlist: ```ts if (!SNAPPABLE_STRUCTURES.includes(this.uiState.ghostStructure)) return; ``` That guard was lost when the rendering was rewritten into the WebGL `RailroadPass`, which now unconditionally highlights every tile in `overlappingRailroads`. The data was always there; only the renderer's filter was protecting it. ## Fix Filter by unit type inside `overlappingRailroads()`, mirroring the existing guard in `computeGhostRailPaths()`. This keeps the snap-eligible type list defined once in the core (`RailNetworkImpl`) and fixes the leak regardless of which renderer consumes the data — rather than re-adding a client-side allowlist a future rewrite could drop again. ## Tests Updated `tests/core/game/RailNetwork.test.ts` for the new signature and added a case asserting `MissileSilo`/`DefensePost`/`SAMLauncher` return `[]` (and don't even query the rail grid). All 23 tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1317,7 +1317,7 @@ export class PlayerImpl implements Player {
|
|||||||
canUpgrade,
|
canUpgrade,
|
||||||
cost,
|
cost,
|
||||||
overlappingRailroads: buildNew
|
overlappingRailroads: buildNew
|
||||||
? rail.overlappingRailroads(canBuild as TileRef)
|
? rail.overlappingRailroads(u, canBuild as TileRef)
|
||||||
: [],
|
: [],
|
||||||
ghostRailPaths: buildNew
|
ghostRailPaths: buildNew
|
||||||
? rail.computeGhostRailPaths(u, canBuild as TileRef)
|
? rail.computeGhostRailPaths(u, canBuild as TileRef)
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ export interface RailNetwork {
|
|||||||
removeStation(unit: Unit): void;
|
removeStation(unit: Unit): void;
|
||||||
findStationsPath(from: TrainStation, to: TrainStation): TrainStation[];
|
findStationsPath(from: TrainStation, to: TrainStation): TrainStation[];
|
||||||
stationManager(): StationManager;
|
stationManager(): StationManager;
|
||||||
overlappingRailroads(tile: TileRef): TileRef[];
|
overlappingRailroads(unitType: UnitType, tile: TileRef): TileRef[];
|
||||||
computeGhostRailPaths(unitType: UnitType, tile: TileRef): TileRef[][];
|
computeGhostRailPaths(unitType: UnitType, tile: TileRef): TileRef[][];
|
||||||
recomputeClusters(): void;
|
recomputeClusters(): void;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -223,7 +223,10 @@ export class RailNetworkImpl implements RailNetwork {
|
|||||||
return editedClusters.size !== 0;
|
return editedClusters.size !== 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
overlappingRailroads(tile: TileRef): TileRef[] {
|
overlappingRailroads(unitType: UnitType, tile: TileRef): TileRef[] {
|
||||||
|
if (![UnitType.City, UnitType.Port, UnitType.Factory].includes(unitType)) {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
const tiles = new Set<TileRef>();
|
const tiles = new Set<TileRef>();
|
||||||
for (const railroad of this.railGrid.query(tile, this.stationRadius)) {
|
for (const railroad of this.railGrid.query(tile, this.stationRadius)) {
|
||||||
for (const t of railroad.tiles) {
|
for (const t of railroad.tiles) {
|
||||||
|
|||||||
@@ -178,7 +178,7 @@ describe("RailNetworkImpl", () => {
|
|||||||
};
|
};
|
||||||
(network as any).railGrid = railGridMock;
|
(network as any).railGrid = railGridMock;
|
||||||
|
|
||||||
const result = network.overlappingRailroads(tile);
|
const result = network.overlappingRailroads(UnitType.City, tile);
|
||||||
|
|
||||||
expect(railGridMock.query).toHaveBeenCalledWith(tile, 3);
|
expect(railGridMock.query).toHaveBeenCalledWith(tile, 3);
|
||||||
expect(result).toEqual([42, 45, 50, 60]); // Deduplicated and sorted
|
expect(result).toEqual([42, 45, 50, 60]); // Deduplicated and sorted
|
||||||
@@ -189,10 +189,30 @@ describe("RailNetworkImpl", () => {
|
|||||||
const railGridMock = { query: vi.fn(() => new Set()) };
|
const railGridMock = { query: vi.fn(() => new Set()) };
|
||||||
(network as any).railGrid = railGridMock;
|
(network as any).railGrid = railGridMock;
|
||||||
|
|
||||||
const result = network.overlappingRailroads(tile);
|
const result = network.overlappingRailroads(UnitType.City, tile);
|
||||||
|
|
||||||
expect(result).toEqual([]);
|
expect(result).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test.each([
|
||||||
|
UnitType.MissileSilo,
|
||||||
|
UnitType.DefensePost,
|
||||||
|
UnitType.SAMLauncher,
|
||||||
|
])(
|
||||||
|
"returns empty array for %s which cannot snap to railroads",
|
||||||
|
(unitType) => {
|
||||||
|
const tile = 42 as any;
|
||||||
|
const railGridMock = {
|
||||||
|
query: vi.fn(() => new Set([{ tiles: [50, 42, 60] }])),
|
||||||
|
};
|
||||||
|
(network as any).railGrid = railGridMock;
|
||||||
|
|
||||||
|
const result = network.overlappingRailroads(unitType, tile);
|
||||||
|
|
||||||
|
expect(result).toEqual([]);
|
||||||
|
expect(railGridMock.query).not.toHaveBeenCalled();
|
||||||
|
},
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("computeGhostRailPaths", () => {
|
describe("computeGhostRailPaths", () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user