Placed buildings block pathfinding #98

Merged

liamjd merged 2 commits from 56-buildings-block-pathfinding into main

2026-09-14 11:16:32 +01:00

liamjd commented

2026-09-14 11:11:31 +01:00

Owner

Placed buildings now block pathfinding. TerrainGrid gains a blocked-cell set consulted by is_walkable(), plus block_cells() / unblock_cells() that update the A* graph incrementally, and game.gd drives them from the GameState.building_placed / building_removed signals.

Closes #56.

Decisions taken

Both were open in the issue body and are pinned by tests rather than left to a comment.

A reroll clears the blocked set. generate() empties _blocked before _build_navigation(). A new map is a new world, and buildings whose anchors are now in the sea are meaningless; a blocked set that outlives its terrain corrupts _main_region and therefore pawn spawning. Pinned by test_regenerating_clears_the_blocked_set.

The graph is driven by the signals, not by Buildings. game.gd listens and calls map.block_cells(), so rubble, reservations and hazards can block cells later without going through the registry. Buildings never touches _astar.

The cost of that second choice, because #83 lands on it directly: the listener runs during the emission, so the graph is stale for the duration of the signal, and listener order becomes load-bearing for any other listener that paths inside its own handler. #83's re-path handler must connect after the navigation listener, or defer a frame. The guarantee that holds unconditionally is the weaker one — the graph is current by the time place() returns.

Correction to the issue body

The issue says blocking is "one call per occupied cell and none of the cliff-edge or diagonal-corner logic needs re-running". That is true for cliff edges and false for diagonal corners, and the difference is a pawn walking through the corner of a building. Recorded in full at #56 (comment 582).

remove_point() drops the edges incident to the point, but _can_traverse() gives a cell authority over edges it is not an endpoint of. For a blocked cell C = (10, 10), the diagonal (10, 11) -> (11, 10) has side_b == C, so blocking C must close it — yet C is neither endpoint and remove_point(C) leaves it intact. Four such edges per cell, joining its orthogonal neighbours in pairs.

So both paths update _blocked, add or remove the points, and then call one shared _refresh_connections_around() that re-derives every edge in the 3x3 around each changed cell from _can_traverse(). Neither path carries bespoke edge logic, so neither can disagree with the other, and the HALF_NEIGHBOURS asymmetry the issue warns about never arises — HALF_NEIGHBOURS stays confined to the full sweep in _build_navigation() where it is correct.

Testing

Unit — 32 new tests, written before the implementation. Full suite: 374 passing, 0 failing, 1585 asserts.

test/unit/test_terrain_navigation.gd (23) covers the issue's list plus four cases it did not name:

corner-cutting, closed on block and reopened on unblock — the tests that catch the correction above
unblock_cells() must re-ask the terrain, so a water cell never gains a point
idempotent blocking, and out-of-bounds cells inert
a wall across the map leaves _main_region as the larger half, not "landmass minus wall" — the case a _main_region only recomputed in generate() gets wrong

test/unit/test_buildings_block_navigation.gd (9) covers the registry seam: the L-shape's hole stays walkable (a bounding-box implementation passes every 1x1 test and fails this one), rotation is derived from the same cells() call, removal hands cells back, and removing one of two adjacent buildings leaves the survivor blocking.

Headed — tools/screenshot_building_blocks_path.gd, 12 checks, all passing. gl_compatibility draws nothing headless, so this runs headed against the real game.tscn and saves four PNGs to tmp/. It wires nothing by hand, which is what makes the game.gd listener genuinely covered — unhook it and CHECK 1 fails.

From the run: main region 256 -> 247 on a 9-cell footprint (exact; no severed region), detour 18 waypoints against 16 direct, and back to 16 after removal — so unblock_cells() leaves no invisible wall where a building stood.

The screenshots show a pawn walking, the rocket dropped on its trail, the trail visibly doglegging around the rocket once the move is re-issued, and the ground clear again after removal.

Not covered

Stale in-flight paths are still stale — a pawn already walking strolls through a building placed in front of it. That is #83, and the second screenshot shows it plainly. The probe prints it as EXPECTED so it does not read as a defect of this change.
The game.gd connection is not covered by the unit suite. test_buildings_block_navigation.gd mirrors the listener rather than invoking it, because game.gd needs the whole scene and cannot be built headless — delete the connection and those 9 tests still pass. The headed probe is what guards it.
"Blocking then unblocking restores an identical path" is asserted as identical cost, not identical waypoints. The connections themselves are pinned exactly by test_unblocking_restores_exactly_the_connections_it_had; AStar2D breaks ties between two equally short routes on internal ordering, so waypoint-for-waypoint equality would assert an implementation detail of the engine rather than of this issue.
Entrances are still #55. This change only leaves room for them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KDzLtT2uajq2ozyFVydZeG

