From 0e87a2d07ed1387ca096c6db816f18d304ff2e48 Mon Sep 17 00:00:00 2001 From: rsnetworkinginc Date: Thu, 23 Jul 2026 15:23:44 -0700 Subject: [PATCH 1/2] fix(engine): require distinct players for a multi-target player requirement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CR 115.1b + CR 601.2c: a single instruction that chooses two or more target players requires them to be different — the same player can't fill more than one of the instruction's target slots. Scheming Symmetry ("Choose two target players.", issue #6459) let the same player be chosen for both slots. Modal cards attach DifferentTargetPlayers from "each mode must target a different player"; a plain multi-target player requirement had no equivalent, so the runtime distinctness authority (validate_target_constraints) never saw a constraint to enforce. A post-lowering pass now attaches TargetSelectionConstraint::DifferentTargetPlayers to every non-modal multi-target Player/Opponent requirement, reusing the existing enforcement. Closes #6459 --- crates/engine/src/parser/oracle.rs | 52 ++++++ .../issue_6459_scheming_symmetry.rs | 148 ++++++++++++++++++ crates/engine/tests/integration/main.rs | 1 + 3 files changed, 201 insertions(+) create mode 100644 crates/engine/tests/integration/issue_6459_scheming_symmetry.rs diff --git a/crates/engine/src/parser/oracle.rs b/crates/engine/src/parser/oracle.rs index 5dfc0cbb8f..fd37e05559 100644 --- a/crates/engine/src/parser/oracle.rs +++ b/crates/engine/src/parser/oracle.rs @@ -18,6 +18,7 @@ use crate::types::ability::{ TargetFilter, TriggerCondition, TriggerDefinition, TypedFilter, }; use crate::types::format::DeckCopyLimit; +use crate::types::game_state::TargetSelectionConstraint; use crate::types::keywords::{EscapeCost, FlashbackCost, Keyword, KeywordKind}; use crate::types::mana::ManaCost; use crate::types::phase::Phase; @@ -2739,6 +2740,56 @@ fn parse_flash_cleanup_sacrifice_casting_option( /// `OracleDocIr.diagnostics`. That keeps the doc IR the single warning channel /// (`OracleDocIr.diagnostics` → `ParsedAbilities.parse_warnings`) rather than /// letting the audit direct-append to `parse_warnings` behind the doc's back. +/// CR 115.1b + CR 601.2c: a single instruction that chooses two or more target +/// PLAYERS requires them to be different — the same player can't be chosen for +/// more than one of that instruction's target slots (Scheming Symmetry, issue +/// #6459: "Choose two target players." allowed picking the same player twice). +/// +/// Modal cards express this via `ModalSelectionConstraint::DifferentTargetPlayers` +/// (parsed from "each mode must target a different player"), but a plain +/// multi-target player requirement had no equivalent, so the runtime +/// distinctness authority (`validate_target_constraints`) never saw a constraint +/// to enforce. Attach `TargetSelectionConstraint::DifferentTargetPlayers` here +/// for every non-modal multi-target player requirement — the shared enforcement +/// then rejects a duplicate player selection uniformly (Scheming Symmetry, and +/// the "any number of target players" class). The constraint is a no-op for a +/// selection of fewer than two players, so attaching it whenever the requirement +/// CAN select multiple players is always safe. +fn ensure_distinct_player_multi_targets(result: &mut ParsedAbilities) { + for ability in &mut result.abilities { + ensure_distinct_player_multi_target(ability); + } + for trigger in &mut result.triggers { + if let Some(execute) = trigger.execute.as_mut() { + ensure_distinct_player_multi_target(execute); + } + } +} + +fn ensure_distinct_player_multi_target(def: &mut AbilityDefinition) { + if def.multi_target.is_none() { + return; + } + // Only player-target requirements are governed by this constraint; an + // object multi-target ("two target creatures") is kept distinct by the + // per-slot legal-set exclusion instead. + if !matches!( + def.effect.target_filter(), + Some(TargetFilter::Player | TargetFilter::Opponent) + ) { + return; + } + if def + .target_constraints + .iter() + .any(|c| matches!(c, TargetSelectionConstraint::DifferentTargetPlayers)) + { + return; + } + def.target_constraints + .push(TargetSelectionConstraint::DifferentTargetPlayers); +} + pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities { let mut result = ParsedAbilities { abilities: Vec::new(), @@ -2847,6 +2898,7 @@ pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities { &static_ids, ); reconcile_host_bound_phase_outs(&mut result); + ensure_distinct_player_multi_targets(&mut result); apply_linked_choice_persisted_player(&mut result, &ir.relations, &ability_ids, &trigger_ids); // Architectural rule: the parser must never silently discard Oracle text. Run 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..99e8e04f4b --- /dev/null +++ b/crates/engine/tests/integration/issue_6459_scheming_symmetry.rs @@ -0,0 +1,148 @@ +//! Scheming Symmetry — "Choose two target players." must require TWO DIFFERENT +//! players (CR 115.1b + CR 601.2c: the same player can't be chosen to fill more +//! than one of a single instruction's target slots). +//! +//! Regression for issue #6459: in a multiplayer game the same player could be +//! chosen for both slots. A modal "each mode must target a different player" +//! card already carries `DifferentTargetPlayers`, but a plain multi-target +//! player requirement had no constraint attached, so the runtime distinctness +//! authority never saw one to enforce. +//! +//! Two layers of proof: +//! * parser — the parsed ability carries `DifferentTargetPlayers`; +//! * runtime — after the first player is chosen, choosing that SAME player for +//! the second slot is rejected, while a DIFFERENT player is accepted (the +//! discriminating end-to-end 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_parses_distinct_player_constraint() { + use engine::types::game_state::TargetSelectionConstraint; + + let parsed = engine::parser::parse_oracle_text( + SCHEMING, + "Scheming Symmetry", + &[], + &["Sorcery".to_string()], + &[], + ); + let ability = parsed + .abilities + .first() + .expect("Scheming Symmetry lowers to a spell ability"); + assert!( + ability + .target_constraints + .iter() + .any(|c| matches!(c, TargetSelectionConstraint::DifferentTargetPlayers)), + "a \"choose two target players\" requirement must carry \ + DifferentTargetPlayers (issue #6459); got {:?}", + ability.target_constraints + ); +} + +#[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 115.1b), while the state stays on the same target slot. + assert!( + matches!( + runner.state().waiting_for, + WaitingFor::TargetSelection { .. } + ), + "a second target slot must be surfaced, got {}", + runner.waiting_for_kind() + ); + let reselect_same = runner.act(GameAction::ChooseTarget { + target: Some(TargetRef::Player(P1)), + }); + assert!( + reselect_same.is_err(), + "CR 115.1b (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 621d6f14a3..decb17e3ed 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -579,6 +579,7 @@ mod issue_6092_ability_block_reason; mod issue_6102_ragavan_exile_cast; mod issue_6157_gold_token_auto_mana_payment; mod issue_629_fractured_sanity_cycling; +mod issue_6459_scheming_symmetry; mod issue_6500_loreseekers_stone_hand_cost; mod issue_654_stridehangar_automaton; mod issue_680_shalai_and_hallar_forgotten_ancient; From b2f8472c2f833159ce698a3b5853b2973cfd6b44 Mon Sep 17 00:00:00 2001 From: rsnetworkinginc Date: Sun, 26 Jul 2026 02:21:04 +0300 Subject: [PATCH 2/2] fix(engine): widen per-instance target distinctness to players MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relocate the issue #6459 fix from a parser-lowering pass to the existing per-instance distinctness authority in legal_targets_for_selected_slot (CR 601.2c + CR 115.3): `already_in_instance` was narrowed to HashSet, so TargetRef::Player fell through and the same player could fill two slots of one instance of "target" (Scheming Symmetry, "Choose two target players."). Widening it to HashSet keeps objects AND players distinct within an instance while still permitting reuse across separate instances, and covers both the selection and validation paths, which route through the same helper. - drop the ensure_distinct_player_multi_targets parser pass and its parse-level test: the runtime authority now enforces the rule with no card-data churn, so the parse-diff is empty - correct the CR citation from CR 115.1b (the Aura rule) to the repo's standard CR 601.2c + CR 115.3 pair at every site - fix the TargetInstanceId doc, which restated CR 115.3 as "mutually distinct objects" — the rule covers objects or players - document TargetSelectionConstraint::DifferentTargetPlayers (modal path) with its CR reference The runtime regression test (real CastSpell -> ChooseTarget in a 3-player game: same player rejected, slot stays open, different player completes) is unchanged and passes against the relocated fix. --- crates/engine/src/game/ability_utils.rs | 28 +++++----- crates/engine/src/parser/oracle.rs | 52 ------------------ crates/engine/src/types/game_state.rs | 8 +-- .../issue_6459_scheming_symmetry.rs | 53 +++++-------------- 4 files changed, 33 insertions(+), 108 deletions(-) diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index 09ff5c75d6..c6dbdf12a7 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -1002,8 +1002,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); @@ -5080,21 +5081,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/parser/oracle.rs b/crates/engine/src/parser/oracle.rs index fd37e05559..5dfc0cbb8f 100644 --- a/crates/engine/src/parser/oracle.rs +++ b/crates/engine/src/parser/oracle.rs @@ -18,7 +18,6 @@ use crate::types::ability::{ TargetFilter, TriggerCondition, TriggerDefinition, TypedFilter, }; use crate::types::format::DeckCopyLimit; -use crate::types::game_state::TargetSelectionConstraint; use crate::types::keywords::{EscapeCost, FlashbackCost, Keyword, KeywordKind}; use crate::types::mana::ManaCost; use crate::types::phase::Phase; @@ -2740,56 +2739,6 @@ fn parse_flash_cleanup_sacrifice_casting_option( /// `OracleDocIr.diagnostics`. That keeps the doc IR the single warning channel /// (`OracleDocIr.diagnostics` → `ParsedAbilities.parse_warnings`) rather than /// letting the audit direct-append to `parse_warnings` behind the doc's back. -/// CR 115.1b + CR 601.2c: a single instruction that chooses two or more target -/// PLAYERS requires them to be different — the same player can't be chosen for -/// more than one of that instruction's target slots (Scheming Symmetry, issue -/// #6459: "Choose two target players." allowed picking the same player twice). -/// -/// Modal cards express this via `ModalSelectionConstraint::DifferentTargetPlayers` -/// (parsed from "each mode must target a different player"), but a plain -/// multi-target player requirement had no equivalent, so the runtime -/// distinctness authority (`validate_target_constraints`) never saw a constraint -/// to enforce. Attach `TargetSelectionConstraint::DifferentTargetPlayers` here -/// for every non-modal multi-target player requirement — the shared enforcement -/// then rejects a duplicate player selection uniformly (Scheming Symmetry, and -/// the "any number of target players" class). The constraint is a no-op for a -/// selection of fewer than two players, so attaching it whenever the requirement -/// CAN select multiple players is always safe. -fn ensure_distinct_player_multi_targets(result: &mut ParsedAbilities) { - for ability in &mut result.abilities { - ensure_distinct_player_multi_target(ability); - } - for trigger in &mut result.triggers { - if let Some(execute) = trigger.execute.as_mut() { - ensure_distinct_player_multi_target(execute); - } - } -} - -fn ensure_distinct_player_multi_target(def: &mut AbilityDefinition) { - if def.multi_target.is_none() { - return; - } - // Only player-target requirements are governed by this constraint; an - // object multi-target ("two target creatures") is kept distinct by the - // per-slot legal-set exclusion instead. - if !matches!( - def.effect.target_filter(), - Some(TargetFilter::Player | TargetFilter::Opponent) - ) { - return; - } - if def - .target_constraints - .iter() - .any(|c| matches!(c, TargetSelectionConstraint::DifferentTargetPlayers)) - { - return; - } - def.target_constraints - .push(TargetSelectionConstraint::DifferentTargetPlayers); -} - pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities { let mut result = ParsedAbilities { abilities: Vec::new(), @@ -2898,7 +2847,6 @@ pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities { &static_ids, ); reconcile_host_bound_phase_outs(&mut result); - ensure_distinct_player_multi_targets(&mut result); apply_linked_choice_persisted_player(&mut result, &ir.relations, &ability_ids, &trigger_ids); // Architectural rule: the parser must never silently discard Oracle text. Run diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 3f53126b8c..f0b29c8fc7 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -6205,14 +6205,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 index 99e8e04f4b..6ecc28e557 100644 --- a/crates/engine/tests/integration/issue_6459_scheming_symmetry.rs +++ b/crates/engine/tests/integration/issue_6459_scheming_symmetry.rs @@ -1,18 +1,17 @@ //! Scheming Symmetry — "Choose two target players." must require TWO DIFFERENT -//! players (CR 115.1b + CR 601.2c: the same player can't be chosen to fill more -//! than one of a single instruction's target slots). +//! 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. A modal "each mode must target a different player" -//! card already carries `DifferentTargetPlayers`, but a plain multi-target -//! player requirement had no constraint attached, so the runtime distinctness -//! authority never saw one to enforce. +//! 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`. //! -//! Two layers of proof: -//! * parser — the parsed ability carries `DifferentTargetPlayers`; -//! * runtime — after the first player is chosen, choosing that SAME player for -//! the second slot is rejected, while a DIFFERENT player is accepted (the -//! discriminating end-to-end behaviour). +//! 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; @@ -30,32 +29,6 @@ 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_parses_distinct_player_constraint() { - use engine::types::game_state::TargetSelectionConstraint; - - let parsed = engine::parser::parse_oracle_text( - SCHEMING, - "Scheming Symmetry", - &[], - &["Sorcery".to_string()], - &[], - ); - let ability = parsed - .abilities - .first() - .expect("Scheming Symmetry lowers to a spell ability"); - assert!( - ability - .target_constraints - .iter() - .any(|c| matches!(c, TargetSelectionConstraint::DifferentTargetPlayers)), - "a \"choose two target players\" requirement must carry \ - DifferentTargetPlayers (issue #6459); got {:?}", - ability.target_constraints - ); -} - #[test] fn scheming_symmetry_rejects_choosing_the_same_player_twice() { let mut scenario = GameScenario::new_n_player(3, 42); @@ -105,7 +78,7 @@ fn scheming_symmetry_rejects_choosing_the_same_player_twice() { .expect("choosing P1 for the first slot must succeed"); // Second slot: choosing the ALREADY-CHOSEN player P1 must be rejected - // (CR 115.1b), while the state stays on the same target slot. + // (CR 601.2c + CR 115.3), while the state stays on the same target slot. assert!( matches!( runner.state().waiting_for, @@ -119,8 +92,8 @@ fn scheming_symmetry_rejects_choosing_the_same_player_twice() { }); assert!( reselect_same.is_err(), - "CR 115.1b (issue #6459): choosing the already-chosen player P1 for the \ - second slot must be rejected" + "CR 601.2c + CR 115.3 (issue #6459): choosing the already-chosen player \ + P1 for the second slot must be rejected" ); assert!( matches!(