@ -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.
**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.
**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.
**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
## Summary
@ -18,11 +19,11 @@ The branch adds **`skill_progression_client.gd`**, **`prototype_interactable_pic
| 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. |
| 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
## Blocking issues
@ -31,17 +32,17 @@ The branch adds **`skill_progression_client.gd`**, **`prototype_interactable_pic
## Suggestions
## 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
## 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: 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).
- 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).
