Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/engine/src/ai_support/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,10 @@ fn condition_reads_only_memo_safe_state(c: &ParsedCondition) -> bool {
| ParsedCondition::CardsLeftYourGraveyardThisTurnAtLeast { .. }
| ParsedCondition::PlayerCountAtLeast { .. }
| ParsedCondition::HasCityBlessing
// CR 903.3 / CR 903.3d: a controller-scoped battlefield scan for a commander
// (via `game::commander`), like the other `YouControl*` predicates — reads no
// combat/damage/pending-cast history, so it is memo-safe.
| ParsedCondition::ControlsCommander { .. }
// CR 503.1: reads only `state.phase`, apply()-constant global state.
| ParsedCondition::IsDuringUpkeep
// CR 102.2 / CR 102.3: reads `state.active_player` plus team topology,
Expand Down
13 changes: 11 additions & 2 deletions crates/engine/src/game/restrictions.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::game::game_object::GameObject;
use crate::types::ability::{
AbilityCost, AbilityDefinition, ActivationRestriction, CastingPermission, CastingRestriction,
ControllerRef, FilterProp, ParsedCondition, QuantityExpr, SpellCastingOptionKind, TargetFilter,
TypeFilter,
CommanderOwnership, ControllerRef, FilterProp, ParsedCondition, QuantityExpr,
SpellCastingOptionKind, TargetFilter, TypeFilter,
};
use crate::types::card_type::{CoreType, Supertype};
use crate::types::counter::{CounterMatch, CounterType};
Expand Down Expand Up @@ -1635,6 +1635,15 @@ pub(crate) fn evaluate_condition(
// CR 702.131c: The city's blessing is a player designation that effects
// and restrictions may identify.
ParsedCondition::HasCityBlessing => state.city_blessing.contains(&player),
// CR 903.3 / CR 903.3d: owner-scoped ("your commander") vs any-owner ("a
// commander") control. Delegates to the single `game::commander` authority —
// the same helpers `layers.rs` uses for `StaticCondition::ControlsCommander` —
// so a live re-evaluation at every activation-legality query correctly
// distinguishes owning your commander from merely controlling a stolen one.
ParsedCondition::ControlsCommander { ownership } => match ownership {
CommanderOwnership::Own => super::commander::controls_own_commander(state, player),
CommanderOwnership::Any => super::commander::controls_any_commander(state, player),
},
// CR 102.1: "The active player is the player whose turn it is."
ParsedCondition::IsYourTurn => state.active_player == player,
// CR 102.3 / CR 805.4a: the active player is on a team other than
Expand Down
132 changes: 50 additions & 82 deletions crates/engine/src/parser/oracle_condition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use super::oracle_nom::condition as nom_condition;
use super::oracle_nom::primitives as nom_primitives;
use super::oracle_target::parse_type_phrase;
use crate::types::ability::{
CommanderOwnership, Comparator, ControllerRef, FilterProp, ParsedCondition, QuantityExpr,
QuantityRef, StaticCondition, TargetFilter, TypedFilter,
Comparator, FilterProp, ParsedCondition, QuantityExpr, QuantityRef, StaticCondition,
TargetFilter, TypedFilter,
};
use crate::types::card_type::CoreType;
use crate::types::counter::CounterMatch;
Expand Down Expand Up @@ -277,30 +277,16 @@ fn static_condition_to_restriction_condition(
// CR 102.3 + CR 805.4a: keep the opponent relation distinct from
// `Not(IsYourTurn)`, which would incorrectly include a teammate's turn.
StaticCondition::DuringOpponentsTurn => Some(ParsedCondition::IsOpponentsTurn),
// CR 903.3d: "If an effect refers to controlling a commander, it refers to a
// permanent on the battlefield that is a commander" — regardless of who OWNS it.
// That is exactly an `ObjectCount` over the `IsCommander` filter scoped to your
// control, so it converts through the same presence bridge as `IsPresent`.
//
// `CommanderOwnership::Own` ("your commander") additionally requires you to own
// the permanent, and `TargetFilter` has no owner axis — it is rejected below
// rather than silently widened to "any commander you control", which would let
// a STOLEN commander satisfy a condition the card restricts to your own.
StaticCondition::ControlsCommander {
ownership: CommanderOwnership::Any,
} => Some(ParsedCondition::QuantityComparison {
lhs: QuantityExpr::Ref {
qty: QuantityRef::ObjectCount {
filter: TargetFilter::Typed(TypedFilter {
controller: Some(ControllerRef::You),
properties: vec![FilterProp::IsCommander],
..Default::default()
}),
},
},
comparator: Comparator::GE,
rhs: QuantityExpr::Fixed { value: 1 },
}),
// CR 903.3 / CR 903.3d: both commander-control phrasings mirror directly onto
// the `ParsedCondition` variant carrying the same `CommanderOwnership` axis.
// `Own` ("your commander") requires you to OWN the permanent (CR 109.5,
// Lieutenant); `Any` ("a commander") is controller-only, any owner (CR 903.3d,
// a stolen commander counts). Runtime evaluation delegates to the single
// `game::commander` authority — the same helpers `layers.rs` uses for the static
// form — so `Own` cannot be silently widened to "any commander you control".
StaticCondition::ControlsCommander { ownership } => {
Some(ParsedCondition::ControlsCommander { ownership })
}
// Source zone/state leaves with an exact restriction evaluator.
StaticCondition::SourceInZone { zone } => Some(ParsedCondition::SourceInZone { zone }),
StaticCondition::SourceIsAttacking => Some(ParsedCondition::SourceIsAttacking),
Expand Down Expand Up @@ -400,9 +386,6 @@ fn static_condition_to_restriction_condition(
| StaticCondition::WasCast { .. }
| StaticCondition::IsRingBearer
| StaticCondition::RingLevelAtLeast { .. }
| StaticCondition::ControlsCommander {
ownership: CommanderOwnership::Own,
}
| StaticCondition::SourceIsTapped
| StaticCondition::IsTapped { .. }
| StaticCondition::SourceIsFaceUp
Expand Down Expand Up @@ -829,7 +812,8 @@ fn capitalize_condition_word(text: &str) -> String {
mod tests {
use super::*;
use crate::types::ability::{
AggregateFunction, CountScope, PlayerScope, SharedQuality, TypeFilter,
AggregateFunction, CommanderOwnership, ControllerRef, CountScope, PlayerScope,
SharedQuality, TypeFilter,
};
use crate::types::card_type::Supertype;
use crate::types::counter::CounterType;
Expand Down Expand Up @@ -1063,16 +1047,8 @@ mod tests {
/// filter-carrying `SourceMatchesFilter`, which `ParsedCondition` has no variant to
/// hold.
///
/// "you control your commander" is the second, and it is the sharper one. The sibling
/// phrase "you control **a** commander" (CR 903.3d — any commander you control,
/// regardless of owner) DOES convert, to an `ObjectCount` over the `IsCommander`
/// filter. The possessive form additionally requires you to OWN the permanent, and
/// `TargetFilter` has no owner axis — so converting it with the same filter would
/// silently let a STOLEN commander satisfy a condition the card restricts to your own.
/// Reject beats approximate.
///
/// Fail-on-revert: routing `Unsupported` back into `parse_restriction_only_condition`,
/// or widening the `Own` arm to reuse the `Any` filter, makes these `Some(..)` again.
/// Fail-on-revert: routing `Unsupported` back into `parse_restriction_only_condition`
/// makes this `Some(..)` again.
#[test]
fn recognized_but_nonrepresentable_condition_fails_the_parse() {
// Assert WHICH `StaticCondition` is rejected by running the conversion directly.
Expand Down Expand Up @@ -1103,52 +1079,44 @@ mod tests {
SharedRestrictionParse::Unsupported
));
assert_eq!(parse_restriction_condition("~ is a creature"), None);

// The possessive commander form requires OWNERSHIP, which `TargetFilter` cannot
// express; its sibling "you control A commander" DOES convert (test below).
let own = shared_static("you control your commander");
assert!(matches!(
own,
StaticCondition::ControlsCommander {
ownership: CommanderOwnership::Own
}
));
assert_eq!(static_condition_to_restriction_condition(own), None);
assert!(matches!(
parse_shared_restriction_condition("you control your commander"),
SharedRestrictionParse::Unsupported
));
assert_eq!(
parse_restriction_condition("you control your commander"),
None
);
}

/// CR 903.3d: "you control a commander" refers to a permanent on the battlefield that
/// is a commander — regardless of owner. It converts to an `ObjectCount` over the
/// `IsCommander` filter scoped to your control.
/// CR 903.3 / CR 903.3d: both commander-control phrasings now convert to the
/// parameterized `ParsedCondition::ControlsCommander` variant carrying the same
/// `CommanderOwnership` axis as the sibling `StaticCondition`/`TriggerCondition`
/// forms. "your commander" → `Own` (CR 109.5, owner-scoped Lieutenant); "a
/// commander" → `Any` (CR 903.3d, any owner, a stolen commander counts).
///
/// This replaces the earlier split where `Any` lowered to an `ObjectCount`
/// `QuantityComparison` and `Own` was rejected outright (the possessive form has no
/// owner-axis `TargetFilter`). Both now delegate to the single `game::commander`
/// runtime authority — the same one `layers.rs` uses for the static form — so `Own`
/// is represented exactly instead of dropped, and `Deflecting Swat`'s "a commander"
/// free-cast condition remains satisfiable.
///
/// The legacy restriction grammar read this as subtype `"commander"` — a subtype no
/// permanent has — so Deflecting Swat's free-cast condition could NEVER be satisfied.
/// Fail-on-revert: collapsing the converter back to the `Any`→`ObjectCount` /
/// `Own`→reject split makes the `Own` assertion fail (it becomes `None`), and the
/// `Any` assertion fail (it becomes a `QuantityComparison`).
#[test]
fn controls_a_commander_converts_to_object_count() {
match shared("you control a commander") {
ParsedCondition::QuantityComparison {
lhs:
QuantityExpr::Ref {
qty:
QuantityRef::ObjectCount {
filter: TargetFilter::Typed(tf),
},
},
comparator: Comparator::GE,
rhs: QuantityExpr::Fixed { value: 1 },
} => {
assert_eq!(tf.controller, Some(ControllerRef::You));
assert!(tf.properties.contains(&FilterProp::IsCommander));
}
other => panic!("expected ObjectCount(IsCommander) >= 1, got {other:?}"),
}
fn both_commander_phrasings_convert_to_controls_commander() {
assert_eq!(
shared("you control your commander"),
ParsedCondition::ControlsCommander {
ownership: CommanderOwnership::Own,
},
);
assert_eq!(
parse_restriction_condition("you control your commander"),
Some(ParsedCondition::ControlsCommander {
ownership: CommanderOwnership::Own,
}),
);
assert_eq!(
shared("you control a commander"),
ParsedCondition::ControlsCommander {
ownership: CommanderOwnership::Any,
},
);
}

/// CR 122.1 + CR 711.2a: a counter BAND must never be widened into an "at least"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,8 @@ expression: "&ir"
"CastingOption": {
"kind": "CastWithoutManaCost",
"condition": {
"type": "QuantityComparison",
"lhs": {
"type": "Ref",
"qty": {
"type": "ObjectCount",
"filter": {
"type": "Typed",
"type_filters": [],
"controller": "You",
"properties": [
{
"type": "IsCommander"
}
]
}
}
},
"comparator": "GE",
"rhs": {
"type": "Fixed",
"value": 1
}
"type": "ControlsCommander",
"ownership": "Any"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,28 +42,8 @@ expression: "&lowered"
{
"kind": "CastWithoutManaCost",
"condition": {
"type": "QuantityComparison",
"lhs": {
"type": "Ref",
"qty": {
"type": "ObjectCount",
"filter": {
"type": "Typed",
"type_filters": [],
"controller": "You",
"properties": [
{
"type": "IsCommander"
}
]
}
}
},
"comparator": "GE",
"rhs": {
"type": "Fixed",
"value": 1
}
"type": "ControlsCommander",
"ownership": "Any"
}
}
]
Expand Down
11 changes: 11 additions & 0 deletions crates/engine/src/types/ability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8217,6 +8217,17 @@ pub enum ParsedCondition {
SpellTargetsFilter {
filter: TargetFilter,
},
/// CR 903.3 + CR 109.5: "you control your commander" — owner-scoped
/// (Lieutenant). CR 903.3d: "you control a commander" — controller-only, any
/// owner. The restriction-layer mirror of `StaticCondition::ControlsCommander`
/// / `TriggerCondition::ControlsCommander`; the `ownership` axis selects which
/// CR clause applies. Evaluated by `restrictions::evaluate_condition`, which
/// delegates to the single `crate::game::commander` authority — the same
/// helpers `layers.rs` uses for the static form, so both condition
/// vocabularies agree on the rule.
ControlsCommander {
ownership: CommanderOwnership,
},
// -- Combinators --
/// CR 601.3 / CR 602.5: All inner conditions must be true. Used for compound
/// casting/activation restrictions like "Cast this spell only if you control a
Expand Down
1 change: 1 addition & 0 deletions crates/engine/tests/integration/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,7 @@ mod swans_prevention_followup;
mod swarm_combat_witness;
mod tales_of_the_ancestors_catch_up_draw;
mod talon_gates_from_hand_activation;
mod tchaka_venerable_king;
mod teamwork_aggregate_legal_actions;
mod teamwork_origin_composition;
mod teferi_time_raveler_sorcery_speed_lock;
Expand Down
Loading
Loading