Pawn needs to know which task it is walking to #112

New issue

Closed

opened 2026-09-18 13:39:49 +01:00 by liamjd · 1 comment

liamjd commented

2026-09-18 13:39:49 +01:00

Owner

Part of milestone #5. Depends on #70. Could be absorbed into #75 if that is where the work lands — filed separately because it is a change to the Pawn record, alongside #74.

Pawns._on_view_arrived() sets every arriving pawn to IDLING. Nothing distinguishes "arrived because it was wandering" from "arrived at the cell it was sent to in order to work", so the work loop has no way to reach the WORKING state that Pawn.PawnState already declares.

The pawn needs to remember what it is walking towards:

Pawn.current_task_id: int = -1, set when the pawn claims a task and cleared when it releases, completes or is cancelled out of one
_on_view_arrived() moves the pawn to WORKING when current_task_id names a live task whose target_cell matches the arrival cell, and to IDLING otherwise
The mismatch case matters: a pawn repathed by #56's building-blocking can arrive somewhere that is no longer the work site

This also gives #75 two of its acceptance criteria almost for free — "if the task is cancelled mid-work, the pawn stops and returns to idle" becomes a check on current_task_id, and the debug panel can show what each pawn is doing rather than just its state.

AC:

current_task_id on the Pawn record, defaulting to -1
Arrival resolves to WORKING or IDLING depending on whether the pawn arrived at its task's target cell
Releasing, completing or cancelling a task clears it
Unit tests for arrival at the target, arrival somewhere else, and cancellation while WORKING

Part of milestone #5. Depends on #70. Could be absorbed into #75 if that is where the work lands — filed separately because it is a change to the `Pawn` record, alongside #74. `Pawns._on_view_arrived()` sets every arriving pawn to `IDLING`. Nothing distinguishes "arrived because it was wandering" from "arrived at the cell it was sent to in order to work", so the work loop has no way to reach the `WORKING` state that `Pawn.PawnState` already declares. The pawn needs to remember what it is walking towards: - `Pawn.current_task_id: int = -1`, set when the pawn claims a task and cleared when it releases, completes or is cancelled out of one - `_on_view_arrived()` moves the pawn to `WORKING` when `current_task_id` names a live task whose `target_cell` matches the arrival cell, and to `IDLING` otherwise - The mismatch case matters: a pawn repathed by #56's building-blocking can arrive somewhere that is no longer the work site This also gives #75 two of its acceptance criteria almost for free — "if the task is cancelled mid-work, the pawn stops and returns to idle" becomes a check on `current_task_id`, and the debug panel can show what each pawn is doing rather than just its state. AC: - `current_task_id` on the `Pawn` record, defaulting to -1 - Arrival resolves to `WORKING` or `IDLING` depending on whether the pawn arrived at its task's target cell - Releasing, completing or cancelling a task clears it - Unit tests for arrival at the target, arrival somewhere else, and cancellation while WORKING

liamjd added this to the Task management for pawns milestone

2026-09-18 13:39:49 +01:00

liamjd referenced this issue from a commit

2026-09-19 08:13:13 +01:00