Placed buildings now block pathfinding. `TerrainGrid` gains a blocked-cell set consulted by `is_walkable()`, plus `block_cells()` / `unblock_cells()` that update the A* graph incrementally, and `game.gd` drives them from the `GameState.building_placed` / `building_removed` signals. Closes #56. ## Decisions taken Both were open in the issue body and are pinned by tests rather than left to a comment. **A reroll clears the blocked set.** `generate()` empties `_blocked` before `_build_navigation()`. A new map is a new world, and buildings whose anchors are now in the sea are meaningless; a blocked set that outlives its terrain corrupts `_main_region` and therefore pawn spawning. Pinned by `test_regenerating_clears_the_blocked_set`. **The graph is driven by the signals, not by `Buildings`.** `game.gd` listens and calls `map.block_cells()`, so rubble, reservations and hazards can block cells later without going through the registry. `Buildings` never touches `_astar`. The cost of that second choice, because #83 lands on it directly: the listener runs *during* the emission, so the graph is stale for the duration of the signal, and listener order becomes load-bearing for any other listener that paths inside its own handler. #83's re-path handler must connect after the navigation listener, or defer a frame. The guarantee that holds unconditionally is the weaker one — the graph is current by the time `place()` returns. ## Correction to the issue body The issue says blocking is "one call per occupied cell and none of the cliff-edge or diagonal-corner logic needs re-running". That is true for cliff edges and **false for diagonal corners**, and the difference is a pawn walking through the corner of a building. Recorded in full at #56 (comment 582). `remove_point()` drops the edges *incident to* the point, but `_can_traverse()` gives a cell authority over edges it is not an endpoint of. For a blocked cell `C = (10, 10)`, the diagonal `(10, 11) -> (11, 10)` has `side_b == C`, so blocking `C` must close it — yet `C` is neither endpoint and `remove_point(C)` leaves it intact. Four such edges per cell, joining its orthogonal neighbours in pairs. So both paths update `_blocked`, add or remove the points, and then call one shared `_refresh_connections_around()` that re-derives every edge in the 3x3 around each changed cell from `_can_traverse()`. Neither path carries bespoke edge logic, so neither can disagree with the other, and the `HALF_NEIGHBOURS` asymmetry the issue warns about never arises — `HALF_NEIGHBOURS` stays confined to the full sweep in `_build_navigation()` where it is correct. ## Testing **Unit — 32 new tests, written before the implementation.** Full suite: 374 passing, 0 failing, 1585 asserts. `test/unit/test_terrain_navigation.gd` (23) covers the issue's list plus four cases it did not name: - corner-cutting, closed on block and reopened on unblock — the tests that catch the correction above - `unblock_cells()` must re-ask the terrain, so a water cell never gains a point - idempotent blocking, and out-of-bounds cells inert - a wall across the map leaves `_main_region` as the *larger half*, not "landmass minus wall" — the case a `_main_region` only recomputed in `generate()` gets wrong `test/unit/test_buildings_block_navigation.gd` (9) covers the registry seam: the L-shape's hole stays walkable (a bounding-box implementation passes every 1x1 test and fails this one), rotation is derived from the same `cells()` call, removal hands cells back, and removing one of two adjacent buildings leaves the survivor blocking. **Headed — `tools/screenshot_building_blocks_path.gd`, 12 checks, all passing.** `gl_compatibility` draws nothing headless, so this runs headed against the real `game.tscn` and saves four PNGs to `tmp/`. It wires nothing by hand, which is what makes the `game.gd` listener genuinely covered — unhook it and CHECK 1 fails. From the run: main region 256 -> 247 on a 9-cell footprint (exact; no severed region), detour 18 waypoints against 16 direct, and back to 16 after removal — so `unblock_cells()` leaves no invisible wall where a building stood. The screenshots show a pawn walking, the rocket dropped on its trail, the trail visibly doglegging around the rocket once the move is re-issued, and the ground clear again after removal. ## Not covered - **Stale in-flight paths are still stale** — a pawn already walking strolls through a building placed in front of it. That is #83, and the second screenshot shows it plainly. The probe prints it as `EXPECTED` so it does not read as a defect of this change. - **The `game.gd` connection is not covered by the unit suite.** `test_buildings_block_navigation.gd` mirrors the listener rather than invoking it, because `game.gd` needs the whole scene and cannot be built headless — delete the connection and those 9 tests still pass. The headed probe is what guards it. - **"Blocking then unblocking restores an identical path" is asserted as identical cost, not identical waypoints.** The connections themselves are pinned exactly by `test_unblocking_restores_exactly_the_connections_it_had`; `AStar2D` breaks ties between two equally short routes on internal ordering, so waypoint-for-waypoint equality would assert an implementation detail of the engine rather than of this issue. - **Entrances are still #55.** This change only leaves room for them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KDzLtT2uajq2ozyFVydZeG

liamjd added 2 commits

2026-09-14 11:11:31 +01:00

A bit more help from Claude than I had hoped for, but we can now block and unblock cells from the astar navigation graph, triggered when a building is placed on the map. Unit tests all pass. Visual test passes. fe85caafe5

Commit the #56 headed probe, and restore pawn.gd.uid 5e6a3c4422

screenshot_building_blocks_path.gd is the only thing covering the
game.gd building_placed listener: the unit tests mirror that wiring
rather than invoking it, so deleting the connection leaves the suite
green. Probes are kept rather than thrown away - they cost nothing and
record how each visual behaviour was verified.

pawn.gd.uid was dropped by an earlier merge tidy while pawn.gd remained.
Nothing referenced the id, but the next editor scan would have
regenerated it as a different one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KDzLtT2uajq2ozyFVydZeG

liamjd merged commit 98433c2a65 into main

2026-09-14 11:16:32 +01:00

liamjd referenced this pull request from a commit

2026-09-14 11:16:33 +01:00

Merge pull request 'Placed buildings block pathfinding' (#98) from 56-buildings-block-pathfinding into main