neon-sprawl/docs/plans/NEO-79-implementation-plan.md

NEO-79 — Implementation plan

Story reference

Field	Value
Key	NEO-79
Title	E5M1-03: IAbilityDefinitionRegistry + DI
Linear	https://linear.app/neon-sprawl/issue/NEO-79/e5m1-03-iabilitydefinitionregistry-di
Module	E5.M1 — CombatRulesEngine · Epic 5 Slice 1 · backlog E5M1-03
Branch	NEO-79-iabilitydefinitionregistry-di
Precursor	NEO-77 — fail-fast AbilityDefinitionCatalog load (Done on main)
Pattern	NEO-67 / NEO-52 — thin registry adapter over startup catalog + DI; strict split after NEO-77 loader
Blocks	NEO-78 — GET world ability-definitions; NEO-80+ — combat engine consumers
Client counterpart	None (server-only); optional client enrichment in NEO-85 via NEO-78 HTTP

Kickoff clarifications

Topic	Question	Agent recommendation	Answer
Enumeration API	Include GetDefinitionsInIdOrder() now?	Yes — mirror IRecipeDefinitionRegistry; NEO-78 GET /game/world/ability-definitions needs ordered defs without reaching into AbilityDefinitionCatalog.	User: include enumeration.
Lookup naming	TryGetDefinition vs TryGetAbility?	TryGetDefinition(string? abilityId, …) — same naming as item/skill/recipe registries; catalog keeps TryGetAbility for direct catalog access.	User: TryGetDefinition.
Hotbar/cast migration	How replace PrototypeAbilityRegistry.TryNormalizeKnown?	TryNormalizeKnown on IAbilityDefinitionRegistry — trim + lowercase + catalog lookup; inject into hotbar/cast APIs. Preserves today’s deny behavior (unknown_ability) without duplicating normalize in two endpoints.	User: registry TryNormalizeKnown.
PrototypeAbilityRegistry	Keep static allowlist or slim down?	Keep id constant strings only — remove HashSet allowlist and TryNormalizeKnown; tests/fixtures keep PrototypePulse etc.	User: constants only.
Program.cs eager resolve	Eager-resolve IAbilityDefinitionRegistry at boot?	Omit — AbilityDefinitionCatalog is already eager-resolved in Program.cs (NEO-77); registry is a thin adapter (NEO-67 default).	Adopted — NEO-67 precedent; no separate question.
Runtime validation	Re-validate abilities in registry?	No — catalog load is fail-fast (NEO-77); registry delegates to loaded rows only.	Adopted — NEO-67 precedent.

Goal, scope, and out-of-scope

Goal: Provide IAbilityDefinitionRegistry backed by the startup-loaded AbilityDefinitionCatalog: resolve by stable abilityId, enumerate definitions in id order, and expose TryNormalizeKnown for hotbar/cast allowlist validation. Register in DI and migrate HotbarLoadoutApi / AbilityCastApi off the static allowlist so unknown ids deny exactly as today while known ids resolve baseDamage / cooldownSeconds from content.

In scope (from Linear + E5M1-03):

• IAbilityDefinitionRegistry + AbilityDefinitionRegistry thin adapter over AbilityDefinitionCatalog.
• DI registration in AddAbilityDefinitionCatalog.
• Unit tests (AAA): known-id lookup with damage + cooldown; unknown-id miss; TryNormalizeKnown trim/case behavior; enumeration order; host resolves registry from DI.
• Migrate hotbar/cast call sites from PrototypeAbilityRegistry.TryNormalizeKnown to injected registry.
• Slim PrototypeAbilityRegistry to id constants only.

Out of scope (from Linear + backlog):

• HTTP GET route (NEO-78).
• Damage application, combat engine, cooldown commit from catalog (NEO-80+).
• Changing loader, E5M1 gate, or catalog load semantics (NEO-77).
• Godot / client changes.

Acceptance criteria checklist

• Unknown ability id fails hotbar/cast validation same as today (unknown_ability, no behavior regression).
• All four prototype ids resolve with expected baseDamage / cooldownSeconds via TryGetDefinition.
• DI resolves IAbilityDefinitionRegistry at host startup (test via InMemoryWebApplicationFactory).
• Unit tests (AAA): lookup for known prototype ids; unknown id returns false without throwing; TryNormalizeKnown accepts trimmed/case-variant input for catalog ids; GetDefinitionsInIdOrder returns four rows ordered by id.

Technical approach

1. IAbilityDefinitionRegistry — mirror IRecipeDefinitionRegistry:

   • TryGetDefinition(string? abilityId, [NotNullWhen(true)] out AbilityDefRow? definition) — null and unknown ids return false without throwing; exact ordinal match on catalog keys.
   • TryNormalizeKnown(string rawAbilityId, [NotNullWhen(true)] out string normalized) — trim, lowercase, empty/whitespace → false; success when normalized id exists in catalog (replaces static allowlist semantics).
   • GetDefinitionsInIdOrder() — all rows ordered by AbilityDefRow.Id (ordinal).
   • XML remarks: combat engine (NEO-81+) and HTTP read model (NEO-78) should depend on this interface, not AbilityDefinitionCatalog.

2. AbilityDefinitionRegistry — primary-constructor adapter over AbilityDefinitionCatalog (same pattern as RecipeDefinitionRegistry); precompute ordered list at construction.

