Demolish mode and DeconstructionTask (#72) #122

Merged

liamjd merged 7 commits from 72-demolish-mode-and-deconstruction-task into main 2026-09-22 20:57:14 +01:00

Conversation 0 Commits 7 Files changed 27 +1806 -38

liamjd commented 2026-09-22 20:49:43 +01:00

Owner

Copy link

Closes #72

Gives Buildings.remove() its caller. You mark a building in a new demolish mode. Depending on its status, the mark either withdraws the building on the spot or queues a DeconstructionTask for a pawn to go and do.

What marking does, by status

Status	Effect
BUILT / RUINED	Flags the record (marked_for_demolition) and queues a DeconstructionTask at the entrance cell. When a pawn completes it, remove() runs and building_removed is emitted
PLANNED	Cancels the construction task. The building is removed immediately and no deconstruction task is created
UNDER_CONSTRUCTION	Same as PLANNED. See the decision below

Clicking a marked building again unmarks it. That cancels the deconstruction task and restores the building's normal appearance.

Decisions

UNDER_CONSTRUCTION cancels immediately (asked for in the AC). Marking a half-built site cancels its construction task and removes it on the spot. The labour already spent on it is lost. There is nothing to recover yet, because materials arrive with #73. Until a building consumes materials, a part-built site holds only spent pawn-ticks, and a deconstruction task would spend more labour reclaiming nothing. Queueing one would also create a fourth building state, under construction and marked, which the task gate, the renderer and the work loop would each have to learn. Revisit when #73 lands. Delivered materials are worth reclaiming, and at that point a started site belongs on the BUILT branch. The tests for this branch are grouped under one heading so that change stays in one place.

The mark is a flag, not a Building.Status. A marked building is still BUILT: it stands, it blocks navigation, and it counts against its type's build_limit. Like construction_blocked, setting the flag redraws the view and emits no signal.

Demolition costs work. BuildingType.deconstruct_work_ticks() derives it from build_days, with a floor, so that a build_days = 0.0 building is not demolished the moment a pawn arrives.

A building carries at most one live task at a time. That is what lets the registry find a building's task by id without first deciding which kind it wanted.

Also in this PR

• GameState.interaction_mode replaces the per-mode booleans: InteractionMode {NONE, PLACE, DEMOLISH, SELECT, TERRAIN}. With an enum, two modes can't both be on at once, and controllers read the value rather than caching their own copy. building_placement_canceled is gone, because it always fired alongside the mode change. BuildingPanel now follows the mode instead of keeping its own copy, so ending demolish mode with a right-click no longer leaves the button lit.

• Buildings are picked by their own collider, not by the tile under them. Views had no collision shape, so a click raycast the terrain and asked which building stood there. Under the pitched camera, the visible body of a building is drawn offset from its tile, and the offset grows with height:

  type	height	aiming at the body landed on
  colony_rocket	14 levels	3 cells away: hit nothing
  sensor_tower	8 levels	2 cells away: hit nothing
  ore_mine	3 levels	the right cell, but only near the centre

  This is why a building could be marked but almost never unmarked. Each view now gets a StaticBody3D sized from its mesh AABB, on physics layer 2 (named buildings). Buildings.pick(camera, screen_position) raycasts against that layer only. PlacementController still picks the terrain cell on purpose, because that is the right answer for placement.

• place() now sets build_progress = 1.0 when a building lands BUILT. Before this, a Colony Centre reported itself as 0% built.

A Godot trap worth knowing

Pressing Demolish while a building card is selected makes ButtonGroup emit the outgoing button's toggled(false) before the incoming toggled(true), so the mode is briefly NONE in between. If the panel writes to its buttons at that moment, it cancels the press Godot is in the middle of performing. You end up with no button pressed and no mode set. _applying_own_toggle keeps the panel's hands off the group during its own transitions, and test_interaction_mode.gd pins both orderings.

Verification

• Suite: 663 tests, 662 passing, 1 pending (#85's placeholder).
• New tests:
  • test_deconstruction_task.gd: the three status branches, unmarking, limits, the standalone registry
  • test_interaction_mode.gd: the mode property, plus the panel's arbitration, run against the real scene
  • test_building_picking.gd: every catalogue type, aimed at its own mesh centre
  • a MARKED section in test_building_view.gd
• Headed: tools/probe_demolish_click.gd puts the real cursor on a colony rocket's body and clicks twice. The building was marked, then unmarked. Screenshots taken before and after showed the orange tint with the shadow dropped, then the building back to its own colour with the shadow restored. The probe exists because warp_mouse() does nothing headless, so the unit tests can't exercise the real hover path.
• End to end through the work loop (a one-off check in game.tscn, not committed): a pawn walked to the entrance, worked the 50 ticks, and the building was removed. Its cells were freed and the pawn went back to idle.

Known gaps, not addressed here

• DeconstructionTask._work() writes demolition progress into build_progress. If you unmark a building partway through, it is left BUILT with a partial figure. This is invisible, because BUILT takes no override, but the field is wrong.
• DeconstructionTask.on_cancel() is pass. Only unmark_for_demolition clears the flag, and it is currently the only caller that cancels a deconstruction task. Any other cancel route, such as #85 or a debug panel, would leave a building marked with no task behind it.
• The Colony Centre can be demolished. colony_rocket takes the BUILT branch like any other building. This is a design decision, not a bug, and should be settled separately.
• Demolish mode has no hover highlight, so you only find out what a click will hit after you click.
• tools/probe_menu_clickthrough.gd now arms placement through the mode. Its CONTROL step already failed headless before this branch (checked against e5093c5), so that probe gives no valid result headless for unrelated reasons.

🤖 Generated with Claude Code

Closes #72 Gives `Buildings.remove()` its caller. You mark a building in a new demolish mode. Depending on its status, the mark either withdraws the building on the spot or queues a `DeconstructionTask` for a pawn to go and do. ## What marking does, by status | Status | Effect | |---|---| | **BUILT** / RUINED | Flags the record (`marked_for_demolition`) and queues a `DeconstructionTask` at the entrance cell. When a pawn completes it, `remove()` runs and `building_removed` is emitted | | **PLANNED** | Cancels the construction task. The building is removed immediately and no deconstruction task is created | | **UNDER_CONSTRUCTION** | Same as PLANNED. See the decision below | Clicking a marked building again unmarks it. That cancels the deconstruction task and restores the building's normal appearance. ## Decisions **UNDER_CONSTRUCTION cancels immediately (asked for in the AC).** Marking a half-built site cancels its construction task and removes it on the spot. The labour already spent on it is lost. There is nothing to recover yet, because materials arrive with #73. Until a building consumes materials, a part-built site holds only spent pawn-ticks, and a deconstruction task would spend *more* labour reclaiming nothing. Queueing one would also create a fourth building state, under construction *and* marked, which the task gate, the renderer and the work loop would each have to learn. **Revisit when #73 lands.** Delivered materials are worth reclaiming, and at that point a started site belongs on the BUILT branch. The tests for this branch are grouped under one heading so that change stays in one place. **The mark is a flag, not a `Building.Status`.** A marked building is still BUILT: it stands, it blocks navigation, and it counts against its type's `build_limit`. Like `construction_blocked`, setting the flag redraws the view and emits no signal. **Demolition costs work.** `BuildingType.deconstruct_work_ticks()` derives it from `build_days`, with a floor, so that a `build_days = 0.0` building is not demolished the moment a pawn arrives. **A building carries at most one live task at a time.** That is what lets the registry find a building's task by id without first deciding which kind it wanted. ## Also in this PR - **`GameState.interaction_mode`** replaces the per-mode booleans: `InteractionMode {NONE, PLACE, DEMOLISH, SELECT, TERRAIN}`. With an enum, two modes can't both be on at once, and controllers read the value rather than caching their own copy. `building_placement_canceled` is gone, because it always fired alongside the mode change. `BuildingPanel` now follows the mode instead of keeping its own copy, so ending demolish mode with a right-click no longer leaves the button lit. - **Buildings are picked by their own collider, not by the tile under them.** Views had no collision shape, so a click raycast the terrain and asked which building stood there. Under the pitched camera, the visible body of a building is drawn offset from its tile, and the offset grows with height: | type | height | aiming at the body landed on | |---|---|---| | `colony_rocket` | 14 levels | 3 cells away: hit nothing | | `sensor_tower` | 8 levels | 2 cells away: hit nothing | | `ore_mine` | 3 levels | the right cell, but only near the centre | This is why a building could be marked but almost never unmarked. Each view now gets a `StaticBody3D` sized from its mesh AABB, on physics layer 2 (named `buildings`). `Buildings.pick(camera, screen_position)` raycasts against that layer only. `PlacementController` still picks the terrain cell on purpose, because that is the right answer for placement. - `place()` now sets `build_progress = 1.0` when a building lands BUILT. Before this, a Colony Centre reported itself as 0% built. ## A Godot trap worth knowing Pressing Demolish while a building card is selected makes ButtonGroup emit the **outgoing** button's `toggled(false)` *before* the incoming `toggled(true)`, so the mode is briefly NONE in between. If the panel writes to its buttons at that moment, it cancels the press Godot is in the middle of performing. You end up with no button pressed and no mode set. `_applying_own_toggle` keeps the panel's hands off the group during its own transitions, and `test_interaction_mode.gd` pins both orderings. ## Verification - **Suite: 663 tests, 662 passing, 1 pending** (#85's placeholder). - New tests: - `test_deconstruction_task.gd`: the three status branches, unmarking, limits, the standalone registry - `test_interaction_mode.gd`: the mode property, plus the panel's arbitration, run against the real scene - `test_building_picking.gd`: every catalogue type, aimed at its own mesh centre - a `MARKED` section in `test_building_view.gd` - **Headed:** `tools/probe_demolish_click.gd` puts the real cursor on a colony rocket's body and clicks twice. The building was marked, then unmarked. Screenshots taken before and after showed the orange tint with the shadow dropped, then the building back to its own colour with the shadow restored. The probe exists because `warp_mouse()` does nothing headless, so the unit tests can't exercise the real hover path. - **End to end through the work loop** (a one-off check in `game.tscn`, not committed): a pawn walked to the entrance, worked the 50 ticks, and the building was removed. Its cells were freed and the pawn went back to idle. ## Known gaps, not addressed here - `DeconstructionTask._work()` writes demolition progress into `build_progress`. If you unmark a building partway through, it is left BUILT with a partial figure. This is invisible, because BUILT takes no override, but the field is wrong. - `DeconstructionTask.on_cancel()` is `pass`. Only `unmark_for_demolition` clears the flag, and it is currently the only caller that cancels a deconstruction task. Any other cancel route, such as #85 or a debug panel, would leave a building marked with no task behind it. - **The Colony Centre can be demolished.** `colony_rocket` takes the BUILT branch like any other building. This is a design decision, not a bug, and should be settled separately. - **Demolish mode has no hover highlight**, so you only find out what a click will hit after you click. - `tools/probe_menu_clickthrough.gd` now arms placement through the mode. Its CONTROL step already failed headless before this branch (checked against `e5093c5`), so that probe gives no valid result headless for unrelated reasons. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

liamjd added 7 commits 2026-09-22 20:49:43 +01:00

Spec the demolish mode and DeconstructionTask (#72) ... 091b8d1329

Tests first, as a spec to code against. Both scripts fail to parse until
DeconstructionTask and BuildingView.State.MARKED exist, the same way #70's
and #71's specs did.

Decisions recorded in the test headers and on the issue:

- Marking a building UNDER_CONSTRUCTION cancels it immediately, as PLANNED
  does. There are no materials to reclaim until #73, so a deconstruction
  task would spend more labour recovering nothing - and it would create a
  fourth building state the task gate, the renderer and the work loop would
  each have to learn.
- marked_for_demolition is a flag alongside construction_blocked, not a
  fifth Building.Status. A marked building is still BUILT: it stands, it
  blocks navigation and it counts against build_limit.
- Demolition costs work, derived from build_days with a floor of one tick
  so a build_days=0.0 building is not demolishable on arrival.
- A building carries at most one live task. That is what lets the registry
  find a building's task by id without first deciding which kind it wanted.

Also specced: place() should set build_progress to 1.0 on the build_days=0.0
branch. It leaves it at 0.0 today, so a Colony Centre claims to be 0% built
for its whole life - harmless until #72 renders a BUILT building from its own
fields.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Implemented all the unit test specifications for deconstruction_task.gd. building_view test work still to be done. 1b710251b7

The transparency of a building marked for demolition is unrelated to build progress. ... e5093c549e

All unit tests complete. Still no way to trigger a demolition.

Replace the per-mode booleans with GameState.interaction_mode (#72) ... fe9b03dbd5

One enum, one property, one signal. Mutual exclusion comes from the type
rather than from every controller remembering to clear the others' flags,
and controllers read GameState.interaction_mode directly instead of caching
a copy that can survive after the mode has ended elsewhere.

- GameState: InteractionMode {NONE, PLACE, DEMOLISH, SELECT, TERRAIN} and
  the interaction_mode property. SELECT and TERRAIN have no controller yet;
  they are listed so adding one is a case rather than a new flag.
- building_placement_canceled is gone. It and the mode change always fired
  together, and the panel now follows the mode instead.
- BuildingPanel drives the mode from its buttons and follows it back, so a
  mode ended by right-click no longer leaves the button lit.
- PlacementController and DemolishController both gate _unhandled_input on
  the mode. DemolishController is later in game.tscn and unhandled input is
  delivered bottom-up, so it took first refusal on every click: with stale
  hover state and no mode check, one click could both mark a building for
  demolition and swallow the placement it was meant to be.
- DemolishController._clear_hover() actually clears. Empty, it let
  target_building hold the last building hovered indefinitely.
- Its right-click cancel waits for the release and measures the travel, and
  does not mark the event handled - the right button also rotates the
  camera, which is why PlacementController does the same dance.

The ButtonGroup trap this turned up, now pinned by tests: pressing Demolish
over a selected card emits the outgoing button's toggled(false) BEFORE the
incoming toggled(true), so the mode is briefly NONE between them. Writing to
the buttons at that moment cancels the press Godot is performing, which left
the panel with no button pressed and the mode at NONE. _applying_own_toggle
keeps the panel's hands off the group while the group is mid-transition.

tools/probe_menu_clickthrough.gd arms placement through the mode as well as
the type, or it would report a blocked click it never actually attempted.
Its CONTROL step already failed before this change - verified against
e5093c5 - so the probe remains void headless for unrelated reasons.

Also carries Liam's uncommitted work in progress, swept in by this commit
rather than landed separately: the DemolishController node in game.tscn, the
demolish button in building_panel.tscn, and the edits to building.gd,
buildings.gd, BuildingTypeCard.tscn and placement_rules.gd.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Pick buildings by their own collider, not by the tile under them (#72) ... 0f8ef9079a

Building views had no collision shape, so DemolishController raycast the
terrain and asked which building stood on the cell that was hit. Under a
pitched camera the visible body of a building is drawn offset from the tile
it stands on, and the offset grows with height. Aiming at the middle of what
the player sees landed here, measured against the shipped catalogue:

    colony_rocket  (14 levels)  ->  3 cells away, hit nothing
    sensor_tower   ( 8 levels)  ->  2 cells away, hit nothing
    ore_mine       ( 3 levels)  ->  the right cell
    warehouse      ( 2 levels)  ->  1 cell away, saved by a 2-cell footprint

So the clickable region was the footprint on the ground near the base, while
the thing the player aims at is the body. Marking a tall building was luck;
unmarking one - where you aim at the lit-up body you just created - was close
to impossible. That is the "I can mark it but I can never cancel it" report,
and it was never a timing problem.

- Buildings._spawn_view() gives each view a StaticBody3D + BoxShape3D sized
  from the mesh AABB, on its own collision layer, carrying the building id.
  It is parented to the view, so it rotates with it and is freed with it.
- Buildings.pick(camera, screen_position) raycasts masked to that layer and
  returns the Building, or null. The layer and the meta key stay private to
  the registry; #78's shared picker can call this and keep its own question
  about pawns.
- DemolishController uses it and no longer needs the map at all.
- project.godot names layer 2 "buildings".

PlacementController deliberately keeps picking the terrain cell: for placing
a building, "the cell under the cursor" is the right answer.

Verified headed as well as by the suite - the unit tests compute a screen
point, but warp_mouse() is a no-op headless so they cannot exercise the real
hover path. tools/probe_demolish_click.gd puts the cursor on a 14-level
colony rocket's body, clicks twice, and reports marked then unmarked, with a
screenshot either side.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Free cells and view when a plan is withdrawn with no task manager (#72) ... d237238612

mark_for_demolition()'s no-manager PLANNED branch called _buildings.erase(id),
which dropped the record and nothing else: the cells stayed in _occupancy and
the mesh stayed on screen, so the building vanished from the registry while
still blocking placement and navigation. It now goes through remove(), the
same path every other removal takes, and returns rather than falling through
to a cancel with no manager behind it.

Unreachable in play - game.gd always wires a manager - but "the registry works
standalone" is a promise #70 made. The existing test only asserted the record
was gone, which is why it passed; the new one asserts the cells and the view,
and was confirmed red against the old code before the fix.

Also:
- remove()'s docstring said it had no callers yet; it now names the three.
- Commit the .uid files for test_building_picking.gd and
  probe_demolish_click.gd, missed in 0f8ef90.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Doc update and added newline. cec5d13031

liamjd merged commit 2ad4d63773 into main 2026-09-22 20:57:14 +01:00

liamjd referenced this pull request from a commit 2026-09-22 20:57:15 +01:00

Merge pull request 'Demolish mode and DeconstructionTask (#72)' (#122) from 72-demolish-mode-and-deconstruction-task into main

liamjd referenced this pull request 2026-09-22 21:05:42 +01:00

Extract a BuildingTask base from ConstructionTask and DeconstructionTask #126

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

assets

Art, 3D models, UI themes, sound, music

  

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

testing

Unit and manual test activities

  

ui

User Interface

  

wontfix

This won't be fixed

No labels

assets

bug

duplicate

enhancement

help wanted

invalid

question

testing

ui

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

liamjd/UntitledColonyBuilder!122

No description provided.