diff --git a/docs/plans/NEO-54-implementation-plan.md b/docs/plans/NEO-54-implementation-plan.md new file mode 100644 index 0000000..eafe6b1 --- /dev/null +++ b/docs/plans/NEO-54-implementation-plan.md @@ -0,0 +1,143 @@ +# NEO-54 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-54 | +| **Title** | E3.M3: Player inventory store + stack/slot rules engine | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-54/e3m3-player-inventory-store-stackslot-rules-engine | +| **Module** | [E3.M3 — ItemizationAndInventorySchema](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) · Epic 3 Slice 1 (E3M3-05) | +| **Branch** | `NEO-54-player-inventory-store-stackslot-rules` | +| **Precursor** | [NEO-52](https://linear.app/neon-sprawl/issue/NEO-52) — `IItemDefinitionRegistry` + DI (**Done** on `main`); [NEO-53](https://linear.app/neon-sprawl/issue/NEO-53) — read-only catalog HTTP (**Done** on `main`) | +| **Pattern** | [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) — `IPlayerSkillProgressionStore` + `SkillProgressionGrantOperations` persistence policy | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Slot capacity** | Fixed bag vs equipment slot counts? | **24 bag + 1 equipment** — enough for gather/craft QA; matches single `equip_stub` row in prototype catalog. | **User:** 24 bag + 1 equipment. | +| **Add overflow** | Partial add vs all-or-nothing deny? | **All-or-nothing** — if the full requested quantity cannot be placed (stack merge + empty slots), deny with **`inventory_full`** and leave inventory unchanged (AC: no partial silent loss). | **User:** all-or-nothing. | +| **Architecture** | Monolithic store vs split engine? | **Split:** **`IPlayerInventoryStore`** (persistence) + **`PlayerInventoryOperations`** (stack/slot rules, reason codes), mirroring NEO-38 store + grant operations. | **User:** split engine. | + +## Goal, scope, and out-of-scope + +**Goal:** Server-authoritative per-player **`ItemInstance`** storage in fixed **`InventorySlot`** containers with stack limits from **`IItemDefinitionRegistry`**, structured deny **`reasonCode`** values, and dual persistence (in-memory + Postgres when configured). + +**In scope (from Linear + [E3M3-05](E3M3-prototype-backlog.md#e3m3-05--player-inventory-store--stackslot-rules-engine)):** + +- **`IPlayerInventoryStore`**, **`InMemoryPlayerInventoryStore`**, **`PostgresPlayerInventoryStore`**, **`PostgresPlayerInventoryBootstrap`**, **`PlayerInventoryServiceCollectionExtensions`**. +- **`PlayerInventoryOperations`** (add/remove stack with catalog validation and slot assignment). +- Domain types: **`InventoryContainerKind`**, **`InventorySlotState`**, **`PlayerInventorySnapshot`**, mutation outcome types with stable **`reasonCode`** strings. +- Migration **`V005__player_inventory.sql`** (sparse occupied-slot rows; empty slots implied). +- Unit tests (AAA) for engine rules; Postgres + in-memory **parity** integration tests (AAA). + +**Out of scope (from Linear):** + +- HTTP, Bruno, client HUD ([NEO-55](https://linear.app/neon-sprawl/issue/NEO-55)). +- Gather/craft automatic grants (callers in E3.M1 / E3.M2). +- Telemetry hook sites ([NEO-56](https://linear.app/neon-sprawl/issue/NEO-56)). +- Optional schema fields (`rarity`, `bindPolicy`, `durabilityMax`) on instances. + +## Acceptance criteria checklist + +- [ ] Add/remove respects **`ItemDef.stackMax`** from catalog. +- [ ] Full bag (or equipment slot occupied when `stackMax` 1) returns stable **`inventory_full`** without partial silent loss. +- [ ] Unknown item → **`invalid_item`**; remove over amount → **`insufficient_quantity`**. +- [ ] Postgres + in-memory parity tests (AAA). + +## Technical approach + +1. **Containers and capacity (kickoff):** + - **`bag`:** **24** fixed slots, indices **0–23**. Accepts defs with **`inventorySlotKind: bag`** only. + - **`equipment`:** **1** fixed slot, index **0**. Accepts defs with **`inventorySlotKind: equipment`** only. + - Routing is automatic from catalog metadata; callers pass **`itemId` + quantity** only (no manual slot placement in this story). + +2. **Slot model:** Each slot is **`{ slotIndex, itemId?, quantity }`**. Empty slot ⇒ **`itemId`** omitted / null and **`quantity` 0**. Occupied slot ⇒ **`quantity`** in **1..stackMax** for that def. **`PlayerInventorySnapshot`** exposes **`BagSlots`** (length 24) and **`EquipmentSlots`** (length 1) for NEO-55 GET projection. + +3. **Add algorithm (`TryAddStack`):** + - Reject non-positive quantity → **`invalid_quantity`** (stable code; document alongside other denies). + - **`IItemDefinitionRegistry.TryGetDefinition`** → on miss **`invalid_item`**. + - Load snapshot; simulate on the def’s container: + 1. Merge into existing stacks of the same **`itemId`** up to **`stackMax`**. + 2. Place remainder in empty slots (one stack per slot, capped at **`stackMax`**). + - If any quantity remains unplaced → **`inventory_full`**, **no write**. + - Else atomically persist new snapshot → success outcome with updated snapshot. + +4. **Remove algorithm (`TryRemoveStack`):** + - Same quantity / catalog guards as add. + - Sum quantity for **`itemId`** in the correct container; if total **< requested** → **`insufficient_quantity`**, no write. + - Drain stacks (lowest **`slotIndex`** first or stable left-to-right order — pick one, document, test). + - Persist; empty slots drop from Postgres rows. + +5. **Persistence policy (NEO-38 / NEO-29 mirror):** + - **`AddPlayerInventoryStore`** chooses **`PostgresPlayerInventoryStore`** when **`ConnectionStrings__NeonSprawl`** is set; else **`InMemoryPlayerInventoryStore`** seeding configured dev player (same pattern as skill progression / hotbar). + - Postgres: **`player_id`** FK → **`player_position`**, transaction per mutation, **`PlayerExists`** gate before write. + - DDL: **`V005__player_inventory.sql`** — **`(player_id, container_kind, slot_index)`** PK, **`item_id`**, **`quantity`**, **`updated_at`**; only non-empty slots stored. + - Bootstrap: **`PostgresPlayerInventoryBootstrap.EnsureSchema`** loads embedded migration from **`db/migrations/`**. + +6. **Store interface (minimal):** + - **`TryGetSnapshot(string playerId, out PlayerInventorySnapshot snapshot)`** — missing player ⇒ false (engine maps to store-missing path if needed). + - **`TryReplaceSnapshot(string playerId, PlayerInventorySnapshot snapshot)`** — atomic replace after engine validation (in-memory per-player lock; Postgres upsert/delete in one transaction). + +7. **Reason codes (snake_case constants, shared by engine + tests):** + - **`inventory_full`**, **`invalid_item`**, **`insufficient_quantity`**, **`invalid_quantity`**. + - Document in code XML + brief **`server/README.md`** inventory subsection (persistence only; HTTP deferred to NEO-55). + +8. **DI / tests:** + - Register in **`Program.cs`** via **`AddPlayerInventoryStore`** after item catalog registration. + - **`InMemoryWebApplicationFactory`**: swap in **`InMemoryPlayerInventoryStore`** (same removal pattern as other Postgres-backed stores). + - Engine tests use factory-resolved **`IItemDefinitionRegistry`** + in-memory store — no HTTP. + +9. **Docs (on land):** Update [E3_M3](E3_M3_ItemizationAndInventorySchema.md) **Related implementation slices** and [documentation_and_implementation_alignment.md](documentation_and_implementation_alignment.md) E3.M3 row. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Items/InventoryContainerKind.cs` | `Bag` / `Equipment` enum for container routing and persistence. | +| `server/NeonSprawl.Server/Game/Items/InventorySlotState.cs` | One slot: index, optional `itemId`, quantity. | +| `server/NeonSprawl.Server/Game/Items/PlayerInventorySnapshot.cs` | Fixed-size bag + equipment slot arrays for read/write. | +| `server/NeonSprawl.Server/Game/Items/IPlayerInventoryStore.cs` | Persistence abstraction: get/replace snapshot per player. | +| `server/NeonSprawl.Server/Game/Items/InMemoryPlayerInventoryStore.cs` | Thread-safe in-memory store; seeds dev player empty inventory. | +| `server/NeonSprawl.Server/Game/Items/PostgresPlayerInventoryStore.cs` | Postgres sparse slot rows; transactional replace. | +| `server/NeonSprawl.Server/Game/Items/PostgresPlayerInventoryBootstrap.cs` | One-time DDL ensure from `V005__player_inventory.sql`. | +| `server/NeonSprawl.Server/Game/Items/PlayerInventoryServiceCollectionExtensions.cs` | Postgres vs in-memory registration from configuration. | +| `server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs` | Add/remove stack rules, reason codes, catalog integration. | +| `server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs` | Success/deny result types (`reasonCode`, snapshot). | +| `server/NeonSprawl.Server/Game/Items/PlayerInventoryReasonCodes.cs` | Stable string constants for denies. | +| `server/db/migrations/V005__player_inventory.sql` | Postgres table for occupied inventory slots. | +| `server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryOperationsTests.cs` | AAA unit/integration: add merge, stack max, bag full, equipment single slot, remove, reason codes. | +| `server/NeonSprawl.Server.Tests/Game/Items/InMemoryPlayerInventoryStoreTests.cs` | Store get/replace, dev player seed, normalization. | +| `server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryPersistenceIntegrationTests.cs` | Postgres: mutate, new factory, snapshot parity (mirror skill progression persistence tests). | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Program.cs` | Register **`AddPlayerInventoryStore`** after item catalog. | +| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Force in-memory inventory store; strip Postgres inventory registration in tests. | +| `server/README.md` | Inventory persistence subsection: slot counts, reason codes, migration note (HTTP in NEO-55). | +| `docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md` | **Related implementation slices** — inventory store + engine bullet (NEO-54). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E3.M3 row — note NEO-54 store/engine when landed. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `PlayerInventoryOperationsTests.cs` | **AAA** via in-memory host: add to empty bag; merge into partial stack respecting **`stackMax`** (e.g. **`field_stim_mk0`** max 20); fill 24 bag slots then **`inventory_full`** on further bag add; add **`prototype_armor_shell`** to equipment slot; second equipment add **`inventory_full`**; unknown id **`invalid_item`**; remove happy path; remove over amount **`insufficient_quantity`**; non-positive quantity **`invalid_quantity`**; snapshot unchanged on deny. | +| `InMemoryPlayerInventoryStoreTests.cs` | **AAA:** dev player seeded empty; **`TryReplaceSnapshot`** round-trip; unknown player false. | +| `PlayerInventoryPersistenceIntegrationTests.cs` | **AAA** `[RequirePostgresFact]`: add stacks via engine, new **`PostgresWebApplicationFactory`**, **`TryGetSnapshot`** matches; deny path does not persist partial state. | + +No Bruno or manual QA doc — no HTTP surface in this story. + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| **Remove drain order** | Drain **lowest slot index first** (deterministic, easy to test). | `adopted` | +| **Cross-container same item id** | Prototype catalog has disjoint ids; engine scopes by container from def — no cross-container moves. | `adopted` | +| **NEO-55 HTTP shape** | Snapshot types here should map cleanly to future GET DTOs (`bagSlots`, `equipmentSlots`). | `deferred` to NEO-55 | +| **Concurrency** | Postgres transaction per mutation; in-memory per-player lock (NEO-38 precedent). | `adopted` | + +None blocking.