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

10 KiB
Raw Permalink Blame History

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 sourceKind with stable reason codes.
  • Valid grant updates persistence; level increases when thresholds crossed (LevelUpEvent semantics 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-4143).

Acceptance criteria checklist

  • Unknown skillId rejected; sourceKind not 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

  1. Route: POST /game/players/{id}/skill-progression (same resource as NEO-37 GET, mirroring GET + POST …/hotbar-loadout). Reject missing/invalid schemaVersion with 400 (same idea as hotbar update).

  2. Player gate: 404 when !IPositionStateStore.TryGetPosition, unchanged from NEO-37.

  3. 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.

  4. Validation: Use ISkillDefinitionRegistry. If unknown skill → deny with unknown_skill (exact string TBD but stable and documented). If known skill but sourceKind not in AllowedXpSourceKinds (case/whitespace normalization consistent with catalog) → source_kind_not_allowed. Non-positive amountinvalid_amount (or reuse a generic bad_grant only if we want fewer codes — prefer specific codes per AC).

  5. 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 with EnsureSchema, player_id + skill_id uniqueness, 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).

  6. GET snapshot (NEO-37 evolution): Replace the hard-coded 0 / 1 builder in SkillProgressionSnapshotApi.BuildSnapshot with registry + store merge so persisted XP appears on GET /game/players/{id}/skill-progression. Preserve schemaVersion 1 if the JSON shape is unchanged (only values differ). Keep ordinal sort for stable serialization; unordered-by-contract rule unchanged.

  7. 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.

  8. Success response: HTTP 200, versioned envelope: e.g. granted: true, progression (same shape as SkillProgressionSnapshotResponse or a shared type), levelUps: array of { skillId, previousLevel, newLevel } (field names camelCase JSON). Empty levelUps when no threshold crossed.

  9. Deny response: HTTP 200 with granted: false, reasonCode, and current progression snapshot (hotbar Updated: false pattern) so clients always receive authoritative state for UI sync — unless a case clearly fits 400 (malformed body / wrong schema version).

  10. Wire-up: AddSkillProgressionStore (or similar) in Program.cs next to AddHotbarLoadoutStore. InMemoryWebApplicationFactory forces the in-memory implementation for isolated tests (same replacement pattern as hotbar/position).

  11. Bruno / docs: Add Post … skill progression grant.bru (exact name follows collection style) under bruno/neon-sprawl-server/skill-progression/; extend server/README.md. Add docs/manual-qa/NEO-38.md during 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 schemaVersion400.
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_case consistent with HotbarLoadoutApi constants).
  • 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).