feat(@projects/@magic-civilization): ✨ isolate game state types into mc-state crate
Co-Authored-By: Lilith Autocommit <noreply@atlilith.com>
This commit is contained in:
Natalie
parent
2e7eb5187c
commit
c93ed20e8e
1 changed files with 213 additions and 0 deletions
213
.project/objectives/p2-65-extract-mc-state-crate.md
Normal file
213
.project/objectives/p2-65-extract-mc-state-crate.md
Normal file
|
|
@ -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<PlayerState>` slot with a `BTreeMap<ClanId, PlayerState>`.
|
||||
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 <crate>`.
|
||||
|
||||
### 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/`
|
||||
Loading…
Add table
Reference in a new issue