docs/ENGINE_FOLLOWUPS_TRIAGE.md

Engine Follow-ups — Triage against OpenRA-Rust/main HEAD 8ca8989 (post-merge)

Cross-checks the seven findings in docs/ENGINE_FOLLOWUPS.md against OpenRA-Rust/main HEAD 8ca8989 (the engine-feature-wave merge, 2026-05-23). The previous version of this doc (HEAD feb8981) classified five findings as "unreproducible — feature lives only on engine-feature-wave"; that branch is now merged so each finding has a real status against the current main.

Executive summary

#	Finding	Post-merge status	Action
1	Tick-rate doc drift	DOC-ONLY (engine constant is 30/step)	FIXED — OpenRA-Bench/CLAUDE.md updated
2	Ore density clamps at 12/cell	BROKEN (clamp still at 12)	DOC-FIXED — schema.py::ore_patches docstring covers the recipe
3	Nuke AoE kills not credited to kills_per_player	FIXED (engine fix shipped this run)	Engine fix + 2 Rust tests landed
4	Explicit harvest(unit_id, x, y) drifts off target	BROKEN (harv re-binds to nearest patch after first deposit)	Failing #[ignore]d test pinned; TODO engine fix
5	ReturnFire stance breaks kite-and-pull	PARTIALLY FIXED (stance:1 fix landed); residual is balance/empirical	TODO empirical re-measure of combat-kite-and-pull
6	Arena generator's default 4-corner mpspawns	DOC-ONLY (auto-MCV varies per seed; engine doesn't offset pre-placed actors)	FIXED — ENGINE_FOLLOWUPS.md rewording
7	Pre-placed proc overlapping ore_patches:	BROKEN (patch is seeded, but proc footprint blocks pathing — silent sterilisation)	Failing #[ignore]d test pinned; TODO validator

Wheel rebuilt cleanly: Installed openra_train-0.1.0 on maturin develop --release against HEAD 8ca8989. openra-sim lib

• integration tests are green except for known pre-existing fixture/environment failures (see end of doc).

---

Finding 1 — Tick-rate discrepancy (doc drift)

Status on main: DOC-ONLY (engine constant is 30/step, not 90).

openra-train/src/env.rs:33 declares pub const DEFAULT_TICKS_PER_STEP: u32 = 30; and Env::step (env.rs:351-364) advances exactly that many ticks per step(). The bench's RustEnvHandle.__init__ does not call with_ticks_per_step so every env.step advances 30 ticks. Under step_until_event (interrupt mode), the loop runs UP TO max_ticks (default 5 in bench code) and breaks on the first signal — per-turn advance is variable in [1, 5].

Action taken: OpenRA-Bench/CLAUDE.md updated in two spots (defective-scenario list and engine-facts list). The "~90 ticks/turn" phrase is gone; replaced with the actual DEFAULT_TICKS_PER_STEP constant + a note to read info["ticks_advanced"] for interrupt-mode runs.

---

Finding 2 — Ore density clamps at 12 cells

Status on main: BROKEN, but documented.

Clamp lives at openra-sim/src/resource.rs:94-95:

let per_cell = ((patch.amount.max(1) + passable_cells - 1) / passable_cells)
    .clamp(1, 12) as u8;

Ceiling is 12 ore/cell; raising the cap is a real (though small) balance change. The cheapest mitigation is the docstring (option (c) in the spec) — already applied to schema.py.

Action taken: openra_bench/scenarios/schema.py::MapDef.ore_patches docstring now reproduces the clamp formula and the recommended ratio amount ≈ 12 · π · radius² · 1.2, plus the 50 cr/ore conversion. New econ packs that use ore_patches: get the recipe inline.

Optional follow-up (NOT in this commit): raise the cap to 24/cell in resource.rs:95 and update the seed_patch_fills_disk density assertion. Out of scope for triage; documented for whoever wants to pick it up.

Bench-side workarounds in place: econ-mine-and-grow.yaml, econ-multi-patch-allocation.yaml, econ-second-base-race.yaml, mcv-deploy-second-base.yaml already inflate amount to fill the cap; the new docstring formalises that recipe so future packs don't re-discover the footgun.

---

Finding 3 — Nuke AoE kills not credited to kills_per_player

Status on main: FIXED in this triage pass.

Pre-fix world.rs::detonate_nuke (lines 2101-2141) removed dead actors via self.actors.remove(&id) without ever incrementing kills_per_player. The _owner parameter was even prefix-_'d to silence the unused-variable warning, telegraphing the bug.

