Merge pull request #69 from ViPro-Technologies/NEO-37-skill-progression-snapshot-read-model
NEO-37: skill progression snapshot read modelpull/70/head
GitHub
commit
3e6cda6383
|
|
@ -0,0 +1,39 @@
|
|||
meta {
|
||||
name: GET skill progression
|
||||
type: http
|
||||
seq: 1
|
||||
}
|
||||
|
||||
docs {
|
||||
NEO-37: per-player read model; join with GET /game/world/skill-definitions for display names.
|
||||
}
|
||||
|
||||
get {
|
||||
url: {{baseUrl}}/game/players/dev-local-1/skill-progression
|
||||
body: none
|
||||
auth: none
|
||||
}
|
||||
|
||||
tests {
|
||||
test("returns 200 JSON with schema v1 and skills array", function () {
|
||||
expect(res.getStatus()).to.equal(200);
|
||||
expect(res.getHeader("content-type")).to.contain("application/json");
|
||||
const body = res.getBody();
|
||||
expect(body.schemaVersion).to.equal(1);
|
||||
expect(body.playerId).to.equal("dev-local-1");
|
||||
expect(body.skills).to.be.an("array");
|
||||
expect(body.skills.length).to.equal(3);
|
||||
});
|
||||
|
||||
test("frozen prototype trio with default xp and level", function () {
|
||||
const body = res.getBody();
|
||||
const intrusion = body.skills.find((x) => x.id === "intrusion");
|
||||
expect(intrusion).to.be.an("object");
|
||||
expect(intrusion.xp).to.equal(0);
|
||||
expect(intrusion.level).to.equal(1);
|
||||
const ids = new Set(body.skills.map((x) => x.id));
|
||||
expect(ids.has("salvage")).to.equal(true);
|
||||
expect(ids.has("refine")).to.equal(true);
|
||||
expect(ids.has("intrusion")).to.equal(true);
|
||||
});
|
||||
}
|
||||
|
|
@ -0,0 +1,3 @@
|
|||
meta {
|
||||
name: skill-progression
|
||||
}
|
||||
|
|
@ -7,11 +7,13 @@
|
|||
| **Module ID** | E2.M2 |
|
||||
| **Epic** | [Epic 2 — Skills and Progression Framework](../epics/epic_02_skills_and_progression.md) |
|
||||
| **Stage target** | Prototype |
|
||||
| **Status** | Planned (see [dependency register](module_dependency_register.md)) |
|
||||
| **Status** | In Progress (see [dependency register](module_dependency_register.md)) |
|
||||
|
||||
## Implementation snapshot
|
||||
|
||||
**Linear (label `E2.M2`):** [Epic 2 project](https://linear.app/neon-sprawl/project/epic-2-classless-skill-and-progression-framework-5200f84bf8b6) — **Slice 2:** [NEO-37](https://linear.app/neon-sprawl/issue/NEO-37) (progression read snapshot) → [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) (apply grant + allowlist + level-up) → [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39) (data-driven `LevelCurve` + CI); [NEO-40](https://linear.app/neon-sprawl/issue/NEO-40) (telemetry hook sites) after NEO-38, parallel with NEO-39. **Slice 3 integration** (multi-label where noted): [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41), [NEO-42](https://linear.app/neon-sprawl/issue/NEO-42), [NEO-43](https://linear.app/neon-sprawl/issue/NEO-43), [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). No implementation landed yet; update this section and the [alignment](documentation_and_implementation_alignment.md) row when stories merge.
|
||||
**Linear (label `E2.M2`):** [Epic 2 project](https://linear.app/neon-sprawl/project/epic-2-classless-skill-and-progression-framework-5200f84bf8b6) — **Slice 2:** [NEO-37](https://linear.app/neon-sprawl/issue/NEO-37) (progression read snapshot) → [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) (apply grant + allowlist + level-up) → [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39) (data-driven `LevelCurve` + CI); [NEO-40](https://linear.app/neon-sprawl/issue/NEO-40) (telemetry hook sites) after NEO-38, parallel with NEO-39. **Slice 3 integration** (multi-label where noted): [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41), [NEO-42](https://linear.app/neon-sprawl/issue/NEO-42), [NEO-43](https://linear.app/neon-sprawl/issue/NEO-43), [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44).
|
||||
|
||||
**NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([implementation plan](../../plans/NEO-37-implementation-plan.md)) — `SkillProgressionSnapshotApi` / `SkillProgressionSnapshotDtos` under `server/NeonSprawl.Server/Game/Skills/`; one row per `ISkillDefinitionRegistry` skill (**xp** 0, **level** 1 until grants persist); **`404`** when the player has no position (same gate as **`GET …/cooldown-snapshot`**). Bruno **`bruno/neon-sprawl-server/skill-progression/`**; manual QA **[`NEO-37.md`](../../manual-qa/NEO-37.md)**; **[server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37)**. **Still backlog within this module:** applying XP grants, `LevelCurve`, `LevelUpEvent`, telemetry hook sites (**NEO-38** onward). Keep this paragraph and [documentation tracking](documentation_and_implementation_alignment.md) in sync as Slice 2 stories merge.
|
||||
|
||||
## Purpose
|
||||
|
||||
|
|
|
|||
|
|
@ -51,7 +51,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not
|
|||
| 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). **NEO-35 landed:** `ISkillDefinitionRegistry` / `SkillDefinitionRegistry` + DI ([NEO-35](../../plans/NEO-35-implementation-plan.md)). **NEO-36 landed:** versioned **`GET /game/world/skill-definitions`** ([NEO-36](../../plans/NEO-36-implementation-plan.md)) — `SkillDefinitionsWorldApi` + `SkillDefinitionsListDtos` in `server/NeonSprawl.Server/Game/Skills/`; Bruno `bruno/neon-sprawl-server/skill-definitions/`; manual QA [`NEO-36`](../../manual-qa/NEO-36.md). **Follow-on (E2.M2):** XP grant validation vs `allowedXpSourceKinds` per [E2_M1](E2_M1_SkillDefinitionRegistry.md). | [NEO-33](../../plans/NEO-33-implementation-plan.md), [NEO-34](../../plans/NEO-34-implementation-plan.md), [NEO-35](../../plans/NEO-35-implementation-plan.md), [NEO-36](../../plans/NEO-36-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/`; [`docs/manual-qa/NEO-36.md`](../../manual-qa/NEO-36.md); [server README — Skill definitions (NEO-36)](../../../server/README.md#skill-definitions-neo-36) |
|
||||
| E2.M2 | Planned | **Backlog (Linear):** Slice 2 vertical slices — [NEO-37](https://linear.app/neon-sprawl/issue/NEO-37) (player skill progression read snapshot) → [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) (apply XP grant + `allowedXpSourceKinds` validation + level-up) → [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39) (data-driven `LevelCurve` + CI); [NEO-40](https://linear.app/neon-sprawl/issue/NEO-40) (telemetry hook sites, parallel with NEO-39 after NEO-38). Slice 3 integration — [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41) / [NEO-42](https://linear.app/neon-sprawl/issue/NEO-42) / [NEO-43](https://linear.app/neon-sprawl/issue/NEO-43) (multi-label with **E3**/**E7**); [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44) (gig XP from combat, **E5.M1**). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37–NEO-40, NEO-41–NEO-43 |
|
||||
| E2.M2 | In Progress | **NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([NEO-37](../../plans/NEO-37-implementation-plan.md)) — `SkillProgressionSnapshotApi` / `SkillProgressionSnapshotDtos` in `server/NeonSprawl.Server/Game/Skills/`; bootstrap read model (**xp** 0, **level** 1) for every registered skill; known-player gate via `IPositionStateStore` (same pattern as cooldown snapshot); Bruno `bruno/neon-sprawl-server/skill-progression/`; manual QA [`NEO-37`](../../manual-qa/NEO-37.md); [server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37). **Follow-on:** [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) (apply XP grant + `allowedXpSourceKinds` validation + level-up) → [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39) (data-driven `LevelCurve` + CI); [NEO-40](https://linear.app/neon-sprawl/issue/NEO-40) (telemetry hook sites). Slice 3 integration — [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41) / [NEO-42](https://linear.app/neon-sprawl/issue/NEO-42) / [NEO-43](https://linear.app/neon-sprawl/issue/NEO-43); [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [NEO-37](../../plans/NEO-37-implementation-plan.md), [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37–NEO-40, NEO-41–NEO-43 |
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen
|
|||
| Module ID | Module Name | Depends On | Contract Needed | Phase Required | Status |
|
||||
|---|---|---|---|---|---|
|
||||
| E2.M1 | SkillDefinitionRegistry | None | SkillDef, SkillCategory, UnlockRequirement, allowedXpSourceKinds | Prototype | In Progress |
|
||||
| E2.M2 | XpAwardAndLevelEngine | E2.M1 | XpGrantEvent, LevelCurve, LevelUpEvent | Prototype | Planned |
|
||||
| E2.M2 | XpAwardAndLevelEngine | E2.M1 | XpGrantEvent, LevelCurve, LevelUpEvent | Prototype | In Progress |
|
||||
| E2.M3 | MasteryAndPerkUnlocks | E2.M2 | MasteryTrack, PerkUnlockEvent, PerkState | Pre-production | Planned |
|
||||
| E2.M4 | ProgressionPacingControls | E2.M2, E9.M2 | XpModifierProfile, CatchUpRule, PacingPolicy | Pre-production | Planned |
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,27 @@
|
|||
# NEO-37 — Manual QA checklist
|
||||
|
||||
| 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 |
|
||||
| Plan | `docs/plans/NEO-37-implementation-plan.md` |
|
||||
| Branch | `NEO-37-skill-progression-snapshot-read-model` |
|
||||
|
||||
## Preconditions
|
||||
|
||||
- Server running with default dev player seeded (same as other **`/game/players/dev-local-1/...`** flows).
|
||||
- Skill catalog loaded (prototype trio); **`skills`** is key-by-**`id`** (array order is not contractual).
|
||||
|
||||
## Checklist
|
||||
|
||||
1. Start **`NeonSprawl.Server`** (e.g. `dotnet run` from `server/NeonSprawl.Server`).
|
||||
2. **`GET /game/players/dev-local-1/skill-progression`** — expect **200** and **`Content-Type`** containing **`application/json`**.
|
||||
|
||||
```bash
|
||||
curl -sS -i "http://localhost:5253/game/players/dev-local-1/skill-progression"
|
||||
```
|
||||
|
||||
3. Parse JSON — **`schemaVersion`** **1**, **`playerId`** **`dev-local-1`**, **`skills`** length **3** with **`id`**s **`intrusion`**, **`refine`**, **`salvage`** (any order); each row **`xp`** **0**, **`level`** **1**.
|
||||
4. Unknown player — **`GET /game/players/missing-player/skill-progression`** — expect **404**.
|
||||
5. Optional: run **`bruno/neon-sprawl-server/skill-progression/Get skill progression.bru`** (see `environments/Local.bru` for **`{{baseUrl}}`**).
|
||||
|
|
@ -0,0 +1,85 @@
|
|||
# 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** envelope patterns (`schemaVersion`, `System.Text.Json` property names). **`skills`** is semantically **unordered** — clients **key by `id`** (unlike the world skill catalog projection where id order is pinned for discovery).
|
||||
|
||||
**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; consumers treat **`skills`** as unordered and key by **`id`** (wire order not contractual).
|
||||
- [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-36’s **`SkillDefinitionsListResponse.CurrentSchemaVersion`**), **`playerId`** (**`id.Trim()`**, consistent with **`HotbarLoadoutResponse`**), and **`skills`**: array of objects **`id`** (skill def id), **`xp`** (int, **0**), **`level`** (int, **1**). **Array order is not part of the public contract** — clients index by **`id`**. Do **not** duplicate catalog fields (`displayName`, etc.); clients join with **`GET /game/world/skill-definitions`** if needed.
|
||||
|
||||
4. **Data source:** For NEO-37, build rows from **`ISkillDefinitionRegistry`** (no persistence). **Sort by `id` (ordinal) before serialization** for stable diffs only — semantics remain keyed by **`id`**, not by array index. **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:** **`bruno/neon-sprawl-server/skill-progression/`** with **`Get skill progression.bru`**. Tests assert **200**, **`schemaVersion`**, **`skills`** length **3**, frozen ids present + **`xp`/`level`** defaults — **do not assert array order**.
|
||||
|
||||
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, trio by **`id`** (map), defaults (**order-independent**). |
|
||||
| `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`**. **`GET`** for **`dev-local-1`** returns **200**; body has those three **`id`**s with **`xp`** **0**, **`level`** **1**, **without asserting JSON array order**. **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.
|
||||
|
|
@ -9,6 +9,10 @@ A small number of files keep the historical **Jira / Atlassian `NEON-*`** key in
|
|||
- Each such file includes a short **Historical** banner at the top pointing at the matching **`NEO-*`** plan (same scope) where kickoff and commits should align.
|
||||
- Prefer opening and linking **`NEO-*`** issues and plans for new contributors.
|
||||
|
||||
## GitHub links in Linear (and docs)
|
||||
|
||||
The canonical public repo path is **`ViPro-Technologies/neon-sprawl`** (see `git remote`). Use that org in **raw GitHub** markdown links (`https://github.com/ViPro-Technologies/neon-sprawl/blob/main/...`). The path **`neon-sprawl/neon-sprawl`** is not the live repo and will **404**.
|
||||
|
||||
## Conventions
|
||||
|
||||
- New or moved plans: `docs/plans/{LINEAR_KEY}-implementation-plan.md` (see [story kickoff](../../.cursor/rules/story-kickoff.md)).
|
||||
|
|
|
|||
|
|
@ -0,0 +1,59 @@
|
|||
# Code review — NEO-37 skill progression snapshot
|
||||
|
||||
**Date:** 2026-05-06 (initial); **re-reviewed** 2026-05-07.
|
||||
|
||||
**Scope:** Branch `NEO-37-skill-progression-snapshot-read-model`; `git` range `f89efdce..a3cbef0` (merge-base `f89efdce` with `origin/main` through current HEAD `a3cbef0`). Issue **NEO-37**.
|
||||
|
||||
**Base:** `origin/main` @ `f89efdce`.
|
||||
|
||||
**Follow-up commits since first review:** `295eb34` (decomposition docs + review feedback); `a3cbef0` (**`skills`** non-contractual ordering, ordinal sort for stable serialization only, tests keyed by **`id`**, Bruno + README + plan + manual QA aligned).
|
||||
|
||||
## Verdict
|
||||
|
||||
**Approve.**
|
||||
|
||||
## Summary
|
||||
|
||||
Adds `GET /game/players/{id}/skill-progression` with server-authoritative defaults (every registered skill row at `xp` 0, `level` 1), gated on `IPositionStateStore.TryGetPosition` like `CooldownSnapshotApi`/`HotbarLoadoutApi`. Response DTOs follow the NEO-36-style versioned envelope (`schemaVersion`, camelCase JSON names); **`skills` array order is not contractual** — clients key by **`id`**. Integration-style tests exercise **404**, frozen trio membership + defaults (**order-independent**), and Bruno no longer asserts sort order on the progression array. README, manual QA, and implementation plan document the same contract. Risk is low: read-only projection with no persistence or grant path yet.
|
||||
|
||||
## Documentation checked
|
||||
|
||||
- `docs/plans/NEO-37-implementation-plan.md` — **matches** route, player gate (`404`), JSON shape, **`skills` unordered-by-contract** (+ optional stable ordinal sort server-side only), Bruno/README/manual QA expectations, and deferral of persistence to NEO-38.
|
||||
|
||||
- `docs/manual-qa/NEO-37.md` — **matches** curl steps and assertions.
|
||||
|
||||
- `docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md` — **matches** story placement and NEO-37 delivery in **Implementation snapshot**; full engine responsibilities in the module body remain description of the target module (NEO-38+). **Updated** after review (2026-05-07) so tracking does not drift.
|
||||
|
||||
- `docs/decomposition/modules/documentation_and_implementation_alignment.md` — **matches** (E2.M2 row **In Progress**, NEO-37 landed narrative + follow-on backlog). **Updated** after review.
|
||||
|
||||
- `docs/decomposition/modules/client_server_authority.md`, `contracts.md` — **N/A** for this additive read-model slice (no new cross-cutting gameplay authority beyond existing “known player via position store” pattern).
|
||||
|
||||
## Blocking issues
|
||||
|
||||
(none)
|
||||
|
||||
## Suggestions
|
||||
|
||||
1. ~~**Documentation follow-up after merge:** Update `docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md` (implementation snapshot — remove or qualify “No implementation landed yet”) and confirm `documentation_and_implementation_alignment.md` E2.M2 row still reads correctly once NEO-37 is merged. Prefer a short PR stacked on this merge or the same sprint so tracking does not drift.~~ **Done.**
|
||||
|
||||
2. ~~**`docs/plans/NEO-37-implementation-plan.md` typography:** Insert a missing space after the comma (“When grants land, prefer …”) under Open questions.~~ **Done.**
|
||||
|
||||
## Nits
|
||||
|
||||
- ~~**Nit:** Bruno `skills are ascending by id` matches `skill-definitions` folder style but is weaker than the C# tests for **exact catalog order**; if regressions swapped registry order without changing ids, lexical sort alone would not catch misuse of a different comparator. Acceptable Bruno scope; parity with sibling collection is coherent.~~ **Superseded** — progression **`skills`** is explicitly **unordered by contract**; Bruno + C# tests key by **`id`** only (**2026-05-07**).
|
||||
|
||||
## Re-review notes
|
||||
|
||||
Explicit **unordered-by-contract** semantics for **`skills`** are coherent: they differentiate this read snapshot from **`GET …/skill-definitions`**, whose catalog order stays pinned for discovery. Server-side **`string.CompareOrdinal` sort before JSON** preserves stable payloads without inviting clients to depend on index ordering—documented on the response DTO, README, plan, manual QA, and `BuildSnapshot`.
|
||||
|
||||
Prior review items (module snapshot, alignment row, plan typo, Bruno-vs-order nit) are **addressed or superseded**; no new blocking findings.
|
||||
|
||||
## Verification
|
||||
|
||||
- Re-review (2026-05-07):
|
||||
`cd server && dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~SkillProgressionSnapshotApiTests"` — passed (2 tests).
|
||||
|
||||
- Recommended before merge: full server test suite:
|
||||
`cd server && dotnet test`
|
||||
|
||||
- Manual: steps in `docs/manual-qa/NEO-37.md`; optional Bruno `bruno/neon-sprawl-server/skill-progression/Get skill progression.bru` with `{{baseUrl}}` from `environments/Local.bru`.
|
||||
|
|
@ -0,0 +1,52 @@
|
|||
using System.Linq;
|
||||
using System.Net;
|
||||
using System.Net.Http.Json;
|
||||
using NeonSprawl.Server.Game.Skills;
|
||||
using Xunit;
|
||||
|
||||
namespace NeonSprawl.Server.Tests.Game.Skills;
|
||||
|
||||
public sealed class SkillProgressionSnapshotApiTests
|
||||
{
|
||||
[Fact]
|
||||
public async Task GetSkillProgression_ShouldReturnNotFound_WhenPlayerUnknown()
|
||||
{
|
||||
// Arrange
|
||||
await using var factory = new InMemoryWebApplicationFactory();
|
||||
var client = factory.CreateClient();
|
||||
|
||||
// Act
|
||||
var response = await client.GetAsync("/game/players/missing-player/skill-progression");
|
||||
|
||||
// Assert
|
||||
Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetSkillProgression_ShouldReturnSchemaV1_WithDefaultXpAndLevel_ForFrozenTrio()
|
||||
{
|
||||
// Arrange
|
||||
await using var factory = new InMemoryWebApplicationFactory();
|
||||
var client = factory.CreateClient();
|
||||
|
||||
// Act
|
||||
var response = await client.GetAsync("/game/players/dev-local-1/skill-progression");
|
||||
|
||||
// Assert
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
var body = await response.Content.ReadFromJsonAsync<SkillProgressionSnapshotResponse>();
|
||||
Assert.NotNull(body);
|
||||
Assert.Equal(SkillProgressionSnapshotResponse.CurrentSchemaVersion, body!.SchemaVersion);
|
||||
Assert.Equal("dev-local-1", body.PlayerId);
|
||||
Assert.NotNull(body.Skills);
|
||||
Assert.Equal(3, body.Skills!.Count);
|
||||
var byId = body.Skills.ToDictionary(static s => s.Id, StringComparer.Ordinal);
|
||||
Assert.Equal(3, byId.Count);
|
||||
foreach (var id in new[] { "intrusion", "refine", "salvage" })
|
||||
{
|
||||
Assert.True(byId.TryGetValue(id, out var row));
|
||||
Assert.Equal(0, row!.Xp);
|
||||
Assert.Equal(1, row.Level);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,53 @@
|
|||
using NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
namespace NeonSprawl.Server.Game.Skills;
|
||||
|
||||
/// <summary>Maps <c>GET /game/players/{{id}}/skill-progression</c> (NEO-37).</summary>
|
||||
public static class SkillProgressionSnapshotApi
|
||||
{
|
||||
public static WebApplication MapSkillProgressionSnapshotApi(this WebApplication app)
|
||||
{
|
||||
app.MapGet(
|
||||
"/game/players/{id}/skill-progression",
|
||||
(string id, IPositionStateStore positions, ISkillDefinitionRegistry registry) =>
|
||||
{
|
||||
if (!positions.TryGetPosition(id, out _))
|
||||
{
|
||||
return Results.NotFound();
|
||||
}
|
||||
|
||||
var trimmedId = id.Trim();
|
||||
return Results.Json(BuildSnapshot(trimmedId, registry));
|
||||
});
|
||||
|
||||
return app;
|
||||
}
|
||||
|
||||
internal static SkillProgressionSnapshotResponse BuildSnapshot(
|
||||
string playerId,
|
||||
ISkillDefinitionRegistry registry)
|
||||
{
|
||||
var defs = registry.GetDefinitionsInIdOrder();
|
||||
var skills = new List<SkillProgressionRowJson>(defs.Count);
|
||||
foreach (var d in defs)
|
||||
{
|
||||
skills.Add(
|
||||
new SkillProgressionRowJson
|
||||
{
|
||||
Id = d.Id,
|
||||
Xp = 0,
|
||||
Level = 1,
|
||||
});
|
||||
}
|
||||
|
||||
// Ordinal sort for stable serialization; consumers must index rows by skill id (NEO-37).
|
||||
skills.Sort(static (a, b) => string.CompareOrdinal(a.Id, b.Id));
|
||||
|
||||
return new SkillProgressionSnapshotResponse
|
||||
{
|
||||
PlayerId = playerId,
|
||||
Skills = skills,
|
||||
SchemaVersion = SkillProgressionSnapshotResponse.CurrentSchemaVersion,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,32 @@
|
|||
using System.Text.Json.Serialization;
|
||||
|
||||
namespace NeonSprawl.Server.Game.Skills;
|
||||
|
||||
/// <summary>JSON body for <c>GET /game/players/{{id}}/skill-progression</c> (NEO-37).</summary>
|
||||
public sealed class SkillProgressionSnapshotResponse
|
||||
{
|
||||
public const int CurrentSchemaVersion = 1;
|
||||
|
||||
[JsonPropertyName("schemaVersion")]
|
||||
public int SchemaVersion { get; init; } = CurrentSchemaVersion;
|
||||
|
||||
[JsonPropertyName("playerId")]
|
||||
public required string PlayerId { get; init; }
|
||||
|
||||
/// <summary>One row per registered skill id. Consumers must treat this as unordered and key rows by <see cref="SkillProgressionRowJson.Id"/> — array order is not part of the public contract.</summary>
|
||||
[JsonPropertyName("skills")]
|
||||
public required IReadOnlyList<SkillProgressionRowJson> Skills { get; init; }
|
||||
}
|
||||
|
||||
/// <summary>XP and level for one skill before grants (defaults) or after NEO-38 persistence.</summary>
|
||||
public sealed class SkillProgressionRowJson
|
||||
{
|
||||
[JsonPropertyName("id")]
|
||||
public required string Id { get; init; }
|
||||
|
||||
[JsonPropertyName("xp")]
|
||||
public int Xp { get; init; }
|
||||
|
||||
[JsonPropertyName("level")]
|
||||
public int Level { get; init; }
|
||||
}
|
||||
|
|
@ -28,6 +28,7 @@ app.MapPositionStateApi();
|
|||
app.MapInteractionApi();
|
||||
app.MapInteractablesWorldApi();
|
||||
app.MapSkillDefinitionsWorldApi();
|
||||
app.MapSkillProgressionSnapshotApi();
|
||||
app.MapTargetingApi();
|
||||
app.MapHotbarLoadoutApi();
|
||||
app.MapCooldownSnapshotApi();
|
||||
|
|
|
|||
|
|
@ -45,6 +45,14 @@ On success, **Information** logs include the resolved skills directory path and
|
|||
curl -sS -i "http://localhost:5253/game/world/skill-definitions"
|
||||
```
|
||||
|
||||
## Skill progression snapshot (NEO-37)
|
||||
|
||||
**`GET /game/players/{id}/skill-progression`** returns a versioned JSON body (`schemaVersion` **1**, **`playerId`**, **`skills`**) with one row per registered skill (**`id`**, **`xp`**, **`level`**). Treat **`skills`** as **unordered** and index rows by **`id`** — array order is not part of the contract (the server may sort for stable serialization). Before XP grants (**NEO-38**) each row boots with **`xp`** **0** and **`level`** **1**. Unknown players (**no row in position state**) receive **404**, same gate as **`GET …/cooldown-snapshot`**. Plan: [NEO-37 implementation plan](../../docs/plans/NEO-37-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-37.md`](../../docs/manual-qa/NEO-37.md); Bruno: `bruno/neon-sprawl-server/skill-progression/`.
|
||||
|
||||
```bash
|
||||
curl -sS -i "http://localhost:5253/game/players/dev-local-1/skill-progression"
|
||||
```
|
||||
|
||||
## Position persistence (NEO-8)
|
||||
|
||||
When **`ConnectionStrings:NeonSprawl`** is set to a valid PostgreSQL connection string, the server uses the **`player_position`** table (relational columns `pos_x` … `sequence`, not JSONB). DDL lives in [`server/db/migrations/V001__player_position.sql`](../db/migrations/V001__player_position.sql); the app applies that script on startup and on first store use (idempotent `CREATE TABLE IF NOT EXISTS`). The configured dev player (`Game:DevPlayerId` / `Game:DefaultPosition`) is seeded once at host startup, matching the in-memory store’s constructor seed (not on every HTTP request).
|
||||
|
|
|
|||
Loading…
Reference in New Issue