diff --git a/docs/plans/NEO-104-implementation-plan.md b/docs/plans/NEO-104-implementation-plan.md new file mode 100644 index 0000000..5fb6ed2 --- /dev/null +++ b/docs/plans/NEO-104-implementation-plan.md @@ -0,0 +1,146 @@ +# NEO-104 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-104 | +| **Title** | E5M3-05: Encounter progress + completion stores | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-104/e5m3-05-encounter-progress-completion-stores | +| **Module** | [E5.M3 — EncounterAndRewardTables](../decomposition/modules/E5_M3_EncounterAndRewardTables.md) · Epic 5 Slice 3 · backlog **E5M3-05** | +| **Branch** | `NEO-104-encounter-progress-completion-stores` | +| **Precursor** | [NEO-103](https://linear.app/neon-sprawl/issue/NEO-103) — `GET /game/world/encounter-definitions` (**landed on `main`**) | +| **Pattern** | [NEO-61](NEO-61-implementation-plan.md) / `InMemoryResourceNodeInstanceStore` — keyed in-memory store + static operations; [NEO-44](NEO-44-implementation-plan.md) — gig progression store DI | +| **Blocks** | [NEO-105](https://linear.app/neon-sprawl/issue/NEO-105) — `EncounterCompletionOperations` + inventory reward apply | +| **Client counterpart** | None this story — player-visible encounter HUD starts at [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110) via [NEO-108](https://linear.app/neon-sprawl/issue/NEO-108) `GET /game/players/{id}/encounter-progress` | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Storage backend** | In-memory only vs Postgres when configured? | **In-memory only** — E5M3 kickoff default; session prototype. | **Adopted** — in-memory only | +| **Progress vs completion** | Single store vs split? | **Split stores** — progress: `started` + `defeatedNpcInstanceIds`; completion: idempotent `completedAt`; operations expose `IsAllRequiredTargetsDefeated`. | **Adopted** — split stores | +| **Encounter binding** | Explicit `encounterId` vs auto-resolve from NPC? | **Auto-resolve** — scan `IEncounterDefinitionRegistry` for encounter whose `requiredNpcInstanceIds` contains the NPC. | **Adopted** — auto-resolve from `npcInstanceId` | +| **Combat / HTTP wiring** | Wire `AbilityCastApi` or HTTP on this story? | **Defer** — E5M3-07 / E5M3-08; NEO-104 ships stores + operations + unit tests only. | **Adopted** (implicit — not asked; backlog out-of-scope) | +| **Activation before defeat** | Auto-activate on `TryMarkTargetDefeated` vs require prior `TryActivateOnFirstEngagement`? | **Require activation** — `TryMarkTargetDefeated` no-ops when not started; activation is separate (NEO-106 wires first damaging cast). | **Adopted** (plan default; backlog activation hook) | + +## Goal, scope, and out-of-scope + +**Goal:** Server-owned per-player encounter **activation**, **defeat progress** (order-independent subset of `requiredNpcInstanceIds`), and an **idempotent completion flag** at the store layer — plus **`EncounterProgressOperations`** that resolve encounters from NPC ids via **`IEncounterDefinitionRegistry`**. + +**In scope (from Linear + [E5M3-05](E5M3-prototype-backlog.md#e5m3-05--encounter-progress--completion-stores)):** + +- **`IEncounterProgressStore`** — keyed by **`(playerId, encounterId)`**; fields: **`started`** (bool), **`defeatedNpcInstanceIds`** (set, normalized lowercase). +- **`IEncounterCompletionStore`** — **`TryMarkCompleted`**, **`IsCompleted`**, **`TryGetCompletedAt`**; second **`TryMarkCompleted`** is no-op (`false`). +- **`InMemoryEncounterProgressStore`**, **`InMemoryEncounterCompletionStore`** — thread-safe `ConcurrentDictionary` + per-key locks (mirror gathering/NPC stores). +- **`EncounterProgressOperations`**: + - **`TryActivateOnFirstEngagement(playerId, npcInstanceId, …)`** — resolve encounter; set `started` if not started and not completed. + - **`TryMarkTargetDefeated(playerId, npcInstanceId, …)`** — requires started + not completed; add NPC to defeated set if in encounter’s required list; idempotent re-mark of same NPC returns success without duplicate progress. + - **`IsAllRequiredTargetsDefeated(playerId, encounterId, …)`** — compares defeated set to registry `requiredNpcInstanceIds` (set equality). +- DI registration in **`EncounterCatalogServiceCollectionExtensions`** or new **`EncounterProgressServiceCollectionExtensions`**. +- **`InMemoryWebApplicationFactory`** — register in-memory encounter stores (remove any Postgres placeholder if added later). +- Unit tests (AAA) with injected fake stores / registry. +- `server/README.md` encounter progress section (brief). + +**Out of scope (from Linear + backlog):** + +- Inventory grants, **`EncounterCompleteEvent`**, **`EncounterCompletionOperations`** ([NEO-105](https://linear.app/neon-sprawl/issue/NEO-105)). +- Combat defeat / ability-cast wiring ([NEO-106](https://linear.app/neon-sprawl/issue/NEO-106)). +- HTTP **`GET /game/players/{id}/encounter-progress`** ([NEO-108](https://linear.app/neon-sprawl/issue/NEO-108)). +- Postgres persistence, Godot, Bruno, `docs/manual-qa/NEO-104.md`. + +## Acceptance criteria checklist + +- [ ] Defeating required NPCs accumulates progress in any order (via `TryMarkTargetDefeated` after activation). +- [ ] Completion predicate true only when all required ids defeated (`IsAllRequiredTargetsDefeated`). +- [ ] Second completion attempt on same player+encounter is idempotent no-op at store layer (`TryMarkCompleted` returns `false`). + +## Technical approach + +1. **Snapshots / keys** + - **`EncounterProgressSnapshot`**: `PlayerId`, `EncounterId`, `Started`, `DefeatedNpcInstanceIds` (read-only set). + - **`EncounterPlayerEncounterKey`**: normalize `playerId` + `encounterId` with trim + lowercase (match registry / position-state id normalization). + - NPC ids normalized the same way when stored and compared. + +2. **`IEncounterProgressStore`** + - `TryGetProgress(playerId, encounterId, out snapshot)` — missing row ⇒ not started, empty defeated set. + - `TryActivate(playerId, encounterId)` — creates row with `started: true`, empty defeated set; returns `true` on first activation, `false` if already started (idempotent) or if completion store says completed. + - `TryAddDefeatedTarget(playerId, encounterId, npcInstanceId)` — requires existing started row; add to set; return `true` if newly added, `false` if duplicate NPC or not started. + +3. **`IEncounterCompletionStore`** + - `bool IsCompleted(playerId, encounterId)` + - `bool TryMarkCompleted(playerId, encounterId, DateTimeOffset completedAt)` — `true` first time, `false` if already completed. + - `bool TryGetCompletedAt(…, out DateTimeOffset completedAt)` + +4. **`EncounterProgressOperations`** (static, mirror `GatherOperations` / `AggroOperations`) + - **`TryResolveEncounterForNpc(npcInstanceId, registry, out encounterId, out definition)`** — scan `GetDefinitionsInIdOrder()` for first encounter where `requiredNpcInstanceIds` contains normalized NPC (prototype: single `prototype_combat_pocket`). + - **`TryActivateOnFirstEngagement`**: resolve encounter → check completion store → `TryActivate` on progress store. + - **`TryMarkTargetDefeated`**: resolve encounter → deny if completed → deny if not started → validate NPC in required list → `TryAddDefeatedTarget`. + - **`IsAllRequiredTargetsDefeated`**: load progress + definition; compare required set to defeated set (order-independent). + - Inject **`TimeProvider`** only where completion store needs timestamps (completion store accepts `completedAt` from caller; NEO-105 passes `TimeProvider.GetUtcNow()`). + +5. **DI** + - `services.AddSingleton();` + - `services.AddSingleton();` + - Extend **`AddEncounterAndRewardCatalogs`** or add **`AddEncounterProgressStores`** called from **`Program.cs`** after encounter catalogs. + +6. **Reserved hook** + - XML comment on **`TryActivateOnFirstEngagement`**: `// TODO(E9.M1): encounter_start telemetry` (activates E5M3-07 hook site). + +7. **Docs** + - `server/README.md` — stores + operations, note combat/HTTP deferred. + - Update [E5_M3](E5_M3_EncounterAndRewardTables.md) implementation snapshot when landed. + +### Prototype encounter behavior (frozen) + +| Step | Expected | +|------|----------| +| Activate | `TryActivateOnFirstEngagement(devPlayer, prototype_npc_melee)` → started, 0/3 defeated | +| Defeat 2 | Mark melee + ranged → `IsAllRequiredTargetsDefeated` false | +| Defeat 3 | Mark elite → `IsAllRequiredTargetsDefeated` true | +| Complete store | `TryMarkCompleted` once → true; second → false | +| Order | elite → melee → ranged also reaches all-defeated | + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Encounters/EncounterProgressSnapshot.cs` | Immutable progress row for store read model. | +| `server/NeonSprawl.Server/Game/Encounters/IEncounterProgressStore.cs` | Progress store contract. | +| `server/NeonSprawl.Server/Game/Encounters/InMemoryEncounterProgressStore.cs` | Thread-safe in-memory implementation. | +| `server/NeonSprawl.Server/Game/Encounters/IEncounterCompletionStore.cs` | Completion store contract. | +| `server/NeonSprawl.Server/Game/Encounters/InMemoryEncounterCompletionStore.cs` | Thread-safe in-memory implementation. | +| `server/NeonSprawl.Server/Game/Encounters/EncounterProgressOperations.cs` | Static orchestration: resolve encounter, activate, mark defeat, all-defeated predicate. | +| `server/NeonSprawl.Server.Tests/Game/Encounters/EncounterProgressOperationsTests.cs` | AAA unit tests: activation, any-order defeats, all-defeated predicate, completion idempotency. | +| `server/NeonSprawl.Server.Tests/Game/Encounters/InMemoryEncounterProgressStoreTests.cs` | AAA store tests: activate idempotency, duplicate defeat, not-started deny. | +| `server/NeonSprawl.Server.Tests/Game/Encounters/InMemoryEncounterCompletionStoreTests.cs` | AAA completion idempotency tests. | +| `docs/plans/NEO-104-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Encounters/EncounterCatalogServiceCollectionExtensions.cs` | Register `IEncounterProgressStore` + `IEncounterCompletionStore` singletons (or call new extension from here). | +| `server/NeonSprawl.Server/Program.cs` | Ensure encounter progress stores registered at startup (if split extension, invoke it). | +| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Swap in test in-memory encounter stores; strip production registrations in test host. | +| `server/README.md` | Document encounter progress/completion stores and operations; defer HTTP/combat to NEO-106/108. | +| `docs/decomposition/modules/E5_M3_EncounterAndRewardTables.md` | Implementation snapshot — progress/completion stores (NEO-104). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M3 row — note NEO-104 stores when landed. | +| `docs/plans/E5M3-prototype-backlog.md` | Mark E5M3-05 AC when complete. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `InMemoryEncounterProgressStoreTests.cs` | **Arrange:** store. **Act/Assert:** first `TryActivate` true; second false; `TryAddDefeatedTarget` without activate false; duplicate NPC add false; two distinct NPCs in set. | +| `InMemoryEncounterCompletionStoreTests.cs` | **Arrange:** store + fixed `completedAt`. **Act:** `TryMarkCompleted` twice. **Assert:** first true, second false; `IsCompleted` true; `TryGetCompletedAt` matches. | +| `EncounterProgressOperationsTests.cs` | **Arrange:** real `EncounterDefinitionRegistry` from test factory catalog + in-memory stores. **Act/Assert:** activate on `prototype_npc_melee`; defeat three NPCs in permuted order → `IsAllRequiredTargetsDefeated` true only after third; partial progress false; `TryMarkTargetDefeated` before activate false; `TryMarkCompleted` idempotent; progress blocked when already completed. | +| `EncounterDefinitionRegistryTests.cs` (optional host test) | **Arrange:** `InMemoryWebApplicationFactory`. **Assert:** resolves `IEncounterProgressStore` + `IEncounterCompletionStore` from DI. | + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| **Multiple encounters sharing one NPC** | First match in `GetDefinitionsInIdOrder()` wins — acceptable for prototype (single encounter). Document in operations XML. | **adopted** | +| **Mark defeat after encounter completed** | `TryMarkTargetDefeated` returns false — fail closed. | **adopted** | +| **Postgres follow-on** | Defer — new migration + store pair in a future issue when session persistence is required. | **deferred** |