From b5306aa933f7eee265627254501ae6263275f4fd Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 31 May 2026 16:55:22 -0400 Subject: [PATCH] NEO-105: Add kickoff implementation plan for encounter completion grants. --- docs/plans/NEO-105-implementation-plan.md | 124 ++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 docs/plans/NEO-105-implementation-plan.md diff --git a/docs/plans/NEO-105-implementation-plan.md b/docs/plans/NEO-105-implementation-plan.md new file mode 100644 index 0000000..3036207 --- /dev/null +++ b/docs/plans/NEO-105-implementation-plan.md @@ -0,0 +1,124 @@ +# NEO-105 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-105 | +| **Title** | E5M3-06: EncounterCompletionOperations + inventory reward apply | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-105/e5m3-06-encountercompletionoperations-inventory-reward-apply | +| **Module** | [E5.M3 — EncounterAndRewardTables](../decomposition/modules/E5_M3_EncounterAndRewardTables.md) · Epic 5 Slice 3 · backlog **E5M3-06** | +| **Branch** | `NEO-105-encounter-completion-inventory-reward-apply` | +| **Precursor** | [NEO-104](https://linear.app/neon-sprawl/issue/NEO-104) — `IEncounterProgressStore` + `IEncounterCompletionStore` + `EncounterProgressOperations` (**landed on `main`**) | +| **Pattern** | [NEO-62](NEO-62-implementation-plan.md) / `GatherOperations` — static ops + reason codes + grant summary; [NEO-69](NEO-69-implementation-plan.md) / `CraftOperations` — simulate-all pre-flight + compensating rollback on partial apply failure | +| **Blocks** | [NEO-106](https://linear.app/neon-sprawl/issue/NEO-106) — combat defeat → progress/completion wiring; [NEO-107](https://linear.app/neon-sprawl/issue/NEO-107) — `EncounterCompleteEvent` persistence + E7.M2 hook stub | +| **Client counterpart** | [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110) — encounter progress + loot feedback labels (via [NEO-108](https://linear.app/neon-sprawl/issue/NEO-108) HTTP read) | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **EncounterCompleteEvent scope** | Define event type on NEO-105 vs defer entirely to NEO-107? | **Define `EncounterCompleteEvent` + return in `EncounterCompletionResult` on success** — E5M3-06 goal calls for internal completion payload; **NEO-107** persists the record and adds E7.M2 hook stub (E5M3-09). | **Adopted** — result payload only; no persistence | +| **Grant transaction** | Pre-flight vs apply-then-hope? | **Simulate all `fixedGrants` on inventory snapshot, then apply sequentially with compensating rollback** — mirror `CraftOperations`; never `TryMarkCompleted` unless every grant applied. | **Adopted** (repo precedent; Linear AC “inventory deny fails closed”) | +| **Completion gate** | Who checks all-defeated? | **`TryCompleteAndGrant` requires `EncounterProgressOperations.IsAllRequiredTargetsDefeated`** + not already completed — NEO-106 calls after last defeat mark. | **Adopted** (backlog + NEO-104 split) | +| **Combat / HTTP wiring** | Wire `AbilityCastApi` or HTTP on this story? | **Defer** — NEO-106 / NEO-108; NEO-105 ships operations + unit tests only. | **Adopted** (backlog out-of-scope) | +| **Bruno** | Add encounter-completion Bruno folder? | **Defer to NEO-108** — no player HTTP route yet; backlog marks Bruno optional here. | **Adopted** (backlog) | + +## Goal, scope, and out-of-scope + +**Goal:** `EncounterCompletionOperations.TryCompleteAndGrant` applies **`RewardTable.fixedGrants`** via **`PlayerInventoryOperations`** exactly once per player+encounter, marks the completion store only when all grants succeed, and returns an internal **`EncounterCompleteEvent`** payload for downstream routers (persistence deferred). + +**In scope (from Linear + [E5M3-06](E5M3-prototype-backlog.md#e5m3-06--encountercompletionoperations--inventory-reward-apply)):** + +- **`EncounterCompletionOperations.TryCompleteAndGrant`** — resolve encounter → reward table → pre-flight simulate → apply fixed grants → `TryMarkCompleted` with `TimeProvider.GetUtcNow()`. +- **`EncounterCompletionResult`**, **`EncounterCompletionReasonCodes`**, **`EncounterGrantApplied`**, **`EncounterCompleteEvent`** (internal record; not persisted). +- Structured deny codes: `already_completed`, `not_ready`, `unknown_encounter`, `unknown_reward_table`, `inventory_full`, `inventory_store_missing`, passthrough `invalid_item` / `invalid_quantity`. +- Compensating inventory rollback when a later grant fails after earlier grants applied, or when `TryMarkCompleted` returns false after grants (race/idempotent replay). +- Unit tests (AAA) with `InMemoryWebApplicationFactory`: first completion grants frozen prototype loot once; second completion no duplicate; bag-full deny without completion flag. +- `server/README.md` encounter completion section (brief). + +**Out of scope (from Linear + backlog):** + +- Combat defeat wiring ([NEO-106](https://linear.app/neon-sprawl/issue/NEO-106)). +- **`EncounterCompleteEvent` store / E7.M2 quest-credit hook** ([NEO-107](https://linear.app/neon-sprawl/issue/NEO-107)). +- HTTP **`GET /game/players/{id}/encounter-progress`** ([NEO-108](https://linear.app/neon-sprawl/issue/NEO-108)). +- Godot HUD — **client counterpart [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110)**. +- Postgres persistence, Bruno, `docs/manual-qa/NEO-105.md`. + +## Acceptance criteria checklist + +- [ ] First completion grants **`scrap_metal_bulk` ×10** and **`contract_handoff_token` ×1** deterministically. +- [ ] Second completion does not duplicate grants. +- [ ] Inventory deny fails closed without marking complete. + +## Technical approach + +1. **`EncounterCompletionReasonCodes`** + - Domain-specific: `already_completed`, `not_ready`, `unknown_encounter`, `unknown_reward_table`, `inventory_store_missing`. + - Passthrough: `inventory_full`, `invalid_item`, `invalid_quantity` from **`PlayerInventoryReasonCodes`**. + +2. **`EncounterGrantApplied`** — readonly record `(ItemId, Quantity)` mirroring **`GatherGrantApplied`**. + +3. **`EncounterCompleteEvent`** — readonly record: `PlayerId`, `EncounterId`, `RewardTableId`, `IReadOnlyList GrantedItems`, `DateTimeOffset CompletedAt`, `IdempotencyKey` (stable string e.g. `{playerId}:{encounterId}` normalized). Returned on success only; **NEO-107** will persist/emit. + +4. **`EncounterCompletionResult`** — `(Success, ReasonCode?, GrantsApplied, EncounterCompleteEvent? CompleteEvent, DateTimeOffset? CompletedAt)`. + +5. **`EncounterCompletionOperations.TryCompleteAndGrant`** + - Normalize **`encounterId`** via **`IEncounterDefinitionRegistry.TryNormalizeKnown`**; load **`EncounterDefRow`**. + - Early deny **`already_completed`** if **`IEncounterCompletionStore.IsCompleted`**. + - Early deny **`not_ready`** if **`!EncounterProgressOperations.IsAllRequiredTargetsDefeated`**. + - Resolve **`rewardTableId`** → **`IRewardTableDefinitionRegistry.TryGetDefinition`**; deny **`unknown_reward_table`** if missing. + - Load **`FixedGrants`** (prototype: scrap ×10, token ×1). + - **Pre-flight:** clone inventory snapshot; **`PlayerInventoryOperations.TrySimulateAddStack`** for each grant in catalog order. + - **Apply:** foreach grant **`TryAddStack`**; on deny/store-missing, **`CompensatingRemoveGrants`** for prior applied rows and return deny **without** marking complete. + - **`TryMarkCompleted(playerId, encounterId, timeProvider.GetUtcNow())`**; if `false`, compensating remove all grants and return **`already_completed`**. + - Success: build **`EncounterCompleteEvent`**, return grants + event + `CompletedAt`. + - Comment-only hook site: `// TODO(E7.M2): quest-credit router consumes EncounterCompleteEvent` (NEO-107 expands). + +6. **No new DI registration** — static operations like **`GatherOperations`**; callers inject stores/registries from existing **`AddEncounterAndRewardCatalogs`** + inventory DI. + +### Prototype completion flow (frozen) + +| Step | Expected | +|------|----------| +| Setup | Activate + defeat all three NPCs → `IsAllRequiredTargetsDefeated` true | +| First grant | `TryCompleteAndGrant` → success; bag +10 scrap, +1 token; completion store marked | +| Replay | Second `TryCompleteAndGrant` → deny `already_completed`; inventory unchanged | +| Bag full | Fill bag slots → `TryCompleteAndGrant` → deny `inventory_full`; `IsCompleted` false | + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Encounters/EncounterCompletionReasonCodes.cs` | Stable deny reason constants. | +| `server/NeonSprawl.Server/Game/Encounters/EncounterGrantApplied.cs` | Successful grant row in result/event. | +| `server/NeonSprawl.Server/Game/Encounters/EncounterCompleteEvent.cs` | Internal completion payload (returned, not persisted on NEO-105). | +| `server/NeonSprawl.Server/Game/Encounters/EncounterCompletionResult.cs` | Server-internal completion resolution envelope. | +| `server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs` | `TryCompleteAndGrant` orchestration. | +| `server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCompletionOperationsTests.cs` | AAA unit tests for grant-once, idempotent deny, inventory-full fail-closed. | +| `docs/plans/NEO-105-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/README.md` | Document `EncounterCompletionOperations`, reason codes, and deferrals to NEO-106/107/108. | +| `docs/decomposition/modules/E5_M3_EncounterAndRewardTables.md` | Implementation snapshot — completion + inventory grant ops (NEO-105). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M3 row — note NEO-105 completion grants when landed. | +| `docs/decomposition/modules/module_dependency_register.md` | E5.M3 note — NEO-105 completion ops landed (when shipped). | +| `docs/plans/E5M3-prototype-backlog.md` | Mark E5M3-06 AC when complete. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `EncounterCompletionOperationsTests.cs` | **Arrange:** `InMemoryWebApplicationFactory` + activate/defeat-all helper for `prototype_combat_pocket`. **Act/Assert:** first `TryCompleteAndGrant` success with scrap ×10 + token ×1 in inventory and `CompleteEvent` populated; second call `already_completed` with unchanged quantities; `not_ready` when only 2/3 defeated; bag-full deny `inventory_full` with `IsCompleted` false and zero grant delta; compensating rollback when bag cannot fit both grants (fill bag before call). | + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| **Grant order** | Apply in **`FixedGrants` catalog order** (scrap then token); rollback in reverse on failure. | **adopted** | +| **Mark-complete race** | If `TryMarkCompleted` false after grants, remove grants and return `already_completed` — treat as lost race to another completer. | **adopted** | +| **Event persistence** | Deferred to **NEO-107** — NEO-105 only returns payload in result. | **adopted** | +| **Postgres follow-on** | Defer — completion store Postgres migration is a future session-persistence issue. | **deferred** |