diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index ed77913be4..b0395dcde7 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -1048,8 +1048,9 @@ pub enum TargetSelectionAdvance { /// CR 601.2c + CR 115.3: Identifies one instance of the word "target" on an /// ability. Slots sharing a `TargetInstanceId` are the SAME "target" (all slots /// of one `multi_target` "up to N target creatures" run) and must be mutually -/// distinct objects; slots with DIFFERENT ids are separate instances that may -/// reuse the same object ("Destroy target artifact and target land"). +/// distinct objects or players; slots with DIFFERENT ids are separate instances +/// that may reuse the same object or player ("Destroy target artifact and +/// target land"). #[derive(Debug, Clone, Copy, PartialEq, Eq)] struct TargetInstanceId(usize); @@ -5368,21 +5369,22 @@ fn legal_targets_for_selected_slot( }); } - // CR 601.2c + CR 115.3: within one instance of "target", the same object - // can't be chosen twice. Remove objects already chosen in prior slots of - // THIS instance. Prior slots of a DIFFERENT instance (separate "target") do - // not constrain this slot — they may legally reuse the same object - // (CR 601.2c "Destroy target artifact and target land" Example). - let already_in_instance: std::collections::HashSet = prior_specs + // CR 601.2c + CR 115.3: within one instance of "target", the same target — + // object OR player — can't be chosen twice. Remove targets already chosen + // in prior slots of THIS instance (issue #6459: Scheming Symmetry's + // "Choose two target players." accepted the same player for both slots + // because this set was narrowed to `ObjectId` and dropped + // `TargetRef::Player`). Prior slots of a DIFFERENT instance (separate + // "target") do not constrain this slot — they may legally reuse the same + // object or player (CR 601.2c "Destroy target artifact and target land" + // Example). + let already_in_instance: std::collections::HashSet = prior_specs .iter() .zip(selected_slots) .filter(|(prior, _)| prior.instance == spec.instance) - .filter_map(|(_, sel)| match sel { - Some(TargetRef::Object(id)) => Some(*id), - _ => None, - }) + .filter_map(|(_, sel)| sel.clone()) .collect(); - legal.retain(|t| !matches!(t, TargetRef::Object(id) if already_in_instance.contains(id))); + legal.retain(|t| !already_in_instance.contains(t)); // CR 115.4: "other target" / "another target" is a separate instance of // "target" but must differ from every target already chosen for this diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 3e3d089566..5b76725066 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -6457,14 +6457,16 @@ impl PublicStateDirty { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "type")] pub enum TargetSelectionConstraint { + /// CR 601.2c + CR 115.3: every chosen player target must be a different + /// player. Attached by the modal path ("each mode must target a different + /// player"), where distinctness spans the ability's modes rather than one + /// instance of the word "target". DifferentTargetPlayers, /// CR 115.1 + CR 601.2c: Object targets must be controlled by different players. DifferentObjectControllers, /// CR 115.1 + CR 601.2c + CR 400.1: Object targets must come from the same /// player-owned zone of the given kind, e.g. "from a single graveyard". - SameZoneOwner { - zone: Zone, - }, + SameZoneOwner { zone: Zone }, /// CR 202.3 + CR 601.2c: the chosen target set's combined mana value must /// satisfy `comparator` against `value`. `value` is a `QuantityExpr` (not /// `i32` like `SearchSelectionConstraint::TotalManaValue`) because the bound diff --git a/crates/engine/tests/integration/issue_6459_scheming_symmetry.rs b/crates/engine/tests/integration/issue_6459_scheming_symmetry.rs new file mode 100644 index 0000000000..23240755ee --- /dev/null +++ b/crates/engine/tests/integration/issue_6459_scheming_symmetry.rs @@ -0,0 +1,133 @@ +//! Scheming Symmetry — "Choose two target players." must require TWO DIFFERENT +//! players (CR 601.2c + CR 115.3: the same target — object or player — can't be +//! chosen multiple times for any one instance of the word "target"). +//! +//! Regression for issue #6459: in a multiplayer game the same player could be +//! chosen for both slots. The per-instance distinctness filter in +//! `legal_targets_for_selected_slot` (`game/ability_utils.rs`) excluded +//! already-chosen OBJECTS but dropped `TargetRef::Player`, so player targets +//! within one instance of "target" were never kept distinct. The fix widens +//! that set from `HashSet` to `HashSet`. +//! +//! Proof is end-to-end at runtime: after the first player is chosen, choosing +//! that SAME player for the second slot is rejected (and the slot stays open), +//! while a DIFFERENT player is accepted (the discriminating behaviour). + +use engine::game::scenario::GameScenario; +use engine::types::ability::TargetRef; +use engine::types::actions::GameAction; +use engine::types::game_state::{CastPaymentMode, WaitingFor}; +use engine::types::mana::ManaCost; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; + +const P0: PlayerId = PlayerId(0); +const P1: PlayerId = PlayerId(1); +const P2: PlayerId = PlayerId(2); + +const SCHEMING: &str = + "Choose two target players. Each of them searches their library for a card, \ +then shuffles and puts that card on top."; + +#[test] +fn scheming_symmetry_rejects_choosing_the_same_player_twice() { + let mut scenario = GameScenario::new_n_player(3, 42); + scenario.at_phase(Phase::PreCombatMain); + for &pid in &[P0, P1, P2] { + scenario.with_library_top(pid, &["Lib A", "Lib B"]); + } + let spell = scenario + .add_spell_to_hand_from_oracle(P0, "Scheming Symmetry", true, SCHEMING) + .with_mana_cost(ManaCost::zero()) + .id(); + let mut runner = scenario.build(); + + let card_id = runner.state().objects[&spell].card_id; + runner + .act(GameAction::CastSpell { + object_id: spell, + card_id, + targets: vec![], + payment_mode: CastPaymentMode::Auto, + }) + .expect("casting the sorcery must be accepted"); + + // First slot: all three players are legal. Choose P1. + let WaitingFor::TargetSelection { + target_slots, + selection, + .. + } = runner.state().waiting_for.clone() + else { + panic!( + "expected a per-slot TargetSelection, got {}", + runner.waiting_for_kind() + ); + }; + let slot0 = &target_slots[selection.current_slot]; + for pid in [P0, P1, P2] { + assert!( + slot0.legal_targets.contains(&TargetRef::Player(pid)), + "{pid:?} must be a legal first-slot target, slot = {slot0:?}" + ); + } + runner + .act(GameAction::ChooseTarget { + target: Some(TargetRef::Player(P1)), + }) + .expect("choosing P1 for the first slot must succeed"); + + // Second slot: choosing the ALREADY-CHOSEN player P1 must be rejected + // (CR 601.2c + CR 115.3), while the state stays on the same target slot. + let WaitingFor::TargetSelection { + target_slots, + selection, + .. + } = runner.state().waiting_for.clone() + else { + panic!( + "expected the second target slot, got {}", + runner.waiting_for_kind() + ); + }; + assert_eq!( + selection.current_slot, 1, + "the duplicate-target check must run against the second target slot" + ); + let slot1 = &target_slots[selection.current_slot]; + assert!( + slot1.legal_targets.contains(&TargetRef::Player(P2)), + "P2 must be a legal alternative in the second target slot, slot = {slot1:?}" + ); + let reselect_same = runner.act(GameAction::ChooseTarget { + target: Some(TargetRef::Player(P1)), + }); + assert!( + reselect_same.is_err(), + "CR 601.2c + CR 115.3 (issue #6459): choosing the already-chosen player \ + P1 for the second slot must be rejected" + ); + assert!( + matches!( + runner.state().waiting_for, + WaitingFor::TargetSelection { .. } + ), + "after the rejected reselection the second target slot must still be open" + ); + + // A DIFFERENT player (P2) is accepted, so the requirement is satisfiable — + // proving the rejection is the distinctness rule, not a dead slot. + runner + .act(GameAction::ChooseTarget { + target: Some(TargetRef::Player(P2)), + }) + .expect("choosing a different player (P2) for the second slot must succeed"); + assert!( + !matches!( + runner.state().waiting_for, + WaitingFor::TargetSelection { .. } + ), + "with two distinct players chosen the spell must leave target selection, got {}", + runner.waiting_for_kind() + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 0b221f2498..1fe781fb5d 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -621,6 +621,7 @@ mod issue_6431_lava_dart_flashback_control_turn; mod issue_6435_mosswort_bridge_hideaway_play; mod issue_6437_fight_rigging_exiled_card_target; mod issue_6440_mockingbird_uncast_copy_ceiling; +mod issue_6459_scheming_symmetry; mod issue_6498_portent_of_calamity; mod issue_6499_flickering_ward_protection_exemption; mod issue_6500_loreseekers_stone_hand_cost;