Surface why a task is blocked, not just that it is #119

New issue

Closed

opened 2026-09-19 13:53:18 +01:00 by liamjd · 0 comments

liamjd commented

2026-09-19 13:53:18 +01:00

Owner

Part of milestone #5. Found while playing #75's work loop: a pawn walks to a site and raises a building, which works, but the failure case is silent.

A task nobody can reach looks exactly like a task nobody has got to yet. Both sit in the queue, no pawn moves, and nothing on screen distinguishes "the builders are busy elsewhere" from "you walled that site off and no pawn will ever go there". #75 made the second case correct: can_start() refuses a target outside the map's main region, so the task is marked BLOCKED instead of livelocking a pawn against it. But correct and silent still leaves the player watching a building forever.

Updated 2026-09-27. Since this was filed, #71 has given blocked construction its own look (Building.construction_blocked, red in BuildingView), and #57 has added toasts. ToastNotifier currently raises one for every task_blocked. So both surfaces now exist, and the problem is that neither can tell a real stall from normal operation. Once #73 lands, every building waiting on its own deliveries would turn red and raise a toast.

A reason, not just a flag

Task.status == BLOCKED says a task cannot start. It does not say why, and the UI cannot guess: the causes are different problems with different fixes.

Unreachable: the work site is cut off from the main landmass. Fix: clear a route, or cancel.
Waiting on children: e.g. #73's HaulTask children are not complete yet. Fix: nothing, it is working as intended.
Not ready: the task type's own _can_start() refused (today: its building record has gone).
Later, no qualified pawn, once #74/#69 can require a role nobody has. Not in scope here; the enum grows a value when it arrives.

The first needs shouting about; the second is normal operation and must not cry wolf.

Design

enum Task.BlockReason {NONE, UNREACHABLE, WAITING_ON_CHILDREN, NOT_READY} and a Task.block_reason field, defaulting to NONE.
Working out the reason is a pure query: Task.evaluate_block(ctx) -> BlockReason, which returns NONE when the task could start. can_start(ctx) keeps its signature and becomes evaluate_block(ctx) == BlockReason.NONE, so TaskManager.complete()'s parent re-check and every existing caller are unchanged.
Only TaskManager.next_for() writes the field, because that is where the block is decided. It sets the reason when it marks a task BLOCKED and resets it to NONE when the task is offered again.
Precedence when several causes hold at once: UNREACHABLE > WAITING_ON_CHILDREN > NOT_READY. Unreachable comes first because it is the one that needs the player, and it will still be true after the children finish.
A change of reason while still BLOCKED re-emits task_blocked. Today it only fires on the transition into BLOCKED, so a task waiting on children whose site is then walled off would stay silent. It does not re-emit when the reason is unchanged, so listeners are not told the same thing every tick.
Surfacing is an allow-list, and only UNREACHABLE is on it. This matches ToastNotifier.for_status_change(), which lists the cases that qualify rather than ruling out the ones that do not.
- Buildings._task_blocked() sets construction_blocked to task.block_reason == UNREACHABLE, not unconditionally true. A reason change can therefore clear the flag as well as set it.
- ToastNotifier.for_blocked() returns null for anything but UNREACHABLE.
- NOT_READY becomes quiet as a side effect. Today it only arises when a task outlives its building, which is not something to tell the player.

Acceptance criteria

Task.BlockReason and Task.block_reason as above: an enum, not a string.
evaluate_block() returns each reason, with the precedence above. can_start() is defined in terms of it.
next_for() sets the reason with the block and clears it when the task is offered again.
task_blocked re-emits on a change of reason while blocked, and not otherwise.
An unreachable construction still marks its building. One that is only waiting on children does not.
Only an unreachable block raises a toast.
The debug panel's task rows show the reason (checked by eye; DebugLayer._task_text()).
Specs: test/unit/test_task_block_reason.gd, plus additions to test_buildings.gd and test_toast_notifier.gd.

Depends on #71 (merged) for the rendering hook and #57 (merged) for the toast. Related to #73, which introduces the first real children, and #124, whose item 3 (task_unblocked emitted for tasks that were never blocked) touches the same branch of next_for().

Out of scope

