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/plans/NEO-89-implementation-plan.md

11 KiB
Raw Blame History

NEO-89 — Implementation plan

Story reference

Field Value
Key NEO-89
Title E5M2-03: Injectable INpcBehaviorDefinitionRegistry + DI
Linear https://linear.app/neon-sprawl/issue/NEO-89/e5m2-03-injectable-inpcbehaviordefinitionregistry-di
Module E5.M2 — NpcAiAndBehaviorProfiles · Epic 5 Slice 2 · backlog E5M2-03
Branch NEO-89-inpcbehaviordefinitionregistry-di
Precursor NEO-88 — fail-fast NpcBehaviorDefinitionCatalog load (Done on main)
Pattern NEO-79 / NEO-77 — thin registry adapter over startup catalog + DI; strict split after NEO-88 loader
Blocks NEO-90 — GET world npc-behavior-definitions; NEO-91+ — NPC instance registry, runtime, aggro
Client counterpart None (server-only); player-visible definitions HTTP is NEO-90; Godot capstone NEO-98

Kickoff clarifications

Topic Question Agent recommendation Answer
GetDefinitionsInIdOrder() Include enumeration API now? Yes — E5M2 backlog + NEO-79; NEO-90 needs ordered defs without NpcBehaviorDefinitionCatalog. User: include enumeration.
PrototypeNpcBehaviorRegistry Add three const behavior id strings for tests/fixtures? Yes — mirror PrototypeAbilityRegistry; keep PrototypeE5M2NpcBehaviorCatalogRules for gate validation only. User: add constants class.
Lookup naming TryGetDefinition vs TryGetBehavior? TryGetDefinition(string? behaviorId, …) — same as ability/item/skill registries; catalog keeps TryGetBehavior. Adopted — NEO-79 precedent; no separate question.
Program.cs eager resolve Eager-resolve INpcBehaviorDefinitionRegistry at boot? Omit — NpcBehaviorDefinitionCatalog already eager in Program.cs (NEO-88); registry is thin adapter (NEO-79). Adopted — NEO-79 precedent.
Call-site migration Migrate any handlers off catalog this story? None — no game handlers inject catalog yet; HTTP/runtime consumers land in NEO-90+. Adopted — repo has no catalog consumers beyond boot/tests.
Client counterpart Godot issue for this story? None — server-only DI; cross-link NEO-90 / NEO-98 for player-visible verification. Adopted (E5M2-03 out of scope).

Goal, scope, and out-of-scope

Goal: Provide INpcBehaviorDefinitionRegistry backed by the startup-loaded NpcBehaviorDefinitionCatalog: resolve by stable behavior id, enumerate definitions in id order, and expose TryNormalizeKnown for future instance-binding / HTTP validation. Register in DI so game code depends on the interface, not the catalog singleton.

In scope (from Linear + E5M2-03):

  • INpcBehaviorDefinitionRegistry + NpcBehaviorDefinitionRegistry thin adapter over NpcBehaviorDefinitionCatalog.
  • PrototypeNpcBehaviorRegistry with three frozen id constants (aligned with E5.M2 freeze box).
  • DI registration in AddNpcBehaviorDefinitionCatalog.
  • Unit tests (AAA): known-id lookup with archetype metadata; unknown-id miss; TryNormalizeKnown trim/case behavior; enumeration order; host resolves registry from DI.

Out of scope (from Linear + backlog):

  • HTTP GET route (NEO-90).
  • NPC instance registry, aggro, runtime tick, combat-target migration (NEO-91+).
  • Changing loader, E5M2 gate, or catalog load semantics (NEO-88).
  • Godot / client changes.

Acceptance criteria checklist

  • Registry resolves all three frozen behavior ids (prototype_melee_pressure, prototype_ranged_control, prototype_elite_mini_boss) with expected row metadata from content.
  • Unknown id normalization / lookup fails closed (TryGetDefinition and TryNormalizeKnown return false without throwing).
  • GetDefinitionsInIdOrder returns three rows ordered by NpcBehaviorDefRow.Id (ordinal).
  • DI resolves INpcBehaviorDefinitionRegistry at host startup (test via InMemoryWebApplicationFactory).
  • Unit tests (AAA): lookup for known prototype ids; unknown id returns false; TryNormalizeKnown accepts trimmed/case-variant input for catalog ids.

Implementation reconciliation (shipped)

  • INpcBehaviorDefinitionRegistry + NpcBehaviorDefinitionRegistry registered in AddNpcBehaviorDefinitionCatalog; catalog-only eager resolve unchanged in Program.cs.
  • PrototypeNpcBehaviorRegistry — three frozen behavior id constants for tests/fixtures.
  • 11 new AAA tests in NpcBehaviorDefinitionRegistryTests; 27 total Npc-filter tests green (includes existing loader tests).

Technical approach

  1. INpcBehaviorDefinitionRegistry — mirror IAbilityDefinitionRegistry:

    • TryGetDefinition(string? behaviorId, [NotNullWhen(true)] out NpcBehaviorDefRow? definition) — null and unknown ids return false without throwing; exact ordinal match on catalog keys.
    • TryNormalizeKnown(string? rawBehaviorId, [NotNullWhen(true)] out string normalized) — trim, lowercase, empty/whitespace → false; success when normalized id exists in catalog.
    • GetDefinitionsInIdOrder() — all rows ordered by NpcBehaviorDefRow.Id (ordinal).
    • XML remarks: HTTP read model (NEO-90) and NPC runtime (NEO-91+) should depend on this interface, not NpcBehaviorDefinitionCatalog.

  2. NpcBehaviorDefinitionRegistry — primary-constructor adapter over NpcBehaviorDefinitionCatalog (same pattern as AbilityDefinitionRegistry); precompute ordered list at construction; delegate lookups to catalog.TryGetBehavior.

  3. PrototypeNpcBehaviorRegistry — public const strings for the three frozen ids; class summary points at INpcBehaviorDefinitionRegistry for validation/lookup (no static allowlist/TryNormalizeKnown).

  4. DI — extend AddNpcBehaviorDefinitionCatalog to register INpcBehaviorDefinitionRegistry → NpcBehaviorDefinitionRegistry after catalog singleton. No change to Program.cs eager-resolve (catalog only).

  5. Docs — tighten server/README.md NPC behavior section if needed (registry is the preferred lookup; catalog for fail-fast boot only). Optional one-line note in E5M2-prototype-backlog.md E5M2-03 row when implementation lands.

  6. Tests — new NpcBehaviorDefinitionRegistryTests.cs mirroring AbilityDefinitionRegistryTests: in-memory catalog helper, loader-backed prototype fixture via NpcBehaviorCatalogTestPaths + repo prototype_npc_behaviors.json, host DI test asserting three freeze-box rows.

Expected prototype lookup values (from content)

id maxHp aggroRadius leashRadius attackDamage telegraphWindupSeconds
prototype_melee_pressure 100 8.0 16.0 15 1.5
prototype_ranged_control 80 10.0 20.0 12 2.0
prototype_elite_mini_boss 200 8.0 18.0 25 2.5

Files to add

Path Purpose
server/NeonSprawl.Server/Game/Npc/INpcBehaviorDefinitionRegistry.cs Public contract: lookup, normalize, enumerate; remarks for NEO-90+ callers.
server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionRegistry.cs Singleton adapter over NpcBehaviorDefinitionCatalog.
server/NeonSprawl.Server/Game/Npc/PrototypeNpcBehaviorRegistry.cs Stable prototype behavior id constants for tests/fixtures.
server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionRegistryTests.cs AAA unit + host DI tests (mirror AbilityDefinitionRegistryTests).
docs/plans/NEO-89-implementation-plan.md This plan.

Files to modify

Path Rationale
server/NeonSprawl.Server/Game/Npc/NpcBehaviorCatalogServiceCollectionExtensions.cs Register INpcBehaviorDefinitionRegistry → NpcBehaviorDefinitionRegistry after catalog singleton; update method summary.
server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalog.cs Reference INpcBehaviorDefinitionRegistry in summary (replace NEO-89 placeholder).
server/README.md Confirm registry is preferred lookup path; catalog reserved for fail-fast boot.

Tests

File Coverage
server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionRegistryTests.cs Unit: TryGetDefinition for melee/ranged/elite with expected MaxHp, AggroRadius, AttackDamage, ArchetypeKind; unknown id + null → false; TryNormalizeKnown with whitespace/mixed case; empty/whitespace → false; GetDefinitionsInIdOrder count 3 + ordinal id order. Loader fixture: repo prototype_npc_behaviors.json via NpcBehaviorCatalogTestPaths, assert all three freeze-box values. Host: InMemoryWebApplicationFactory resolves INpcBehaviorDefinitionRegistry from DI; lookup + enumeration smoke.
Existing NpcBehaviorDefinitionCatalogLoaderTests Regression: no behavior change expected; run Npc test filter after implementation.

No manual QA for this story (server-only DI; no HTTP or Godot surface). Bruno proof is NEO-90.

Open questions / risks

Question / risk Agent recommendation Status
Case-only mismatch — caller sends id not in catalog after normalize Fail closed — catalog ids are lowercase frozen strings; TryNormalizeKnown lowercases input. adopted
Dual lookup surfaces — TryGetDefinition vs TryNormalizeKnown Keep both — future instance binding/HTTP may need normalize-only; runtime needs row lookup without duplicating trim rules. adopted
Duplicate id source — constants vs ExpectedBehaviorIds Constants for tests; PrototypeE5M2NpcBehaviorCatalogRules remains authoritative for loader/CI gate set equality. adopted