NEO-47: add implementation plan for perk unlock engine

Kickoff clarifications, tier-2 auto-path recommendation, and file/test lists for PerkState persistence and level-up hooks.
2026-05-17 18:34:50 -04:00 · 2026-05-17 18:34:50 -04:00 · e297b3a023
parent 619146da85
commit e297b3a023
1 changed files with 159 additions and 0 deletions
--- a/docs/plans/NEO-47-implementation-plan.md
+++ 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<PerkUnlockEvent>`** 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<string> 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. |