~bigbes/game-prototype-ftl

a1f0ec5512bf44f4623fd022550b285c1980ce14 — Eugene Blikh 30 days ago d77efb6
docs(combat): record review conclusion
1 files changed, 27 insertions(+), 4 deletions(-)

M docs/tasks/combat.md
M docs/tasks/combat.md => docs/tasks/combat.md +27 -4
@@ 232,10 232,33 @@ Notes:

## Conclusion

### Deviations from plan
Outcome: combat encounter implemented and integrated; reviewer cleared the diff with no Critical or Important findings (HEAD `d77efb6`).

Invariants:
- IV1 — `combat.go` and `ship.go` import only `math/rand`; no Ebitengine references.
- IV2 — only `updateCombat` and `resolveIncoming` mutate `Combat` / `CombatSide` fields; `main.go` reset uses whole-struct replacement via `NewCombat()`.
- IV3 — `resolveIncoming` guards `Hull > 0` before decrement; hull cannot underflow.
- IV4 — shield layers clamped before the regen loop; power-drop test confirms layers fall in one tick.
- IV5 — `Result != GameOngoing` early-return freezes state; tie-tick resolves to `GameDefeat` per `if/else if` ordering.
- IV6 — `updateCombat` takes `*rand.Rand` as the only randomness source; `TestUpdateCombat_determinism` covers it.
- IV7 — `GOOS=js GOARCH=wasm go build .` clean.
- IV8 — `HudY/HudX/HudColW/HudColCount` defined only in `hud.go`; render and main read them.

### Assumptions check

- AS1 — held (deferred). `inpututil.IsKeyJustPressed(ebiten.KeyR)` is the same package as the mouse path that already works in WASM; user-side browser smoke confirms.
- AS2 — held. `math/rand` builds and vets clean for `GOOS=js GOARCH=wasm`.
- AS3 — held (deferred). 60 TPS smoothness is judged at user-side browser smoke; constants are tunable in `combat.go`.

- PH3 — `drawShieldsIndicator` and `drawEvasionReadout` each gained an `s Ship` parameter (plan signatures took only `c Combat, sys []System` and `sys []System`). Why: the first implementation called `NewPlayerShip()` per frame inside the renderer to locate the Shields/Engines room, allocating a 5-room slice each draw and hiding the coupling between layout and renderer. Fix threads `g.ship` through (commit `9eb78ec`). Aligns with GPC1 (explicit interfaces) and the plan bullet's intent ("call sites use `s.Rooms` indexing by role").
### Unknowns outcome

- UK1 — still-open. Game-feel of hulls 20/15, weapon 8/charge, shields 3 s regen, enemy 6 s fire, evasion 5%/level needs the user-side browser playthrough at multiple power splits. All tunables are `const` in `combat.go` (PC4) — retuning is one diff.
- UK2 — still-open. Legibility of shield-layer dots and evasion readout at 640×360 needs the user-side browser smoke. Fallback: text "MISS"/"BLOCK" toast.

### Deviations from plan

### Known risks
- PH3 — `drawShieldsIndicator` and `drawEvasionReadout` each gained an `s Ship` parameter (plan signatures took only `c Combat, sys []System` and `sys []System`). Why: the first implementation called `NewPlayerShip()` per frame inside the renderer to locate the Shields/Engines room, allocating a 5-room slice each draw and hiding the coupling between layout and renderer. Fix threads `g.ship` through (commit `9eb78ec`). Aligns with GPC1 and the plan bullet's intent ("call sites use `s.Rooms` indexing by role").

- Tests cannot be executed in this Claude Code session: Ebitengine's GLFW init panics on `currentMouseLocation()` when no Window Server is attached. Native build, WASM build, and `go vet ./...` are clean. PH1 + PH2 unit tests must be run by the user in a real terminal at verify time (see `## Verify`).
Verified by:
- `go test ./...` deferred — Ebitengine GLFW `init()` panics on `currentMouseLocation()` in this session (no Window Server). User: run in a regular terminal to confirm 51/51 (32 prior + 17 combat + 2 ship).
- Browser smoke deferred — `make play-web` and play through several encounters at different power splits to settle UK1 + UK2 and confirm AS1 + AS3.