diff --git a/docs/plans/NEO-106-implementation-plan.md b/docs/plans/NEO-106-implementation-plan.md index 34d5389..eea27dc 100644 --- a/docs/plans/NEO-106-implementation-plan.md +++ b/docs/plans/NEO-106-implementation-plan.md @@ -54,7 +54,7 @@ - **Wiring:** `EncounterCombatWiring.TryProcessCastOutcome` — activate on `damageDealt > 0`, mark on `targetDefeated`, `TryCompleteAndGrant` when all required NPCs defeated. - **Integration:** `AbilityCastApi` injects encounter/inventory deps; call order preserves NEO-44 gig XP before encounter completion. -- **Tests:** `EncounterCombatWiringTests` (6); `AbilityCastApiTests` — defeat-all-three loot once, two-defeat no loot, 75 gig XP after three defeats. +- **Tests:** `EncounterCombatWiringTests` (8); `AbilityCastApiTests` — defeat-all-three loot once, two-defeat no loot, 75 gig XP after three defeats. - **Docs:** `server/README.md`; E5.M3 module snapshot + alignment register + backlog updated. ## Technical approach @@ -127,7 +127,7 @@ | Test file | What it covers | |-----------|----------------| -| `EncounterCombatWiringTests.cs` | **Arrange:** `InMemoryWebApplicationFactory` services. **Act/Assert:** damaging hit activates encounter; defeat marks progress; third defeat invokes completion (inventory + `IsCompleted`); non-encounter NPC no-op; already-completed encounter skips activate/mark; completion not called when only 2/3 defeated. | +| `EncounterCombatWiringTests.cs` | **Arrange:** `InMemoryWebApplicationFactory` services. **Act/Assert:** damaging hit activates encounter; defeat marks progress; third defeat invokes completion (inventory + `IsCompleted`); non-encounter NPC no-op; already-completed encounter skips activate/mark; completion not called when only 2/3 defeated (non-lethal third hit); bag-full third defeat retains all-defeated progress without completion mark. | | `AbilityCastApiTests.cs` | **Arrange:** HTTP client + pulse bind + per-NPC lock/defeat helpers. **Act/Assert:** defeat all three via cast → `scrap_metal_bulk` ×10 + `contract_handoff_token` ×1 once; two defeats → zero loot; gig XP 75 on breach after three defeats, skill XP unchanged; existing NEO-44 single-defeat test still passes. | ## Open questions / risks diff --git a/docs/reviews/2026-05-31-NEO-106.md b/docs/reviews/2026-05-31-NEO-106.md index 4ed4a7b..ff72304 100644 --- a/docs/reviews/2026-05-31-NEO-106.md +++ b/docs/reviews/2026-05-31-NEO-106.md @@ -22,7 +22,7 @@ NEO-106 adds **`EncounterCombatWiring.TryProcessCastOutcome`**, a static orchest | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E5.M3 row notes **NEO-106** combat wiring; NEO-108+ HTTP still outstanding. | | `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E5.M3 note cites **NEO-106** wiring landed. | | Full-stack epic decomposition | **N/A** — plan explicitly defers client counterpart to **NEO-110** via NEO-108; no prototype slice completion claim. | -| `server/README.md` | **Partially matches** — new **Encounter combat wiring (NEO-106)** section is accurate; encounter progress section still has one stale “land in NEO-106 / NEO-108” line (see Nits). | +| `server/README.md` | **Matches** — new **Encounter combat wiring (NEO-106)** section is accurate; encounter progress section defers HTTP read to NEO-108 only. | ## Blocking issues @@ -30,29 +30,29 @@ None. ## Suggestions -1. **Unit test for 2/3 defeated** — Plan test table lists “completion not called when only 2/3 defeated.” Integration test **`PostAbilityCast_ShouldNotGrantEncounterLoot_WhenOnlyTwoNpcDefeated`** covers this at HTTP level; a wiring-level AAA test (two **`DefeatNpc`** calls, third **`TryProcessCastOutcome`** with `targetDefeated: false`) would mirror the plan table and isolate completion gating without HTTP. +1. ~~**Unit test for 2/3 defeated** — Plan test table lists “completion not called when only 2/3 defeated.” Integration test **`PostAbilityCast_ShouldNotGrantEncounterLoot_WhenOnlyTwoNpcDefeated`** covers this at HTTP level; a wiring-level AAA test (two **`DefeatNpc`** calls, third **`TryProcessCastOutcome`** with `targetDefeated: false`) would mirror the plan table and isolate completion gating without HTTP.~~ **Done.** `TryProcessCastOutcome_ShouldNotComplete_WhenOnlyTwoTargetsDefeatedAndThirdHitNonLethal`. -2. **Inventory-full on third defeat (wiring path)** — README and plan document fail-closed retry semantics; NEO-105 ops tests cover grant deny. Consider a wiring or integration test that fills the bag before the third lethal cast and asserts progress all-defeated but **`IsCompleted`** false and zero loot delta — exercises the documented retry path end-to-end. +2. ~~**Inventory-full on third defeat (wiring path)** — README and plan document fail-closed retry semantics; NEO-105 ops tests cover grant deny. Consider a wiring or integration test that fills the bag before the third lethal cast and asserts progress all-defeated but **`IsCompleted`** false and zero loot delta — exercises the documented retry path end-to-end.~~ **Done.** `TryProcessCastOutcome_ShouldRetainProgressWithoutCompletion_WhenBagFullOnThirdDefeat`. -3. **`encounter_complete` success hook comment** — **`EncounterCompletionOperations`** has NEO-109 deny hook + E7.M2 success TODO; **`EncounterCombatWiring`** logs completion deny at Debug. A brief `// TODO(E9.M1): catalog emit — encounter_complete` on the success branch (or in **`TryCompleteAndGrant`**) would align with NEO-109 scope without runtime code. +3. ~~**`encounter_complete` success hook comment** — **`EncounterCompletionOperations`** has NEO-109 deny hook + E7.M2 success TODO; **`EncounterCombatWiring`** logs completion deny at Debug. A brief `// TODO(E9.M1): catalog emit — encounter_complete` on the success branch (or in **`TryCompleteAndGrant`**) would align with NEO-109 scope without runtime code.~~ **Done.** Hook comments on **`EncounterCombatWiring`** success path and **`EncounterCompletionOperations.TryCompleteAndGrant`**. ## Nits -- Nit: **`server/README.md`** encounter progress section (line ~154) still reads “Combat cast wiring … land in NEO-106 / NEO-108” — combat wiring is landed; reword to “HTTP progress read lands in NEO-108” (the next line already links the NEO-106 section). +- ~~Nit: **`server/README.md`** encounter progress section (line ~154) still reads “Combat cast wiring … land in NEO-106 / NEO-108” — combat wiring is landed; reword to “HTTP progress read lands in NEO-108” (the next line already links the NEO-106 section).~~ **Done.** -- Nit: **`PostAbilityCast_ShouldGrantEncounterLootOnce_WhenAllThreeNpcDefeated`** reads **`inventoryAfter`** and resolves **`completionStore`** after the defeat loop (still under **Act**). Moving those reads into **Assert** would tighten AAA; behavior is unchanged. +- ~~Nit: **`PostAbilityCast_ShouldGrantEncounterLootOnce_WhenAllThreeNpcDefeated`** reads **`inventoryAfter`** and resolves **`completionStore`** after the defeat loop (still under **Act**). Moving those reads into **Assert** would tighten AAA; behavior is unchanged.~~ **Done.** Inventory GET and completion store resolve moved to **Assert** in both encounter-loot integration tests. ## Verification ```bash -# NEO-106-focused (9 tests — passed locally) +# NEO-106-focused (11 tests — passed locally) dotnet test NeonSprawl.sln --filter "FullyQualifiedName~EncounterCombatWiring|FullyQualifiedName~PostAbilityCast_ShouldGrantEncounterLoot|FullyQualifiedName~PostAbilityCast_ShouldNotGrantEncounterLoot|FullyQualifiedName~PostAbilityCast_ShouldGrantGigXpPerDefeat" -# Broader encounter slice (69 tests — passed locally) +# Broader encounter slice (71 tests — passed locally) dotnet test NeonSprawl.sln --filter "FullyQualifiedName~Encounters" # CI parity python3 scripts/validate_content.py ``` -**Local run:** 9/9 NEO-106-focused tests passed; 69/69 `Encounters` filter tests passed. +**Local run:** 11/11 NEO-106-focused tests passed; 71/71 `Encounters` filter tests passed. diff --git a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs index 5db6470..5cfb71a 100644 --- a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs @@ -880,11 +880,11 @@ public sealed class AbilityCastApiTests client, factory, PrototypeNpcRegistry.PrototypeNpcEliteId); + + // Assert var inventoryAfter = await client.GetFromJsonAsync( "/game/players/dev-local-1/inventory"); var completionStore = factory.Services.GetRequiredService(); - - // Assert Assert.NotNull(inventoryBefore); Assert.NotNull(inventoryAfter); Assert.Equal( @@ -915,11 +915,11 @@ public sealed class AbilityCastApiTests client, factory, PrototypeNpcRegistry.PrototypeNpcRangedId); + + // Assert var inventoryAfter = await client.GetFromJsonAsync( "/game/players/dev-local-1/inventory"); var completionStore = factory.Services.GetRequiredService(); - - // Assert Assert.NotNull(inventoryBefore); Assert.NotNull(inventoryAfter); Assert.Equal( diff --git a/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCombatWiringTests.cs b/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCombatWiringTests.cs index 825a5f7..16fb3bb 100644 --- a/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCombatWiringTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Encounters/EncounterCombatWiringTests.cs @@ -167,6 +167,81 @@ public sealed class EncounterCombatWiringTests Assert.Equal(CountBagItem(before!, "contract_handoff_token"), CountBagItem(after!, "contract_handoff_token")); } + [Fact] + public async Task TryProcessCastOutcome_ShouldNotComplete_WhenOnlyTwoTargetsDefeatedAndThirdHitNonLethal() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + DefeatNpc(deps, MeleeNpc); + DefeatNpc(deps, RangedNpc); + + // Act + EncounterCombatWiring.TryProcessCastOutcome( + PlayerId, + EliteNpc, + damageDealt: 25, + targetDefeated: false, + deps.EncounterRegistry, + deps.RewardTableRegistry, + deps.ProgressStore, + deps.CompletionStore, + deps.ItemRegistry, + deps.InventoryStore, + TimeProvider.System); + + // Assert + Assert.False( + EncounterProgressOperations.IsAllRequiredTargetsDefeated( + PlayerId, + EncounterId, + deps.EncounterRegistry, + deps.ProgressStore)); + Assert.False(deps.CompletionStore.IsCompleted(PlayerId, EncounterId)); + deps.InventoryStore.TryGetSnapshot(PlayerId, out var inventory); + Assert.Equal(0, CountBagItem(inventory!, "scrap_metal_bulk")); + Assert.Equal(0, CountBagItem(inventory!, "contract_handoff_token")); + } + + [Fact] + public async Task TryProcessCastOutcome_ShouldRetainProgressWithoutCompletion_WhenBagFullOnThirdDefeat() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + DefeatNpc(deps, MeleeNpc); + DefeatNpc(deps, RangedNpc); + FillBag(deps); + deps.InventoryStore.TryGetSnapshot(PlayerId, out var beforeInventory); + + // Act + EncounterCombatWiring.TryProcessCastOutcome( + PlayerId, + EliteNpc, + damageDealt: 25, + targetDefeated: true, + deps.EncounterRegistry, + deps.RewardTableRegistry, + deps.ProgressStore, + deps.CompletionStore, + deps.ItemRegistry, + deps.InventoryStore, + TimeProvider.System); + + // Assert + Assert.True( + EncounterProgressOperations.IsAllRequiredTargetsDefeated( + PlayerId, + EncounterId, + deps.EncounterRegistry, + deps.ProgressStore)); + Assert.False(deps.CompletionStore.IsCompleted(PlayerId, EncounterId)); + deps.InventoryStore.TryGetSnapshot(PlayerId, out var afterInventory); + Assert.Equal(TotalBagQuantity(beforeInventory!), TotalBagQuantity(afterInventory!)); + Assert.Equal(0, CountBagItem(afterInventory!, "scrap_metal_bulk")); + Assert.Equal(0, CountBagItem(afterInventory!, "contract_handoff_token")); + } + [Fact] public async Task TryProcessCastOutcome_ShouldNotActivate_WhenZeroDamageWithoutDefeat() { @@ -215,6 +290,34 @@ public sealed class EncounterCombatWiringTests DefeatNpc(deps, EliteNpc); } + private static void FillBag(EncounterCombatWiringTestDependencies deps) + { + for (var i = 0; i < PlayerInventorySnapshot.BagSlotCount; i++) + { + var add = PlayerInventoryOperations.TryAddStack( + PlayerId, + "survey_drone_kit", + quantity: 1, + deps.ItemRegistry, + deps.InventoryStore); + Assert.Equal(PlayerInventoryMutationKind.Applied, add.Kind); + } + } + + private static int TotalBagQuantity(PlayerInventorySnapshot snapshot) + { + var total = 0; + foreach (var slot in snapshot.BagSlots) + { + if (!slot.IsEmpty) + { + total += slot.Quantity; + } + } + + return total; + } + private static EncounterCombatWiringTestDependencies ResolveDependencies(InMemoryWebApplicationFactory factory) { var services = factory.Services; diff --git a/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs b/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs index 5579776..e9575ac 100644 --- a/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs +++ b/server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs @@ -86,6 +86,10 @@ public static class EncounterCombatWiring playerId, encounterId, completion.ReasonCode); + return; } + + // --- Telemetry hook site (NEO-109): future E9.M1 catalog event `encounter_complete` --- + // TODO(E9.M1): catalog emit — once per player+encounter when grants commit (NEO-106 cast path). } } diff --git a/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs b/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs index 692b085..01ed842 100644 --- a/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs +++ b/server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs @@ -108,6 +108,7 @@ public static class EncounterCompletionOperations } // TODO(E7.M2): quest-credit router consumes EncounterCompleteEvent (NEO-107 expands persistence + hook). + // TODO(E9.M1): catalog emit — encounter_complete on successful grant commit (NEO-109). var normalizedPlayerId = EncounterProgressIds.NormalizePlayerId(playerId); var completeEvent = new EncounterCompleteEvent( diff --git a/server/README.md b/server/README.md index db739e3..4a05c76 100644 --- a/server/README.md +++ b/server/README.md @@ -151,7 +151,7 @@ Per-player encounter state is server-owned in **`IEncounterProgressStore`** (act - **`TryMarkTargetDefeated(…)`** — requires prior activation; accumulates defeats in any order; idempotent per NPC. - **`IsAllRequiredTargetsDefeated(playerId, encounterId, …)`** — set equality vs catalog **`requiredNpcInstanceIds`**. -Combat cast wiring (**`AbilityCastApi`**) and **`GET /game/players/{id}/encounter-progress`** land in NEO-106 / NEO-108. Plan: [NEO-104 implementation plan](../../docs/plans/NEO-104-implementation-plan.md). +**`GET /game/players/{id}/encounter-progress`** lands in NEO-108. Plan: [NEO-104 implementation plan](../../docs/plans/NEO-104-implementation-plan.md). **Combat wiring (NEO-106):** see [Encounter combat wiring (NEO-106)](#encounter-combat-wiring-neo-106) — **`EncounterCombatWiring`** on **`POST …/ability-cast`** accept path.