diff --git a/.project/objectives/objectives.json b/.project/objectives/objectives.json index 965010f6..8437ee4f 100644 --- a/.project/objectives/objectives.json +++ b/.project/objectives/objectives.json @@ -1,12 +1,12 @@ { - "generated_at": "2026-06-05T02:16:20Z", + "generated_at": "2026-06-05T02:59:51Z", "totals": { - "done": 242, + "done": 243, "in_progress": 1, "missing": 1, "oos": 29, - "partial": 20, - "stub": 10, + "partial": 21, + "stub": 8, "superseded": 4, "total": 307 }, @@ -2968,7 +2968,7 @@ "id": "p2-65", "title": "Extract `GameState` and pending-queue data types into a dedicated `mc-state` crate", "priority": "p2", - "status": "stub", + "status": "partial", "scope": "game1", "owner": "simulator-infra", "updated_at": "2026-06-04", @@ -3159,7 +3159,7 @@ "id": "p2-73-ui-theme-token-pipeline", "title": "UI theme pipeline — generate ui_theme.tres from design-tokens.json + apply globally", "priority": "p2", - "status": "stub", + "status": "done", "scope": "game1", "owner": "godot-engine", "updated_at": "2026-06-04", @@ -3586,10 +3586,6 @@ "owner": "asset-audio", "remaining": 1 }, - { - "owner": "godot-engine", - "remaining": 1 - }, { "owner": "godot-ui", "remaining": 1 diff --git a/.project/objectives/p2-65-extract-mc-state-crate.md b/.project/objectives/p2-65-extract-mc-state-crate.md index eea743f2..d73b069e 100644 --- a/.project/objectives/p2-65-extract-mc-state-crate.md +++ b/.project/objectives/p2-65-extract-mc-state-crate.md @@ -69,57 +69,93 @@ 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`) +- ✓ New crate `src/simulator/crates/mc-state/` with `Cargo.toml` listing only + data-shape deps (`mc-core`, `mc-city`, `mc-civics`, `mc-culture`, `mc-combat`, + `mc-trade`, `mc-tech`, `mc-comms`, `mc-replay`, `mc-observation`, + `serde`, `serde_json`). No `mc-ai`, `mc-turn`, GPU deps, rayon, etc. — verified + via `cargo tree -p mc-state -e normal` (11 mc-* shape crates + serde, no + mc-ai/mc-turn/rayon/GPU). Landed Phases 1/3a (commits `380619f89`, `9745f59e3`). +- ✓ Pure data-only modules moved verbatim (Phases 3a/3b, commits `9745f59e3`, + `6de3c67a8`): + - `game_state.rs` (struct + impl blocks; +`ransom`/`capture`/`combat_event`/ + `patrol` sibling shapes relocated to `mc-state`) - 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.) + `BuildingActionRequest`) moved with `GameState` + - **Deviation (honest footnote):** `combat_balance.rs` did NOT move to mc-state. + Phase 0a (commit pre-`6de3c67a8`) unified `CombatBalance` into + `mc_core::combat_balance` (8-field struct); `mc-turn/src/combat_balance.rs` + remains a thin re-export shim wrapping `load_combat_balance`. Decoupling + intent met via mc-core rather than mc-state — the bullet-as-written is + superseded by the Phase-0a resolution. +- ✓ `ScoringWeights` field on `GameState` resolved without re-introducing the + `mc-state → mc-ai` cycle. **Option 1 chosen** — `ScoringWeights` + the 4 + `mc_ai::tactical` typed fields (`TacticalMemory`, `BuildingPriors`, + `TacticalUnitSpec`, `TacticalBuildingSpec`) relocated to `mc-core` + (`scoring_weights` + `tactical_types`), mc-ai re-exports for compat. + Landed Phase 0c (commit `059fafb51`). mc-state carries no mc-ai dep. +- ✓ `mc-turn` becomes a logic-only crate that depends on `mc-state` for the + data shapes it mutates. `mc-turn` no longer OWNS any state data type + (`GameState`/`PlayerState`/`MapUnit`/action-requests/ransom/capture/patrol/ + combat-event shapes all live in mc-state); it retains turn-step logic only + (`processor.rs`, `action_handlers/`, `victory.rs`, `courier_resolver.rs`, + `formation_move.rs`, `chronicle.rs`, `gpu/`, the resolver free-fns, etc.). + Internals reach the shapes via the crate-private `pub(crate) use + mc_state::game_state;` alias (`mc-turn/src/lib.rs`). + **Honest footnote — the mc-ai dep is RETAINED and that is correct:** mc-turn + calls mc-ai *decision helpers* during turn resolution (8 sites — + `mc_ai::evaluator::ScoringWeights`, `mc_ai::abstract_state`, + `mc_ai::game_state::axes_to_flat`, `mc_ai::tactical::ransom_decision`, + `mc_ai::policy::PersonalityPriors`). That is logic dispatch ("turn-step asks + AI to decide"), not a state-data cycle. mc-state still carries no mc-ai dep, + so the data-ownership rail holds. Landed C2 (commit `e5d459703`). +- ✓ All consumer crates' `Cargo.toml` files updated (C1 add, C3 drop — commits + `8aff906fe`, `a6d52cf00`): + - Added `mc-state` to api-gdext, mc-player-api, mc-sim, mc-save, mc-vision, + tests/integration. + - Dropped `mc-turn` entirely from **mc-save**, **mc-vision**, and + **mc-mcts-service** (zero remaining `mc_turn::` usage of any kind — verified + by grep + `cargo build -p mc-mcts-service` green without it). + - api-gdext / mc-player-api / mc-sim KEEP `mc-turn` — they genuinely invoke + turn-step machinery (TurnProcessor, action_handlers, courier_resolver, + chronicle, VictoryConfig, combat_balance loader). + - mc-core never had a real mc-turn code dep (all references were doc comments, + retargeted to `mc_state::` in C1) — no dev-dep needed. +- ✓ All `use mc_turn::game_state::…` / `use mc_turn::{GameState, …}` import + sites rewritten to `use mc_state::…`, plus the inline expression-position + uses (`mc_turn::MapUnit::new(…)`, `mc_turn::PlayerState {…}`) and ALL + multi-line brace imports + nested `game_state::{…}` forms. The `mc-turn` + crate-root re-export (`lib.rs:63`) + the `game_state.rs` shim file are + DELETED. Verified: `grep -rn "mc_turn::game_state\|mc_turn::GameState" + src/simulator` → **0** (C2, commit `e5d459703`); completeness brace-grep + (single + multi-line) → 0. The only `crate::game_state` paths remaining are + mc-turn's own internals resolving through the `pub(crate)` alias — exempt. +- ✓ Workspace gates green (at C3 head `a6d52cf00`, apricot = RUN host): + - `cargo test --workspace --no-run` (default features) exit 0. + - `cargo test -p mc-state` 12/12 (serde round-trip for the moved shapes). + - `cargo test -p mc-turn --lib` 234/234 (1 ignored, pre-existing + `five_players_overflow`); `serde_roundtrip` 6/6; `full_turn_golden` 3/3. + - `cargo test -p mc-ai --lib` 268/268; `mc-player-api --lib` 126/126. + - `cargo check -p magic-civ-physics` (the api-wasm crate) exit 0 — it deps + only mc-core, unaffected by the extraction. + - **Honest footnote — `--features gpu`:** `cargo check -p mc-turn + --features gpu` fails with a PRE-EXISTING `CombatResult` field-drift error + in the gpu-gated test block (`gpu/mod.rs`), missing + `attacker_post_combat_heal`/`cleave_secondary_damage`/ + `new_defender_status_effects` (added to `mc_combat::resolver::CombatResult` + by prior combat work). This file was NOT touched by any Phase-4 commit + (`git log 243ea9c28..HEAD -- gpu/mod.rs` empty) and the error is unrelated + to the mc-state extraction; it fails on the baseline too. My + `crate::game_state::` paths in gpu/mod.rs resolve correctly. Tracked as + pre-existing gpu test-debt, not a Phase-4 regression. +- ✓ Save-format compatibility verified: `serde_roundtrip` 6/6 + + `full_turn_golden` 3/3 byte-identical pre/post the game_state move and the + shim deletion (serde shapes carry no module paths; the moves were verbatim). - ❌ 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. + objective. **STILL OPEN — this is Phase 7, the sole remaining bullet. See + the Phase-7 resume note below.** ## Source-of-truth rails @@ -533,3 +569,70 @@ the mc-turn/mc-ai/mc-player-api parity trio. Do NOT split `game_state.rs` (>500 flagged debt. Pre-existing unrelated failure `mc-turn::abstract_projection:: five_players_overflow_truncates_to_max_players` (MAX_PLAYERS 4→12 staleness) fails on HEAD, touches none of the moved types — NOT a regression. + +## 2026-06-04 Wave-D session 2 — Phase 4 + Phase 5 LANDED (K=8/9, status stays `partial`) + +**Green head: `a6d52cf00`.** Bullet 6 driven to grep-zero, both shims deleted, dead +mc-turn deps dropped. Only bullet 9 (Phase 7 SimConfig wiring) remains — see the +Phase-7 resume note below. Per objective-integrity (K=N for `done`), K=8/9 → `partial`. + +### Commits (each green-between; apricot = RUN host, cargo runs in CWD) + +| Chunk | Commit | What | +|---|---|---| +| C1 | `8aff906fe` | Add `mc-state` dep to 5 consumers (api-gdext, mc-player-api, mc-sim, mc-save, mc-vision); sweep 44 real `mc_turn::game_state::`/`mc_turn::GameState` code refs + 14 doc-comment refs → `mc_state::game_state::`. Both shims left in place (green boundary). | +| C2 | `e5d459703` | Delete `lib.rs:63` crate-root re-export; `pub mod game_state;` → `pub(crate) use mc_state::game_state;`; delete `mc-turn/src/game_state.rs` shim file. Split ALL remaining `use mc_turn::{…state…}` brace imports (single + multi-line + nested `game_state::{…}`) into mc-state + mc-turn halves across mc-turn integration tests, mc-sim (lib + 4 bins incl. solo_dominion), mc-player-api tests, tests/integration. Retarget mc-turn-internal bare `crate::{GameState,…}`/`crate::MoveRequest` → `crate::game_state::…`. Sweep inline `mc_turn::MapUnit::new()`/`PlayerState{}` sites. Add `mc-state` to tests/integration/Cargo.toml. | +| C3 | `a6d52cf00` | Drop dead `mc-turn` dep from mc-save, mc-vision, mc-mcts-service (zero `mc_turn::` usage; mc-mcts verified `cargo build` green without it). | + +### Evidence (at `a6d52cf00`) + +- Brief gate grep `mc_turn::game_state\|mc_turn::GameState` (excluding the lib.rs + explanatory comment, which no longer contains the token) → **0**. + Completeness brace-grep (single + multi-line, perl `-0777`) → **0**. +- `cargo test --workspace --no-run` (default features) exit 0. +- Save-format: `serde_roundtrip` 6/6, `full_turn_golden` 3/3 (byte-identical). +- Parity: mc-turn lib 234/234 (1 ignored, pre-existing `five_players_overflow`); + mc-ai 268/268; mc-player-api 126/126; mc-state 12/12; mc-save 5+5+1; + mc-vision 29/29 (1 ignored). +- `cargo check -p magic-civ-physics` (api-wasm) exit 0. +- **`--features gpu`:** PRE-EXISTING `CombatResult` field-drift error in + `gpu/mod.rs` test block (untouched by Phase 4 — `git log 243ea9c28..HEAD -- + gpu/mod.rs` empty; unrelated to mc-state). Not a regression. Tracked as gpu + test-debt. + +### Theme-lane firewall + +This session touched ONLY Rust (`src/simulator/**`) + this objective `.md` + +the objectives index. The concurrent design-system lane's files +(`ui_theme.tres`, `theme_assets.gd`, `project.godot`, `tools/build-ui-theme.py`, +`scenes/tests/ui_theme_proof.*`) were left modified/untracked and were NEVER +staged — verified at every commit. + +## Phase 7 resume note (the ONLY remaining bullet — start here next session) + +**Goal (bullet 9 / closes p2-55f bullet 2):** add `mc_state::SimConfig` carrying +`combat_balance` (8-field `mc_core::CombatBalance`) + room for future global +config. Wire it ONCE at api-gdext extension registration; have `GdGameState::init` +populate `self.inner.combat_balance` from it. Today every ephemeral `GdGameState` +instantiates a fresh default and discards `combat_balance` — this is the +no-persistent-`GdGameState` constraint that triggered the whole objective. + +**Steps:** +1. New `mc-state/src/config.rs`: `pub struct SimConfig { pub combat_balance: + mc_core::CombatBalance, … }` + `Default`. Re-export `mc_state::SimConfig`. +2. api-gdext registration (`lib.rs` `#[gdextension]` init or a `GdSimConfig` + singleton): load `combat_balance.json` via DataLoader once, build `SimConfig`. +3. `GdGameState::init` (or the `GameState` constructor it calls) reads the + singleton and sets `self.inner.combat_balance`. +4. **Risk (from §Risk register):** if the singleton is a `OnceCell`, cargo-test + parallelism may fight it. Mitigate by giving `GameState` a + `with_combat_balance(cb)` constructor for tests and reserving the `OnceCell` + for the api-gdext path only. + +**Gate:** the same trio (`serde_roundtrip` + `full_turn_golden` + parity) + +`cargo build -p api-gdext` + a check that a loaded `combat_balance.json` actually +reaches `state.combat_balance` (assert in a new api-gdext test). When green and +bullet 9 ✓ → flip status `partial` → `done` and regen the index. + +**Effort:** S–M, ~half a session. Discrete design work — do NOT bundle with other +objectives. It is the proof-of-value that retroactively justifies the crate split.