Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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 @@ -1921,7 +1921,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 @@ -2006,7 +2006,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