diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 6204f1e..a6b1fe1 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -48,7 +48,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen | E3.M4 | SinkAndDurabilityLifecycle | E3.M3, E8.M3 | DurabilityState, ItemSinkEvent, RepairCostRule | Pre-production | Planned | | E3.M5 | EconomyBalancePolicy | E3.M4, E9.M2 | EconomyPolicy, PriceBandRule, FaucetSinkRatio | Pre-production | Planned | -**E3.M3 note:** Epic 3 **Slice 1** backlog in Linear ([Epic 3 — Crafting, Gathering, and Itemization Economy](https://linear.app/neon-sprawl/project/epic-3-crafting-gathering-and-itemization-economy-65785ed05bc2)): [NEO-50](https://linear.app/neon-sprawl/issue/NEO-50) → [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56); label **`E3.M3`**. See [E3M3-prototype-backlog.md](../../plans/E3M3-prototype-backlog.md), [E3_M3_ItemizationAndInventorySchema.md](E3_M3_ItemizationAndInventorySchema.md). **NEO-50** (content + CI), **NEO-51** (server fail-fast load), **NEO-52** (`IItemDefinitionRegistry` + DI), and **NEO-53** (`GET /game/world/item-definitions`) moved the register row to **In Progress**; later slices update the alignment table as they land. +**E3.M3 note:** Epic 3 **Slice 1** backlog in Linear ([Epic 3 — Crafting, Gathering, and Itemization Economy](https://linear.app/neon-sprawl/project/epic-3-crafting-gathering-and-itemization-economy-65785ed05bc2)): [NEO-50](https://linear.app/neon-sprawl/issue/NEO-50) → [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56); label **`E3.M3`**. See [E3M3-prototype-backlog.md](../../plans/E3M3-prototype-backlog.md), [E3_M3_ItemizationAndInventorySchema.md](E3_M3_ItemizationAndInventorySchema.md). **NEO-50** (content + CI), **NEO-51** (server fail-fast load), **NEO-52** (`IItemDefinitionRegistry` + DI), **NEO-53** (`GET /game/world/item-definitions`), and **NEO-54** (inventory store + stack/slot rules engine) moved the register row to **In Progress**; later slices update the alignment table as they land. ### Epic 4 — World Topology diff --git a/docs/reviews/2026-05-23-NEO-54.md b/docs/reviews/2026-05-23-NEO-54.md index 4e36849..2037c49 100644 --- a/docs/reviews/2026-05-23-NEO-54.md +++ b/docs/reviews/2026-05-23-NEO-54.md @@ -19,7 +19,7 @@ NEO-54 delivers the planned split between **`IPlayerInventoryStore`** (in-memory | [`docs/plans/NEO-54-implementation-plan.md`](../plans/NEO-54-implementation-plan.md) | **Matches** — 24 bag + 1 equipment, split store/engine, reason codes, `V005`, Postgres/in-memory parity tests, DI via catalog extensions, docs/README updates; acceptance checklist complete. | | [`docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md`](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) | **Matches** — NEO-54 bullet under **Related implementation slices**; frozen roster/stack limits align with tests. | | [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E3.M3 row notes NEO-54 landed; NEO-55 HTTP still planned. | -| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Partially matches** — E3.M3 **In Progress**; footnote still lists NEO-50–NEO-53 only (see suggestion). | +| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E3.M3 **In Progress**; footnote lists NEO-50–NEO-54. | | [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — server-authoritative inventory mutations; no client-trusted placement in this slice. | | [`server/README.md`](../../server/README.md) | **Matches** — Player inventory store section: containers, reason codes, persistence note, NEO-55 deferral. | @@ -31,17 +31,17 @@ None. ## Suggestions -1. **AAA in Postgres deny test** — In `PlayerInventoryPersistenceIntegrationTests.TryAddStack_WhenDenied_ShouldNotPersistPartialState`, move `Assert.Equal(PlayerInventorySnapshot.BagSlotCount, OccupiedBagSlotCount(afterDeny!))` from the first **Act** block into **Assert** (or a dedicated assert phase after both act steps), per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert). +1. ~~**AAA in Postgres deny test** — In `PlayerInventoryPersistenceIntegrationTests.TryAddStack_WhenDenied_ShouldNotPersistPartialState`, move `Assert.Equal(PlayerInventorySnapshot.BagSlotCount, OccupiedBagSlotCount(afterDeny!))` from the first **Act** block into **Assert** (or a dedicated assert phase after both act steps), per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert).~~ **Done.** — removed in-Act assertion; slot count verified only in **Assert** via fresh factory read-back. -2. **`StoreMissing` coverage (optional)** — `PlayerInventoryMutationKind.StoreMissing` is documented on `PlayerInventoryMutationOutcome` but untested. A small operations test for an unknown `playerId` (in-memory store without that bucket) would lock the contract before NEO-55 HTTP maps it. +2. ~~**`StoreMissing` coverage (optional)** — `PlayerInventoryMutationKind.StoreMissing` is documented on `PlayerInventoryMutationOutcome` but untested. A small operations test for an unknown `playerId` (in-memory store without that bucket) would lock the contract before NEO-55 HTTP maps it.~~ **Done.** — `TryAddStack_ForUnknownPlayer_ShouldReturnStoreMissing` in `PlayerInventoryOperationsTests.cs`. -3. **Multi-slot add (optional)** — Add a test that places quantity **> `stackMax`** across **multiple new** empty slots (e.g. 41 × `field_stim_mk0` on empty bag → three occupied slots). Merge and bag-full paths are covered; this would explicitly guard the `FillEmptySlots` loop beyond single-slot adds. +3. ~~**Multi-slot add (optional)** — Add a test that places quantity **> `stackMax`** across **multiple new** empty slots (e.g. 41 × `field_stim_mk0` on empty bag → three occupied slots). Merge and bag-full paths are covered; this would explicitly guard the `FillEmptySlots` loop beyond single-slot adds.~~ **Done.** — `TryAddStack_WhenQuantityExceedsStackMax_ShouldSpanMultipleEmptySlots` asserts 20+20+1 across three slots. -4. **Dependency register footnote (optional)** — Extend the **E3.M3 note** in [`module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) to mention **NEO-54** alongside NEO-50–NEO-53 (same pattern as NEO-52/NEO-53 reviews). +4. ~~**Dependency register footnote (optional)** — Extend the **E3.M3 note** in [`module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) to mention **NEO-54** alongside NEO-50–NEO-53 (same pattern as NEO-52/NEO-53 reviews).~~ **Done.** ## Nits -- Nit: **`PlayerInventoryMutationOutcome.cs`** — enum and record parameters use **2-space** indent; neighboring outcome types (e.g. `SkillProgressionGrantApplyOutcome.cs`) use **4-space**. Format-only tidy. +- ~~Nit: **`PlayerInventoryMutationOutcome.cs`** — enum and record parameters use **2-space** indent; neighboring outcome types (e.g. `SkillProgressionGrantApplyOutcome.cs`) use **4-space**. Format-only tidy.~~ **Done.** - Nit: **`PostgresPlayerInventoryStore.TryReplaceSnapshot`** — delete-all + insert per mutation is correct for snapshot replace; concurrent writers are last-write-wins (same class of risk as other NEO-29-style stores). Fine for prototype; NEO-55 HTTP may want documenting or serializing per player if load tests show races. diff --git a/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryOperationsTests.cs b/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryOperationsTests.cs index 9e7994f..fbac0b9 100644 --- a/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryOperationsTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryOperationsTests.cs @@ -224,6 +224,53 @@ public sealed class PlayerInventoryOperationsTests Assert.Equal(TotalBagQuantity(before!), TotalBagQuantity(after!)); } + [Fact] + public void TryAddStack_WhenQuantityExceedsStackMax_ShouldSpanMultipleEmptySlots() + { + // Arrange + using var factory = new InMemoryWebApplicationFactory(); + var registry = factory.Services.GetRequiredService(); + var store = factory.Services.GetRequiredService(); + + // Act + var outcome = PlayerInventoryOperations.TryAddStack( + "dev-local-1", + "field_stim_mk0", + quantity: 41, + registry, + store); + + // Assert + Assert.Equal(PlayerInventoryMutationKind.Applied, outcome.Kind); + Assert.Equal(41, CountItem(outcome.Snapshot!, "field_stim_mk0")); + Assert.Equal(3, OccupiedBagSlotCount(outcome.Snapshot!)); + Assert.Equal(20, outcome.Snapshot!.BagSlots[0].Quantity); + Assert.Equal(20, outcome.Snapshot.BagSlots[1].Quantity); + Assert.Equal(1, outcome.Snapshot.BagSlots[2].Quantity); + } + + [Fact] + public void TryAddStack_ForUnknownPlayer_ShouldReturnStoreMissing() + { + // Arrange + using var factory = new InMemoryWebApplicationFactory(); + var registry = factory.Services.GetRequiredService(); + var store = factory.Services.GetRequiredService(); + + // Act + var outcome = PlayerInventoryOperations.TryAddStack( + "unknown-player-xyz", + "scrap_metal_bulk", + quantity: 1, + registry, + store); + + // Assert + Assert.Equal(PlayerInventoryMutationKind.StoreMissing, outcome.Kind); + Assert.Null(outcome.ReasonCode); + Assert.Null(outcome.Snapshot); + } + private static int CountItem(PlayerInventorySnapshot snapshot, string itemId) { var total = 0; diff --git a/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryPersistenceIntegrationTests.cs index 627f3c7..37fcc29 100644 --- a/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryPersistenceIntegrationTests.cs @@ -79,8 +79,6 @@ public sealed class PlayerInventoryPersistenceIntegrationTests(PostgresIntegrati quantity: 1, registry, store); - store.TryGetSnapshot("dev-local-1", out var afterDeny); - Assert.Equal(PlayerInventorySnapshot.BagSlotCount, OccupiedBagSlotCount(afterDeny!)); } // Act — verify on fresh host diff --git a/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs b/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs index c60b80d..edec38b 100644 --- a/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs +++ b/server/NeonSprawl.Server/Game/Items/PlayerInventoryMutationOutcome.cs @@ -3,19 +3,19 @@ namespace NeonSprawl.Server.Game.Items; /// Result of or (NEO-54). public enum PlayerInventoryMutationKind { - /// Validation or rules deny — inventory unchanged. - Denied, + /// Validation or rules deny — inventory unchanged. + Denied, - /// could not read or write the player bucket. - StoreMissing, + /// could not read or write the player bucket. + StoreMissing, - /// Mutation applied and persisted. - Applied, + /// Mutation applied and persisted. + Applied, } /// Populated when is . /// Authoritative snapshot after apply, or unchanged snapshot on deny; null when store missing. public readonly record struct PlayerInventoryMutationOutcome( - PlayerInventoryMutationKind Kind, - string? ReasonCode, - PlayerInventorySnapshot? Snapshot); + PlayerInventoryMutationKind Kind, + string? ReasonCode, + PlayerInventorySnapshot? Snapshot);