160 lines
13 KiB
Markdown
160 lines
13 KiB
Markdown
# 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
|
||
|
||
- [x] `IPlayerPerkStateStore` with in-memory + Postgres path (migration) mirroring skill progression pattern.
|
||
- [x] Unlock engine reads catalog + skill level; emits internal `PerkUnlockEvent` when perks unlock.
|
||
- [x] Branch pick per tier is **mutually exclusive** where a tier requires a pick; stable deny `reasonCode` on invalid picks.
|
||
- [x] Level-up path (after `SkillProgressionGrantOperations`) triggers re-evaluation for affected skill.
|
||
- [x] 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. |
|