fix(@projects/@magic-civilization): 🐛 add unit_id to unit killed events
Co-Authored-By: Lilith Autocommit <noreply@atlilith.com>
This commit is contained in:
parent
e183765464
commit
d492a0b97c
1 changed files with 89 additions and 0 deletions
|
|
@ -1233,3 +1233,92 @@ closed and the policy is now exercising those branches.
|
|||
- Combat overflow at `mc-turn/src/processor.rs:2425` did NOT reproduce
|
||||
in this 250-turn run. The harness PvP path apparently exercises a
|
||||
different code path than the bench. Tracked under p2-67-followup.
|
||||
|
||||
## 2026-05-12 — Real-game analysis follow-up: Bug 3 (attacks don't kill)
|
||||
|
||||
### RC
|
||||
|
||||
Long-game recap (`.local/demo-runs/2026-05-12-claude-vs-easy-ai-250-turn/recap.md`)
|
||||
showed `Units killed (UnitDestroyed events): 0` over 237 turns despite 5
|
||||
attack actions and 951 fortify actions. Two compounding bugs:
|
||||
|
||||
- **Bug A — `mc-turn`:** `resolve_single_pvp_attack`'s `Killed` /
|
||||
`Destroyed` / `!attacker_survived` branches (`processor.rs:2074-2126`)
|
||||
called `swap_remove` on the dead unit but never pushed a
|
||||
`TurnEvent::UnitKilled`. The discovery-loop emit at
|
||||
`processor.rs:2702` only fires for proximity-discovered combats —
|
||||
queued `AttackRequest`s (the GDScript click path + `apply_action`
|
||||
dispatch path) were silent.
|
||||
- **Bug B — `mc-player-api`:** `translate_processor_events`
|
||||
(`dispatch.rs:411`) explicitly enumerated `TurnEvent::UnitKilled` in
|
||||
the drop-set, so even when the chronicle DID carry a kill, the wire
|
||||
layer threw it away. `Event::UnitDestroyed` had a struct definition
|
||||
but no producer.
|
||||
|
||||
### Fix
|
||||
|
||||
- Added `unit_id: u32` field to `TurnEvent::UnitKilled` (`#[serde(default)]`
|
||||
for legacy fixture compat).
|
||||
- New `UnitKilledEvent` struct in `mc-turn::combat_event` + new
|
||||
`units_killed: Vec<UnitKilledEvent>` field on `PendingCaptureEvents`.
|
||||
`drain_into` translates each entry to a `TurnEvent::UnitKilled` push.
|
||||
- Each `swap_remove` call site in `resolve_single_pvp_attack` (3 sites:
|
||||
`Killed`, `Destroyed`, `!attacker_survived`) now stages a
|
||||
`UnitKilledEvent` before mutating the unit vec.
|
||||
- Discovery-loop emit at `processor.rs:2702` populates `unit_id`.
|
||||
- `dispatch.rs::translate_processor_events` now translates
|
||||
`TurnEvent::UnitKilled { unit_id, .. }` → `Event::UnitDestroyed { unit_id: format!("u_{unit_id}"), killer_unit_id: None }`. `killer_unit_id` correlation deferred (chronicle carries clan-id, not attacker unit id).
|
||||
- `api-gdext::replay::dict_for_event` includes the new `unit_id` field
|
||||
in the GDScript-visible dictionary. Also wired
|
||||
`CityBuildingCompleted` translation that was previously latent.
|
||||
|
||||
### Evidence
|
||||
|
||||
- `crates/mc-turn/tests/queued_pvp_unit_killed.rs` (new) — minimal
|
||||
2-player fixture, single queued `AttackRequest` against a 1-HP
|
||||
defender, asserts exactly one `TurnEvent::UnitKilled { defender =
|
||||
ClanId(1), unit_id = 200, attacker = ClanId(0) }` lands in
|
||||
`result.events_emitted`. Passes.
|
||||
- `crates/mc-player-api/tests/queued_attack_unit_destroyed.rs` (new)
|
||||
— same fixture via `apply_action(Attack)` + `apply_action(EndTurn)`,
|
||||
asserts exactly one `Event::UnitDestroyed { unit_id: "u_200", .. }`
|
||||
comes back from dispatch. Passes.
|
||||
- `long_game_transcript` re-run (250-turn ceiling, terminated at turn
|
||||
236/237 for "10 consecutive EndTurn-only" stuck-guard):
|
||||
- **before**: `Units killed (UnitDestroyed events): 0`
|
||||
- **after**: `Units killed (UnitDestroyed events): 1998`
|
||||
|
||||
### Files touched
|
||||
|
||||
- `src/simulator/crates/mc-replay/src/event.rs` — added `unit_id` to
|
||||
`TurnEvent::UnitKilled`.
|
||||
- `src/simulator/crates/mc-replay/tests/award_computation.rs` — fixture
|
||||
updates.
|
||||
- `src/simulator/crates/mc-replay/src/awards.rs` — unchanged (uses
|
||||
`attacker` field only; `..` rest covers the new field).
|
||||
- `src/simulator/crates/mc-turn/src/combat_event.rs` — new
|
||||
`UnitKilledEvent` struct.
|
||||
- `src/simulator/crates/mc-turn/src/game_state.rs` — extended
|
||||
`PendingCaptureEvents` (new vec + drain + is_empty).
|
||||
- `src/simulator/crates/mc-turn/src/processor.rs` — staged event in 3
|
||||
swap-remove sites; populated `unit_id` in discovery-loop emit;
|
||||
imported `UnitKilledEvent`.
|
||||
- `src/simulator/crates/mc-player-api/src/dispatch.rs` — removed
|
||||
`UnitKilled` from drop-set; added translation arm.
|
||||
- `src/simulator/api-gdext/src/replay.rs` — `unit_id` field in
|
||||
`UnitKilled` dict; new `CityBuildingCompleted` arm.
|
||||
|
||||
### Residual / deferred
|
||||
|
||||
- `killer_unit_id` on `Event::UnitDestroyed` stays `None` — the
|
||||
chronicle variant carries only the killing clan-id, not the specific
|
||||
attacker unit id. Wiring that requires a new field on
|
||||
`TurnEvent::UnitKilled` (`killer_unit_id: u32`) plus threading it
|
||||
through `resolve_single_pvp_attack` and the discovery loop. Tracked
|
||||
for a future follow-up; not blocking dispatch-event correctness.
|
||||
- Destroy-posture civilian kills now emit BOTH
|
||||
`TurnEvent::CivilianDestroyed` AND `TurnEvent::UnitKilled`. The wire
|
||||
layer drops `CivilianDestroyed` and surfaces `UnitDestroyed` via
|
||||
`UnitKilled`, so net wire behaviour is correct. AI memory /
|
||||
chronicle subscribers that consume `CivilianDestroyed` separately
|
||||
are unaffected.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue