Skip to content
15 changes: 13 additions & 2 deletions crates/engine/src/parser/oracle_effect/subject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2836,13 +2836,24 @@ pub(super) fn parse_subject_application(
// In trigger effects: "they" refers to the triggering player (for player-type
// subjects like "an opponent") or the triggering source (for object subjects).
// Outside trigger context: anaphoric reference to previously mentioned objects.
if lower == "they" {
// CR 608.2d: an optional "may" modal parallels the "that player may " /
// "the player may " forms above — "they may pay {2}" (Wandering Archaic,
// Umbilicus) is the pronoun-subject counterpart of "that player may pay
// {2}" (Smothering Tithe, Mind Whip); both must set `is_optional` so
// `lower_subject_predicate_ast` marks the lowered ability optional and
// `resolve_they_pronoun`'s existing player/object dispatch is unchanged.
if let Ok((_, is_optional)) = all_consuming(alt((
value(true, tag::<_, _, OracleError<'_>>("they may")),
value(false, tag("they")),
)))
.parse(lower.as_str())
{
return Some(SubjectApplication {
affected: resolve_they_pronoun(ctx),
target: None,
multi_target: None,
inherits_parent: false,
is_optional: false,
is_optional,
});
}

Expand Down
217 changes: 217 additions & 0 deletions crates/engine/src/parser/oracle_trigger_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21888,6 +21888,63 @@ fn smothering_tithe_that_player_pays_as_triggering_player() {
}
}

/// CR 608.2d + CR 608.2k (issue #6477): the bare-pronoun counterpart of
/// `smothering_tithe_that_player_pays_as_triggering_player` — "they may pay"
/// anaphors back to "an opponent" from the trigger condition and must resolve
/// identically to the explicit "that player may pay" phrasing: the opponent
/// who cast the spell pays (not the Wandering Archaic controller), and the
/// payment is optional (CR 608.2d) so a decline can gate the copy. The copy's
/// "that spell" target is the untargeted spell object the trigger condition
/// already named, carried forward per CR 608.2k.
#[test]
fn wandering_archaic_they_pay_as_triggering_player() {
let def = parse_trigger_line(
"Whenever an opponent casts an instant or sorcery spell, they may pay {2}. If they don't, you may copy that spell. You may choose new targets for the copy.",
"Wandering Archaic",
);

assert_eq!(def.mode, TriggerMode::SpellCast);
let execute = def.execute.as_ref().expect("should have execute");
match &*execute.effect {
Effect::PayCost {
payer,
cost: AbilityCost::Mana { cost },
..
} => {
assert_eq!(
payer,
&TargetFilter::TriggeringPlayer,
"the opponent who cast the spell pays, not the Wandering Archaic controller"
);
assert_eq!(cost, &crate::types::mana::ManaCost::generic(2));
}
other => panic!("expected PayCost, got: {other:?}"),
}
assert!(execute.optional, "they may pay should be optional");

let sub = execute
.sub_ability
.as_ref()
.expect("copy should remain chained");
assert_eq!(
sub.condition,
Some(AbilityCondition::Not {
condition: Box::new(AbilityCondition::effect_performed())
}),
"the copy is gated on the opponent having declined payment"
);
assert!(sub.optional, "you may copy that spell");
match &*sub.effect {
Effect::CopySpell {
target, retarget, ..
} => {
assert_eq!(target, &TargetFilter::TriggeringSource);
assert_eq!(retarget, &CopyRetargetPermission::MayChooseNewTargets);
}
other => panic!("expected CopySpell sub_ability, got: {other:?}"),
}
}

/// CR 603.4: Wedding Ring — "an opponent who controls F draws
/// a card" parses the relative clause into an `ObjectCount >= 1`
/// intervening-if scoped to the triggering player, ANDed with the
Expand Down Expand Up @@ -25701,3 +25758,163 @@ fn thieving_skydiver_dependent_continuation_is_never_replicated_or_branch() {
"reach guard: Thieving Skydiver must build a multi-node chain (GainControl + Attach continuation), got {node_count}",
);
}

// ---------------------------------------------------------------------------
// CR 608.2c + CR 608.2d (issue #6477 review follow-up): the "they may" subject
// arm in `subject.rs` is a class fix, not a Wandering-Archaic special case —
// every card whose Oracle text puts the "may" modal on the bare pronoun
// "they" (rather than the explicit "that player"/"the player" forms already
// handled) previously fell through `parse_subject_application` with NO match
// (only the exact string "they", with no trailing "may", was accepted). The
// caller's `unwrap_or` fallback then silently substituted
// `SubjectApplication { affected: TargetFilter::Any, is_optional: false, .. }`
// — an unbound target AND a mandatory (non-"may") ability, both wrong. These
// four tests lock in the corrected behavior for every other printed card
// found to share the pattern (via a before/after parse diff), so the fix's
// wider blast radius is intentional and covered, not an unexplained
// side effect.
// ---------------------------------------------------------------------------

/// Mishra's Command mode 1: "Choose target player. They may discard up to X
/// cards." Before the fix: `Discard { target: Any, .. }`, non-optional —
/// unbound to the just-chosen player and mandatory despite "may". After: the
/// discard binds to `ParentTarget` (the chosen player) and is optional.
#[test]
fn mishras_command_they_may_discard_binds_to_chosen_player_and_is_optional() {
let parsed = parse_oracle_text(
"Choose two \u{2014}\n\u{2022} Choose target player. They may discard up to X cards. Then they draw a card for each card discarded this way.\n\u{2022} This spell deals X damage to target creature.\n\u{2022} This spell deals X damage to target planeswalker.\n\u{2022} Target creature gets +X/+0 and gains haste until end of turn.",
"Mishra's Command",
&[],
&["Sorcery".to_string()],
&[],
);
let mode1 = parsed
.abilities
.first()
.expect("Mishra's Command must parse mode 1 as the first ability");
assert!(
matches!(*mode1.effect, Effect::TargetOnly { .. }),
"mode 1's head is the target-player slot, got {:?}",
mode1.effect
);
let discard = mode1
.sub_ability
.as_ref()
.expect("the discard must remain chained to the chosen target");
match &*discard.effect {
Effect::Discard { target, .. } => {
assert_eq!(
target,
&TargetFilter::ParentTarget,
"\"they\" discard must bind to the just-chosen target player, not float unbound"
);
}
other => panic!("expected Discard, got {other:?}"),
}
assert!(
discard.optional,
"\"they may discard\" must be optional, not mandatory"
);
}

/// Undercity Plunder: "Target opponent discards a card. Then they may
/// discard an additional card. If they don't, conjure ..." Before the fix:
/// the second Discard's target was `Any` (unbound) and non-optional, so the
/// "if they don't" branch's condition was unreachable in practice.
#[test]
fn undercity_plunder_they_may_discard_additional_binds_to_parent_target() {
let parsed = parse_oracle_text(
"Target opponent discards a card. Then they may discard an additional card. If they don't, conjure a duplicate of a random card from their library into your hand. It perpetually gains \"You may spend mana as though it were mana of any color to cast this spell.\"",
"Undercity Plunder",
&[],
&["Sorcery".to_string()],
&[],
);
let head = parsed
.abilities
.first()
.expect("Undercity Plunder must parse the initial discard");
let second_discard = head
.sub_ability
.as_ref()
.expect("\"they may discard an additional card\" must remain chained");
match &*second_discard.effect {
Effect::Discard { target, .. } => {
assert_eq!(
target,
&TargetFilter::ParentTarget,
"the additional discard must bind to the same targeted opponent"
);
}
other => panic!("expected Discard, got {other:?}"),
}
assert!(
second_discard.optional,
"\"they may discard an additional card\" must be optional"
);
let conjure_gate = second_discard
.sub_ability
.as_ref()
.expect("the \"if they don't\" conjure branch must remain chained");
assert_eq!(
conjure_gate.condition,
Some(AbilityCondition::Not {
condition: Box::new(AbilityCondition::effect_performed())
}),
"the conjure branch is gated on declining the additional discard"
);
}

/// Tarnation: "Whenever a player commits a crime, they may draw a card."
/// Before the fix: `Draw { target: Any, .. }`, non-optional — the draw had no
/// player bound to it at all.
#[test]
fn tarnation_they_may_draw_binds_to_triggering_player() {
let def = parse_trigger_line(
"Whenever a player commits a crime, they may draw a card. (Targeting opponents, anything they control, and/or cards in their graveyards is a crime.)",
"Tarnation",
);
let execute = def.execute.as_ref().expect("should have execute");
match &*execute.effect {
Effect::Draw { target, .. } => {
assert_eq!(
target,
&TargetFilter::TriggeringPlayer,
"\"they\" draws for the player who committed the crime"
);
}
other => panic!("expected Draw, got {other:?}"),
}
assert!(execute.optional, "\"they may draw\" must be optional");
}

/// Smart Ass: "... If defending player has no cards with the chosen name in
/// their hand, they may reveal their hand. If they don't reveal their hand,
/// this creature can't be blocked this turn." Before the fix:
/// `RevealHand { target: Any, .. }`, non-optional.
#[test]
fn smart_ass_they_may_reveal_hand_is_optional_and_bound() {
let def = parse_trigger_line(
"Whenever this creature attacks, choose a card name. If defending player has no cards with the chosen name in their hand, they may reveal their hand. If they don't reveal their hand, this creature can't be blocked this turn.",
"Smart Ass",
);
let execute = def.execute.as_ref().expect("should have execute");
let reveal = execute
.sub_ability
.as_ref()
.expect("the reveal-hand clause must remain chained to the naming choice");
match &*reveal.effect {
Effect::RevealHand { target, .. } => {
assert_ne!(
target,
&TargetFilter::Any,
"\"they\" reveal must bind to a real player referent, not float unbound"
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
other => panic!("expected RevealHand, got {other:?}"),
}
assert!(
reveal.optional,
"\"they may reveal their hand\" must be optional"
);
}
Loading
Loading