applications/magicciv
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(@projects/@magic-civilization): 🧹 p3-18 — remove dead Civ-V explicit-embark model (single source)

Browse source 
With Civ-VI auto-embark live (move onto water → embarked), the half-built Civ-V
explicit path was a redundant second model — and dead besides (gated on the
`amphibious` keyword, which no unit carries). Per owner direction (unify on one
model, no two definitions), it is removed:

- mc-core: drop ActionKind::Embark/Disembark (+ idx/as_str/from_str), the four
  embark DisabledReasons, the amphibious Embark/Disembark action-gen block, and
  the now-dead UnitCapability is_embarked/adjacent_water/adjacent_land fields
  (+ all fixtures and the amphibious action tests).
- mc-turn: drop handle_embark/handle_disembark + their dispatch arms + tests.
- mc-ai: drop the dead capability fields from the action-query construction.
- mc-state: MapUnit::is_embarked doc now describes auto-embark (its sole writer
  is process_one_move).
- api-gdext: the legal_actions_for/can_invoke FFI embark inputs are now vestigial
  (no longer affect legality); kept for ABI stability + flagged for the P5
  GDScript-mirror trim.

The only embark model is now the data-driven auto-embark (P1b/P2/P3).

Tests: mc-core 263, mc-ai 287, mc-turn 244 green (−4 = the deleted explicit-embark
tests). gdextension-api full compile deferred to the build host (local cargo
resolver panics on a pre-existing thiserror/wasm feature-unification issue,
unrelated to this change — no Cargo.toml touched).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Natalie 2026-06-25 05:17:48 -04:00
parent efff773c97
commit dabcc6716a
5 changed files with 25 additions and 265 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
src/simulator/api-gdext/src/action.rs
View file

