184 lines
14 KiB
Markdown
184 lines
14 KiB
Markdown
# NEO-127 — Implementation plan
|
||
|
||
## Story reference
|
||
|
||
| Field | Value |
|
||
|--------|--------|
|
||
| **Key** | NEO-127 |
|
||
| **Title** | E7M2-04: RewardRouterOperations (apply QuestRewardBundle) |
|
||
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-127/e7m2-04-rewardrouteroperations-apply-questrewardbundle |
|
||
| **Module** | [E7.M2 — RewardAndUnlockRouter](../decomposition/modules/E7_M2_RewardAndUnlockRouter.md) · Epic 7 Slice 2 · backlog **E7M2-04** |
|
||
| **Branch** | `NEO-127-e7m2-reward-router-operations` |
|
||
| **Precursor** | [NEO-125](https://linear.app/neon-sprawl/issue/NEO-125) **`Done`** — catalog load + bundle validation · [NEO-126](https://linear.app/neon-sprawl/issue/NEO-126) **`Done`** — `IRewardDeliveryStore` + `RewardDeliveryEvent` |
|
||
| **Pattern** | [NEO-105](NEO-105-implementation-plan.md) — `EncounterCompletionOperations` simulate-all + sequential apply + compensating rollback; [NEO-107](NEO-107-implementation-plan.md) — store `TryRecord` after grants commit |
|
||
| **Blocks** | [NEO-128](https://linear.app/neon-sprawl/issue/NEO-128) (`QuestStateOperations.TryMarkComplete` wiring) |
|
||
| **Client counterpart** | none — server engine ([E7M2-prototype-backlog](E7M2-prototype-backlog.md#e7m2-04--rewardrouteroperations-apply-questrewardbundle)) |
|
||
|
||
## Kickoff clarifications
|
||
|
||
**No clarifications needed.** Linear AC, [E7M2-04 backlog](E7M2-prototype-backlog.md#e7m2-04--rewardrouteroperations-apply-questrewardbundle), [E7M2 kickoff decisions](E7M2-prototype-backlog.md#kickoff-decisions-decomposition-defaults), and landed NEO-105/NEO-126 code settle:
|
||
|
||
- **Namespace `Game/Rewards/`** — co-locate router with `IRewardDeliveryStore` (NEO-126); bundle DTOs stay on `QuestDefRow` in `Game/Quests/` (NEO-125).
|
||
- **Grant order** — item grants first, skill XP second; compensating **item** rollback on inventory or skill-apply failure (NEO-105 precedent). Prototype bundles have ≤1 skill row; no XP rollback helper exists.
|
||
- **Skill XP path** — call **`SkillProgressionGrantOperations.TryApplyGrant`** with **`MissionRewardSkillXpConstants.MissionRewardSourceKind`** (not the fire-and-forget **`MissionRewardSkillXpGrant`** wrapper) so deny envelopes map to router reason codes.
|
||
- **Idempotent replay** — **`IRewardDeliveryStore.TryGet`** before apply; return success with existing **`RewardDeliveryEvent`** without re-granting (backlog “idempotent replay via store”).
|
||
- **Empty bundle** — null or both-empty grant lists → success with empty snapshots and **`TryRecord`** (NEO-126 allows empty arrays; CI guarantees all four prototype quests have bundles).
|
||
- **Store race** — if grants succeed but **`TryRecord`** returns `false`, **rollback this call's item + skill XP grants** and return the winner's store event (mirror **`EncounterCompletionOperations`** when **`TryMarkCompleted`** loses); do not retain duplicate grants on the loser.
|
||
|
||
## Goal, scope, and out-of-scope
|
||
|
||
**Goal:** Server-internal apply for quest **`completionRewardBundle`** rows (items + skill XP) with structured outcomes and durable idempotency via **`IRewardDeliveryStore`**.
|
||
|
||
**In scope (from Linear + [E7M2-04](E7M2-prototype-backlog.md#e7m2-04--rewardrouteroperations-apply-questrewardbundle)):**
|
||
|
||
- **`RewardRouterOperations.TryDeliverQuestCompletion`** in `server/NeonSprawl.Server/Game/Rewards/`.
|
||
- Item grants via **`PlayerInventoryOperations`** (simulate-all pre-flight, sequential apply, compensating rollback).
|
||
- Skill XP via **`SkillProgressionGrantOperations.TryApplyGrant`** with fixed **`mission_reward`** source kind.
|
||
- **`RewardDeliveryReasonCodes`** + **`RewardDeliveryResult`** (success, deny reason, grant snapshots, **`RewardDeliveryEvent?`**).
|
||
- Unit + integration tests (AAA): happy path (skill-only + item+skill bundles), inventory deny, idempotent store replay, apply-phase item rollback, skill deny passthrough.
|
||
- `server/README.md` router section.
|
||
|
||
**Out of scope (from Linear):**
|
||
|
||
- Quest state transition hooks ([NEO-128](https://linear.app/neon-sprawl/issue/NEO-128) E7M2-05).
|
||
- HTTP DTOs / **`completionRewardSummary`** ([NEO-129](https://linear.app/neon-sprawl/issue/NEO-129) E7M2-06).
|
||
- E9.M1 telemetry hook comments ([NEO-130](https://linear.app/neon-sprawl/issue/NEO-130) E7M2-07).
|
||
- Godot client, Bruno (optional on NEO-128).
|
||
|
||
## Acceptance criteria checklist
|
||
|
||
- [x] Bundle rows apply atomically where possible; compensating rollback on partial failure before store record.
|
||
- [x] Skill XP uses **`mission_reward`** source kind only.
|
||
|
||
## Implementation reconciliation (shipped)
|
||
|
||
- **Operations:** `RewardRouterOperations.TryDeliverQuestCompletion` — item pre-flight + apply, skill XP via `TryApplyGrant` (`mission_reward`), compensating item rollback + per-grant perk tracking + **`TryRevertAppliedSkillGrants`** on **`TryRecord`** race loss, `IRewardDeliveryStore.TryRecord`.
|
||
- **Types:** `RewardDeliveryResult`, `RewardDeliveryReasonCodes`.
|
||
- **Tests:** 14 AAA tests in `RewardRouterOperationsTests` (…, TryRecord race rollback, concurrent race no double-grant, perk revert on simulated race loss, concurrent level-up winner perk preserved).
|
||
- **Docs:** `server/README.md` reward router section; E7.M2 module anchor + alignment register updated.
|
||
|
||
## Technical approach
|
||
|
||
### 1. Result + reason types (`Game/Rewards/`)
|
||
|
||
**`RewardDeliveryReasonCodes`** — stable strings for router denies:
|
||
|
||
| Code | When |
|
||
|------|------|
|
||
| **`already_delivered`** | **`IRewardDeliveryStore.TryGet`** hit before apply (idempotent replay — success path, not a deny). |
|
||
| **`inventory_store_missing`** | Inventory snapshot unavailable. |
|
||
| **`inventory_full`**, **`invalid_item`**, **`invalid_quantity`** | Passthrough from **`PlayerInventoryReasonCodes`** via **`PlayerInventoryOperations`**. |
|
||
| **`unknown_skill`**, **`invalid_amount`**, **`source_kind_not_allowed`** | Passthrough from **`SkillProgressionSnapshotApi`** reason constants via **`TryApplyGrant`**. |
|
||
| **`progression_store_missing`** | **`SkillProgressionGrantApplyKind.ProgressionStoreMissing`**. |
|
||
|
||
**`RewardDeliveryResult`** — readonly record:
|
||
|
||
| Field | Role |
|
||
|-------|------|
|
||
| **`Success`** | Grants committed (includes idempotent replay). |
|
||
| **`ReasonCode`** | Set when **`Success`** is false. |
|
||
| **`GrantedItems`** | Applied item snapshot (`RewardItemGrantApplied`). |
|
||
| **`GrantedSkillXp`** | Applied skill XP snapshot (`RewardSkillXpGrantApplied`). |
|
||
| **`DeliveryEvent`** | Populated on success (fresh or replay). |
|
||
| **`DeliveredAt`** | From event on success. |
|
||
|
||
### 2. `RewardRouterOperations.TryDeliverQuestCompletion`
|
||
|
||
Signature (static ops, mirror encounter completion):
|
||
|
||
```csharp
|
||
public static RewardDeliveryResult TryDeliverQuestCompletion(
|
||
string playerId,
|
||
string questId,
|
||
QuestRewardBundleRow? bundle,
|
||
IItemDefinitionRegistry itemRegistry,
|
||
IPlayerInventoryStore inventoryStore,
|
||
ISkillDefinitionRegistry skillRegistry,
|
||
IPlayerSkillProgressionStore skillProgressionStore,
|
||
ISkillLevelCurve levelCurve,
|
||
PerkUnlockEngine perkUnlockEngine,
|
||
IPlayerPerkStateStore perkStore,
|
||
IRewardDeliveryStore deliveryStore,
|
||
TimeProvider timeProvider)
|
||
```
|
||
|
||
**Flow:**
|
||
|
||
1. **Normalize ids** via **`RewardDeliveryIds`**; fail closed on empty player/quest id (deny with appropriate code or empty reason — match store fail-closed).
|
||
2. **Idempotent replay** — if **`deliveryStore.TryGet`**, return **`Success: true`** with existing event and grant snapshots (no inventory/skill mutation).
|
||
3. **Resolve grants** — treat null **`bundle`** as empty item + skill lists.
|
||
4. **Item pre-flight** — clone inventory snapshot; **`TrySimulateAddStack`** for each **`bundle.ItemGrants`** row in catalog order.
|
||
5. **Item apply** — foreach row **`TryAddStack`**; accumulate **`RewardItemGrantApplied`**; on deny/store-missing, return deny **without** skill apply.
|
||
6. **Skill XP apply** — foreach **`bundle.SkillXpGrants`** row, **`TryApplyGrant(playerId, skillId, amount, MissionRewardSourceKind, …)`**:
|
||
- On **`Denied`**, **`CompensatingRemoveItems`** for applied item rows and return mapped **`ReasonCode`** from **`outcome.Response!.ReasonCode`**.
|
||
- On **`ProgressionStoreMissing`**, compensating item rollback + deny.
|
||
- On **`Granted`**, append **`RewardSkillXpGrantApplied`**.
|
||
7. **Record** — build **`RewardDeliveryEvent`** with **`DeliveredAt = timeProvider.GetUtcNow()`**, **`IdempotencyKey = RewardDeliveryIds.MakeIdempotencyKey(...)`**, grant snapshots.
|
||
8. **`deliveryStore.TryRecord`** — if `true`, return fresh success; if `false` (race), **compensating rollback** of this call's item + skill XP grants, then return **`TryGet`** winner event with **`already_delivered`** (or deny if store miss).
|
||
|
||
**Compensating item rollback** — private helper mirroring **`EncounterCompletionOperations.CompensatingRemoveGrants`**: reverse-order **`TryRemoveStack`** for applied item rows only (no skill XP undo).
|
||
|
||
**Compensating skill progression rollback** — track perks unlocked per skill grant (`CollectPerksUnlockedSince`); on **`TryRecord`** race loss, **`TryRevertAppliedSkillGrants`** reverses each applied skill XP row via negative delta and removes only perks this call unlocked, then **`ReevaluateAfterLevelUp`** at post-rollback levels re-applies path-auto perks the winner's XP still qualifies for (avoids full snapshot restore clobbering a concurrent winner).
|
||
|
||
### 3. Skill XP source kind enforcement
|
||
|
||
Always pass **`MissionRewardSkillXpConstants.MissionRewardSourceKind`** at apply time — content rows do not carry **`sourceKind`** (NEO-124/NEO-43). Startup catalog gates already require skills to allow **`mission_reward`**; router still maps **`source_kind_not_allowed`** if a deny occurs.
|
||
|
||
### 4. Tests (`RewardRouterOperationsTests`)
|
||
|
||
Use **`InMemoryWebApplicationFactory`** + resolved DI (same pattern as **`EncounterCompletionOperationsTests`**):
|
||
|
||
| Test | Coverage |
|
||
|------|----------|
|
||
| **`TryDeliverQuestCompletion_ShouldApplyGatherIntroSkillXp_WhenBundleIsSkillOnly`** | **`prototype_quest_gather_intro`**-shaped bundle → salvage +25; store recorded; replay via second call returns same event without double XP. |
|
||
| **`TryDeliverQuestCompletion_ShouldApplyItemAndSkillXp_WhenOperatorChainBundle`** | **`survey_drone_kit` ×1** + salvage +50; inventory + progression assertions. |
|
||
| **`TryDeliverQuestCompletion_ShouldDenyInventoryFull_WithoutStoreRecord`** | Fill bag; deny **`inventory_full`**; **`TryGet`** false. |
|
||
| **`TryDeliverQuestCompletion_ShouldRollbackItems_WhenSkillApplyDenied`** | Item grant succeeds then skill deny (inject invalid amount or use test-only bundle row) → item removed; no store record. |
|
||
| **`TryDeliverQuestCompletion_ShouldReturnExistingEvent_WhenAlreadyDelivered`** | Pre-seed **`IRewardDeliveryStore`**; call router → success, no inventory/XP change. |
|
||
| **`TryDeliverQuestCompletion_ShouldUseMissionRewardSourceKind`** | Assert grant succeeds for catalog skill; optional negative with skill lacking allowlist (unit-level bundle row). |
|
||
|
||
Optional host smoke: resolve **`RewardRouterOperations`** is static — assert factory provides all injected stores/registries used in one happy-path call.
|
||
|
||
### 5. Docs
|
||
|
||
- **`server/README.md`** — new **Reward router (NEO-127)** section: operation entry point, reason codes, apply order, idempotency + store interaction, pointer to NEO-128 wiring.
|
||
- **`documentation_and_implementation_alignment.md`** — E7.M2 row: router apply landed (NEO-127).
|
||
- **`E7_M2_RewardAndUnlockRouter.md`** — implementation anchor for **`RewardRouterOperations`**.
|
||
|
||
## Files to add
|
||
|
||
| Path | Purpose |
|
||
|------|---------|
|
||
| `server/NeonSprawl.Server/Game/Rewards/RewardDeliveryReasonCodes.cs` | Stable router deny reason strings. |
|
||
| `server/NeonSprawl.Server/Game/Rewards/RewardDeliveryResult.cs` | Structured outcome + grant snapshots + optional event. |
|
||
| `server/NeonSprawl.Server/Game/Rewards/RewardRouterOperations.cs` | `TryDeliverQuestCompletion` apply + rollback + store record. |
|
||
| `server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs` | AAA happy path, deny, rollback, idempotent replay tests. |
|
||
| `docs/plans/NEO-127-implementation-plan.md` | This plan. |
|
||
|
||
## Files to modify
|
||
|
||
| Path | Rationale |
|
||
|------|-----------|
|
||
| `server/README.md` | Document `RewardRouterOperations`, reason codes, idempotency flow. |
|
||
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E7.M2 alignment — router apply (NEO-127). |
|
||
| `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` | Add NEO-127 implementation anchor after delivery store bullet. |
|
||
|
||
## Tests
|
||
|
||
| Target | Coverage |
|
||
|--------|----------|
|
||
| `server/NeonSprawl.Server.Tests/Game/Rewards/RewardRouterOperationsTests.cs` | Skill-only + item+skill happy paths; `inventory_full` fail-closed; item compensating rollback on skill deny; store idempotent replay; `mission_reward` apply path. |
|
||
| **Optional** — extend existing host/DI test | Factory resolves all router dependencies (smoke only). |
|
||
| **Manual verification** | `dotnet test server/NeonSprawl.Server.Tests --filter RewardRouterOperationsTests`; dev server boot unchanged until NEO-128 wires quest complete. |
|
||
|
||
## Open questions / risks
|
||
|
||
| Question or risk | Agent recommendation | Status |
|
||
|------------------|----------------------|--------|
|
||
| Multi-row skill XP partial apply | Prototype bundles have ≤1 skill row; implement sequential apply with item rollback on any skill deny; defer XP compensating rollback unless a future story requires multi-row bundles. | **adopted** |
|
||
| `MissionRewardSkillXpGrant` vs `TryApplyGrant` | Use **`TryApplyGrant`** directly for deny envelopes; keep **`MissionRewardSkillXpGrant`** as HTTP/convenience wrapper only. | **adopted** |
|
||
| Null bundle on future quests | Treat as empty grants + record delivery for idempotency; NEO-128 may skip router call when bundle absent — both safe. | **adopted** |
|
||
| Quest-state rollback on deny | NEO-128 scope per E7M2 fail-closed decision; NEO-127 returns structured deny only. | **deferred** |
|
||
|
||
None beyond the above.
|