10 KiB
NEO-38 — Implementation plan
Story reference
| Field | Value |
|---|---|
| Key | NEO-38 |
| Title | Apply skill XP grant + allowlist + level-up |
| Linear | https://linear.app/neon-sprawl/issue/NEO-38/apply-skill-xp-grant-allowlist-level-up |
| Module | E2.M2 — XpAwardAndLevelEngine |
| Blocked by | NEO-37 (read snapshot) — implemented on main; this branch builds on that contract. |
Kickoff clarifications
| Topic | Question / note | Resolution |
|---|---|---|
| Persistence | NEO-38 AC: follow NEO-29 pattern vs in-memory-only + follow-up | User: mirror Hotbar / NEO-29 — abstract store, InMemory* fallback, Postgres* when ConnectionStrings__NeonSprawl is set, with schema bootstrap analogous to PostgresHotbarLoadoutBootstrap. |
| Successful POST body | Surface level-up in response vs minimal + GET only | User asked for recommendation → Agent: HTTP 200 with versioned JSON that includes full progression snapshot (aligned with GET …/skill-progression) plus an explicit levelUps (or equivalent) array for thresholds crossed on this grant. User: confirmed snapshot + level-ups. |
Goal, scope, and out-of-scope
Goal: Server-authoritative apply XP grant path: validate skillId (ISkillDefinitionRegistry.TryGetDefinition) and sourceKind against SkillDefRow.AllowedXpSourceKinds; persist per-player XP; recompute level from an inline placeholder curve (acceptable until NEO-39); return structured denies (stable reason codes, hotbar-style 200 + granted: false where that pattern already fits the repo) and level-up data on success.
In scope (from Linear):
- Reject unknown skill id and disallowed
sourceKindwith stable reason codes. - Valid grant updates persistence; level increases when thresholds crossed (
LevelUpEventsemantics surfaced in JSON for this story). - Versioned POST + automated tests; Bruno requests;
server/README.md. - Persistence: in-memory + Postgres when configured, per kickoff decision.
Out of scope (from Linear):
- Data-driven level curve file + CI (NEO-39).
- Telemetry catalog (NEO-40).
- Slice 3 activity wiring (NEO-41–43).
Acceptance criteria checklist
- Unknown
skillIdrejected;sourceKindnot in allowlist rejected (stable reason codes). - Valid grant updates stored progression; level increases when thresholds crossed (level-up payload on success).
- Versioned POST + tests; Bruno; README.
- Persistence: NEO-29-style dual backend (in-memory + Postgres when configured).
Technical approach
-
Route:
POST /game/players/{id}/skill-progression(same resource as NEO-37 GET, mirroringGET+POST…/hotbar-loadout). Reject missing/invalidschemaVersionwith 400 (same idea as hotbar update). -
Player gate:
404when!IPositionStateStore.TryGetPosition, unchanged from NEO-37. -
Request body (versioned): e.g.
schemaVersion,skillId,amount(positive int),sourceKind(string matching catalog values:activity,mission_reward,trainer,book_or_item). Single grant per request for a small, testable surface; batch grants can be a later issue if needed. -
Validation: Use
ISkillDefinitionRegistry. If unknown skill → deny withunknown_skill(exact string TBD but stable and documented). If known skill butsourceKindnot inAllowedXpSourceKinds(case/whitespace normalization consistent with catalog) →source_kind_not_allowed. Non-positiveamount→invalid_amount(or reuse a genericbad_grantonly if we want fewer codes — prefer specific codes per AC). -
Persistence: Introduce
IPlayerSkillProgressionStore(name may match existing naming conventions) with operations sufficient to read XP per skill and apply a delta atomically per request, returning new XP and whether level changed. In-memory implementation keyed by normalized player id; Postgres implementation withEnsureSchema,player_id+skill_iduniqueness, XP column (level derived from XP + curve helper to avoid drift). No row for a skill ⇒ treat as XP 0 / level 1 at read time (GET and POST deny paths that return current snapshot). -
GET snapshot (NEO-37 evolution): Replace the hard-coded 0 / 1 builder in
SkillProgressionSnapshotApi.BuildSnapshotwith registry + store merge so persisted XP appears onGET /game/players/{id}/skill-progression. PreserveschemaVersion1 if the JSON shape is unchanged (only values differ). Keep ordinal sort for stable serialization; unordered-by-contract rule unchanged. -
Level curve (placeholder): Inline C# thresholds or formula (documented in code comments, subsumed by NEO-39). Example pattern: simple per-level XP thresholds up to a small cap so tests can cross at least one level-up with realistic amounts.
-
Success response: HTTP 200, versioned envelope: e.g.
granted: true,progression(same shape asSkillProgressionSnapshotResponseor a shared type),levelUps: array of{ skillId, previousLevel, newLevel }(field names camelCase JSON). EmptylevelUpswhen no threshold crossed. -
Deny response: HTTP 200 with
granted: false,reasonCode, and currentprogressionsnapshot (hotbarUpdated: falsepattern) so clients always receive authoritative state for UI sync — unless a case clearly fits 400 (malformed body / wrong schema version). -
Wire-up:
AddSkillProgressionStore(or similar) inProgram.csnext toAddHotbarLoadoutStore.InMemoryWebApplicationFactoryforces the in-memory implementation for isolated tests (same replacement pattern as hotbar/position). -
Bruno / docs: Add
Post … skill progression grant.bru(exact name follows collection style) underbruno/neon-sprawl-server/skill-progression/; extendserver/README.md. Adddocs/manual-qa/NEO-38.mdduring implementation per project convention.
Files to add
| Path | Purpose |
|---|---|
server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs |
Abstraction: read XP map / apply grant for one player+skill. |
server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs |
In-memory persistence; normalized player keys. |
server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs |
Postgres implementation when connection string present. |
server/NeonSprawl.Server/Game/Skills/PostgresSkillProgressionBootstrap.cs |
Schema ensure (mirror hotbar bootstrap style). |
server/NeonSprawl.Server/Game/Skills/SkillProgressionServiceCollectionExtensions.cs |
Chooses Postgres vs in-memory from configuration. |
server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs |
Versioned POST request + success/deny response DTOs (levelUps, reasonCode, etc.). |
server/NeonSprawl.Server/Game/Skills/SkillLevelCurvePlaceholder.cs |
Inline threshold/floor helpers until NEO-39 (pure functions, unit-testable). |
server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs |
In-memory host: unknown skill, bad source kind, success, level-up, GET reflects POST, 404 player gate, bad schema → 400. |
server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs |
Postgres harness: grant survives new host/process (mirror HotbarLoadoutPersistenceIntegrationTests). |
bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru |
Manual smoke for happy + one deny path. |
docs/manual-qa/NEO-38.md |
Curl/Bruno checklist for grant + GET verification. |
Files to modify
| Path | Rationale |
|---|---|
server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs |
Register POST; inject store; change GET builder to merge registry + stored XP via placeholder level curve. |
server/NeonSprawl.Server/Program.cs |
Register AddSkillProgressionStore and ensure catalog/store order remains valid. |
server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs |
Swap/replace IPlayerSkillProgressionStore with in-memory implementation for tests. |
server/README.md |
Document POST grant, reason codes, persistence note, Bruno path, links to plan + manual QA. |
Tests
| Test file | What it covers |
|---|---|
SkillProgressionGrantApiTests.cs |
AAA integration tests: denies (unknown_skill, source_kind_not_allowed); success increases XP; levelUps when threshold crossed; GET matches after grant; 404 without position; malformed / wrong schemaVersion → 400. |
SkillProgressionGrantPersistenceIntegrationTests.cs |
Postgres: write grant, new factory/client, GET (or POST read-back) shows persisted XP. |
SkillProgressionSnapshotApiTests.cs (existing) |
Update expectations if GET implementation changes from hard-coded zeros to store-backed merge (same defaults when store empty). |
Bruno + docs/manual-qa/NEO-38.md supplement automated coverage.
Open questions / risks
- Reason code naming: Final strings should be listed once in README and reused in tests/Bruno to avoid drift (prefer
snake_caseconsistent withHotbarLoadoutApiconstants). - Placeholder curve vs. content: Keep thresholds boring and documented; NEO-39 replaces the helper without changing persistence schema if XP remains the stored value.
- Concurrency: Postgres store should use a transaction per grant; in-memory uses concurrent structures or per-player locks if needed for tests under parallel load (prototype: acceptable to document single-threaded assumption if minimal).