From 7e4e0ca843cd20589e4ae75dae80434d95efd3d8 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 25 May 2026 17:25:22 -0400 Subject: [PATCH 1/5] NEO-89: Add implementation plan for NPC behavior registry DI. --- docs/plans/NEO-89-implementation-plan.md | 113 +++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 docs/plans/NEO-89-implementation-plan.md diff --git a/docs/plans/NEO-89-implementation-plan.md b/docs/plans/NEO-89-implementation-plan.md new file mode 100644 index 0000000..2f8edd9 --- /dev/null +++ b/docs/plans/NEO-89-implementation-plan.md @@ -0,0 +1,113 @@ +# 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** | From 70dc293246fefc42142fc2f0e0b642fae12503d5 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 25 May 2026 17:27:14 -0400 Subject: [PATCH 2/5] NEO-89: Add INpcBehaviorDefinitionRegistry and DI registration. --- .../Npc/NpcBehaviorDefinitionRegistryTests.cs | 302 ++++++++++++++++++ .../Npc/INpcBehaviorDefinitionRegistry.cs | 23 ++ ...aviorCatalogServiceCollectionExtensions.cs | 5 +- .../Game/Npc/NpcBehaviorDefinitionCatalog.cs | 2 +- .../Game/Npc/NpcBehaviorDefinitionRegistry.cs | 55 ++++ .../Game/Npc/PrototypeNpcBehaviorRegistry.cs | 9 + 6 files changed, 394 insertions(+), 2 deletions(-) create mode 100644 server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionRegistryTests.cs create mode 100644 server/NeonSprawl.Server/Game/Npc/INpcBehaviorDefinitionRegistry.cs create mode 100644 server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionRegistry.cs create mode 100644 server/NeonSprawl.Server/Game/Npc/PrototypeNpcBehaviorRegistry.cs diff --git a/server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionRegistryTests.cs b/server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionRegistryTests.cs new file mode 100644 index 0000000..0f28a8e --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionRegistryTests.cs @@ -0,0 +1,302 @@ +using System.IO; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NeonSprawl.Server.Game.Npc; +using NeonSprawl.Server.Tests; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Npc; + +public class NpcBehaviorDefinitionRegistryTests +{ + private static NpcBehaviorDefinitionRegistry CreateRegistryFromRows(IReadOnlyDictionary byId) + { + var catalog = new NpcBehaviorDefinitionCatalog("/tmp/npc-behaviors", byId, catalogJsonFileCount: 1); + return new NpcBehaviorDefinitionRegistry(catalog); + } + + private static NpcBehaviorDefRow MeleeRow() => + new( + PrototypeNpcBehaviorRegistry.PrototypeMeleePressure, + "Melee Pressure", + "melee_pressure", + 100, + 8.0, + 16.0, + 1.5, + 15, + 3.0); + + private static NpcBehaviorDefRow RangedRow() => + new( + PrototypeNpcBehaviorRegistry.PrototypeRangedControl, + "Ranged Control", + "ranged_control", + 80, + 10.0, + 20.0, + 2.0, + 12, + 4.0); + + private static NpcBehaviorDefRow EliteRow() => + new( + PrototypeNpcBehaviorRegistry.PrototypeEliteMiniBoss, + "Elite Mini-Boss", + "elite_mini_boss", + 200, + 8.0, + 18.0, + 2.5, + 25, + 5.0); + + [Fact] + public void TryGetDefinition_ShouldReturnTrueAndExpectedMetadata_WhenMeleeExists() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [PrototypeNpcBehaviorRegistry.PrototypeMeleePressure] = MeleeRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition(PrototypeNpcBehaviorRegistry.PrototypeMeleePressure, out var def); + // Assert + Assert.True(found); + Assert.NotNull(def); + Assert.Equal(100, def.MaxHp); + Assert.Equal(8.0, def.AggroRadius); + Assert.Equal(16.0, def.LeashRadius); + Assert.Equal(15, def.AttackDamage); + Assert.Equal(1.5, def.TelegraphWindupSeconds); + Assert.Equal("melee_pressure", def.ArchetypeKind); + Assert.Equal("Melee Pressure", def.DisplayName); + } + + [Fact] + public void TryGetDefinition_ShouldReturnTrueAndExpectedMetadata_WhenEliteExists() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [PrototypeNpcBehaviorRegistry.PrototypeEliteMiniBoss] = EliteRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition(PrototypeNpcBehaviorRegistry.PrototypeEliteMiniBoss, out var def); + // Assert + Assert.True(found); + Assert.NotNull(def); + Assert.Equal(200, def.MaxHp); + Assert.Equal(25, def.AttackDamage); + Assert.Equal(2.5, def.TelegraphWindupSeconds); + } + + [Fact] + public void TryGetDefinition_ShouldReturnFalse_WhenBehaviorIdIsNull() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [PrototypeNpcBehaviorRegistry.PrototypeMeleePressure] = MeleeRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition(null, out var def); + // Assert + Assert.False(found); + Assert.Null(def); + } + + [Fact] + public void TryGetDefinition_ShouldReturnFalse_WhenIdUnknown() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [PrototypeNpcBehaviorRegistry.PrototypeMeleePressure] = MeleeRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition("not_a_real_behavior", out var def); + // Assert + Assert.False(found); + Assert.Null(def); + } + + [Fact] + public void TryNormalizeKnown_ShouldReturnTrueAndLowercaseId_WhenInputHasWhitespaceAndMixedCase() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [PrototypeNpcBehaviorRegistry.PrototypeMeleePressure] = MeleeRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryNormalizeKnown(" Prototype_Melee_Pressure ", out var normalized); + // Assert + Assert.True(found); + Assert.Equal(PrototypeNpcBehaviorRegistry.PrototypeMeleePressure, normalized); + } + + [Fact] + public void TryNormalizeKnown_ShouldReturnFalse_WhenInputIsNull() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [PrototypeNpcBehaviorRegistry.PrototypeMeleePressure] = MeleeRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryNormalizeKnown(null, out var normalized); + // Assert + Assert.False(found); + Assert.Equal(string.Empty, normalized); + } + + [Fact] + public void TryNormalizeKnown_ShouldReturnFalse_WhenInputIsWhitespaceOnly() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [PrototypeNpcBehaviorRegistry.PrototypeMeleePressure] = MeleeRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryNormalizeKnown(" ", out var normalized); + // Assert + Assert.False(found); + Assert.Equal(string.Empty, normalized); + } + + [Fact] + public void TryNormalizeKnown_ShouldReturnFalse_WhenIdNotInCatalog() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [PrototypeNpcBehaviorRegistry.PrototypeMeleePressure] = MeleeRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryNormalizeKnown("prototype_unknown", out var normalized); + // Assert + Assert.False(found); + Assert.Equal("prototype_unknown", normalized); + } + + [Fact] + public void GetDefinitionsInIdOrder_ShouldListAllRowsOrderedById_WhenMultipleBehaviors() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [PrototypeNpcBehaviorRegistry.PrototypeRangedControl] = RangedRow(), + [PrototypeNpcBehaviorRegistry.PrototypeEliteMiniBoss] = EliteRow(), + [PrototypeNpcBehaviorRegistry.PrototypeMeleePressure] = MeleeRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var list = registry.GetDefinitionsInIdOrder(); + // Assert + Assert.Equal(3, list.Count); + Assert.Equal(PrototypeNpcBehaviorRegistry.PrototypeEliteMiniBoss, list[0].Id); + Assert.Equal(PrototypeNpcBehaviorRegistry.PrototypeMeleePressure, list[1].Id); + Assert.Equal(PrototypeNpcBehaviorRegistry.PrototypeRangedControl, list[2].Id); + } + + [Fact] + public void TryGetDefinition_ShouldMatchLoaderCatalog_WhenUsingPrototypeFixture() + { + // Arrange + var root = Directory.CreateTempSubdirectory("neon-sprawl-npc-registry-loader-"); + try + { + var npcBehaviorsDir = Path.Combine(root.FullName, "content", "npc-behaviors"); + var schemaDir = Path.Combine(root.FullName, "content", "schemas"); + Directory.CreateDirectory(npcBehaviorsDir); + Directory.CreateDirectory(schemaDir); + var schemaPath = Path.Combine(schemaDir, "npc-behavior-def.schema.json"); + File.Copy(NpcBehaviorCatalogTestPaths.DiscoverRepoNpcBehaviorDefSchemaPath(), schemaPath, overwrite: true); + File.Copy( + Path.Combine(NpcBehaviorCatalogTestPaths.DiscoverRepoNpcBehaviorsDirectory(), "prototype_npc_behaviors.json"), + Path.Combine(npcBehaviorsDir, "prototype_npc_behaviors.json"), + overwrite: true); + var loaded = NpcBehaviorDefinitionCatalogLoader.Load(npcBehaviorsDir, schemaPath, NullLogger.Instance); + var registry = new NpcBehaviorDefinitionRegistry(loaded); + // Act + var meleeOk = registry.TryGetDefinition(PrototypeNpcBehaviorRegistry.PrototypeMeleePressure, out var melee); + var rangedOk = registry.TryGetDefinition(PrototypeNpcBehaviorRegistry.PrototypeRangedControl, out var ranged); + var eliteOk = registry.TryGetDefinition(PrototypeNpcBehaviorRegistry.PrototypeEliteMiniBoss, out var elite); + var list = registry.GetDefinitionsInIdOrder(); + // Assert + Assert.True(meleeOk); + Assert.NotNull(melee); + Assert.Equal(100, melee!.MaxHp); + Assert.Equal(8.0, melee.AggroRadius); + Assert.Equal(16.0, melee.LeashRadius); + Assert.Equal(15, melee.AttackDamage); + Assert.Equal(1.5, melee.TelegraphWindupSeconds); + Assert.True(rangedOk); + Assert.NotNull(ranged); + Assert.Equal(80, ranged!.MaxHp); + Assert.Equal(10.0, ranged.AggroRadius); + Assert.Equal(20.0, ranged.LeashRadius); + Assert.Equal(12, ranged.AttackDamage); + Assert.Equal(2.0, ranged.TelegraphWindupSeconds); + Assert.True(eliteOk); + Assert.NotNull(elite); + Assert.Equal(200, elite!.MaxHp); + Assert.Equal(8.0, elite.AggroRadius); + Assert.Equal(18.0, elite.LeashRadius); + Assert.Equal(25, elite.AttackDamage); + Assert.Equal(2.5, elite.TelegraphWindupSeconds); + Assert.Equal(3, list.Count); + Assert.Equal(PrototypeNpcBehaviorRegistry.PrototypeEliteMiniBoss, list[0].Id); + Assert.Equal(PrototypeNpcBehaviorRegistry.PrototypeRangedControl, list[^1].Id); + } + finally + { + try + { + Directory.Delete(root.FullName, recursive: true); + } + catch (IOException) + { + // Best-effort: transient lock or race on some hosts; temp dir is unique per run. + } + } + } + + [Fact] + public async Task Host_ShouldResolveRegistryFromDi_WhenStartupSucceeds() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + using var client = factory.CreateClient(); + _ = await client.GetAsync("/health"); + // Act + var registry = factory.Services.GetRequiredService(); + var meleeFound = registry.TryGetDefinition(PrototypeNpcBehaviorRegistry.PrototypeMeleePressure, out var melee); + var unknown = registry.TryGetDefinition("not_a_real_behavior", out var missing); + var normalized = registry.TryNormalizeKnown(" Prototype_Ranged_Control ", out var rangedId); + var list = registry.GetDefinitionsInIdOrder(); + // Assert + Assert.True(meleeFound); + Assert.NotNull(melee); + Assert.Equal(100, melee!.MaxHp); + Assert.Equal(15, melee.AttackDamage); + Assert.False(unknown); + Assert.Null(missing); + Assert.True(normalized); + Assert.Equal(PrototypeNpcBehaviorRegistry.PrototypeRangedControl, rangedId); + Assert.Equal(3, list.Count); + Assert.Contains( + list, + b => b.Id == PrototypeNpcBehaviorRegistry.PrototypeEliteMiniBoss && b.MaxHp == 200 && b.AttackDamage == 25); + } +} diff --git a/server/NeonSprawl.Server/Game/Npc/INpcBehaviorDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Npc/INpcBehaviorDefinitionRegistry.cs new file mode 100644 index 0000000..b72582b --- /dev/null +++ b/server/NeonSprawl.Server/Game/Npc/INpcBehaviorDefinitionRegistry.cs @@ -0,0 +1,23 @@ +using System.Diagnostics.CodeAnalysis; + +namespace NeonSprawl.Server.Game.Npc; + +/// +/// Read-only access to validated entries loaded at startup (). +/// +/// +/// E5.M2 (NPC runtime / instance binding): callers validating behavior ids and resolving aggro/telegraph metadata should depend on this interface +/// rather than so behavior defs stay centralized. +/// NEO-90: HTTP/read-model projections should depend on this interface rather than reaching into the catalog. +/// +public interface INpcBehaviorDefinitionRegistry +{ + /// Attempts to resolve a behavior by stable id (see npc-behavior-def.schema.json). Unknown ids and null return false without throwing. + bool TryGetDefinition(string? behaviorId, [NotNullWhen(true)] out NpcBehaviorDefRow? definition); + + /// Trims and lowercases ; returns true when the normalized id exists in the loaded catalog. null, empty, and whitespace return false without throwing. + bool TryNormalizeKnown(string? rawBehaviorId, [NotNullWhen(true)] out string normalized); + + /// Every loaded definition, ordered by (ordinal). + IReadOnlyList GetDefinitionsInIdOrder(); +} diff --git a/server/NeonSprawl.Server/Game/Npc/NpcBehaviorCatalogServiceCollectionExtensions.cs b/server/NeonSprawl.Server/Game/Npc/NpcBehaviorCatalogServiceCollectionExtensions.cs index 9f16625..9263535 100644 --- a/server/NeonSprawl.Server/Game/Npc/NpcBehaviorCatalogServiceCollectionExtensions.cs +++ b/server/NeonSprawl.Server/Game/Npc/NpcBehaviorCatalogServiceCollectionExtensions.cs @@ -8,7 +8,7 @@ namespace NeonSprawl.Server.Game.Npc; /// DI registration for the fail-fast NPC behavior catalog (NEO-88). public static class NpcBehaviorCatalogServiceCollectionExtensions { - /// Binds and registers as a singleton. + /// Binds and registers and as singletons. public static IServiceCollection AddNpcBehaviorDefinitionCatalog(this IServiceCollection services, IConfiguration configuration) { services.AddOptions() @@ -32,6 +32,9 @@ public static class NpcBehaviorCatalogServiceCollectionExtensions return NpcBehaviorDefinitionCatalogLoader.Load(npcBehaviorsDir, schemaPath, logger); }); + services.AddSingleton(sp => + new NpcBehaviorDefinitionRegistry(sp.GetRequiredService())); + return services; } } diff --git a/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalog.cs b/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalog.cs index 14d2264..6122d97 100644 --- a/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalog.cs +++ b/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalog.cs @@ -2,7 +2,7 @@ using System.Collections.ObjectModel; namespace NeonSprawl.Server.Game.Npc; -/// In-memory NPC behavior catalog loaded at startup (NEO-88). Game callers should use INpcBehaviorDefinitionRegistry (NEO-89). +/// In-memory NPC behavior catalog loaded at startup (NEO-88). Game callers should use . public sealed class NpcBehaviorDefinitionCatalog( string npcBehaviorsDirectory, IReadOnlyDictionary byId, diff --git a/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionRegistry.cs new file mode 100644 index 0000000..2ae0b96 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionRegistry.cs @@ -0,0 +1,55 @@ +using System.Diagnostics.CodeAnalysis; + +namespace NeonSprawl.Server.Game.Npc; + +/// Adapter over (NEO-89). +public sealed class NpcBehaviorDefinitionRegistry(NpcBehaviorDefinitionCatalog catalog) : INpcBehaviorDefinitionRegistry +{ + private readonly IReadOnlyList _definitionsInIdOrder = BuildDefinitionsInIdOrder(catalog); + + /// + public bool TryGetDefinition(string? behaviorId, [NotNullWhen(true)] out NpcBehaviorDefRow? definition) + { + if (behaviorId is null) + { + definition = null; + return false; + } + + return catalog.TryGetBehavior(behaviorId, out definition); + } + + /// + public bool TryNormalizeKnown(string? rawBehaviorId, [NotNullWhen(true)] out string normalized) + { + if (rawBehaviorId is null) + { + normalized = string.Empty; + return false; + } + + normalized = rawBehaviorId.Trim().ToLowerInvariant(); + if (normalized.Length == 0) + { + normalized = string.Empty; + return false; + } + + return catalog.TryGetBehavior(normalized, out _); + } + + /// + public IReadOnlyList GetDefinitionsInIdOrder() => _definitionsInIdOrder; + + private static IReadOnlyList BuildDefinitionsInIdOrder(NpcBehaviorDefinitionCatalog catalog) + { + var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + var list = new List(ids.Length); + foreach (var id in ids) + { + list.Add(catalog.ById[id]); + } + + return list; + } +} diff --git a/server/NeonSprawl.Server/Game/Npc/PrototypeNpcBehaviorRegistry.cs b/server/NeonSprawl.Server/Game/Npc/PrototypeNpcBehaviorRegistry.cs new file mode 100644 index 0000000..62d3b9e --- /dev/null +++ b/server/NeonSprawl.Server/Game/Npc/PrototypeNpcBehaviorRegistry.cs @@ -0,0 +1,9 @@ +namespace NeonSprawl.Server.Game.Npc; + +/// Stable prototype NPC behavior id constants for tests and fixtures (NEO-89). Validation and lookup use . +public static class PrototypeNpcBehaviorRegistry +{ + public const string PrototypeMeleePressure = "prototype_melee_pressure"; + public const string PrototypeRangedControl = "prototype_ranged_control"; + public const string PrototypeEliteMiniBoss = "prototype_elite_mini_boss"; +} From e25b1406c79c2cf0d789776c590de631552fe31d Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 25 May 2026 17:27:17 -0400 Subject: [PATCH 3/5] NEO-89: Reconcile plan, backlog, and module docs for NPC registry. --- .../modules/E5_M2_NpcAiAndBehaviorProfiles.md | 4 ++-- ...documentation_and_implementation_alignment.md | 2 +- .../modules/module_dependency_register.md | 2 +- docs/plans/E5M2-prototype-backlog.md | 6 ++++-- docs/plans/NEO-89-implementation-plan.md | 16 +++++++++++----- server/README.md | 2 +- 6 files changed, 20 insertions(+), 12 deletions(-) diff --git a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md index baf004e..0f3eaf8 100644 --- a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md +++ b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md @@ -7,8 +7,8 @@ | **Module ID** | E5.M2 | | **Epic** | [Epic 5 — PvE Combat](../epics/epic_05_pve_combat.md) | | **Stage target** | Prototype | -| **Status** | In Progress — 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-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-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) | +| **Status** | In Progress — 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-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-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) | ## Purpose diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 04af82e..8972f16 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -57,7 +57,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E3.M3 | In Progress | **NEO-50 landed:** frozen prototype six-item catalog in [`content/items/prototype_items.json`](../../../content/items/prototype_items.json); [`item-def.schema.json`](../../../content/schemas/item-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py). **NEO-51 landed:** fail-fast server load of `content/items/*_items.json` at startup — `server/NeonSprawl.Server/Game/Items/` ([NEO-51](../../plans/NEO-51-implementation-plan.md)); [server README — Item catalog](../../../server/README.md#item-catalog-contentitems-neo-51). **NEO-52 landed:** injectable **`IItemDefinitionRegistry`** + lookup tests ([NEO-52](../../plans/NEO-52-implementation-plan.md)). **NEO-53 landed:** **`GET /game/world/item-definitions`** — `ItemDefinitionsWorldApi` + DTOs in `Game/Items/` ([NEO-53](../../plans/NEO-53-implementation-plan.md), [`NEO-53` manual QA](../../manual-qa/NEO-53.md)); [server README — Item definitions (NEO-53)](../../../server/README.md#item-definitions-neo-53); Bruno `bruno/neon-sprawl-server/item-definitions/`. **NEO-54 landed:** **`IPlayerInventoryStore`** + **`PlayerInventoryOperations`** — 24 bag + 1 equipment slots, stack rules, `V005` migration ([NEO-54](../../plans/NEO-54-implementation-plan.md)). **NEO-55 landed:** **`GET`/`POST /game/players/{id}/inventory`** — `PlayerInventoryApi` + DTOs in `Game/Items/` ([NEO-55](../../plans/NEO-55-implementation-plan.md)); [server README — Player inventory (NEO-54 store, NEO-55 HTTP)](../../../server/README.md#player-inventory-neo-54-store-neo-55-http); Bruno `bruno/neon-sprawl-server/inventory/`. **NEO-56 landed:** comment-only **`item_created`** / **`inventory_transfer_denied`** telemetry hook sites in [`PlayerInventoryOperations`](../../../server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs) ([NEO-56](../../plans/NEO-56-implementation-plan.md), [`NEO-56` manual QA](../../manual-qa/NEO-56.md)); no E9.M1 ingest. **NEO-72 landed:** client inventory HUD — **`inventory_client.gd`**, **`item_definitions_client.gd`**, **`InventoryLabel`**, boot hydrate + **`I`** refresh ([NEO-72](../../plans/NEO-72-implementation-plan.md), [`NEO-72` manual QA](../../manual-qa/NEO-72.md)); `client/README.md` inventory HUD section. **NEO-75 landed:** **`prototype_economy_hud_section.gd`** collapse toggle + capstone manual QA ([NEO-75](../../plans/NEO-75-implementation-plan.md), [`NEO-75` manual QA](../../manual-qa/NEO-75.md)); Epic 3 Slice 5 client complete. | [NEO-50](../../plans/NEO-50-implementation-plan.md), [NEO-51](../../plans/NEO-51-implementation-plan.md), [NEO-52](../../plans/NEO-52-implementation-plan.md), [NEO-53](../../plans/NEO-53-implementation-plan.md), [NEO-54](../../plans/NEO-54-implementation-plan.md), [NEO-55](../../plans/NEO-55-implementation-plan.md), [NEO-56](../../plans/NEO-56-implementation-plan.md), [NEO-72](../../plans/NEO-72-implementation-plan.md), [NEO-75](../../plans/NEO-75-implementation-plan.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3M3-prototype-backlog](../../plans/E3M3-prototype-backlog.md), [E3_M3](E3_M3_ItemizationAndInventorySchema.md) | | E3.M2 | Ready | **NEO-65 landed:** frozen prototype eight-recipe catalog in [`content/recipes/prototype_recipes.json`](../../../content/recipes/prototype_recipes.json); [`recipe-def.schema.json`](../../../content/schemas/recipe-def.schema.json) + [`recipe-io-row.schema.json`](../../../content/schemas/recipe-io-row.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) Slice 3 gates ([NEO-65](../../plans/NEO-65-implementation-plan.md)). **NEO-66 landed:** fail-fast server load of `content/recipes/*_recipes.json` at startup — `server/NeonSprawl.Server/Game/Crafting/` ([NEO-66](../../plans/NEO-66-implementation-plan.md)); [server README — Recipe catalog](../../../server/README.md#recipe-catalog-contentrecipes-neo-66). **NEO-67 landed:** injectable **`IRecipeDefinitionRegistry`** + lookup tests ([NEO-67](../../plans/NEO-67-implementation-plan.md)). **NEO-68 landed:** **`GET /game/world/recipe-definitions`** — `RecipeDefinitionsWorldApi` + DTOs in `Game/Crafting/` ([NEO-68](../../plans/NEO-68-implementation-plan.md), [`NEO-68` manual QA](../../manual-qa/NEO-68.md)); [server README — Recipe definitions (NEO-68)](../../../server/README.md#recipe-definitions-neo-68); Bruno `bruno/neon-sprawl-server/recipe-definitions/`. **NEO-69 landed:** **`CraftOperations.TryCraft`** + **`CraftResult`** — pre-flight inputs/outputs, inventory mutation, refine XP ([NEO-69](../../plans/NEO-69-implementation-plan.md)); [server README — Craft engine (NEO-69)](../../../server/README.md#craft-engine-neo-69-and-craft-http-neo-70). **NEO-70 landed:** **`POST /game/players/{id}/craft`** — `PlayerCraftApi` + wire **`CraftResponse`**; Bruno gather→refine→make spine ([NEO-70](../../plans/NEO-70-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/craft/`. **NEO-71 landed:** comment-only **`item_crafted`** / **`craft_failed`** telemetry hook sites in **`CraftOperations.TryCraft`** ([NEO-71](../../plans/NEO-71-implementation-plan.md)); [server README — Craft telemetry hooks (NEO-71)](../../../server/README.md#craft-telemetry-hooks-neo-71). **NEO-42 landed (prep):** **`RefineActivitySkillXpGrant`** / **`RefineSkillXpConstants`** — wired from craft engine success path. Epic 3 Slice 3 server backlog **E3M2-01**–**E3M2-07** complete. Upstream: E3.M1 **Ready**, E3.M3 inventory landed. **NEO-74 landed:** client craft UI — **`recipe_definitions_client.gd`**, **`craft_client.gd`**, **`craft_recipe_panel.gd`**, **`CraftFeedbackLabel`**, **refine** skill row ([NEO-74](../../plans/NEO-74-implementation-plan.md), [`NEO-74` manual QA](../../manual-qa/NEO-74.md)); `client/README.md` craft section. **NEO-75 landed:** capstone playable gather→refine→make loop — [`NEO-75` manual QA](../../manual-qa/NEO-75.md), **`prototype_economy_hud_section.gd`**; Epic 3 Slice 5 client complete. | [NEO-42](../../plans/NEO-42-implementation-plan.md), [NEO-65](../../plans/NEO-65-implementation-plan.md), [NEO-66](../../plans/NEO-66-implementation-plan.md), [NEO-67](../../plans/NEO-67-implementation-plan.md), [NEO-68](../../plans/NEO-68-implementation-plan.md), [NEO-69](../../plans/NEO-69-implementation-plan.md), [NEO-70](../../plans/NEO-70-implementation-plan.md), [NEO-71](../../plans/NEO-71-implementation-plan.md), [NEO-74](../../plans/NEO-74-implementation-plan.md), [NEO-75](../../plans/NEO-75-implementation-plan.md), [E3M2-prototype-backlog](../../plans/E3M2-prototype-backlog.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3_M2](E3_M2_RefinementAndRecipeExecution.md) | | E5.M1 | Ready | **NEO-76 landed:** frozen prototype four-ability catalog in [`content/abilities/prototype_abilities.json`](../../../content/abilities/prototype_abilities.json); [`ability-def.schema.json`](../../../content/schemas/ability-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) E5M1 four-id gate ([NEO-76](../../plans/NEO-76-implementation-plan.md)). **NEO-77 landed:** fail-fast server load of `content/abilities/*_abilities.json` at startup — `server/NeonSprawl.Server/Game/Combat/` ([NEO-77](../../plans/NEO-77-implementation-plan.md)); [server README — Ability catalog](../../../server/README.md#ability-catalog-contentabilities-neo-77). **NEO-79 landed:** injectable **`IAbilityDefinitionRegistry`** + DI; hotbar/cast use **`TryNormalizeKnown`** ([NEO-79](../../plans/NEO-79-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/hotbar-loadout/` + `ability-cast/` unknown-ability deny smokes. **NEO-78 landed:** **`GET /game/world/ability-definitions`** — `AbilityDefinitionsWorldApi` + DTOs in `Game/Combat/` ([NEO-78](../../plans/NEO-78-implementation-plan.md)); [server README — Ability definitions (NEO-78)](../../../server/README.md#ability-definitions-neo-78); Bruno `bruno/neon-sprawl-server/ability-definitions/`. **NEO-80 landed:** **`ICombatEntityHealthStore`** / **`InMemoryCombatEntityHealthStore`** — lazy prototype dummy HP at **`PrototypeCombatConstants.MaxPrototypeTargetHp` (100)**; DI via **`AddCombatEntityHealthStore()`** ([NEO-80](../../plans/NEO-80-implementation-plan.md)); [server README — Combat entity health (NEO-80)](../../../server/README.md#combat-entity-health-neo-80). **NEO-81 landed:** **`CombatOperations.TryResolve`** + **`CombatResult`** + **`CombatReasonCodes`** — defeated pre-check deny, catalog damage ([NEO-81](../../plans/NEO-81-implementation-plan.md)); [server README — Combat engine (NEO-81)](../../../server/README.md#combat-engine-neo-81). **NEO-82 landed:** **`AbilityCastApi`** → **`CombatOperations.TryResolve`**; nested wire **`combatResolution`** on accept; per-ability catalog cooldown; **`target_defeated`** cast deny ([NEO-82](../../plans/NEO-82-implementation-plan.md)); [server README — Ability cast (NEO-82)](../../../server/README.md#ability-cast-neo-31-neo-82); Bruno `bruno/neon-sprawl-server/ability-cast/` defeat spine. **NEO-83 landed:** **`GET /game/world/combat-targets`** — `CombatTargetsWorldApi` + DTOs; authoritative HP snapshot from **`ICombatEntityHealthStore`** ([NEO-83](../../plans/NEO-83-implementation-plan.md)); [server README — Combat targets snapshot (NEO-83)](../../../server/README.md#combat-targets-snapshot-neo-83); Bruno `bruno/neon-sprawl-server/combat-targets/`. **NEO-44 landed:** **`CombatDefeatGigXpGrant`** on cast **`targetDefeated`** — **25** gig XP to **`breach`** via **`IPlayerGigProgressionStore`** (V007); **`GET …/gig-progression`** ([NEO-44](../../plans/NEO-44-implementation-plan.md), [`NEO-44` manual QA](../../manual-qa/NEO-44.md)); [server README — Gig progression (NEO-44)](../../../server/README.md#gig-progression-snapshot-neo-44); Bruno `bruno/neon-sprawl-server/gig-progression/`. **NEO-84 landed:** comment-only **`ability_used`** / **`enemy_defeat`** telemetry hook sites in **`CombatOperations.TryResolve`**; reserved **`encounter_start`** / **`player_death`** ([NEO-84](../../plans/NEO-84-implementation-plan.md)); [server README — Combat telemetry hooks (NEO-84)](../../../server/README.md#combat-telemetry-hooks-neo-84). **NEO-85 landed:** client combat HUD — **`ability_cast_client.gd`** parses **`combatResolution`**; **`combat_targets_client.gd`**, **`CastFeedbackLabel`** resolution copy, **`CombatTargetHpLabel`**; event-driven GET refresh ([NEO-85](../../plans/NEO-85-implementation-plan.md), [`NEO-85` manual QA](../../manual-qa/NEO-85.md)); `client/README.md` combat HUD section. **NEO-86 landed:** playable combat capstone — **`gig_progression_client.gd`**, **`GigXpLabel`**, defeat-triggered gig GET refresh; capstone manual QA [`NEO-86`](../../manual-qa/NEO-86.md); `client/README.md` end-to-end combat loop section ([NEO-86](../../plans/NEO-86-implementation-plan.md)). **Epic 5 Slice 1 client capstone complete.** **Backlog decomposed:** Epic 5 Slice 1 — **E5M1-01**–**E5M1-12** landed. **Precursor landed:** E1.M4 cast funnel (NEO-28/31/32). | [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md), [E5_M1](E5_M1_CombatRulesEngine.md), [NEO-77](../../plans/NEO-77-implementation-plan.md), [NEO-79](../../plans/NEO-79-implementation-plan.md), [NEO-78](../../plans/NEO-78-implementation-plan.md), [NEO-80](../../plans/NEO-80-implementation-plan.md), [NEO-81](../../plans/NEO-81-implementation-plan.md), [NEO-82](../../plans/NEO-82-implementation-plan.md), [NEO-83](../../plans/NEO-83-implementation-plan.md), [NEO-44](../../plans/NEO-44-implementation-plan.md), [NEO-84](../../plans/NEO-84-implementation-plan.md), [NEO-85](../../plans/NEO-85-implementation-plan.md), [NEO-86](../../plans/NEO-86-implementation-plan.md), [E1M4-prototype-backlog.md](../../plans/E1M4-prototype-backlog.md) | -| E5.M2 | In Progress | **NEO-87 landed:** frozen prototype three-behavior catalog in [`content/npc-behaviors/prototype_npc_behaviors.json`](../../../content/npc-behaviors/prototype_npc_behaviors.json); [`npc-behavior-def.schema.json`](../../../content/schemas/npc-behavior-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) E5M2 three-id gate ([NEO-87](../../plans/NEO-87-implementation-plan.md)). **NEO-88 landed:** fail-fast server load of `content/npc-behaviors/*_npc_behaviors.json` at startup — `server/NeonSprawl.Server/Game/Npc/` ([NEO-88](../../plans/NEO-88-implementation-plan.md)); [server README — NPC behavior catalog](../../../server/README.md#npc-behavior-catalog-contentnpc-behaviors-neo-88). **Backlog decomposed:** Epic 5 Slice 2 — [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md) **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) → **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98); client capstone **NEO-98**. Three archetypes (`prototype_melee_pressure`, `prototype_ranged_control`, `prototype_elite_mini_boss`); replaces alpha/beta dummies with **`prototype_npc_melee` / `ranged` / `elite`**. Owns **`ThreatState`**, **`TelegraphEvent`**, **`GET …/npc-runtime-snapshot`**, session **`IPlayerCombatHealthStore`**. Upstream **E5.M1 Ready**. | [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md), [E5_M2](E5_M2_NpcAiAndBehaviorProfiles.md), [NEO-87](../../plans/NEO-87-implementation-plan.md), [NEO-88](../../plans/NEO-88-implementation-plan.md), label **`E5.M2`** on NEO-87–NEO-98 | +| E5.M2 | In Progress | **NEO-87 landed:** frozen prototype three-behavior catalog in [`content/npc-behaviors/prototype_npc_behaviors.json`](../../../content/npc-behaviors/prototype_npc_behaviors.json); [`npc-behavior-def.schema.json`](../../../content/schemas/npc-behavior-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) E5M2 three-id gate ([NEO-87](../../plans/NEO-87-implementation-plan.md)). **NEO-88 landed:** fail-fast server load of `content/npc-behaviors/*_npc_behaviors.json` at startup — `server/NeonSprawl.Server/Game/Npc/` ([NEO-88](../../plans/NEO-88-implementation-plan.md)); [server README — NPC behavior catalog](../../../server/README.md#npc-behavior-catalog-contentnpc-behaviors-neo-88). **NEO-89 landed:** injectable **`INpcBehaviorDefinitionRegistry`** + DI; **`PrototypeNpcBehaviorRegistry`** id constants ([NEO-89](../../plans/NEO-89-implementation-plan.md)). **Backlog decomposed:** Epic 5 Slice 2 — [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md) **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) → **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98); client capstone **NEO-98**. Three archetypes (`prototype_melee_pressure`, `prototype_ranged_control`, `prototype_elite_mini_boss`); replaces alpha/beta dummies with **`prototype_npc_melee` / `ranged` / `elite`**. Owns **`ThreatState`**, **`TelegraphEvent`**, **`GET …/npc-runtime-snapshot`**, session **`IPlayerCombatHealthStore`**. Upstream **E5.M1 Ready**. | [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md), [E5_M2](E5_M2_NpcAiAndBehaviorProfiles.md), [NEO-87](../../plans/NEO-87-implementation-plan.md), [NEO-88](../../plans/NEO-88-implementation-plan.md), [NEO-89](../../plans/NEO-89-implementation-plan.md), label **`E5.M2`** on NEO-87–NEO-98 | --- diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 4070a04..8782147 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -78,7 +78,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen **E5.M1 note:** Epic 5 **Slice 1** backlog in Linear ([Epic 5 — PvE Combat and Encounter Content](https://linear.app/neon-sprawl/project/epic-5-pve-combat-and-encounter-content-cf3c94c3-5346-40e0-8aaf-281e846f2846)): [NEO-76](https://linear.app/neon-sprawl/issue/NEO-76) → [NEO-86](https://linear.app/neon-sprawl/issue/NEO-86); label **`E5.M1`**. Gig XP on defeat: **E5M1-10** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md), [E5_M1_CombatRulesEngine.md](E5_M1_CombatRulesEngine.md). Builds on E1.M4 cast funnel (NEO-28/31/32); extends **`POST …/ability-cast`** with **`CombatResolution`**. **NEO-76 landed:** frozen four-ability catalog + CI ([NEO-76 plan](../../plans/NEO-76-implementation-plan.md)). **NEO-77 landed:** fail-fast server load of `content/abilities/*_abilities.json` ([NEO-77 plan](../../plans/NEO-77-implementation-plan.md)). **NEO-79 landed:** injectable **`IAbilityDefinitionRegistry`** + DI; hotbar/cast use **`TryNormalizeKnown`** ([NEO-79 plan](../../plans/NEO-79-implementation-plan.md)). **NEO-78 landed:** **`GET /game/world/ability-definitions`** — `AbilityDefinitionsWorldApi` + DTOs in `Game/Combat/` ([NEO-78 plan](../../plans/NEO-78-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/ability-definitions/`. **NEO-80 landed:** **`ICombatEntityHealthStore`** / **`InMemoryCombatEntityHealthStore`** — lazy prototype dummy HP at **`PrototypeCombatConstants.MaxPrototypeTargetHp` (100)**; DI via **`AddCombatEntityHealthStore()`** ([NEO-80 plan](../../plans/NEO-80-implementation-plan.md)); [server README — Combat entity health (NEO-80)](../../../server/README.md#combat-entity-health-neo-80). **NEO-81 landed:** **`CombatOperations.TryResolve`** + **`CombatResult`** + **`CombatReasonCodes`** — defeated pre-check deny vocabulary (`target_defeated`, `unknown_ability`, `unknown_target`) ([NEO-81 plan](../../plans/NEO-81-implementation-plan.md)); [server README — Combat engine (NEO-81)](../../../server/README.md#combat-engine-neo-81). **NEO-82 landed:** **`AbilityCastApi`** invokes **`CombatOperations.TryResolve`** after E1.M4 gates; nested wire **`combatResolution`** on accept; per-ability catalog **`cooldownSeconds`** on successful resolve; **`target_defeated`** JSON cast deny without cooldown ([NEO-82 plan](../../plans/NEO-82-implementation-plan.md)); [server README — Ability cast (NEO-82)](../../../server/README.md#ability-cast-neo-31-neo-82); Bruno `bruno/neon-sprawl-server/ability-cast/` defeat spine. **NEO-83 landed:** **`GET /game/world/combat-targets`** — `CombatTargetsWorldApi` + DTOs; authoritative HP snapshot from **`ICombatEntityHealthStore`** for client HUD ([NEO-83 plan](../../plans/NEO-83-implementation-plan.md)); [server README — Combat targets snapshot (NEO-83)](../../../server/README.md#combat-targets-snapshot-neo-83); Bruno `bruno/neon-sprawl-server/combat-targets/`. **NEO-44 landed:** **`CombatDefeatGigXpGrant`** on cast **`targetDefeated`** — **25** gig XP to **`breach`** via **`IPlayerGigProgressionStore`** (V007); **`GET …/gig-progression`** ([NEO-44 plan](../../plans/NEO-44-implementation-plan.md)); [server README — Gig progression (NEO-44)](../../../server/README.md#gig-progression-snapshot-neo-44). **NEO-84 landed:** comment-only **`ability_used`** / **`enemy_defeat`** telemetry hook sites in **`CombatOperations.TryResolve`**; reserved **`encounter_start`** / **`player_death`** ([NEO-84 plan](../../plans/NEO-84-implementation-plan.md)); [server README — Combat telemetry hooks (NEO-84)](../../../server/README.md#combat-telemetry-hooks-neo-84). **NEO-85 landed:** client combat HUD — **`combat_targets_client.gd`**, **`CombatTargetHpLabel`**, **`CastFeedbackLabel`** resolution copy; event-driven **`GET /game/world/combat-targets`** refresh ([NEO-85 plan](../../plans/NEO-85-implementation-plan.md), [`NEO-85` manual QA](../../manual-qa/NEO-85.md)); [client README — Combat feedback + target HP HUD (NEO-85)](../../../client/README.md#combat-feedback--target-hp-hud-neo-85). **NEO-86 landed:** playable combat capstone — **`gig_progression_client.gd`**, **`GigXpLabel`**, defeat-triggered gig GET refresh; capstone manual QA [`NEO-86`](../../manual-qa/NEO-86.md) ([NEO-86 plan](../../plans/NEO-86-implementation-plan.md)); [client README — End-to-end combat loop (NEO-86)](../../../client/README.md#end-to-end-combat-loop-neo-86). **Epic 5 Slice 1 complete — register row Ready.** -**E5.M2 note:** Epic 5 **Slice 2** backlog in Linear ([Epic 5 — PvE Combat and Encounter Content](https://linear.app/neon-sprawl/project/epic-5-pve-combat-and-encounter-content-cf3c94c3-5346-40e0-8aaf-281e846f2846)): [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) → [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98); label **`E5.M2`**. Client capstone **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98). See [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md), [E5_M2_NpcAiAndBehaviorProfiles.md](E5_M2_NpcAiAndBehaviorProfiles.md). Upstream **E5.M1 Ready**. **NEO-87 landed:** frozen three-behavior catalog + CI ([NEO-87 plan](../../plans/NEO-87-implementation-plan.md)). **NEO-88 landed:** fail-fast server load of `content/npc-behaviors/*_npc_behaviors.json` ([NEO-88 plan](../../plans/NEO-88-implementation-plan.md)); [server README — NPC behavior catalog](../../../server/README.md#npc-behavior-catalog-contentnpc-behaviors-neo-88). Replaces **`prototype_target_alpha` / `beta`** with three NPC archetype instances; owns **`ThreatState`**, **`TelegraphEvent`**, session **`IPlayerCombatHealthStore`**. +**E5.M2 note:** Epic 5 **Slice 2** backlog in Linear ([Epic 5 — PvE Combat and Encounter Content](https://linear.app/neon-sprawl/project/epic-5-pve-combat-and-encounter-content-cf3c94c3-5346-40e0-8aaf-281e846f2846)): [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) → [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98); label **`E5.M2`**. Client capstone **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98). See [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md), [E5_M2_NpcAiAndBehaviorProfiles.md](E5_M2_NpcAiAndBehaviorProfiles.md). Upstream **E5.M1 Ready**. **NEO-87 landed:** frozen three-behavior catalog + CI ([NEO-87 plan](../../plans/NEO-87-implementation-plan.md)). **NEO-88 landed:** fail-fast server load of `content/npc-behaviors/*_npc_behaviors.json` ([NEO-88 plan](../../plans/NEO-88-implementation-plan.md)); [server README — NPC behavior catalog](../../../server/README.md#npc-behavior-catalog-contentnpc-behaviors-neo-88). **NEO-89 landed:** injectable **`INpcBehaviorDefinitionRegistry`** + DI ([NEO-89 plan](../../plans/NEO-89-implementation-plan.md)). Replaces **`prototype_target_alpha` / `beta`** with three NPC archetype instances; owns **`ThreatState`**, **`TelegraphEvent`**, session **`IPlayerCombatHealthStore`**. ### Epic 6 — PvP Security diff --git a/docs/plans/E5M2-prototype-backlog.md b/docs/plans/E5M2-prototype-backlog.md index 033bb26..fd354a2 100644 --- a/docs/plans/E5M2-prototype-backlog.md +++ b/docs/plans/E5M2-prototype-backlog.md @@ -142,8 +142,10 @@ Working backlog for **Epic 5 — Slice 2** ([NPC archetypes and telegraphs](../d **Acceptance criteria** -- [ ] Registry resolves all three frozen behavior ids. -- [ ] Unknown id normalization fails closed. +- [x] Registry resolves all three frozen behavior ids. +- [x] Unknown id normalization fails closed. + +**Landed ([NEO-89](https://linear.app/neon-sprawl/issue/NEO-89)):** `INpcBehaviorDefinitionRegistry` + `NpcBehaviorDefinitionRegistry` + `PrototypeNpcBehaviorRegistry` in `server/NeonSprawl.Server/Game/Npc/`; plan [NEO-89-implementation-plan.md](NEO-89-implementation-plan.md). --- diff --git a/docs/plans/NEO-89-implementation-plan.md b/docs/plans/NEO-89-implementation-plan.md index 2f8edd9..92cf127 100644 --- a/docs/plans/NEO-89-implementation-plan.md +++ b/docs/plans/NEO-89-implementation-plan.md @@ -45,11 +45,17 @@ ## 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. +- [x] Registry resolves all three frozen behavior ids (`prototype_melee_pressure`, `prototype_ranged_control`, `prototype_elite_mini_boss`) with expected row metadata from content. +- [x] Unknown id normalization / lookup fails closed (`TryGetDefinition` and `TryNormalizeKnown` return false without throwing). +- [x] `GetDefinitionsInIdOrder` returns three rows ordered by `NpcBehaviorDefRow.Id` (ordinal). +- [x] DI resolves `INpcBehaviorDefinitionRegistry` at host startup (test via `InMemoryWebApplicationFactory`). +- [x] 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 diff --git a/server/README.md b/server/README.md index ab9a4e9..d660636 100644 --- a/server/README.md +++ b/server/README.md @@ -104,7 +104,7 @@ On startup the host loads every **`*_npc_behaviors.json`** under the npc-behavio **Docker / CI:** include **`content/npc-behaviors`** and **`content/schemas/npc-behavior-def.schema.json`** in the mounted **`content/`** tree; set **`Content__NpcBehaviorsDirectory`** when layout differs. -On success, **Information** logs include the resolved npc-behaviors directory path, distinct behavior count, and catalog file count. Game code should use **`INpcBehaviorDefinitionRegistry`** for lookups (NEO-89). The catalog singleton remains for fail-fast startup only; do not inject **`NpcBehaviorDefinitionCatalog`** in new game code. +On success, **Information** logs include the resolved npc-behaviors directory path, distinct behavior count, and catalog file count. Game code should use **`INpcBehaviorDefinitionRegistry`** for lookups (`TryGetDefinition`, `TryNormalizeKnown`, `GetDefinitionsInIdOrder`; NEO-89). The catalog singleton remains for fail-fast startup only; do not inject **`NpcBehaviorDefinitionCatalog`** in new game code. Stable prototype behavior id constants: **`PrototypeNpcBehaviorRegistry`**. ## Ability definitions (NEO-78) From f4fafea7e26ec32509438886de84b93cf1a958c5 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 25 May 2026 17:27:57 -0400 Subject: [PATCH 4/5] NEO-89: Add code review for NPC behavior registry DI branch. --- docs/reviews/2026-05-25-NEO-89.md | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 docs/reviews/2026-05-25-NEO-89.md diff --git a/docs/reviews/2026-05-25-NEO-89.md b/docs/reviews/2026-05-25-NEO-89.md new file mode 100644 index 0000000..e3892a6 --- /dev/null +++ b/docs/reviews/2026-05-25-NEO-89.md @@ -0,0 +1,58 @@ +# Code review — NEO-89 injectable `INpcBehaviorDefinitionRegistry` + DI + +**Date:** 2026-05-25 +**Scope:** Branch `NEO-89-inpcbehaviordefinitionregistry-di` · commits `7e4e0ca`–`e25b140` vs `origin/main` +**Base:** `origin/main` + +## Verdict + +**Approve with nits** + +## Summary + +NEO-89 delivers **E5M2-03**: a thin **`INpcBehaviorDefinitionRegistry`** adapter over the NEO-88 startup-loaded **`NpcBehaviorDefinitionCatalog`**, registered in **`AddNpcBehaviorDefinitionCatalog`** alongside the catalog singleton. The implementation mirrors **`AbilityDefinitionRegistry`** (NEO-79): exact-id **`TryGetDefinition`**, trim/lowercase **`TryNormalizeKnown`**, precomputed **`GetDefinitionsInIdOrder`**, and **`PrototypeNpcBehaviorRegistry`** id constants for tests. Eleven new AAA unit/host tests pass; **27** Npc-filter tests green including loader regression. Docs (plan, E5M2 backlog E5M2-03, alignment register, module register, E5.M2 status, `server/README.md`) are reconciled. No HTTP, runtime, or client surface — appropriately scoped for a DI-only story. Risk is low: read-only adapter with no behavior change to catalog load semantics. + +## Documentation checked + +| Document | Result | +|----------|--------| +| [`docs/plans/NEO-89-implementation-plan.md`](../plans/NEO-89-implementation-plan.md) | **Matches** — kickoff decisions, scope, acceptance checklist, reconciliation (11 tests, DI pattern, no `Program.cs` eager registry resolve). | +| [`docs/plans/E5M2-prototype-backlog.md`](../plans/E5M2-prototype-backlog.md) · **E5M2-03** | **Matches** — AC checkboxes + landed note. | +| [`docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md`](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) | **Partially matches** — Status/Linear rows cite NEO-89; freeze **Rules** still only cite CI + server startup (NEO-88), not registry lookup surface (see Suggestion 1). | +| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) · **E5.M2** | **Matches** — NEO-89 landed note added. | +| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E5.M2 note cites NEO-89 registry + DI. | +| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **N/A** — server-only read registry; no runtime or client mutation. | +| [`server/README.md`](../../server/README.md) | **Matches** — registry methods documented; catalog reserved for fail-fast boot; `PrototypeNpcBehaviorRegistry` noted. | +| [`server/NeonSprawl.Server/Game/Combat/AbilityDefinitionRegistry.cs`](../../server/NeonSprawl.Server/Game/Combat/AbilityDefinitionRegistry.cs) (pattern reference) | **Matches** — NPC registry is structurally identical to NEO-79 precedent. | +| Full-stack epic decomposition | **Matches** — E5M2-03 is server-only; kickoff defers player-visible HTTP to NEO-90 and Godot to NEO-98; no false prototype-complete claim. | +| Manual QA | **N/A** — plan documents skip: automated unit/host tests only; Bruno proof deferred to NEO-90. | + +Register/tracking: alignment table and module register updates are correct; E5.M2 remains **In Progress** toward NEO-90+. + +## Blocking issues + +None. + +## Suggestions + +1. **E5.M2 freeze Rules paragraph** — In [`E5_M2_NpcAiAndBehaviorProfiles.md`](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md), extend the **Rules** sentence to cite **`INpcBehaviorDefinitionRegistry`** + NEO-89 as the preferred game lookup path (mirror E5.M1 freeze rules citing **`IAbilityDefinitionRegistry`** / NEO-79 after NEO-77 server load). + +2. **Optional E5.M2 implementation blurb** — Add a short “NPC behavior definition registry (NEO-89)” paragraph under the module doc (same style as E3.M3’s **`IItemDefinitionRegistry`** section) so future NEO-90+ stories have a single anchor for the contract without rereading the plan. + +## Nits + +- Nit: No dedicated unit test that **`TryGetDefinition`** rejects case-variant ids (e.g. `"Prototype_Melee_Pressure"`) — fail-closed by design and covered indirectly via **`TryNormalizeKnown`**; optional explicit test if NEO-90 HTTP validation wants to document the split. + +- Nit: **`TryGetDefinition`** for **`prototype_ranged_control`** metadata is only asserted in the loader-fixture and host tests, not a standalone unit test like melee/elite — acceptable given loader coverage. + +- Nit: Inline **`MeleeRow`/`RangedRow`/`EliteRow`** test helpers duplicate freeze-box numbers — same established pattern as ability registry tests. + +## Verification + +```bash +cd /home/don/neon-sprawl/server +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~NpcBehavior" +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj +``` + +Confirmed during review: **27** Npc-filter tests passed. From 3134bb6ff6d57a94be0b0cc16dca409c42a6a744 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 25 May 2026 17:28:46 -0400 Subject: [PATCH 5/5] =?UTF-8?q?NEO-89:=20Address=20code=20review=20?= =?UTF-8?q?=E2=80=94=20E5.M2=20module=20doc=20and=20case-id=20test.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../modules/E5_M2_NpcAiAndBehaviorProfiles.md | 6 +++++- docs/plans/NEO-89-implementation-plan.md | 2 +- docs/reviews/2026-05-25-NEO-89.md | 8 ++++---- .../Npc/NpcBehaviorDefinitionRegistryTests.cs | 16 ++++++++++++++++ 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md index 0f3eaf8..5f37af3 100644 --- a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md +++ b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md @@ -75,7 +75,11 @@ The **first shipped three-archetype NPC spine** under `content/npc-behaviors/*.j **NPC instance ids (E5M2-05):** **`prototype_npc_melee`**, **`prototype_npc_ranged`**, **`prototype_npc_elite`** — each binds one behavior def + world anchor (replaces **`prototype_target_alpha` / `beta`**). -**Rules:** Do **not** rename frozen behavior **`id`** values without a migration issue. CI ([`scripts/validate_content.py`](../../../scripts/validate_content.py)) and server startup ([NEO-88](../../plans/NEO-88-implementation-plan.md)) enforce **exactly** these three behavior ids, duplicate-`id` rejection, and **`leashRadius` > `aggroRadius`** per row. Relaxing or changing that gate belongs in a new Linear issue once the catalog intentionally grows. Catalog + CI plan: [NEO-87 implementation plan](../../plans/NEO-87-implementation-plan.md). +**Rules:** Do **not** rename frozen behavior **`id`** values without a migration issue. CI ([`scripts/validate_content.py`](../../../scripts/validate_content.py)) and server startup ([NEO-88](../../plans/NEO-88-implementation-plan.md)) enforce **exactly** these three behavior ids, duplicate-`id` rejection, and **`leashRadius` > `aggroRadius`** per row. Relaxing or changing that gate belongs in a new Linear issue once the catalog intentionally grows. Catalog + CI plan: [NEO-87 implementation plan](../../plans/NEO-87-implementation-plan.md). **`INpcBehaviorDefinitionRegistry`** + DI **landed** ([NEO-89](../../plans/NEO-89-implementation-plan.md), E5M2-03) — game code and HTTP read models should inject the registry, not **`NpcBehaviorDefinitionCatalog`**. + +**Server load (NEO-88):** On host startup, `server/NeonSprawl.Server/Game/Npc/` loads `content/npc-behaviors/*_npc_behaviors.json` with the same validation gates as CI and **refuses to listen** when the catalog is invalid. Config and discovery: [server README — NPC behavior catalog](../../../server/README.md#npc-behavior-catalog-contentnpc-behaviors-neo-88). Plan: [NEO-88 implementation plan](../../plans/NEO-88-implementation-plan.md). + +**NPC behavior definition registry (NEO-89):** **`INpcBehaviorDefinitionRegistry`** in `server/NeonSprawl.Server/Game/Npc/` wraps the startup-loaded catalog for read-only lookup by stable behavior `id` (`TryGetDefinition`), trim/lowercase validation (`TryNormalizeKnown`), and ordered enumeration (`GetDefinitionsInIdOrder`). Stable prototype id constants: **`PrototypeNpcBehaviorRegistry`**. **NEO-90+** (HTTP definitions, instance registry, runtime tick) should inject the interface rather than `NpcBehaviorDefinitionCatalog`. Plan: [NEO-89 implementation plan](../../plans/NEO-89-implementation-plan.md). ## Risks and telemetry diff --git a/docs/plans/NEO-89-implementation-plan.md b/docs/plans/NEO-89-implementation-plan.md index 92cf127..541b4ac 100644 --- a/docs/plans/NEO-89-implementation-plan.md +++ b/docs/plans/NEO-89-implementation-plan.md @@ -55,7 +55,7 @@ - **`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). +- **12** new AAA tests in **`NpcBehaviorDefinitionRegistryTests`** (includes case-variant **`TryGetDefinition`** deny); **28** total Npc-filter tests green (includes existing loader tests). ## Technical approach diff --git a/docs/reviews/2026-05-25-NEO-89.md b/docs/reviews/2026-05-25-NEO-89.md index e3892a6..300fe46 100644 --- a/docs/reviews/2026-05-25-NEO-89.md +++ b/docs/reviews/2026-05-25-NEO-89.md @@ -18,7 +18,7 @@ NEO-89 delivers **E5M2-03**: a thin **`INpcBehaviorDefinitionRegistry`** adapter |----------|--------| | [`docs/plans/NEO-89-implementation-plan.md`](../plans/NEO-89-implementation-plan.md) | **Matches** — kickoff decisions, scope, acceptance checklist, reconciliation (11 tests, DI pattern, no `Program.cs` eager registry resolve). | | [`docs/plans/E5M2-prototype-backlog.md`](../plans/E5M2-prototype-backlog.md) · **E5M2-03** | **Matches** — AC checkboxes + landed note. | -| [`docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md`](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) | **Partially matches** — Status/Linear rows cite NEO-89; freeze **Rules** still only cite CI + server startup (NEO-88), not registry lookup surface (see Suggestion 1). | +| [`docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md`](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) | **Matches** — Status/Linear rows cite NEO-89; freeze **Rules** + NEO-88/NEO-89 implementation blurbs (review follow-up). | | [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) · **E5.M2** | **Matches** — NEO-89 landed note added. | | [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E5.M2 note cites NEO-89 registry + DI. | | [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **N/A** — server-only read registry; no runtime or client mutation. | @@ -35,13 +35,13 @@ None. ## Suggestions -1. **E5.M2 freeze Rules paragraph** — In [`E5_M2_NpcAiAndBehaviorProfiles.md`](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md), extend the **Rules** sentence to cite **`INpcBehaviorDefinitionRegistry`** + NEO-89 as the preferred game lookup path (mirror E5.M1 freeze rules citing **`IAbilityDefinitionRegistry`** / NEO-79 after NEO-77 server load). +1. ~~**E5.M2 freeze Rules paragraph** — In [`E5_M2_NpcAiAndBehaviorProfiles.md`](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md), extend the **Rules** sentence to cite **`INpcBehaviorDefinitionRegistry`** + NEO-89 as the preferred game lookup path (mirror E5.M1 freeze rules citing **`IAbilityDefinitionRegistry`** / NEO-79 after NEO-77 server load).~~ **Done.** -2. **Optional E5.M2 implementation blurb** — Add a short “NPC behavior definition registry (NEO-89)” paragraph under the module doc (same style as E3.M3’s **`IItemDefinitionRegistry`** section) so future NEO-90+ stories have a single anchor for the contract without rereading the plan. +2. ~~**Optional E5.M2 implementation blurb** — Add a short “NPC behavior definition registry (NEO-89)” paragraph under the module doc (same style as E3.M3’s **`IItemDefinitionRegistry`** section) so future NEO-90+ stories have a single anchor for the contract without rereading the plan.~~ **Done.** ## Nits -- Nit: No dedicated unit test that **`TryGetDefinition`** rejects case-variant ids (e.g. `"Prototype_Melee_Pressure"`) — fail-closed by design and covered indirectly via **`TryNormalizeKnown`**; optional explicit test if NEO-90 HTTP validation wants to document the split. +- ~~Nit: No dedicated unit test that **`TryGetDefinition`** rejects case-variant ids (e.g. `"Prototype_Melee_Pressure"`) — fail-closed by design and covered indirectly via **`TryNormalizeKnown`**; optional explicit test if NEO-90 HTTP validation wants to document the split.~~ **Done.** - Nit: **`TryGetDefinition`** for **`prototype_ranged_control`** metadata is only asserted in the loader-fixture and host tests, not a standalone unit test like melee/elite — acceptable given loader coverage. diff --git a/server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionRegistryTests.cs b/server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionRegistryTests.cs index 0f28a8e..692a5be 100644 --- a/server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionRegistryTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionRegistryTests.cs @@ -125,6 +125,22 @@ public class NpcBehaviorDefinitionRegistryTests Assert.Null(def); } + [Fact] + public void TryGetDefinition_ShouldReturnFalse_WhenIdHasMixedCase() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [PrototypeNpcBehaviorRegistry.PrototypeMeleePressure] = MeleeRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition("Prototype_Melee_Pressure", out var def); + // Assert + Assert.False(found); + Assert.Null(def); + } + [Fact] public void TryNormalizeKnown_ShouldReturnTrueAndLowercaseId_WhenInputHasWhitespaceAndMixedCase() {