don
/
neon-sprawl
mirror of https://github.com/ViPro-Technologies/neon-sprawl
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity
neon-sprawl/docs/reviews/2026-05-30-NEO-98-followup.md

6.1 KiB
Raw Permalink Blame History

Code review — NEO-98 follow-up (capstone QA integration fixes)

Date: 2026-05-30
Scope: Branch NEO-98-e5m2-12-playable-npc-telegraph-combat-capstone vs origin/main — commits b03128a … 44fb4ad (delta since 2026-05-30-NEO-98.md at 817ed26)
Base: origin/main

Verdict

Approve with nits

Summary

Since the first review, NEO-98 expanded beyond docs-only into capstone-driven server integration: defeated NPCs no longer keep attacking (TryStopOnTargetDefeat on cast accept + defeated guard in AdvanceOne), and telegraph damage is range-gated via new NPC attack abilities (content/abilities/prototype_npc_abilities.json, behavior attackAbilityId, NpcAttackOperations horizontal maxRange check). Content/schema/CI gates cross-reference behavior attackDamage against ability baseDamage. Manual QA and README document melee 2 m strike range and the whiff case. Follow-up doc suggestions (E5.M2 module freeze, plan scope reconciliation, alignment register, client README melee row, plan timing split) are addressed in a docs batch after this review.

Documentation checked

Path Result
docs/plans/NEO-98-implementation-plan.md Matches — kickoff Capstone QA scope expansion; revised Out of scope; reconciliation + files list cover defeat-stop and attack-ability range binding; melee spine splits ≥ 3 s telegraph / ≥ 4.5 s damage.
docs/manual-qa/NEO-98.md Matches — strike-range column, step 8 2 m proximity + whiff note, Esc regression (steps 19–21), full CombatTargetHpLabel format on step 4.
client/README.md Matches — flow table uses full target HP label; Melee hit row documents within ~2 m + whiff.
docs/plans/E5M2-prototype-backlog.md (E5M2-12) Matches — fourth AC (three-archetype defeat) present; landed note intact.
docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md Matches — freeze table includes attackAbilityId + NPC attack ability strike ranges; NEO-95/NEO-98 bullets document range-gated resolve and defeat-clear.
docs/decomposition/modules/documentation_and_implementation_alignment.md Matches — E5.M2 Ready + NEO-98 capstone + server integration clause.
docs/decomposition/modules/module_dependency_register.md Matches — E5.M2 note unchanged from docs batch; still cites NEO-98 capstone manual QA.
server/README.md Matches — threat table documents Attack range, Defeat clear, and attackAbilityId resolve path.
docs/manual-qa/NEO-97.md Matches — component regression still valid (elite at origin within 4 m slam); no update required for capstone range work.
Full-stack epic decomposition Matches — Godot capstone manual QA remains the prototype-complete proof; server fixes support playable QA without Bruno.

Blocking issues

None.

Suggestions

  1. E5.M2 module doc — NEO-95 / NEO-98 server behavior — Update the Session player combat HP (NEO-95) bullet and prototype freeze table to document attackAbilityId, NPC attack ability ids, and strike maxRange values (2 / 10 / 4 m). Note attackDamage on behavior rows is a projection/cross-ref field, not the resolve source after NEO-98. Done. E5_M2_NpcAiAndBehaviorProfiles.md freeze table + NPC attack ability table; NEO-95/NEO-98 bullets updated.

  2. Plan scope reconciliation — Revise Out of scope (or add a Decisions row) to record capstone QA drove catalog + server changes: prototype_npc_abilities.json, behavior attackAbilityId, defeat-stop, range-gated resolve. Keeps kickoff “docs-primary” honest while documenting the adopted integration path. Done. Kickoff Capstone QA scope expansion row + revised Out of scope / reconciliation in NEO-98-implementation-plan.md.

  3. documentation_and_implementation_alignment.md E5.M2 row — Append a short NEO-98 server integration clause (defeat clears aggro/runtime; telegraph damage uses attack ability maxRange) alongside the existing capstone manual QA note. Done. NEO-95 + NEO-98 clauses updated in alignment register.

  4. client/README.md melee hit row — Mirror manual QA step 8: tester must stay within ~2 m of the melee marker for 85/100; backing off during windup whiffs. Done. Flow table Melee hit row updated.

Nits

  • Nit: Plan §1 melee spine prose still collapses telegraph + HP into a single ≥ 4.5 s wait; manual QA correctly splits ≥ 3 s (telegraph) and ≥ 4.5 s (damage) — tighten plan technical approach for consistency. Done. Plan melee spine steps 3–4 split timing + 2 m whiff note.

  • Nit: AdvanceAll now requires IAbilityDefinitionRegistry + ICombatEntityHealthStore — any future caller besides NpcRuntimeSnapshotWorldApi must pass the expanded signature (only one production caller today).

  • Nit: Whiff path advances NPC state to recover without damage — correct per server README; optional manual QA note for Ranged/Elite if testers back off during windup (not required for capstone AC).

Verification

# NEO-98 server spine (in-memory; no Docker)
cd server/NeonSprawl.Server.Tests && dotnet test \
  --filter "FullyQualifiedName~NpcRuntime|FullyQualifiedName~NpcAttack|FullyQualifiedName~PostAbilityCast_ShouldStopNpcCombat"

# Content cross-ref gates
python3 scripts/validate_content.py

Local run: 41/41 filtered tests passed; 500/511 full suite passed (11 Postgres/docker integration failures — environment infra, unrelated to this diff).

Manual (required before merge): docs/manual-qa/NEO-98.md — confirm step 8 (85/100 only when within 2 m), defeated melee does not keep damaging player after step 9, three-archetype defeat, Esc + NEO-28 deny (steps 19–20).

Optional: docs/manual-qa/NEO-97.md incoming-threat row regression.