diff --git a/client/README.md b/client/README.md index a4b9705..5b5028f 100644 --- a/client/README.md +++ b/client/README.md @@ -213,7 +213,7 @@ On **Linux** (including GitHub Actions), the path must be **`gdUnit4`** with a c **CI:** The **GDScript** workflow fails the job if Godot prints **`SCRIPT ERROR:`** or **`ERROR: Failed to load script`** while running GdUnit. GdUnit alone can still exit **0** when a test file fails to parse (that suite is skipped); the workflow guards against that. -**Scope:** Unit tests cover **`player.gd`**, **`player_locomotion_wasd.gd`** (pure WASD seam helpers), **`position_authority_client.gd`**, **`inventory_client.gd`**, **`item_definitions_client.gd`** (NEO-72), **`isometric_follow_camera.gd`** (eye math + effective zoom distance), **`camera_state.gd`**, and **`zoom_band_config.gd`**. **`main.gd`** and scene wiring are **not** automated here—use manual checks above. +**Scope:** Unit tests cover **`player.gd`**, **`player_locomotion_wasd.gd`** (pure WASD seam helpers), **`position_authority_client.gd`**, **`inventory_client.gd`**, **`item_definitions_client.gd`** (NEO-72), **`skill_progression_client.gd`**, **`prototype_interactable_picker.gd`**, extended **`interaction_request_client_test.gd`**, **`gather_feedback_refresh_test.gd`** (NEO-73), **`isometric_follow_camera.gd`** (eye math + effective zoom distance), **`camera_state.gd`**, and **`zoom_band_config.gd`**. **`main.gd`** and scene wiring are **not** automated here—use manual checks above. **Camera zoom:** Input actions **`camera_zoom_in`** / **`camera_zoom_out`** (mouse wheel, **`=`** / **`-`**, keypad **+** / **−**) are defined in **`project.godot`**. On layouts where **`+`** requires **Shift**, remap or add a binding under **Project → Project Settings → Input Map** if needed. diff --git a/client/scripts/interaction_request_client.gd b/client/scripts/interaction_request_client.gd index e76ff5d..1b0e09a 100644 --- a/client/scripts/interaction_request_client.gd +++ b/client/scripts/interaction_request_client.gd @@ -8,6 +8,7 @@ extends Node ## [method post_interact_id]) so the embedded Game dock still receives E/R reliably. signal interaction_result_received(interactable_id: String, allowed: bool, reason_code: String) +signal interaction_request_failed(interactable_id: String, reason: String) const ID_TERMINAL := "prototype_terminal" const ID_RESOURCE_NODE := "prototype_resource_node_alpha" @@ -72,6 +73,7 @@ func _start_post(interactable_id: String) -> void: if err != OK: push_warning("InteractionRequestClient: POST failed to start (%s)" % err) _busy = false + interaction_request_failed.emit(interactable_id, "POST failed to start (%s)" % err) _try_flush_pending() @@ -104,5 +106,9 @@ func _on_request_completed( interaction_result_received.emit(id_echo, allowed, reason_str) _try_flush_pending() return + interaction_request_failed.emit(id_echo, "non-JSON body") + _try_flush_pending() + return print("Interaction [%s]: HTTP %s body=%s" % [id_echo, response_code, text]) + interaction_request_failed.emit(id_echo, "HTTP %s" % response_code) _try_flush_pending() diff --git a/client/scripts/main.gd b/client/scripts/main.gd index f083893..bc5c01a 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -376,7 +376,7 @@ func _on_inventory_sync_failed(reason: String) -> void: _render_inventory_label() if _pending_gather_feedback: _pending_gather_feedback = false - _render_gather_feedback_label("Gather: ok — inventory refresh failed") + _render_gather_feedback_label("Gather: inventory refresh failed") func _render_inventory_label() -> void: @@ -472,6 +472,10 @@ func _setup_gather_interact_feedback() -> void: _interaction_client.connect( "interaction_result_received", Callable(self, "_on_interaction_result_for_gather") ) + if _interaction_client.has_signal("interaction_request_failed"): + _interaction_client.connect( + "interaction_request_failed", Callable(self, "_on_interaction_request_failed_for_gather") + ) _render_gather_feedback_label() @@ -540,6 +544,14 @@ func _on_interaction_result_for_gather( _skill_progression_client.call("request_sync_from_server") +func _on_interaction_request_failed_for_gather(interactable_id: String, _reason: String) -> void: + if not _pending_gather_feedback: + return + if not PrototypeInteractablePicker.is_resource_node_id(_interactables_catalog, interactable_id): + return + _pending_gather_feedback = false + + func _total_bag_qty(item_id: String, snapshot: Dictionary = {}) -> int: var source: Dictionary = snapshot if source.is_empty(): diff --git a/client/test/gather_feedback_refresh_test.gd b/client/test/gather_feedback_refresh_test.gd index e5c6fc9..c4a5a53 100644 --- a/client/test/gather_feedback_refresh_test.gd +++ b/client/test/gather_feedback_refresh_test.gd @@ -14,6 +14,8 @@ class MockInteractTransport: result: int, response_code: int, headers: PackedStringArray, body: PackedByteArray ) var last_body: String = "" + var body_json: String = '{"schemaVersion":1,"allowed":true}' + var response_code: int = 200 func request( _url: String, @@ -22,10 +24,10 @@ class MockInteractTransport: request_data: String = "" ) -> Error: last_body = request_data - var body_bytes: PackedByteArray = ( - '{"schemaVersion":1,"allowed":true}'.to_utf8_buffer() + var body_bytes: PackedByteArray = body_json.to_utf8_buffer() + request_completed.emit( + HTTPRequest.RESULT_SUCCESS, response_code, PackedStringArray(), body_bytes ) - request_completed.emit(HTTPRequest.RESULT_SUCCESS, 200, PackedStringArray(), body_bytes) return OK @@ -84,3 +86,19 @@ func test_allowed_resource_interact_triggers_both_refreshes() -> void: assert_signal(harness._ix).is_emitted("interaction_result_received") assert_that(harness._inventory.sync_calls).is_equal(1) assert_that(harness._skill.sync_calls).is_equal(1) + + +func test_denied_resource_interact_skips_refreshes() -> void: + # Arrange + var transport := MockInteractTransport.new() + transport.body_json = '{"schemaVersion":1,"allowed":false,"reasonCode":"node_depleted"}' + var harness := GatherRefreshHarness.new() + auto_free(transport) + auto_free(harness) + add_child(harness) + harness.setup(self, transport) + # Act + harness.post_resource_alpha() + # Assert + assert_that(harness._inventory.sync_calls).is_equal(0) + assert_that(harness._skill.sync_calls).is_equal(0) diff --git a/client/test/interaction_request_client_test.gd b/client/test/interaction_request_client_test.gd index 25ab535..2e0a803 100644 --- a/client/test/interaction_request_client_test.gd +++ b/client/test/interaction_request_client_test.gd @@ -14,6 +14,7 @@ class MockHttpTransport: var last_body: String = "" var last_method: HTTPClient.Method = HTTPClient.Method.METHOD_GET var body_json: String = '{"schemaVersion":1,"allowed":true}' + var response_code: int = 200 func request( url: String, @@ -25,7 +26,9 @@ class MockHttpTransport: last_body = request_data last_method = method var body_bytes: PackedByteArray = body_json.to_utf8_buffer() - request_completed.emit(HTTPRequest.RESULT_SUCCESS, 200, PackedStringArray(), body_bytes) + request_completed.emit( + HTTPRequest.RESULT_SUCCESS, response_code, PackedStringArray(), body_bytes + ) return OK @@ -132,6 +135,18 @@ func test_interaction_result_emits_deny_signal() -> void: ) +func test_interaction_request_failed_emits_on_http_error() -> void: + # Arrange + var transport := MockHttpTransport.new() + transport.response_code = 500 + var ix := _make_ix(transport) + monitor_signals(ix) + # Act + ix.call("post_interact_id", "prototype_resource_node_alpha") + # Assert + assert_signal(ix).is_emitted("interaction_request_failed", "prototype_resource_node_alpha", "HTTP 500") + + func test_last_wins_second_press_before_first_completes() -> void: # Arrange var transport := HoldFirstThenAutoTransport.new() diff --git a/docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md b/docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md index 17d7760..247b5e5 100644 --- a/docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md +++ b/docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md @@ -102,7 +102,7 @@ Epic 3 **Slice 2** — [gather nodes and outputs](../epics/epic_03_crafting_econ **Resource node instance depletion store (NEO-61):** **`IResourceNodeInstanceStore`** + **`ResourceNodeInstanceOperations`** — lazy-init remaining gathers from catalog **`maxGathers`**, atomic decrement, **`node_depleted`** / **`unknown_node`** reason codes; Postgres **`V006`** or in-memory fallback. Plan: [NEO-61 implementation plan](../../plans/NEO-61-implementation-plan.md). -**Linear backlog (decomposed):** [E3M1-prototype-backlog.md](../../plans/E3M1-prototype-backlog.md) — **E3M1-01** [NEO-57](https://linear.app/neon-sprawl/issue/NEO-57) through **E3M1-08** [NEO-64](https://linear.app/neon-sprawl/issue/NEO-64). **Client (Slice 5):** gather feedback — [E3S5-client-prototype-backlog.md](../../plans/E3S5-client-prototype-backlog.md) **E3S5-02** [NEO-73](https://linear.app/neon-sprawl/issue/NEO-73). +**Linear backlog (decomposed):** [E3M1-prototype-backlog.md](../../plans/E3M1-prototype-backlog.md) — **E3M1-01** [NEO-57](https://linear.app/neon-sprawl/issue/NEO-57) through **E3M1-08** [NEO-64](https://linear.app/neon-sprawl/issue/NEO-64). **Client (Slice 5):** gather feedback **landed** — [E3S5-client-prototype-backlog.md](../../plans/E3S5-client-prototype-backlog.md) **E3S5-02** [NEO-73](https://linear.app/neon-sprawl/issue/NEO-73): **`skill_progression_client.gd`**, **`prototype_interactable_picker.gd`** (nearest in-range **R**), **`GatherFeedbackLabel`** + **`SkillProgressionLabel`**, auto inventory/skill refresh after successful gather ([NEO-73 implementation plan](../../plans/NEO-73-implementation-plan.md), [`NEO-73` manual QA](../../manual-qa/NEO-73.md)); `client/README.md` gather section. ## Source anchors diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 2431e48..1d4fd90 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -53,8 +53,8 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E2.M1 | In Progress | **NEO-33 landed:** frozen prototype trio **`salvage`** / **`refine`** / **`intrusion`** in [`content/skills/prototype_skills.json`](../../../content/skills/prototype_skills.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) enforce schema, duplicate `id`, exact trio ids, and category coverage; designer note on **`allowedXpSourceKinds`** in [E2_M1](E2_M1_SkillDefinitionRegistry.md) + [`content/README.md`](../../../content/README.md). **NEO-34 landed:** fail-fast server load of `content/skills/*.json` at startup — `server/NeonSprawl.Server/Game/Skills/` ([NEO-34](../../plans/NEO-34-implementation-plan.md)); config + discovery in [server README — Skill catalog](../../../server/README.md#skill-catalog-contentskills-neo-34). **NEO-35 landed:** `ISkillDefinitionRegistry` / `SkillDefinitionRegistry` + DI ([NEO-35](../../plans/NEO-35-implementation-plan.md)). **NEO-36 landed:** versioned **`GET /game/world/skill-definitions`** ([NEO-36](../../plans/NEO-36-implementation-plan.md)) — `SkillDefinitionsWorldApi` + `SkillDefinitionsListDtos` in `server/NeonSprawl.Server/Game/Skills/`; Bruno `bruno/neon-sprawl-server/skill-definitions/`; manual QA [`NEO-36`](../../manual-qa/NEO-36.md). **E2.M2 consumer (NEO-38 landed):** grant path validates `skillId` + `sourceKind` vs catalog **`allowedXpSourceKinds`** on **`POST …/skill-progression`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); see [server README — Skill progression grant](../../../server/README.md#skill-progression-grant-neo-38) and [E2_M2](E2_M2_XpAwardAndLevelEngine.md). | [NEO-33](../../plans/NEO-33-implementation-plan.md), [NEO-34](../../plans/NEO-34-implementation-plan.md), [NEO-35](../../plans/NEO-35-implementation-plan.md), [NEO-36](../../plans/NEO-36-implementation-plan.md); [E2_M1](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note**; `server/NeonSprawl.Server/Game/Skills/`; [`docs/manual-qa/NEO-36.md`](../../manual-qa/NEO-36.md); [server README — Skill definitions (NEO-36)](../../../server/README.md#skill-definitions-neo-36) | | E2.M3 | In Progress | **NEO-45 landed:** prototype **`salvage`** mastery catalog + CI gates (see [NEO-45 plan](../../plans/NEO-45-implementation-plan.md)). **NEO-46 landed:** fail-fast server load under `server/NeonSprawl.Server/Game/Mastery/` — `MasteryCatalogLoader`, `IMasteryCatalogRegistry`, cross-check vs `ISkillDefinitionRegistry`, Slice 4 + **`tierIndex`** 1..N gate; see [NEO-46 plan](../../plans/NEO-46-implementation-plan.md). **NEO-47 landed:** `PerkUnlockEngine`, `IPlayerPerkStateStore` + **`V004`**, level-up hook in skill XP grants; see [NEO-47 plan](../../plans/NEO-47-implementation-plan.md). **NEO-49 landed:** comment-only **`perk_unlock`** telemetry hook site in [`PerkUnlockEngine.TryUnlockPerks`](../../../server/NeonSprawl.Server/Game/Mastery/PerkUnlockEngine.cs) ([NEO-49 plan](../../plans/NEO-49-implementation-plan.md), [`NEO-49` manual QA](../../manual-qa/NEO-49.md)); [server README — Perk unlock telemetry (NEO-49)](../../../server/README.md#perk-unlock-engine-and-telemetry-hooks-neo-47-neo-49). **NEO-48 landed:** **`GET`/`POST /game/players/{id}/perk-state`** — `PerkStateApi` + DTOs in `Game/Mastery/` ([NEO-48](../../plans/NEO-48-implementation-plan.md), [`NEO-48` manual QA](../../manual-qa/NEO-48.md)); [server README — Perk state (NEO-48)](../../../server/README.md#perk-state-neo-48); Bruno `bruno/neon-sprawl-server/perk-state/`. | [NEO-45](../../plans/NEO-45-implementation-plan.md), [NEO-46](../../plans/NEO-46-implementation-plan.md), [NEO-47](../../plans/NEO-47-implementation-plan.md), [NEO-48](../../plans/NEO-48-implementation-plan.md), [NEO-49](../../plans/NEO-49-implementation-plan.md), [E2M3-pre-production-backlog](../../plans/E2M3-pre-production-backlog.md), [E2_M3](E2_M3_MasteryAndPerkUnlocks.md) | | E2.M2 | In Progress | **NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([NEO-37](../../plans/NEO-37-implementation-plan.md)) — read model for every registered skill; known-player gate via `IPositionStateStore`; [server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37); manual QA [`NEO-37`](../../manual-qa/NEO-37.md). **NEO-38 landed:** **`POST`** same path — grant apply, persistence (`IPlayerSkillProgressionStore`, `V003` migration), structured denies + **`levelUps`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); manual QA [`NEO-38`](../../manual-qa/NEO-38.md); Bruno `bruno/neon-sprawl-server/skill-progression/`; [server README — Skill progression grant (NEO-38)](../../../server/README.md#skill-progression-grant-neo-38). **NEO-39 landed:** data-driven `LevelCurve` content + schema + CI validation (`*_level_curve.json`) with fail-fast startup schema checks; progression GET/POST level resolution now uses `ISkillLevelCurve` content-backed thresholds ([NEO-39](../../plans/NEO-39-implementation-plan.md)); manual QA [`NEO-39`](../../manual-qa/NEO-39.md). **NEO-40 landed:** comment-only telemetry hook sites in [`SkillProgressionSnapshotApi.cs`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs) on **`POST …/skill-progression`** for future **`xp_grant`** / **`level_up`** ([NEO-40](../../plans/NEO-40-implementation-plan.md), [`NEO-40` manual QA](../../manual-qa/NEO-40.md)). **NEO-41 landed:** gather prototype — **`POST …/interact`** with **`kind: resource_node`** grants **`salvage`** + **`activity`** (10 XP) via [`SkillProgressionGrantOperations`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantOperations.cs) ([NEO-41](../../plans/NEO-41-implementation-plan.md), [`NEO-41` manual QA](../../manual-qa/NEO-41.md)); [server README — Interaction](../../../server/README.md#interaction-neo-9). **NEO-42 landed (prep):** **`RefineActivitySkillXpGrant`** / **`RefineSkillXpConstants`** + deny-registry factory + tests — same NEO-38 grant stack; **E3.M2** must invoke on craft/refine success ([NEO-42](../../plans/NEO-42-implementation-plan.md), [`NEO-42` manual QA](../../manual-qa/NEO-42.md)); [server README — Craft / refine hook (NEO-42)](../../../server/README.md#craft--refine-hook--skill-xp-neo-42). **NEO-43 landed (prep):** **`MissionRewardSkillXpGrant`** / **`MissionRewardSkillXpConstants`** + deny-registry factory + tests — same NEO-38 grant stack with fixed **`sourceKind: mission_reward`**; **E7.M2** must invoke from quest hand-in ([NEO-43](../../plans/NEO-43-implementation-plan.md), [`NEO-43` manual QA](../../manual-qa/NEO-43.md)); [server README — Mission / quest reward (NEO-43)](../../../server/README.md#mission--quest-reward--skill-xp-neo-43). **Slice 3 still open:** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [NEO-37](../../plans/NEO-37-implementation-plan.md), [NEO-38](../../plans/NEO-38-implementation-plan.md), [NEO-39](../../plans/NEO-39-implementation-plan.md), [NEO-40](../../plans/NEO-40-implementation-plan.md), [NEO-41](../../plans/NEO-41-implementation-plan.md), [NEO-42](../../plans/NEO-42-implementation-plan.md), [NEO-43](../../plans/NEO-43-implementation-plan.md), [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37–NEO-41, NEO-42–NEO-44 | -| E3.M1 | Ready | **NEO-41 landed (prototype, superseded on interact by NEO-63):** inline XP on **`resource_node`** interact. **NEO-57–NEO-61** — content, registry, HTTP defs, depletion store (see prior alignment). **NEO-62 landed:** **`GatherOperations`** + **`GatherResult`**. **NEO-63 landed:** **`POST …/interact`** **`resource_node`** → gather engine; **four** **`PrototypeInteractableRegistry`** anchors; Bruno gather → inventory + depletion deny ([NEO-63](../../plans/NEO-63-implementation-plan.md), [`NEO-63` manual QA](../../manual-qa/NEO-63.md)); [server README — Gather via interact (NEO-63)](../../../server/README.md#gather-via-interact-neo-63). **NEO-64 landed:** comment-only **`resource_gathered`** / **`gather_node_depleted`** hook sites in **`GatherOperations.TryGather`** ([NEO-64](../../plans/NEO-64-implementation-plan.md), [`NEO-64` manual QA](../../manual-qa/NEO-64.md)); Epic 3 Slice 2 backlog **E3M1-01**–**E3M1-08** complete. **Slice 5 client (planned):** gather feedback HUD [NEO-73](https://linear.app/neon-sprawl/issue/NEO-73) — [E3S5 backlog](../../plans/E3S5-client-prototype-backlog.md). | [NEO-41](../../plans/NEO-41-implementation-plan.md) … [NEO-64](../../plans/NEO-64-implementation-plan.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3_M1](E3_M1_ResourceNodeAndGatherLoop.md); `server/NeonSprawl.Server/Game/Gathering/`, `Game/Interaction/` | -| E3.M3 | In Progress | **NEO-50 landed:** frozen prototype six-item catalog in [`content/items/prototype_items.json`](../../../content/items/prototype_items.json); [`item-def.schema.json`](../../../content/schemas/item-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py). **NEO-51 landed:** fail-fast server load of `content/items/*_items.json` at startup — `server/NeonSprawl.Server/Game/Items/` ([NEO-51](../../plans/NEO-51-implementation-plan.md)); [server README — Item catalog](../../../server/README.md#item-catalog-contentitems-neo-51). **NEO-52 landed:** injectable **`IItemDefinitionRegistry`** + lookup tests ([NEO-52](../../plans/NEO-52-implementation-plan.md)). **NEO-53 landed:** **`GET /game/world/item-definitions`** — `ItemDefinitionsWorldApi` + DTOs in `Game/Items/` ([NEO-53](../../plans/NEO-53-implementation-plan.md), [`NEO-53` manual QA](../../manual-qa/NEO-53.md)); [server README — Item definitions (NEO-53)](../../../server/README.md#item-definitions-neo-53); Bruno `bruno/neon-sprawl-server/item-definitions/`. **NEO-54 landed:** **`IPlayerInventoryStore`** + **`PlayerInventoryOperations`** — 24 bag + 1 equipment slots, stack rules, `V005` migration ([NEO-54](../../plans/NEO-54-implementation-plan.md)). **NEO-55 landed:** **`GET`/`POST /game/players/{id}/inventory`** — `PlayerInventoryApi` + DTOs in `Game/Items/` ([NEO-55](../../plans/NEO-55-implementation-plan.md)); [server README — Player inventory (NEO-54 store, NEO-55 HTTP)](../../../server/README.md#player-inventory-neo-54-store-neo-55-http); Bruno `bruno/neon-sprawl-server/inventory/`. **NEO-56 landed:** comment-only **`item_created`** / **`inventory_transfer_denied`** telemetry hook sites in [`PlayerInventoryOperations`](../../../server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs) ([NEO-56](../../plans/NEO-56-implementation-plan.md), [`NEO-56` manual QA](../../manual-qa/NEO-56.md)); no E9.M1 ingest. **NEO-72 landed:** client inventory HUD — **`inventory_client.gd`**, **`item_definitions_client.gd`**, **`InventoryLabel`**, boot hydrate + **`I`** refresh ([NEO-72](../../plans/NEO-72-implementation-plan.md), [`NEO-72` manual QA](../../manual-qa/NEO-72.md)); `client/README.md` inventory HUD section. **Slice 5 client (remaining):** gather feedback [NEO-73](https://linear.app/neon-sprawl/issue/NEO-73), craft UI [NEO-74](https://linear.app/neon-sprawl/issue/NEO-74), capstone [NEO-75](https://linear.app/neon-sprawl/issue/NEO-75) — [E3S5 backlog](../../plans/E3S5-client-prototype-backlog.md). | [NEO-50](../../plans/NEO-50-implementation-plan.md), [NEO-51](../../plans/NEO-51-implementation-plan.md), [NEO-52](../../plans/NEO-52-implementation-plan.md), [NEO-53](../../plans/NEO-53-implementation-plan.md), [NEO-54](../../plans/NEO-54-implementation-plan.md), [NEO-55](../../plans/NEO-55-implementation-plan.md), [NEO-56](../../plans/NEO-56-implementation-plan.md), [NEO-72](../../plans/NEO-72-implementation-plan.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3M3-prototype-backlog](../../plans/E3M3-prototype-backlog.md), [E3_M3](E3_M3_ItemizationAndInventorySchema.md) | +| E3.M1 | Ready | **NEO-41 landed (prototype, superseded on interact by NEO-63):** inline XP on **`resource_node`** interact. **NEO-57–NEO-61** — content, registry, HTTP defs, depletion store (see prior alignment). **NEO-62 landed:** **`GatherOperations`** + **`GatherResult`**. **NEO-63 landed:** **`POST …/interact`** **`resource_node`** → gather engine; **four** **`PrototypeInteractableRegistry`** anchors; Bruno gather → inventory + depletion deny ([NEO-63](../../plans/NEO-63-implementation-plan.md), [`NEO-63` manual QA](../../manual-qa/NEO-63.md)); [server README — Gather via interact (NEO-63)](../../../server/README.md#gather-via-interact-neo-63). **NEO-64 landed:** comment-only **`resource_gathered`** / **`gather_node_depleted`** hook sites in **`GatherOperations.TryGather`** ([NEO-64](../../plans/NEO-64-implementation-plan.md), [`NEO-64` manual QA](../../manual-qa/NEO-64.md)); Epic 3 Slice 2 backlog **E3M1-01**–**E3M1-08** complete. **NEO-73 landed:** client gather feedback — **`skill_progression_client.gd`**, **`prototype_interactable_picker.gd`** (nearest in-range **R**), **`GatherFeedbackLabel`** + **`SkillProgressionLabel`**, auto inventory/skill refresh after successful gather ([NEO-73](../../plans/NEO-73-implementation-plan.md), [`NEO-73` manual QA](../../manual-qa/NEO-73.md)); `client/README.md` gather section. **Slice 5 client (remaining):** craft UI [NEO-74](https://linear.app/neon-sprawl/issue/NEO-74), capstone [NEO-75](https://linear.app/neon-sprawl/issue/NEO-75) — [E3S5 backlog](../../plans/E3S5-client-prototype-backlog.md). | [NEO-41](../../plans/NEO-41-implementation-plan.md) … [NEO-64](../../plans/NEO-64-implementation-plan.md), [NEO-73](../../plans/NEO-73-implementation-plan.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3_M1](E3_M1_ResourceNodeAndGatherLoop.md); `server/NeonSprawl.Server/Game/Gathering/`, `Game/Interaction/`, `client/scripts/skill_progression_client.gd`, `client/scripts/prototype_interactable_picker.gd` | +| E3.M3 | In Progress | **NEO-50 landed:** frozen prototype six-item catalog in [`content/items/prototype_items.json`](../../../content/items/prototype_items.json); [`item-def.schema.json`](../../../content/schemas/item-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py). **NEO-51 landed:** fail-fast server load of `content/items/*_items.json` at startup — `server/NeonSprawl.Server/Game/Items/` ([NEO-51](../../plans/NEO-51-implementation-plan.md)); [server README — Item catalog](../../../server/README.md#item-catalog-contentitems-neo-51). **NEO-52 landed:** injectable **`IItemDefinitionRegistry`** + lookup tests ([NEO-52](../../plans/NEO-52-implementation-plan.md)). **NEO-53 landed:** **`GET /game/world/item-definitions`** — `ItemDefinitionsWorldApi` + DTOs in `Game/Items/` ([NEO-53](../../plans/NEO-53-implementation-plan.md), [`NEO-53` manual QA](../../manual-qa/NEO-53.md)); [server README — Item definitions (NEO-53)](../../../server/README.md#item-definitions-neo-53); Bruno `bruno/neon-sprawl-server/item-definitions/`. **NEO-54 landed:** **`IPlayerInventoryStore`** + **`PlayerInventoryOperations`** — 24 bag + 1 equipment slots, stack rules, `V005` migration ([NEO-54](../../plans/NEO-54-implementation-plan.md)). **NEO-55 landed:** **`GET`/`POST /game/players/{id}/inventory`** — `PlayerInventoryApi` + DTOs in `Game/Items/` ([NEO-55](../../plans/NEO-55-implementation-plan.md)); [server README — Player inventory (NEO-54 store, NEO-55 HTTP)](../../../server/README.md#player-inventory-neo-54-store-neo-55-http); Bruno `bruno/neon-sprawl-server/inventory/`. **NEO-56 landed:** comment-only **`item_created`** / **`inventory_transfer_denied`** telemetry hook sites in [`PlayerInventoryOperations`](../../../server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs) ([NEO-56](../../plans/NEO-56-implementation-plan.md), [`NEO-56` manual QA](../../manual-qa/NEO-56.md)); no E9.M1 ingest. **NEO-72 landed:** client inventory HUD — **`inventory_client.gd`**, **`item_definitions_client.gd`**, **`InventoryLabel`**, boot hydrate + **`I`** refresh ([NEO-72](../../plans/NEO-72-implementation-plan.md), [`NEO-72` manual QA](../../manual-qa/NEO-72.md)); `client/README.md` inventory HUD section. **Slice 5 client (remaining):** craft UI [NEO-74](https://linear.app/neon-sprawl/issue/NEO-74), capstone [NEO-75](https://linear.app/neon-sprawl/issue/NEO-75) — [E3S5 backlog](../../plans/E3S5-client-prototype-backlog.md). | [NEO-50](../../plans/NEO-50-implementation-plan.md), [NEO-51](../../plans/NEO-51-implementation-plan.md), [NEO-52](../../plans/NEO-52-implementation-plan.md), [NEO-53](../../plans/NEO-53-implementation-plan.md), [NEO-54](../../plans/NEO-54-implementation-plan.md), [NEO-55](../../plans/NEO-55-implementation-plan.md), [NEO-56](../../plans/NEO-56-implementation-plan.md), [NEO-72](../../plans/NEO-72-implementation-plan.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3M3-prototype-backlog](../../plans/E3M3-prototype-backlog.md), [E3_M3](E3_M3_ItemizationAndInventorySchema.md) | | E3.M2 | Ready | **NEO-65 landed:** frozen prototype eight-recipe catalog in [`content/recipes/prototype_recipes.json`](../../../content/recipes/prototype_recipes.json); [`recipe-def.schema.json`](../../../content/schemas/recipe-def.schema.json) + [`recipe-io-row.schema.json`](../../../content/schemas/recipe-io-row.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) Slice 3 gates ([NEO-65](../../plans/NEO-65-implementation-plan.md)). **NEO-66 landed:** fail-fast server load of `content/recipes/*_recipes.json` at startup — `server/NeonSprawl.Server/Game/Crafting/` ([NEO-66](../../plans/NEO-66-implementation-plan.md)); [server README — Recipe catalog](../../../server/README.md#recipe-catalog-contentrecipes-neo-66). **NEO-67 landed:** injectable **`IRecipeDefinitionRegistry`** + lookup tests ([NEO-67](../../plans/NEO-67-implementation-plan.md)). **NEO-68 landed:** **`GET /game/world/recipe-definitions`** — `RecipeDefinitionsWorldApi` + DTOs in `Game/Crafting/` ([NEO-68](../../plans/NEO-68-implementation-plan.md), [`NEO-68` manual QA](../../manual-qa/NEO-68.md)); [server README — Recipe definitions (NEO-68)](../../../server/README.md#recipe-definitions-neo-68); Bruno `bruno/neon-sprawl-server/recipe-definitions/`. **NEO-69 landed:** **`CraftOperations.TryCraft`** + **`CraftResult`** — pre-flight inputs/outputs, inventory mutation, refine XP ([NEO-69](../../plans/NEO-69-implementation-plan.md)); [server README — Craft engine (NEO-69)](../../../server/README.md#craft-engine-neo-69-and-craft-http-neo-70). **NEO-70 landed:** **`POST /game/players/{id}/craft`** — `PlayerCraftApi` + wire **`CraftResponse`**; Bruno gather→refine→make spine ([NEO-70](../../plans/NEO-70-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/craft/`. **NEO-71 landed:** comment-only **`item_crafted`** / **`craft_failed`** telemetry hook sites in **`CraftOperations.TryCraft`** ([NEO-71](../../plans/NEO-71-implementation-plan.md)); [server README — Craft telemetry hooks (NEO-71)](../../../server/README.md#craft-telemetry-hooks-neo-71). **NEO-42 landed (prep):** **`RefineActivitySkillXpGrant`** / **`RefineSkillXpConstants`** — wired from craft engine success path. Epic 3 Slice 3 server backlog **E3M2-01**–**E3M2-07** complete. Upstream: E3.M1 **Ready**, E3.M3 inventory landed. **Slice 5 client (planned):** craft UI [NEO-74](https://linear.app/neon-sprawl/issue/NEO-74); capstone playable loop [NEO-75](https://linear.app/neon-sprawl/issue/NEO-75) — [E3S5 backlog](../../plans/E3S5-client-prototype-backlog.md). | [NEO-42](../../plans/NEO-42-implementation-plan.md), [NEO-65](../../plans/NEO-65-implementation-plan.md), [NEO-66](../../plans/NEO-66-implementation-plan.md), [NEO-67](../../plans/NEO-67-implementation-plan.md), [NEO-68](../../plans/NEO-68-implementation-plan.md), [NEO-69](../../plans/NEO-69-implementation-plan.md), [NEO-70](../../plans/NEO-70-implementation-plan.md), [NEO-71](../../plans/NEO-71-implementation-plan.md) … [NEO-75](https://linear.app/neon-sprawl/issue/NEO-75), [E3M2-prototype-backlog](../../plans/E3M2-prototype-backlog.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3_M2](E3_M2_RefinementAndRecipeExecution.md) | --- diff --git a/docs/reviews/2026-05-24-NEO-73.md b/docs/reviews/2026-05-24-NEO-73.md index d3c6615..752cfdc 100644 --- a/docs/reviews/2026-05-24-NEO-73.md +++ b/docs/reviews/2026-05-24-NEO-73.md @@ -2,11 +2,12 @@ **Date:** 2026-05-24 **Scope:** Branch `NEO-73-e3m1-client-gather-feedback-on-interact` · commits `4682db4`–`b52428e` vs `origin/main` -**Base:** `origin/main` +**Base:** `origin/main` +**Follow-up:** Review feedback addressed in follow-up commit (interaction failure signal, docs, tests). ## Verdict -**Approve with nits** — implementation matches the adopted plan and E3S5-02 acceptance criteria; GdUnit suites pass. Non-blocking gaps: alignment register still lists NEO-73 as planned, and `_pending_gather_feedback` can stick on interact HTTP failures until a later inventory refresh. +**Approve** — implementation matches the adopted plan and E3S5-02 acceptance criteria; GdUnit suites pass. Follow-up cleared pending-gather flag on interact HTTP failures and updated alignment/module docs. ## Summary @@ -18,11 +19,11 @@ The branch adds **`skill_progression_client.gd`**, **`prototype_interactable_pic |----------|--------| | [`docs/plans/NEO-73-implementation-plan.md`](../plans/NEO-73-implementation-plan.md) | **Matches** — kickoff decisions (nearest **R**, inventory delta copy, dedicated skill label) reflected; acceptance checklist and reconciliation complete. | | [`docs/plans/E3S5-client-prototype-backlog.md`](../plans/E3S5-client-prototype-backlog.md) · **E3S5-02** | **Matches** — acceptance criteria checked; scope/out-of-scope honored. | -| [`docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md`](../decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md) | **Partially matches** — cites NEO-73 as Slice 5 client backlog but no “landed” client note yet (plan §8 deferred to merge). | -| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Partially matches** — E3.M1 row still says **Slice 5 client (planned): gather feedback HUD NEO-73**; should flip to **landed** on merge. E3.M3 row still lists NEO-73 under “remaining.” | +| [`docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md`](../decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md) | **Matches** — NEO-73 client slice **landed** note added. | +| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E3.M1 row notes NEO-73 landed; NEO-73 removed from E3.M3 Slice 5 remaining list. | | [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — client displays outcomes; gather resolution stays server-side via **`POST …/interact`**. | | [`docs/manual-qa/NEO-73.md`](../manual-qa/NEO-73.md) | **Matches** — four-node **R** walkthrough, depletion deny, terminal does not overwrite gather line, no manual **I**. | -| [`client/README.md`](../../client/README.md) gather feedback section | **Matches** — endpoints, **R** nearest-node rule, HUD labels, boot hydrate. Automated-tests **Scope** bullet still omits NEO-73 suites (see Suggestions). | +| [`client/README.md`](../../client/README.md) gather feedback section | **Matches** — endpoints, **R** nearest-node rule, HUD labels, boot hydrate; automated test Scope lists NEO-73 suites. | | Full-stack epic decomposition | **Matches** — E3S5-02 **client** counterpart for server NEO-63; Godot manual QA present; does not claim Slice 5 capstone (NEO-75) complete. | ## Blocking issues @@ -31,17 +32,17 @@ The branch adds **`skill_progression_client.gd`**, **`prototype_interactable_pic ## Suggestions -1. **Update alignment register + E3.M1 module note on merge** — Plan §8 explicitly deferred [`documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) E3.M1 row and E3.M1 client slice note until implementation completes. Add a **NEO-73 landed** clause (gather HUD, **`skill_progression_client`**, four-node **R** picker) and move NEO-73 off E3.M3 “remaining Slice 5” list. +1. ~~**Update alignment register + E3.M1 module note on merge** — Plan §8 explicitly deferred [`documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) E3.M1 row and E3.M1 client slice note until implementation completes. Add a **NEO-73 landed** clause (gather HUD, **`skill_progression_client`**, four-node **R** picker) and move NEO-73 off E3.M3 “remaining Slice 5” list.~~ **Done.** -2. **Clear `_pending_gather_feedback` when interact HTTP fails** — `InteractionRequestClient` only emits **`interaction_result_received`** on parsed **200**; non-200 paths print and return. `main.gd` sets **`_pending_gather_feedback = true`** in **`_forward_interact_resource_nearest`** before POST. If the request fails (500, malformed body, transport error), the flag stays set and a later **`inventory_received`** (e.g. manual **I**) can run **`_finalize_pending_gather_feedback`** with a stale **`_gather_pre_scrap_qty`**, painting a spurious gather delta. Clear the flag on non-200 (new signal or connect to a failure hook) or reset in **`_on_interaction_result_for_gather`** only after a matching success/deny **200**. +2. ~~**Clear `_pending_gather_feedback` when interact HTTP fails** — `InteractionRequestClient` only emits **`interaction_result_received`** on parsed **200**; non-200 paths print and return. `main.gd` sets **`_pending_gather_feedback = true`** in **`_forward_interact_resource_nearest`** before POST. If the request fails (500, malformed body, transport error), the flag stays set and a later **`inventory_received`** (e.g. manual **I**) can run **`_finalize_pending_gather_feedback`** with a stale **`_gather_pre_scrap_qty`**, painting a spurious gather delta. Clear the flag on non-200 (new signal or connect to a failure hook) or reset in **`_on_interaction_result_for_gather`** only after a matching success/deny **200**.~~ **Done.** **`interaction_request_failed`** signal + **`_on_interaction_request_failed_for_gather`** clears pending flag. -3. **`client/README.md` automated tests Scope** — Add **`skill_progression_client.gd`**, **`prototype_interactable_picker.gd`**, and extended **`interaction_request_client_test.gd`** to the Scope bullet list (same discoverability fix as NEO-72). +3. ~~**`client/README.md` automated tests Scope** — Add **`skill_progression_client.gd`**, **`prototype_interactable_picker.gd`**, and extended **`interaction_request_client_test.gd`** to the Scope bullet list (same discoverability fix as NEO-72).~~ **Done.** -4. **GdUnit: deny path for gather refresh** — `gather_feedback_refresh_test.gd` only asserts the allow → dual-refresh path. A deny mock asserting **zero** inventory/skill sync calls would lock the “no refresh on **`node_depleted`**” contract. +4. ~~**GdUnit: deny path for gather refresh** — `gather_feedback_refresh_test.gd` only asserts the allow → dual-refresh path. A deny mock asserting **zero** inventory/skill sync calls would lock the “no refresh on **`node_depleted`**” contract.~~ **Done.** ## Nits -- Nit: **`Gather: ok — inventory refresh failed`** (`main.gd` **`_on_inventory_sync_failed`**) reads as success; consider **`Gather: inventory refresh failed`** for clarity. +- ~~Nit: **`Gather: ok — inventory refresh failed`** (`main.gd` **`_on_inventory_sync_failed`**) reads as success; consider **`Gather: inventory refresh failed`** for clarity.~~ **Done.** - Nit: Rapid **R** while a gather POST is in flight overwrites **`_gather_pre_scrap_qty`**; delta can be wrong if two responses complete out of order. Prototype-acceptable; last-wins POST coalescing already exists on the interaction client. - Nit: **`KNOWN_RESOURCE_NODE_IDS`** in **`prototype_interactable_picker.gd`** duplicates the frozen four-node table — intentional catalog-empty fallback; keep in sync with **`E3_M1`** if ids ever change (they are frozen). @@ -58,7 +59,7 @@ godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreH -a res://test/interaction_request_client_test.gd ``` -**14/14** passed; no `PackedStringArray`/`PackedByteArray` header conversion errors on **`skill_progression_client.gd`**. +**16/16** passed; no `PackedStringArray`/`PackedByteArray` header conversion errors on **`skill_progression_client.gd`** (includes deny + HTTP failure tests after follow-up). Manual (required before merge — real **`HTTPRequest`** + four nodes):