From 504d3228dd40ca63adc805975cbaaa2e14d51215 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 25 May 2026 13:17:47 -0400 Subject: [PATCH] NEO-85: Address code review doc sync and 404 test gap. Update module dependency register and E5.M1 related-slices entry for NEO-85 landed; add combat-targets HTTP 404 GdUnit case; strike review suggestions. --- client/test/combat_targets_client_test.gd | 15 +++++++++++++++ .../modules/E5_M1_CombatRulesEngine.md | 2 ++ .../modules/module_dependency_register.md | 2 +- docs/reviews/2026-05-25-NEO-85.md | 12 ++++++------ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/client/test/combat_targets_client_test.gd b/client/test/combat_targets_client_test.gd index 9964ffa..0c2476e 100644 --- a/client/test/combat_targets_client_test.gd +++ b/client/test/combat_targets_client_test.gd @@ -105,3 +105,18 @@ func test_invalid_schema_emits_sync_failed() -> void: # Assert assert_that(failed.size()).is_equal(1) assert_that(str(failed[0])).contains("schemaVersion") + + +func test_http_404_emits_sync_failed() -> void: + # Arrange + var transport := MockHttpTransport.new() + transport.response_code = 404 + transport.body_json = "" + var c := _make_client(transport) + var failed: Array = [] + c.connect("targets_sync_failed", func(reason: String) -> void: failed.append(reason)) + # Act + c.call("request_sync_from_server") + # Assert + assert_that(failed.size()).is_equal(1) + assert_that(str(failed[0])).is_equal("HTTP 404") diff --git a/docs/decomposition/modules/E5_M1_CombatRulesEngine.md b/docs/decomposition/modules/E5_M1_CombatRulesEngine.md index 3c37a46..2579823 100644 --- a/docs/decomposition/modules/E5_M1_CombatRulesEngine.md +++ b/docs/decomposition/modules/E5_M1_CombatRulesEngine.md @@ -64,6 +64,8 @@ See Epic 5 **Slice 1 — Combat rules MVP**: target lock, basic attacks, 4–6 a **E5M1-10 (NEO-44):** **`CombatDefeatGigXpGrant`** on cast accept when **`targetDefeated`** — **25** gig XP to prototype main gig **`breach`** via **`IPlayerGigProgressionStore`** (not E2.M2 skill XP); **`GET /game/players/{id}/gig-progression`** ([NEO-44 plan](../../plans/NEO-44-implementation-plan.md)); [server README — Gig progression snapshot (NEO-44)](../../../server/README.md#gig-progression-snapshot-neo-44); Bruno `bruno/neon-sprawl-server/gig-progression/`. +**E5M1-11 (NEO-85):** client combat HUD — **`ability_cast_client.gd`** parses nested **`combatResolution`** on accept; **`combat_targets_client.gd`** + **`CombatTargetHpLabel`**; **`CastFeedbackLabel`** damage/defeat copy; event-driven **`GET /game/world/combat-targets`** refresh on cast accept and lock change ([NEO-85 plan](../../plans/NEO-85-implementation-plan.md), [`NEO-85` manual QA](../../manual-qa/NEO-85.md)); [client README — Combat feedback + target HP HUD (NEO-85)](../../../client/README.md#combat-feedback--target-hp-hud-neo-85). + ## Linear backlog (decomposed) Full story tables, kickoff defaults, and **`blockedBy` graph:** [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md). diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 37f7677..acc5af7 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -76,7 +76,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen | E5.M3 | EncounterAndRewardTables | E5.M1, E3.M3, E7.M2 | EncounterDef, RewardTable, EncounterCompleteEvent | Prototype | Planned | | E5.M4 | GroupCombatScaling | E5.M3, E8.M1 | ScalingProfile, PartySizeModifier, CombatDifficultyBand | Pre-production | Planned | -**E5.M1 note:** Epic 5 **Slice 1** backlog in Linear ([Epic 5 — PvE Combat and Encounter Content](https://linear.app/neon-sprawl/project/epic-5-pve-combat-and-encounter-content-cf3c94c3-5346-40e0-8aaf-281e846f2846)): [NEO-76](https://linear.app/neon-sprawl/issue/NEO-76) → [NEO-86](https://linear.app/neon-sprawl/issue/NEO-86); label **`E5.M1`**. Gig XP on defeat: **E5M1-10** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md), [E5_M1_CombatRulesEngine.md](E5_M1_CombatRulesEngine.md). Builds on E1.M4 cast funnel (NEO-28/31/32); extends **`POST …/ability-cast`** with **`CombatResolution`**. **NEO-76 landed:** frozen four-ability catalog + CI ([NEO-76 plan](../../plans/NEO-76-implementation-plan.md)). **NEO-77 landed:** fail-fast server load of `content/abilities/*_abilities.json` ([NEO-77 plan](../../plans/NEO-77-implementation-plan.md)). **NEO-79 landed:** injectable **`IAbilityDefinitionRegistry`** + DI; hotbar/cast use **`TryNormalizeKnown`** ([NEO-79 plan](../../plans/NEO-79-implementation-plan.md)). **NEO-78 landed:** **`GET /game/world/ability-definitions`** — `AbilityDefinitionsWorldApi` + DTOs in `Game/Combat/` ([NEO-78 plan](../../plans/NEO-78-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/ability-definitions/`. **NEO-80 landed:** **`ICombatEntityHealthStore`** / **`InMemoryCombatEntityHealthStore`** — lazy prototype dummy HP at **`PrototypeCombatConstants.MaxPrototypeTargetHp` (100)**; DI via **`AddCombatEntityHealthStore()`** ([NEO-80 plan](../../plans/NEO-80-implementation-plan.md)); [server README — Combat entity health (NEO-80)](../../../server/README.md#combat-entity-health-neo-80). **NEO-81 landed:** **`CombatOperations.TryResolve`** + **`CombatResult`** + **`CombatReasonCodes`** — defeated pre-check deny vocabulary (`target_defeated`, `unknown_ability`, `unknown_target`) ([NEO-81 plan](../../plans/NEO-81-implementation-plan.md)); [server README — Combat engine (NEO-81)](../../../server/README.md#combat-engine-neo-81). **NEO-82 landed:** **`AbilityCastApi`** invokes **`CombatOperations.TryResolve`** after E1.M4 gates; nested wire **`combatResolution`** on accept; per-ability catalog **`cooldownSeconds`** on successful resolve; **`target_defeated`** JSON cast deny without cooldown ([NEO-82 plan](../../plans/NEO-82-implementation-plan.md)); [server README — Ability cast (NEO-82)](../../../server/README.md#ability-cast-neo-31-neo-82); Bruno `bruno/neon-sprawl-server/ability-cast/` defeat spine. **NEO-83 landed:** **`GET /game/world/combat-targets`** — `CombatTargetsWorldApi` + DTOs; authoritative HP snapshot from **`ICombatEntityHealthStore`** for client HUD ([NEO-83 plan](../../plans/NEO-83-implementation-plan.md)); [server README — Combat targets snapshot (NEO-83)](../../../server/README.md#combat-targets-snapshot-neo-83); Bruno `bruno/neon-sprawl-server/combat-targets/`. **NEO-44 landed:** **`CombatDefeatGigXpGrant`** on cast **`targetDefeated`** — **25** gig XP to **`breach`** via **`IPlayerGigProgressionStore`** (V007); **`GET …/gig-progression`** ([NEO-44 plan](../../plans/NEO-44-implementation-plan.md)); [server README — Gig progression (NEO-44)](../../../server/README.md#gig-progression-snapshot-neo-44). **NEO-84 landed:** comment-only **`ability_used`** / **`enemy_defeat`** telemetry hook sites in **`CombatOperations.TryResolve`**; reserved **`encounter_start`** / **`player_death`** ([NEO-84 plan](../../plans/NEO-84-implementation-plan.md)); [server README — Combat telemetry hooks (NEO-84)](../../../server/README.md#combat-telemetry-hooks-neo-84). Pending client: **E5M1-11** [NEO-85](https://linear.app/neon-sprawl/issue/NEO-85) → **E5M1-12** [NEO-86](https://linear.app/neon-sprawl/issue/NEO-86). +**E5.M1 note:** Epic 5 **Slice 1** backlog in Linear ([Epic 5 — PvE Combat and Encounter Content](https://linear.app/neon-sprawl/project/epic-5-pve-combat-and-encounter-content-cf3c94c3-5346-40e0-8aaf-281e846f2846)): [NEO-76](https://linear.app/neon-sprawl/issue/NEO-76) → [NEO-86](https://linear.app/neon-sprawl/issue/NEO-86); label **`E5.M1`**. Gig XP on defeat: **E5M1-10** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md), [E5_M1_CombatRulesEngine.md](E5_M1_CombatRulesEngine.md). Builds on E1.M4 cast funnel (NEO-28/31/32); extends **`POST …/ability-cast`** with **`CombatResolution`**. **NEO-76 landed:** frozen four-ability catalog + CI ([NEO-76 plan](../../plans/NEO-76-implementation-plan.md)). **NEO-77 landed:** fail-fast server load of `content/abilities/*_abilities.json` ([NEO-77 plan](../../plans/NEO-77-implementation-plan.md)). **NEO-79 landed:** injectable **`IAbilityDefinitionRegistry`** + DI; hotbar/cast use **`TryNormalizeKnown`** ([NEO-79 plan](../../plans/NEO-79-implementation-plan.md)). **NEO-78 landed:** **`GET /game/world/ability-definitions`** — `AbilityDefinitionsWorldApi` + DTOs in `Game/Combat/` ([NEO-78 plan](../../plans/NEO-78-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/ability-definitions/`. **NEO-80 landed:** **`ICombatEntityHealthStore`** / **`InMemoryCombatEntityHealthStore`** — lazy prototype dummy HP at **`PrototypeCombatConstants.MaxPrototypeTargetHp` (100)**; DI via **`AddCombatEntityHealthStore()`** ([NEO-80 plan](../../plans/NEO-80-implementation-plan.md)); [server README — Combat entity health (NEO-80)](../../../server/README.md#combat-entity-health-neo-80). **NEO-81 landed:** **`CombatOperations.TryResolve`** + **`CombatResult`** + **`CombatReasonCodes`** — defeated pre-check deny vocabulary (`target_defeated`, `unknown_ability`, `unknown_target`) ([NEO-81 plan](../../plans/NEO-81-implementation-plan.md)); [server README — Combat engine (NEO-81)](../../../server/README.md#combat-engine-neo-81). **NEO-82 landed:** **`AbilityCastApi`** invokes **`CombatOperations.TryResolve`** after E1.M4 gates; nested wire **`combatResolution`** on accept; per-ability catalog **`cooldownSeconds`** on successful resolve; **`target_defeated`** JSON cast deny without cooldown ([NEO-82 plan](../../plans/NEO-82-implementation-plan.md)); [server README — Ability cast (NEO-82)](../../../server/README.md#ability-cast-neo-31-neo-82); Bruno `bruno/neon-sprawl-server/ability-cast/` defeat spine. **NEO-83 landed:** **`GET /game/world/combat-targets`** — `CombatTargetsWorldApi` + DTOs; authoritative HP snapshot from **`ICombatEntityHealthStore`** for client HUD ([NEO-83 plan](../../plans/NEO-83-implementation-plan.md)); [server README — Combat targets snapshot (NEO-83)](../../../server/README.md#combat-targets-snapshot-neo-83); Bruno `bruno/neon-sprawl-server/combat-targets/`. **NEO-44 landed:** **`CombatDefeatGigXpGrant`** on cast **`targetDefeated`** — **25** gig XP to **`breach`** via **`IPlayerGigProgressionStore`** (V007); **`GET …/gig-progression`** ([NEO-44 plan](../../plans/NEO-44-implementation-plan.md)); [server README — Gig progression (NEO-44)](../../../server/README.md#gig-progression-snapshot-neo-44). **NEO-84 landed:** comment-only **`ability_used`** / **`enemy_defeat`** telemetry hook sites in **`CombatOperations.TryResolve`**; reserved **`encounter_start`** / **`player_death`** ([NEO-84 plan](../../plans/NEO-84-implementation-plan.md)); [server README — Combat telemetry hooks (NEO-84)](../../../server/README.md#combat-telemetry-hooks-neo-84). **NEO-85 landed:** client combat HUD — **`combat_targets_client.gd`**, **`CombatTargetHpLabel`**, **`CastFeedbackLabel`** resolution copy; event-driven **`GET /game/world/combat-targets`** refresh ([NEO-85 plan](../../plans/NEO-85-implementation-plan.md), [`NEO-85` manual QA](../../manual-qa/NEO-85.md)); [client README — Combat feedback + target HP HUD (NEO-85)](../../../client/README.md#combat-feedback--target-hp-hud-neo-85). Pending client capstone: **E5M1-12** [NEO-86](https://linear.app/neon-sprawl/issue/NEO-86). ### Epic 6 — PvP Security diff --git a/docs/reviews/2026-05-25-NEO-85.md b/docs/reviews/2026-05-25-NEO-85.md index 2885d53..250eb79 100644 --- a/docs/reviews/2026-05-25-NEO-85.md +++ b/docs/reviews/2026-05-25-NEO-85.md @@ -18,8 +18,8 @@ NEO-85 delivers **E5M1-11**: the Godot client parses nested **`combatResolution` |----------|--------| | [`docs/plans/NEO-85-implementation-plan.md`](../plans/NEO-85-implementation-plan.md) | **Matches** — kickoff decisions (event-driven refresh, extend `CastFeedbackLabel`, dedicated HP label), scope, acceptance checklist, and reconciliation align with code. | | [`docs/plans/E5M1-prototype-backlog.md`](../plans/E5M1-prototype-backlog.md) · **E5M1-11** | **Matches** — AC checked; landed note cites clients + manual QA. | -| [`docs/decomposition/modules/E5_M1_CombatRulesEngine.md`](../decomposition/modules/E5_M1_CombatRulesEngine.md) | **Partially matches** — Status line cites E5M1-11 NEO-85 landed; **Related implementation slices** lacks an **E5M1-11** entry (NEO-78–NEO-84 each have one). | -| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Conflicts** — E5.M1 note still ends with **Pending client: E5M1-11 NEO-85** despite landed implementation. | +| [`docs/decomposition/modules/E5_M1_CombatRulesEngine.md`](../decomposition/modules/E5_M1_CombatRulesEngine.md) | **Matches** — Status line cites E5M1-11 NEO-85 landed; **Related implementation slices** includes **E5M1-11** entry. Done. | +| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E5.M1 note cites **NEO-85 landed**; capstone **E5M1-12 NEO-86** pending. Done. | | [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E5.M1 row cites NEO-85 landed + manual QA + client README. | | [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — no client-side damage math; HP from server GET snapshot; cast feedback from wire fields only. | | [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) · **E1.M4** | **Partially matches** — NEO-85 extends **`ability_cast_client.gd`** / **`CastFeedbackLabel`** (E1.M4 cast funnel) but E1.M4 row stops at NEO-32; optional cross-reference only. | @@ -33,11 +33,11 @@ None. ## Suggestions -1. **Stale `module_dependency_register` E5.M1 note** — Replace **Pending client: E5M1-11 NEO-85** with a **NEO-85 landed** one-liner (mirror the E5.M1 alignment row: `combat_targets_client.gd`, `CombatTargetHpLabel`, event-driven GET, manual QA link). +1. ~~**Stale `module_dependency_register` E5.M1 note** — Replace **Pending client: E5M1-11 NEO-85** with a **NEO-85 landed** one-liner (mirror the E5.M1 alignment row: `combat_targets_client.gd`, `CombatTargetHpLabel`, event-driven GET, manual QA link).~~ **Done.** -2. **Missing E5M1-11 Related implementation slice** — Add an **E5M1-11 (NEO-85)** bullet under **Related implementation slices** in `E5_M1_CombatRulesEngine.md` (client combat HUD; link plan, manual QA, `client/README.md` combat section) so module readers stay consistent with E5M1-04–E5M1-10 entries. +2. ~~**Missing E5M1-11 Related implementation slice** — Add an **E5M1-11 (NEO-85)** bullet under **Related implementation slices** in `E5_M1_CombatRulesEngine.md` (client combat HUD; link plan, manual QA, `client/README.md` combat section) so module readers stay consistent with E5M1-04–E5M1-10 entries.~~ **Done.** -3. **HTTP error-path test gap** — Plan §Tests lists **404/invalid schema** failure for `combat_targets_client_test.gd`; implementation covers schema mismatch only. Add a mock **404** (or non-2xx) case asserting **`targets_sync_failed`** with **`HTTP 404`** for parity with other world GET clients. +3. ~~**HTTP error-path test gap** — Plan §Tests lists **404/invalid schema** failure for `combat_targets_client_test.gd`; implementation covers schema mismatch only. Add a mock **404** (or non-2xx) case asserting **`targets_sync_failed`** with **`HTTP 404`** for parity with other world GET clients.~~ **Done.** ## Nits @@ -57,6 +57,6 @@ godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreH -a res://test/combat_feedback_refresh_test.gd ``` -**GdUnit (local run):** 15/15 passed (8 + 4 + 3 suites). +**GdUnit (local run):** 16/16 passed (8 + 5 + 3 suites). **Manual:** [`docs/manual-qa/NEO-85.md`](../manual-qa/NEO-85.md) — server + Godot; Tab lock **`prototype_target_alpha`**, digit cast **`prototype_pulse`**, verify cast resolution copy and HP label stepping through defeat; optional **`POST /game/__dev/combat-targets-fixture`** between runs.