144 lines
12 KiB
Markdown
144 lines
12 KiB
Markdown
# 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
|
||
|
||
- [x] Add/remove respects **`ItemDef.stackMax`** from catalog.
|
||
- [x] Full bag (or equipment slot occupied when `stackMax` 1) returns stable **`inventory_full`** without partial silent loss.
|
||
- [x] Unknown item → **`invalid_item`**; remove over amount → **`insufficient_quantity`**.
|
||
- [x] 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.
|