diff --git a/crates/engine/src/ai_support/filter.rs b/crates/engine/src/ai_support/filter.rs index 837f7d6aff..7c2fadf8ef 100644 --- a/crates/engine/src/ai_support/filter.rs +++ b/crates/engine/src/ai_support/filter.rs @@ -918,6 +918,8 @@ fn condition_reads_only_memo_safe_state(c: &ParsedCondition) -> bool { | ParsedCondition::CardsLeftYourGraveyardThisTurnAtLeast { .. } | ParsedCondition::PlayerCountAtLeast { .. } | ParsedCondition::HasCityBlessing + // CR 503.1: reads only `state.phase`, apply()-constant global state. + | ParsedCondition::IsDuringUpkeep | ParsedCondition::IsYourTurn => true, } } diff --git a/crates/engine/src/game/restrictions.rs b/crates/engine/src/game/restrictions.rs index fcc586e577..62e5e17153 100644 --- a/crates/engine/src/game/restrictions.rs +++ b/crates/engine/src/game/restrictions.rs @@ -1518,6 +1518,9 @@ pub(crate) fn evaluate_condition( ParsedCondition::HasCityBlessing => state.city_blessing.contains(&player), // CR 102.1: "The active player is the player whose turn it is." ParsedCondition::IsYourTurn => state.active_player == player, + // CR 503.1: The game is currently in the upkeep step. Player scope, if + // any, is composed by the caller via `And([Not(IsYourTurn), ..])`. + ParsedCondition::IsDuringUpkeep => state.phase == Phase::Upkeep, // CR 601.3d + CR 608.2c: "if it targets a [filter]" — gates a casting // permission on the chosen targets of the in-flight spell. Read from // `state.pending_cast.ability.targets` when targets have been committed. @@ -3092,6 +3095,54 @@ mod tests { )); } + #[test] + fn opponents_upkeep_activation_condition_gates_on_step_and_scope() { + // CR 602.5b + CR 102.1 + CR 503.1: "Activate only during an opponent's + // upkeep" is the composed restriction the parser emits for Trade Caravan — + // `And([Not(IsYourTurn), IsDuringUpkeep])`. Drive the real + // `evaluate_condition` across the full turn-scope × step matrix so the gate + // holds only in an opponent's upkeep step. + let mut state = crate::types::game_state::GameState::new_two_player(42); + let condition = ParsedCondition::And { + conditions: vec![ + ParsedCondition::Not { + condition: Box::new(ParsedCondition::IsYourTurn), + }, + ParsedCondition::IsDuringUpkeep, + ], + }; + let activator = PlayerId(0); + + // Opponent's turn, upkeep step -> allowed. + state.active_player = PlayerId(1); + state.phase = Phase::Upkeep; + assert!(evaluate_condition( + &state, + activator, + ObjectId(1), + &condition + )); + + // Opponent's turn, non-upkeep step -> denied (IsDuringUpkeep false). + state.phase = Phase::PreCombatMain; + assert!(!evaluate_condition( + &state, + activator, + ObjectId(1), + &condition + )); + + // Your turn, upkeep step -> denied (Not(IsYourTurn) false). + state.active_player = PlayerId(0); + state.phase = Phase::Upkeep; + assert!(!evaluate_condition( + &state, + activator, + ObjectId(1), + &condition + )); + } + #[test] fn evaluates_creatures_you_control_total_power_condition() { let mut state = crate::types::game_state::GameState::new_two_player(42); diff --git a/crates/engine/src/parser/oracle.rs b/crates/engine/src/parser/oracle.rs index 0d9a98cbdf..04dd27d50b 100644 --- a/crates/engine/src/parser/oracle.rs +++ b/crates/engine/src/parser/oracle.rs @@ -6781,6 +6781,13 @@ fn parse_activation_during_role_gate(i: &str) -> OracleResult<'_, ActivationRest tag::<_, _, OracleError<'_>>("as a sorcery"), ), value(ActivationRestriction::AsInstant, tag("as an instant")), + value( + opponents_upkeep_activation_restriction(), + alt(( + tag("during an opponent's upkeep"), + tag("during an opponents upkeep"), + )), + ), value( opponents_turn_activation_restriction(), alt(( @@ -7010,6 +7017,24 @@ fn opponents_turn_activation_condition() -> ParsedCondition { } } +/// CR 602.5b + CR 102.1 + CR 503.1: "Activate only during an opponent's upkeep" +/// gates activation to the upkeep step of a turn where the activator is not the +/// active player (Trade Caravan). Composed from the same `Not(IsYourTurn)` +/// opponent-turn leaf as `opponents_turn_activation_condition` plus the +/// `IsDuringUpkeep` step predicate, so the opponent scope reuses the existing +/// composition idiom instead of a dedicated `DuringOpponents*` restriction +/// sibling. +fn opponents_upkeep_activation_restriction() -> ActivationRestriction { + ActivationRestriction::RequiresCondition { + condition: Some(ParsedCondition::And { + conditions: vec![ + opponents_turn_activation_condition(), + ParsedCondition::IsDuringUpkeep, + ], + }), + } +} + pub(super) fn strip_activated_constraints(text: &str) -> (String, ActivatedConstraintAst) { let mut remaining = text.trim().trim_end_matches('.').trim().to_string(); let mut constraints = ActivatedConstraintAst::default(); diff --git a/crates/engine/src/parser/oracle_tests.rs b/crates/engine/src/parser/oracle_tests.rs index c768d5d951..2529b76926 100644 --- a/crates/engine/src/parser/oracle_tests.rs +++ b/crates/engine/src/parser/oracle_tests.rs @@ -248,6 +248,53 @@ fn activated_ability_opponent_turn_restriction_uses_not_your_turn_condition() { ); } +/// CR 602.5b + CR 102.1 + CR 503.1: "Activate only during an opponent's upkeep" +/// (Trade Caravan) composes the opponent-turn scope (`Not(IsYourTurn)`) with the +/// upkeep-step predicate (`IsDuringUpkeep`) inside a single `RequiresCondition`, +/// reusing the same composition idiom as the bare opponent-turn gate above rather +/// than a dedicated `DuringOpponents*` restriction sibling. Before the fix the +/// tail dropped to `Effect::Unimplemented` and the ability was activatable at any +/// time. +#[test] +fn activated_ability_opponents_upkeep_restriction_composes_scope_and_step() { + let r = parse( + "Remove two currency counters from ~: Untap target basic land. \ + Activate only during an opponent's upkeep.", + "Trade Caravan", + &[], + &["Creature"], + &["Human", "Nomad"], + ); + + assert_eq!(r.abilities.len(), 1, "got {:#?}", r.abilities); + let ability = &r.abilities[0]; + assert!( + !matches!(ability.effect.as_ref(), Effect::Unimplemented { .. }) + && ability + .sub_ability + .as_ref() + .is_none_or(|sub| !matches!(sub.effect.as_ref(), Effect::Unimplemented { .. })), + "expected no unimplemented fallback, got {:#?}", + ability + ); + + let expected = ActivationRestriction::RequiresCondition { + condition: Some(ParsedCondition::And { + conditions: vec![ + ParsedCondition::Not { + condition: Box::new(ParsedCondition::IsYourTurn), + }, + ParsedCondition::IsDuringUpkeep, + ], + }), + }; + assert!( + ability.activation_restrictions.contains(&expected), + "expected composed opponent's-upkeep restriction, got {:?}", + ability.activation_restrictions + ); +} + /// CR 508.1: a STANDALONE combat-window activation gate — "Activate only before /// attackers are declared" / "Activate only before combat" (Arcum's Whistle, /// Arcum's Sleigh) with no "during " first half — must map to the enforced diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 319ebecc63..6a18212e23 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -7600,6 +7600,15 @@ pub enum ParsedCondition { /// ability-resolution layer), the same way `ParsedCondition::And` mirrors /// `AbilityCondition::And`. IsYourTurn, + /// CR 503.1: True when the game is currently in the upkeep step. A + /// turn-structure predicate with NO player scope — it asks only "is it an + /// upkeep step", not whose. Player scope composes at the restriction layer: + /// "during an opponent's upkeep" is `And([Not(IsYourTurn), IsDuringUpkeep])` + /// (CR 102.1 fixes the active player from the turn), mirroring how + /// opponent-turn activation timing reuses `Not(IsYourTurn)` (see + /// `opponents_turn_activation_condition`) rather than a dedicated + /// `DuringOpponents*` restriction sibling. + IsDuringUpkeep, /// CR 601.3d + CR 702.8a + CR 608.2c: The in-flight spell being cast targets at /// least one object that matches `filter`. Gates a target-dependent casting /// permission (Timely Ward — "you may cast this spell as though it had flash if diff --git a/docs/parser-misparse-backlog.md b/docs/parser-misparse-backlog.md index d3bfd7c824..3130248b0b 100644 --- a/docs/parser-misparse-backlog.md +++ b/docs/parser-misparse-backlog.md @@ -5071,7 +5071,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top -### 28. Trigger/activation timing or ordinal restriction dropped (14 cards) +### 28. Trigger/activation timing or ordinal restriction dropped (13 cards) **Signature.** A timing/scope restriction (OnlyDuringYourTurn / OncePerTurn / 'during an opponent's turn' / Nth-spell ordinal / cast-timing) is null; the constraint tail is not parsed. @@ -5090,7 +5090,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top - Shadowheart, Sharran Cleric - Skarrgan Hellkite - Tomb Tyrant -- Trade Caravan - Uthros Research Craft - Uthros, Titanic Godcore