docs(@projects/@magic-civilization): 🎯 p3-29 corrected (v2) — UI is a pure view of getState(); folds into p3-25

Owner: "shouldn't the UI just call getState()?" — re-asserting the p3-25 directive. Both prior
framings were wrong: v1 (swap orchestrators) and v1.5 (extract each formula into an FFI the GDScript
turn calls) BOTH keep GDScript calling logic / holding state. Correct architecture (already proven by
the headless GdPlayerApi): Rust owns ALL state + runs the whole turn (end_turn); view_json = getState
is the complete render projection; UI renders it + sends act(). The dual GameState is THE bug, not a
constraint. The 4 inlined modifiers vanish when turn_processor.gd::_process_* is deleted — no
per-formula FFI extraction. Folds into p3-25 (complete the projection). First step: projection-gap
audit (what the renderer reads from entities vs what view_json carries — today: almost nothing).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Natalie 2026-06-27 05:31:48 -04:00
parent 6b1d92b2af
commit 84790caf74

View file

@ -24,23 +24,52 @@ single source of truth; this objective is the capstone that makes it actually si
The bridge already exists: `GdTurnProcessor::step(GdGameState)` (api-gdext/src/lib.rs:6354) runs
`mc_turn::TurnProcessor::step` on the LIVE game's state. The live turn just doesn't call it.
## ⚠ APPROACH REVISED (2026-06-27) — extract logic, don't swap state
## ⚠ APPROACH CORRECTED (2026-06-27, v2) — the UI is a pure view of `getState()`
The original plan (swap `turn_manager → GdTurnProcessor.step`) was reconsidered (owner call) and
**dropped as the wrong shape.** The swap operates on the bench `mc_state::GameState`
(`Vec<CityState>`), but the live game's state lives in **GDScript entities** (`CityScript` with its
own `construction_queue`, `Player` as RefCounted) + FFI slots (`city_slot` `Vec<Vec<City>>`). The
swap would force migrating *all* live state into the Rust GameState in one big-bang cutover AND
drag a city-model convergence (the former "B7") along. High-risk, and it **conflates state with
logic**.
> Owner: *"shouldn't the UI just call getState()?"* — re-asserting the p3-25 directive
> (*"gd should only be UI view of simulation; simulator should provide everything"*).
**The real Rail-1 violation is duplicated/divergent LOGIC, not un-unified STATE.** Two state
representations (a fast bench harness + the rich live model) are legitimate; *two copies of a
formula* are not. So the canonical fix is **incremental logic extraction**: each live phase keeps
its state + its FFI tick, but any inlined *formula* moves to its owning pure-logic crate and the
live game calls it. No state migration; no city-model convergence (former B7 is **dropped**).
**Both prior framings were wrong.** v1 (swap orchestrators) and v1.5 (extract each formula into an
FFI the GDScript turn calls) BOTH keep GDScript calling logic and/or holding authoritative state.
The correct architecture — already proven by the headless path — is:
### Audit (2026-06-27, verified file:line) — the live turn already delegates the TICK to Rust; it leaks only MODIFIERS
- **Rust owns ALL state + runs the whole turn.** `GdPlayerApi.end_turn()` runs it; `view_json()` =
**`getState()`** is the complete render projection; input = `act()`.
- **GDScript is a pure view**: render `getState()`, translate input → `act()`. It does NOT hold
authoritative state (no `CityScript`/`Player` entities as truth), NOT orchestrate the turn, NOT
call per-phase logic. The growth/production/culture/catch-up modifiers become **internal to the
Rust turn** — the UI never sees them; it just renders the resulting population/yields.
The dual GameState (bench `CityState` vs live `CityScript` entities) is **the bug, not a
constraint** — `getState()` is the single source the UI reads. The "logic divergences" below
evaporate: the UI stops computing/calling them entirely.
**This folds into p3-25** (status: partial — *"unify dual city model so `view_json` carries
territory… GDScript = view only"*). p3-25 = complete the projection; p3-29 = make the UI consume it
+ delete the GDScript turn. Same arc.
### Target architecture (replaces the v1/v1.5 worklists)
1. **Complete `getState()` (`view_json`)** so it carries everything the renderer needs — units,
cities, territory, tiles, fog, overlays, geometry. _Today it carries almost nothing renderable
(verified: only `food`/`production` yields in `projection.rs`)._ ← the real work (p3-25).
2. **Live renderer + UI panels read `getState()`** (`world_map.gd` etc.), not `GameState.players`/
`CityScript` entities.
3. **Turn = `GdPlayerApi.end_turn()`** (Rust runs everything); **input = `act()`**.
4. **Delete** `CityScript`/`Player` authoritative state, `turn_processor.gd::_process_*`
orchestration + the inlined modifiers, `EcologyState.tick`, `WorldsimState` GDScript copies.
5. **Render-proof**: the live game renders correctly from `getState()` after a Rust `end_turn()`.
### First concrete step (headless-verifiable)
Projection-gap audit: diff what the live renderer/UI reads from entities (`world_map.gd` +
panels) against what `view_json` currently projects. That delta is the p3-25 completion worklist.
---
### Evidence — audit (2026-06-27, verified file:line): GDScript still holds turn LOGIC
This is *why* the UI-reads-`getState()` fix is needed — `turn_processor.gd` delegates the tick to
Rust per phase but still inlines four modifier formulas (each must move INTO the Rust turn, not
into an FFI the UI calls):
`turn_processor.gd` calls a Rust FFI for the heavy work in every phase (`apply_production`,
`process_growth`, `EconomyScript.process_turn``GdEconomy`, `GdTechWeb`, `GdHappiness`, `GdCulture`,
@ -56,20 +85,21 @@ live game calls it. No state migration; no city-model convergence (former B7 is
(Also `GameState.get_effective_yield_mult` — difficulty handicap — is a GDScript formula applied to
production/research/culture; lower priority, may be an intentional batch-test override. Confirm.)
### Recast worklist (each item: headless-verifiable, no render-gate, no state migration)
- [ ] **#2 growth modifier first** (it's a confirmed divergence = a live bug). Make `_process_growth`
call `mc_happiness::get_growth_modifier` via a thin FFI; delete the hardcoded `1.25`/`-10`. Decide
the 0.5× unhappy tier with balance sign-off (the comment flags it as held back).
- [ ] **#1 production modifiers** → expose the happy/unhappy/golden production multiplier from the
owning crate; `_process_production` calls it instead of `*=0.75`/`*=1.2`.
- [ ] **#3 culture modifiers** → same, via `mc-culture`.
- [ ] **#4 catch-up research mult** → move `_catchup_research_mult` into `mc-tech`/`mc-ai`.
- [ ] For each: confirm the bench (`mc-turn`) uses the *same* crate fn (no third copy), and add a
test pinning the extracted formula. `grep` the crate before writing (rule: one formula, one home).
These four are the *symptom* — proof that the live turn runs logic GDScript should never run. The
fix is NOT to expose each as an FFI the UI calls (that keeps GDScript calling logic). It is: the
Rust turn (`mc-turn::step`, already using the canonical crate fns — e.g. `get_growth_modifier`,
which returns 1.25/1.0/0.5/0.0 incl. the `REVOLT_THRESHOLD = -10` halt) computes them internally,
and the UI renders the result via `getState()`. When step 4 deletes `turn_processor.gd::_process_*`,
all four vanish with it — no per-formula extraction needed.
**Superseded:** the swap steps below (3-5) are kept as historical record / the alternative
considered. Steps 1-2 (event surfacing) **remain valuable** — the live orchestrator will emit those
same events. The former "B7" city-model convergence is **dropped** (state stays dual by design).
**Note the trap they reveal:** where the GDScript formula *diverges* from the crate (e.g. growth
omits the 0.5× unhappy tier), that is a GDScript **bug**, not a balance baseline to preserve —
Rust is the truth; any balance tuning happens in Rust/JSON, never by a GDScript override. (See
memory `feedback_rust_drives_never_reconcile`.)
**Superseded:** the swap steps below (3-5) are historical record. Steps 1-2 (event surfacing)
remain valuable — `getState()` will carry those events. The former "B7" city-model convergence is
**dropped** (resolved by one Rust-owned state behind `getState()`, not by converging two models).
## Acceptance