From b03128a3b0b3ba951320e0a8bcbff80590f2cfb8 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 30 May 2026 17:05:30 -0400 Subject: [PATCH] =?UTF-8?q?NEO-98:=20address=20code=20review=20=E2=80=94?= =?UTF-8?q?=20Esc=20regression=20step=20and=20AC=20parity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- client/README.md | 4 ++-- .../modules/E5_M2_NpcAiAndBehaviorProfiles.md | 2 +- docs/manual-qa/NEO-98.md | 10 ++++++---- docs/plans/E5M2-prototype-backlog.md | 1 + docs/reviews/2026-05-30-NEO-98.md | 12 ++++++------ 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/client/README.md b/client/README.md index 45ff794..01da5c4 100644 --- a/client/README.md +++ b/client/README.md @@ -172,11 +172,11 @@ Epic 5 Slice 2 capstone — aggro, readable telegraph, incoming NPC damage, and | Step | Input / trigger | HUD / server outcome | |------|-----------------|----------------------| | Boot | Godot **F5** + fresh server | Hotbar slot 0 binds **`prototype_pulse`**; player combat HP **100/100** when poll starts | -| Melee lock | **Tab** on **`prototype_npc_melee`** | Marker brightens; **`CombatTargetHpLabel`** **`100/100`** | +| Melee lock | **Tab** on **`prototype_npc_melee`** | Marker brightens; **`CombatTargetHpLabel`** **`Target HP: prototype_npc_melee 100/100`** | | Melee aggro | **1** (damaging cast) | **`NpcStateLabel`** → **`aggro`**; combat poll ~1 Hz | | Melee telegraph | Wait **≥ 3 s** | **`telegraph_windup`** + **`TelegraphLabel`** countdown | | Melee hit | Wait **≥ 4.5 s** from first cast | **`PlayerCombatHpLabel`** → **`85/100`** (15 damage) | -| Melee defeat | **1** ×4 total (cooldown between) | **`CombatTargetHpLabel`** **`0/100 (defeated)`** | +| Melee defeat | **1** ×4 total (cooldown between) | **`CombatTargetHpLabel`** includes **`0/100 (defeated)`** | | Ranged | Lock + cast ×4; wait **≥ 4 s** for telegraph | Defeat at **80** HP; telegraph visible before damage | | Elite | Lock + cast ×8; wait **≥ 5 s** for telegraph | Defeat at **200** HP; telegraph visible before damage | diff --git a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md index 9e03b90..194d066 100644 --- a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md +++ b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md @@ -8,7 +8,7 @@ | **Epic** | [Epic 5 — PvE Combat](../epics/epic_05_pve_combat.md) | | **Stage target** | Prototype | | **Status** | Ready — Slice 2 backlog [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md): **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) catalog + CI landed · **E5M2-02** [NEO-88](https://linear.app/neon-sprawl/issue/NEO-88) server load landed · **E5M2-03** [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) registry + DI landed · **E5M2-04** [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) behavior-definitions GET landed · **E5M2-05** [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) NPC instance registry + combat-target migration landed · **E5M2-06** [NEO-92](https://linear.app/neon-sprawl/issue/NEO-92) aggro rule + threat store landed · **E5M2-07** [NEO-93](https://linear.app/neon-sprawl/issue/NEO-93) NPC runtime state machine landed · **E5M2-08** [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) runtime snapshot GET landed · **E5M2-09** [NEO-95](https://linear.app/neon-sprawl/issue/NEO-95) NPC attack resolve + player combat HP landed · **E5M2-10** [NEO-96](https://linear.app/neon-sprawl/issue/NEO-96) NPC runtime telemetry hooks landed · **E5M2-11** [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) client telegraph HUD landed · **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) playable capstone landed | -| **Linear** | Label **`E5.M2`** · [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md) **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) · **E5M2-02** [NEO-88](https://linear.app/neon-sprawl/issue/NEO-88) · **E5M2-03** [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) · **E5M2-04** [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) · **E5M2-05** [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) → **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) | +| **Linear** | Label **`E5.M2`** · [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md) **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) · **E5M2-02** [NEO-88](https://linear.app/neon-sprawl/issue/NEO-88) · **E5M2-03** [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) · **E5M2-04** [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) · **E5M2-05** [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) · **E5M2-06** [NEO-92](https://linear.app/neon-sprawl/issue/NEO-92) · **E5M2-07** [NEO-93](https://linear.app/neon-sprawl/issue/NEO-93) · **E5M2-08** [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) · **E5M2-09** [NEO-95](https://linear.app/neon-sprawl/issue/NEO-95) · **E5M2-10** [NEO-96](https://linear.app/neon-sprawl/issue/NEO-96) · **E5M2-11** [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) · **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) | ## Purpose diff --git a/docs/manual-qa/NEO-98.md b/docs/manual-qa/NEO-98.md index 545e4f0..cc82bf4 100644 --- a/docs/manual-qa/NEO-98.md +++ b/docs/manual-qa/NEO-98.md @@ -13,6 +13,7 @@ - **Fresh dev player:** stop any running server, then start a new instance so in-memory **`dev-local-1`** NPC HP, aggro/runtime rows, and player combat HP reset. - **No Bruno/curl seeding** for this checklist — hotbar **`prototype_pulse`** bind is automatic (NEO-85 dev bootstrap). - NEO-97 telegraph HUD landed on `main` (PR #136). +- **`CombatTargetHpLabel`** uses NEO-85 format **`Target HP: {id} {current}/{max}`** (e.g. **`Target HP: prototype_npc_melee 100/100`**) — match HP substring or full line. ## Archetype reference @@ -44,7 +45,7 @@ Player casts **`prototype_pulse`** (**25** damage, ~**3 s** cooldown). ### Melee — full telegraph + player HP spine 3. Walk to orange **Melee** marker west of spawn **`(-3, -3)`**. -4. Press **Tab** until **`prototype_npc_melee`** is locked with **`Validity: ok`** — marker **brightens**; **`CombatTargetHpLabel`** shows **`100/100`**. +4. Press **Tab** until **`prototype_npc_melee`** is locked with **`Validity: ok`** — marker **brightens**; **`CombatTargetHpLabel`** shows **`Target HP: prototype_npc_melee 100/100`** (or **`…`** briefly until GET completes). 5. Press **1** (damaging cast): **`CastFeedbackLabel`** shows damage; **`NpcStateLabel`** eventually shows **`NPC state: Melee → aggro`** (~1 Hz poll). 6. **`PlayerCombatHpLabel`** shows **`Player HP: 100/100`** once combat poll starts. 7. Wait **≥ 3 s** from first cast: **`NpcStateLabel`** shows **`telegraph_windup`**; **`TelegraphLabel`** shows countdown (e.g. **`Telegraph: Melee … · ~1.x s (melee)`**) ticking between polls. @@ -68,8 +69,9 @@ Player casts **`prototype_pulse`** (**25** damage, ~**3 s** cooldown). ### Regression -19. Cast without lock still shows **`ability_cast_denied: invalid_target`** on **`CastFeedbackLabel`** (NEO-28). -20. **`CombatTargetHpLabel`** + cooldown HUD still refresh after accepted casts (NEO-85 / NEO-32). +19. Press **Esc** to clear lock: **`CombatTargetHpLabel`** shows **`Target HP: — (Tab → elite/melee/ranged near spawn)`** (or equivalent no-lock line). +20. Cast without lock: **`CastFeedbackLabel`** shows **`ability_cast_denied: invalid_target`** (NEO-28). +21. **`CombatTargetHpLabel`** + cooldown HUD still refresh after accepted casts (NEO-85 / NEO-32) — re-lock any NPC and press **1** once to confirm. ## Notes @@ -80,7 +82,7 @@ Player casts **`prototype_pulse`** (**25** damage, ~**3 s** cooldown). ## Acceptance -- [ ] Steps 1–20 completable in one session without Bruno/curl. +- [ ] Steps 1–21 completable in one session without Bruno/curl. - [ ] Melee telegraph countdown readable **before** player HP drops to **85/100** (steps 7–8). - [ ] All three archetypes show **`defeated`** in **`CombatTargetHpLabel`** (steps 9, 14, 18). - [ ] Aggro on first damaging cast feels deterministic (no silent failures). diff --git a/docs/plans/E5M2-prototype-backlog.md b/docs/plans/E5M2-prototype-backlog.md index 5bc5ed1..09c77bb 100644 --- a/docs/plans/E5M2-prototype-backlog.md +++ b/docs/plans/E5M2-prototype-backlog.md @@ -377,6 +377,7 @@ Working backlog for **Epic 5 — Slice 2** ([NPC archetypes and telegraphs](../d - [x] Human completes script with server + client; telegraph timing readable before damage. - [x] Epic 5 Slice 2 AC: telegraph timing matches UI feedback; aggro rules feel deterministic in manual QA. +- [x] All three archetypes defeated in one session; **`CombatTargetHpLabel`** shows defeated for each (manual QA steps 9, 14, 18). - [x] Re-read [epic_05 Definition of Done](../decomposition/epics/epic_05_pve_combat.md#definition-of-done) for prototype minimums. **Landed ([NEO-98](https://linear.app/neon-sprawl/issue/NEO-98)):** playable NPC telegraph combat capstone — [`NEO-98` manual QA](../manual-qa/NEO-98.md); `client/README.md` end-to-end NPC telegraph combat loop section; plan [NEO-98-implementation-plan.md](NEO-98-implementation-plan.md). **Epic 5 Slice 2 client capstone complete.** diff --git a/docs/reviews/2026-05-30-NEO-98.md b/docs/reviews/2026-05-30-NEO-98.md index 801b2f2..ae7bb23 100644 --- a/docs/reviews/2026-05-30-NEO-98.md +++ b/docs/reviews/2026-05-30-NEO-98.md @@ -17,7 +17,7 @@ NEO-98 closes **E5M2-12** and Epic 5 Slice 2 client acceptance with a **docs-onl | Path | Result | |------|--------| | `docs/plans/NEO-98-implementation-plan.md` | **Matches** — kickoff decisions (docs-primary, fresh restart, Melee→Ranged→Elite, melee full / ranged+elite light, NEO-97 as component regression), scope, file list, and reconciliation align with the diff; no integration fixes shipped. | -| `docs/plans/E5M2-prototype-backlog.md` (E5M2-12) | **Partially matches** — landed note and three AC items checked; plan’s fourth AC (“all three archetypes defeated”) is absent from backlog AC list (see Suggestions). | +| `docs/plans/E5M2-prototype-backlog.md` (E5M2-12) | **Matches** — landed note and four AC items checked (includes three-archetype defeat). | | `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | **Matches** — Status **Ready** with E5M2-11 + E5M2-12 landed; capstone cross-link + manual QA pointer added. | | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E5.M2 row **Ready**, **NEO-98 landed**, Slice 2 client capstone complete. | | `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E5.M2 note summarizes NEO-92–NEO-98 and marks Slice 2 complete. | @@ -32,15 +32,15 @@ None. ## Suggestions -1. **Regression step — clear lock before `invalid_target` check** — After step 18 the tester is still locked on defeated **`prototype_npc_elite`**; step 19 (“cast without lock”) will hit **`target_defeated`**, not **`invalid_target`**, unless lock is cleared first. Add an **Esc** step (mirror [NEO-86 manual QA](../manual-qa/NEO-86.md) steps 8–10) before the NEO-28 regression assertion. +1. ~~**Regression step — clear lock before `invalid_target` check** — After step 18 the tester is still locked on defeated **`prototype_npc_elite`**; step 19 (“cast without lock”) will hit **`target_defeated`**, not **`invalid_target`**, unless lock is cleared first. Add an **Esc** step (mirror [NEO-86 manual QA](../manual-qa/NEO-86.md) steps 8–10) before the NEO-28 regression assertion.~~ **Done.** Manual QA step 19 **Esc** clear; NEO-28 check is step 20. -2. **E5M2-12 backlog AC parity** — [NEO-98 implementation plan](../plans/NEO-98-implementation-plan.md) lists a fourth AC (“All three archetypes defeated… **`CombatTargetHpLabel`** shows defeated for each”); [E5M2-prototype-backlog.md](../plans/E5M2-prototype-backlog.md) E5M2-12 section omits it. Add the checkbox for traceability with the plan and manual QA steps 9/14/18. +2. ~~**E5M2-12 backlog AC parity** — [NEO-98 implementation plan](../plans/NEO-98-implementation-plan.md) lists a fourth AC (“All three archetypes defeated… **`CombatTargetHpLabel`** shows defeated for each”); [E5M2-prototype-backlog.md](../plans/E5M2-prototype-backlog.md) E5M2-12 section omits it. Add the checkbox for traceability with the plan and manual QA steps 9/14/18.~~ **Done.** ## Nits -- Nit: **E5.M2 Summary `Linear` row** still abbreviates **`E5M2-05 … → E5M2-12`** while the **Status** row now lists E5M2-06–E5M2-11 inline — optional consistency pass. +- ~~Nit: **E5.M2 Summary `Linear` row** still abbreviates **`E5M2-05 … → E5M2-12`** while the **Status** row now lists E5M2-06–E5M2-11 inline — optional consistency pass.~~ **Done.** Linear row lists E5M2-01–E5M2-12 inline. -- Nit: Manual QA / README use shorthand **`100/100`** on **`CombatTargetHpLabel`**; NEO-85 format includes **`Target HP: {id}`** prefix — testers should match substring, not exact label text (or tighten one example step to full format). +- ~~Nit: Manual QA / README use shorthand **`100/100`** on **`CombatTargetHpLabel`**; NEO-85 format includes **`Target HP: {id}`** prefix — testers should match substring, not exact label text (or tighten one example step to full format).~~ **Done.** Preconditions note + step 4 full format; README flow table updated. - Nit: Plan / backlog AC boxes are **`[x]`** while **`docs/manual-qa/NEO-98.md` Acceptance** stays **`[ ]`** until a human run — intentional per NEO-86; keep that distinction when pasting PR text. @@ -51,6 +51,6 @@ None. cd server/NeonSprawl.Server.Tests && dotnet test --filter "FullyQualifiedName~NpcRuntime" ``` -**Manual (required before merge):** [`docs/manual-qa/NEO-98.md`](../manual-qa/NEO-98.md) — fresh server restart, Godot **F5**, Melee steps 3–10 (telegraph visible **before** **85/100**), Ranged + Elite defeat with telegraph visible, regression steps 19–20. +**Manual (required before merge):** [`docs/manual-qa/NEO-98.md`](../manual-qa/NEO-98.md) — fresh server restart, Godot **F5**, Melee steps 3–10 (telegraph visible **before** **85/100**), Ranged + Elite defeat with telegraph visible, regression steps 19–21 (**Esc** before NEO-28 deny). **Optional component regression:** [`docs/manual-qa/NEO-97.md`](../manual-qa/NEO-97.md) — elite incoming-threat row when locking a different NPC while elite holds aggro.