Files
gamedev-the-steward/docs/RESOURCE_NODE_MIGRATION.md
T
admin f5e7326d49 docs: update plans with phase progress, bug log, and current architecture state
RESOURCE_NODE_MIGRATION.md:
- Add progress overview table with per-phase completion status
- Document key divergences from original plan (zone fallback removal,
  navigation_failed signal, stale-path prevention, duplicate detection)
- Add bug log table (11 bugs found and fixed during implementation)
- Mark completed phases 1-3 with checkmarks
- Split phase 4 into 4a (wood, done) and 4b (player parity, not started)
- Replace flat implementation order with checkable table
- Update definition-of-done with per-criterion status

PROJECT_CONTEXT.md:
- Advance snapshot commit reference from e30d208 to b8752f8
- Note that gather_food/gather_wood now use ResourceNode instances
- Explain zone marker status (player use only, NPCs use nodes)
- Update runtime flow diagram with navigation_failed pathway
- Add resource_nodes/ to repository map
- Update technical debt entries to reflect current state
2026-07-04 19:31:43 +02:00

564 lines
20 KiB
Markdown

# The Steward — ResourceNode Migration Plan
## Progress Overview
| Phase | Description | Status |
|-------|-------------|--------|
| **1** | ResourceNode scene + registration | ✅ Complete |
| **2** | Food target selection + reservation | ✅ Complete |
| **3** | Authoritative extraction on completion | ✅ Complete |
| **4a** | Wood target selection (NPC) | ✅ Complete |
| **4b** | Player parity (extraction contract) | ⏳ Not started |
| **5** | Jajce world placement | ❌ Not started |
| **6** | Remove remaining zone model | ❌ Not started |
### Key divergences from the original plan
- **Zone fallback removed** (Phase 3): `gather_food`/`gather_wood` no longer fall back to `FarmZone`/`ForestZone` markers when no resource node is available. The original plan called for a logged fallback; instead, the NPC goes idle (via `navigation_failed` → wander) to guarantee resource conservation. The zone markers are still available for player interaction.
- **`navigation_failed` signal** (discovered during Phase 4): Added a separate signal pathway to distinguish genuine arrival from navigation failure. The original plan treated all "arrivals" uniformly.
- **Stale-path prevention** (discovered during Phase 4): Added `path_request_id` + `path_pending` to reject async path results that arrive after a new target was set or the NPC died.
- **Duplicate node_id detection** (discovered during Phase 4): Added startup validation that disables nodes with duplicate IDs and pushes an error.
### Bugs found and fixed
| # | Bug | Phase found | Fix |
|---|-----|-------------|-----|
| 1 | Navigation failure: empty path caused NPC to idle permanently | Phase 3 stump | `_report_arrival_once()` on empty path (later replaced by `navigation_failed` signal) |
| 2 | Unavailable-target deadlock: replan didn't trigger `npc_task_changed` | Phase 3 | Set `current_task = ""` so tick detects task change and emits signal |
| 3 | Zone fallback broke conservation (fixed during Phase 4 removal) | Phase 3 | Removed zone fallback for gather tasks; NPC stays idle if no node available |
| 4 | NPC collision blocking: NPCs physically blocked each other at shared destinations | Phase 3 | `collision_layer=2`, `collision_mask=1` on NpcVisual; `arrival_distance` 0.6→1.5 (later reverted to 0.6 after navigation_failed signal) |
| 5 | Repetitive task selection: NPC chose same task every tick | Phase 3 | `last_task` tracking with -1.5 score penalty when same task repeats |
| 6 | Wander didn't wander: wandered to rest marker instead of random offset | Phase 3 | Random ±6 unit offset from current position |
| 7 | `:=` infers from Variant: warnings treated as errors from `odig` | Phase 2-3 | Use `=` instead of `:=` for calls returning `Variant` |
| 8 | `Variant as StringName`: invalid cast | Phase 2-3 | Check for `null` on Dictionary `.get()` instead of casting |
| 9 | Empty-path re-entrancy: immediate arrival report caused re-entrant signal chain | Phase 4 | Defer to `stuck_counter`/`stuck_time` in `_physics_process` instead |
| 10 | Stale async path: `await physics_frame` result used after target or death changed | Phase 4 | `path_request_id`/`path_pending` guard |
| 11 | Stolen extraction: NPC extracted from node after reservation was lost | Phase 4 | Guard extraction with `node.reserved_by == npc.id` |
## Decision
The current task zones are temporary prototype scaffolding.
NPCs and the player should ultimately interact with actual world objects rather
than travel to one abstract marker per task:
```text
Current:
gather_food -> FarmZone -> village food increases
First migration:
gather_food -> choose BerryBush_01 -> travel to its interaction point
-> extract berries -> bush amount decreases
-> village food increases by the extracted amount
```
This migration should happen on the current flat map before the Jajce
environment is integrated. It is easier to debug target selection, reservation,
depletion, and task completion without terrain and navigation changes happening
at the same time.
The old zones remain only as a temporary fallback while actions are migrated.
They are not part of the target architecture.
## Why this is the next systems step
Resource nodes provide the first concrete bridge between:
- abstract simulation decisions;
- spatial target selection;
- active-world navigation;
- finite world state;
- visible cause and effect;
- player and NPC use of the same objects;
- future inventories, ownership, regeneration, and persistence.
They also make build-in-public footage more legible. A viewer can understand a
villager walking to a specific berry bush and depleting it more easily than a
villager entering an invisible “food zone.”
## Scope the first version correctly
The first `ResourceNode` supports only depletable material sources:
- berry bush or crop source -> food;
- tree or wood pile -> wood.
Do not initially use `ResourceNode` for:
- beds or campfires;
- guard posts;
- study locations;
- food storage;
- homes;
- generic workstations.
Those objects share target-selection behavior but not resource-extraction
semantics.
The likely family of world targets is:
```text
WorldActionTarget
├── ResourceNode finite extraction: bush, tree, ore
├── ActivitySite rest, patrol, study, socialize
├── StorageNode deposit and withdraw items
└── Workstation transform inputs into outputs
```
Do not implement this complete inheritance tree now. Build `ResourceNode` from
real requirements, then extract shared target behavior when a second target
type proves what is actually common.
## Fit with the current code
The existing flow is:
```text
SimNPC chooses a task string
-> SimulationManager emits npc_task_changed
-> WorldViewManager maps the task to one Marker3D
-> NpcVisual travels to that marker
-> arrival tells SimulationManager to start work
-> task duration completes
-> SimVillage applies a hard-coded task result
```
The migrated flow should become:
```text
SimNPC chooses an action
-> target resolver finds available matching ResourceNodes
-> one target is reserved for the NPC
-> NPC simulation stores the target's stable ID
-> NpcVisual travels to the target interaction point
-> arrival begins work
-> completion extracts the actual available amount
-> village receives exactly that amount
-> target updates or becomes depleted
-> reservation is released
```
During migration:
```text
gather_food -> prefer matching ResourceNode -> fallback FarmZone
gather_wood -> prefer matching ResourceNode -> fallback ForestZone
other tasks -> existing zones
```
Fallbacks should log clearly so accidental reliance on zones is visible.
## Repository-specific file layout
Keep scripts beside their feature scenes, matching the repository's existing
feature-oriented structure:
```text
world/
└── resource_nodes/
├── ResourceNode.gd
├── ResourceNode.gd.uid
└── ResourceNode.tscn
```
Add instances under the current `main.tscn` while developing:
```text
Main
├── ResourceNodes
│ ├── BerryBush_01
│ ├── BerryBush_02
│ ├── Tree_01
│ └── Tree_02
└── TaskZone temporary fallback
```
When `JajceWorld` is integrated, move or recreate the world-object instances
under:
```text
JajceWorld
└── WorldObjects
└── ResourceNodes
```
Target discovery must not depend on that exact scene path. Resource nodes should
register through a group or a small registry.
## First ResourceNode contract
The first node should answer:
- What is my stable world-object ID?
- Which action can use me?
- Which resource do I yield?
- How much remains?
- How much can one completed action extract?
- Am I enabled and usable?
- Who, if anyone, has reserved me?
- Where should an active character stand?
- Did extraction change or deplete me?
Recommended initial data:
```gdscript
class_name ResourceNode
extends Node3D
signal amount_changed(node_id: StringName, amount_remaining: float)
signal depleted(node_id: StringName)
signal reservation_changed(node_id: StringName, agent_id: int)
@export var node_id: StringName
@export var action_id: StringName = &"gather_food"
@export var resource_id: StringName = &"food"
@export var amount_remaining: float = 10.0
@export var yield_per_action: float = 2.0
@export var enabled: bool = true
@export var can_npcs_use: bool = true
@export var can_player_use: bool = true
@export var hide_visual_when_depleted: bool = false
@onready var interaction_point: Marker3D = $InteractionPoint
```
Use `StringName` IDs as a small migration step away from duplicated free-form
`String` values. Later, action and resource definitions can become custom
`Resource` assets without changing every world node's conceptual contract.
### Avoid duplicated state
Do not store a separately mutable `is_depleted` flag in the first version.
Derive it:
```gdscript
func is_depleted() -> bool:
return amount_remaining <= 0.0
```
If regeneration or non-depleting sources later require more states, introduce
them deliberately.
### Extraction result is authoritative
Extraction must return the amount actually removed:
```gdscript
func extract(requested_amount: float = yield_per_action) -> float:
if not can_extract():
return 0.0
var extracted := minf(requested_amount, amount_remaining)
amount_remaining -= extracted
# Emit signals and update presentation.
return extracted
```
The village receives this returned amount. Do not let both `ResourceNode` and
`SimVillage.apply_npc_task()` independently award the configured yield.
### Stable IDs
`node_id` must be:
- non-empty;
- unique inside the world;
- stable across save/load;
- independent of display name and scene path.
For the first hand-authored nodes, explicit IDs such as
`&"berry_bush_01"` are acceptable. Add startup validation for duplicates.
## Scene structure
The first reusable scene can remain small:
```text
ResourceNode (Node3D)
├── Visual (Node3D)
├── InteractionPoint (Marker3D)
└── DebugLabel (Label3D)
```
Use `Node3D` for `Visual` rather than requiring `MeshInstance3D`; this permits a
mesh, imported model, particles, or multiple visual children later.
The debug label should be controlled by a project/debug setting or inspector
toggle rather than a compile-time constant. It should show:
- node ID;
- resource;
- remaining amount;
- reservation owner;
- depleted/disabled state.
## Reservation rules
Without reservations, several NPCs can choose one nearly depleted bush and all
walk toward it.
The first version needs a single-user reservation:
```text
available -> reserved by agent -> in use -> released
```
A reservation must be released when:
- the task completes;
- the NPC dies;
- the target depletes;
- travel fails;
- the action is interrupted or replaced;
- the target is disabled;
- the NPC or visual is removed.
Reservation is not ownership. Ownership, permission, crime, and social
consequences belong to later systems.
## Target selection
The first resolver may choose the nearest available matching node. It must still
return a reason trace:
```text
candidate BerryBush_01: accepted, distance 8.2
candidate BerryBush_02: rejected, reserved by NPC 2
candidate BerryBush_03: rejected, depleted
selected BerryBush_01
```
Future scoring can include:
- travel cost;
- remaining yield;
- danger;
- tool requirements;
- ownership and permission;
- crowding;
- profession;
- expected completion time;
- knowledge of the target.
Do not implement those before the nearest-available version works.
## Simulation-state boundary
The first `ResourceNode` is a Godot world node and may temporarily own its
remaining amount. That is acceptable for the migration prototype, but it is not
the final persistence boundary.
Preserve these rules now:
- `SimNPC` stores a target ID, not a `Node` reference;
- simulation decisions do not store `NodePath`;
- visual movement receives only a resolved position;
- target extraction is coordinated through one adapter/registry;
- node state has an obvious serialization representation.
Later, authoritative resource state can move into serializable simulation
records while `ResourceNode` becomes its active-world presentation.
## Required changes by existing file
### `simulation/SimNPC.gd`
- add a selected target ID;
- clear it when returning to idle or changing actions;
- do not store `ResourceNode` references;
- preserve current task lifecycle during the first migration.
### `world/world_view_manager.gd`
- query available resource nodes for `gather_food` and `gather_wood`;
- reserve the selected target;
- send its interaction position to `NpcVisual`;
- use the old marker only when no valid node exists;
- distinguish arrival at a specific target from arrival at a generic task zone.
Target discovery can begin here, but should move into a focused resolver or
registry once selection has meaningful rules.
### `simulation/SimulationManager.gd`
- coordinate target completion and reservation release;
- extract from the selected node once;
- give `SimVillage` the actual result;
- handle interruption, death, and failure cleanup;
- prevent old hard-coded production from also firing.
### `simulation/SimVillage.gd`
- add an explicit resource-delta method;
- separate generic resource application from task-name matching;
- retain current action effects for non-migrated tasks.
### `player/player.gd`
- eventually resolve and use the same world targets as NPCs;
- do not add a second player-only harvest path with different rules.
Player use can follow NPC migration rather than block its first test.
## Migration phases
### ✅ Phase 1 — ResourceNode scene and registration *[complete]*
- create `ResourceNode.gd` and `ResourceNode.tscn`;
- create a `ResourceNodes` parent in `main.tscn`;
- place two bushes and two trees;
- give every node a stable unique ID;
- show debug state;
- expose group/registry discovery.
**Exit:** nodes register, validate IDs, change amount, deplete, and update debug
presentation without involving NPC logic.
### ✅ Phase 2 — Food target selection and reservation *[complete]*
- migrate only `gather_food`;
- find nearest available bush;
- reserve it;
- store its ID on the NPC;
- navigate to its interaction point;
- release on interruption and death;
- ~~retain `FarmZone` fallback with a visible warning~~ *(removed in Phase 4,
see divergences above)*
**Exit:** two NPCs do not select the same single-user bush, and an NPC can
replan when its chosen bush becomes unavailable.
### ✅ Phase 3 — Authoritative completion *[complete]*
- extract only after work duration completes;
- return actual extracted amount;
- apply that amount to village food;
- prevent double production;
- deplete and hide/alter the source as configured;
- choose a new source for later tasks.
**Exit:** resource conservation holds:
```text
total bush decrease == total village food increase
```
for the initial gather-food scenario.
### 🔶 Phase 4 — Wood and player parity *(partially complete)*
**Wood migration** ✅ complete:
- `gather_wood` uses `ResourceNode.find_available("gather_wood")` in WVM
- Tree_01 (15 wood, yield 3.0) and Tree_02 (12 wood, yield 3.0) placed
- Extraction follows same path as food (reserve → travel → work → extract → village resource delta)
- Zone fallback removed; NPC goes idle if no tree available
**Player parity** ⏳ not started:
- Player currently still uses zone-marker interaction in `player.gd`
- Player should use `ResourceNode.extract()` via proximity detection or raycast
- Player interaction bypasses reservation (player presses E at a node → extract if available)
- `PlayerInteraction.gd` exists as an empty stub
**Updated exit:** food and wood no longer require their abstract zones during
normal play *for NPCs*. Player parity is required to fully exit this phase.
### Phase 5 — Jajce world placement
- place bushes and trees as real world objects in `JajceWorld`;
- keep their IDs stable;
- validate interaction points against terrain and navigation;
- use the old task zones only for rest, eat, patrol, and study;
- verify target discovery does not rely on parent scene paths.
**Exit:** the existing food and wood loops work after moving from the flat map
to Terrain3D.
### Phase 6 — Remove the remaining zone model
This is not part of the first ResourceNode implementation.
Introduce the next concrete target types:
- storage for eating and item transfer;
- activity sites for rest, study, and guard work;
- workstations when production recipes begin.
Remove each old zone only when its replacement has:
- target selection;
- active interaction position;
- reservation/capacity;
- completion or ongoing-use behavior;
- failure/interruption handling;
- debug visibility;
- save representation.
**Exit:** `TaskZone` and task-to-marker mapping are deleted, not merely unused.
## Test scenarios
At minimum, exercise:
1. one NPC and one bush;
2. two NPCs and one bush;
3. one NPC and two bushes at different distances;
4. selected bush depletes before arrival;
5. selected bush depletes exactly on completion;
6. NPC dies while traveling;
7. NPC dies while working;
8. navigation fails;
9. node is disabled while reserved;
10. fallback zone is used because no matching node exists;
11. player and NPC contend for the same source;
12. duplicate node IDs are detected.
Use a fixed seed and concise reason tracing where possible.
## Video opportunities
The migration naturally creates strong short-form demonstrations:
- “My NPCs stopped walking to invisible work zones.”
- “Two hungry villagers want the same berry bush.”
- “This bush contains the food—the UI counter no longer creates it.”
- “What happens when the closest resource is already reserved?”
- “A depleted forest changes what every woodcutter decides.”
Show the beautiful result first, then reveal target selection, reservation, and
resource conservation through debug presentation.
## Implementation status
| # | Task | Status | Commits |
|---|------|--------|---------|
| 1 | Create `ResourceNode.gd` and `ResourceNode.tscn` | ✅ Done | `9cd38a1` |
| 2 | Add `Main/ResourceNodes` | ✅ Done | `9cd38a1` |
| 3 | Place BerryBush_01, BerryBush_02, Tree_01, Tree_02 | ✅ Done | `9cd38a1` |
| 4 | Add stable IDs and registration validation | ✅ Done | `9cd38a1`, `1837bc0` |
| 5 | Verify extraction and depletion manually | ✅ Done | `9cd38a1` |
| 6 | Add target ID to `SimNPC` | ✅ Done | `9cd38a1` |
| 7 | Migrate `gather_food` target selection | ✅ Done | `9cd38a1`, `c6033b6` |
| 8 | Add reservation and cleanup | ✅ Done | `9cd38a1`, `b8752f8` |
| 9 | Apply actual extracted yield on completion | ✅ Done | `c6033b6` |
| 10 | Add navigation failure handling (replaces regression scenarios) | ✅ Done | `b8752f8` |
| 11 | Migrate wood | ✅ Done | `c6033b6`, `8049c52` |
| 12 | Give the player the same extraction path | ⏳ Not started | — |
| 13 | Continue with the Jajce world scaffold | ❌ Not started | — |
## Definition of done for the first migration
| Criterion | Status |
|-----------|--------|
| Food and wood use actual finite world objects | ✅ (NPCs) / ⏳ (player) |
| NPCs select available targets and reserve them | ✅ |
| NPC state stores stable target IDs rather than nodes or scene paths | ✅ |
| Active visuals navigate to explicit interaction points | ✅ |
| Work duration completes before extraction | ✅ |
| Village resources increase only by the amount actually extracted | ✅ |
| Depletion changes availability and presentation | ✅ |
| Failure, interruption, and death release reservations | ✅ |
| Old food and forest zone fallbacks are no longer used by NPCs | ✅ (fallback removed by design) |
| The player and NPCs can use the same extraction contract | ⏳ (player side not started) |
| Behavior is inspectable and covered by reproducible scenarios | ✅ (debug logging, debug overlay on nodes) |