Engine fix shipped: world.rs::detonate_nuke now uses owner and credits kills_per_player once per dead actor whose owner is NOT the firing owner (mirrors the projectile-resolve scoping — friendly-fire kills do NOT credit, so an agent can't game units_killed_gte by nuking its own units).

Pinning: OpenRA-Rust/openra-sim/tests/test_nuke_credits_kills.rs — two tests:

• nuke_credits_each_enemy_kill_to_firing_owner — fire mslo at a cluster of 5 enemy e1s, assert kills_for_player(owner) rises by exactly 5.
• nuke_does_not_credit_friendly_fire_kills — fire mslo at agent's own e1 cluster, assert no credit.

Existing test still green: test_superweapons.rs::mslo_nuke_kills_enemy_cluster runs unchanged (the new credit logic is additive).

Bench-side cleanup queued (TODO, NOT this commit):

• OpenRA-Bench/openra_bench/scenarios/packs/spec-nuke-strike.yaml currently uses enemy_buildings_destroyed_gte with a silo-cluster workaround (lines ~62-70 by triage finding #3). With the fix in place, the hard-tier predicate can switch to units_killed_gte: 5 against an infantry cluster — restoring the original "nuke clears infantry cluster" semantics.

---

Finding 4 — Explicit harvest(unit_id, x, y) orders drift off target

Status on main: BROKEN.

world.rs::order_harvest (lines 1372-1402) is idempotent and stores last_harvest_cell: Some(target) — so the FIRST FindingOre cycle honours the explicit target. But tick_harvesters (line 2261) calls terrain.find_nearest_resource(center, 15) with center = last_harvest_cell.or(loc), and last_harvest_cell is overwritten each successful harvest tick (line 2328) to the actually-harvested cell. After the first deposit, the harv's "explicit target" has already drifted to wherever it last mined; subsequent FindingOre cycles search around that point and pick up whatever ore is closest, without remembering which patch the original harvest() order selected. With multiple patches inside a 15-cell window, the explicit allocation evaporates within ~30 ticks.

Pinning: openra-sim/tests/test_harvest_explicit_target.rs::explicit_harvest_target_stays_bound_to_far_patch — #[ignore]d. Sets up a near patch (10,15) and far patch (40,15), issues an explicit Harvest order targeting the FAR patch, runs 3000 ticks, asserts every harvested cell stays within Chebyshev distance ≤ 4 of the far patch. Currently the harv would re-bind to whichever patch is closer to its mining position. Flip to #[test] after the fix lands.

TODO engine fix: add bound_patch: Option<(x, y, radius)> to Activity::Harvest, set by order_harvest from the explicit target (radius derived from the registered OrePatchDef containing the cell, default 3). FindingOre's resource search restricts to cells inside the patch disc when bound_patch is set. Cleared by a new Harvest order to a different target or by stop. Multi-step change (touches Activity::Harvest shape, order_harvest, tick_harvesters, all #[derive]'d serialization tests) — not a one-liner; pinned-test

• TODO is the right form.

Bench-side workarounds: econ-multi-patch-allocation.yaml:65-87, 212-216 pre-stages one harv beside each patch and relies on the auto-route binding each to the nearest proc — the workaround dodges the explicit-allocation discipline by hand-placing harvs at the target. Once the engine fix lands, the pack docstring and pre-staged positions can be removed; harvest() becomes load-bearing again.

---

Finding 5 — ReturnFire-stance auto-fire breaks kite-and-pull

Status on main: PARTIALLY FIXED — empirical re-measure needed.

The original finding's option (b) — "Pure ReturnFire that fires only after taking hits" — landed in commit 9e999e2 fix(engine): clarify stance — stance:1 true return-fire-only. Pinned by openra-sim/tests/test_stance_semantics.rs::test_stance_1_return_fire_only_against_passive_enemy. A stance:1 unit next to a passive (stance:0) enemy now holds fire indefinitely.

But the scenario breaks for a different reason: once a stance:1 raider takes one hit from a 3tnk (hunt bot), it auto-fires forever (the 60-tick window self-refreshes on each subsequent hit). So the 1v1 kite question becomes a balance question: does a passive ReturnFire 2tnk out-trade an aggressive 3tnk by auto-fire alone? On the OLD stance:1 (auto-engage on any in-range enemy) the answer was yes — the original finding's empirical claim. On the NEW stance:1 the answer depends on exact weapon DPS values which changed across the wave merges; uncertain without a fresh empirical run.

TODO empirical re-measure: run the stand-still policy (no agent orders) on combat-kite-and-pull.yaml against main HEAD 8ca8989. Suggested test: OpenRA-Rust/openra-sim/tests/test_kite_1v1.rs — 1 2tnk at (10,10) stance:1 vs 1 3tnk at (40,10) stance:3 on a 60×30 arena, run 500 frames with no orders, assert 2tnk dies. If it passes, finding #5 is genuinely fixed and the bench's 3-raider focus-fire workaround in combat-kite-and-pull.yaml:181-183 can be reverted to a true 1v1. If it fails, the residual is balance: tune the 2tnk's 90mm reload delay or damage in gamerules.rs.

Bench-side workaround: combat-kite-and-pull.yaml uses a 3-raider stack to substitute focus-fire micro for distance-control (documented inline). Workaround is correct given current uncertainty; remove only after the empirical re-measure resolves.

---

Finding 6 — Arena generator's default 4-corner mpspawns rotate per-seed

Status on main: DOC-ONLY (engine code does not offset pre-placed actor coords).

Re-reading the engine source: build_scenario_actor (openra-train/src/env.rs:2338+) places each scenario actor at its literal sa.position — there is NO offset math anywhere. The finding's claim "authored position: [6, 5] actually appears at y=29-33 on some seeds" is not substantiated by the code. The practical effect that DOES exist: assign_spawn_points (world.rs:4684+) picks one mpspawn per seed for the auto-spawned MCV, deterministic on seed. So when a pack relies on the auto-MCV's corner being a specific one (not its position absolute), that DOES vary per seed.

Action taken: docs/ENGINE_FOLLOWUPS.md finding #6 reworded to remove the misleading "engine offsets pre-placed actors" claim.

Bench-side workaround: the spawns: [[6, 5]] declaration in perception-target-vs-fog.yaml:62-68, 157, 200, 252 (and any other arena-generator pack that hand-places agent buildings AND relies on a fixed auto-MCV corner) is correct and stays.

Optional follow-up (NOT in this commit): change openra_bench/mapgen.py::_default_spawns default to a single centred mpspawn so position invariants hold without the workaround. Touches default behaviour for every existing arena-generator pack — out of scope for a doc-cleanup pass.

---

Finding 7 — Pre-placed proc inside an ore_patches: disc silently empties patch

Status on main: BROKEN (with refined understanding).

env.rs::build_world_for_episode lines 958-1029 seed ore patches FIRST (line 966-974: seed_ore_patch for each declared patch), then inject scenario actors. A proc placed inside a patch disc occupies its footprint cells via terrain.occupy_footprint, marking them ground-impassable WITHOUT clearing the resource layer. Result: ore IS placed (the original finding's "silently zero ore" wording is slightly off), but find_path cannot reach those cells from the harvester. The harvester loops "FindingOre → can't path → fail" indefinitely on the cells under the proc footprint.

Pinning: openra-data/tests/test_proc_overlapping_patch_warns.rs — #[ignore]d. Loads a scenario with proc at (40,10) and ore_patches: [{x:40, y:10, amount:5000, radius:3}], asserts the (currently nonexistent) validate_layout returns a warning string containing proc_overlaps_patch. Flip to #[test] after the validator ships.

TODO engine fix: add oramap::validate_layout(&MapDef) -> Vec<String> that returns one warning per overlapping (building, patch) pair. Surface via OpenRAEnv.last_warnings at world-build. Cheap; the optional second fix (skip ore-seeding on cells already occupied by a building footprint, or re-order seed-after-actor-injection) is more invasive — leave for later.

Bench-side workaround: trivial (place the proc just outside the patch radius). Already widely applied; no specific files to clean up.

---

Pre-existing test failures (NOT this triage pass)

Documented for clarity so they aren't conflated with finding-related failures:

• openra-sim lib test gamerules::tests::defaults_have_all_common_units — MCV vs Vehicle kind classification regression, predates this pass (logged in OpenRA-Bench/ENGINE_AUDIT.md).
• openra-sim integration tests sync_hash_verify (×2), debug_orders_and_hashes — sync-hash reference fixtures stale after recent merges.
• openra-sim integration test tank_attack_terminates_when_no_armament_loaded
  • asset-loading data tests (extract_*, find_*, parse_infantry_yaml, e1_rifleman_typed_fields, render_*, etc.) — all need the vendored OpenRA mod dir at OpenRA-Rust/vendor/OpenRA/mods/ra/ (not present in this workspace; environmental, not code-related).
• openra-data test_per_player_starting_cash, etc. — earlier these failed with MissingBaseMap because the rush-hour-arena.oramap fixture wasn't in openra-data/tests/fixtures/. Fixed in this pass by copying the map from OpenRA-Bench/data/maps/. All four per_player_starting_cash + 3 ore_patches data tests are green now.

Summary of changes shipped this pass

Engine (OpenRA-Rust):

• openra-sim/src/world.rs::detonate_nuke — credit nuke AoE kills to firing owner (mirrors projectile-resolve path; friendly-fire is excluded).
• openra-sim/tests/test_nuke_credits_kills.rs — new (2 tests).
• openra-sim/tests/test_harvest_explicit_target.rs — new (#[ignore]d, pins finding #4).
• openra-data/tests/test_proc_overlapping_patch_warns.rs — new (#[ignore]d, pins finding #7).
• openra-data/tests/fixtures/rush-hour-arena.oramap — copied in from OpenRA-Bench/data/maps/ so existing data tests run cleanly in this workspace.

Bench (OpenRA-Bench):

• CLAUDE.md — finding #1 doc fix in two places.
• openra_bench/scenarios/schema.py::MapDef.ore_patches docstring — finding #2 doc fix (clamp formula + recommended ratio).
• docs/ENGINE_FOLLOWUPS_TRIAGE.md — this file (refreshed against post-merge HEAD).
• docs/ENGINE_FOLLOWUPS.md — finding #6 reworded to drop the misleading "engine offsets pre-placed actors" claim.

TODOs surfaced:

• finding #4: implement bound_patch on Activity::Harvest (see test).
• finding #5: empirical re-measure on combat-kite-and-pull to verify whether the stance:1 fix is sufficient.
• finding #7: implement oramap::validate_layout (see test).
• finding #2: optional cap raise from 12 → 24/cell.
• spec-nuke-strike.yaml: switch hard-tier predicate to units_killed_gte now that finding #3 is fixed.