114 lines
10 KiB
Markdown
114 lines
10 KiB
Markdown
# 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](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) · Epic 5 Slice 2 · backlog **E5M2-03** |
|
|
| **Branch** | `NEO-89-inpcbehaviordefinitionregistry-di` |
|
|
| **Precursor** | [NEO-88](https://linear.app/neon-sprawl/issue/NEO-88) — fail-fast `NpcBehaviorDefinitionCatalog` load (**Done** on `main`) |
|
|
| **Pattern** | [NEO-79](https://linear.app/neon-sprawl/issue/NEO-79) / [NEO-77](https://linear.app/neon-sprawl/issue/NEO-77) — thin registry adapter over startup catalog + DI; strict split after NEO-88 loader |
|
|
| **Blocks** | [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) — GET world npc-behavior-definitions; [NEO-91+](E5M2-prototype-backlog.md) — NPC instance registry, runtime, aggro |
|
|
| **Client counterpart** | None (server-only); player-visible definitions HTTP is [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90); Godot capstone [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) |
|
|
|
|
## Kickoff clarifications
|
|
|
|
| Topic | Question | Agent recommendation | Answer |
|
|
|--------|----------|----------------------|--------|
|
|
| **`GetDefinitionsInIdOrder()`** | Include enumeration API now? | **Yes** — E5M2 backlog + NEO-79; [NEO-90](https://linear.app/neon-sprawl/issue/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](https://linear.app/neon-sprawl/issue/NEO-90) / [NEO-98](https://linear.app/neon-sprawl/issue/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](E5M2-prototype-backlog.md#e5m2-03--injectable-inpcbehaviordefinitionregistry--di)):**
|
|
|
|
- `INpcBehaviorDefinitionRegistry` + `NpcBehaviorDefinitionRegistry` thin adapter over `NpcBehaviorDefinitionCatalog`.
|
|
- `PrototypeNpcBehaviorRegistry` with three frozen id constants (aligned with [E5.M2 freeze box](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md)).
|
|
- 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](https://linear.app/neon-sprawl/issue/NEO-90)).
|
|
- NPC instance registry, aggro, runtime tick, combat-target migration ([NEO-91+](E5M2-prototype-backlog.md)).
|
|
- Changing loader, E5M2 gate, or catalog load semantics ([NEO-88](https://linear.app/neon-sprawl/issue/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.
|
|
|
|
## Technical approach
|
|
|
|
1. **`INpcBehaviorDefinitionRegistry`** — mirror [`IAbilityDefinitionRegistry`](../../server/NeonSprawl.Server/Game/Combat/IAbilityDefinitionRegistry.cs):
|
|
- `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](https://linear.app/neon-sprawl/issue/NEO-90)) and NPC runtime ([NEO-91+](E5M2-prototype-backlog.md)) should depend on this interface, not `NpcBehaviorDefinitionCatalog`.
|
|
|
|
2. **`NpcBehaviorDefinitionRegistry`** — primary-constructor adapter over `NpcBehaviorDefinitionCatalog` (same pattern as [`AbilityDefinitionRegistry`](../../server/NeonSprawl.Server/Game/Combat/AbilityDefinitionRegistry.cs)); 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`](../../server/NeonSprawl.Server/Game/Npc/NpcBehaviorCatalogServiceCollectionExtensions.cs) to register `INpcBehaviorDefinitionRegistry` → `NpcBehaviorDefinitionRegistry` after catalog singleton. No change to `Program.cs` eager-resolve (catalog only).
|
|
|
|
5. **Docs** — tighten [`server/README.md`](../../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-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](https://linear.app/neon-sprawl/issue/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** |
|