diff --git a/.project/objectives/p2-65-extract-mc-state-crate.md b/.project/objectives/p2-65-extract-mc-state-crate.md new file mode 100644 index 00000000..8d4ac891 --- /dev/null +++ b/.project/objectives/p2-65-extract-mc-state-crate.md @@ -0,0 +1,213 @@ +--- +id: p2-65 +title: "Extract `GameState` and pending-queue data types into a dedicated `mc-state` crate" +priority: p2 +status: stub +scope: game1 +category: architecture +owner: simulator-infra +created: 2026-05-09 +updated_at: 2026-05-09 +blocked_by: [] +follow_ups: [] +--- + +## Context + +`mc-turn::game_state::GameState` is the canonical full-simulation state +struct (893 LOC at `src/simulator/crates/mc-turn/src/game_state.rs`). It +mixes data-only types (`GameState`, `PlayerState`, `MapUnit`, `TechState`, +`PendingCaptureEvents`, eight action-request structs, `RallyCommand`, +`BuildingRallyPoint`, `CityEcology`) with the `mc-turn` crate that *also* +owns turn-step logic (`processor.rs`, `action_handlers/`, `combat_event.rs`, +`end_conditions.rs`, `victory.rs`, etc.). + +Today **9+ source files** across **6 simulator crates and both API surfaces** +import these types via `use mc_turn::…` — every consumer therefore pulls +the full turn-step compile graph (rayon, GPU optional deps, action-handler +modules) just to talk about state shape. Reverse-direction is also irregular: +`mc-turn` depends on `mc-ai::evaluator::ScoringWeights` for one field on +`GameState`, even though the dataflow is "AI reads state", not "state +embeds AI logic". + +Consumers that import state types from `mc-turn`: + +| Crate / surface | Sites | +|---|---| +| `mc-ai` | `lib.rs`, `evaluator.rs`, `abstract_state.rs` (note: distinct from `mc-ai::game_state` which is the AI-internal abstraction) | +| `mc-mcts-service` | `protocol.rs` | +| `mc-replay` | (verify — historic re-export plumbing) | +| `mc-sim` | `event_dispatch.rs` | +| `mc-core` | `derived_stats.rs` | +| `mc-turn` (internal) | every action handler, capture, courier, ransom, victory, processor | +| `api-gdext` | `lib.rs` (init), `ai.rs`, `building_action.rs`, `tests/capture_bridge.rs` | +| `api-wasm` | (verify) | + +Ref count for the strict newtype set +(`GameState`/`PlayerState`/`MapUnit`/`TechState`/`PendingCaptureEvents` + +6 action-request structs + `RallyCommand`/`BuildingRallyPoint`/`CityEcology`): +≥9 distinct files (`grep -l "use mc_turn::\\(GameState\\|...\\)"`, 2026-05-09). +Add internal `mc-turn` files (~30 import sites including action handlers, +processor, victory, end_conditions) for the full rewrite scope. + +Triggering pain — this objective surfaced from p2-55f bullet 2 ("DataLoader → +CombatBalance JSON wiring at game start"): wiring is blocked because the +runtime has no persistent `GdGameState` (every call site instantiates a +fresh one and discards it after one Rust step). A clean `mc-state` crate +makes the per-site config-threading discipline obvious, and would let +`api-gdext` expose a single `mc_state::SimConfig` setter that all +ephemeral `GdGameState` instances pull from. + +## Acceptance + +- ❌ New crate `src/simulator/crates/mc-state/` with `Cargo.toml` listing only + data-shape deps (`mc-core`, `mc-city`, `mc-culture`, `mc-combat`, + `mc-economy`, `mc-trade`, `mc-tech`, `mc-replay`, `mc-observation`, + `serde`, `serde_json`). No `mc-ai`, `mc-turn`, GPU deps, rayon, etc. +- ❌ Pure data-only modules moved verbatim: + - `game_state.rs` (struct + impl blocks for `Default`, `PendingCaptureEvents`, + `GameState::is_empty`, `RallyCommand`) + - `combat_balance.rs` (config struct + `load_combat_balance`) + - The action-request structs (`AttackRequest`, `BombardRequest`, + `PillageRequest`, `VolleyRequest`, `ChargeRequest`, + `BuildingActionRequest`) move with `GameState` since they live on it +- ❌ `ScoringWeights` field on `GameState` resolved without re-introducing the + `mc-state → mc-ai` cycle. Two viable shapes: + 1. `ScoringWeights` moved to `mc-core` (config-shaped, no logic). + 2. `mc-state::GameState` carries an opaque `serde_json::Value` / + bincode blob and `mc-ai` re-hydrates at use site. + Option 1 is preferred (typed, no encoding tax). Decide in Phase 0. +- ❌ `mc-turn` becomes a logic-only crate that depends on `mc-state` for the + data shapes it mutates. `mc-turn` still owns: `processor.rs`, + `action_handlers/`, `combat_event.rs`, `end_conditions.rs`, `victory.rs`, + `capture.rs`, `courier_resolver.rs`, `ransom.rs`, `formation_move.rs`, + `patrol.rs`, `chronicle.rs`, `prologue.rs`, `policy.rs`, `gpu/`, + `bridge_contract_tests.rs`, `processor_invariants.rs`, + `building_action_handlers.rs`, `derived_stats_tests.rs`, + `improvement_tests.rs`, `spatial_index.rs`. +- ❌ All consumer crates' `Cargo.toml` files updated: + - Crates that *only* read state shape (e.g. `mc-replay`, + `mc-mcts-service::protocol`, `mc-sim::event_dispatch`, + `mc-core::derived_stats`) drop `mc-turn` from their dependency lists + and add `mc-state`. + - Crates that need both (e.g. `mc-ai`) keep `mc-turn` only if they invoke + turn-step machinery; otherwise downgrade to `mc-state`. +- ❌ All `use mc_turn::game_state::…` / `use mc_turn::{GameState, …}` import + sites rewritten to `use mc_state::…`. Verified via + `grep -rn "mc_turn::game_state\|mc_turn::GameState" src/` returning zero + hits. +- ❌ Workspace gates green: + - `cargo check --workspace` (with and without GPU feature) + - `cargo test -p mc-state` (round-trip serde tests for `GameState`, + `PendingCaptureEvents`, `CombatBalance`) + - `cargo test -p mc-turn` (existing test suite, byte-identical) + - `cargo test -p mc-ai` (parity preserved) + - `cargo build -p api-gdext` and `cargo build -p api-wasm` +- ❌ Save-format compatibility verified: at least one cycle-N save loaded + byte-identical post-extraction. (Serde shapes don't change; module + paths in `#[serde(...)]` strings should never have been there, but + audit just in case.) +- ❌ DataLoader → CombatBalance JSON wiring (p2-55f bullet 2) lands as a + follow-up using `mc_state::SimConfig`-style config singleton wired once + at `api-gdext` registration; closes the unblocking driver for this + objective. + +## Source-of-truth rails + +- **Rust crate**: `mc-state` owns data shapes; `mc-turn` owns mutation. + `mc-ai` reads `mc-state` shapes; never the reverse. No new cycles. +- **JSON path**: untouched. Save format is bit-identical pre/post. +- **`mc-core` newtypes**: untouched. State types continue to compose them. + +## Out of scope + +- Splitting `GameState` into smaller value-type aggregates (e.g. extracting + `EconomyState`, `MilitaryState`, `DiplomacyState` substructs). Worth a + follow-up after the crate move stabilises. +- Replacing the `Vec` slot with a `BTreeMap`. + Tracked in p1-44c-style migrations. +- Deleting `mc-turn::ransom::RANSOM_OFFER_DURATION_TURNS` const fallback — + closes once p2-55f bullet 2 lands on top of `mc-state`. + +## Phase plan + +### Phase 0 — Decide `ScoringWeights` ownership (1-2 hr) + +Read `mc-ai::evaluator::ScoringWeights` definition. If it's a data-only +config struct (likely), move to `mc-core::config` or `mc-balance`. If it +embeds AI heuristic logic, keep in `mc-ai` and use the opaque-blob +escape hatch on `GameState`. Update objective with the chosen route. + +### Phase 1 — Crate skeleton (1 hr) + +- Create `src/simulator/crates/mc-state/{Cargo.toml,src/lib.rs}`. +- Add to `[workspace.members]` in root `Cargo.toml`. +- Empty `lib.rs` declaring the future `pub mod` set (`game_state`, + `combat_balance`, `pending_capture_events`). +- `cargo check --workspace` green. + +### Phase 2 — Move `combat_balance.rs` (smallest unit, 30 min) + +Verify the move surface — only `mc-turn::processor` and `mc-turn::game_state` +depend on it today. Easy first move; validates the dep-graph plan before +the larger surgery. + +### Phase 3 — Move `game_state.rs` (~half-day) + +- `git mv mc-turn/src/game_state.rs mc-state/src/game_state.rs` +- Resolve `ScoringWeights` per Phase 0. +- Adjust `use` paths inside the moved file. +- `mc-turn/src/lib.rs` becomes a thin re-export shim + (`pub use mc_state::game_state::*;`) for one cycle to avoid touching + every consumer in the same patch. +- `cargo check --workspace` green; `cargo test -p mc-turn` green. + +### Phase 4 — Rewrite consumer imports (~1-2 hr mechanical) + +`grep -rln "use mc_turn::game_state\|use mc_turn::\\(GameState\\|...\\)"` → +sweep each file with `sd` / `sed`. Drop the re-export shim on the next +cycle once green. + +### Phase 5 — Cargo.toml dep-graph cleanup (1 hr) + +For each consumer crate's `Cargo.toml`: if its only `mc-turn` use-site was +state types, downgrade to `mc-state`. Verify by `cargo tree -p `. + +### Phase 6 — Verification gates (1 hr) + +- `cargo check --workspace` (default + `--all-features`). +- `cargo test -p mc-state` (new — serde round-trip tests). +- `cargo test -p mc-turn`, `cargo test -p mc-ai`, `cargo test -p mc-replay`. +- `cargo build -p api-gdext`, `cargo build -p api-wasm`. +- Apricot smoke: one 1-seed batch on the resulting `BUILD_REF` to verify + no runtime regression (defensive — should be byte-identical). + +### Phase 7 — Close p2-55f as the proof-of-value + +With `mc-state` in place, add `mc_state::SimConfig` carrying +`combat_balance` (and future global config), wire it once on `api-gdext` +extension registration, have `GdGameState::init` populate +`self.inner.combat_balance` from it. Closes p2-55f bullet 2 cleanly. + +## Risk register + +- **Save-format breakage**: serde shapes don't carry module paths, so the + on-disk JSON should be invariant. Confirm by round-tripping a real save. +- **Test parallelism + global config**: if Phase 7 uses a `OnceCell`, + cargo-test parallelism may fight it. Mitigate by giving each test its + own `GameState::with_combat_balance(cb)` constructor and reserving the + `OnceCell` for the api-gdext path only. +- **`ScoringWeights` cycle**: if Phase 0 finds it can't move cleanly, fall + back to the opaque-blob escape hatch (typed via a `#[serde(with = ...)]` + adapter — costs one bincode round-trip per `GameState` clone, but unblocks). +- **mc-turn re-export shim leaking**: must be deleted in Phase 4. Add a + `cargo deny`-style grep gate to prevent regression. + +## References + +- Triggering objective: `p2-55f` (combat_balance.json wiring) — surfaced + the no-persistent-`GdGameState` runtime constraint. +- Adjacent state-shape work: `p1-44c` (per-building production queues), + `p2-57a` (typed `ResourceStockpile`), `p2-46` (`TurnEventCollector`). +- Internal: `src/simulator/crates/mc-turn/src/game_state.rs:1-893` +- Cargo dep audit: `grep -rln "use mc_turn::\\(GameState\\|game_state\\|PlayerState\\|MapUnit\\|TechState\\|PendingCaptureEvents\\|...\\)" src/simulator/`