diff --git a/crates/engine/src/parser/oracle_effect/subject.rs b/crates/engine/src/parser/oracle_effect/subject.rs index 2d36e7ba43..a74bf1f96d 100644 --- a/crates/engine/src/parser/oracle_effect/subject.rs +++ b/crates/engine/src/parser/oracle_effect/subject.rs @@ -3333,7 +3333,8 @@ fn try_split_pump_compound( // CR 608.2d: a pump compounded with a modal keyword grant -- // "gets +1/+1 and gains your choice of deathtouch or lifelink" (Alchemist's - // Gift) -- has a grant half that is a two-branch player choice, so it cannot + // Gift) -- has a grant half that is an N-branch player choice (two or more + // options, e.g. Golem Artisan's "flying, trample, or haste"), so it cannot // collapse into a single `ContinuousModification` the way a fixed "and gains // trample" does (which the guard above routes to `build_continuous_clause`'s // coalescing path). Route the choice through the same @@ -3368,7 +3369,7 @@ fn try_split_pump_compound( } /// CR 608.2d: build the modal keyword-grant half of a pump -/// compound ("gets +1/+1 AND gains your choice of X or Y") as a `ChooseOneOf` +/// compound ("gets +1/+1 AND gains your choice of X, Y, or Z") as a `ChooseOneOf` /// sub_ability. Reuses the same `parse_keyword_choice_grant` / /// `keyword_choice_branch` builders as the standalone `build_keyword_choice_clause`, /// keyed to the pumped creature via `static_affected_for_application` @@ -3384,13 +3385,13 @@ fn build_keyword_choice_sub_ability( // "gains your choice of ...". `parse_keyword_choice_grant` anchors on the // bare "gain ..." form, so deconjugate the remainder here first. let normalized = deconjugate_verb(remainder); - let (first, second, duration) = parse_keyword_choice_grant(&normalized)?; + let (keywords, duration) = parse_keyword_choice_grant(&normalized)?; let affected = static_affected_for_application(application); let choice_duration = duration.clone(); - let branches = vec![ - keyword_choice_branch(first, affected.clone(), None, duration.clone()), - keyword_choice_branch(second, affected, None, duration), - ]; + let branches = keywords + .into_iter() + .map(|kw| keyword_choice_branch(kw, affected.clone(), None, duration.clone())) + .collect(); Some(( AbilityDefinition::new( AbilityKind::Spell, @@ -3403,18 +3404,29 @@ fn build_keyword_choice_sub_ability( )) } -fn parse_keyword_choice_grant(predicate: &str) -> Option<(Keyword, Keyword, Option)> { +fn parse_keyword_choice_grant(predicate: &str) -> Option<(Vec, Option)> { let lower = predicate.to_lowercase(); - // Shape 1: "gain your choice of X or Y" — an explicit keyword-grant menu. + // Shape 1: "gain your choice of X, Y, or Z" — an explicit keyword-grant menu + // of two OR MORE options (Golem Artisan: "flying, trample, or haste"). Reuse + // the nom-based `split_choice_list_items` splitter (shared with the counter- + // choice and "from among" paths) so an Oxford-comma N-ary list parses without + // manual byte slicing. if let Ok((choice_text, _)) = tag::<_, _, OracleError<'_>>("gain your choice of ").parse(lower.as_str()) { let (keyword_text, duration) = super::strip_trailing_duration(choice_text); - let (_, (left, right)) = nom_primitives::split_once_on(keyword_text.trim(), " or ").ok()?; - let first = parse_granted_keyword_fragment(left.trim())?; - let second = parse_granted_keyword_fragment(right.trim())?; - return Some((first, second, duration.or(Some(Duration::UntilEndOfTurn)))); + let items = super::split_choice_list_items(keyword_text.trim())?; + // `separated_list1` succeeds on a single item when there is no separator + // at all; require ≥2 so a lone keyword is not mistaken for a "choice". + if items.len() < 2 { + return None; + } + let keywords: Vec = items + .iter() + .map(|item| parse_granted_keyword_fragment(item.trim())) + .collect::>>()?; + return Some((keywords, duration.or(Some(Duration::UntilEndOfTurn)))); } // Shape 2: "gain/have protection from X or from the color of your choice" @@ -3446,7 +3458,10 @@ fn parse_keyword_choice_grant(predicate: &str) -> Option<(Keyword, Keyword, Opti let second = Keyword::Protection(crate::types::keywords::parse_protection_target( right.trim(), )); - Some((first, second, duration.or(Some(Duration::UntilEndOfTurn)))) + Some(( + vec![first, second], + duration.or(Some(Duration::UntilEndOfTurn)), + )) } fn keyword_choice_branch( @@ -3476,12 +3491,12 @@ fn build_keyword_choice_clause( application: &SubjectApplication, predicate: &str, ) -> Option { - let (first, second, duration) = parse_keyword_choice_grant(predicate)?; + let (keywords, duration) = parse_keyword_choice_grant(predicate)?; let affected = static_affected_for_application(application); - let branches = vec![ - keyword_choice_branch(first, affected.clone(), None, duration.clone()), - keyword_choice_branch(second, affected, None, duration), - ]; + let branches = keywords + .into_iter() + .map(|kw| keyword_choice_branch(kw, affected.clone(), None, duration.clone())) + .collect(); let choose_effect = Effect::ChooseOneOf { chooser: PlayerFilter::Controller, diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index d08f1ace28..3d0b673ec3 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -13582,6 +13582,113 @@ fn targeted_keyword_choice_grant_uses_target_only_then_choose_one_of() { } } +/// Issue #5992: Golem Artisan's "{2}: Target artifact creature gains your choice +/// of flying, trample, or haste until end of turn." A 3-way Oxford-comma choice +/// list must yield a `ChooseOneOf` of THREE branches — the old binary +/// `split_once_on(text, " or ")` path returned `None` here (falling through to +/// `Unimplemented`). Also asserts `target.controller == None`: the real card has +/// NO "you control" restriction on its target (the issue's paraphrase was wrong). +#[test] +fn golem_artisan_three_way_keyword_choice_grant_has_no_controller_restriction() { + let def = parse_effect_chain( + "Target artifact creature gains your choice of flying, trample, or haste until end of turn", + AbilityKind::Spell, + ); + + let Effect::TargetOnly { + target: TargetFilter::Typed(target), + } = &*def.effect + else { + panic!("expected TargetOnly head, got {:?}", def.effect); + }; + // GAP-FIX: positively assert the target carries NO controller restriction — + // Golem Artisan targets ANY artifact creature, not just ones you control. + assert_eq!(target.controller, None); + + let choice = def + .sub_ability + .as_deref() + .expect("targeted keyword choice must chain a choice prompt"); + let Effect::ChooseOneOf { chooser, branches } = &*choice.effect else { + panic!("expected ChooseOneOf sub-ability, got {:?}", choice.effect); + }; + assert_eq!(*chooser, PlayerFilter::Controller); + assert_eq!(branches.len(), 3); + + for (branch, keyword) in [ + (&branches[0], Keyword::Flying), + (&branches[1], Keyword::Trample), + (&branches[2], Keyword::Haste), + ] { + let Effect::GenericEffect { + static_abilities, + duration, + target, + } = &*branch.effect + else { + panic!( + "expected keyword GenericEffect branch, got {:?}", + branch.effect + ); + }; + assert_eq!(*duration, Some(Duration::UntilEndOfTurn)); + assert_eq!(*target, None); + assert_eq!( + static_abilities[0].affected, + Some(TargetFilter::ParentTarget) + ); + assert!(static_abilities[0] + .modifications + .contains(&ContinuousModification::AddKeyword { keyword })); + } +} + +/// Issue #5992 generality guard: a DIFFERENT real card with the same N-ary +/// "your choice of" keyword-grant shape — Assassin Initiate, +/// "{1}: This creature gains your choice of flying, deathtouch, or lifelink until +/// end of turn." (verbatim Scryfall Oracle text). Self-referential (no target), +/// so the `ChooseOneOf` is the head effect, but must still expand to 3 branches. +#[test] +fn assassin_initiate_three_way_keyword_choice_grant_is_generic() { + let def = parse_effect_chain( + "This creature gains your choice of flying, deathtouch, or lifelink until end of turn", + AbilityKind::Spell, + ); + + // No target restriction ("This creature") — the ChooseOneOf is the head. + let branches = match &*def.effect { + Effect::ChooseOneOf { chooser, branches } => { + assert_eq!(*chooser, PlayerFilter::Controller); + branches.clone() + } + other => panic!("expected ChooseOneOf head, got {other:?}"), + }; + assert_eq!(branches.len(), 3); + + let mut granted: Vec = branches + .iter() + .map(|branch| { + let Effect::GenericEffect { + static_abilities, .. + } = &*branch.effect + else { + panic!( + "expected keyword GenericEffect branch, got {:?}", + branch.effect + ); + }; + match &static_abilities[0].modifications[0] { + ContinuousModification::AddKeyword { keyword } => keyword.clone(), + other => panic!("expected AddKeyword modification, got {other:?}"), + } + }) + .collect(); + granted.sort_by_key(|k| format!("{k}")); + let mut expected = vec![Keyword::Flying, Keyword::Deathtouch, Keyword::Lifelink]; + expected.sort_by_key(|k| format!("{k}")); + assert_eq!(granted, expected); +} + #[test] fn shared_target_tap_or_untap_uses_resolution_choice() { let def = parse_effect_chain("Tap or untap target permanent", AbilityKind::Spell); diff --git a/crates/engine/tests/integration/issue_5992_golem_artisan.rs b/crates/engine/tests/integration/issue_5992_golem_artisan.rs new file mode 100644 index 0000000000..5ff056ef99 --- /dev/null +++ b/crates/engine/tests/integration/issue_5992_golem_artisan.rs @@ -0,0 +1,213 @@ +//! Issue #5992 — Golem Artisan's "gains your choice of flying, trample, or +//! haste" activated ability. +//! +//! Verbatim Scryfall Oracle (Golem Artisan, {5}, Commander Legends, a 3/3 +//! Artifact Creature — Golem): +//! +//! {2}: Target artifact creature gets +1/+1 until end of turn. +//! {2}: Target artifact creature gains your choice of flying, trample, or haste +//! until end of turn. +//! +//! The second ability is a THREE-way Oxford-comma keyword choice. On `main`, +//! `parse_keyword_choice_grant` only handled a binary "X or Y" split, so this +//! 3-way list failed to parse and the ability fell through to `Unimplemented` — +//! no branch ever surfaced. The fix routes the choice list through the nom-based +//! `split_choice_list_items` splitter so an N-ary list parses into N branches. +//! +//! These are REAL end-to-end runtime tests: they activate the ability, drive the +//! surfaced `ChooseOneOfBranch` prompt to completion, and assert the chosen +//! keyword actually lands on the targeted artifact creature after +//! `evaluate_layers`. `drive_resolution` has no `ChooseOneOfBranch` arm and +//! legitimately halts there — that halt IS the positive proof the 3-way prompt +//! was offered. +//! +//! There is NO "you control" restriction on the target (the real card targets +//! ANY artifact creature). The second test proves this holds through the real +//! targeting-legality pipeline by targeting an OPPONENT-controlled artifact +//! creature. + +use engine::game::layers::evaluate_layers; +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::actions::GameAction; +use engine::types::card_type::CoreType; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::keywords::Keyword; +use engine::types::mana::{ManaType, ManaUnit}; +use engine::types::phase::Phase; + +const GOLEM_ARTISAN_ORACLE: &str = "{2}: Target artifact creature gets +1/+1 until \ + end of turn.\n{2}: Target artifact creature gains your choice of flying, \ + trample, or haste until end of turn."; + +/// The choice ability is the SECOND printed ability (index 1); index 0 is the +/// +1/+1 pump. +const CHOICE_ABILITY_INDEX: usize = 1; + +/// Two colorless mana — enough to pay the ability's `{2}` activation cost. +fn two_generic() -> Vec { + vec![ + ManaUnit::new(ManaType::Colorless, ObjectId(0), false, vec![]), + ManaUnit::new(ManaType::Colorless, ObjectId(0), false, vec![]), + ] +} + +/// Make an already-created creature also an artifact (an ARTIFACT CREATURE), +/// keeping its Creature type. The `CardBuilder::as_artifact` helper strips +/// Creature, so push the core type directly onto both the computed and base +/// type lines (base survives `evaluate_layers`). +fn make_artifact_creature(runner: &mut GameRunner, id: ObjectId) { + let obj = runner.state_mut().objects.get_mut(&id).unwrap(); + obj.card_types.core_types.push(CoreType::Artifact); + obj.base_card_types.core_types.push(CoreType::Artifact); +} + +/// After the ability has resolved (state halted at `ChooseOneOfBranch`), choose +/// the branch whose description names `keyword`, then drive priority until the +/// stack empties and layers settle. +fn choose_branch_and_settle(runner: &mut GameRunner, keyword: &str) { + let index = match &runner.state().waiting_for { + WaitingFor::ChooseOneOfBranch { + branch_descriptions, + branches, + .. + } => { + assert_eq!( + branches.len(), + 3, + "the 3-way 'flying, trample, or haste' choice must offer 3 branches, got {:?}", + branch_descriptions + ); + branch_descriptions + .iter() + .position(|d| d.contains(keyword)) + .unwrap_or_else(|| { + panic!("expected a '{keyword}' branch among {branch_descriptions:?}") + }) + } + other => { + panic!("expected a ChooseOneOfBranch prompt (the 3-way keyword choice), got {other:?}") + } + }; + + runner + .act(GameAction::ChooseBranch { index }) + .expect("choosing the keyword branch must succeed"); + + for _ in 0..16 { + match &runner.state().waiting_for { + WaitingFor::Priority { .. } => { + if runner.state().stack.is_empty() && runner.state().deferred_triggers.is_empty() { + evaluate_layers(runner.state_mut()); + return; + } + runner.pass_both_players(); + } + other => panic!("unexpected waiting state after choosing the branch: {other:?}"), + } + } + + panic!("stack/deferred_triggers did not settle within 16 priority passes"); +} + +/// Self-target: Golem Artisan activates its choice ability targeting ITSELF and +/// gains the chosen keyword (Trample) end-to-end. +#[test] +fn golem_artisan_three_way_choice_grants_trample_to_self() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + for pid in [P0, P1] { + scenario.with_library_top(pid, &["Lib A", "Lib B", "Lib C", "Lib D"]); + } + let golem = scenario + .add_creature_from_oracle(P0, "Golem Artisan", 3, 3, GOLEM_ARTISAN_ORACLE) + .id(); + scenario.with_mana_pool(P0, two_generic()); + + let mut runner = scenario.build(); + make_artifact_creature(&mut runner, golem); + + assert!( + !runner.state().objects[&golem].has_keyword(&Keyword::Trample), + "precondition: Golem Artisan has no trample before the ability" + ); + + runner + .activate(golem, CHOICE_ABILITY_INDEX) + .target_object(golem) + .resolve(); + + choose_branch_and_settle(&mut runner, "Trample"); + + assert!( + runner + .state() + .objects + .get(&golem) + .unwrap() + .has_keyword(&Keyword::Trample), + "the chosen keyword (trample) must be granted to the targeted artifact creature" + ); + assert!( + !runner.state().objects[&golem].has_keyword(&Keyword::Flying), + "the unchosen keyword (flying) must NOT be granted" + ); + assert!( + !runner.state().objects[&golem].has_keyword(&Keyword::Haste), + "the unchosen keyword (haste) must NOT be granted" + ); +} + +/// GAP-FIX: opponent-target. The real card has NO "you control" restriction. +/// Golem Artisan (P0) activates its choice ability targeting an OPPONENT-owned +/// (P1) artifact creature; the targeting must be LEGAL and the chosen keyword +/// (Haste) must land on the opponent's creature. This exercises the real +/// targeting-legality pipeline, not just the parsed `TargetFilter` shape. +#[test] +fn golem_artisan_three_way_choice_can_target_opponent_artifact_creature() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + for pid in [P0, P1] { + scenario.with_library_top(pid, &["Lib A", "Lib B", "Lib C", "Lib D"]); + } + let golem = scenario + .add_creature_from_oracle(P0, "Golem Artisan", 3, 3, GOLEM_ARTISAN_ORACLE) + .id(); + // A vanilla artifact creature under the OPPONENT's control. + let opponent_golem = scenario.add_creature(P1, "Opposing Construct", 2, 2).id(); + scenario.with_mana_pool(P0, two_generic()); + + let mut runner = scenario.build(); + make_artifact_creature(&mut runner, golem); + make_artifact_creature(&mut runner, opponent_golem); + + assert!( + !runner.state().objects[&opponent_golem].has_keyword(&Keyword::Haste), + "precondition: the opponent's artifact creature has no haste" + ); + + // Activating targeting the OPPONENT's artifact creature must be accepted + // (no "you control" restriction). If targeting were illegal, `.resolve()` + // would fail to reach the ChooseOneOfBranch prompt and the helper panics. + runner + .activate(golem, CHOICE_ABILITY_INDEX) + .target_object(opponent_golem) + .resolve(); + + choose_branch_and_settle(&mut runner, "Haste"); + + assert!( + runner + .state() + .objects + .get(&opponent_golem) + .unwrap() + .has_keyword(&Keyword::Haste), + "the chosen keyword (haste) must land on the OPPONENT's targeted artifact creature" + ); + // Golem Artisan itself was not the target — it stays vanilla. + assert!( + !runner.state().objects[&golem].has_keyword(&Keyword::Haste), + "the untargeted source must not gain the keyword" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 3e93172310..6ce4fa5d58 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -556,6 +556,7 @@ mod issue_5983_sothera_dies_edict; mod issue_5984_aloy_discover_on_attack; mod issue_5988_braids_arisen_nightmare; mod issue_5989_ardyn_exile_copy; +mod issue_5992_golem_artisan; mod issue_5996_planetarium_look_cast; mod issue_5997_malcolm_treasure_trigger; mod issue_5999_aura_exile_host;