Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/engine/src/database/synthesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9712,7 +9712,7 @@ pub fn synthesize_siege_intrinsics(face: &mut CardFace) {
protector_replacement.execute = Some(Box::new(AbilityDefinition::new(
AbilityKind::Spell,
Effect::Choose {
choice_type: ChoiceType::Opponent { restriction: None },
choice_type: ChoiceType::opponent(),
persist: true,
selection: crate::types::ability::TargetSelectionMode::Chosen,
},
Expand Down Expand Up @@ -9841,7 +9841,7 @@ pub fn synthesize_tribute_intrinsics(face: &mut CardFace) {
let choose_stage = AbilityDefinition::new(
AbilityKind::Spell,
Effect::Choose {
choice_type: ChoiceType::Opponent { restriction: None },
choice_type: ChoiceType::opponent(),
persist: true,
selection: crate::types::ability::TargetSelectionMode::Chosen,
},
Expand Down
10 changes: 5 additions & 5 deletions crates/engine/src/game/ability_rw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3435,7 +3435,7 @@ fn legacy_guess_subject(subject: &GuessSubject) -> bool {

fn legacy_choice_type(choice_type: &crate::types::ability::ChoiceType) -> bool {
match choice_type {
crate::types::ability::ChoiceType::Opponent { restriction } => {
crate::types::ability::ChoiceType::Opponent { restriction, .. } => {
restriction.as_deref().is_some_and(legacy_player_filter)
}
crate::types::ability::ChoiceType::CreatureType { .. }
Expand All @@ -3449,7 +3449,7 @@ fn legacy_choice_type(choice_type: &crate::types::ability::ChoiceType) -> bool {
| crate::types::ability::ChoiceType::LandType
| crate::types::ability::ChoiceType::CardPredicate { .. }
| crate::types::ability::ChoiceType::CardPredicateGuess { .. }
| crate::types::ability::ChoiceType::Player
| crate::types::ability::ChoiceType::Player { .. }
| crate::types::ability::ChoiceType::TwoColors
| crate::types::ability::ChoiceType::Word
| crate::types::ability::ChoiceType::Artist
Expand Down Expand Up @@ -5728,7 +5728,7 @@ fn rw_guess_subject(subject: &GuessSubject) -> RwProfile {

fn rw_choice_type(choice_type: &crate::types::ability::ChoiceType) -> RwProfile {
match choice_type {
crate::types::ability::ChoiceType::Opponent { restriction } => match restriction {
crate::types::ability::ChoiceType::Opponent { restriction, .. } => match restriction {
Some(filter) => rw_player_filter(filter),
None => RwProfile::empty(),
},
Expand All @@ -5743,7 +5743,7 @@ fn rw_choice_type(choice_type: &crate::types::ability::ChoiceType) -> RwProfile
| crate::types::ability::ChoiceType::LandType
| crate::types::ability::ChoiceType::CardPredicate { .. }
| crate::types::ability::ChoiceType::CardPredicateGuess { .. }
| crate::types::ability::ChoiceType::Player
| crate::types::ability::ChoiceType::Player { .. }
| crate::types::ability::ChoiceType::TwoColors
| crate::types::ability::ChoiceType::Word
| crate::types::ability::ChoiceType::Artist
Expand Down Expand Up @@ -6824,7 +6824,7 @@ mod tests {
#[test]
fn b7_choose_persist_member_bound() {
let choose = |persist: bool| Effect::Choose {
choice_type: ChoiceType::Opponent { restriction: None },
choice_type: ChoiceType::opponent(),
persist,
selection: TargetSelectionMode::default(),
};
Expand Down
2 changes: 1 addition & 1 deletion crates/engine/src/game/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2030,7 +2030,7 @@ fn fmt_choice_type(ct: &ChoiceType) -> String {
ChoiceType::CardPredicate { .. } => "card predicate",
ChoiceType::CardPredicateGuess { .. } => "card predicate guess",
ChoiceType::Opponent { .. } => "opponent",
ChoiceType::Player => "player",
ChoiceType::Player { .. } => "player",
ChoiceType::TwoColors => "two colors",
ChoiceType::Word => "word",
ChoiceType::Artist => "artist",
Expand Down
116 changes: 90 additions & 26 deletions crates/engine/src/game/effects/choose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use rand::Rng;

use crate::game::players;
use crate::types::ability::{
ChoiceType, ChoiceValue, ChosenAttribute, Effect, EffectError, EffectKind, ResolvedAbility,
SeatDirection, TargetSelectionMode,
ChoiceType, ChoiceValue, ChosenAttribute, Effect, EffectError, EffectKind,
PlayerChoiceDistinctness, ResolvedAbility, SeatDirection, TargetSelectionMode,
};
use crate::types::card_type::CoreType;
use crate::types::events::GameEvent;
Expand Down Expand Up @@ -180,7 +180,7 @@ pub(crate) fn resolve_random_in_chain(
// it to the sub via `apply_parent_chain_context`.
if matches!(
choice_type,
ChoiceType::Player | ChoiceType::Opponent { .. }
ChoiceType::Player { .. } | ChoiceType::Opponent { .. }
) {
if let Ok(pid) = chosen.parse::<u8>() {
let mut updated = ability.chosen_players.clone();
Expand Down Expand Up @@ -306,7 +306,7 @@ pub(crate) fn bind_named_choice(
| ChoiceType::BasicLandType
| ChoiceType::Color { .. }
| ChoiceType::Keyword { .. }
| ChoiceType::Player
| ChoiceType::Player { .. }
| ChoiceType::Opponent { .. }
// CR 613.1: A persisted `Label` gates `ChosenLabelIs`
// continuous statics — anchor-word modal permanents
Expand Down Expand Up @@ -505,12 +505,16 @@ const LAND_TYPES: &[&str] = &[
/// casting or resolution. If an option would be illegal, it can't be chosen.
///
/// `already_chosen` is the resolution-scoped list of players picked by earlier
/// `Choose(Player)` instructions in this chain. CR 608.2c + the Gluntch card
/// ruling ("three distinct players") require each successive "choose a player"
/// to exclude players already chosen — `ChoiceType::Player` and
/// `ChoiceType::Opponent` filter them out. When fewer eligible players remain
/// than the card asks for, the options list is empty and the choice (and its
/// dependent effect) does nothing — the standard empty-options path.
/// `Choose(Player)` instructions in this chain. `ChoiceType::Player` and
/// `ChoiceType::Opponent` only consult it when their `distinctness` is
/// `DistinctFromPriorChoices` (CR 608.2c + the Gluntch ordinal-cued "choose a
/// second/third player" ruling, "three distinct players"). The default
/// `Independent` distinctness never filters on it — the "Offering" cycle
/// ruling (Benevolent/Infernal/Intellectual/Sylvan Offering) confirms a
/// repeated "Choose an opponent." may pick the same player again. When
/// `DistinctFromPriorChoices` narrows the eligible set below what the card
/// asks for, the options list is empty and the choice (and its dependent
/// effect) does nothing — the standard empty-options path.
fn compute_options(
state: &GameState,
choice_type: &ChoiceType,
Expand Down Expand Up @@ -612,15 +616,23 @@ fn compute_options(
// (in a free-for-all game, every other player). `players::opponents`
// already drops eliminated players (CR 104.3a — a player who loses
// leaves the game and is no longer an opponent).
// CR 608.2c: Exclude players already chosen earlier in this resolution.
// CR 608.2c: `DistinctFromPriorChoices` excludes players already chosen
// earlier in this resolution; the default `Independent` does not (the
// "Offering" cycle may repeat the same opponent).
// CR 102.3 + CR 608.2d: When a `restriction` is present ("with the most
// life among your opponents"), narrow the eligible set to opponents
// satisfying that `PlayerFilter` — the controller then picks ONE of the
// qualifying opponents (CR 608.2d handles ties), keeping it a single
// pick rather than fanning the effect out to every tied opponent.
ChoiceType::Opponent { restriction } => players::opponents(state, controller)
ChoiceType::Opponent {
restriction,
distinctness,
} => players::opponents(state, controller)
.iter()
.filter(|id| !already_chosen.contains(id))
.filter(|id| {
*distinctness != PlayerChoiceDistinctness::DistinctFromPriorChoices
|| !already_chosen.contains(id)
})
.filter(|id| {
restriction.as_ref().is_none_or(|filter| {
super::matches_player_scope(state, **id, filter, controller, source_id)
Expand All @@ -629,11 +641,16 @@ fn compute_options(
.map(|id| id.0.to_string())
.collect(),
// CR 102.1: A player is one of the people in the game.
// CR 608.2c: Exclude players already chosen earlier in this resolution.
ChoiceType::Player => state
// CR 608.2c: `DistinctFromPriorChoices` (Gluntch's "choose a
// second/third player") excludes players already chosen earlier in
// this resolution; the default `Independent` does not.
ChoiceType::Player { distinctness } => state
.seat_order
.iter()
.filter(|id| !already_chosen.contains(id))
.filter(|id| {
*distinctness != PlayerChoiceDistinctness::DistinctFromPriorChoices
|| !already_chosen.contains(id)
})
.map(|id| id.0.to_string())
.collect(),
ChoiceType::TwoColors => two_color_options(),
Expand Down Expand Up @@ -1266,7 +1283,7 @@ mod tests {
#[test]
fn choose_opponent_lists_opponents() {
let mut state = GameState::new_two_player(42);
let ability = make_choose_ability(ChoiceType::Opponent { restriction: None });
let ability = make_choose_ability(ChoiceType::opponent());
let mut events = Vec::new();
resolve(&mut state, &ability, &mut events).unwrap();
match &state.waiting_for {
Expand All @@ -1278,10 +1295,56 @@ mod tests {
}
}

/// Issue #6381 (Benevolent Offering): the "Offering" cycle ruling —
/// "You may choose the same opponent for each of the effects, or you may
/// choose different opponents" — means the default `Independent`
/// distinctness must NOT exclude an opponent chosen by an earlier
/// `Choose(Opponent)` in the same resolution. In a two-player game this is
/// the difference between a legal repeat pick (correct) and an impossible
/// no-op second choice (the reported bug).
#[test]
fn choose_opponent_independent_by_default_allows_repeat_choice() {
let mut state = GameState::new_two_player(42);
let mut ability = make_choose_ability(ChoiceType::opponent());
ability.chosen_players = vec![PlayerId(1)];
let mut events = Vec::new();
resolve(&mut state, &ability, &mut events).unwrap();
match &state.waiting_for {
WaitingFor::NamedChoice { options, .. } => {
assert_eq!(
options,
&["1"],
"the previously-chosen opponent must remain a legal repeat pick"
);
}
other => panic!("Expected NamedChoice, got {:?}", other),
}
}

#[test]
fn choose_player_lists_all_players() {
let mut state = GameState::new_two_player(42);
let ability = make_choose_ability(ChoiceType::Player);
let ability = make_choose_ability(ChoiceType::player());
let mut events = Vec::new();
resolve(&mut state, &ability, &mut events).unwrap();
match &state.waiting_for {
WaitingFor::NamedChoice { options, .. } => {
assert_eq!(options.len(), 2);
assert!(options.contains(&"0".to_string()));
assert!(options.contains(&"1".to_string()));
}
other => panic!("Expected NamedChoice, got {:?}", other),
}
}

#[test]
fn choose_player_independent_by_default_allows_repeat_choice() {
// The default `Independent` distinctness (bare "choose a player") does
// NOT exclude a player already chosen earlier in this resolution —
// only the ordinal-cued `DistinctFromPriorChoices` (Gluntch) does.
let mut state = GameState::new_two_player(42);
let mut ability = make_choose_ability(ChoiceType::player());
ability.chosen_players = vec![PlayerId(0)];
let mut events = Vec::new();
resolve(&mut state, &ability, &mut events).unwrap();
match &state.waiting_for {
Expand All @@ -1295,11 +1358,12 @@ mod tests {
}

#[test]
fn choose_player_excludes_already_chosen_players() {
// CR 608.2c + Gluntch ruling: a successive "choose a player" omits
// players already chosen earlier in the same resolution.
fn choose_player_distinct_from_prior_excludes_already_chosen_players() {
// CR 608.2c + Gluntch ruling ("choose a second/third player"): a
// successive `DistinctFromPriorChoices` pick omits players already
// chosen earlier in the same resolution.
let mut state = GameState::new_two_player(42);
let mut ability = make_choose_ability(ChoiceType::Player);
let mut ability = make_choose_ability(ChoiceType::player_distinct_from_prior());
ability.chosen_players = vec![PlayerId(0)];
let mut events = Vec::new();
resolve(&mut state, &ability, &mut events).unwrap();
Expand All @@ -1312,7 +1376,7 @@ mod tests {
}

#[test]
fn choose_player_with_all_players_chosen_resolves_as_no_op() {
fn choose_player_distinct_from_prior_with_all_players_chosen_resolves_as_no_op() {
// CR 609.3 (issue #3040): when every eligible player is already chosen,
// the engine-enumerated option set is empty — choosing is impossible, so
// the choice does nothing and resolution continues. It must NOT raise a
Expand All @@ -1324,7 +1388,7 @@ mod tests {
state.waiting_for = WaitingFor::Priority {
player: PlayerId(0),
};
let mut ability = make_choose_ability(ChoiceType::Player);
let mut ability = make_choose_ability(ChoiceType::player_distinct_from_prior());
ability.chosen_players = vec![PlayerId(0), PlayerId(1)];
let mut events = Vec::new();
resolve(&mut state, &ability, &mut events).unwrap();
Expand Down Expand Up @@ -1425,7 +1489,7 @@ mod tests {
let mut state = GameState::new_two_player(42);
let mut ability = ResolvedAbility::new(
Effect::Choose {
choice_type: ChoiceType::Player,
choice_type: ChoiceType::player(),
persist: false,
selection: TargetSelectionMode::Random,
},
Expand Down Expand Up @@ -1454,7 +1518,7 @@ mod tests {
// Building-block regression: a Chosen Choose is left to the interactive
// `resolve` path (returns false; raises nothing here).
let mut state = GameState::new_two_player(42);
let mut ability = make_choose_ability(ChoiceType::Player);
let mut ability = make_choose_ability(ChoiceType::player());
let mut events = Vec::new();
assert!(!resolve_random_in_chain(
&mut state,
Expand Down
2 changes: 1 addition & 1 deletion crates/engine/src/game/engine_resolution_choices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5633,7 +5633,7 @@ pub(super) fn handle_resolution_choice(
// single GameState slot cleared after every drain.
if matches!(
choice_type,
ChoiceType::Player | ChoiceType::Opponent { .. }
ChoiceType::Player { .. } | ChoiceType::Opponent { .. }
) {
if let Ok(pid) = choice.parse::<u8>() {
if let Some(frame) = state.active_ability_continuation_frame_mut() {
Expand Down
2 changes: 1 addition & 1 deletion crates/engine/src/game/triggers_ordering_parity_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1906,7 +1906,7 @@ fn choose_opponent_then_draw() -> ResolvedAbility {
target: TargetFilter::Controller,
});
ra(Effect::Choose {
choice_type: ChoiceType::Opponent { restriction: None },
choice_type: ChoiceType::opponent(),
persist: false,
selection: TargetSelectionMode::default(),
})
Expand Down
4 changes: 2 additions & 2 deletions crates/engine/src/parser/oracle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1863,7 +1863,7 @@ fn ability_chain_has_player_choice(def: &AbilityDefinition) -> bool {
matches!(
def.effect.as_ref(),
Effect::Choose {
choice_type: ChoiceType::Player | ChoiceType::Opponent { .. },
choice_type: ChoiceType::Player { .. } | ChoiceType::Opponent { .. },
..
}
) || def
Expand Down Expand Up @@ -1948,7 +1948,7 @@ fn filter_references_source_chosen_player(filter: &TargetFilter) -> bool {
/// sub-ability chain) to `persist: true` so its choice is stored durably.
fn persist_player_choice_in_ability(def: &mut AbilityDefinition) {
if let Effect::Choose {
choice_type: ChoiceType::Player | ChoiceType::Opponent { .. },
choice_type: ChoiceType::Player { .. } | ChoiceType::Opponent { .. },
persist,
..
} = def.effect.as_mut()
Expand Down
Loading
Loading