Telling the player how to fix it, e.g. highlighting the cells that would restore the route.
A pawn stranded away from a reachable site: can_start() cannot see it, and Pawns._start_task() releases the task back to PENDING rather than blocking it (#75).

Part of milestone #5. Found while playing #75's work loop: a pawn walks to a site and raises a building, which works, but the failure case is silent. A task nobody can reach looks exactly like a task nobody has got to yet. Both sit in the queue, no pawn moves, and nothing on screen distinguishes "the builders are busy elsewhere" from "you walled that site off and no pawn will ever go there". #75 made the second case correct: `can_start()` refuses a target outside the map's main region, so the task is marked BLOCKED instead of livelocking a pawn against it. But correct and silent still leaves the player watching a building forever. > **Updated 2026-09-27.** Since this was filed, #71 has given blocked construction its own look (`Building.construction_blocked`, red in `BuildingView`), and #57 has added toasts. `ToastNotifier` currently raises one for **every** `task_blocked`. So both surfaces now exist, and the problem is that neither can tell a real stall from normal operation. Once #73 lands, every building waiting on its own deliveries would turn red and raise a toast. ## A reason, not just a flag `Task.status == BLOCKED` says a task cannot start. It does not say why, and the UI cannot guess: the causes are different problems with different fixes. - **Unreachable**: the work site is cut off from the main landmass. Fix: clear a route, or cancel. - **Waiting on children**: e.g. #73's HaulTask children are not complete yet. Fix: nothing, it is working as intended. - **Not ready**: the task type's own `_can_start()` refused (today: its building record has gone). - Later, **no qualified pawn**, once #74/#69 can require a role nobody has. Not in scope here; the enum grows a value when it arrives. The first needs shouting about; the second is normal operation and must not cry wolf. ## Design - `enum Task.BlockReason {NONE, UNREACHABLE, WAITING_ON_CHILDREN, NOT_READY}` and a `Task.block_reason` field, defaulting to `NONE`. - **Working out the reason is a pure query**: `Task.evaluate_block(ctx) -> BlockReason`, which returns `NONE` when the task could start. `can_start(ctx)` keeps its signature and becomes `evaluate_block(ctx) == BlockReason.NONE`, so `TaskManager.complete()`'s parent re-check and every existing caller are unchanged. - **Only `TaskManager.next_for()` writes the field**, because that is where the block is decided. It sets the reason when it marks a task BLOCKED and resets it to `NONE` when the task is offered again. - **Precedence** when several causes hold at once: `UNREACHABLE` > `WAITING_ON_CHILDREN` > `NOT_READY`. Unreachable comes first because it is the one that needs the player, and it will still be true after the children finish. - **A change of reason while still BLOCKED re-emits `task_blocked`.** Today it only fires on the transition into BLOCKED, so a task waiting on children whose site is then walled off would stay silent. It does not re-emit when the reason is unchanged, so listeners are not told the same thing every tick. - **Surfacing is an allow-list, and only `UNREACHABLE` is on it.** This matches `ToastNotifier.for_status_change()`, which lists the cases that qualify rather than ruling out the ones that do not. - `Buildings._task_blocked()` sets `construction_blocked` to `task.block_reason == UNREACHABLE`, not unconditionally `true`. A reason change can therefore clear the flag as well as set it. - `ToastNotifier.for_blocked()` returns `null` for anything but `UNREACHABLE`. - `NOT_READY` becomes quiet as a side effect. Today it only arises when a task outlives its building, which is not something to tell the player. ## Acceptance criteria - [x] `Task.BlockReason` and `Task.block_reason` as above: an enum, not a string. - [x] `evaluate_block()` returns each reason, with the precedence above. `can_start()` is defined in terms of it. - [x] `next_for()` sets the reason with the block and clears it when the task is offered again. - [ ] `task_blocked` re-emits on a change of reason while blocked, and not otherwise. - [ ] An unreachable construction still marks its building. One that is only waiting on children does not. - [x] Only an unreachable block raises a toast. - [ ] The debug panel's task rows show the reason (checked by eye; `DebugLayer._task_text()`). - [x] Specs: `test/unit/test_task_block_reason.gd`, plus additions to `test_buildings.gd` and `test_toast_notifier.gd`. Depends on #71 (merged) for the rendering hook and #57 (merged) for the toast. Related to #73, which introduces the first real children, and #124, whose item 3 (`task_unblocked` emitted for tasks that were never blocked) touches the same branch of `next_for()`. ## Out of scope - Telling the player *how* to fix it, e.g. highlighting the cells that would restore the route. - A pawn stranded away from a reachable site: `can_start()` cannot see it, and `Pawns._start_task()` releases the task back to PENDING rather than blocking it (#75).

liamjd added this to the Task management for pawns milestone

2026-09-19 13:53:18 +01:00

liamjd added the

label

2026-09-19 13:53:18 +01:00

liamjd referenced this issue

2026-09-19 14:02:41 +01:00

Pawn work loop: claim, navigate, work, complete #120

liamjd referenced this issue

2026-09-19 14:06:05 +01:00

Render PLANNED and UNDER_CONSTRUCTION buildings distinctly from BUILT #71

liamjd referenced this issue