Merge pull request #64 from ViPro-Technologies/NEO-35-e2m1-skilldefinition-registry
NEO-35: SkillDefinitionRegistry service + lookup testspull/66/head
GitHub
commit
efad0e51b2
|
|
@ -50,7 +50,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not
|
|||
| E1.M2 | Ready | **Slice 2 prototype slice closed:** follow + `CameraState` ([NEO-15](https://linear.app/neon-sprawl/issue/NEO-15)); zoom bands ([NEO-16](https://linear.app/neon-sprawl/issue/NEO-16)); occlusion ([NEO-17](https://linear.app/neon-sprawl/issue/NEO-17)); occluder pick-through ([NEO-20](https://linear.app/neon-sprawl/issue/NEO-20)); contracts + hardening ([NEO-18](https://linear.app/neon-sprawl/issue/NEO-18)). Client-local; no server use of camera pose. | [NEO-15](../../plans/NEO-15-implementation-plan.md), [NEO-16](../../plans/NEO-16-implementation-plan.md), [NEO-17](../../plans/NEO-17-implementation-plan.md), [NEO-18](../../plans/NEO-18-implementation-plan.md), [NEO-20](../../plans/NEO-20-implementation-plan.md); `client/scripts/isometric_follow_camera.gd`, `camera_state.gd`, `zoom_band_config.gd`, `occlusion_policy.gd`, `ground_pick.gd`; [E1_M2_IsometricCameraController.md](E1_M2_IsometricCameraController.md) |
|
||||
| E1.M3 | In Progress | **NEO-23 landed:** JSON v1 targeting read + select (`GET`/`POST …/target`, `Game/Targeting/`, [NEO-23](../../plans/NEO-23-implementation-plan.md)) — `PlayerTargetStateResponse` / soft lock / `reasonCode` denials; wire name differs from illustrative **`TargetState`** in module doc (called out in plan + README). **NEO-24 landed:** client tab-target + lock HUD synced to server ([NEO-24](../../plans/NEO-24-implementation-plan.md)) — `TargetSelectionClient` ([`client/scripts/target_selection_client.gd`](../../../client/scripts/target_selection_client.gd)), Tab / Esc bindings, `TargetLockLabel` HUD, hybrid movement-triggered refresh via new `PositionAuthorityClient.authoritative_ack` signal (fires on every server-confirmed position, boot + `move-stream` 200) with a 250 ms cooldown; manual QA in [`docs/manual-qa/NEO-24.md`](../../manual-qa/NEO-24.md). **NEO-25 landed:** versioned **`GET /game/world/interactables`** ([NEO-25](../../plans/NEO-25-implementation-plan.md)) — `InteractablesListResponse` / `InteractablesWorldApi` in `server/NeonSprawl.Server/Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` (fetch-driven props + glow); [server README — Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25). **NEO-26 landed:** prototype **`SelectionEvent`** — **`TargetSelectionClient.selection_event`** ([NEO-26](../../plans/NEO-26-implementation-plan.md)) when **`lockedTargetId`** changes; optional **`log_selection_events`**. **NEO-27 landed:** Slice 3 telemetry hook sites documented — `selection_event` maps to product `target_changed` in `target_selection_client.gd`; server lock transition hook comments in `InMemoryPlayerTargetLockStore`; reserved `ability_cast_requested` / `ability_cast_denied` comments in `client/scripts/main.gd` (all TODO(E9.M1)); see [NEO-27](../../plans/NEO-27-implementation-plan.md) and [`docs/manual-qa/NEO-27.md`](../../manual-qa/NEO-27.md). **Follow-on / still open:** richer **`InteractableDescriptor`** consumers beyond list projection + existing `POST …/interact`; production telemetry ingest/catalog after E9.M1. **Precursor (E1.M1):** [NEO-9](../../plans/NEO-9-implementation-plan.md) `POST …/interact`. | Linear [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)–[NEO-27](https://linear.app/neon-sprawl/issue/NEO-27); [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md); [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md); [server README — Targeting](../../../server/README.md#targeting-neo-23), [Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25), [Interaction](../../../server/README.md#interaction-neo-9) |
|
||||
| E1.M4 | In Progress | **NEO-29 landed:** prototype `HotbarLoadout` v1 server contract (`GET`/`POST /game/players/{id}/hotbar-loadout`) with stable deny reason codes, per-player scope, and persistence policy matching NEO-8/NS-17 (Postgres when configured, in-memory fallback otherwise). Client boot hydration is wired via `hotbar_loadout_client.gd` + `hotbar_state.gd` from `main.gd`; manual QA checklist created. **NEO-31 landed:** prototype **`POST /game/players/{id}/ability-cast`**, digit-key cast wiring from `main.gd`, `ability_cast_client.gd`, and `hotbar_cast_slot_resolver.gd` (target id from `lockedTargetId` in cached target state); GdUnit covers resolver + HTTP client; manual QA [NEO-31](../../manual-qa/NEO-31.md). **NEO-28 landed:** cast POST validates **lock + registry + range** (`invalid_target`, `out_of_range`); **`AbilityCastClient.cast_result_received`** + **`CastFeedbackLabel`** HUD line; manual QA [NEO-28](../../manual-qa/NEO-28.md). **NEO-30 landed:** Slice 3 cast funnel telemetry **hook-site comments** in [`AbilityCastApi.cs`](../../../server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs) (`ability_cast_requested` on authoritative accept, `ability_cast_denied` + non-empty `reasonCode` on each JSON deny branch); client dev hooks unchanged; manual QA [NEO-30](../../manual-qa/NEO-30.md); plan [NEO-30](../../plans/NEO-30-implementation-plan.md). **NEO-32 landed:** `GET /game/players/{id}/cooldown-snapshot` + `on_cooldown` cast deny + prototype global cooldown commit; [`cooldown_snapshot_client.gd`](../../../client/scripts/cooldown_snapshot_client.gd), [`cooldown_state.gd`](../../../client/scripts/cooldown_state.gd), **`CooldownSlotsLabel`** in [`main.gd`](../../../client/scripts/main.gd); manual QA [NEO-32](../../manual-qa/NEO-32.md); plan [NEO-32](../../plans/NEO-32-implementation-plan.md). | [NEO-29](../../plans/NEO-29-implementation-plan.md), [NEO-31](../../plans/NEO-31-implementation-plan.md), [NEO-28](../../plans/NEO-28-implementation-plan.md), [NEO-30](../../plans/NEO-30-implementation-plan.md), [NEO-32](../../plans/NEO-32-implementation-plan.md); [E1_M4_AbilityInputScaffold.md](E1_M4_AbilityInputScaffold.md); [E1M4 prototype backlog](../../plans/E1M4-prototype-backlog.md); `server/NeonSprawl.Server/Game/AbilityInput/`; [`client/scripts/hotbar_loadout_client.gd`](../../../client/scripts/hotbar_loadout_client.gd), [`client/scripts/hotbar_state.gd`](../../../client/scripts/hotbar_state.gd), [`client/scripts/ability_cast_client.gd`](../../../client/scripts/ability_cast_client.gd), [`client/scripts/hotbar_cast_slot_resolver.gd`](../../../client/scripts/hotbar_cast_slot_resolver.gd), [`client/scripts/cooldown_snapshot_client.gd`](../../../client/scripts/cooldown_snapshot_client.gd), [`client/scripts/cooldown_state.gd`](../../../client/scripts/cooldown_state.gd); [server README — Hotbar loadout](../../../server/README.md#hotbar-loadout-neo-29), [Ability cast (NEO-31)](../../../server/README.md#ability-cast-neo-31), [Cooldown snapshot (NEO-32)](../../../server/README.md#cooldown-snapshot-neo-32) |
|
||||
| E2.M1 | In Progress | **NEO-33 landed:** frozen prototype trio **`salvage`** / **`refine`** / **`intrusion`** in [`content/skills/prototype_skills.json`](../../../content/skills/prototype_skills.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) enforce schema, duplicate `id`, exact trio ids, and category coverage; designer note on **`allowedXpSourceKinds`** in [E2_M1](E2_M1_SkillDefinitionRegistry.md) + [`content/README.md`](../../../content/README.md). **NEO-34 landed:** fail-fast server load of `content/skills/*.json` at startup — `server/NeonSprawl.Server/Game/Skills/` ([NEO-34](../../plans/NEO-34-implementation-plan.md)); config + discovery in [server README — Skill catalog](../../../server/README.md#skill-catalog-contentskills-neo-34). **Follow-on:** registry service ([NEO-35](https://linear.app/neon-sprawl/issue/NEO-35)), read HTTP + Bruno ([NEO-36](https://linear.app/neon-sprawl/issue/NEO-36)). | [NEO-33](../../plans/NEO-33-implementation-plan.md), [NEO-34](../../plans/NEO-34-implementation-plan.md); [E2_M1](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note**; `server/NeonSprawl.Server/Game/Skills/` |
|
||||
| E2.M1 | In Progress | **NEO-33 landed:** frozen prototype trio **`salvage`** / **`refine`** / **`intrusion`** in [`content/skills/prototype_skills.json`](../../../content/skills/prototype_skills.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) enforce schema, duplicate `id`, exact trio ids, and category coverage; designer note on **`allowedXpSourceKinds`** in [E2_M1](E2_M1_SkillDefinitionRegistry.md) + [`content/README.md`](../../../content/README.md). **NEO-34 landed:** fail-fast server load of `content/skills/*.json` at startup — `server/NeonSprawl.Server/Game/Skills/` ([NEO-34](../../plans/NEO-34-implementation-plan.md)); config + discovery in [server README — Skill catalog](../../../server/README.md#skill-catalog-contentskills-neo-34). **NEO-35 landed:** `ISkillDefinitionRegistry` / `SkillDefinitionRegistry` + DI ([NEO-35](../../plans/NEO-35-implementation-plan.md)). **Follow-on:** read HTTP + Bruno ([NEO-36](https://linear.app/neon-sprawl/issue/NEO-36)). | [NEO-33](../../plans/NEO-33-implementation-plan.md), [NEO-34](../../plans/NEO-34-implementation-plan.md), [NEO-35](../../plans/NEO-35-implementation-plan.md); [E2_M1](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note**; `server/NeonSprawl.Server/Game/Skills/` |
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,77 @@
|
|||
# NEO-35 — Implementation plan
|
||||
|
||||
## Story reference
|
||||
|
||||
| Field | Value |
|
||||
|--------|--------|
|
||||
| **Key** | NEO-35 |
|
||||
| **Title** | E2.M1: SkillDefinitionRegistry service + lookup tests |
|
||||
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-35/e2m1-skilldefinitionregistry-service-lookup-tests |
|
||||
|
||||
## Kickoff clarifications
|
||||
|
||||
| Topic | Question | Answer |
|
||||
|--------|------------|--------|
|
||||
| Scope / sequencing | Any open decisions before planning? | **None.** Linear description, [E2.M1 — SkillDefinitionRegistry](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md), and shipped **NEO-34** (`SkillDefinitionCatalog`, `SkillDefRow`, loader, DI) fix scope, acceptance, and out-of-scope items. **NEO-34** is **Done**, so the dependency chain is satisfied for implementation. |
|
||||
| Kickoff continuation | User asked to continue kickoff after board/relationship clarification | Proceed with this plan on branch `NEO-35-e2m1-skilldefinition-registry`; no additional product questions pending. |
|
||||
|
||||
## Goal, scope, and out-of-scope
|
||||
|
||||
**Goal:** Provide a small **`ISkillDefinitionRegistry`** (name as in Linear; implementation class name may be `SkillDefinitionRegistry`) **backed by** the startup-loaded **`SkillDefinitionCatalog`**: resolve by stable **`id`**, enumerate definitions, read **`category`**, **`displayName`**, and **`allowedXpSourceKinds`**. Surface is **registered in DI** so **NEO-36** (HTTP/read model) and **E2.M2** callers can depend on the interface. Document on the interface (and briefly in code where helpful) how **E2.M2** will use this for **grant validation** (`allowedXpSourceKinds` vs `XpGrantEvent.sourceKind`).
|
||||
|
||||
**In scope (from Linear):**
|
||||
|
||||
- C# types aligned with [`content/schemas/skill-def.schema.json`](../../content/schemas/skill-def.schema.json) for **prototype fields** (reuse or thinly wrap existing **`SkillDefRow`** from NEO-34 — avoid duplicating divergent models).
|
||||
- Unit tests: known prototype ids resolve; **unknown id** returns absent / false **without throwing**; assert at least one **`allowedXpSourceKinds`** expectation on a known skill.
|
||||
- XML / remarks on the public contract for **E2.M2** and future callers.
|
||||
|
||||
**Out of scope (from Linear):**
|
||||
|
||||
- **`UnlockRequirement`** gating logic (future slice; schema has no such field today).
|
||||
- Wiring into the XP engine (**E2.M2**).
|
||||
|
||||
## Acceptance criteria checklist
|
||||
|
||||
- [x] Tests: **happy path** — known prototype `id` (e.g. from existing test fixtures / `salvage` trio) resolves with expected metadata.
|
||||
- [x] Tests: **unknown id** — lookup indicates absence / `Try*` returns false **without throwing**.
|
||||
- [x] Tests: **`allowedXpSourceKinds`** — at least one assertion on a known skill’s list (per E2.M1 contract).
|
||||
- [x] **`ISkillDefinitionRegistry`** (or chosen public interface name) is **registered in DI** alongside the catalog so **NEO-36** can inject it without reaching for `SkillDefinitionCatalog` internals.
|
||||
|
||||
## Technical approach
|
||||
|
||||
1. **Interface:** `ISkillDefinitionRegistry` with:
|
||||
- **`TryGet`** (or equivalent) for **id → definition** with non-throwing miss semantics.
|
||||
- **Enumeration** of all definitions (e.g. `IEnumerable<SkillDefRow>` or read-only collection) for projections.
|
||||
- Document that **E2.M2** must reject XP grants when `sourceKind` ∉ `AllowedXpSourceKinds` for the target skill (behavior deferred to E2.M2; this story only exposes data).
|
||||
|
||||
2. **Implementation:** `SkillDefinitionRegistry` as a **thin adapter** over **`SkillDefinitionCatalog`** (constructor-injected singleton dependency on `SkillDefinitionCatalog`).
|
||||
|
||||
3. **DI:** Extend **`AddSkillDefinitionCatalog`** (or adjacent extension in the same file) to register **`ISkillDefinitionRegistry`** as singleton after / with the catalog registration. No requirement to eagerly-resolve the registry in **`Program.cs`** if **`SkillDefinitionCatalog`** is already resolved (NEO-34); optional `_ = app.Services.GetRequiredService<ISkillDefinitionRegistry>()` only if we want symmetry — default **omit** unless a test or ordering reason appears.
|
||||
|
||||
4. **Models:** Prefer exposing **`SkillDefRow`** on the interface (already matches schema fields). If a separate public DTO is needed for naming only, keep it a **record with identical fields** and map in one line — avoid a second parallel shape long-term.
|
||||
|
||||
## Files to add
|
||||
|
||||
| Path | Purpose |
|
||||
|------|---------|
|
||||
| `server/NeonSprawl.Server/Game/Skills/ISkillDefinitionRegistry.cs` | Public contract: lookup, enumerate, XML for E2.M2 / NEO-36. |
|
||||
| `server/NeonSprawl.Server/Game/Skills/SkillDefinitionRegistry.cs` | Singleton adapter over `SkillDefinitionCatalog`. |
|
||||
|
||||
## Files to modify
|
||||
|
||||
| Path | Rationale |
|
||||
|------|-----------|
|
||||
| `server/NeonSprawl.Server/Game/Skills/SkillCatalogServiceCollectionExtensions.cs` | Register `ISkillDefinitionRegistry` → `SkillDefinitionRegistry` after catalog registration. |
|
||||
| `server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalog.cs` | Optional: tighten summary comment if it still says “NEO-35 may wrap” once registry exists. |
|
||||
|
||||
## Tests
|
||||
|
||||
| Test file | What it covers |
|
||||
|-----------|------------------|
|
||||
| `server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionRegistryTests.cs` | **Unit:** build a **`SkillDefinitionCatalog`** in-memory (same constructor as production) with a minimal `ById` dictionary **or** load via **`SkillDefinitionCatalogLoader`** using the existing temp-layout helper pattern from **`SkillDefinitionCatalogLoaderTests`**; assert **`TryGet`** for `salvage` (or trio) returns true and **`AllowedXpSourceKinds`** contains expected values; assert unknown id returns false and does not throw; assert enumeration count matches catalog. **AAA** labels per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert). |
|
||||
|
||||
Optional (only if valuable for regression): one assertion in **`SkillDefinitionCatalogLoaderTests`** host test that **`ISkillDefinitionRegistry`** resolves from **`InMemoryWebApplicationFactory`** — not strictly required if unit tests + extension registration are sufficient.
|
||||
|
||||
## Open questions / risks
|
||||
|
||||
**None.** If **`SkillDefRow`** is later replaced by enums for category / source kinds, the registry interface can evolve in a follow-up issue without blocking NEO-36 string-based JSON projection.
|
||||
|
|
@ -0,0 +1,45 @@
|
|||
# Code review: NEO-35 (SkillDefinitionRegistry)
|
||||
|
||||
**Date:** 2026-05-04
|
||||
|
||||
**Scope:** Branch `NEO-35-e2m1-skilldefinition-registry` vs `origin/main` (ahead 2 commits): `ISkillDefinitionRegistry`, `SkillDefinitionRegistry`, DI registration, `SkillDefinitionRegistryTests`, plan `docs/plans/NEO-35-implementation-plan.md`, comment tweak on `SkillDefinitionCatalog`.
|
||||
|
||||
**Base:** `origin/main`
|
||||
|
||||
## Verdict
|
||||
|
||||
**Approve with nits**
|
||||
|
||||
## Summary
|
||||
|
||||
The change adds a thin registry adapter over `SkillDefinitionCatalog` with `TryGetDefinition`, ordered enumeration, XML guidance for E2.M2 and NEO-36, and singleton DI registration. Unit tests cover happy path, unknown id, id-ordered listing, loader-backed consistency, and host DI resolution. Behavior matches the NEO-35 plan and E2.M1 module intent for this slice (read-only metadata; grant enforcement remains E2.M2). `dotnet test` on `NeonSprawl.Server.Tests` passes (110 tests).
|
||||
|
||||
## Documentation checked
|
||||
|
||||
| Document | Result |
|
||||
|----------|--------|
|
||||
| `docs/plans/NEO-35-implementation-plan.md` | **Matches** — interface + adapter + DI + tests align with acceptance checklist and technical approach (`TryGet*` semantics, `SkillDefRow`, enumeration, AAA tests). |
|
||||
| `docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md` | **Matches** — exposes stable ids, categories, display names, `allowedXpSourceKinds`; documents E2.M2 validation expectation on the interface; does not implement unlock gating or XP engine (correctly out of scope). |
|
||||
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E2.M1 row describes registry / `allowedXpSourceKinds`; implementation fits. |
|
||||
| `docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md` (cross-read) | **Matches** — interface remarks align with “reject grants when `sourceKind` not listed.” |
|
||||
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E2.M1 snapshot row updated for NEO-35 landed. |
|
||||
|
||||
## Blocking issues
|
||||
|
||||
(none)
|
||||
|
||||
## Suggestions
|
||||
|
||||
1. ~~**`Host_ShouldResolveRegistryFromDi_WhenStartupSucceeds` AAA (`SkillDefinitionRegistryTests.cs`)** — `// Act` currently performs `GetAsync("/health")`, `GetRequiredService<ISkillDefinitionRegistry>()`, and `TryGetDefinition` together. Per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), Act should be the behavior under test only (typically one call, or a short intentional sequence for one scenario). Prefer moving `/health` to **Arrange** as a startup warm-up if ordering matters, dropping it if `CreateClient()` already builds the host enough for the catalog, or splitting host-health vs DI-resolution into separate tests so each method has a single clear Act.~~ **Done.**
|
||||
|
||||
2. ~~**Post-merge doc hygiene** — Update `documentation_and_implementation_alignment.md` E2.M1 / NEO-35 line when this story merges so the implementation tracking table matches reality.~~ **Done.**
|
||||
|
||||
## Nits
|
||||
|
||||
1. ~~**Nit:** `finally` block uses an empty `catch { }` around `Directory.Delete` in `TryGetDefinition_ShouldMatchLoaderCatalog_WhenUsingPrototypeFixture`. [csharp-style](../../.cursor/rules/csharp-style.md) prefers avoiding empty catches; for tests, catching a specific exception type (or a tiny helper) is enough if you want to silence flaky cleanup on Windows/locked files.~~ **Done.**
|
||||
|
||||
2. ~~**Nit:** `TryGetDefinition` with a `null` `skillId` will throw from the underlying dictionary lookup; the XML says misses do not throw. That is fine for normal string ids; if you want the contract to include null as “not found,” add an explicit guard—only worth it if public API should treat null like unknown.~~ **Done.**
|
||||
|
||||
## Verification
|
||||
|
||||
- `dotnet test server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj` — all tests passed at review time.
|
||||
|
|
@ -0,0 +1,171 @@
|
|||
using System.IO;
|
||||
using System.Text;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using NeonSprawl.Server.Game.Skills;
|
||||
using NeonSprawl.Server.Tests;
|
||||
using Xunit;
|
||||
|
||||
namespace NeonSprawl.Server.Tests.Game.Skills;
|
||||
|
||||
public class SkillDefinitionRegistryTests
|
||||
{
|
||||
private static SkillDefinitionRegistry CreateRegistryFromRows(IReadOnlyDictionary<string, SkillDefRow> byId)
|
||||
{
|
||||
var catalog = new SkillDefinitionCatalog("/tmp/catalog", byId, catalogJsonFileCount: 1);
|
||||
return new SkillDefinitionRegistry(catalog);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TryGetDefinition_ShouldReturnTrueAndExpectedXpKinds_WhenIdExists()
|
||||
{
|
||||
// Arrange
|
||||
var rows = new Dictionary<string, SkillDefRow>(StringComparer.Ordinal)
|
||||
{
|
||||
["intrusion"] = new SkillDefRow(
|
||||
"intrusion",
|
||||
"tech",
|
||||
"Intrusion",
|
||||
new[] { "activity", "mission_reward", "book_or_item" }),
|
||||
};
|
||||
var registry = CreateRegistryFromRows(rows);
|
||||
// Act
|
||||
var found = registry.TryGetDefinition("intrusion", out var def);
|
||||
// Assert
|
||||
Assert.True(found);
|
||||
Assert.NotNull(def);
|
||||
Assert.Equal("tech", def.Category);
|
||||
Assert.Equal("Intrusion", def.DisplayName);
|
||||
Assert.Contains("activity", def.AllowedXpSourceKinds, StringComparer.Ordinal);
|
||||
Assert.Contains("book_or_item", def.AllowedXpSourceKinds, StringComparer.Ordinal);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TryGetDefinition_ShouldReturnFalse_WhenSkillIdIsNull()
|
||||
{
|
||||
// Arrange
|
||||
var rows = new Dictionary<string, SkillDefRow>(StringComparer.Ordinal)
|
||||
{
|
||||
["salvage"] = new SkillDefRow("salvage", "gather", "Salvage", new[] { "activity" }),
|
||||
};
|
||||
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<string, SkillDefRow>(StringComparer.Ordinal)
|
||||
{
|
||||
["salvage"] = new SkillDefRow("salvage", "gather", "Salvage", new[] { "activity" }),
|
||||
};
|
||||
var registry = CreateRegistryFromRows(rows);
|
||||
// Act
|
||||
var found = registry.TryGetDefinition("not_a_real_skill", out var def);
|
||||
// Assert
|
||||
Assert.False(found);
|
||||
Assert.Null(def);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetDefinitionsInIdOrder_ShouldListAllRowsOrderedById_WhenMultipleSkills()
|
||||
{
|
||||
// Arrange
|
||||
var rows = new Dictionary<string, SkillDefRow>(StringComparer.Ordinal)
|
||||
{
|
||||
["salvage"] = new SkillDefRow("salvage", "gather", "Salvage", new[] { "activity", "mission_reward" }),
|
||||
["refine"] = new SkillDefRow("refine", "process", "Refine", new[] { "activity" }),
|
||||
["intrusion"] = new SkillDefRow("intrusion", "tech", "Intrusion", new[] { "trainer" }),
|
||||
};
|
||||
var registry = CreateRegistryFromRows(rows);
|
||||
// Act
|
||||
var list = registry.GetDefinitionsInIdOrder();
|
||||
// Assert
|
||||
Assert.Equal(3, list.Count);
|
||||
Assert.Equal("intrusion", list[0].Id);
|
||||
Assert.Equal("refine", list[1].Id);
|
||||
Assert.Equal("salvage", list[2].Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TryGetDefinition_ShouldMatchLoaderCatalog_WhenUsingPrototypeFixture()
|
||||
{
|
||||
// Arrange
|
||||
var root = Directory.CreateTempSubdirectory("neon-sprawl-registry-loader-");
|
||||
try
|
||||
{
|
||||
var skillsDir = Path.Combine(root.FullName, "content", "skills");
|
||||
var schemaDir = Path.Combine(root.FullName, "content", "schemas");
|
||||
Directory.CreateDirectory(skillsDir);
|
||||
Directory.CreateDirectory(schemaDir);
|
||||
var schemaPath = Path.Combine(schemaDir, "skill-def.schema.json");
|
||||
File.Copy(SkillCatalogTestPaths.DiscoverRepoSkillDefSchemaPath(), schemaPath, overwrite: true);
|
||||
var json = """
|
||||
{
|
||||
"skills": [
|
||||
{
|
||||
"id": "salvage",
|
||||
"category": "gather",
|
||||
"displayName": "Salvage",
|
||||
"allowedXpSourceKinds": ["activity", "mission_reward"]
|
||||
},
|
||||
{
|
||||
"id": "refine",
|
||||
"category": "process",
|
||||
"displayName": "Refine",
|
||||
"allowedXpSourceKinds": ["activity", "mission_reward", "trainer"]
|
||||
},
|
||||
{
|
||||
"id": "intrusion",
|
||||
"category": "tech",
|
||||
"displayName": "Intrusion",
|
||||
"allowedXpSourceKinds": ["activity", "mission_reward", "book_or_item"]
|
||||
}
|
||||
]
|
||||
}
|
||||
""";
|
||||
File.WriteAllText(Path.Combine(skillsDir, "prototype.json"), json, Encoding.UTF8);
|
||||
var loaded = SkillDefinitionCatalogLoader.Load(skillsDir, schemaPath, NullLogger.Instance);
|
||||
var registry = new SkillDefinitionRegistry(loaded);
|
||||
// Act
|
||||
var ok = registry.TryGetDefinition("refine", out var refine);
|
||||
// Assert
|
||||
Assert.True(ok);
|
||||
Assert.NotNull(refine);
|
||||
Assert.Equal("process", refine.Category);
|
||||
Assert.Contains("trainer", refine.AllowedXpSourceKinds, StringComparer.Ordinal);
|
||||
}
|
||||
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<ISkillDefinitionRegistry>();
|
||||
var found = registry.TryGetDefinition("salvage", out var salvage);
|
||||
// Assert
|
||||
Assert.True(found);
|
||||
Assert.NotNull(salvage);
|
||||
Assert.Contains("activity", salvage.AllowedXpSourceKinds, StringComparer.Ordinal);
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,21 @@
|
|||
using System.Diagnostics.CodeAnalysis;
|
||||
|
||||
namespace NeonSprawl.Server.Game.Skills;
|
||||
|
||||
/// <summary>
|
||||
/// Read-only access to validated <see cref="SkillDefRow"/> entries loaded at startup (<see cref="SkillDefinitionCatalog"/>).
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para><b>E2.M2 (XP grants):</b> callers issuing skill XP must ensure <c>XpGrantEvent.sourceKind</c> (or successor)
|
||||
/// is listed on the target skill’s <see cref="SkillDefRow.AllowedXpSourceKinds"/>; the award engine should reject
|
||||
/// grants outside that set once implemented.</para>
|
||||
/// <para><b>NEO-36:</b> HTTP/read-model projections should depend on this interface rather than reaching into the catalog.</para>
|
||||
/// </remarks>
|
||||
public interface ISkillDefinitionRegistry
|
||||
{
|
||||
/// <summary>Attempts to resolve a skill by stable <c>id</c> (see <c>skill-def.schema.json</c>). Unknown ids and <c>null</c> return <c>false</c> without throwing.</summary>
|
||||
bool TryGetDefinition(string? skillId, [NotNullWhen(true)] out SkillDefRow? definition);
|
||||
|
||||
/// <summary>Every loaded definition, ordered by <see cref="SkillDefRow.Id"/> (ordinal).</summary>
|
||||
IReadOnlyList<SkillDefRow> GetDefinitionsInIdOrder();
|
||||
}
|
||||
|
|
@ -7,7 +7,7 @@ namespace NeonSprawl.Server.Game.Skills;
|
|||
/// <summary>DI registration for the fail-fast skill catalog (NEO-34).</summary>
|
||||
public static class SkillCatalogServiceCollectionExtensions
|
||||
{
|
||||
/// <summary>Binds <see cref="ContentPathsOptions"/> and registers <see cref="SkillDefinitionCatalog"/> as a singleton.</summary>
|
||||
/// <summary>Binds <see cref="ContentPathsOptions"/> and registers <see cref="SkillDefinitionCatalog"/> and <see cref="ISkillDefinitionRegistry"/> as singletons.</summary>
|
||||
public static IServiceCollection AddSkillDefinitionCatalog(this IServiceCollection services, IConfiguration configuration)
|
||||
{
|
||||
services.AddOptions<ContentPathsOptions>()
|
||||
|
|
@ -29,6 +29,9 @@ public static class SkillCatalogServiceCollectionExtensions
|
|||
return SkillDefinitionCatalogLoader.Load(skillsDir, schemaPath, logger);
|
||||
});
|
||||
|
||||
services.AddSingleton<ISkillDefinitionRegistry>(sp =>
|
||||
new SkillDefinitionRegistry(sp.GetRequiredService<SkillDefinitionCatalog>()));
|
||||
|
||||
return services;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ using System.Collections.ObjectModel;
|
|||
|
||||
namespace NeonSprawl.Server.Game.Skills;
|
||||
|
||||
/// <summary>In-memory skill catalog loaded at startup (NEO-34). NEO-35 may wrap with lookup services.</summary>
|
||||
/// <summary>In-memory skill catalog loaded at startup (NEO-34). Game code should prefer <see cref="ISkillDefinitionRegistry"/> for lookups (NEO-35).</summary>
|
||||
public sealed class SkillDefinitionCatalog
|
||||
{
|
||||
public SkillDefinitionCatalog(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,39 @@
|
|||
using System.Diagnostics.CodeAnalysis;
|
||||
|
||||
namespace NeonSprawl.Server.Game.Skills;
|
||||
|
||||
/// <summary>Adapter over <see cref="SkillDefinitionCatalog"/> (NEO-35).</summary>
|
||||
public sealed class SkillDefinitionRegistry(SkillDefinitionCatalog catalog) : ISkillDefinitionRegistry
|
||||
{
|
||||
/// <inheritdoc />
|
||||
public bool TryGetDefinition(string? skillId, [NotNullWhen(true)] out SkillDefRow? definition)
|
||||
{
|
||||
if (skillId is null)
|
||||
{
|
||||
definition = null;
|
||||
return false;
|
||||
}
|
||||
|
||||
if (catalog.ById.TryGetValue(skillId, out var row))
|
||||
{
|
||||
definition = row;
|
||||
return true;
|
||||
}
|
||||
|
||||
definition = null;
|
||||
return false;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public IReadOnlyList<SkillDefRow> GetDefinitionsInIdOrder()
|
||||
{
|
||||
var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray();
|
||||
var list = new List<SkillDefRow>(ids.Length);
|
||||
foreach (var id in ids)
|
||||
{
|
||||
list.Add(catalog.ById[id]);
|
||||
}
|
||||
|
||||
return list;
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue