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

86 lines
6.1 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# NEO-37 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-37 |
| **Title** | Skill progression snapshot (read model) |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-37/skill-progression-snapshot-read-model |
| **Module** | [E2.M2 — XpAwardAndLevelEngine](../decomposition/modules/E2_M2_XpAwardAndLevelEngine.md) |
## Kickoff clarifications
**Asked (Agent `AskQuestion`):** Lock plan defaults for route, 404 rule, JSON shape (`schemaVersion` + `playerId` + `skills` with `id` / `xp` / `level`), Bruno folder style, and `docs/manual-qa/NEO-37.md`.
**Answer:** User chose **Proceed — use these defaults in the plan**.
## Goal, scope, and out-of-scope
**Goal:** Ship a versioned **GET** read model for per-player skill XP and level for every `SkillDef` id from **`ISkillDefinitionRegistry`**, bootstrapping **xp = 0** and **level = 1** before any grants exist. Match **NEO-36** JSON envelope patterns (`schemaVersion`, ordered rows, `System.Text.Json` property names).
**In scope (from Linear):**
- Versioned JSON for the agreed player-scoped path.
- One row per registered skill id; coherent defaults (0 XP, level 1).
- Unit/integration tests (AAA) and Bruno collection folder mirroring NEO-36 style.
- Server README section for the endpoint.
**Out of scope (from Linear):**
- Applying XP grants ([NEO-38](https://linear.app/neon-sprawl/issue/NEO-38/apply-skill-xp-grant-allowlist-level-up)).
- Data-driven level curves (separate issue; this slice uses fixed baseline level **1** for all rows).
- Gather/craft/combat integration (Epic 2 Slice 3).
## Acceptance criteria checklist
- [x] Versioned JSON response for **`GET /game/players/{id}/skill-progression`** (`schemaVersion` **1**).
- [x] One row per registered skill id; **`xp`** = **0**, **`level`** = **1** for all rows in this story; **`skills`** ordered by **`ISkillDefinitionRegistry.GetDefinitionsInIdOrder()`**.
- [x] Automated tests (AAA); Bruno folder mirroring NEO-36 (`skill-definitions` conventions).
- [x] **`server/README.md`** section documenting the endpoint (curl + pointers to plan / manual QA / Bruno).
## Technical approach
1. **Route:** **`GET /game/players/{id}/skill-progression`** alongside existing **`/game/players/{id}/…`** reads (`cooldown-snapshot`, `hotbar-loadout`, etc.).
2. **Player gate:** **`404`** when **`!IPositionStateStore.TryGetPosition(id, …)`**, matching **`CooldownSnapshotApi`** (known player/session only).
3. **Response:** Top-level **`schemaVersion`** (**1**, const on a response type like NEO-36s **`SkillDefinitionsListResponse.CurrentSchemaVersion`**), **`playerId`** (**`id.Trim()`**, consistent with **`HotbarLoadoutResponse`**), and **`skills`**: array of objects **`id`** (skill def id), **`xp`** (int, **0**), **`level`** (int, **1**). Do **not** duplicate catalog fields (`displayName`, etc.); clients join with **`GET /game/world/skill-definitions`** if needed.
4. **Data source:** For NEO-37, build the array **only** from the registry (no persistence). **NEO-38** will introduce stored progression and merge/override defaults; until then there is **no separate store interface** unless implementation discovers a need for testing seams—prefer a small private/static builder method next to the route to keep diff small.
5. **Bruno:** Add **`bruno/neon-sprawl-server/skill-progression/`** with **`Get skill progression.bru`** (optional **`folder.bru`** matching sibling folders). Use **`{{baseUrl}}`**; tests assert **200**, JSON, **`schemaVersion`**, **`playerId`**, **`skills`** length **3**, ordinal **id** order (`intrusion`, `refine`, `salvage`), and **`xp`/`level`** defaults on one row.
6. **README:** Add a subsection after the NEO-36 skill definitions block — curl example for **`dev-local-1`** (or generic `{id}`) and links to this plan, **`docs/manual-qa/NEO-37.md`**, and Bruno path.
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs` | `Map*` extension registering the GET route; position check + registry → JSON. |
| `server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotDtos.cs` | Versioned response + per-skill row DTOs (`schemaVersion`, `playerId`, `skills`). |
| `server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionSnapshotApiTests.cs` | **`InMemoryWebApplicationFactory`**: success for **`dev-local-1`**; **404** for unknown player; **ReadFromJsonAsync** asserts schema, order, defaults. |
| `bruno/neon-sprawl-server/skill-progression/Get skill progression.bru` | Manual / Bruno verification (mirror **`skill-definitions/Get skill definitions.bru`** style). |
| `bruno/neon-sprawl-server/skill-progression/folder.bru` | Optional; align with Bruno folder metadata used beside **`skill-definitions`**. |
| `docs/manual-qa/NEO-37.md` | Short checklist: run server, GET with known vs unknown player id. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Program.cs` | Call **`app.MapSkillProgressionSnapshotApi()`** (exact name may match class) next to other `Map*Api` registrations. |
| `server/README.md` | Document **`GET /game/players/{id}/skill-progression`**, curl, and links (plan / manual QA / Bruno). |
## Tests
| Test file | What it covers |
|-----------|------------------|
| `server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionSnapshotApiTests.cs` | **`GET`** returns **404** for **`missing-player`** (or equivalent unknown id consistent with **`CooldownSnapshotApiTests`**). **`GET`** for **`dev-local-1`** returns **200**, **`schemaVersion`** **1**, **`playerId`** matches, **`skills`** ids **`intrusion`**, **`refine`**, **`salvage`**, each **`xp`** **0**, **`level`** **1**. **AAA** comments per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert). |
Bruno scripts complement automated coverage; they do not replace it.
## Open questions / risks
- **NEO-38 merge point:** When grants land,prefer **replacing** the pure “zeros builder” with a store-backed projection without changing the **public JSON contract** (only values change). If **`schemaVersion`** must bump later, coordinate with clients.
- **None** otherwise.