diff --git a/crates/engine/src/ai_support/candidates.rs b/crates/engine/src/ai_support/candidates.rs index 01dc85c51a..5853fe3380 100644 --- a/crates/engine/src/ai_support/candidates.rs +++ b/crates/engine/src/ai_support/candidates.rs @@ -18,7 +18,8 @@ use crate::types::counter::CounterMatch; use crate::types::game_state::{ CastOfferKind, CastPaymentMode, CastingVariant, CompanionDeclaration, ConvokeMode, CostResume, CounterCostChoice, CounterMoveChoice, CounterRemoveChoice, GameState, MulliganDecisionPhase, - PayCostKind, PayableResource, PendingMulliganAction, TargetSelectionSlot, WaitingFor, + PayCostKind, PayableResource, PendingMulliganAction, RetargetScope, TargetSelectionSlot, + WaitingFor, }; use crate::types::identifiers::ObjectId; use crate::types::interaction::MAX_INTERACTION_LIST_LEN; @@ -3039,20 +3040,26 @@ pub fn candidate_actions_broad_with_probe( )] } } - // CR 115.7: Retarget — keep current targets as default. + // CR 115.7a: propose every legal alternative. The previous arm proposed + // ONLY `current_targets`, which `apply_retarget` rejects whenever the + // current target is not in `legal_new_targets` — three rejections in a + // row halt the AI controller and freeze the game. WaitingFor::RetargetChoice { player, + stack_entry_index, + scope, current_targets, - .. - } => { - vec![candidate( - GameAction::RetargetSpell { - new_targets: current_targets.clone(), - }, - TacticalClass::Selection, - Some(*player), - )] - } + legal_new_targets, + } => retarget_actions( + state, + *stack_entry_index, + scope, + current_targets, + legal_new_targets, + ) + .into_iter() + .map(|action| candidate(action, TacticalClass::Selection, Some(*player))) + .collect(), // CR 701.62a: AI selects one card to manifest — one action per card option WaitingFor::ManifestDreadChoice { player, cards, .. } => { if cards.is_empty() { @@ -3537,6 +3544,160 @@ fn authorize_candidate_actors(state: &GameState, actions: &mut [CandidateAction] } } +/// CR 115.7: Submissions `apply_retarget` will accept for a parked +/// `RetargetChoice`, derived from the prompt's own payload and filtered through +/// the same per-slot authority the reducer consults. Sound by construction, and +/// complete for `Single`; the `All` arm bounds its enumeration deliberately — +/// see ENUMERATION BOUND below. Not a legality oracle: a submission absent from +/// this set is not thereby illegal. +/// +/// Shared by the engine's candidate generator and `phase-ai`'s fallback so the +/// two cannot disagree about what a legal retarget is — they previously agreed +/// only in being wrong the same way. +pub fn retarget_actions( + state: &GameState, + stack_entry_index: usize, + scope: &RetargetScope, + current_targets: &[TargetRef], + legal_new_targets: &[TargetRef], +) -> Vec { + // CR 115.7a: the pool is FLAT for a multi-role mana node — it `flat_map`s + // every surfaced role filter together, so it is a per-slot SUPERSET + // (`change_targets.rs`, multi-role branch). `apply_retarget` re-checks each + // changed submission positionally via `retarget_slot_violation`, so a + // proposal built from a pool member legal only for another slot would be + // rejected. Consult the same authority here rather than re-deriving + // legality, so every proposed action is accepted by construction — + // including CR 115.7d's unchanged submissions, which that authority exempts. + let slot_legal = |new_targets: &[TargetRef]| { + state + .stack + .get(stack_entry_index) + .and_then(|entry| entry.ability()) + .is_none_or(|ability| { + crate::game::ability_utils::retarget_slot_violation( + state, + ability, + current_targets, + new_targets, + ) + .is_none() + }) + }; + + match scope { + // CR 115.7a: "each target can be changed only to another legal target." + // One proposal per member of the pool `apply_retarget` validates + // against. That pool normally still contains the current target — that + // member IS CR 115.7a's "the original target is unchanged" fallback, so + // it is offered rather than filtered out. An empty pool yields no + // proposals; after Unit 1 `change_targets::resolve` no longer parks that + // state. + // + // KNOWN GAP, carried deliberately. Each proposal is a ONE-element list, + // because `apply_retarget`'s `Single` arm hard-requires exactly one + // target. When the parked entry's `current_targets` has MORE than one + // element, applying such a proposal assigns `ability.targets` that + // one-element list and TRUNCATES the remaining slots — contrary to BOTH + // subrules that reach this arm (CR 115.7a / CR 115.7b), neither of which + // permits an undisturbed slot to be dropped. + // `change_targets::forced_retarget_targets` already implements that slot + // preservation on the FORCED path; the interactive path has no + // equivalent, and cannot have one while the reducer's arm rejects any + // length but 1. + // + // TWO TEMPLATES, TWO REMEDIES. `try_parse_change_targets` + // (parser/oracle_effect/mod.rs) maps two oracle wordings governed by + // DIFFERENT subrules onto this one `RetargetScope::Single` variant, so + // the deferred fix must DISPATCH ON THE TEMPLATE rather than apply CR + // 115.7b's remedy uniformly: + // - "change a target of " → CR 115.7b: "the process described in rule + // 115.7a is followed, except that only one of those targets may be + // changed (rather than all of them or none of them)". Remedy: one + // slot changes, every other declared target stays in place. + // - "change the target of " → CR 115.7a, which ends: "If all the + // targets aren't changed to other legal targets, none of them are + // changed." Remedy for a multi-target entry: ALL-OR-NONE, not + // one-changes-rest-stay. This is Bolt Bend's wording. Bolt Bend + // supplies the WORDING only: it reads "with a single target". Of the + // 22 printed cards matching `o:"change the target of"`, the six that + // omit that literal phrase (I'm Rubber You're Glue, Muck Drubb, + // Rebound, Ricochet, Silver Wyvern, Torchling) each restrict to one + // target by the equivalent "targets ONLY " / "a single " + // construction instead. So no printed card on this template can + // present a multi-target entry — the gap below is reachable today + // only by synthetic stack entries, which is why its fixture is + // labelled synthetic rather than card-driven. + // + // DEFERRED(out-of-run): interactive Single-scope retarget collapses + // multi-target lists (CR 115.7a / CR 115.7b) — upstream cause filter.rs + // FilterProp::HasSingleTarget is permissive with no resolution-time + // validation; fix needs filter.rs + interaction.rs, both outside phase + // 1's frozen scope. + // + // Behavioural delta this phase knowingly takes: at base the AI FROZE on + // this class (its sole proposal was rejected, so nothing could discharge + // the prompt); now it PROGRESSES and TRUNCATES. Pinned by + // `retarget_prompt_softlock.rs` row 2e and `phase-ai`'s + // `retarget_fallback_action.rs` row 2f, whose SCOPE notes record the + // acceptance as observed behaviour and explicitly not as CR-115.7a / + // CR-115.7b legality. + RetargetScope::Single => legal_new_targets + .iter() + .map(|target| vec![target.clone()]) + .filter(|new_targets| slot_legal(new_targets)) + .map(|new_targets| GameAction::RetargetSpell { new_targets }) + .collect(), + // CR 115.7d: "the player may leave any number of the targets unchanged, + // even if those targets would be illegal." Leaving every target + // unchanged anchors the list; each single-slot substitution to another + // legal target is offered on top of it. The anchor goes through the same + // `slot_legal` filter as every other proposal and passes unconditionally, + // because `retarget_slot_violation` exempts unchanged positions — no + // carve-out is needed, and none is made. + // + // ENUMERATION BOUND, stated rather than left silent: this emits the + // unchanged anchor plus every SINGLE-slot substitution. CR 115.7d permits + // changing several targets at once ("any number of the targets + // unchanged"), and those simultaneous multi-slot proposals are NOT + // enumerated — the set would be the product of the per-slot pools, and + // bounding the AI's branching factor is worth more than the extra + // proposals. This is a search-space bound, not a legality claim: the + // reducer accepts a multi-slot change if some other agent submits one, + // because `retarget_slot_violation` validates each changed position + // independently and never requires that only one position moved. + RetargetScope::All => { + let mut actions = Vec::new(); + let anchor = current_targets.to_vec(); + if slot_legal(&anchor) { + actions.push(GameAction::RetargetSpell { + new_targets: anchor, + }); + } + for slot in 0..current_targets.len() { + for target in legal_new_targets { + if current_targets[slot] == *target { + continue; + } + let mut new_targets = current_targets.to_vec(); + new_targets[slot] = target.clone(); + if slot_legal(&new_targets) { + actions.push(GameAction::RetargetSpell { new_targets }); + } + } + } + actions + } + // A forced retarget is applied by `change_targets::resolve` without a + // prompt, so this scope never reaches an interactive `RetargetChoice`; + // `apply_retarget` rejects it unconditionally (`engine.rs`, the + // `RetargetScope::ForcedTo(_)` arm) and the parser emits only + // `Single`/`All` (`parser/oracle_effect/mod.rs`, the retarget-scope + // combinator). There is no legal submission to propose. + RetargetScope::ForcedTo(_) => Vec::new(), + } +} + fn candidate( action: GameAction, tactical_class: TacticalClass, diff --git a/crates/engine/src/ai_support/mod.rs b/crates/engine/src/ai_support/mod.rs index 0d62fdb000..10907bceb0 100644 --- a/crates/engine/src/ai_support/mod.rs +++ b/crates/engine/src/ai_support/mod.rs @@ -38,7 +38,7 @@ use crate::types::zones::Zone; pub(crate) use candidates::power_threshold_witness; pub use candidates::{ candidate_actions, candidate_actions_broad, candidate_actions_exact, - candidate_actions_with_probe, ActionMetadata, CandidateAction, TacticalClass, + candidate_actions_with_probe, retarget_actions, ActionMetadata, CandidateAction, TacticalClass, }; pub use combat_withdrawal::{ combat_withdrawal_fact_for_current_target, CombatWithdrawalFact, CombatWithdrawalTargetRole, diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index a4b33bf2e5..cb46f2c054 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -7387,15 +7387,38 @@ fn chain_has_target_sink_after_deferred_effect(sub_ability: Option<&ResolvedAbil /// CR 115.7a: "each target can be changed only to another legal target." A /// multi-slot node's replacement targets are submitted positionally, but -/// `legal_new_targets_for_stack_ability` can only return a FLAT union pool +/// `legal_new_targets_for_stack_entry` can only return a FLAT union pool /// (one `Vec`, no slot structure), so the union alone would let a /// count-source-legal player be assigned into the recipient slot. This is the /// seam where slot identity IS available: re-validate each submitted target /// against the filter of the slot it actually lands in. /// -/// Returns `Some(slot_index)` for the first positionally-illegal submission. -/// `None` = the submission is slot-legal, or this node declares no per-slot -/// structure this function knows about. +/// Takes the prompt's `current_targets` and **exempts positions whose submission +/// is unchanged**: CR 115.7d ("the player may leave any number of the targets +/// unchanged, even if those targets would be illegal") licenses this outright for +/// the "choose new targets" scope. CR 115.7a does not grant an equivalent licence +/// — its unchanged-target allowance is conditional ("if a target can't be changed +/// to another legal target") — but it does not need to: it constrains only targets +/// that ARE changed, so a slot already holding its own submission was never +/// changed and "changed only to another legal target" has nothing to bite on. +/// The exemption is therefore correct without a scope parameter, by licence under +/// 115.7d and by non-application under 115.7a. +/// +/// CR 115.7d's SECOND sentence — new targets "must not cause any unchanged targets +/// to become illegal" — is vacuous under today's model and is deliberately not +/// enforced here: `validate_targets_for_ability` evaluates each slot's filter +/// against the state and the ability, never against a sibling slot's choice, so no +/// submission can invalidate a neighbour. A future filter that reads sibling slots +/// must revisit this. +/// +/// Consumed by `engine::apply_retarget` AND by +/// `ai_support::candidates::retarget_actions`, so the reducer and the AI +/// generator cannot disagree about which submissions are legal. +/// +/// Returns `Some(slot_index)` for the first positionally-illegal CHANGED +/// submission. `None` = the submission is slot-legal, every illegal position was +/// left unchanged, or this node declares no per-slot structure this function +/// knows about. /// /// SCOPE: today this recognizes any node `mana_multi_role` admits — both the /// two-surfaced-slot `Both` and the one-surfaced-slot context-ref recipient @@ -7407,19 +7430,47 @@ fn chain_has_target_sink_after_deferred_effect(sub_ability: Option<&ResolvedAbil pub fn retarget_slot_violation( state: &GameState, ability: &ResolvedAbility, + current_targets: &[TargetRef], new_targets: &[TargetRef], ) -> Option { let role = mana_multi_role(&ability.effect)?; role.surfaced_filters() .zip(new_targets.iter()) - .position(|((_slot, filter), submitted)| { - targeting::validate_targets_for_ability( + .enumerate() + .find_map(|(slot, ((_slot, filter), submitted))| { + // CR 115.7d: "the player may leave any number of the targets + // unchanged, even if those targets would be illegal." CR 115.7a says + // the same thing for the other scope from the other direction: a + // target is "changed only to another legal target", and a slot + // already holding its own submission was not changed at all. So a + // position whose submission equals its current target is exempt + // under BOTH retarget scopes, which is why this authority needs no + // scope parameter. + // + // `apply_retarget`'s pool-membership stage already exempts exactly + // these positions (its `All` arm's `continue` on + // `current_targets.get(idx) == Some(target)`); before this, the + // per-slot stage re-rejected them, so the one submission CR 115.7d + // guarantees — leave everything unchanged — was refused for every + // node `mana_multi_role` admits whose current target had become + // slot-illegal. The forced seam + // (`change_targets::forced_retarget_targets`) has always conjoined + // "changes" with "legal", and its doc already claims parity with + // this function; this is that same conjunction, here. + // + // Index `current_targets` rather than zipping it: a third `.zip` + // would truncate the scan and silently skip validation for any + // position beyond `current_targets.len()`, where `get` correctly + // yields `None` (no current target cannot be "unchanged"). + let changes = current_targets.get(slot) != Some(submitted); + let illegal = targeting::validate_targets_for_ability( state, std::slice::from_ref(submitted), filter, ability, ) - .is_empty() + .is_empty(); + (changes && illegal).then_some(slot) }) } @@ -8268,7 +8319,7 @@ mod tests { } /// Matrix row 8b — CR 115.7a: "each target can be changed only to another - /// legal target." A flat `legal_new_targets_for_stack_ability` union pool + /// legal target." A flat `legal_new_targets_for_stack_entry` union pool /// cannot express per-slot legality, so `retarget_slot_violation` re-checks /// each submission against the filter of the slot it actually lands in. #[test] @@ -8300,11 +8351,17 @@ mod tests { assert_eq!(role.surfaced_filters().count(), 2); // Positive: a slot-legal submission is accepted. Without this the - // negative below could pass because EVERYTHING is rejected. + // negative below could pass because EVERYTHING is rejected. Both slots + // genuinely CHANGE against the current targets passed here, so this case + // proves legality rather than the CR 115.7d unchanged-position exemption. assert_eq!( retarget_slot_violation( &state, &ability, + &[ + TargetRef::Player(PlayerId(1)), + TargetRef::Player(PlayerId(0)), + ], &[ TargetRef::Player(PlayerId(0)), TargetRef::Player(PlayerId(1)), @@ -8315,11 +8372,17 @@ mod tests { ); // Negative: P0 is in the flat union pool (legal for the recipient slot) - // but illegal in the COUNT SOURCE slot it was submitted into. + // but illegal in the COUNT SOURCE slot it was submitted into. Slot 1 + // genuinely changes (P1 -> P0) against the current targets, so the + // exemption does not apply and the violation must be reported. assert_eq!( retarget_slot_violation( &state, &ability, + &[ + TargetRef::Player(PlayerId(1)), + TargetRef::Player(PlayerId(1)), + ], &[ TargetRef::Player(PlayerId(1)), TargetRef::Player(PlayerId(0)), @@ -8331,6 +8394,82 @@ mod tests { ); } + /// Matrix row 2d — CR 115.7d: "the player may leave any number of the + /// targets unchanged, even if those targets would be illegal." A submission + /// that changes nothing must never be rejected for slot legality, even when + /// its current target is illegal for the slot it sits in. CR 115.7a licenses + /// the same exemption for the "change the target(s)" scope: a slot already + /// holding its own submission was not changed at all. + #[test] + fn retarget_slot_violation_exempts_an_unchanged_illegal_target() { + use crate::types::ability::ManaTargetRole; + + let mut state = GameState::new_two_player(24); + let source = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Retarget Mana Source".to_string(), + Zone::Battlefield, + ); + + // Recipient: any player. Count source: an OPPONENT of P0 (i.e. P1 only). + // P0 is therefore legal for slot 0 and ILLEGAL for slot 1. + let role = ManaTargetRole::Both { + recipient: TargetFilter::Player, + count_source: TargetFilter::Typed( + TypedFilter::default().controller(ControllerRef::Opponent), + ), + }; + let ability = mana_ability_with_role(role.clone(), source); + + // Reach guard: the node is admitted and has two discriminable slots. + assert!(mana_multi_role(&ability.effect).is_some()); + assert_eq!(role.surfaced_filters().count(), 2); + + // Reach guard: the function still DISCRIMINATES. Slot 1 genuinely + // changes P1 -> P0 and is illegal there, so a violation is still + // reported. Without this, the exemption assertion below could pass in a + // world where this authority stopped rejecting anything at all. + assert_eq!( + retarget_slot_violation( + &state, + &ability, + &[ + TargetRef::Player(PlayerId(1)), + TargetRef::Player(PlayerId(1)), + ], + &[ + TargetRef::Player(PlayerId(1)), + TargetRef::Player(PlayerId(0)), + ], + ), + Some(1), + "reach guard: a CHANGED slot-illegal submission is still a violation" + ); + + // CR 115.7d: slot 1 holds P0, which is illegal for the opponent-only + // count-source slot — but the submission leaves it unchanged, so there + // is no violation to report. + assert_eq!( + retarget_slot_violation( + &state, + &ability, + &[ + TargetRef::Player(PlayerId(1)), + TargetRef::Player(PlayerId(0)), + ], + &[ + TargetRef::Player(PlayerId(1)), + TargetRef::Player(PlayerId(0)), + ], + ), + None, + "CR 115.7d: an unchanged position is exempt from slot legality even \ + though P0 is illegal in slot 1" + ); + } + use crate::types::ability::{ AbilityCost, AbilityKind, AggregateFunction, BounceSelection, CardTypeSetSource, CastManaObjectScope, CastManaSpentMetric, Comparator, ContinuousModification, diff --git a/crates/engine/src/game/effects/change_targets.rs b/crates/engine/src/game/effects/change_targets.rs index 83ad129089..a42dd01905 100644 --- a/crates/engine/src/game/effects/change_targets.rs +++ b/crates/engine/src/game/effects/change_targets.rs @@ -3,7 +3,7 @@ use crate::types::ability::{ Effect, EffectError, EffectKind, ResolvedAbility, TargetFilter, TargetRef, }; use crate::types::events::GameEvent; -use crate::types::game_state::{GameState, WaitingFor}; +use crate::types::game_state::{GameState, StackEntry, StackEntryKind, WaitingFor}; use crate::types::keywords::Keyword; use crate::types::ObjectId; @@ -118,13 +118,35 @@ pub fn resolve( // CR 115.7: Enumerate legal new targets by re-evaluating the stack entry's // own targeting restriction against the current game state. // - // CR 303.4a: An Aura spell's target is defined by its enchant *ability*, not + // CR 303.4a: An Aura SPELL's target is defined by its enchant *ability*, not // by its effect's target field — the synthesized spell ability carries a - // placeholder effect with no targetable filter (`target_filter()` is `None`). - // Enumerate Aura hosts from the source's `Keyword::Enchant(filter)` instead, - // mirroring the Aura branch of `casting::spell_has_legal_targets`. Non-Aura - // spells/abilities fall back to the effect's declared target filter. - let legal_new_targets = legal_new_targets_for_stack_ability(state, &stack_ability); + // placeholder effect with no targetable filter (`target_filter()` is `None`), + // so Aura hosts are enumerated from the source's `Keyword::Enchant(filter)` + // instead, mirroring the Aura branch of `casting::spell_has_legal_targets`. + // CR 115.1b: that substitution is keyed on the STACK ENTRY being the Aura + // spell — "An Aura permanent doesn't target anything; only the spell is + // targeted. (An activated or triggered ability of an Aura permanent can also + // be targeted.)" Every other entry — including a triggered or activated + // ability whose source happens to be a resident Aura — falls back to its own + // effect's declared target filter. + let legal_new_targets = legal_new_targets_for_stack_entry(state, stack_entry_index); + + // CR 115.7a: "If a target can't be changed to another legal target, the + // original target is unchanged, even if the original target is itself + // illegal by then." An empty pool IS that case, so there is no choice to + // make. Parking anyway produces a prompt nothing can discharge: + // `apply_retarget`'s `Single` arm requires membership in this (empty) set, + // and `interaction.rs`'s projection asks for N picks from zero candidates. + // Resolve as a no-change instead — mirroring the empty-`current_targets` + // no-op guard above. + if legal_new_targets.is_empty() { + events.push(GameEvent::EffectResolved { + kind: EffectKind::from(&ability.effect), + source_id: ability.source_id, + subject: None, + }); + return Ok(()); + } state.waiting_for = WaitingFor::RetargetChoice { player: ability.controller, @@ -217,24 +239,43 @@ pub fn legal_new_targets_for_stack_entry( state .stack .get(stack_entry_index) - .and_then(|entry| entry.ability()) - .map(|ability| legal_new_targets_for_stack_ability(state, ability)) + .map(|entry| legal_new_targets_for_entry(state, entry)) .unwrap_or_default() } -fn legal_new_targets_for_stack_ability( - state: &GameState, - stack_ability: &ResolvedAbility, -) -> Vec { - // CR 303.4a: An Aura spell's target is defined by its enchant ability, not - // by the placeholder effect synthesized for the spell on the stack. - if let Some(filter) = aura_enchant_filter(state, stack_ability.source_id) { - return find_legal_targets( - state, - &filter, - stack_ability.controller, - stack_ability.source_id, - ); +fn legal_new_targets_for_entry(state: &GameState, entry: &StackEntry) -> Vec { + let Some(stack_ability) = entry.ability() else { + return Vec::new(); + }; + + // CR 303.4a: "An Aura spell requires a target, which is defined by its + // enchant ability." That is a statement about the Aura SPELL — the object on + // the stack whose resolution puts the Aura onto the battlefield — and it is + // needed here only because the cast path synthesizes a placeholder spell + // ability whose `target_filter()` is `None`. + // + // CR 115.1b + CR 113.7a: A triggered or activated ability of an Aura already + // on the battlefield is a DIFFERENT object on the stack — 113.7a, "once + // activated or triggered, an ability exists on the stack independently of its + // source" — and 115.1b says outright that "an Aura permanent doesn't target + // anything; only the spell is targeted. (An activated or triggered ability of + // an Aura permanent can also be targeted.)" So it declares its own target + // through its own effect (Pain for All: "When this Aura enters, enchanted + // creature deals damage equal to its power to any other target"). + // Keying this branch on "the source object is an Aura" instead of on the + // stack entry claimed those abilities too and handed back the Aura's + // "creature you control" enchant pool for them — a pool that cannot even + // contain the ability's current target, so every retarget submission was + // rejected and no actor could discharge the prompt. + if matches!(entry.kind, StackEntryKind::Spell { .. }) { + if let Some(filter) = aura_enchant_filter(state, stack_ability.source_id) { + return find_legal_targets( + state, + &filter, + stack_ability.controller, + stack_ability.source_id, + ); + } } // CR 115.7 + CR 601.2c: A multi-role mana declares its recipient AND its @@ -249,8 +290,10 @@ fn legal_new_targets_for_stack_ability( // // The pool is necessarily FLAT (`Vec`, no slot structure), so it // is a SUPERSET pre-filter for the UI/AI. Per-slot CR 115.7a legality is - // enforced at the assignment seam by `retarget_slot_violation` - // (`engine.rs::apply_retarget`). Single-role manas take + // enforced at the assignment seam by `retarget_slot_violation`, consulted by + // BOTH `engine.rs::apply_retarget` and the AI generator + // (`ai_support::candidates::retarget_actions`) so the two cannot propose and + // reject different sets. Single-role manas take // `mana_multi_role == None` and are served entirely by the standard branch, // exactly as before. if let Some(role) = crate::types::ability::mana_multi_role(&stack_ability.effect) { diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 6617467c17..d3291c12d8 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -12263,11 +12263,11 @@ fn apply_action( new_targets, }, )?, - // CR 115.7: Retarget a single-target spell via a board click. The + // CR 115.7a: Retarget a single-target spell via a board click. The // universal `ChooseTarget` action — already consumed by every other - // targeting state — drives single-target retargets (Bolt Bend, - // Redirect, Misdirection) so the player picks the new target directly - // on the battlefield instead of through a dialog. + // targeting state — drives "change the target of" retargets (Bolt Bend, + // Misdirection — NOT Redirect, which is "choose new targets", so CR + // 115.7d `All`) so the player picks the new target on the battlefield. ( WaitingFor::RetargetChoice { player, @@ -12591,23 +12591,28 @@ fn apply_retarget( // CR 115.7a: "each target can be changed only to another legal target." The // `legal_new_targets` pool checked above is flat, so for a multi-slot node it - // cannot tell slot 0's legal set from slot 1's. Re-check positionally against - // the node's own per-slot filters before mutating the stack. Applies to both + // cannot tell slot 0's legal set from slot 1's. Re-check each CHANGED + // position against its own slot filter before mutating the stack; CR 115.7d + // exempts unchanged positions, which `retarget_slot_violation` applies for + // both this caller and the AI generator. Applies to both // `Single` and `All`. It is NOT a blanket no-op for `Single`: alongside the // two-surfaced-slot `Both`, `mana_multi_role` also admits the context-ref // recipient `Both` (surfaced == 1, generic == 0), which is parser-reachable // ("That player adds {R} for each card in target opponent's hand"). A - // `Single`-scope retarget (Bolt Bend, Redirect) of that shape therefore does - // run this per-slot validation — CR 115.7a-correct, and the reason the check + // `Single`-scope retarget (Bolt Bend) of that shape therefore does run + // this per-slot validation — CR 115.7a-correct, and the reason the check // is wired for both scopes rather than only `All`. if let Some(ability) = state .stack .get(stack_entry_index) .and_then(|entry| entry.ability()) { - if let Some(slot) = - crate::game::ability_utils::retarget_slot_violation(state, ability, &new_targets) - { + if let Some(slot) = crate::game::ability_utils::retarget_slot_violation( + state, + ability, + current_targets, + &new_targets, + ) { return Err(EngineError::InvalidAction(format!( "Retarget: chosen target is not legal for target slot {slot}" ))); @@ -19361,7 +19366,44 @@ mod stage2_injector_tests { // `origin/main:crates/engine/src/game/engine.rs:12773`, and its offset from // `begin_pending_trigger_target_selection` (`:12662`) is STILL 134 — the // control that caught this row's one historical SILENT drift. - "game/engine.rs:12851".to_string(), + // CR 115.7 retarget-softlock fix (phase 1), MEASURED in the rebased tree + // after the cherry-pick onto `origin/main` `1098f1b2ee`: + // `:12851 ⇒ :12856`, +5, and ONLY this engine.rs entry moved — the four + // `effects/mod.rs` + `scoped_library_search` entries are untouched by + // this change. `git diff -U0 origin/main` on this file has exactly TWO + // hunks above this producer, both inside `apply_retarget`: + // `@@ -12594,2 +12594,4 @@` (+2 — the CR 115.7d clause added to the + // per-slot re-check's comment) and `@@ -12608,3 +12610,6 @@` (+3 — the + // `retarget_slot_violation` call gaining its `current_targets` argument + // and wrapping across lines). `+2 +3 = +5`. + // The coordinate below was MEASURED, never carried and never computed + // from the pre-rebase value: this literal raised a CONFLICT on the + // cherry-pick, which is exactly the anchor-rot class the note above + // documents. `12851+5` agreeing with the measurement is a check on the + // measurement, not a substitute for it. Identity re-established on BOTH + // controls: the line at `:12856` is sha256-identical + // (`8a544e87…5cc7d63`) to the producer at + // `origin/main:crates/engine/src/game/engine.rs:12851`, and its offset + // from `begin_pending_trigger_target_selection` (now `:12722`, was + // `:12717` upstream) is STILL 134 — this change adds nothing inside that + // function, and the offset is what discriminates when the same mint text + // occurs at several coordinates in this crate. + // Stated WITHOUT a whole-file delta, per the note above: THIS drift entry + // is a further hunk in the same file, it is self-referential, and any + // whole-file figure would be falsified by the next wording edit. It is + // identified by POSITION instead — it sits BELOW the producer, so it + // cannot move it. (It also cannot be counted: `code_of` strips comments, + // and the needle is assembled.) + // Neither above-producer hunk mints a prompt: both sit in the + // VALIDATION half of `apply_retarget` — the half that consumes an + // already-minted `RetargetChoice`, and it is THAT HALF which never writes + // `state.waiting_for`. Scoped deliberately: `apply_retarget` as a whole + // DOES write it, at its tail (`:12664`, `WaitingFor::Priority`), so the + // claim is about the two hunks' half of the function and not about the + // function. That tail write is below both hunks and is not a `*Choice` + // producer, so it is not the needle either way. The census set is still + // exactly 5. + "game/engine.rs:12856".to_string(), ], "the five production producers, NAMED: the CR 603.5 gate in `resolve_chain_body` \ plus the two repeated-optional-payment drivers, the per-player acceptance cursor \ diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 026fc41ac2..c06da98fb8 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -932,6 +932,7 @@ mod refurbished_familiar; mod relic_of_progenitus_6446; mod render_silent_cant_cast; mod repro_pilot_crew; +mod retarget_prompt_softlock; mod revealed_card_type_disjunction_518; mod rhys_evermore_remove_counters; mod riot_control_regression; diff --git a/crates/engine/tests/integration/retarget_prompt_softlock.rs b/crates/engine/tests/integration/retarget_prompt_softlock.rs new file mode 100644 index 0000000000..a341f8ccff --- /dev/null +++ b/crates/engine/tests/integration/retarget_prompt_softlock.rs @@ -0,0 +1,948 @@ +//! CR 115.7 — a parked retarget prompt must always be answerable. +//! +//! Three propositions, all reached through production entry points: +//! +//! 1. the retarget pool comes from the stack entry's OWN targeting authority, +//! not from an Aura host's enchant filter (an Aura's triggered ability is a +//! different object on the stack than the Aura spell — CR 115.1b + CR 113.7a, +//! against the Aura SPELL's own rule, CR 303.4a); +//! 2. an empty pool resolves as a CR 115.7a no-change instead of parking a +//! prompt nothing can discharge; +//! 3. every submission the AI proposes for a parked prompt is accepted by the +//! reducer, because both consult the same per-slot authority +//! (`retarget_slot_violation`). + +use engine::ai_support::candidate_actions; +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::game::zones::create_object; +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{ + mana_multi_role, ControllerRef, Effect, EffectKind, ManaProduction, ManaTargetRole, + QuantityExpr, ResolvedAbility, TargetFilter, TargetRef, TypedFilter, +}; +use engine::types::actions::GameAction; +use engine::types::card_type::CoreType; +use engine::types::events::GameEvent; +use engine::types::game_state::{ + CastingVariant, GameState, RetargetScope, StackEntry, StackEntryKind, WaitingFor, +}; +use engine::types::identifiers::{CardId, ObjectId}; +use engine::types::keywords::Keyword; +use engine::types::mana::{ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; +use engine::types::zones::Zone; + +/// Scryfall, fetched verbatim — not paraphrased. The second line is the one this +/// file's headline test retargets. +const PAIN_FOR_ALL_ORACLE: &str = "Enchant creature you control\n\ + When this Aura enters, enchanted creature deals damage equal to its power to any other target.\n\ + Whenever enchanted creature is dealt damage, it deals that much damage to each opponent."; + +const BOLT_BEND_ORACLE: &str = + "This spell costs {3} less to cast if you control a creature with power 4 or greater.\n\ + Change the target of target spell or ability with a single target."; + +const BLOSSOMING_DEFENSE_ORACLE: &str = + "Target creature you control gets +2/+2 and gains hexproof until end of turn. \ + (It can't be the target of spells or abilities your opponents control.)"; + +const LIGHTNING_BOLT_ORACLE: &str = "Lightning Bolt deals 3 damage to any target."; + +fn add_mana(runner: &mut GameRunner, player: PlayerId, mana: &[ManaType]) { + let dummy = ObjectId(0); + let pool = &mut runner + .state_mut() + .players + .iter_mut() + .find(|p| p.id == player) + .unwrap() + .mana_pool; + for m in mana { + pool.add(ManaUnit::new(*m, dummy, false, vec![])); + } +} + +/// Pass priority until `done`, returning every event the engine emitted on the +/// way. Mirrors `issue_2938_deflecting_swat.rs`'s driver, but keeps the events +/// so a resolution can be asserted on rather than inferred. +fn pass_priority_until(runner: &mut GameRunner, mut done: F) -> Vec +where + F: FnMut(&GameState) -> bool, +{ + let mut events = Vec::new(); + for _ in 0..32 { + if done(runner.state()) { + return events; + } + match &runner.state().waiting_for { + WaitingFor::Priority { .. } => { + let result = runner + .act(GameAction::PassPriority) + .expect("PassPriority must succeed while driving resolution"); + events.extend(result.events); + } + other => panic!("unexpected wait state while driving resolution: {other:?}"), + } + } + panic!("priority loop exhausted before reaching the expected state"); +} + +fn parked_retarget_pool(runner: &GameRunner) -> Vec { + let WaitingFor::RetargetChoice { + legal_new_targets, .. + } = runner.state().waiting_for.clone() + else { + panic!( + "expected a parked RetargetChoice, got {:?}", + runner.state().waiting_for + ); + }; + legal_new_targets +} + +fn retarget_candidates(state: &GameState) -> Vec> { + candidate_actions(state) + .into_iter() + .filter_map(|candidate| match candidate.action { + GameAction::RetargetSpell { new_targets } => Some(new_targets), + _ => None, + }) + .collect() +} + +/// Rows 1a + 1b — CR 115.1b + CR 303.4a: an Aura's *triggered* ability declares +/// its own target ("any other target"), so its retarget pool must come from that +/// effect's filter. CR 115.1b is the on-point rule: "An Aura permanent doesn't +/// target anything; only the spell is targeted. (An activated or triggered +/// ability of an Aura permanent can also be targeted.)" Keying the CR 303.4a Aura substitution on the source object +/// instead of on the stack entry handed back the Aura's "creature you control" +/// enchant pool, which cannot even contain the trigger's current target — so no +/// submission was legal and the prompt could never be discharged. +#[test] +fn aura_hosted_trigger_retarget_pool_uses_the_abilitys_own_filter() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + // Power 4+ so Bolt Bend costs {R} rather than {3}{R}. + let host = scenario + .add_creature(P0, "Smaug the Impenetrable", 8, 7) + .id(); + let bystander = scenario.add_creature(P0, "Goblin", 1, 1).id(); + let victim = scenario.add_creature(P1, "Bear", 2, 2).id(); + let aura = scenario + .add_enchantment_from_oracle(P0, "Pain for All", PAIN_FOR_ALL_ORACLE) + .id(); + let bolt_bend = scenario + .add_spell_to_hand_from_oracle(P0, "Bolt Bend", true, BOLT_BEND_ORACLE) + .id(); + + let mut runner = scenario.build(); + add_mana(&mut runner, P0, &[ManaType::Red]); + + // Pain for All is a PRINTED Aura ("Enchant creature you control"), not a + // bestow creature. `attach_as_bestowed_aura` is the only attach helper the + // scenario builder exposes, and it routes through + // `casting::apply_bestow_aura_form`, which GRANTS the broad bestow + // `Enchant(creature)` to any object carrying no `Keyword::Enchant` — and + // `add_enchantment_from_oracle` installs none. Seed the printed filter first + // so the grant is skipped (it is idempotent on an existing `Enchant`) and the + // Aura keeps the filter the card actually prints. + // + // BOTH fields, deliberately. `layers::seed_live_characteristics_from_base` + // resets `obj.keywords = obj.base_keywords.clone()` at the top of every full + // layer pass, and `attach_as_bestowed_aura` itself calls + // `layers_dirty.mark_full()`, so a write to `keywords` alone cannot survive + // its own attach call. A previous revision wrote only `keywords`, and only + // AFTER attaching: the grant had already fired into both fields, so that + // write was overwritten by the broad filter and row 1b below was VACUOUS — + // a broad `Enchant(creature)` pool contains the victim, so its assertion + // passed at base too. Verified by the guard below, which failed against + // `controller: None` before this seed existed. + { + let printed_enchant = vec![Keyword::Enchant(TargetFilter::Typed( + TypedFilter::creature().controller(ControllerRef::You), + ))]; + let aura_obj = runner.state_mut().objects.get_mut(&aura).unwrap(); + aura_obj.base_keywords = printed_enchant.clone(); + aura_obj.keywords = printed_enchant; + } + runner.attach_as_bestowed_aura(aura, host); + + // The ETB trigger, already on the stack targeting P1's creature. + // `&["Enchant"]` is the MTGJSON keyword-name list the `card-test` convention + // passes for an Aura; the previous `&[]` parsed only via + // `parse_keyword_line`'s non-MTGJSON fallback, which is a different code + // path from the one a real card takes. + let parsed = parse_oracle_text( + PAIN_FOR_ALL_ORACLE, + "Pain for All", + &["Enchant".to_string()], + &["Enchantment".to_string()], + &["Aura".to_string()], + ); + let targeting_triggers: Vec<&Effect> = parsed + .triggers + .iter() + .filter_map(|trigger| trigger.execute.as_ref()) + .map(|ability| ability.effect.as_ref()) + .filter(|effect| effect.target_filter().is_some()) + .collect(); + assert_eq!( + targeting_triggers.len(), + 1, + "fixture guard: exactly one Pain for All trigger declares a target — the \ + ETB damage trigger this row retargets" + ); + let trigger_effect = targeting_triggers[0].clone(); + + let trigger_id = ObjectId(901); + runner.state_mut().stack.push_back(StackEntry { + id: trigger_id, + source_id: aura, + controller: P0, + kind: StackEntryKind::TriggeredAbility { + source_id: aura, + ability: Box::new(ResolvedAbility::new( + trigger_effect, + vec![TargetRef::Object(victim)], + aura, + P0, + )), + condition: None, + trigger_event: None, + description: None, + source_name: String::new(), + subject_match_count: None, + die_result: None, + provenance: None, + }, + }); + + runner + .cast(bolt_bend) + .target_objects(&[trigger_id]) + .commit(); + pass_priority_until(&mut runner, |state| { + matches!(state.waiting_for, WaitingFor::RetargetChoice { .. }) + }); + + // Structural guard: the entry under test really is a triggered ability, so + // this row cannot silently degrade into re-testing the Aura SPELL branch. + assert!( + matches!( + runner.state().stack[0].kind, + StackEntryKind::TriggeredAbility { .. } + ), + "fixture guard: stack[0] must be the Aura's triggered ability" + ); + + // Premise guard — pins the LIVE enchant filter, which is row 1b's whole + // discriminator: the narrow "creature you control" filter excludes P1's + // creatures, so a pool derived from it cannot contain the victim, and it + // yields no player, so it cannot contain row 1a's players either. + // + // ASSERTED, never assigned — the assignment happens once, above the attach, + // where it can actually take. This guard is what caught that the previous + // revision's post-attach write to `keywords` alone did NOT take: it failed + // here with `controller: None`, the broad bestow grant, proving row 1b had + // been passing against a pool that contains the victim at base too. Nothing + // else between the fixture and the pool pins this filter, so without this + // assertion the same regression is silent. + assert_eq!( + runner.state().objects[&aura].keywords, + vec![Keyword::Enchant(TargetFilter::Typed( + TypedFilter::creature().controller(ControllerRef::You), + ))], + "fixture guard: the Aura's live enchant filter must be the narrow printed \ + one, or this row's pool discriminator is vacuous" + ); + + let pool = parked_retarget_pool(&runner); + + // Positive reach-guard: a P0 creature is in the pool under BOTH the old + // enchant-filter derivation and the new effect-filter one, so the two + // discriminating assertions below cannot pass merely by the pool being + // populated with anything at all. + assert!( + pool.contains(&TargetRef::Object(bystander)), + "reach guard: the pool must be populated, got {pool:?}" + ); + + // Row 1a — the trigger's own "any other target" filter enumerates players + // (CR 115.4). The Aura's "creature you control" enchant filter cannot. + assert!( + pool.contains(&TargetRef::Player(P1)), + "CR 115.1b: the pool must come from the TRIGGER's own target filter, which \ + reaches players; got {pool:?}" + ); + + // Row 1b — the ability's current target must stay retargetable-to. It is a + // creature P1 controls, which the "creature you control" enchant filter + // excludes outright. + assert!( + pool.contains(&TargetRef::Object(victim)), + "CR 115.7a: the current target must remain in the pool; got {pool:?}" + ); +} + +/// Row 1d — CR 115.7a: "If a target can't be changed to another legal target, +/// the original target is unchanged." An empty pool IS that case, so the effect +/// must resolve as a no-change rather than park a prompt with zero candidates, +/// which neither a human nor the AI can discharge. +#[test] +fn retarget_with_no_legal_alternative_resolves_as_no_change() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + // P0 controls NO creature — that is what empties "target creature you + // control". P1 controls one, so the battlefield is not globally empty. + scenario.add_creature(P1, "Bear", 2, 2); + let bolt_bend = scenario + .add_spell_to_hand_from_oracle(P0, "Bolt Bend", true, BOLT_BEND_ORACLE) + .id(); + + let mut runner = scenario.build(); + // P0 controls no power-4 creature, so Bolt Bend costs the full {3}{R}. + add_mana( + &mut runner, + P0, + &[ + ManaType::Red, + ManaType::Colorless, + ManaType::Colorless, + ManaType::Colorless, + ], + ); + + // The removal, modeled: Blossoming Defense's target left the battlefield in + // an earlier priority window and is now a P0 creature card in the graveyard. + let doomed = create_object( + runner.state_mut(), + CardId(801), + P0, + "Doomed Bear".to_string(), + Zone::Graveyard, + ); + runner + .state_mut() + .objects + .get_mut(&doomed) + .unwrap() + .card_types + .core_types = vec![CoreType::Creature]; + + let defense_parsed = parse_oracle_text( + BLOSSOMING_DEFENSE_ORACLE, + "Blossoming Defense", + &[], + &["Instant".to_string()], + &[], + ); + let defense_id = create_object( + runner.state_mut(), + CardId(802), + P0, + "Blossoming Defense".to_string(), + Zone::Stack, + ); + runner + .state_mut() + .objects + .get_mut(&defense_id) + .unwrap() + .card_types + .core_types = vec![CoreType::Instant]; + let defense_ability = ResolvedAbility::new( + defense_parsed.abilities[0].effect.as_ref().clone(), + // CR 115.7a: an illegal-but-recorded current target. A stack entry with + // NO current targets no-ops earlier in `change_targets::resolve`, which + // would satisfy this row's assertion without the empty-pool guard ever + // running. + vec![TargetRef::Object(doomed)], + defense_id, + P0, + ); + runner.state_mut().stack.push_back(StackEntry { + id: defense_id, + source_id: defense_id, + controller: P0, + kind: StackEntryKind::Spell { + card_id: CardId(802), + ability: Some(Box::new(defense_ability)), + casting_variant: CastingVariant::Normal, + actual_mana_spent: 0, + }, + }); + + // Structural guard: pins WHY the pool is empty — the filter matched nothing, + // rather than the battlefield being empty so any filter would have. + let controls_creature = |state: &GameState, player: PlayerId| { + state + .objects + .values() + .filter(|obj| obj.zone == Zone::Battlefield && obj.controller == player) + .any(|obj| obj.card_types.core_types.contains(&CoreType::Creature)) + }; + assert!( + !controls_creature(runner.state(), P0), + "fixture guard: P0 must control no creature, which is what empties the pool" + ); + // Non-vacuity control for the guard above, NOT a production discriminator: + // it cannot catch an engine regression, only a fixture that has degraded + // into a globally empty battlefield, where "the pool is empty" would be + // uninformative. Kept deliberately and labelled so it is not read as + // coverage. A third assertion (`objects[&bystander].zone == Battlefield`) + // stood here and was removed as strictly entailed: P1's Bear is the only P1 + // creature in this fixture, so this assertion is true exactly when that one + // was. + assert!( + controls_creature(runner.state(), P1), + "fixture guard: P1 must control a creature, so the battlefield is not \ + globally empty" + ); + + runner + .cast(bolt_bend) + .target_objects(&[defense_id]) + .commit(); + + // Stop the moment ChangeTargets resolves — passing further would resolve + // Blossoming Defense itself and remove the entry this row asserts on. + let events = pass_priority_until(&mut runner, |state| { + matches!(state.waiting_for, WaitingFor::RetargetChoice { .. }) + || !state.stack.iter().any(|entry| entry.id == bolt_bend) + }); + + // Discriminating: at base the prompt parks here. + assert!( + !matches!( + runner.state().waiting_for, + WaitingFor::RetargetChoice { .. } + ), + "CR 115.7a: an empty pool must not park an unanswerable prompt" + ); + + // Positive reach-guard: "no prompt parked" is a negative, so prove the + // ChangeTargets effect actually resolved rather than fizzling en route. + assert!( + events.iter().any(|event| matches!( + event, + GameEvent::EffectResolved { + kind: EffectKind::ChangeTargets, + .. + } + )), + "reach guard: the ChangeTargets effect must have resolved, got {events:?}" + ); + + // Positive reach-guard: CR 115.7a's "the original target is unchanged". + let defense_entry = runner + .state() + .stack + .iter() + .find(|entry| entry.id == defense_id) + .expect("the retargeted entry is still on the stack"); + assert_eq!( + defense_entry.ability().unwrap().targets, + vec![TargetRef::Object(doomed)], + "CR 115.7a: the original target is unchanged" + ); +} + +/// Push a single-target Lightning-Bolt-shaped spell at stack index 0 targeting +/// `victim`, and return its id. +fn push_bolt_entry(runner: &mut GameRunner, victim: ObjectId) -> ObjectId { + let parsed = parse_oracle_text( + LIGHTNING_BOLT_ORACLE, + "Lightning Bolt", + &[], + &["Instant".to_string()], + &[], + ); + let bolt_id = create_object( + runner.state_mut(), + CardId(77), + P1, + "Lightning Bolt".to_string(), + Zone::Stack, + ); + runner + .state_mut() + .objects + .get_mut(&bolt_id) + .unwrap() + .card_types + .core_types = vec![CoreType::Instant]; + let ability = ResolvedAbility::new( + parsed.abilities[0].effect.as_ref().clone(), + vec![TargetRef::Object(victim)], + bolt_id, + P1, + ); + runner.state_mut().stack.push_back(StackEntry { + id: bolt_id, + source_id: bolt_id, + controller: P1, + kind: StackEntryKind::Spell { + card_id: CardId(77), + ability: Some(Box::new(ability)), + casting_variant: CastingVariant::Normal, + actual_mana_spent: 0, + }, + }); + bolt_id +} + +/// Row 2a — CR 115.7a: every candidate the AI generates for a parked +/// `RetargetChoice` must be a submission the reducer accepts. At base the sole +/// candidate is `current_targets`, which `apply_retarget` rejects whenever the +/// current target has dropped out of the pool (hexproof gained, protection +/// granted) — three rejections in a row halt the AI controller. +#[test] +fn ai_retarget_candidates_are_accepted_by_the_reducer() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + let alternative = scenario.add_creature(P0, "Goblin", 1, 1).id(); + scenario.add_creature(P0, "Bystander", 1, 1); + let victim = scenario.add_creature(P1, "Bear", 2, 2).id(); + + let mut runner = scenario.build(); + push_bolt_entry(&mut runner, victim); + + // The current target is deliberately ABSENT from the pool: it became + // illegal after the spell was cast. Unit 1 does not eliminate this state. + runner.state_mut().waiting_for = WaitingFor::RetargetChoice { + player: P0, + stack_entry_index: 0, + scope: RetargetScope::Single, + current_targets: vec![TargetRef::Object(victim)], + legal_new_targets: vec![TargetRef::Object(alternative)], + }; + + let candidates = retarget_candidates(runner.state()); + + // Positive reach-guard: the arm was reached and produced retarget actions. + assert!( + !candidates.is_empty(), + "reach guard: the RetargetChoice arm must produce candidates" + ); + + // Discriminating: at base the single candidate is `[victim]`, which the + // reducer rejects with "chosen target not in legal alternatives". + for new_targets in &candidates { + let mut probe = GameRunner::from_state(runner.state().clone()); + probe + .act(GameAction::RetargetSpell { + new_targets: new_targets.clone(), + }) + .unwrap_or_else(|err| { + panic!("candidate {new_targets:?} must be accepted by the reducer: {err:?}") + }); + } + + runner + .act(GameAction::RetargetSpell { + new_targets: vec![TargetRef::Object(alternative)], + }) + .expect("the retarget submission must be accepted"); + assert!(matches!( + runner.state().waiting_for, + WaitingFor::Priority { .. } + )); + assert_eq!( + runner.state().stack[0].ability().unwrap().targets, + vec![TargetRef::Object(alternative)], + "CR 115.7a: the accepted submission is applied to the stack entry" + ); +} + +/// Row 2c — CR 115.7d: an "All" retarget offers the unchanged anchor AND every +/// single-slot substitution. At base exactly one candidate is produced, so a +/// multi-slot prompt could only ever be answered one way — and if that way is +/// rejected, not at all. +/// +/// SCOPE OF THE CLAIM — read before citing this row. It pins the generator's +/// ENUMERATION SHAPE for a node OUTSIDE the per-slot admitted class +/// (`mana_multi_role` is `None`, so `retarget_slot_violation` early-outs and +/// nothing filters the enumeration). It does NOT claim every enumerated +/// submission is CR-legal, and it must not be cited as evidence that it is: +/// +/// * `expected` contains `[b, b]` and `[a, a]`. The generator substitutes one +/// slot at a time from the FLAT pool with no distinctness check, so a pool +/// member already held by another slot produces a duplicate. Whether a +/// duplicate is legal is a per-INSTANCE question (CR 601.2c: the same target +/// can't be chosen twice for any ONE instance of "target", but may be chosen +/// once for EACH instance if the spell uses "target" in several places), and +/// the flat pool carries no instance structure to answer it with. That is +/// the same pre-existing flat-pool gap `retarget_slot_violation`'s SCOPE +/// note records for `Attach` / `MoveCounters` / `Fight`. These two entries +/// are therefore recorded as OBSERVED CURRENT BEHAVIOUR, deliberately not +/// endorsed. +/// * The fixture is synthetic in the same direction: Lightning Bolt's "any +/// target" is ONE instance of "target", so a genuine Bolt cannot hold two +/// targets at all. The two-target list is fabricated to isolate enumeration, +/// which is why per-slot legality is out of this row's reach rather than +/// merely unasserted. +#[test] +fn all_scope_retarget_candidates_cover_every_slot() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + let a = scenario.add_creature(P0, "Alpha", 1, 1).id(); + let b = scenario.add_creature(P0, "Beta", 1, 1).id(); + let c = scenario.add_creature(P0, "Gamma", 1, 1).id(); + + let mut runner = scenario.build(); + + // A non-mana two-target node: `mana_multi_role` returns `None`, so + // `retarget_slot_violation` early-outs and this row isolates enumeration. + let parsed = parse_oracle_text( + LIGHTNING_BOLT_ORACLE, + "Lightning Bolt", + &[], + &["Instant".to_string()], + &[], + ); + let source = create_object( + runner.state_mut(), + CardId(803), + P0, + "Two-Slot Source".to_string(), + Zone::Battlefield, + ); + let ability = ResolvedAbility::new( + parsed.abilities[0].effect.as_ref().clone(), + vec![TargetRef::Object(a), TargetRef::Object(b)], + source, + P0, + ); + assert!( + mana_multi_role(&ability.effect).is_none(), + "fixture guard: this row's node is outside the per-slot admitted class" + ); + runner.state_mut().stack.push_back(StackEntry { + id: ObjectId(902), + source_id: source, + controller: P0, + kind: StackEntryKind::ActivatedAbility { + source_id: source, + ability: Box::new(ability), + }, + }); + + let current_targets = vec![TargetRef::Object(a), TargetRef::Object(b)]; + runner.state_mut().waiting_for = WaitingFor::RetargetChoice { + player: P0, + stack_entry_index: 0, + scope: RetargetScope::All, + current_targets: current_targets.clone(), + legal_new_targets: vec![ + TargetRef::Object(a), + TargetRef::Object(b), + TargetRef::Object(c), + ], + }; + + let candidates = retarget_candidates(runner.state()); + + // Positive reach-guards — each covers half the claim. Ordered BEFORE the + // exact-equality assert deliberately: after it they are strictly entailed by + // it and can never fire, which is how they previously stood. Row 2e already + // uses this ordering. They localize a failure to "the anchor was dropped" or + // "no substitution was offered" before the exact list is compared. + assert!( + candidates.iter().any(|c| *c != current_targets), + "reach guard: at least one per-slot substitution was offered" + ); + assert!( + candidates.contains(¤t_targets), + "reach guard: CR 115.7d's unchanged anchor survived" + ); + + // Derived from the arm's own shape: the anchor, plus one substitution per + // (slot, pool member) pair minus the two identity pairs. `[b, b]` and + // `[a, a]` are the duplicate-producing entries; per this row's SCOPE note + // they are pinned as observed behaviour, NOT asserted to be CR-legal. + let expected = vec![ + vec![TargetRef::Object(a), TargetRef::Object(b)], + vec![TargetRef::Object(b), TargetRef::Object(b)], + vec![TargetRef::Object(c), TargetRef::Object(b)], + vec![TargetRef::Object(a), TargetRef::Object(a)], + vec![TargetRef::Object(a), TargetRef::Object(c)], + ]; + assert_eq!( + candidates, expected, + "the anchor plus one substitution per (slot, pool member) pair — \ + enumeration shape only; see this row's SCOPE note on the duplicates" + ); +} + +/// Build the synthetic multi-role mana fixture rows 2e and 2g share: an +/// `Effect::Mana` whose `ManaTargetRole::Both` declares a recipient slot legal +/// only for an OPPONENT of P0 and a count-source slot legal for any player. +/// +/// `current_targets` is per row and load-bearing in opposite directions — 2e +/// needs slot 0 legal-but-different, 2g needs slot 0 currently illegal — so it +/// is a parameter, never a shared constant. +fn push_multi_role_mana_entry(runner: &mut GameRunner, current_targets: Vec) { + let role = ManaTargetRole::Both { + // Slot 0 (recipient, surfaced first): only an opponent of P0, i.e. P1. + recipient: TargetFilter::Typed(TypedFilter::default().controller(ControllerRef::Opponent)), + // Slot 1 (count source): any player. + count_source: TargetFilter::Player, + }; + let source = create_object( + runner.state_mut(), + CardId(901), + P0, + "Multi-Role Mana Source".to_string(), + Zone::Battlefield, + ); + let entry_id = create_object( + runner.state_mut(), + CardId(901), + P0, + "Multi-Role Mana Ability".to_string(), + Zone::Stack, + ); + let ability = ResolvedAbility::new( + Effect::Mana { + produced: ManaProduction::Colorless { + count: QuantityExpr::Fixed { value: 1 }, + }, + restrictions: vec![], + grants: vec![], + expiry: None, + target: Some(role), + }, + current_targets, + source, + P0, + ); + runner.state_mut().stack.push_back(StackEntry { + id: entry_id, + source_id: source, + controller: P0, + kind: StackEntryKind::ActivatedAbility { + source_id: source, + ability: Box::new(ability), + }, + }); +} + +/// Both structural reach-guards for the synthetic multi-role rows. Without the +/// first, `retarget_actions`' `is_none_or` returns `true` and every candidate +/// passes unfiltered while `apply_retarget`'s per-slot stage is skipped +/// entirely; without the second, the fixture can silently degrade into a node +/// `mana_multi_role` rejects. Either degradation makes these rows vacuous. +fn assert_multi_role_entry_is_live(runner: &GameRunner) { + assert!( + runner.state().stack[0].ability().is_some(), + "reach guard: stack index 0 must carry the ability under test" + ); + assert!( + mana_multi_role(&runner.state().stack[0].ability().unwrap().effect).is_some(), + "reach guard: the node must be inside the per-slot admitted class" + ); +} + +/// Row 2e — CR 115.7a: the retarget pool is a FLAT union over both role filters, +/// so it contains members legal only for the *other* slot. The generator must +/// consult the same per-slot authority the reducer does, rather than proposing +/// a pool member the reducer will reject. +/// +/// SCOPE OF THE CLAIM — read before citing this row. It pins SLOT LEGALITY: that +/// the generator filters the flat pool through the same authority the reducer +/// applies. It does NOT claim the submission it accepts is CR-115.7a / +/// CR-115.7b-legal, and must not be cited as if it did. +/// +/// This fixture's `current_targets` has LENGTH 2 and the accepted submission has +/// LENGTH 1, because the reducer's `Single` arm hard-requires exactly one target. +/// Applying it assigns `ability.targets = [P1]` and TRUNCATES the count-source +/// slot. Both subrules that reach this arm say otherwise, and they prescribe +/// DIFFERENT remedies — `RetargetScope::Single` is produced by two oracle +/// templates (`try_parse_change_targets`, parser/oracle_effect/mod.rs), so the +/// deferred fix must DISPATCH ON THE TEMPLATE rather than apply CR 115.7b's +/// remedy uniformly: +/// - "change a target of " → CR 115.7b: "the process described in rule 115.7a +/// is followed, except that only one of those targets may be changed (rather +/// than all of them or none of them)". Remedy: one slot changes, every other +/// declared target stays in place. +/// - "change the target of " → CR 115.7a, which ends: "If all the targets +/// aren't changed to other legal targets, none of them are changed." Remedy +/// for a multi-target entry: ALL-OR-NONE, not one-changes-rest-stay. This is +/// Bolt Bend's wording — the WORDING only. Bolt Bend itself reads "with a +/// single target", and of the 22 printed cards matching +/// `o:"change the target of"`, the six omitting that literal phrase each +/// restrict to one target by the equivalent "targets ONLY " / "a single +/// " construction. No printed card on this template can therefore present +/// a multi-target entry, which is why the fixture below is synthetic. +/// +/// Under neither remedy may an undisturbed slot simply be dropped. The length-1 +/// acceptance is recorded here as OBSERVED CURRENT BEHAVIOUR, deliberately NOT +/// endorsed: +/// +/// DEFERRED(out-of-run): interactive Single-scope retarget collapses +/// multi-target lists (CR 115.7a / CR 115.7b) — upstream cause filter.rs +/// FilterProp::HasSingleTarget is permissive with no resolution-time +/// validation; fix needs filter.rs + interaction.rs, both outside phase 1's +/// frozen scope. +/// +/// The honest behavioural delta this phase knowingly takes: at BASE the AI froze +/// on this class — the generator's sole proposal was rejected, so no actor could +/// discharge the prompt. AFTER this change it PROGRESSES and TRUNCATES. Progress +/// with a corrupted target list is the trade; it is not a claim that the result +/// is rules-correct. A genuinely CR-115.7a / CR-115.7b-correct submission is not +/// expressible against today's reducer contract at all, which is why the +/// deferral above names two paths outside this run's frozen scope rather than a +/// local repair. +#[test] +fn multi_role_mana_single_retarget_candidates_are_slot_legal() { + let mut runner = GameScenario::new().build(); + + // Slot 0 holds P1 (legal for the opponent-only recipient slot); slot 1 holds + // P0 (legal for the any-player count-source slot). Slot 0 must NOT already + // hold P0, or submitting `[P0]` would be an exempt non-change and would + // survive for a reason unrelated to slot legality. + let current_targets = vec![TargetRef::Player(P1), TargetRef::Player(P0)]; + push_multi_role_mana_entry(&mut runner, current_targets.clone()); + assert_multi_role_entry_is_live(&runner); + + let legal_new_targets = vec![TargetRef::Player(P0), TargetRef::Player(P1)]; + runner.state_mut().waiting_for = WaitingFor::RetargetChoice { + player: P0, + stack_entry_index: 0, + scope: RetargetScope::Single, + current_targets, + legal_new_targets: legal_new_targets.clone(), + }; + + let candidates = retarget_candidates(runner.state()); + + // Reach-guard (a): something survived — excludes the world where the + // generator returns nothing and the discriminator holds by emptiness. + assert!( + !candidates.is_empty(), + "reach guard: a slot-legal submission must still be offered" + ); + // Reach-guard (b): something was dropped — excludes the opposite world where + // the slot filter never engaged and every pool member was proposed. + assert!( + candidates.len() < legal_new_targets.len(), + "reach guard: the slot-legality filter must have removed a pool member, \ + got {candidates:?}" + ); + + // Discriminating: P0 is in the flat pool but is legal only for slot 1, and a + // `Single` submission lands in slot 0. + assert_eq!( + candidates, + vec![vec![TargetRef::Player(P1)]], + "CR 115.7a: only the slot-0-legal pool member may be proposed" + ); + + // Discriminating: every candidate is accepted by the reducer. At base the + // sole candidate is `[P1, P0]` (length 2), which the `Single` arm rejects. + // + // "Accepted by the reducer" is the WHOLE claim here — acceptance, not + // rules-correctness. Applying this length-1 submission to a length-2 target + // list truncates the count-source slot, contrary to CR 115.7a / CR 115.7b + // alike — neither remedy permits dropping an undisturbed slot. See this + // row's SCOPE note and the DEFERRED(out-of-run) entry it carries; that + // truncation is why this loop deliberately asserts only that the submission + // is accepted, and never asserts the resulting `ability.targets`. + for new_targets in &candidates { + let mut probe = GameRunner::from_state(runner.state().clone()); + probe + .act(GameAction::RetargetSpell { + new_targets: new_targets.clone(), + }) + .unwrap_or_else(|err| { + panic!("candidate {new_targets:?} must be accepted by the reducer: {err:?}") + }); + } +} + +/// Row 2g — CR 115.7d: "the player may leave any number of the targets +/// unchanged, even if those targets would be illegal." End-to-end proof that the +/// generator proposes the unchanged anchor AND the reducer accepts it. At base +/// the two disagree: pool membership exempts the anchor while per-slot +/// validation rejects it, so the prompt is unanswerable. +#[test] +fn all_scope_unchanged_anchor_is_proposed_and_accepted_when_current_targets_are_slot_illegal() { + let mut runner = GameScenario::new().build(); + + // The mirror image of row 2e: slot 0 holds P0, which is ILLEGAL for the + // opponent-only recipient slot, so the CR 115.7d exemption is the only thing + // that can let the unchanged anchor through. + let current_targets = vec![TargetRef::Player(P0), TargetRef::Player(P1)]; + push_multi_role_mana_entry(&mut runner, current_targets.clone()); + assert_multi_role_entry_is_live(&runner); + + runner.state_mut().waiting_for = WaitingFor::RetargetChoice { + player: P0, + stack_entry_index: 0, + scope: RetargetScope::All, + current_targets: current_targets.clone(), + legal_new_targets: vec![TargetRef::Player(P0), TargetRef::Player(P1)], + }; + + let candidates = retarget_candidates(runner.state()); + + // Positive reach-guard: without this, "the anchor is accepted" could pass in + // a world where the generator emits only the anchor and slot validation + // never engaged at all. + assert!( + candidates.len() >= 2, + "reach guard: substitutions must be offered alongside the anchor, got {candidates:?}" + ); + assert!( + candidates.iter().any(|c| *c != current_targets), + "reach guard: at least one slot substitution was offered" + ); + + // Discriminating (generator half): the unchanged anchor is proposed. + assert!( + candidates.contains(¤t_targets), + "CR 115.7d: the unchanged anchor must be proposed, got {candidates:?}" + ); + + // Reducer half. Stated as a COUNTERFACTUAL because the straightforward + // reading was measured and refuted: at base this line is NOT REACHED. The + // base generator proposes only the anchor, so the reach guard above + // (`candidates.len() >= 2`) fails first and is the row's actual base + // discriminator. WERE it reached at base, base would fail here with + // "Retarget: chosen target is not legal for target slot 0" — that is a + // derivation from base source (base `retarget_slot_violation` has no CR + // 115.7d exemption, so this fixture's slot 0 returns `Some(0)`), not an + // observation. + // + // What this call uniquely guards at candidate is therefore NARROWER than a + // second independent discriminator: the generator and the reducer now + // consult one authority, so it can only fail if the REDUCER drifts strictly + // more restrictive than the GENERATOR. That is an anti-drift consistency + // check between the two consumers, and it is worth keeping as one — but it + // is not independent evidence, and must not be cited as such. + runner + .act(GameAction::RetargetSpell { + new_targets: current_targets.clone(), + }) + .expect("CR 115.7d: leaving every target unchanged must be accepted"); + + // Behavioural: the unchanged submission is a completed retarget, not a skip. + // `WaitingFor::Priority` is the load-bearing half — only `apply_retarget`'s + // tail sets it, so it cannot hold unless the reducer ran to completion. + // + // A `stack[0].ability().targets == current_targets` assertion stood here and + // was REMOVED as trivially satisfied: `push_multi_role_mana_entry` already + // set those targets, and the submission IS `current_targets`, so the write + // `apply_retarget` performs is a no-op and the assertion holds whether or + // not the reducer ever ran. It read as behavioural evidence while proving + // nothing. The `.expect()` above plus this check are the real evidence. + assert!(matches!( + runner.state().waiting_for, + WaitingFor::Priority { .. } + )); +} diff --git a/crates/phase-ai/src/search.rs b/crates/phase-ai/src/search.rs index fcc3ae168e..c7d08fb1ab 100644 --- a/crates/phase-ai/src/search.rs +++ b/crates/phase-ai/src/search.rs @@ -10,7 +10,7 @@ use std::io::BufReader; use engine::ai_support::{ build_decision_context, build_decision_context_for_semantic_owner, certify_fetch_then_cast, - certify_pact_plan, is_pact_payment_cast, is_targeted_exchange_root, + certify_pact_plan, is_pact_payment_cast, is_targeted_exchange_root, retarget_actions, root_may_yield_adverse_exchange, targeted_exchange_verdict, validated_candidate_actions_for_semantic_owner, AiDecisionContract, TargetedExchangeVerdict, }; @@ -2014,12 +2014,50 @@ pub fn fallback_action( Some(GameAction::SubmitPayAmount { amount: *min }) } - // Retarget: keep current targets. + // CR 115.7a: a retarget must change to ANOTHER legal target; keeping the + // current targets is rejected by `apply_retarget` whenever the current + // target is not in the pool. Share the engine's enumeration so this + // fallback and `candidate_actions` cannot drift. + // + // This arm can yield `None`, and that is deliberate: there is no + // submission to fall back to. Falling back + // to `current_targets` was tried and is WRONG — worked through, the empty + // case is reachable only under `Single` (the `All` arm always pushes the + // unchanged anchor, which `retarget_slot_violation` exempts; `ForcedTo` + // never parks a prompt at all), and empty under `Single` entails + // that the current target is NOT in `legal_new_targets`. `apply_retarget`'s + // `Single` arm rejects on precisely that condition — `!legal_new_targets + // .contains(&new_targets[0])` — and it runs BEFORE the per-slot authority, + // with no unchanged-position exemption. So the fallback is rejected over + // its whole live domain; row 2b of `retarget_fallback_action.rs` says the + // same thing about the pre-fix behaviour. + // + // DEFERRED(out-of-run): a `Single`-scope prompt EVERY member of whose + // stored pool fails the per-slot check has NO reducer-accepted + // submission. (The pool merely excluding the current target is NOT + // sufficient — row 2b of `retarget_fallback_action.rs` is exactly that + // case and its submission IS accepted.) That is a reducer-level gap, + // not an AI one, and it shares the upstream cause already carried + // below — `FilterProp::HasSingleTarget` is permissive with no + // resolution-time validation. `None` is the correct signal for it: the + // AI reporting "I have no legal action" is honest, whereas submitting a + // knowingly-rejected action would launder an engine gap into an AI + // retry loop. WaitingFor::RetargetChoice { - current_targets, .. - } => Some(GameAction::RetargetSpell { - new_targets: current_targets.clone(), - }), + stack_entry_index, + scope, + current_targets, + legal_new_targets, + .. + } => retarget_actions( + state, + *stack_entry_index, + scope, + current_targets, + legal_new_targets, + ) + .into_iter() + .next(), // Companion reveal: decline. WaitingFor::CompanionReveal { .. } => Some(GameAction::DeclareCompanion { diff --git a/crates/phase-ai/tests/retarget_fallback_action.rs b/crates/phase-ai/tests/retarget_fallback_action.rs new file mode 100644 index 0000000000..a301422fea --- /dev/null +++ b/crates/phase-ai/tests/retarget_fallback_action.rs @@ -0,0 +1,260 @@ +//! CR 115.7 — `phase_ai::search::fallback_action` must answer a parked +//! `RetargetChoice` with a submission the reducer accepts. +//! +//! The fallback is the AI's last resort when scoring produces nothing, so a +//! fallback that proposes an illegal submission is a freeze by a second route. +//! Both rows drive `fallback_action` itself — the seam these rows exist to +//! carry. That is an ENTRY POINT, not an isolation: `fallback_action` ends in +//! `gate(action)`, which filters through `contract.contains_action`, and the +//! contract builds its candidates from the same engine enumerator that reaches +//! `retarget_actions` (`ai_support::context`'s `AiDecisionContract::issue` -> +//! `candidate_actions_for_semantic_owner_with_probe`). Its `issued` helper reads +//! `contract.candidates` even more directly. So these rows assert a +//! CO-DEPENDENCE property — the fallback answers the prompt only when the +//! generator and the fallback agree — and reverting EITHER side alone yields +//! `None` and fails the `is_some()` guard. They do not isolate the fallback from +//! the generator, and must not be cited as if they did. + +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::game::zones::create_object; +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{ + mana_multi_role, ControllerRef, Effect, ManaProduction, ManaTargetRole, QuantityExpr, + ResolvedAbility, TargetFilter, TargetRef, TypedFilter, +}; +use engine::types::actions::GameAction; +use engine::types::card_type::CoreType; +use engine::types::game_state::{ + CastingVariant, RetargetScope, StackEntry, StackEntryKind, WaitingFor, +}; +use engine::types::identifiers::CardId; +use engine::types::phase::Phase; +use engine::types::zones::Zone; +use phase_ai::config::{create_config, AiDifficulty, Platform}; + +const LIGHTNING_BOLT_ORACLE: &str = "Lightning Bolt deals 3 damage to any target."; + +/// `fallback_action` gates its result on the decision contract, which is exact +/// set membership against the contract's own candidates — so the contract MUST +/// be issued for the prompt's own player, or the row degrades into asserting +/// nothing. +fn fallback_for_prompt(runner: &GameRunner) -> Option { + let config = create_config(AiDifficulty::VeryHard, Platform::Native); + let contract = engine::ai_support::AiDecisionContract::issue(runner.state(), P0); + phase_ai::search::fallback_action(runner.state(), &config, &contract) +} + +/// Row 2b — CR 115.7a: the fallback must propose ANOTHER legal target. At base +/// it returns `current_targets`, which `apply_retarget` rejects whenever the +/// current target has dropped out of the pool. +#[test] +fn fallback_retarget_action_is_legal() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + let alternative = scenario.add_creature(P0, "Goblin", 1, 1).id(); + scenario.add_creature(P0, "Bystander", 1, 1); + let victim = scenario.add_creature(P1, "Bear", 2, 2).id(); + + let mut runner = scenario.build(); + + // The same fixture shape as the engine-side row: a single-target spell on + // the stack whose current target is deliberately absent from the pool. + let parsed = parse_oracle_text( + LIGHTNING_BOLT_ORACLE, + "Lightning Bolt", + &[], + &["Instant".to_string()], + &[], + ); + let bolt_id = create_object( + runner.state_mut(), + CardId(77), + P1, + "Lightning Bolt".to_string(), + Zone::Stack, + ); + runner + .state_mut() + .objects + .get_mut(&bolt_id) + .unwrap() + .card_types + .core_types = vec![CoreType::Instant]; + let bolt_ability = ResolvedAbility::new( + parsed.abilities[0].effect.as_ref().clone(), + vec![TargetRef::Object(victim)], + bolt_id, + P1, + ); + runner.state_mut().stack.push_back(StackEntry { + id: bolt_id, + source_id: bolt_id, + controller: P1, + kind: StackEntryKind::Spell { + card_id: CardId(77), + ability: Some(Box::new(bolt_ability)), + casting_variant: CastingVariant::Normal, + actual_mana_spent: 0, + }, + }); + + // A directly parked prompt: no pending cast, so `fallback_action`'s + // cancel-cast escape cannot fire before the `RetargetChoice` arm. + runner.state_mut().waiting_for = WaitingFor::RetargetChoice { + player: P0, + stack_entry_index: 0, + scope: RetargetScope::Single, + current_targets: vec![TargetRef::Object(victim)], + legal_new_targets: vec![TargetRef::Object(alternative)], + }; + + let action = fallback_for_prompt(&runner); + + // Positive reach-guard, both halves: `Some` alone would also be satisfied by + // a `CancelCast` returned from an escape that ran before the arm. + assert!(action.is_some(), "reach guard: the fallback must answer"); + assert!( + matches!(action, Some(GameAction::RetargetSpell { .. })), + "reach guard: the RetargetChoice arm must be the one that answered, got {action:?}" + ); + + // Discriminating: at base the action is `RetargetSpell { [victim] }`, which + // the reducer rejects with "chosen target not in legal alternatives". + runner + .act(action.unwrap()) + .expect("the fallback's action must be accepted by the reducer"); +} + +/// Row 2f — CR 115.7a, at the `phase-ai` seam: a flat pool member legal only for +/// another slot must never be proposed, because `apply_retarget` re-checks each +/// changed submission against its own slot filter. +/// +/// SCOPE OF THE CLAIM — read before citing this row. It pins SLOT LEGALITY at the +/// `phase-ai` fallback seam: that the fallback filters the flat pool through the +/// same authority the reducer applies. It does NOT claim the submission it +/// accepts is CR-115.7a / CR-115.7b-legal, and must not be cited as if it did. +/// +/// This fixture is the same shape as `retarget_prompt_softlock.rs` row 2e: +/// `current_targets` has LENGTH 2 while the accepted submission has LENGTH 1, +/// because the reducer's `Single` arm hard-requires exactly one target. Applying +/// it assigns `ability.targets = [P1]` and TRUNCATES the count-source slot. +/// Neither subrule that reaches this arm permits dropping an undisturbed slot: +/// CR 115.7b changes one target and leaves every other declared target in place, +/// and CR 115.7a is all-or-none. Because the two prescribe DIFFERENT remedies and +/// `RetargetScope::Single` is produced by both oracle templates, the deferred fix +/// must dispatch on the template; row 2e's SCOPE note carries the full +/// two-template analysis. The length-1 acceptance asserted below is recorded as +/// OBSERVED CURRENT BEHAVIOUR, deliberately NOT endorsed: +/// +/// DEFERRED(out-of-run): interactive Single-scope retarget collapses +/// multi-target lists (CR 115.7a / CR 115.7b) — upstream cause filter.rs +/// FilterProp::HasSingleTarget is permissive with no resolution-time +/// validation; fix needs filter.rs + interaction.rs, both outside phase 1's +/// frozen scope. +#[test] +fn fallback_multi_role_retarget_action_is_slot_legal() { + let mut runner = GameScenario::new().build(); + + let role = ManaTargetRole::Both { + // Slot 0 (recipient, surfaced first): only an opponent of P0, i.e. P1. + recipient: TargetFilter::Typed(TypedFilter::default().controller(ControllerRef::Opponent)), + // Slot 1 (count source): any player. + count_source: TargetFilter::Player, + }; + let source = create_object( + runner.state_mut(), + CardId(901), + P0, + "Multi-Role Mana Source".to_string(), + Zone::Battlefield, + ); + let entry_id = create_object( + runner.state_mut(), + CardId(901), + P0, + "Multi-Role Mana Ability".to_string(), + Zone::Stack, + ); + + // Slot 0 holds P1, not P0 — so proposing `[P0]` would be a genuine change + // into a slot it is illegal for, rather than an exempt non-change. + let current_targets = vec![TargetRef::Player(P1), TargetRef::Player(P0)]; + let ability = ResolvedAbility::new( + Effect::Mana { + produced: ManaProduction::Colorless { + count: QuantityExpr::Fixed { value: 1 }, + }, + restrictions: vec![], + grants: vec![], + expiry: None, + target: Some(role), + }, + current_targets.clone(), + source, + P0, + ); + runner.state_mut().stack.push_back(StackEntry { + id: entry_id, + source_id: source, + controller: P0, + kind: StackEntryKind::ActivatedAbility { + source_id: source, + ability: Box::new(ability), + }, + }); + + // Both structural reach-guards: without the entry, `retarget_actions`' + // `is_none_or` passes every candidate unfiltered and `apply_retarget` skips + // its per-slot stage, so this row would be vacuous in both directions. + assert!( + runner.state().stack[0].ability().is_some(), + "reach guard: stack index 0 must carry the ability under test" + ); + assert!( + mana_multi_role(&runner.state().stack[0].ability().unwrap().effect).is_some(), + "reach guard: the node must be inside the per-slot admitted class" + ); + + runner.state_mut().waiting_for = WaitingFor::RetargetChoice { + player: P0, + stack_entry_index: 0, + scope: RetargetScope::Single, + current_targets, + legal_new_targets: vec![TargetRef::Player(P0), TargetRef::Player(P1)], + }; + + let action = fallback_for_prompt(&runner); + + assert!(action.is_some(), "reach guard: the fallback must answer"); + assert!( + matches!(action, Some(GameAction::RetargetSpell { .. })), + "reach guard: the RetargetChoice arm must be the one that answered, got {action:?}" + ); + + // Discriminating: `[P0]` is in the flat pool but legal only for slot 1, so + // it must never be the proposal; only the slot-0-legal `[P1]` may be. + assert_eq!( + action, + Some(GameAction::RetargetSpell { + new_targets: vec![TargetRef::Player(P1)], + }), + "CR 115.7a: the fallback must not propose a pool member legal only for \ + another slot" + ); + + // Discriminating: at base the action is `[P1, P0]` (length 2), which the + // `Single` arm rejects outright. + // + // "Accepted by the reducer" is the WHOLE claim here — acceptance, not + // rules-correctness. Applying this length-1 submission to the length-2 + // target list truncates the count-source slot, contrary to CR 115.7a / + // CR 115.7b alike. See this row's SCOPE note and the DEFERRED(out-of-run) + // entry it carries; that truncation is why this row deliberately asserts + // only that the submission is accepted, and never asserts the resulting + // `ability.targets`. + runner.act(action.unwrap()).expect( + "the fallback's action must be ACCEPTED by the reducer — acceptance only, not a \ + claim of CR-115.7a / CR-115.7b legality; see this row's SCOPE note", + ); +}