Parent and child tasks: blocking and cascading cancel #68

New issue

Closed

opened 2026-09-07 20:37:19 +01:00 by liamjd · 2 comments

liamjd commented

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

Owner

Part of milestone #5. Depends on #66 and #67.

Tasks may spawn child tasks (construction spawns haul tasks per required resource; later, terrain levelling). The parent must not be workable until its children are complete, and cancelling a parent must cancel its children.

Design decisions

Two points from the original AC were revised before implementation:

Linking is manager-side, not task-side. TaskManager.add_subtask(parent, child) rather than Task.add_child(task). The manager is the only thing that assigns ids, sets creation_seq and emits task_added, so registration stays on one path; it is also the only place that can validate the parent is registered and not already COMPLETE/CANCELLED. Task stays a data object with no back-reference to its manager (which would also create a second reference cycle).
Task.parent is a WeakRef. Entity extends RefCounted and GDScript has no cycle collector, so a strong parent <-> children pair leaks for the process lifetime. Expose it through a property getter so existing call sites keep working, and annotate the get_ref() result explicitly (it returns Variant).

Strong references to a task are held by _tasks while active, _completed after completion, and parent.children throughout. A cancelled child is erased from _tasks and survives only via parent.children — cancelled children are therefore kept in the array so a cancelled subtree stays inspectable.

AC

TaskManager.add_subtask(parent: Task, child: Task, priority: int = -1) -> int registers the child and links both ways; -1 means inherit the parent's priority, any other value overrides it
Adding a subtask to an unregistered, COMPLETE or CANCELLED parent is rejected with push_error and does not register the child
Task.can_start(ctx) becomes concrete on the base class: it returns false while any child is not COMPLETE, then delegates to a new abstract Task._can_start(ctx) hook. Existing stub tasks and tests are renamed to the hook.
Cancelling a parent cancels every child that is not already COMPLETE, depth-first, children before the parent; a CLAIMED child is released from its pawn first. Collect ids before mutating _tasks, and hold a local strong reference to the subtree root across the cascade so weak parent links do not go null mid-walk.
Completing the last child re-evaluates the parent so it is not left BLOCKED. Reuse the evaluation path next_for() already uses rather than adding a parallel one — the explicit call is so the status is correct between next_for() calls (signals, UI).
The creation_seq special case for children in TaskManager.add() is removed: children get creation_seq = id like any other task. Ordering comes from priority inheritance plus the BLOCKED gate, not from a sequence trick. (The old parent.creation_seq + 1 gave every sibling the same value, which makes the unstable sort_custom tie-break non-deterministic.)
Unit tests for each of the above with stub tasks, including a depth-two tree, and covering parent returning null for a root task

Part of milestone #5. Depends on #66 and #67. Tasks may spawn child tasks (construction spawns haul tasks per required resource; later, terrain levelling). The parent must not be workable until its children are complete, and cancelling a parent must cancel its children. ### Design decisions Two points from the original AC were revised before implementation: - **Linking is manager-side, not task-side.** `TaskManager.add_subtask(parent, child)` rather than `Task.add_child(task)`. The manager is the only thing that assigns ids, sets `creation_seq` and emits `task_added`, so registration stays on one path; it is also the only place that can validate the parent is registered and not already COMPLETE/CANCELLED. `Task` stays a data object with no back-reference to its manager (which would also create a second reference cycle). - **`Task.parent` is a `WeakRef`.** `Entity extends RefCounted` and GDScript has no cycle collector, so a strong `parent` <-> `children` pair leaks for the process lifetime. Expose it through a property getter so existing call sites keep working, and annotate the `get_ref()` result explicitly (it returns `Variant`). Strong references to a task are held by `_tasks` while active, `_completed` after completion, and `parent.children` throughout. A cancelled child is erased from `_tasks` and survives only via `parent.children` — cancelled children are therefore **kept** in the array so a cancelled subtree stays inspectable. ### AC - `TaskManager.add_subtask(parent: Task, child: Task, priority: int = -1) -> int` registers the child and links both ways; `-1` means inherit the parent's priority, any other value overrides it - Adding a subtask to an unregistered, COMPLETE or CANCELLED parent is rejected with `push_error` and does not register the child - `Task.can_start(ctx)` becomes concrete on the base class: it returns false while any child is not COMPLETE, then delegates to a new abstract `Task._can_start(ctx)` hook. Existing stub tasks and tests are renamed to the hook. - Cancelling a parent cancels every child that is not already COMPLETE, depth-first, children before the parent; a CLAIMED child is released from its pawn first. Collect ids before mutating `_tasks`, and hold a local strong reference to the subtree root across the cascade so weak `parent` links do not go null mid-walk. - Completing the last child re-evaluates the parent so it is not left BLOCKED. Reuse the evaluation path `next_for()` already uses rather than adding a parallel one — the explicit call is so the status is correct *between* `next_for()` calls (signals, UI). - The `creation_seq` special case for children in `TaskManager.add()` is removed: children get `creation_seq = id` like any other task. Ordering comes from priority inheritance plus the BLOCKED gate, not from a sequence trick. (The old `parent.creation_seq + 1` gave every sibling the same value, which makes the unstable `sort_custom` tie-break non-deterministic.) - Unit tests for each of the above with stub tasks, including a depth-two tree, and covering `parent` returning null for a root task

liamjd added this to the Task management for pawns milestone

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

liamjd added the

enhancement

label

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

liamjd referenced this issue

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

HaulTask as a child of ConstructionTask #73

liamjd referenced this issue from a commit

2026-09-11 07:43:28 +01:00

Complete the TaskManager queue and fix task selection ordering

liamjd referenced this issue

2026-09-11 07:44:00 +01:00

Task data model and TaskManager queue #84

liamjd added this to the Untitled Colony Builder project

2026-09-11 07:50:18 +01:00

liamjd self-assigned this

2026-09-11 07:50:19 +01:00

liamjd referenced this issue

2026-09-11 21:47:46 +01:00

Decide what cancelling a child task does to its parent #85

liamjd commented

2026-09-11 21:48:02 +01:00

Author

Owner

Cancelling a child on its own is out of scope here and tracked separately as #85 — the rules in this issue leave such a parent BLOCKED indefinitely, which is recorded (not endorsed) by test_a_cancelled_child_leaves_its_parent_unworkable in test/unit/test_task_tree.gd.

Cancelling a child on its own is out of scope here and tracked separately as #85 — the rules in this issue leave such a parent BLOCKED indefinitely, which is recorded (not endorsed) by `test_a_cancelled_child_leaves_its_parent_unworkable` in `test/unit/test_task_tree.gd`.

liamjd added reference 68-parent-and-child-task-cascade

2026-09-11 21:59:17 +01:00

liamjd commented

2026-09-11 21:59:55 +01:00

Author

Owner

Closed in PR#86

liamjd closed this issue

2026-09-11 21:59:55 +01:00