diff --git a/docs/plans/NEO-47-implementation-plan.md b/docs/plans/NEO-47-implementation-plan.md new file mode 100644 index 0000000..8c991e7 --- /dev/null +++ b/docs/plans/NEO-47-implementation-plan.md @@ -0,0 +1,159 @@ +# NEO-47 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-47 | +| **Title** | E2.M3: Perk unlock engine + PerkState persistence | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-47/e2m3-perk-unlock-engine-perkstate-persistence | +| **Module** | [E2.M3 — MasteryAndPerkUnlocks](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) · Epic 2 Slice 4 | +| **Branch** | `NEO-47-perk-unlock-engine-perkstate-persistence` | +| **Blocked by** | [NEO-46](https://linear.app/neon-sprawl/issue/NEO-46) (catalog load — **Done** on `main`), [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) (skill level source — **Done** on `main`) | + +## Kickoff clarifications + +| Topic | Question | Answer | +|--------|----------|--------| +| **Tier 2 branch behavior** | At level 4, does the player pick a branch again or do tier-2 perks unlock automatically from the tier-1 choice? | User did not understand the question at kickoff → **Agent recommendation (adopted for this story):** **auto-path** — tier 1 is the only explicit branch pick; when skill level reaches tier 2’s `requiredLevel` (4 for salvage), perks on the **same `branchId`** as the tier-1 pick unlock with **no second pick**. Tier 2 branch rows stay in the catalog for metadata/perks; `TrySelectBranch` denies with `tier_branch_not_selectable` if all branches in that tier carry perks (see rule below). | +| **Postgres schema** | Normalized tables vs JSON blob vs defer Postgres? | User asked for recommendation → **Agent recommendation (adopted):** **normalized** tables + Flyway **`V004`**, mirroring NEO-38 (`player_mastery_branch_pick`, `player_unlocked_perk`). Linear AC requires in-memory + Postgres in this story. | +| **PerkUnlockEvent surfacing** | Return list vs DI sink vs both? | User asked for recommendation → **Agent recommendation (adopted):** **return `IReadOnlyList`** from engine methods; callers (grant hook, future HTTP in NEO-48) may ignore for now. NEO-49 can add an optional `IPerkUnlockEventSink` without changing persistence. | +| **Level-up hook site** | Inside `SkillProgressionGrantOperations` vs separate service? | **User:** run **inside `SkillProgressionGrantOperations`** when `levelUps` is non-empty (same path as XP write). Implementation: call a small **`PerkUnlockEngine`** instance method from there (logic stays testable in `Game/Mastery/`, not duplicated in HTTP). | + +### Tier 1 vs tier 2 (plain language) + +Prototype **salvage** mastery ([`prototype_salvage_mastery.json`](../../content/mastery/prototype_salvage_mastery.json)): + +| Tier | Skill level gate | What happens | +|------|------------------|--------------| +| **1** | 2 | Player **chooses** `scrap_efficiency` **or** `bulk_haul`. **No perks** yet — only records the branch. | +| **2** | 4 | **One perk** unlocks on the path they already chose (e.g. `salvage_scrap_efficiency_1` if they picked `scrap_efficiency` at tier 1). **No second choice** between the two branches. | + +So “mutually exclusive per tier” applies to **tier 1’s pick**; tier 2 is “follow your path,” not a new fork. + +**Catalog rule (engine):** A tier **requires an explicit branch pick** when **every** branch in that tier has an empty `perkIds` array. A tier **auto-unlocks from path** when branches have perks and the player already has a `branchId` pick on the highest prior tier that required a pick (for prototype: tier 1 → tier 2). + +## Goal, scope, and out-of-scope + +**Goal:** Server-authoritative **`PerkState`** (unlocked `perkId` set + `branchId` per `tierIndex` per track) with unlock evaluation driven by **skill level** from E2.M2. When level crosses a tier gate, eligible tiers become selectable or auto-unlock per rules above. **Re-evaluate** on skill level-up after XP grants. Emit internal **`PerkUnlockEvent`** when perks unlock. + +**In scope (from Linear):** + +- `IPlayerPerkStateStore` — in-memory + Postgres + migration (NEO-38 pattern). +- Unlock engine: catalog + skill level; `PerkUnlockEvent` on unlock. +- Mutually exclusive branch pick where applicable; stable **`reasonCode`** on invalid picks. +- Level-up path after `SkillProgressionGrantOperations` when `levelUps` non-empty. +- Unit/integration tests (AAA): eligibility, branch select, deny paths, level-up auto-unlock. + +**Out of scope (from Linear):** + +- HTTP read/write (**NEO-48**). +- Gameplay `effectKind` application; respec; gig perks. + +## Acceptance criteria checklist + +- [ ] `IPlayerPerkStateStore` with in-memory + Postgres path (migration) mirroring skill progression pattern. +- [ ] Unlock engine reads catalog + skill level; emits internal `PerkUnlockEvent` when perks unlock. +- [ ] Branch pick per tier is **mutually exclusive** where a tier requires a pick; stable deny `reasonCode` on invalid picks. +- [ ] Level-up path (after `SkillProgressionGrantOperations`) triggers re-evaluation for affected skill. +- [ ] Unit/integration tests (AAA): eligibility, branch select, deny paths. + +## Technical approach + +1. **Domain model (`Game/Mastery/`):** + - **`PerkStateSnapshot`** — `branchPicks`: `skillId` → `tierIndex` → `branchId`; `unlockedPerkIds`: set of `perkId`. + - **`PerkUnlockEvent`** — `playerId`, `perkId`, `skillId`, `tierIndex`, `branchId?`, `source` (`branch_pick` | `level_up`). + - **`PerkUnlockEngine`** — pure orchestration: `TrySelectBranch(...)`, `ReevaluateAfterLevelUp(...)`, both return **`PerkBranchSelectOutcome`** / **`PerkReevaluationOutcome`** with `success`, `reasonCode?`, `events`, updated snapshot. + +2. **Eligibility (skill level):** Resolve level via `IPlayerSkillProgressionStore.GetXpTotals` + `ISkillLevelCurve.LevelFromTotalXp` (same as GET skill progression). Tier *T* is **reachable** when `level >= tier.RequiredLevel`. + +3. **`TrySelectBranch(playerId, skillId, tierIndex, branchId)`:** + - Deny: `unknown_track`, `unknown_tier`, `unknown_branch`, `level_too_low`, `branch_already_chosen`, `tier_branch_not_selectable` (tier auto-unlocks from path — all branches have perks). + - Accept: persist pick; unlock any `perkIds` on the chosen branch for that tier (tier 1: none); emit events. + - Player write gate: store returns false when player missing from position/progression pattern → `player_not_found` (align with skill XP store). + +4. **`ReevaluateAfterLevelUp(playerId, skillId, newLevel)`:** + - For each tier on track where `requiredLevel <= newLevel` and tier is **path-auto** (has perks, prior pick exists): unlock perks for branch matching prior pick’s `branchId`; skip already-unlocked ids; persist + events. + - Idempotent: re-running does not duplicate perks or events in state. + +5. **Persistence (`IPlayerPerkStateStore`):** + - `GetSnapshot(string playerId)` + - `bool TrySetBranchPick(string playerId, string skillId, int tierIndex, string branchId)` — fails if pick exists or player missing. + - `bool TryAddUnlockedPerks(string playerId, IEnumerable perkIds)` — additive, idempotent per perk. + - **Postgres:** `V004__player_perk_state.sql` — `player_mastery_branch_pick (player_id, skill_id, tier_index, branch_id)` PK `(player_id, skill_id, tier_index)`; `player_unlocked_perk (player_id, perk_id)` PK `(player_id, perk_id)`; FK to `player_position`. Bootstrap: `PostgresPerkStateBootstrap` (mirror `PostgresSkillProgressionBootstrap`). + +6. **DI:** `AddPerkStateStore(IServiceCollection, IConfiguration)` — Postgres when `ConnectionStrings:NeonSprawl` set, else in-memory. Register **`PerkUnlockEngine`** as singleton (depends on store, `IMasteryCatalogRegistry`, `IPlayerSkillProgressionStore`, `ISkillLevelCurve`). + +7. **Level-up hook:** In **`SkillProgressionGrantOperations.TryApplyGrant`**, after successful XP apply, foreach entry in `levelUps` call `perkUnlockEngine.ReevaluateAfterLevelUp(playerId, levelUp.SkillId, levelUp.NewLevel)`. Inject `PerkUnlockEngine` via new overload or optional parameter on `TryApplyGrant` (prefer overload used by HTTP + interact to avoid static service locator). Events returned but not yet exposed on HTTP response (NEO-48). + +8. **Reason codes (stable, `snake_case`):** Document in code constants on `PerkUnlockReasonCodes` (or engine type); mirror in tests. + + | Code | When | + |------|------| + | `unknown_track` | No mastery track for `skillId` | + | `unknown_tier` | `tierIndex` not on track | + | `unknown_branch` | `branchId` not in tier | + | `level_too_low` | Skill level below `requiredLevel` | + | `branch_already_chosen` | Pick already stored for `(skillId, tierIndex)` | + | `tier_branch_not_selectable` | Tier auto-unlocks from path (no manual pick) | + | `player_not_found` | Store cannot write player | + +9. **Tests:** Use repo `content/mastery` + level curve (`level 2` @ 100 XP, `level 4` @ 450 XP). Manipulate XP via `IPlayerSkillProgressionStore` or `TryApplyGrant` in integration tests. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/db/migrations/V004__player_perk_state.sql` | Flyway DDL for branch picks + unlocked perks. | +| `server/NeonSprawl.Server/Game/Mastery/PerkStateSnapshot.cs` | In-memory read model for one player. | +| `server/NeonSprawl.Server/Game/Mastery/PerkUnlockEvent.cs` | Internal unlock event DTO. | +| `server/NeonSprawl.Server/Game/Mastery/PerkUnlockReasonCodes.cs` | Stable deny reason constants. | +| `server/NeonSprawl.Server/Game/Mastery/PerkBranchSelectOutcome.cs` | Result of `TrySelectBranch`. | +| `server/NeonSprawl.Server/Game/Mastery/PerkReevaluationOutcome.cs` | Result of `ReevaluateAfterLevelUp`. | +| `server/NeonSprawl.Server/Game/Mastery/IPlayerPerkStateStore.cs` | Persistence abstraction. | +| `server/NeonSprawl.Server/Game/Mastery/InMemoryPlayerPerkStateStore.cs` | In-memory store (dev player seed pattern from skill store). | +| `server/NeonSprawl.Server/Game/Mastery/PostgresPlayerPerkStateStore.cs` | Postgres implementation. | +| `server/NeonSprawl.Server/Game/Mastery/PostgresPerkStateBootstrap.cs` | One-time DDL apply for V004. | +| `server/NeonSprawl.Server/Game/Mastery/PerkStateServiceCollectionExtensions.cs` | `AddPerkStateStore` + register `PerkUnlockEngine`. | +| `server/NeonSprawl.Server/Game/Mastery/PerkUnlockEngine.cs` | Catalog + level + store orchestration. | +| `server/NeonSprawl.Server.Tests/Game/Mastery/PerkUnlockEngineTests.cs` | Unit tests: tier-1 pick, denies, level-4 auto-unlock, idempotent reevaluate. | +| `server/NeonSprawl.Server.Tests/Game/Mastery/PerkStatePersistenceIntegrationTests.cs` | Postgres: branch pick + unlock survive new factory. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantOperations.cs` | Call `PerkUnlockEngine.ReevaluateAfterLevelUp` for each `levelUps` entry on successful grant (user kickoff). | +| `server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs` | Pass `PerkUnlockEngine` into `TryApplyGrant` from POST handler. | +| `server/NeonSprawl.Server/Game/Interaction/InteractionApi.cs` | Pass engine into gather XP grant path (NEO-41). | +| `server/NeonSprawl.Server/Program.cs` | `AddPerkStateStore(builder.Configuration)`. | +| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Replace `IPlayerPerkStateStore` with in-memory implementation for isolated tests. | +| `server/NeonSprawl.Server/NeonSprawl.Server.csproj` | Copy `V004` SQL to output (if not already covered by existing migration copy pattern for V003). | + +## Tests + +| Test file | Coverage | +|-----------|----------| +| `PerkUnlockEngineTests.cs` | **AAA:** At salvage level 2, `TrySelectBranch` tier 1 `scrap_efficiency` succeeds; other branch’s tier-2 perk not unlocked. **Deny:** `level_too_low` at level 1; `branch_already_chosen` on second pick; `unknown_branch`; `tier_branch_not_selectable` on tier 2 pick. **Level-up:** set XP to 450+, call `ReevaluateAfterLevelUp` → `salvage_scrap_efficiency_1` unlocked; `bulk_haul` perk not unlocked. **Idempotent:** second reevaluate adds no duplicate perks. | +| `PerkStatePersistenceIntegrationTests.cs` | **RequirePostgres:** pick branch + unlock via engine, new `PostgresWebApplicationFactory`, `GetSnapshot` shows pick + perk. | +| `SkillProgressionGrantOperations` (via existing grant API tests or one new test) | Grant salvage XP crossing level 4 with tier-1 pick already stored → perk unlocked without HTTP perk endpoint (hook integration smoke). | + +All new `[Fact]` methods use `// Arrange` / `// Act` / `// Assert` per [csharp-style](../../.cursor/rules/csharp-style.md). + +## Open questions / risks + +- **Tier rule generalization:** “Empty `perkIds` on all branches ⇒ pick tier” must stay aligned with future multi-tier catalogs; add loader comment cross-ref to engine. +- **Grant hook + Postgres transactions:** Skill XP write and perk unlock are separate stores today (same as hotbar vs position); acceptable for prototype; document if cross-store atomicity becomes a requirement. +- **NEO-48 DTO shape:** Internal `PerkStateSnapshot` should map cleanly to versioned HTTP JSON without renaming fields. + +None blocking after kickoff clarifications. + +## Decisions + +| Decision | Rationale | +|----------|-----------| +| Tier 2 = auto-path from tier-1 `branchId` | E2M3 backlog “chosen branch path”; user needed plain-language explanation — adopted agent recommendation. | +| Normalized Postgres tables (V004) | User asked for recommendation; matches NEO-38 and query-friendly perk checks for future gates. | +| Return event list from engine | User asked for recommendation; minimal surface for NEO-47; NEO-49 can add sink. | +| Hook inside `SkillProgressionGrantOperations` | Explicit user choice; delegate to `PerkUnlockEngine` for testability. | +| Code under `Game/Mastery/` | NEO-46 boundary; cohesive with catalog registry. |