From ba75aeed79d01d061ee28711debd9548fb98c614 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 7 Jun 2026 13:36:21 -0400 Subject: [PATCH] NEO-122: Address follow-up review doc drift and QA numbering. Reconcile plan with economy HUD layout refactor, fix manual QA steps, update regression notes for NEO-74/75/110, and strike through resolved review suggestions. --- client/README.md | 2 +- docs/manual-qa/NEO-110.md | 2 +- docs/manual-qa/NEO-122.md | 10 ++++++---- docs/manual-qa/NEO-74.md | 5 +++-- docs/manual-qa/NEO-75.md | 4 ++-- docs/plans/NEO-122-implementation-plan.md | 15 ++++++++++++++- docs/reviews/2026-06-07-NEO-122-follow-up.md | 15 ++++++++------- 7 files changed, 35 insertions(+), 18 deletions(-) diff --git a/client/README.md b/client/README.md index 0942103..32870b0 100644 --- a/client/README.md +++ b/client/README.md @@ -369,7 +369,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), **`skill_progression_client.gd`**, **`gig_progression_client.gd`** (NEO-86), **`quest_progress_client.gd`**, **`quest_definitions_client.gd`** (NEO-122), **`prototype_interactable_picker.gd`**, extended **`interaction_request_client_test.gd`**, **`gather_feedback_refresh_test.gd`** (NEO-73), **`craft_client.gd`**, **`recipe_definitions_client.gd`**, **`craft_feedback_refresh_test.gd`** (NEO-74), **`prototype_economy_hud_section.gd`** (NEO-75), **`combat_targets_client.gd`**, **`combat_feedback_refresh_test.gd`** (NEO-85), **`gig_feedback_refresh_test.gd`** (NEO-86), **`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`**, **`gig_progression_client.gd`** (NEO-86), **`quest_progress_client.gd`**, **`quest_definitions_client.gd`** (NEO-122), **`prototype_interactable_picker.gd`**, extended **`interaction_request_client_test.gd`**, **`gather_feedback_refresh_test.gd`** (NEO-73), **`craft_client.gd`**, **`recipe_definitions_client.gd`**, **`craft_recipe_panel_test.gd`** (NEO-122 follow-up), **`craft_feedback_refresh_test.gd`** (NEO-74), **`prototype_economy_hud_section.gd`** (NEO-75), **`combat_targets_client.gd`**, **`combat_feedback_refresh_test.gd`** (NEO-85), **`gig_feedback_refresh_test.gd`** (NEO-86), **`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`**. **`isometric_follow_camera.gd`** also maps **macOS trackpad** two-finger scroll and pinch to the same discrete zoom bands using **accumulated delta** (one band step per ~1.25 pan units, **150 ms** cooldown between trackpad steps) so small gestures do not traverse all bands at once. On layouts where **`+`** requires **Shift**, remap or add a binding under **Project → Project Settings → Input Map** if needed. diff --git a/docs/manual-qa/NEO-110.md b/docs/manual-qa/NEO-110.md index 1164e13..eef18fa 100644 --- a/docs/manual-qa/NEO-110.md +++ b/docs/manual-qa/NEO-110.md @@ -37,7 +37,7 @@ Player casts **`prototype_pulse`** (**25** damage, ~**3 s** cooldown). ## Checklist 1. Start server: `cd server/NeonSprawl.Server && dotnet run`. -2. Run Godot main scene (**F5**). Confirm boot lines: **`Encounter: … not started (0/3)`** and **`Loot: —`** on **`EncounterProgressLabel`** / **`EncounterCompleteLabel`** (combat HUD block, below **`PlayerCombatHpLabel`**). +2. Run Godot main scene (**F5**). Confirm boot lines: **`Encounter: … not started (0/3)`** and **`Loot: —`** on **`EncounterProgressLabel`** / **`EncounterCompleteLabel`** (in **`HudRootScroll`**, below **`PlayerCombatHpLabel`**). Scroll **`HudRootScroll`** if the debug stack is taller than the viewport. 3. Defeat **`prototype_npc_melee`**: walk to orange marker, **Tab** lock, press **1** until **`Cast: … — defeated!`**. Verify **`EncounterProgressLabel`** shows **`1/3`** (may lag one GET behind cast — wait ~1 s or defeat next NPC). 4. Defeat **`prototype_npc_ranged`**: lock at SE marker, cast until defeated. Verify **`2/3`**. 5. Defeat **`prototype_npc_elite`**: lock at origin, cast until defeated (**8** pulses). Verify **`completed (3/3)`** and **`EncounterCompleteLabel`** lists **`scrap_metal_bulk ×10`** and **`contract_handoff_token ×1`** (or raw ids if defs still loading). diff --git a/docs/manual-qa/NEO-122.md b/docs/manual-qa/NEO-122.md index 0eb29af..8cf94ba 100644 --- a/docs/manual-qa/NEO-122.md +++ b/docs/manual-qa/NEO-122.md @@ -29,7 +29,7 @@ ## Checklist 1. Start server: `cd server/NeonSprawl.Server && dotnet run`. -2. Run Godot main scene (**F5**). Confirm **`QuestProgressLabel`** shows four **`not started`** lines and **`QuestAcceptFeedbackLabel`** shows the idle hint (below encounter loot labels). +2. Run Godot main scene (**F5**). Confirm **`QuestProgressLabel`** shows four **`not started`** lines and **`QuestAcceptFeedbackLabel`** shows the idle hint (in **`HudRootScroll`**, below encounter loot labels). Economy panel pinned bottom-left (not off-screen). 3. Press **Q**. Verify gather intro becomes **`active step 1`** and accept feedback shows success. 4. Gather scrap until gather quest completes (resource node **R**). Verify counter updates on **`QuestProgressLabel`** after each successful gather (may lag one GET). 5. Press **Shift+Q** before gather is complete — verify **`Quest accept: denied — prerequisite_incomplete`** for refine (or **`no eligible quest`** if refine is not yet eligible). @@ -37,8 +37,9 @@ 7. Open **Economy HUD** recipe panel — **Refine Scrap (Standard)** is the **first** row (scroll starts at top). Craft when **≥5** scrap in bag. 8. Craft **`refine_scrap_standard`** with Economy HUD open. Verify refine quest **`completed`** after successful craft. 9. Press **Q** again on already-completed gather. Verify deny feedback on **`QuestAcceptFeedbackLabel`** without Bruno. -9. Optional: stop server while Godot running, press **Q** — verify **`Quests: error — …`** or **`Quest accept: failed — …`** visible on HUD. -10. Optional: block **`GET /game/world/quest-definitions`** only while progress GET succeeds — verify **`definitions error — …`** under **`QuestProgressLabel`** with raw quest ids as fallback. +10. Optional: stop server while Godot running, press **Q** — verify **`Quests: error — …`** or **`Quest accept: failed — …`** visible on HUD. +11. Optional: block **`GET /game/world/quest-definitions`** only while progress GET succeeds — verify **`definitions error — …`** under **`QuestProgressLabel`** with raw quest ids as fallback. +12. Toggle **Economy HUD** off/on — debug labels in **`HudRootScroll`** remain; recipe repopulate + scroll reset still works. ## Notes @@ -48,6 +49,7 @@ ## Acceptance -- [ ] Steps 1–8 completable in one session without Bruno/curl. +- [ ] Steps 1–9 completable in one session without Bruno/curl. - [ ] Quest status visible after **Q** accept and updates after gather/craft without manual refresh key. - [ ] Failed GET/accept shows visible HUD error copy. +- [ ] Economy HUD craft panel visible for step 7–8 (recipes first row, no overlap with quest labels). diff --git a/docs/manual-qa/NEO-74.md b/docs/manual-qa/NEO-74.md index 974808e..9db5b73 100644 --- a/docs/manual-qa/NEO-74.md +++ b/docs/manual-qa/NEO-74.md @@ -16,8 +16,8 @@ 1. Start server: `cd server/NeonSprawl.Server && dotnet run`. 2. Run Godot main scene (**F5**). Confirm **`SkillProgressionLabel`** shows **`salvage`** and **`refine`** rows after boot. -3. Scroll the recipe panel below skills — **eight** prototype recipes with **Craft** buttons. -4. On a fresh player (empty bag), click **Craft** on **`refine_scrap_standard`**. **`CraftFeedbackLabel`** shows **`Craft: denied — insufficient_materials`**. Press **I** — inventory unchanged. +3. Scroll the recipe panel in **Economy HUD** (`BodyScroll/Body/CraftRecipePanel`) — **eight** prototype recipes with **Craft** buttons. **Refine Scrap (Standard)** is the first row (NEO-122 layout). +4. On a fresh player (empty bag), click **Craft** on **`refine_scrap_standard`**. **`CraftFeedbackLabel`** (in **`HudRootScroll`**) shows **`Craft: denied — insufficient_materials`**. Press **I** — inventory unchanged. 5. Gather **≥5** **`scrap_metal_bulk`** (see [NEO-73 manual QA](NEO-73.md) **R** at resource nodes). 6. Click **Craft** on **`refine_scrap_standard`**. **`CraftFeedbackLabel`** shows **`Craft: +1 …`** (refined plate output). **`InventoryLabel`** updates without **I**. **`refine`** xp increases on **`SkillProgressionLabel`**. 7. Gather **≥1** more scrap if needed; ensure **≥2** **`refined_plate_stock`** and **≥1** **`scrap_metal_bulk`** in bag (craft **`refine_scrap_standard`** again or gather). @@ -28,3 +28,4 @@ - Craft quantity is fixed at **1** per button (no spinner in this story). - Success copy comes from POST **`outputsGranted`**; inventory and skill rows refresh via GET after success only. +- **NEO-122 layout:** debug feedback labels live in **`HudRootScroll`**; economy block (recipes, inventory, skills) is pinned bottom-left — scroll economy **`BodyScroll`** for inventory/skills below recipes. diff --git a/docs/manual-qa/NEO-75.md b/docs/manual-qa/NEO-75.md index d7c43b0..32c9d16 100644 --- a/docs/manual-qa/NEO-75.md +++ b/docs/manual-qa/NEO-75.md @@ -26,13 +26,13 @@ ## Checklist 1. Start server: `cd server/NeonSprawl.Server && dotnet run`. -2. Run Godot main scene (**F5**). Confirm **`Economy HUD`** toggle is **on**; **`InventoryLabel`** shows empty bag; **`SkillProgressionLabel`** shows **`salvage`** and **`refine`** rows. +2. Run Godot main scene (**F5**). Confirm **`Economy HUD`** toggle is **on**; **`InventoryLabel`** shows empty bag; **`SkillProgressionLabel`** shows **`salvage`** and **`refine`** rows. Scroll **`BodyScroll`** if inventory/skills are below the recipe list (NEO-122 recipes-first order). 3. **WASD** to **`prototype_resource_node_alpha`** anchor **(12, −6)**. Press **R** once. **`GatherFeedbackLabel`** shows **`+1`** scrap delta; inventory scrap updates without **I**; **`salvage`** xp increases. 4. Walk to **`prototype_subsurface_vein_beta`** **(18, −6)**; press **R** (**+2** scrap). Repeat at **`prototype_bio_mat_gamma`** **(12, 0)** (**+3**) and **`prototype_urban_bulk_delta`** **(18, 0)** (**+5**). Inventory shows **11** scrap total. 5. In the recipe panel, click **Craft** on **`refine_scrap_standard`**. **`CraftFeedbackLabel`** shows success; inventory shows **6** scrap and **1** **`refined_plate_stock`**; **`refine`** xp increases. 6. Click **Craft** on **`refine_scrap_standard`** again. Inventory shows **1** scrap and **2** plates. 7. Click **Craft** on **`make_field_stim_mk0`**. Success feedback shows **`field_stim_mk0`** output; **`InventoryLabel`** lists **`Field Stim Mk0`** (or agreed display name) with quantity **≥ 1**; **`refine`** xp increases again. -8. Uncheck **`Economy HUD`** — inventory, skills, and recipe panel hide; gather/craft feedback labels above remain visible. Check **`Economy HUD`** again — panel repopulates with current data. +8. Uncheck **`Economy HUD`** — inventory, skills, and recipe panel hide; gather/craft feedback labels in **`HudRootScroll`** remain visible. Check **`Economy HUD`** again — panel repopulates with current data. ## Notes diff --git a/docs/plans/NEO-122-implementation-plan.md b/docs/plans/NEO-122-implementation-plan.md index 0c1057f..759c3a3 100644 --- a/docs/plans/NEO-122-implementation-plan.md +++ b/docs/plans/NEO-122-implementation-plan.md @@ -58,7 +58,7 @@ - [x] Player sees quest status change after accept without Bruno. - [x] Objective progress updates after gather/craft/encounter in Godot session. - [x] Failed GET/accept surfaces visible HUD error (match interaction client pattern). -- [ ] `docs/manual-qa/NEO-122.md` Godot steps exercisable (human QA pending). +- [x] `docs/manual-qa/NEO-122.md` Godot steps exercisable (human QA passed 2026-06-07). ## Implementation reconciliation (shipped) @@ -69,6 +69,17 @@ - **Tests:** `client/test/quest_progress_client_test.gd` (12 cases across both suites), `client/test/quest_definitions_client_test.gd`. - **Docs:** `client/README.md` quest HUD section; `docs/manual-qa/NEO-122.md`. +### NEO-122 follow-up (economy HUD layout — manual QA step 7) + +During NEO-122 QA, **`refine_scrap_standard`** was unreachable when the economy block sat below the viewport inside the tall **`HudRoot`** stack. Follow-up commits (post–first review) refactored prototype HUD layout **without changing quest contracts**: + +- **`HudRootScroll`** — **`ScrollContainer`** wrapping debug/combat/quest labels; height clamped above the economy panel. +- **`EconomyHudSection`** — reparented to **`UICanvas`**; pinned bottom-left via **`prototype_economy_hud_section.gd`** **`_apply_viewport_layout`** (explicit **`position`/`size`**, opaque panel fill, **`z_index`**). +- **`BodyScroll/Body`** — single economy scroll; child order **Recipes → Inventory → Skills → Gig XP** so craft rows appear first. +- **`craft_recipe_panel.gd`** — flattened to **`VBoxContainer`** + **`RecipeRows`** (no nested scroll); populate resets economy body scroll to recipes via parent **`scroll_body_to_recipes()`**. +- **Tests:** `client/test/craft_recipe_panel_test.gd`; viewport layout tests in **`prototype_economy_hud_section_test.gd`**. +- **Regression:** spot-check [NEO-74](../manual-qa/NEO-74.md), [NEO-75](../manual-qa/NEO-75.md), [NEO-110](../manual-qa/NEO-110.md) after merge (collapsed economy toggle, feedback labels in **`HudRootScroll`**, encounter labels scroll when stack is tall). + ## Technical approach ### Server contract (landed — NEO-119 / NEO-120 / NEO-115) @@ -183,6 +194,8 @@ Debounce: skip if **`quest_progress_client`** accept busy (same as craft busy gu |-----------|----------------| | `client/test/quest_progress_client_test.gd` | Mock GET 200: **`not_started`** / **`active`** with **`objectiveCounters`** / **`completed`** rows; **`quest_row`**; GET 404 → **`quest_sync_failed`**; POST accept **`accepted: true`** and **`accepted: false`** + **`reasonCode`**. AAA layout (`# Arrange` / `# Act` / `# Assert`). | | `client/test/quest_definitions_client_test.gd` | Parse v1 defs JSON (four quests); **`display_name_for`** returns catalog **`displayName`**; unknown id fallback. AAA layout. | +| `client/test/craft_recipe_panel_test.gd` | Recipe row populate; economy body scroll reset on repopulate; **`craft_requested`** signal. AAA layout. | +| `client/test/prototype_economy_hud_section_test.gd` | Economy collapse toggle; viewport pin + **`HudRootScroll`** clamp (NEO-122 follow-up). AAA layout. | No new **C#** tests (client-only; server covered by **`QuestProgressApiTests`** / **`QuestAcceptApiTests`**). No Bruno changes. diff --git a/docs/reviews/2026-06-07-NEO-122-follow-up.md b/docs/reviews/2026-06-07-NEO-122-follow-up.md index f8a2ce7..0eb518a 100644 --- a/docs/reviews/2026-06-07-NEO-122-follow-up.md +++ b/docs/reviews/2026-06-07-NEO-122-follow-up.md @@ -3,7 +3,8 @@ **Date:** 2026-06-07 **Scope:** Branch `NEO-122-client-quest-progress-accept-hud` vs `origin/main` — commits `b36fe6d` … `632a747` (delta after [initial review](2026-06-07-NEO-122.md)) **Base:** `origin/main` -**Prior review:** `docs/reviews/2026-06-07-NEO-122.md` — prior suggestions **1** / **3** and nits **1–2** addressed in `b36fe6d`; suggestion **2** (extract quest HUD) still deferred. +**Prior review:** `docs/reviews/2026-06-07-NEO-122.md` — prior suggestions **1** / **3** and nits **1–2** addressed in `b36fe6d`; suggestion **2** (extract quest HUD) still deferred. +**Follow-up:** suggestions **1–2** below are **done** (strikethrough + **Done.**); suggestion **3** is manual regression spot-check for author/reviewer. ## Verdict @@ -17,10 +18,10 @@ Seven commits landed after the first review: code-review follow-up (quest-defini | Path | Result | |------|--------| -| `docs/plans/NEO-122-implementation-plan.md` | **Partially matches** — quest HUD reconciliation still accurate; **economy HUD layout refactor** (HudRootScroll, pinned economy panel, recipe panel restructure, recipe-first ordering) **not** recorded in plan reconciliation or kickoff scope. | -| `docs/manual-qa/NEO-122.md` | **Partially matches** — step 7 craft visibility added; **duplicate step 9** numbering (two lines labeled **9.**). | -| `client/README.md` | **Matches** quest + economy node paths (`HudRootScroll`, `BodyScroll/Body`); test-scope paragraph omits new **`craft_recipe_panel_test.gd`**. | -| `docs/manual-qa/NEO-74.md` / `NEO-75.md` / `NEO-110.md` | **Not updated** — no HudRootScroll / economy-pin notes; spot-check recommended (non-blocking). | +| `docs/plans/NEO-122-implementation-plan.md` | ~~**Partially matches**~~ **Matches** — NEO-122 follow-up reconciliation added (economy HUD layout refactor). **Done.** | +| `docs/manual-qa/NEO-122.md` | ~~**Partially matches**~~ **Matches** — duplicate step 9 fixed; acceptance includes craft visibility. **Done.** | +| `client/README.md` | ~~**Partially matches**~~ **Matches** — test-scope includes **`craft_recipe_panel_test.gd`**. **Done.** | +| `docs/manual-qa/NEO-74.md` / `NEO-75.md` / `NEO-110.md` | **Updated** — HudRootScroll / economy-pin regression notes added. **Done.** | | `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Matches** — unchanged since first review. | | Full-stack epic decomposition | **Matches** — still client slice only; NEO-123 capstone deferred. | @@ -30,9 +31,9 @@ Seven commits landed after the first review: code-review follow-up (quest-defini ## Suggestions -1. **Reconcile implementation plan** — Append a short **NEO-122 follow-up** bullet to `docs/plans/NEO-122-implementation-plan.md` **Implementation reconciliation**: economy HUD pinned layout, `HudRootScroll`, `craft_recipe_panel` VBox + `RecipeRows`, recipes-first in `BodyScroll/Body`, and manual QA step 7 rationale. Keeps [planning-implementation-docs](../../.cursor/rules/planning-implementation-docs.md) honest about scope expansion. +1. ~~**Reconcile implementation plan** — Append a short **NEO-122 follow-up** bullet to `docs/plans/NEO-122-implementation-plan.md` **Implementation reconciliation**: economy HUD pinned layout, `HudRootScroll`, `craft_recipe_panel` VBox + `RecipeRows`, recipes-first in `BodyScroll/Body`, and manual QA step 7 rationale. Keeps [planning-implementation-docs](../../.cursor/rules/planning-implementation-docs.md) honest about scope expansion.~~ **Done.** Follow-up subsection in plan reconciliation; tests table extended. -2. **Fix manual QA numbering** — In `docs/manual-qa/NEO-122.md`, renumber the second **9.** (server-down optional) to **10.** and the definitions-error optional to **11.**; update **Acceptance** if it should include the craft-visibility step (currently “Steps 1–8” while required steps now include **9**). +2. ~~**Fix manual QA numbering** — In `docs/manual-qa/NEO-122.md`, renumber the second **9.** (server-down optional) to **10.** and the definitions-error optional to **11.**; update **Acceptance** if it should include the craft-visibility step (currently “Steps 1–8” while required steps now include **9**).~~ **Done.** Steps 1–12; acceptance covers steps 1–9 + craft visibility. 3. **Regression spot-check** — Run abbreviated passes of [NEO-74](../manual-qa/NEO-74.md), [NEO-75](../manual-qa/NEO-75.md), and [NEO-110](../manual-qa/NEO-110.md) after merge: collapsed economy toggle, gather/craft feedback still visible in **`HudRootScroll`**, encounter labels still scroll when debug stack is tall.