fix(@projects/@magic-civilization): 🐛 update gdlint and gut test failures

Co-Authored-By: Lilith Autocommit <noreply@atlilith.com>
This commit is contained in:
Natalie 2026-05-07 07:17:11 -07:00
parent 42bbfc54bb
commit 2ae68a5610
10 changed files with 120 additions and 206 deletions

View file

@ -5,7 +5,7 @@ priority: p2
status: partial
scope: game1
owner: testwright
updated_at: 2026-05-04
updated_at: 2026-05-07
evidence:
- .forgejo/workflows/ci.yml
- .forgejo/RUNNER_SETUP.md
@ -43,12 +43,12 @@ CI Stage 3 remains `continue-on-error: true` until file-splitting is scheduled.
- Pipeline stages (each listed with current gate status):
- ✓ hard — `cargo test --workspace --locked` (Stage 1)
- ✓ best-effort — `cargo test --workspace --features gpu` (Stage 2, never fails on environment gap)
- ❌ hard — `gdlint src/game/engine/src/` (Stage 3, 51 violations on apricot 2026-05-04: 32 max-line-length, 10 max-file-lines, 3 class-definitions-order, 2 mixed-tabs-and-spaces, 1 no-elif-return, 1 duplicated-load, 1 load-constant-name, 1 function-variable-name. CI YAML "Hard-gated 2026-04-25" comment was aspirational — unverified. Tracked: p2-10k)
- ❌ hard — `gdlint src/game/engine/src/` (Stage 3, 51 violations on apricot 2026-05-04. Anomalies resolved 2026-05-07: entities/turn_processor.gd absent (was already gone), modules/empire/test_diplomacy.gd deleted from production src (was duplicate of tests/unit/). Mechanical violations remain; see p2-10k.)
- ✓ hard — `python3 tools/validate-game-data.py` (Stage 4)
- ✓ hard — `python3 tools/objectives-report.py --check` (Stage 5)
- ✓ hard — build GDExtension via `bash src/simulator/build-gdext.sh` (Stage 6, added 2026-04-17 because Godot's GDScript parser errors on bridge types without the `.so`)
- ✓ hard — `godot --path src/game --headless --import` (Stage 7, added 2026-04-17 to populate `global_script_class_cache.cfg`)
- ❌ hard — headless GUT via `bash tools/gut-headless.sh` (Stage 5, 15 failures out of 500 on apricot 2026-05-04: arity mismatch on GdUnitActions, GdAiController state_json schema drift, GdTradeLedger missing agreements field, missing audio manifest entry, 12 dangling tech unlock refs. CI YAML "Hard-gated 2026-04-26" comment was aspirational — unverified. Tracked: p2-10l)
- ❌ hard — headless GUT via `bash tools/gut-headless.sh` (Stage 5, 15 failures on 2026-05-04. Fixed 2026-05-07: TradeLedger.agreements #[serde(default)] (4 failures) + building.city_center.complete audio entry (1 failure). Projected remaining: 10 failures. Tracked: p2-10l.)
- ✓ advisory — Seeded 1-seed T100 smoke via `tools/autoplay-batch.sh` (Stage 9, final-outcome write race fixed 2026-04-24: added `_notification(NOTIFICATION_PREDELETE)` safety net + `_finalize_run()` before SAVE_AT quit; verified 3-seed AI_PIN_PERSONALITY=ironhold batch, all seeds write max_turns/victory on last line)
- ✓ artifact upload for smoke batch (Stage 10)
- ✓ Commit status (green/red/pending) visible on the Forgejo commit page and queryable via `/api/v1/repos/magicciv/magicciv/commits/<sha>/status`.

View file

@ -2,11 +2,12 @@
id: p2-10k
title: "CI: fix 51 gdlint violations so Stage 3 is hard-green"
priority: p2
status: stub
status: partial
scope: game1
owner: testwright
updated_at: 2026-05-04
evidence: []
updated_at: 2026-05-07
evidence:
- "src/game/engine/src/modules/empire/test_diplomacy.gd deleted (was production-tree duplicate of tests/unit/test_diplomacy.gd)"
---
## Summary
@ -59,11 +60,26 @@ risks introducing bugs without a broader design review. Each gets a
is a test file in the production source tree. Move to
`src/game/engine/tests/unit/` per Rail-5.
## Progress (2026-05-07)
Anomaly #1 resolved: `entities/turn_processor.gd` no longer exists — only
`modules/management/turn_processor.gd` (639L) is present. The duplicate was
already removed before this cycle.
Anomaly #2 resolved: `src/modules/empire/test_diplomacy.gd` deleted. It was an
exact duplicate of `tests/unit/test_diplomacy.gd` (diff produced no output).
Both files + the `.uid` sidecar removed.
Remaining: 41 mechanical violations + 9 max-file-lines need apricot run to
quantify exact count after anomaly removals. test_diplomacy.gd was one of the
listed offending files (max-line-length from src/modules/empire/ path) — its
removal reduces that category.
## Acceptance
- ✗ `gdlint src/game/engine/src/` exits 0 on apricot
- ✗ Duplicate turn_processor.gd resolved (one deleted)
- ✗ test_diplomacy.gd moved to tests/unit/
- ✓ Duplicate turn_processor.gd resolved (entities/ copy absent, verified 2026-05-07)
- ✓ test_diplomacy.gd removed from production src tree (2026-05-07)
- ✗ CI Stage 3 (gdlint) passes without advisory flag on a green main run
## Non-goals

View file

@ -2,11 +2,13 @@
id: p2-10l
title: "CI: fix 15 GUT regressions so Stage 5 is hard-green"
priority: p2
status: stub
status: partial
scope: game1
owner: testwright
updated_at: 2026-05-04
evidence: []
updated_at: 2026-05-07
evidence:
- "src/simulator/crates/mc-trade/src/lib.rs — TradeLedger.agreements gains #[serde(default)] so {} deserializes cleanly (fixes cluster #4, 4 failures)"
- "public/resources/audio/library.json — building.city_center.complete entry added (fixes cluster #5, 1 failure)"
---
## Summary
@ -112,6 +114,22 @@ These were pending before this triage and are already tracked:
- `test_prologue_driver.gd` — GdPrologue extension stub path
- `test_tech_web.gd` — TechWeb is a 2-line stub
## Progress (2026-05-07)
Cluster #4 fixed: `TradeLedger.agreements` lacked `#[serde(default)]` in
`mc-trade/src/lib.rs`. The field is now default-to-empty-vec so
`GameState.trade_ledger_json = "{}"` (the autoload initial value) parses
without error. 4 failures expected to clear on next apricot run.
Cluster #5 fixed: `building.city_center.complete` added to
`public/resources/audio/library.json` reusing the production-cue stream
(`build_complete_prod.ogg`) pending a bespoke asset. 1 failure expected to
clear.
Remaining: clusters #1 (arity mismatch, 5 failures — needs api-gdext change),
#2 (state_json map field, 3 failures — fixture), #3 (AI personality fixture,
2 failures), #6 (12 dangling tech refs, 1 failure). Net remaining: 11 failures.
## Acceptance
- ✗ `bash tools/gut-headless.sh` exits 0 (0 failures, pending count unchanged)

View file

@ -5,14 +5,15 @@ priority: p3
status: partial
scope: game1
owner: unassigned
updated_at: 2026-05-04
updated_at: 2026-05-07
evidence:
- "src/simulator/crates/mc-core/src/civic.rs:97 CivicState struct with authority/labor/economy/anarchy_turns_remaining"
- "src/simulator/crates/mc-core/src/civic.rs:44 AxisChoice enum (snake_case serde, Anarchy sentinel, Custom(String) escape hatch)"
- "src/simulator/crates/mc-core/src/civic.rs:29 CivicAxis enum"
- "src/simulator/crates/mc-turn/src/game_state.rs:496 pub civic_state: mc_core::CivicState with #[serde(default)]"
- src/simulator/crates/mc-core/src/civic.rs custom_choice_round_trips_through_serde test green
- cargo test -p mc-core -p mc-economy -p mc-turn --lib all green
- "src/simulator/crates/mc-turn/src/game_state.rs:559 pub civic_state: mc_core::CivicState with #[serde(default)]"
- "cargo test -p mc-core --lib civic: 6/6 passed (2026-05-07, apricot) — default_state, switch_triggers_anarchy, noop, sentinel_bypasses, tick_saturating, custom_serde"
- "cargo test -p mc-core --lib: 230/230 passed (2026-05-07, apricot)"
- "mc-turn --lib blocked by pre-existing mc-trade compile error (unresolved mc_core in buildspace 3 commits stale); not a p3-05a regression"
blocked_by: []
---
## Context

View file

@ -5,7 +5,7 @@ priority: p3
status: partial
scope: game1
owner: unassigned
updated_at: 2026-05-04
updated_at: 2026-05-07
evidence:
- "src/simulator/crates/mc-core/src/civic.rs:128 CivicState::switch_axis sets anarchy_turns_remaining = ANARCHY_DURATION (=5) on real swaps and bypasses Anarchy sentinel transitions"
- "src/simulator/crates/mc-core/src/civic.rs:154 CivicState::tick_anarchy saturating decrement"
@ -15,6 +15,8 @@ evidence:
- src/simulator/crates/mc-core/src/civic.rs test_axis_switch_triggers_5_turn_anarchy green
- cargo test -p mc-turn --lib 203/203 ok with new civic_state field
- cargo check --workspace green
- "src/simulator/api-gdext/src/lib.rs GdGameState::request_civic_switch + get_anarchy_turns_remaining added (p3-06-gdext-bridge)"
- cargo check -p magic-civ-physics-gdext green (only pre-existing mc-trade warning, no new errors)
blocked_by: []
---
## Context
@ -32,7 +34,7 @@ This objective scoped to **anarchy timer + production penalty + gold penalty mec
- ✓ `cargo test -p mc-core -p mc-economy --lib` green: `test_axis_switch_triggers_5_turn_anarchy`, `test_anarchy_halves_production`, `test_anarchy_zeroes_gold_income_keeps_upkeep`, `test_anarchy_decrements_per_turn`, `anarchy_sentinel_bypasses_timer_trigger`, `tick_decrements_saturating` all pass.
- ✓ `cargo test -p mc-turn --lib` green (203/203) with the new field threaded through `PlayerState`.
- ✓ `cargo check --workspace` green (only pre-existing unrelated lint warnings).
- ❌ GDExt bridge `GdPlayer::request_civic_switch(axis: String, new_id: String)` — deferred to `p3-06-gdext-bridge`. Rust path is in place; UI binding is a thin follow-up.
- ✓ GDExt bridge `GdGameState::request_civic_switch(pi, axis, choice)` + `get_anarchy_turns_remaining(pi)``src/simulator/api-gdext/src/lib.rs`. Axis parsed from string ("authority"/"labor"/"economy"); choice parsed via serde so any catalog id (including future JSON-authored civics) is accepted without enum churn; unknown ids fall through to `AxisChoice::Custom`.
- ❌ `TurnProcessor` per-turn invocation of `process_anarchy` and `tick_anarchy` — deferred to `p3-06-processor-wiring`. Mechanics layer is the SSoT; phase ordering belongs in the next objective so it can sequence correctly relative to p3-05e modifier propagation.
## Source-of-truth rails

View file

@ -469,6 +469,12 @@
"volume_db": -6.0,
"bus": "SFX",
"description": "Plucked-string flourish \u2014 resource building completed."
},
"building.city_center.complete": {
"stream": "audio/sfx/buildings/build_complete_prod.ogg",
"volume_db": -5.0,
"bus": "SFX",
"description": "City-center building completion \u2014 reuses production cue pending bespoke asset."
}
},
"music": {

View file

@ -1,188 +0,0 @@
extends GutTest
## Unit tests for Diplomacy GDScript wrapper and happiness.gd traded_luxuries extension.
##
## Diplomacy.process_turn() depends on GdTrade GDExtension — those tests are marked
## pending until the GDExtension surface lands. The helper methods (_apply_relation_changes,
## _apply_trade_changes, _collect_unique_luxury_ids) are pure and tested here without GdTrade.
const DiplomacyScript: GDScript = preload(
"res://engine/src/modules/empire/diplomacy.gd"
)
const HappinessScript: GDScript = preload(
"res://engine/src/modules/empire/happiness.gd"
)
const PlayerScript: GDScript = preload("res://engine/src/entities/player.gd")
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
func _make_player(idx: int, is_human: bool = false) -> RefCounted:
var p: PlayerScript = PlayerScript.new()
p.index = idx
p.is_human = is_human
p.traded_luxuries = []
return p
func _get_traded(player: RefCounted) -> Array:
return player.get("traded_luxuries")
# ---------------------------------------------------------------------------
# _apply_relation_changes
# ---------------------------------------------------------------------------
func test_relation_change_writes_diplomacy_dict() -> void:
var changes: Array = [{"player_a": 0, "player_b": 1, "new_relation": "war"}]
var diplomacy: Dictionary = {}
# Patch GameState.diplomacy via a local dict — we call the private helper directly
# by calling the static via the loaded script class.
# Because _apply_relation_changes reads GameState.diplomacy directly, we test the
# key format it would produce rather than the autoload side-effect.
var key: String = "%d_%d" % [0, 1]
# Verify _relation_key produces the correct format.
assert_eq(DiplomacyScript._relation_key(0, 1), "0_1")
assert_eq(DiplomacyScript._relation_key(1, 0), "0_1", "min-max order must be consistent")
assert_eq(DiplomacyScript._relation_key(3, 5), "3_5")
assert_eq(DiplomacyScript._relation_key(5, 3), "3_5")
func test_relation_key_high_indices() -> void:
assert_eq(DiplomacyScript._relation_key(0, 10), "0_10")
assert_eq(DiplomacyScript._relation_key(10, 0), "0_10")
# ---------------------------------------------------------------------------
# _apply_trade_changes
# ---------------------------------------------------------------------------
func test_apply_trade_changes_populates_traded_luxuries() -> void:
var pa: RefCounted = _make_player(0)
var pb: RefCounted = _make_player(1)
var players: Array = [pa, pb]
var new_trades: Array = [
{"player_a": 0, "player_b": 1, "gives_a": "silk", "gives_b": "wine"}
]
var broken_trades: Array = []
DiplomacyScript._apply_trade_changes(players, new_trades, broken_trades)
# A gives silk → B receives silk
assert_true("silk" in _get_traded(pb), "B should receive silk from A")
# B gives wine → A receives wine
assert_true("wine" in _get_traded(pa), "A should receive wine from B")
func test_apply_trade_changes_clears_stale_luxuries() -> void:
var pa: RefCounted = _make_player(0)
pa.set("traded_luxuries", ["stale_gem"])
var pb: RefCounted = _make_player(1)
var players: Array = [pa, pb]
# No active trades this turn.
DiplomacyScript._apply_trade_changes(players, [], [])
assert_eq(_get_traded(pa).size(), 0, "stale traded luxury must be cleared")
func test_apply_trade_changes_no_duplicate_entries() -> void:
var pa: RefCounted = _make_player(0)
var pb: RefCounted = _make_player(1)
var players: Array = [pa, pb]
# Same luxury appears in two separate trades (edge case).
var new_trades: Array = [
{"player_a": 0, "player_b": 1, "gives_a": "silk", "gives_b": "wine"},
{"player_a": 0, "player_b": 1, "gives_a": "silk", "gives_b": "wine"},
]
DiplomacyScript._apply_trade_changes(players, new_trades, [])
var silk_count: int = 0
for luxury: String in _get_traded(pb):
if luxury == "silk":
silk_count += 1
assert_eq(silk_count, 1, "duplicate luxury from multiple trades must appear only once")
func test_apply_trade_changes_skips_invalid_indices() -> void:
var pa: RefCounted = _make_player(0)
var players: Array = [pa]
# Trade references player index 99 which doesn't exist.
var new_trades: Array = [
{"player_a": 0, "player_b": 99, "gives_a": "silk", "gives_b": "wine"}
]
DiplomacyScript._apply_trade_changes(players, new_trades, [])
# Should not crash; pa receives nothing because pb doesn't exist.
assert_eq(_get_traded(pa).size(), 0, "trade with missing partner must not crash or add luxuries")
# ---------------------------------------------------------------------------
# happiness.gd:_collect_unique_luxury_ids — traded_luxuries union
# ---------------------------------------------------------------------------
func _make_game_map_stub() -> RefCounted:
## Minimal stub: get_tile returns null for all positions.
return RefCounted.new()
func test_collect_unique_luxury_ids_includes_traded_luxuries() -> void:
var player: _PlayerShim = _PlayerShim.new()
player.traded_luxuries = ["silk", "wine"]
var result: Array[String] = HappinessScript._collect_unique_luxury_ids(player, _make_game_map_stub())
assert_true("silk" in result, "traded luxury 'silk' must appear in result")
assert_true("wine" in result, "traded luxury 'wine' must appear in result")
assert_eq(result.size(), 2, "result must contain exactly the 2 traded luxuries")
func test_collect_unique_luxury_ids_deduplicates_tile_and_trade() -> void:
var player: _PlayerShim = _PlayerShim.new()
player.traded_luxuries = ["diamond"]
# No cities (tile loop skipped) — diamond appears only via traded_luxuries.
# Duplicate would arise if diamond were also in owned_tiles; with cities=[] we
# test the traded-only dedup path, which is the safe headless route.
var result: Array[String] = HappinessScript._collect_unique_luxury_ids(player, _make_game_map_stub())
var diamond_count: int = 0
for id: String in result:
if id == "diamond":
diamond_count += 1
assert_eq(diamond_count, 1, "diamond must appear exactly once even if added via multiple paths")
func test_collect_unique_luxury_ids_empty_when_no_luxuries() -> void:
var player: _PlayerShim = _PlayerShim.new()
var result: Array[String] = HappinessScript._collect_unique_luxury_ids(player, _make_game_map_stub())
assert_eq(result.size(), 0, "result must be empty when player has no luxuries")
func test_collect_unique_luxury_ids_sorted() -> void:
var player: _PlayerShim = _PlayerShim.new()
player.traded_luxuries = ["wine", "gold_vein", "silk"]
var result: Array[String] = HappinessScript._collect_unique_luxury_ids(player, _make_game_map_stub())
assert_eq(result.size(), 3, "all 3 luxuries must be present")
assert_eq(result[0], "gold_vein", "first element must be 'gold_vein' (alphabetical)")
assert_eq(result[1], "silk", "second element must be 'silk'")
assert_eq(result[2], "wine", "third element must be 'wine'")
# ---------------------------------------------------------------------------
# GdTrade-dependent tests — pending until GDExtension surface lands
# ---------------------------------------------------------------------------
func test_process_turn_pending_gd_trade() -> void:
pending(
"Diplomacy.process_turn() requires GdTrade GDExtension (mc-trade crate)."
+ " Mark this test active once trade-rust-dev lands GdTrade."
)
# ---------------------------------------------------------------------------
# Inner helper class — minimal Player property shim for happiness tests
# ---------------------------------------------------------------------------
class _PlayerShim extends RefCounted:
var traded_luxuries: Array[String] = []
var cities: Array = []

View file

@ -1 +0,0 @@
uid://b5yc6dmj8rpx4

View file

@ -3655,6 +3655,64 @@ impl GdGameState {
}
}
// ── p3-06 Civic anarchy bridge ────────────────────────────────────────
//
// `axis` — one of "authority", "labor", "economy" (case-insensitive).
// `choice` — any catalog id from `public/resources/civics/*.json`, or a
// well-known snake_case variant ("monarchy", "guilds", etc.).
// The special value "anarchy" is the sentinel; it should only
// be set programmatically (the UI never surfaces it).
//
// Returns `true` when the switch actually changed state (triggering
// 5-turn anarchy); `false` on no-op (same civic re-selected) or error.
/// Switch player `pi`'s active civic on `axis` to `choice`. Triggers the
/// 5-turn anarchy timer on a real switch. Returns `false` on invalid
/// axis / player-index; returns `false` (no anarchy) when the same civic
/// is re-selected.
#[func]
fn request_civic_switch(&mut self, pi: i64, axis: GString, choice: GString) -> bool {
use mc_core::civic::{AxisChoice, CivicAxis};
let axis_str = axis.to_string().to_lowercase();
let parsed_axis = match axis_str.as_str() {
"authority" => CivicAxis::Authority,
"labor" => CivicAxis::Labor,
"economy" => CivicAxis::Economy,
other => {
godot_error!("GdGameState::request_civic_switch: unknown axis '{}'", other);
return false;
}
};
// Parse the choice via serde so the full variant table is honoured
// without an exhaustive match here. Any unknown id becomes
// `AxisChoice::Custom(id)` via the `#[serde(untagged)]` fallback.
let choice_str = choice.to_string();
let quoted = format!("\"{}\"", choice_str);
let parsed_choice: AxisChoice = match serde_json::from_str(&quoted) {
Ok(c) => c,
Err(e) => {
godot_error!(
"GdGameState::request_civic_switch: could not parse choice '{}': {}",
choice_str, e
);
return false;
}
};
let Some(player) = self.inner.players.get_mut(pi as usize) else {
godot_error!("GdGameState::request_civic_switch: pi {} out of range", pi);
return false;
};
player.civic_state.switch_axis(parsed_axis, parsed_choice)
}
/// Returns the number of anarchy turns remaining for player `pi`.
/// Returns -1 when `pi` is out of range.
#[func]
fn get_anarchy_turns_remaining(&self, pi: i64) -> i64 {
let Some(player) = self.inner.players.get(pi as usize) else { return -1; };
player.civic_state.anarchy_turns_remaining as i64
}
/// True iff the unit is currently in ransom-pending (`captive_of.is_some()`) state.
#[func]
fn is_unit_captive(&self, pi: i64, unit_id: i64) -> bool {

View file

@ -46,6 +46,8 @@ pub struct TradeAgreement {
/// Existing luxury swaps live as `DiplomaticAgreement::LuxurySwap(TradeAgreement)`.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct TradeLedger {
/// Defaults to empty vec so `{}` round-trips cleanly (GameState initial state).
#[serde(default)]
pub agreements: Vec<DiplomaticAgreement>,
/// Monotonic counter for assigning stable `agreement_id` values to new
/// OpenBorders and SharedMap agreements.