Spec the pawn knowing which task it is walking to (#112)

liamjd commented

2026-09-19 08:13:33 +01:00

Author

Owner

Spec written on branch 112-pawn-knows-its-task — test/unit/test_pawn_task_arrival.gd, 14 tests, all red for the right reason (the API does not exist). Full suite: 522 passing, the 14 new ones failing, nothing else disturbed.

Files that change

entities/pawns/pawn.gd — var current_task_id: int = -1
entities/pawns/pawns.gd — tasks injection, assign_task(), clear_task(), the arrival branch, and the signal handlers
entities/tasks/task_manager.gd — get_task() and a task_released signal
common/game.gd — one line, pawns.tasks = task_manager, beside the existing buildings.tasks = task_manager

Decisions the spec makes, for confirmation

1. Pawns gets the TaskManager, not the other way round. Mirrors Buildings.tasks: injected by game.gd, left null everywhere else. The alternative — TaskManager holding a Pawns reference so it can clear on cancel — points the dependency the wrong way, since the manager is deliberately ignorant of who is doing the work.

2. Clearing is driven by signals, not by callers remembering. cancel() and complete() already emit; release() does not, so the spec adds task_released(task) for symmetry. The tasks setter connects all three and clears any pawn holding that id. Without the third signal, the release path has to be cleared by hand from #75's work loop, which is exactly the kind of thing that gets forgotten once and then stays broken quietly.

3. get_task(id) returns null for an unknown id, with no push_error(). A cancelled task is erased from tasks, so a pawn arriving with a stale id is the normal case rather than a fault — and GUT fails a test on any push_error() raised during it, so shouting here would make the honest case untestable.

4. stop() does not clear the task. repath_pawns() is stop-then-move_to, so clearing in stop() would drop the task of every pawn a building re-routed — the #56 scenario this issue exists to handle. Two tests pin it.

5. clear_task() idles a WORKING pawn but leaves a MOVING one walking. Whether to turn a now-taskless walker around is #75's call, not this issue's; all that is owed here is that its arrival resolves to IDLING.

6. Arriving off-site keeps the task. The pawn goes IDLING but current_task_id survives, so #75 can send it again rather than having to re-claim from scratch.

Deliberately out of scope

remove() on a pawn that holds a claimed task leaves that task assigned to a dead id. The fix is a release() from the removal path, but it needs #111's decision on whether assignee stays single-valued first — filing separately unless you would rather it rode along here.

Spec written on branch `112-pawn-knows-its-task` — `test/unit/test_pawn_task_arrival.gd`, 14 tests, all red for the right reason (the API does not exist). Full suite: 522 passing, the 14 new ones failing, nothing else disturbed. ### Files that change - `entities/pawns/pawn.gd` — `var current_task_id: int = -1` - `entities/pawns/pawns.gd` — `tasks` injection, `assign_task()`, `clear_task()`, the arrival branch, and the signal handlers - `entities/tasks/task_manager.gd` — `get_task()` and a `task_released` signal - `common/game.gd` — one line, `pawns.tasks = task_manager`, beside the existing `buildings.tasks = task_manager` ### Decisions the spec makes, for confirmation **1. `Pawns` gets the `TaskManager`, not the other way round.** Mirrors `Buildings.tasks`: injected by `game.gd`, left null everywhere else. The alternative — `TaskManager` holding a `Pawns` reference so it can clear on cancel — points the dependency the wrong way, since the manager is deliberately ignorant of who is doing the work. **2. Clearing is driven by signals, not by callers remembering.** `cancel()` and `complete()` already emit; `release()` does not, so the spec adds `task_released(task)` for symmetry. The `tasks` setter connects all three and clears any pawn holding that id. Without the third signal, the release path has to be cleared by hand from #75's work loop, which is exactly the kind of thing that gets forgotten once and then stays broken quietly. **3. `get_task(id)` returns null for an unknown id, with no `push_error()`.** A cancelled task is erased from `tasks`, so a pawn arriving with a stale id is the normal case rather than a fault — and GUT fails a test on any `push_error()` raised during it, so shouting here would make the honest case untestable. **4. `stop()` does not clear the task.** `repath_pawns()` is stop-then-`move_to`, so clearing in `stop()` would drop the task of every pawn a building re-routed — the #56 scenario this issue exists to handle. Two tests pin it. **5. `clear_task()` idles a WORKING pawn but leaves a MOVING one walking.** Whether to turn a now-taskless walker around is #75's call, not this issue's; all that is owed here is that its arrival resolves to IDLING. **6. Arriving off-site keeps the task.** The pawn goes IDLING but `current_task_id` survives, so #75 can send it again rather than having to re-claim from scratch. ### Deliberately out of scope `remove()` on a pawn that holds a claimed task leaves that task assigned to a dead id. The fix is a `release()` from the removal path, but it needs #111's decision on whether `assignee` stays single-valued first — filing separately unless you would rather it rode along here.

liamjd referenced this issue from a pull request that will close it,

2026-09-19 09:42:11 +01:00

Pawn knows which task it is walking to #116

liamjd referenced this issue from a commit

2026-09-19 09:42:11 +01:00

Pawn knows which task it is walking to (#112)

liamjd closed this issue

2026-09-19 09:49:21 +01:00

liamjd referenced this issue

2026-09-19 10:06:29 +01:00

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

liamjd referenced this issue