Render PLANNED and UNDER_CONSTRUCTION buildings distinctly from BUILT #71

New issue

Closed

opened 2026-09-07 20:37:21 +01:00 by liamjd · 0 comments

liamjd commented

2026-09-07 20:37:21 +01:00

Owner

Part of milestone #5. Depends on #70. See also #119.

A placed-but-unbuilt building needs to look like a plan, not a finished structure, or the player cannot tell what the pawns have done.

AC:

PLANNED: translucent or wireframe version of the building mesh (can reuse the ghost's material approach; note Buildings._spawn_view() deliberately avoids duplicating the ghost, so give the view its own material rather than sharing one)
BLOCKED: if the construction task cannot be started, a translucent red version.
UNDER_CONSTRUCTION: a visible difference from PLANNED (e.g. opacity scaling with progress, or a scaffold colour)
BUILT: current rendering, unchanged
Buildings updates the view when the record's status changes, without rebuilding the mesh
Verified headed with a screenshot of all three states side by side, attached to the PR; the headless suite cannot see this

Implementation outline

Four appearances, but only three of them are a Building.Status. BLOCKED is a
Task.Status, not a building one — and it must not become one:
ConstructionTask._can_start() gates on status in [PLANNED, UNDER_CONSTRUCTION], so a
building parked in a hypothetical Building.Status.BLOCKED would be permanently
unstartable and could never unblock itself. The view therefore needs its own state enum,
derived from the record plus one transient flag.

New: `entities/buildings/building_view.gd`

class_name BuildingView extends RefCounted, all static, no node state. Pure functions so
the decision table is testable headless — which matters more here than usual, because
gl_compatibility renders nothing headless and the suite can never see the result.

enum State {PLANNED, BLOCKED, UNDER_CONSTRUCTION, BUILT}

static func state_for(building: Building) -> State
static func material_for(state: State, base_color: Color, progress: float) -> StandardMaterial3D
static func apply(view: MeshInstance3D, building: Building) -> void

state_for() maps Building.Status straight through, except that
construction_blocked overrides PLANNED/UNDER_CONSTRUCTION with BLOCKED. RUINED falls
through to BUILT for now — out of scope, flagged with a TODO.
material_for() returns a new StandardMaterial3D every call. Never a shared static
one: BuildingMeshes.build() already hands out one shared Mesh per
[type.id, tile_size, level_height], so a shared override would repaint every building
of that type at once. That is the trap _spawn_view()'s comment already warns about,
one level up.
Tint is base_color.lerp(STATE_TINT, 0.5) carrying the state's alpha, not the flat state
colour. A flat tint makes every planned building an identical blue blob; blending keeps
the type's placeholder_color legible while still reading as a plan.
cast_shadow off for every state except BUILT. A plan should not throw a shadow.
BUILT sets material_override = null and shadows back on, so the baked placeholder
material shows through untouched — that is the "unchanged" AC, and it is why the
transition has to be reversible rather than one-way.

`Building` gains two fields

var construction_blocked: bool = false  # transient; derived from the task, not saved
var build_progress: float = 0.0         # 0..1, drives the UNDER_CONSTRUCTION fade

Both are written only through Buildings, so there is one place that refreshes the view.
construction_blocked is explicitly not part of the save format: it is a cache of a
task's status, and a reload recomputes it the moment a pawn next asks for work.

`Buildings` — refresh, don't rebuild

_spawn_view() ends with BuildingView.apply(view, building) rather than leaving the
mesh bare, so a building spawns already looking like a plan instead of flicking from
BUILT on the next status change. That also covers the load path: a PLANNED building
restored from a save draws correctly without a transition ever firing.
set_status() already exists, already no-ops on an unchanged status, and already emits
building_status_changed. Add _refresh_view(id) to it. No new mesh — apply()
only touches material_override and cast_shadow.
New set_construction_blocked(id: int, blocked: bool) -> void and
set_build_progress(id: int, progress: float) -> void; both no-op on an unchanged
value, both call _refresh_view(id).
New view_for(id: int) -> MeshInstance3D, null when there is none. Public because the
alternative is tests reaching into _views or guessing child order, and a debug overlay
will want the same handle.

Where BLOCKED comes from

TaskManager.next_for() is the only place a task is marked BLOCKED, and it does it
generically — it knows nothing about buildings. Two small additions:

TaskManager gains signal task_blocked(task: Task) and
signal task_unblocked(task: Task), emitted from next_for() only when the status
actually crosses into or out of BLOCKED. Capture the old status before the
transition_to() call; a task re-evaluated as still blocked for the tenth time is not
an event.
Buildings.tasks becomes a property whose setter connects those two signals
(disconnecting any previous manager first). The handler checks task is ConstructionTask
and calls set_construction_blocked(task.building_id, …). Wiring it in the setter rather
than in game.gd means the tests get it for free — they already assign
registry.tasks = manager — and there is no second place to forget it.

Known limitation, worth stating rather than rediscovering: next_for() only runs when
a pawn asks for work, so a blocked building stays plain until some pawn next goes looking,
and if every pawn is busy that can lag arbitrarily. This is pre-existing — Task.status
has exactly the same lag, and the debug panel already shows it — and is not made worse
here. Fixing it means a re-evaluation sweep on a timer, which is a separate issue if it
ever bites.

Where progress comes from

ConstructionTask._work() already decrements work_remaining and already flips PLANNED to
UNDER_CONSTRUCTION. After the decrement it computes 1.0 - work_remaining / float(total)
from building.type.build_work_ticks() and calls ctx.buildings.set_build_progress().
Guard total <= 0: build_days of 0.0 means the type is not pawn-raised at all, and the
division would be by zero.

Touched files

File	Change
`entities/buildings/building_view.gd`	new; the whole decision table
`entities/buildings/building.gd`	`construction_blocked`, `build_progress`
`entities/buildings/buildings.gd`	`_refresh_view()`, `view_for()`, two setters, `tasks` setter, `_spawn_view()` tail
`entities/tasks/task_manager.gd`	`task_blocked` / `task_unblocked` signals
`entities/tasks/construction_task.gd`	report progress from `_work()`
`test/unit/test_building_view.gd`	new; pure state and material tests
`test/unit/test_buildings.gd`	new section: view refresh through the registry
`test/unit/test_construction_task.gd`	new section: progress reporting
`tools/screenshot_building_states.gd`	new; the headed four-up verification

Adding a class_name needs --headless --editor --quit before the suite will parse.

Decisions taken — worth overruling if you disagree

Progress-scaled opacity over a flat scaffold colour. The AC allows either. Progress
reads better and the data is nearly free, but it costs the build_progress field and a
write per worked tick. A flat colour drops both, and is a one-line change to
material_for() if the fade turns out to be invisible at these sizes.
BLOCKED is in scope here; the reason is not. #119 wants unreachable to look
different from waiting-on-materials, and names this issue as the hook. This builds the
hook with a bool; #119 widens it to the reason enum and teaches state_for() that a
waiting-on-children task must not read as blocked. Bool first means #119 changes one
function rather than introducing the path.
A separate BuildingView class rather than methods on Buildings. The suite cannot
see pixels, so the decision table is the only thing it can pin. Pure static functions
make that table testable without a scene, a map, or a mesh.

Out of scope

A wireframe or hologram shader. Translucency gets there; a real shader is a
look-and-feel pass, not this.
RUINED's own appearance — nothing sets that status yet.
Showing why a building is blocked (#119), and any build-progress readout in the UI.

Part of milestone #5. Depends on #70. See also #119. A placed-but-unbuilt building needs to look like a plan, not a finished structure, or the player cannot tell what the pawns have done. AC: - PLANNED: translucent or wireframe version of the building mesh (can reuse the ghost's material approach; note `Buildings._spawn_view()` deliberately avoids duplicating the ghost, so give the view its own material rather than sharing one) - BLOCKED: if the construction task cannot be started, a translucent red version. - UNDER_CONSTRUCTION: a visible difference from PLANNED (e.g. opacity scaling with progress, or a scaffold colour) - BUILT: current rendering, unchanged - `Buildings` updates the view when the record's status changes, without rebuilding the mesh - Verified headed with a screenshot of all three states side by side, attached to the PR; the headless suite cannot see this --- ## Implementation outline Four appearances, but only three of them are a `Building.Status`. **BLOCKED is a `Task.Status`, not a building one** — and it must not become one: `ConstructionTask._can_start()` gates on `status in [PLANNED, UNDER_CONSTRUCTION]`, so a building parked in a hypothetical `Building.Status.BLOCKED` would be permanently unstartable and could never unblock itself. The view therefore needs its own state enum, derived from the record plus one transient flag. ### New: `entities/buildings/building_view.gd` `class_name BuildingView extends RefCounted`, all static, no node state. Pure functions so the decision table is testable headless — which matters more here than usual, because `gl_compatibility` renders nothing headless and the suite can never see the result. ```gdscript enum State {PLANNED, BLOCKED, UNDER_CONSTRUCTION, BUILT} static func state_for(building: Building) -> State static func material_for(state: State, base_color: Color, progress: float) -> StandardMaterial3D static func apply(view: MeshInstance3D, building: Building) -> void ``` - `state_for()` maps `Building.Status` straight through, except that `construction_blocked` overrides PLANNED/UNDER_CONSTRUCTION with BLOCKED. RUINED falls through to BUILT for now — out of scope, flagged with a TODO. - `material_for()` returns a **new** `StandardMaterial3D` every call. Never a shared static one: `BuildingMeshes.build()` already hands out one shared `Mesh` per `[type.id, tile_size, level_height]`, so a shared override would repaint every building of that type at once. That is the trap `_spawn_view()`'s comment already warns about, one level up. - Tint is `base_color.lerp(STATE_TINT, 0.5)` carrying the state's alpha, not the flat state colour. A flat tint makes every planned building an identical blue blob; blending keeps the type's `placeholder_color` legible while still reading as a plan. - `cast_shadow` off for every state except BUILT. A plan should not throw a shadow. - BUILT sets `material_override = null` and shadows back on, so the baked placeholder material shows through untouched — that is the "unchanged" AC, and it is why the transition has to be reversible rather than one-way. ### `Building` gains two fields ```gdscript var construction_blocked: bool = false # transient; derived from the task, not saved var build_progress: float = 0.0 # 0..1, drives the UNDER_CONSTRUCTION fade ``` Both are written only through `Buildings`, so there is one place that refreshes the view. `construction_blocked` is explicitly **not** part of the save format: it is a cache of a task's status, and a reload recomputes it the moment a pawn next asks for work. ### `Buildings` — refresh, don't rebuild - `_spawn_view()` ends with `BuildingView.apply(view, building)` rather than leaving the mesh bare, so a building spawns already looking like a plan instead of flicking from BUILT on the next status change. That also covers the load path: a PLANNED building restored from a save draws correctly without a transition ever firing. - `set_status()` already exists, already no-ops on an unchanged status, and already emits `building_status_changed`. Add `_refresh_view(id)` to it. **No new mesh** — `apply()` only touches `material_override` and `cast_shadow`. - New `set_construction_blocked(id: int, blocked: bool) -> void` and `set_build_progress(id: int, progress: float) -> void`; both no-op on an unchanged value, both call `_refresh_view(id)`. - New `view_for(id: int) -> MeshInstance3D`, null when there is none. Public because the alternative is tests reaching into `_views` or guessing child order, and a debug overlay will want the same handle. ### Where BLOCKED comes from `TaskManager.next_for()` is the only place a task is marked BLOCKED, and it does it generically — it knows nothing about buildings. Two small additions: - `TaskManager` gains `signal task_blocked(task: Task)` and `signal task_unblocked(task: Task)`, emitted from `next_for()` only when the status actually crosses into or out of BLOCKED. Capture the old status before the `transition_to()` call; a task re-evaluated as still blocked for the tenth time is not an event. - `Buildings.tasks` becomes a property whose setter connects those two signals (disconnecting any previous manager first). The handler checks `task is ConstructionTask` and calls `set_construction_blocked(task.building_id, …)`. Wiring it in the setter rather than in `game.gd` means the tests get it for free — they already assign `registry.tasks = manager` — and there is no second place to forget it. **Known limitation, worth stating rather than rediscovering:** `next_for()` only runs when a pawn asks for work, so a blocked building stays plain until some pawn next goes looking, and if every pawn is busy that can lag arbitrarily. This is pre-existing — `Task.status` has exactly the same lag, and the debug panel already shows it — and is not made worse here. Fixing it means a re-evaluation sweep on a timer, which is a separate issue if it ever bites. ### Where progress comes from `ConstructionTask._work()` already decrements `work_remaining` and already flips PLANNED to UNDER_CONSTRUCTION. After the decrement it computes `1.0 - work_remaining / float(total)` from `building.type.build_work_ticks()` and calls `ctx.buildings.set_build_progress()`. Guard `total <= 0`: `build_days` of 0.0 means the type is not pawn-raised at all, and the division would be by zero. ### Touched files | File | Change | | --- | --- | | `entities/buildings/building_view.gd` | new; the whole decision table | | `entities/buildings/building.gd` | `construction_blocked`, `build_progress` | | `entities/buildings/buildings.gd` | `_refresh_view()`, `view_for()`, two setters, `tasks` setter, `_spawn_view()` tail | | `entities/tasks/task_manager.gd` | `task_blocked` / `task_unblocked` signals | | `entities/tasks/construction_task.gd` | report progress from `_work()` | | `test/unit/test_building_view.gd` | new; pure state and material tests | | `test/unit/test_buildings.gd` | new section: view refresh through the registry | | `test/unit/test_construction_task.gd` | new section: progress reporting | | `tools/screenshot_building_states.gd` | new; the headed four-up verification | Adding a `class_name` needs `--headless --editor --quit` before the suite will parse. ## Decisions taken — worth overruling if you disagree 1. **Progress-scaled opacity over a flat scaffold colour.** The AC allows either. Progress reads better and the data is nearly free, but it costs the `build_progress` field and a write per worked tick. A flat colour drops both, and is a one-line change to `material_for()` if the fade turns out to be invisible at these sizes. 2. **BLOCKED is in scope here; the *reason* is not.** #119 wants unreachable to look different from waiting-on-materials, and names this issue as the hook. This builds the hook with a bool; #119 widens it to the reason enum and teaches `state_for()` that a waiting-on-children task must **not** read as blocked. Bool first means #119 changes one function rather than introducing the path. 3. **A separate `BuildingView` class rather than methods on `Buildings`.** The suite cannot see pixels, so the decision table is the only thing it can pin. Pure static functions make that table testable without a scene, a map, or a mesh. ## Out of scope - A wireframe or hologram shader. Translucency gets there; a real shader is a look-and-feel pass, not this. - RUINED's own appearance — nothing sets that status yet. - Showing *why* a building is blocked (#119), and any build-progress readout in the UI.

liamjd added this to the Task management for pawns milestone

2026-09-07 20:37:21 +01:00

liamjd added the

label

2026-09-07 20:37:21 +01:00

liamjd referenced this issue from a commit

2026-09-18 13:11:30 +01:00

ConstructionTask: placing a building enqueues construction

liamjd referenced this issue

2026-09-18 13:11:51 +01:00

ConstructionTask: placing a building enqueues construction #109

liamjd referenced this issue