diff --git a/docs/B4.md b/docs/B4.md index 3fb068a..8e91b09 100644 --- a/docs/B4.md +++ b/docs/B4.md @@ -1,15 +1,31 @@ # B4 — the colony turn and the fleet movement pass, old vs new -**Status (2026-09-08): code complete, cross-built, staged; every VM step still owed.** -VM140 was held by another lane for the whole of this milestone, so nothing was deployed, the -game was not stopped or relaunched, and `C:\SOTS\binkw32.dll` / `C:\SOTS\shimdist` were not -touched. Everything below is offline work plus what the *binary* says; the run list is at the -end. The build lives in its own tree (`/srv/re-lab/build/sots-engine-b4`) and its own dist -(`/srv/re-lab/shim/dist-b4`), not the shared ones. Build -`b4-final-20260908T0500Z`, exports 66 names identical to `binkw32.dll`. Host suite **30/30**. `tools/clean_room_check.sh` OK. +**Result (2026-09-08): verified on the live game. 36 compared, 0 divergences.** -Ghidra was available this round and was used at the end to write the verified prototypes back -into the shared project (`reva-server` stopped for the run and restarted afterwards). +| pass | build | records | verdict | +|---|---|---|---| +| scout (`b4scout`) | `b4-fix1-…0600Z` | 28 colony + 1 movement | `tracecmp.py` exit 0, 0 invalid | +| trace (`b4trace`) | `b4-fix1-…0600Z` | 28 colony + 7 `MoveFleet` + 1 pass | exit 0, 0 invalid | +| compare (`b4compare`) | `b4-fix2-…0615Z` | 36 calls, **all 36 compared** | **0 diverged, 0 errors**, exit 0 | + +The headline is the scout's, and it is what the milestone was for: **the generator did not move +on a single one of the 28 systems** — `left` delta 0 and the `mt[624]` block hash identical +before and after, on every record — which is exactly what the static sweep predicted (only +`ProcessRebellion` draws, and no colony rebelled this turn). `fpu_cw = 0x127f` (53-bit) on all +28, confirming the two earlier lanes rather than re-investigating. + +The one fleet that actually moved reproduced bit for bit: fleet 34 went +`(-11.9286022, 4.71900415, 2.31785131) -> (-10.5634995, 5.97172451, 1.56473362)` and our +float32 position is identical in all three components; its ship's range went 20 -> 18, i.e. +exactly `speed 2 x dt 1.0`. `sim::PlanFleetMovement`'s predicted call schedule matched the +observed one exactly (all seven fleets, `dt = 1.0`, in fleet-vector order — no pursuits in this +save). + +Three bugs in my own hooks were caught by the runs, all of which would have produced a *clean +compare that checked nothing* (see "Three hook bugs the live runs caught"). Host suite 30/30, +`tools/clean_room_check.sh` OK. Ghidra was used to write the verified prototypes back into the +shared project (`reva-server` stopped for the run and restarted afterwards). The VM is released +at the main menu with `hooks=trace` and build `b4-fix2-20260908T0615Z` deployed. ## What was hooked @@ -39,6 +55,37 @@ hooked, because M0's lesson is that a wrong `thiscall` prototype crashes the gam `push ecx; fstp DWORD PTR [esp]`. It returns `AL`, and the caller tests it. * `ProcessFleetMovement` ends in a plain `ret` with `mov esi,ecx` and no stack reads. +## Three hook bugs the live runs caught + +None would have shown up as a divergence. Each would have made a compare that reported +"0 diverged" while checking less than it claimed — the same class of failure the harness audit +lane is chasing, found the same way B3 found its own: by looking at the trace rather than at +the verdict. + +1. **`describe_args` runs before `regions()`.** The template calls them in that order and both + descriptors took their snapshot in `regions()`, so every argument sourced from it was **one + call stale** — the first record showed zeros, the rest showed the previous system's state. + The `inputs` region was always right (its "before" is captured after `regions()` returns), so + the compare would have been sound while the trace's own argument record lied. Fixed: capture + in `describe_args`, reuse in `regions()`. +2. **The StrategyServer has two bases four bytes apart.** A `ServerSystem`'s `owner` word + (+0x10) points at a base four bytes *above* the one the class's own methods receive in ECX + (the generator accessor at 0x007437f0 does `owner - 4`). Every `StrategyServer_off_*` in the + contract is relative to the raw/owner base, so the colony hook — which starts from + `sys->owner` — was right, and the two movement hooks, which applied the same offsets to + `this`, were not. `ProcessFleetMovement` read an empty player vector, declared **zero** + gate-traffic regions, and reported `players=0 fleets=0`; tracecmp dutifully printed "1 call, + 0 diverged" for a hook that compared nothing. Fixed with an explicit `raw_server()` rebase. +3. **`StrategyServer_off_Fleets` was the Ghidra-base number written down as a raw-base one.** + With `0x64` instead of `0x60` the hook enumerated the fleet vector's *spare capacity* rather + than its elements: the trace pass showed `ProcessFleetMovement` reporting 1 fleet while + `MoveFleet` was called 7 times in the same turn. The gate-traffic totals came out all-zero + and matched ours — correct by luck, from a garbage fleet list. Fixed to `0x60`; the compare + pass then reported 7 fleets in exactly the order `MoveFleet` was called for them. + +The contract entry for `StrategyServer_off_Players` now spells the two-base hazard out, and the +three corrected offsets carry the reason in their prototype text. + ## The declared input boundary — say it out loud Both colony and movement targets are mostly *dispatchers*. Being honest about that is what @@ -285,62 +332,104 @@ Every constant was checked bit by bit, because B2 and B3 were both bitten here. of records, but on a large map it is every fleet times up to five passes times the recursion depth. The `b4scout` config exists for exactly that reason: it leaves `MoveFleet` off. -## What remains (needs the VM) +## What the runs actually exercised — and what they did not -The lane holding VM140 must be finished first; then, in this order: +A clean compare over 36 calls is worth exactly as much as the coverage behind it, so here is +the coverage, region by region, from the compare log itself. -1. Deploy `/srv/re-lab/shim/dist-b4` (build `b4-final-20260908T0500Z`): `scp` it to - `C:\SOTS\shimdist-b4\` and run `deploy.ps1 -Dist C:\SOTS\shimdist-b4` — **a separate staging - directory from the shared `C:\SOTS\shimdist`**, so no other lane's dist is overwritten. -2. **Scout pass.** Copy `shim.cfg.b4scout` over `C:\SOTS\shim.cfg`, relaunch, load - `ref-turn2.sav`, press End Turn once, pull `C:\SOTS\shim.trace.jsonl` → - `b4-scout.jsonl`. `tracecmp.py` must exit 0 with 0 invalid records. `MoveFleet` is off in - this config, so the file stays small. **Read off it before going further:** - * `fpu_cw` on every record — expect `0x027f`; `0x007f` / `0x003f` means 24-bit x87 - precision and the float mapping needs the PC24 route (B3's open question). - * per `ServerSystem::ProcessTurn` record: `args.rng_left_in` minus `side.rng.after.left`. - **Expect 0 on every system.** A non-zero delta names a system whose `ProcessRebellion` - fired, and that system's compare record is then expected to diverge on `rng` and only on - `rng`. - * how many systems report `owned`, `stable`, a non-zero `ibon`/`pbon`, a non-empty - `civilians` list and a non-zero `bats2`/`rcex`. That is the coverage table for step 4. - * per `ProcessFleetMovement` record: the `fleet_state` list — how many fleets exist, how many - have waypoints, and whether any waypoint type is 4 or 5. If none is, gate traffic is - exercised in its zero branch only and must be reported that way. -3. **Trace pass.** Copy `shim.cfg.b4trace` (adds `MoveFleet`), relaunch, load `ref-turn2.sav`, - End Turn → `b4-trace-golden.jsonl`. Check the `MoveFleet` record sequence against - `sim::PlanFleetMovement`'s prediction: the fleet ids and `dt` values should appear in the - pass order documented above. A mismatch is a finding about the schedule, not about the step. -4. **Compare pass.** Copy `shim.cfg.b4compare`, relaunch, load `ref-turn2.sav`, End Turn → - `b4-compare.jsonl`. Expected: - * `ServerSystem::ProcessTurn` — **0 divergences on every system**, on all twelve regions. - The reference save is a turn-2 two-empire game, so `infra`/`ibon`/`pbon` are probably - exercised in their no-op branches and `bats2`/`rcex` in their all-zero branch; say which - regions actually carried a value rather than implying the rest passed. - * `MoveFleet` — 0 divergences on `pos`, `prev_pos` and every `ship[i].range` for a call that - did **not** arrive. A call that arrived is expected to match on those too (the snap is a - verbatim copy) but the RNG and the undeclared arrival state are the original's; a - divergence on `pos` after an arrival is a real finding. - * `ProcessFleetMovement` — 0 divergences on every `gate_traffic[i]`. - Any other diff is a real finding: report it, do not tune the formula. -5. Restore the previous `shim.cfg` (`hooks=trace`) and leave the game at the main menu, as - M1/M2/B1/B3 left it. +**`ServerSystem::ProcessTurn`, 28 calls.** Only **3 systems are owned** (indices 4 and 15, the +two players' home worlds; index 16, an independent/NPC colony owned by player 7 and the only +non-home one). The other 25 are unowned. -**Not done, and worth saying:** +| region | carried a value? | what that means | +|---|---|---| +| `ntdev` | **yes, moved on 3** | the stable-increment path is verified on all three owned colonies | +| `rcex` | **yes, moved on 6** | the recon countdown sweep is genuinely exercised, including a nibble reaching zero | +| `rcex_mask` | one system held a stale bit with a zero counter | the "skip the sweep when the word is zero" branch is verified | +| `infra` | matched on all 28, but never *changed* | owned colonies sit at 1.0 (the apply is a no-op) and unowned ones at 0.0, so the decay is only exercised in its clamp branch | +| `ibon` / `pbon` | matched, never changed | both home colonies are at their cap, so `ApplyPopBonus` short-circuits and `ApplyInfraBonus` returns at `Infra >= 1`; the accrual gate fails on `ntdev` | +| `tres` / `haltv` | matched at 0 / false on all 28 | zero branch only | +| `bats2` / `bats_mask` | zero on all 28 | **the battle countdown is completely untested** | +| `rng` | matched on all 28, bit for bit | the real result: no colony drew a word | + +**`MoveFleet`, 7 calls.** Six are fleets with **no waypoints at all** (`wpt_type = -1`), which +exercise only the early-out. **One call does real work**: fleet 34, waypoint type 1 (a straight +run) to a system, `speed 2 x dt 1.0`, and it matched on `pos`, `prev_pos` and its ship's range. +So the step arithmetic is verified for exactly one straight-line move. Not exercised at all: +the range clamp against a real limit (the ship had 20 range for a 2-unit move), the stranded +case, node-line travel (type 2), a node route (type 3), a gate teleport (type 4), a +probabilistic jump (type 5, and with it every RNG draw in the movement path), the multi-waypoint +recursion, and an arrival. + +**`ProcessFleetMovement`, 1 call.** All eight `gate_traffic` words matched — but every one of +them is **0 before and 0 after**. Three fleets carry non-zero traffic words (10, 12, 12…) yet +none has a waypoint, so nothing is summed. **The gate-traffic total is verified in its zero +branch only.** The pass schedule was checked separately against `sim::PlanFleetMovement` and +matched, but this save has no pursuits, so only the "everything else, dt 1.0" pass is covered. + +## Writes of the original that the declared regions do NOT cover + +Stated explicitly, because a clean compare says nothing about any of it. This is the content of +the `coverage` block once `HookPolicy` carries the field (the harness audit lane owns that +struct change); it lives as a comment at the top of each descriptor header until then. + +**`ServerSystem::ProcessTurn`** — `state: partial` +- **the addiction sweep's morale events**: `ours` computes the `{species, event id, delta}` + list, but nothing applies it, so the system's `Morale int[7]` and its **morale-event vector + append** are never written or compared. *Risk: high — this is precisely B3's failure mode, a + list append outside every declared region.* Not exercised by this save either (no addiction). +- the whole plague pass, imperial and civilian population growth, `AdjustResources`, the + in-orbit refuel, `ProcessSlaves` and `ProcessRebellion`. *Risk: high; no guard.* +- the independent-colony imperial↔civilian population drift (0x007514f0). *Risk: medium.* +- `TnsOH`, the build queue's own list, and every ship the queue creates. *Risk: medium.* + +**`MoveFleet`** — `state: partial` +- **every arrival handler**: `FleetArrives`, the `SEFleetArrived` **event object and its + dispatch**, the next-leg pathability check, and the **insert into the server's in-motion set** + at +0x210. *Risk: high — an event post and two `std::set` inserts, none declared.* +- the departure block: the script hook at StrategyServer+0x1b4, per-ship action cancellation, + and `ServerSystem::FleetDeparts` (which rewrites a system's fleet vector and its per-player + presence bitmasks). *Risk: high.* +- **the waypoint vector itself** — a completed leg pops its waypoint and a failed probabilistic + jump rewrites the whole route. *Risk: high.* +- the tanker top-up and the fleet flag word at +0x10c. *Risk: medium.* +- the node-line step: our side computes `speed x dt` for every waypoint type, so a **type-2 + waypoint's step is wrong by construction**. *Risk: high; mitigated only by `wpt_type` being on + the record — treat type 2 as not compared.* + +**`ProcessFleetMovement`** — `state: partial` +- the entire pass schedule and therefore every `MoveFleet` call it makes. *Mitigation: + `sim::PlanFleetMovement` predicts the order and the trace was checked against it.* +- the per-fleet destination position at +0xec and the flag-0x2 / flag-0x100 clears. *Medium.* +- **`OnFleetArrived`**: it takes the set difference of +0x200 and +0x210, clears both, and + dispatches a per-player arrival event. *Risk: high.* + +## Runs (`/srv/re-lab/shim/traces/`) + +| file | mode | build | calls | result | +|---|---|---|---|---| +| `b4-scout.jsonl` | trace | `b4-fix1-…0600Z` | 29 | exit 0, 0 invalid; **RNG delta 0 on all 28 systems** | +| `b4-trace-golden.jsonl` | trace | `b4-fix1-…0600Z` | 36 | exit 0, 0 invalid; schedule matched the prediction | +| `b4-compare.jsonl` | compare | `b4-fix2-…0615Z` | 36 | **36 compared, 0 diverged, 0 errors** | +| `b4-compare-turn2/3.png` | | | | savings 289,688 -> 532,369, the reference oracle state | + +Workload each time: main menu -> Load Game -> Single Player -> `ref-turn2.sav` -> Launch -> +turn 2 (savings 289,688) -> **End Turn** -> turn 3 (savings 532,369). + +## Still owed * **Replace mode is not offered** for any of the three hooks, so there is no End-Turn oracle - result for this milestone. That is a deliberate consequence of the input boundary, not an - omission — the strongest available evidence here is the compare plus the RNG post-state. -* **Sub-paths the reference save will not exercise**, and which must therefore be reported as - untested rather than passed: plague, rebellion (and with it every RNG draw in a colony turn), - slaves, terraforming (the reference colony sits at its ideal), the addiction sweep at any - phase, the unowned-system infrastructure decay, a non-home colony's `ntdev` reset, the - probabilistic jump, the gate teleport, node-line travel, and a stranded fleet. A save with a - plague, a rebelling colony, a Hiver gate network and a Zuul node bore would exercise most of - them and is the natural next workload. -* The **node-line step** is modelled and unit-tested but not hooked (see "What our side runs"), - and under shipped data the stutter ramp is a constant anyway (correction 13). -* `ComputeOutputFromRates` was read instruction by instruction and every correction is folded - into `game/sim/colony`, but it is **not hooked**: it repairs damaged ships in orbit as a side - effect (`0x00751590` with its estimate flag clear), so a compare hook cannot run it on a - scratch copy of the system without isolating the ships too. That is its own milestone. + result for this milestone. That is a consequence of the input boundary above, not an + omission: our side models a slice of each function, and feeding that slice to the game would + strand every arriving fleet or skip a colony's whole turn. +* **A richer save.** The reference save cannot exercise plague, rebellion (and with it every + colony RNG draw), slaves, terraforming, the addiction sweep, the battle countdown, a + bonus-absorbing colony, gate traffic, node-line travel, a probabilistic jump, an arrival, or + a stranded fleet. A save with a plague, a rebelling colony, a Hiver gate network and a Zuul + node bore would cover most of them and is the natural next workload. +* **The node-line step is not wired into the hook** — it needs the node graph walked from the + live server. Until then a type-2 waypoint is compared with the wrong step. +* **`ComputeOutputFromRates` is read but not hooked**: it repairs damaged ships in orbit as a + side effect, so it cannot run on a scratch copy of the system. Its own milestone. +* The three descriptors are `unstated` to the harness; the text above is ready to drop into the + `coverage` field the moment `HookPolicy` carries it. diff --git a/include/generated/sots_addresses.h b/include/generated/sots_addresses.h index 4e40ca4..d7a7c50 100644 --- a/include/generated/sots_addresses.h +++ b/include/generated/sots_addresses.h @@ -1,5 +1,5 @@ // GENERATED — do not edit. Facts about Sword of the Stars.exe (GOG 1.8.1). -// Source: sots-re ghidra/addresses.json @ 5b6f812, generated 2026-09-08 by tools/gen_addresses.py +// Source: sots-re ghidra/addresses.json @ b878c4c, generated 2026-09-08 by tools/gen_addresses.py // Runtime address = (uintptr_t)GetModuleHandle(NULL) + RVA (the exe is ASLR-relocated). #pragma once #include @@ -577,18 +577,18 @@ constexpr uint32_t ServerSystem_off_ntdev = 0x000002c4; constexpr uint32_t ServerSystem_off_rbtn = 0x000002cc; // offset int ModCount (the turn counter), relative to the RAW server base a ServerSystem's +0x10 points at [verified] constexpr uint32_t StrategyServer_off_ModCount = 0x00000008; -// offset std::vector (begin @+0x50, end @+0x54); numPlayers = (end-begin)>>2 [verified] +// offset std::vector (begin @+0x50, end @+0x54); numPlayers = (end-begin)>>2 /* TWO BASES: every StrategyServer_off_* here is relative to the RAW base a ServerSystem's owner word (+0x10) points at. The class's own methods receive a base FOUR BYTES LOWER in ECX (0x007437f0 does owner-4), so a hook on MoveFleet or ProcessFleetMovement must add 4 to `this` before applying these */ [verified] constexpr uint32_t StrategyServer_off_Players = 0x00000050; -// offset std::vector (begin @+0x64, end @+0x68) -- one flat global list, not per player [verified] -constexpr uint32_t StrategyServer_off_Fleets = 0x00000064; +// offset std::vector (begin @+0x60, end @+0x64) -- ALL fleets of ALL players, one flat global list. B4 live: the earlier 0x64 was the Ghidra-base number transcribed as a raw-base one; it made ProcessFleetMovement enumerate the vector's spare capacity instead of its elements [verified] +constexpr uint32_t StrategyServer_off_Fleets = 0x00000060; // offset 16-bucket hash of entity id -> object, hashed by (key & 0xF); 0x008b9240 is the lookup [verified] -constexpr uint32_t StrategyServer_off_EntityHash = 0x00000084; +constexpr uint32_t StrategyServer_off_EntityHash = 0x00000080; // offset Mars::RNG* relative to the RAW base (== the -4-adjusted base's +0x16c) [verified] constexpr uint32_t StrategyServer_off_RNGPtr = 0x00000168; // offset std::set> cleared at the head of ProcessFleetMovement [verified] -constexpr uint32_t StrategyServer_off_ArrivedSet = 0x00000204; +constexpr uint32_t StrategyServer_off_ArrivedSet = 0x00000200; // offset std::set> that MoveFleet fills for fleets still in motion; OnFleetArrived takes the set difference [verified] -constexpr uint32_t StrategyServer_off_InMotionSet = 0x00000214; +constexpr uint32_t StrategyServer_off_InMotionSet = 0x00000210; // offset int id (also the entity-hash key) [verified] constexpr uint32_t StarFleet_off_Id = 0x00000004; // offset the object whose +0x80 holds the entity hash the waypoint target is resolved in [verified] diff --git a/src/shim/hooks/colony_turn.cpp b/src/shim/hooks/colony_turn.cpp index 44999d9..e10a135 100644 --- a/src/shim/hooks/colony_turn.cpp +++ b/src/shim/hooks/colony_turn.cpp @@ -326,6 +326,9 @@ void init_colony_turn(std::uintptr_t exe_base, void (*log_line)(const char* line } void ServerSystemProcessTurnHook::describe_args(std::vector& out, void* self) { + // The template calls describe_args BEFORE regions, so the snapshot is taken here and + // regions reuses it. Capturing in regions() left every argument one call stale. + capture(self); out.push_back(tv::ptr(self).named("system")); // The snapshot is the whole input record; it doubles as the `inputs` region below, but the // harness never compares arguments, so the interesting context lives here. @@ -336,7 +339,6 @@ void ServerSystemProcessTurnHook::describe_args(std::vector& out, void* self } void ServerSystemProcessTurnHook::regions(std::vector& out, void* self) { - capture(self); if (!self) return; // Only the words `sim::ProcessColonyTurn` models are declared. Everything the callees diff --git a/src/shim/hooks/colony_turn.h b/src/shim/hooks/colony_turn.h index d29ad33..4053367 100644 --- a/src/shim/hooks/colony_turn.h +++ b/src/shim/hooks/colony_turn.h @@ -17,6 +17,24 @@ // ProcessRebellion (ProcessPlague, the civilian growth pass and ProcessSlaves were each swept // to call depth one and are draw-free), so on a system with no rebellion the post-call state // must be **identical** -- and any movement of it names the system whose rebellion fired. +// COVERAGE STATEMENT (for meta.hooks[H].coverage once HookPolicy carries the field; the +// harness audit lane owns that struct change, so this lives as text until it lands). +// state: "partial" +// unmodelled: +// - what: "the whole plague pass (ProcessPlague), imperial and civilian population growth, +// the resource debit (AdjustResources), the in-orbit refuel, ProcessSlaves and +// ProcessRebellion" +// risk: high mitigation: "none - those fields are not declared regions at all" +// - what: "the addiction sweep's morale events: ours computes the {species, event id, +// delta} list but nothing applies it, so the system's Morale int[7] and its +// morale-event vector are never written or compared" +// risk: high why: "this is exactly B3's failure mode - a list append outside every +// declared region" mitigation: "guard:morale (not yet declared)" +// - what: "the independent-colony imperial<->civilian pop drift (0x007514f0), called +// between the bonus pools and the stability check" +// risk: medium mitigation: "none" +// - what: "TnsOH, the build queue's own list, and every ship the queue creates" +// risk: medium mitigation: "none" #pragma once #include diff --git a/src/shim/hooks/fleet_movement.cpp b/src/shim/hooks/fleet_movement.cpp index b6c3c2c..8ba7c53 100644 --- a/src/shim/hooks/fleet_movement.cpp +++ b/src/shim/hooks/fleet_movement.cpp @@ -32,6 +32,18 @@ constexpr std::size_t kMaxPlayers = 64; // table); the generated header's highest declared offset is +0x10c (Flags). constexpr std::size_t kFleetGuardSize = 0x120; +// The StrategyServer has two bases in play. A ServerSystem's `owner` word (+0x10) points at a +// base four bytes ABOVE the one the class's own methods receive in ECX -- the accessor at +// 0x007437f0 does `owner - 4` before reading the generator. Every `StrategyServer_off_*` in the +// address contract is relative to the **raw** (owner) base, so a hook whose `this` is the +// method base has to add four before applying them. Getting this wrong silently reads a +// neighbouring field: the first scout run declared zero gate-traffic regions because the +// player vector came back empty. +constexpr std::size_t kServerRawDelta = 4; +void* raw_server(void* self) { + return self ? static_cast(self) + kServerRawDelta : nullptr; +} + using ResolveWaypointFn = void*(SHIM_THISCALL*)(void* fleet); using RelationFn = int(SHIM_THISCALL*)(void* player, void* other); @@ -92,7 +104,8 @@ std::uint32_t fpu_control_word() { #endif } -void* rng_of(void* server) { +void* rng_of(void* self) { + void* server = raw_server(self); if (!readable(server, A::StrategyServer_off_RNGPtr + 4)) return nullptr; void* r = ptr_at(server, A::StrategyServer_off_RNGPtr); return readable(r, kRngSize) ? r : nullptr; @@ -122,8 +135,9 @@ std::vector fleet_ships(const void* fleet) { return ships; } -std::vector server_players(void* server) { +std::vector server_players(void* self) { std::vector out; + void* server = raw_server(self); if (!readable(server, A::StrategyServer_off_Players + 8)) return out; void** begin = static_cast(ptr_at(server, A::StrategyServer_off_Players)); void** end = static_cast(ptr_at(server, A::StrategyServer_off_Players + 4)); @@ -134,8 +148,9 @@ std::vector server_players(void* server) { return out; } -std::vector server_fleets(void* server) { +std::vector server_fleets(void* self) { std::vector out; + void* server = raw_server(self); if (!readable(server, A::StrategyServer_off_Fleets + 8)) return out; void** begin = static_cast(ptr_at(server, A::StrategyServer_off_Fleets)); void** end = static_cast(ptr_at(server, A::StrategyServer_off_Fleets + 4)); @@ -309,6 +324,9 @@ void init_fleet_movement(std::uintptr_t exe_base, void (*log_line)(const char* l void StrategyServerMoveFleetHook::describe_args(std::vector& out, void* self, void* fleet, float dt) { + // The template calls describe_args BEFORE regions, so the snapshot is taken here; regions + // then reuses it. Capturing in regions() left every argument one call stale. + capture_move(self, fleet, dt); out.push_back(tv::ptr(self).named("server")); out.push_back(tv::ptr(fleet).named("fleet")); out.push_back(tv::f32(dt).named("dt")); @@ -326,7 +344,8 @@ Tv StrategyServerMoveFleetHook::describe_ret(bool r) { return tv::boolean(r); } void StrategyServerMoveFleetHook::regions(std::vector& out, void* self, void* fleet, float dt) { - capture_move(self, fleet, dt); + (void)self; // describe_args already captured everything this needs + (void)dt; if (!readable(fleet, A::StarFleet_off_Flags + 4)) return; trace::Region pos; @@ -459,6 +478,8 @@ bool StrategyServerMoveFleetHook::ours(void* self, void* fleet, float dt) { // ---- ProcessFleetMovement -------------------------------------------------------------------- void StrategyServerProcessFleetMovementHook::describe_args(std::vector& out, void* self) { + // Same ordering rule as MoveFleet: capture here, before regions runs. + capture_pfm(self); out.push_back(tv::ptr(self).named("server")); out.push_back(tv::i32(g_pfm.playerCount).named("players")); out.push_back(tv::i32(static_cast(g_pfm.fleets.size())).named("fleets")); @@ -476,10 +497,9 @@ void StrategyServerProcessFleetMovementHook::describe_args(std::vector& out, void StrategyServerProcessFleetMovementHook::regions(std::vector& out, void* self) { - // Captured BEFORE the original runs, so the argument record shows the pre-move fleet - // state; `ours` re-captures afterwards, because that is when the original sums the - // traffic. - capture_pfm(self); + // describe_args already captured the pre-move fleet state that the argument record shows; + // `ours` re-captures afterwards, because that is when the original sums the traffic. + (void)self; g_pfm.names.clear(); g_pfm.names.reserve(g_pfm.players.size()); for (std::size_t i = 0; i < g_pfm.players.size(); ++i) { diff --git a/src/shim/hooks/fleet_movement.h b/src/shim/hooks/fleet_movement.h index f92ca34..d296a6a 100644 --- a/src/shim/hooks/fleet_movement.h +++ b/src/shim/hooks/fleet_movement.h @@ -24,6 +24,35 @@ // input boundary: everything else. The pass schedule is recorded in the arguments (ours // predicts the call order so a trace can be checked against it) but is not // itself compared, because reproducing it would mean running MoveFleet. +// COVERAGE STATEMENT (see the note in colony_turn.h about where this will live). +// +// MoveFleet -- state: "partial" +// - what: "every arrival handler: FleetArrives, the SEFleetArrived event object and its +// dispatch, the next-leg pathability check, and the insert into the server's +// in-motion set at +0x214" +// risk: high why: "an event post and two std::set inserts, none of them declared" +// - what: "the departure block: the script hook at StrategyServer+0x1b4, per-ship action +// cancellation, and ServerSystem::FleetDeparts (which rewrites a system's fleet +// vector and its per-player presence bitmasks)" +// risk: high mitigation: "none" +// - what: "the waypoint vector itself - a leg that completes pops its waypoint, and a failed +// probabilistic jump rewrites the whole route" +// risk: high mitigation: "none" +// - what: "the tanker top-up (flag 0x1000 ships are refuelled to full each step) and the +// fleet flag word at +0x10c (bit 1 set when the fleet moved)" +// risk: medium mitigation: "none" +// - what: "the node-line step: our side computes speed x dt for every waypoint type, so a +// type-2 waypoint's step is wrong by construction" +// risk: high mitigation: "the record carries wpt_type; treat type 2 as not compared" +// +// ProcessFleetMovement -- state: "partial" +// - what: "the entire pass schedule and therefore every MoveFleet call it makes" +// risk: high mitigation: "sim::PlanFleetMovement predicts the order; check the trace" +// - what: "the per-fleet destination position at +0xec and the flag-0x2 / flag-0x100 clears" +// risk: medium mitigation: "none" +// - what: "OnFleetArrived: it takes the set difference of +0x204 and +0x214, clears both, +// and dispatches a per-player arrival event" +// risk: high mitigation: "none" #pragma once #include