3. DI — extend AddAbilityDefinitionCatalog to register IAbilityDefinitionRegistry → AbilityDefinitionRegistry after catalog singleton. No change to Program.cs eager-resolve (catalog only).

4. Hotbar/cast migration — inject IAbilityDefinitionRegistry into:

   • HotbarLoadoutApi POST handler — replace PrototypeAbilityRegistry.TryNormalizeKnown with registry.TryNormalizeKnown.
   • AbilityCastApi POST handler — same replacement for request abilityId validation.

5. PrototypeAbilityRegistry — retain only public const id strings (PrototypePulse, etc.); remove static HashSet and TryNormalizeKnown. Update class summary to point at IAbilityDefinitionRegistry.

6. Docs — update server/README.md ability-catalog section (registry is preferred lookup; hotbar/cast migrated). Optional one-line E5M1 backlog note when implementation lands.

7. Tests — new AbilityDefinitionRegistryTests.cs mirroring RecipeDefinitionRegistryTests: in-memory catalog helper, loader-backed prototype fixture via AbilityCatalogTestPaths + repo prototype_abilities.json, host DI test asserting four prototype rows with expected damage/cooldown from E5.M1 freeze box. Existing hotbar/cast API tests should remain green without changing expected deny/accept behavior.

Expected prototype lookup values (from content)

id	baseDamage	cooldownSeconds
prototype_pulse	25	3.0
prototype_guard	0	6.0
prototype_dash	0	4.0
prototype_burst	40	5.0

Files to add

Path	Purpose
server/NeonSprawl.Server/Game/Combat/IAbilityDefinitionRegistry.cs	Public contract: lookup, normalize, enumerate; remarks for NEO-78/81+ callers.
server/NeonSprawl.Server/Game/Combat/AbilityDefinitionRegistry.cs	Singleton adapter over AbilityDefinitionCatalog.
server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionRegistryTests.cs	AAA unit + host DI tests (mirror RecipeDefinitionRegistryTests).
docs/plans/NEO-79-implementation-plan.md	This plan.

Files to modify

Path	Rationale
server/NeonSprawl.Server/Game/Combat/AbilityCatalogServiceCollectionExtensions.cs	Register IAbilityDefinitionRegistry → AbilityDefinitionRegistry after catalog singleton.
server/NeonSprawl.Server/Game/Combat/AbilityDefinitionCatalog.cs	Tighten summary to reference IAbilityDefinitionRegistry (replace NEO-79 placeholder).
server/NeonSprawl.Server/Game/AbilityInput/HotbarLoadoutApi.cs	Inject registry; migrate allowlist validation to TryNormalizeKnown.
server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs	Inject registry; migrate request ability validation to TryNormalizeKnown.
server/NeonSprawl.Server/Game/AbilityInput/PrototypeAbilityRegistry.cs	Remove static allowlist/TryNormalizeKnown; keep id const strings for tests.
server/README.md	Document IAbilityDefinitionRegistry as preferred lookup; note hotbar/cast migration.

Tests

File	Coverage
server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionRegistryTests.cs	Unit: TryGetDefinition for prototype_pulse / prototype_burst with expected BaseDamage + CooldownSeconds; unknown id + null → false; TryNormalizeKnown with leading/trailing space and mixed case; empty/whitespace → false; GetDefinitionsInIdOrder count 4 + ordinal id order. Loader fixture: copy repo prototype_abilities.json via AbilityCatalogTestPaths, load catalog, assert all four freeze-box values. Host: InMemoryWebApplicationFactory resolves IAbilityDefinitionRegistry from DI; lookup + enumeration smoke.
Existing HotbarLoadoutApiTests, AbilityCastApiTests, HotbarLoadoutPersistenceIntegrationTests, CooldownSnapshotApiTests	Regression: no test changes expected if behavior preserved; run full AbilityInput test suite after migration.

No manual QA for this story (server-only DI + internal migration; HTTP proof is NEO-78). Optional Bruno deny smokes added: hotbar-loadout/Post loadout unknown ability deny.bru, ability-cast/Post cast unknown ability deny.bru — HTTP regression for unknown_ability after catalog-backed validation.

Open questions / risks

Question / risk	Agent recommendation	Status
Case-only mismatch — client sends id not in catalog after normalize	Deny unknown_ability — same as today; catalog ids are lowercase frozen strings.	adopted
Dual lookup surfaces — TryGetDefinition vs TryNormalizeKnown	Keep both — hotbar/cast need normalize-only; combat/HTTP need row lookup without re-implementing trim rules.	adopted
Test const indirection	Keep PrototypeAbilityRegistry const strings in existing tests — no mass string-literal churn.	adopted

Reconciliation (implementation)

• IAbilityDefinitionRegistry + AbilityDefinitionRegistry registered in AddAbilityDefinitionCatalog; catalog-only eager resolve unchanged in Program.cs.
• HotbarLoadoutApi / AbilityCastApi inject registry and call TryNormalizeKnown; PrototypeAbilityRegistry reduced to id constants.
• 10 new AAA tests in AbilityDefinitionRegistryTests; 31 existing AbilityInput tests unchanged and green (41 total in filtered run).
• Bruno: optional unknown_ability deny smokes under hotbar-loadout/ and ability-cast/ (see Tests section).