@ -6,6 +6,12 @@
//! posture)` →
//! `Array[Dictionary]` with `{kind, enabled, disabled_reason}` entries
//!
//! NOTE (p3-18): `is_embarked` / `adjacent_water` / `adjacent_land` are
//! vestigial — embarkation is now automatic (Civ-VI: move onto water →
//! embarked) with no explicit Embark/Disembark action, so these inputs no
//! longer affect legality. They remain in the signature for ABI stability and
//! are slated for removal in the P5 GDScript-mirror pass.
//!
//! `posture` is a Dictionary carrying archetype state fields (all optional,
//! defaulting to false/0 if absent):
//! { is_aiming, is_fire_arrows, is_pursuing, is_shield_wall, is_braced,
@ -60,6 +66,11 @@ impl GdUnitActions {
adjacent_land: bool,
posture: Dictionary,
) -> Array<Dictionary> {
// p3-18 — embarkation is auto (Civ-VI): there is no explicit Embark
// action, so these legacy embark inputs no longer affect action
// legality. Accepted for ABI stability; the bridge + GDScript callers
// shed them in the p3-18 P5 GDScript-mirror pass.
let _ = (is_embarked, adjacent_water, adjacent_land);
let cap = UnitCapability {
unit_type: unit_type.to_string(),
keywords: keywords
@ -73,9 +84,6 @@ impl GdUnitActions {
is_patrolling: false,
is_sentrying,
is_deployed,
is_embarked,
adjacent_water,
adjacent_land,
is_aiming: bool_from_dict(&posture, "is_aiming"),
is_fire_arrows: bool_from_dict(&posture, "is_fire_arrows"),
is_pursuing: bool_from_dict(&posture, "is_pursuing"),
@ -124,6 +132,9 @@ impl GdUnitActions {
let Some(action_kind) = ActionKind::from_str(&kind_str) else {
return false;
};
// p3-18 — auto-embark: legacy embark inputs no longer affect legality
// (see legal_actions_for). Kept for ABI stability; trimmed in P5.
let _ = (is_embarked, adjacent_water, adjacent_land);
let cap = UnitCapability {
unit_type: unit_type.to_string(),
keywords: keywords
@ -137,9 +148,6 @@ impl GdUnitActions {
is_patrolling: false,
is_sentrying,
is_deployed,
is_embarked,
adjacent_water,
adjacent_land,
is_aiming: bool_from_dict(&posture, "is_aiming"),
is_fire_arrows: bool_from_dict(&posture, "is_fire_arrows"),
is_pursuing: bool_from_dict(&posture, "is_pursuing"),

3
src/simulator/crates/mc-ai/src/tactical/movement.rs
View file

@ -235,9 +235,6 @@ fn non_motion_macro(unit: &TacticalUnit) -> Option<Action> {
is_patrolling: unit.patrol_order.is_some(),
is_sentrying: false,
is_deployed: unit.is_deployed,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,

147
src/simulator/crates/mc-core/src/action.rs
View file

@ -29,8 +29,6 @@ pub enum ActionKind {
PackSiege,
DeploySiege,
Bombard,
Embark,
Disembark,
// Formation actions — parameters live in request structs queued on GameState.
SetRallyPoint,
ClearRallyPoint,
@ -142,8 +140,6 @@ impl ActionKind {
ActionKind::IssuePatrol => 14,
ActionKind::CancelPatrol => 15,
ActionKind::EditPatrol => 16,
ActionKind::Embark => 12,
ActionKind::Disembark => 13,
ActionKind::SetRallyPoint => 17,
ActionKind::ClearRallyPoint => 18,
ActionKind::CommandFormation => 19,
@ -216,8 +212,6 @@ impl ActionKind {
ActionKind::IssuePatrol => "issue_patrol",
ActionKind::CancelPatrol => "cancel_patrol",
ActionKind::EditPatrol => "edit_patrol",
ActionKind::Embark => "embark",
ActionKind::Disembark => "disembark",
ActionKind::SetRallyPoint => "set_rally_point",
ActionKind::ClearRallyPoint => "clear_rally_point",
ActionKind::CommandFormation => "command_formation",
@ -279,8 +273,6 @@ impl ActionKind {
"issue_patrol" => Some(ActionKind::IssuePatrol),
"cancel_patrol" => Some(ActionKind::CancelPatrol),
"edit_patrol" => Some(ActionKind::EditPatrol),
"embark" => Some(ActionKind::Embark),
"disembark" => Some(ActionKind::Disembark),
"set_rally_point" => Some(ActionKind::SetRallyPoint),
"clear_rally_point" => Some(ActionKind::ClearRallyPoint),
"command_formation" => Some(ActionKind::CommandFormation),
@ -346,10 +338,6 @@ pub enum DisabledReason {
NotSentrying,
AlreadyDeployed,
NotDeployed,
AlreadyEmbarked,
NotEmbarked,
NoAdjacentWater,
NoAdjacentLand,
// p2-53g
NotRanged,
AlreadyAiming,
@ -410,10 +398,6 @@ impl DisabledReason {
DisabledReason::NotSentrying => "disabled_reason_not_sentrying",
DisabledReason::AlreadyDeployed => "disabled_reason_already_deployed",
DisabledReason::NotDeployed => "disabled_reason_not_deployed",
DisabledReason::AlreadyEmbarked => "disabled_reason_already_embarked",
DisabledReason::NotEmbarked => "disabled_reason_not_embarked",
DisabledReason::NoAdjacentWater => "disabled_reason_no_adjacent_water",
DisabledReason::NoAdjacentLand => "disabled_reason_no_adjacent_land",
DisabledReason::NotRanged => "disabled_reason_not_ranged",
DisabledReason::AlreadyAiming => "disabled_reason_already_aiming",
DisabledReason::AlreadyFireArrows => "disabled_reason_already_fire_arrows",
@ -497,14 +481,6 @@ pub struct UnitCapability {
pub is_sentrying: bool,
/// True if a siege unit is in deployed posture (cannot move, can Bombard).
pub is_deployed: bool,
/// True if an amphibious unit is currently on a water tile.
pub is_embarked: bool,
/// True if the unit's current tile has at least one adjacent water hex.
/// Used by the Embark gate; supplied by the bridge from grid lookup.
pub adjacent_water: bool,
/// True if the unit's current tile has at least one adjacent land hex.
/// Used by the Disembark gate; supplied by the bridge from grid lookup.
pub adjacent_land: bool,
// p2-53g ranged posture state
/// True if the unit is currently aiming (AimedShot pending — fires on next attack).
pub is_aiming: bool,
@ -732,28 +708,9 @@ pub fn legal_actions(capability: &UnitCapability) -> Vec<ActionAvailability> {
});
}
// Amphibious — embark/disembark for shore-crossing units
if capability.keywords.iter().any(|k| k == "amphibious") {
let is_embarked = capability.is_embarked;
// Embark: requires !embarked + adjacent water
out.push(if is_embarked {
ActionAvailability::disabled(ActionKind::Embark, DisabledReason::AlreadyEmbarked)
} else if capability.adjacent_water {
ActionAvailability::enabled(ActionKind::Embark)
} else {
ActionAvailability::disabled(ActionKind::Embark, DisabledReason::NoAdjacentWater)
});
// Disembark: requires embarked + adjacent land
out.push(if !is_embarked {
ActionAvailability::disabled(ActionKind::Disembark, DisabledReason::NotEmbarked)
} else if capability.adjacent_land {
ActionAvailability::enabled(ActionKind::Disembark)
} else {
ActionAvailability::disabled(ActionKind::Disembark, DisabledReason::NoAdjacentLand)
});
}
// p3-18 — embarkation is auto (Civ-VI): a land unit that moves onto water
// embarks, onto land disembarks (see mc-turn process_one_move). There is no
// explicit Embark/Disembark action, so none is generated here.
// p2-53g: ranged-specific actions (gate: "ranged" keyword)
let is_ranged_unit = capability.keywords.iter().any(|k| k == "ranged");
@ -1070,9 +1027,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
@ -1097,9 +1051,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
@ -1162,9 +1113,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
@ -1232,9 +1180,6 @@ mod tests {
is_patrolling: false,
is_sentrying: true,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
@ -1331,36 +1276,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
is_shield_wall: false,
is_braced: false,
rage_turns_remaining: 0,
war_cry_used_this_battle: false,
multi_turn_in_progress: false,
is_stealthed: false,
is_field_aura: false,
is_ambushing: false,
is_escorted: false,
}
}
fn amphibious_cap(is_embarked: bool, adjacent_water: bool, adjacent_land: bool) -> UnitCapability {
UnitCapability {
unit_type: "melee".into(),
keywords: vec!["amphibious".into()],
has_movement: true,
is_fortified: false,
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked,
adjacent_water,
adjacent_land,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
@ -1415,41 +1330,6 @@ mod tests {
assert_eq!(attack.disabled_reason, Some(DisabledReason::AlreadyDeployed));
}
#[test]
fn amphibious_on_land_with_adjacent_water_can_embark() {
let actions = legal_actions(&amphibious_cap(false, true, false));
let embark = actions.iter().find(|a| a.kind == ActionKind::Embark).unwrap();
assert!(embark.enabled, "amphibious on land near water can embark");
let disembark = actions.iter().find(|a| a.kind == ActionKind::Disembark).unwrap();
assert!(!disembark.enabled, "not embarked means cannot disembark");
assert_eq!(disembark.disabled_reason, Some(DisabledReason::NotEmbarked));
}
#[test]
fn amphibious_on_land_no_adjacent_water_cannot_embark() {
let actions = legal_actions(&amphibious_cap(false, false, false));
let embark = actions.iter().find(|a| a.kind == ActionKind::Embark).unwrap();
assert!(!embark.enabled);
assert_eq!(embark.disabled_reason, Some(DisabledReason::NoAdjacentWater));
}
#[test]
fn amphibious_embarked_with_adjacent_land_can_disembark() {
let actions = legal_actions(&amphibious_cap(true, false, true));
let disembark = actions.iter().find(|a| a.kind == ActionKind::Disembark).unwrap();
assert!(disembark.enabled, "embarked unit next to land can disembark");
let embark = actions.iter().find(|a| a.kind == ActionKind::Embark).unwrap();
assert!(!embark.enabled, "already embarked");
assert_eq!(embark.disabled_reason, Some(DisabledReason::AlreadyEmbarked));
}
#[test]
fn amphibious_embarked_no_adjacent_land_cannot_disembark() {
let actions = legal_actions(&amphibious_cap(true, false, false));
let disembark = actions.iter().find(|a| a.kind == ActionKind::Disembark).unwrap();
assert!(!disembark.enabled);
assert_eq!(disembark.disabled_reason, Some(DisabledReason::NoAdjacentLand));
}
// p2-53g: ranged-specific action tests
fn ranged_cap_base(has_movement: bool) -> UnitCapability {
@ -1461,9 +1341,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
@ -1529,9 +1406,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing,
@ -1592,9 +1466,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
@ -1619,9 +1490,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
@ -1740,9 +1608,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
@ -1767,9 +1632,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,
@ -1794,9 +1656,6 @@ mod tests {
is_patrolling: false,
is_sentrying: false,
is_deployed: false,
is_embarked: false,
adjacent_water: false,
adjacent_land: false,
is_aiming: false,
is_fire_arrows: false,
is_pursuing: false,

7
src/simulator/crates/mc-state/src/game_state.rs
View file

@ -1163,8 +1163,11 @@ pub struct MapUnit {
/// `handle_deploy_siege`, cleared by `handle_pack_siege`.
#[serde(default)]
pub is_deployed: bool,
/// Amphibious unit is currently on a water tile (embarked). Defence -50%;
/// cannot fortify. Set by `handle_embark`, cleared by `handle_disembark`.
/// Land unit currently embarked on a water tile (p3-18, Civ-VI auto-embark).
/// Defence is halved in combat (`mc_combat::embarked_defence_penalty`). Set
/// automatically by `mc-turn::process_one_move` from the destination tile —
/// moving onto water embarks, onto land disembarks; there is no explicit
/// embark action.
#[serde(default)]
pub is_embarked: bool,
/// Rally command to execute when this unit first reaches its rally hex.

113
src/simulator/crates/mc-turn/src/action_handlers/mod.rs
View file

@ -75,8 +75,6 @@ pub fn invoke(
}
Ok(())
}
ActionKind::Embark => handle_embark(state, player_idx, unit_idx),
ActionKind::Disembark => handle_disembark(state, player_idx, unit_idx),
// Formation commands: handled via GameState queues, not the unit-action dispatch path.
ActionKind::SetRallyPoint
| ActionKind::ClearRallyPoint
@ -473,39 +471,9 @@ fn handle_pack_siege(
Ok(())
}
fn handle_embark(
state: &mut GameState,
player_idx: usize,
unit_idx: usize,
) -> Result<(), ActionError> {
let unit = get_unit_mut(state, player_idx, unit_idx, ActionKind::Embark)?;
if unit.is_embarked {
return Err(ActionError {
kind: ActionKind::Embark,
reason: DisabledReason::AlreadyEmbarked,
});
}
unit.is_embarked = true;
// Cannot fortify while embarked; clear any existing fortify.
unit.is_fortified = false;
Ok(())
}
fn handle_disembark(
state: &mut GameState,
player_idx: usize,
unit_idx: usize,
) -> Result<(), ActionError> {
let unit = get_unit_mut(state, player_idx, unit_idx, ActionKind::Disembark)?;
if !unit.is_embarked {
return Err(ActionError {
kind: ActionKind::Disembark,
reason: DisabledReason::NotEmbarked,
});
}
unit.is_embarked = false;
Ok(())
}
// p3-18 — embarkation is auto (Civ-VI): mc-turn `process_one_move` toggles
// `MapUnit::is_embarked` from the destination tile. There is no explicit
// Embark/Disembark action handler.
// ── p2-53i: Scout action handlers ───────────────────────────────────────────
@ -852,79 +820,4 @@ mod tests {
assert_eq!(err.reason, DisabledReason::SiegeMustDeployFirst);
}
fn state_with_amphibious_unit(is_embarked: bool) -> (GameState, usize, usize) {
let unit = MapUnit {
col: 0,
row: 0,
hp: 30,
max_hp: 30,
attack: 6,
defense: 3,
is_fortified: false,
is_embarked,
unit_id: "shore_guard".into(),
held_resources: vec![],
patrol_order: None,
..Default::default()
};
let player = PlayerState {
player_index: 0,
gold: 0,
cities: vec![],
unit_upkeep: vec![1],
strategic_axes: Default::default(),
scoring_weights: Default::default(),
expansion_points: 0,
city_improvements: Default::default(),
city_ecology: vec![],
tech_state: None,
science_pool: 0,
player_tech: None,
science_yield: 0,
units: vec![unit],
city_positions: vec![],
capital_position: None,
culture_total: 0,
culture_pool: Default::default(),
arcane_lore_pop_deducted: false,
traded_luxuries: Default::default(),
relations: Default::default(),
strategic_ledger: Default::default(),
wonders_built: Default::default(),
explored_deposits: Default::default(),
..Default::default()
};
let state = GameState { turn: 0, players: vec![player], grid: None, ..Default::default() };
(state, 0, 0)
}
#[test]
fn embark_sets_flag_and_clears_fortify() {
let (mut state, pi, ui) = state_with_amphibious_unit(false);
state.players[pi].units[ui].is_fortified = true;
invoke(&mut state, pi, ui, ActionKind::Embark).unwrap();
assert!(state.players[pi].units[ui].is_embarked);
assert!(!state.players[pi].units[ui].is_fortified, "embarked unit cannot be fortified");
}
#[test]
fn embark_when_already_embarked_errors() {
let (mut state, pi, ui) = state_with_amphibious_unit(true);
let err = invoke(&mut state, pi, ui, ActionKind::Embark).unwrap_err();
assert_eq!(err.reason, DisabledReason::AlreadyEmbarked);
}
#[test]
fn disembark_clears_flag() {
let (mut state, pi, ui) = state_with_amphibious_unit(true);
invoke(&mut state, pi, ui, ActionKind::Disembark).unwrap();
assert!(!state.players[pi].units[ui].is_embarked);
}
#[test]
fn disembark_when_not_embarked_errors() {
let (mut state, pi, ui) = state_with_amphibious_unit(false);
let err = invoke(&mut state, pi, ui, ActionKind::Disembark).unwrap_err();
assert_eq!(err.reason, DisabledReason::NotEmbarked);
}
}