diff --git a/client/src/adapter/generated/interaction/index.ts b/client/src/adapter/generated/interaction/index.ts index 58d5e9ea40..d836708684 100644 --- a/client/src/adapter/generated/interaction/index.ts +++ b/client/src/adapter/generated/interaction/index.ts @@ -29,7 +29,7 @@ export type InteractionSummaryCode = "decision" | "candidate" | "source" | "sele export type InteractionZoneCode = "battlefield" | "hand" | "library" | "graveyard" | "exile" | "stack" | "command" | "outsideGame" | "other"; -export type InteractionIntentCode = "choose" | "keep" | "sacrifice" | "return" | "exile" | "tap" | "crew" | "saddle" | "station" | "ringBearer" | "blight" | "pay" | "attack" | "block"; +export type InteractionIntentCode = "choose" | "keep" | "sacrifice" | "return" | "exile" | "tap" | "crew" | "saddle" | "station" | "ringBearer" | "blight" | "pay" | "attack" | "block" | "damage" | "destroy" | "regenerate" | "counter" | "untap" | "mill" | "discard" | "draw" | "gainLife" | "loseLife" | "fight" | "attach" | "copy" | "gainControl" | "reveal" | "modify" | "buff" | "debuff"; export type AggregateComparator = "greaterThan" | "lessThan" | "atLeast" | "atMost" | "equal" | "notEqual"; diff --git a/crates/engine/src/ai_support/candidates.rs b/crates/engine/src/ai_support/candidates.rs index 76386eb8de..3639b9256d 100644 --- a/crates/engine/src/ai_support/candidates.rs +++ b/crates/engine/src/ai_support/candidates.rs @@ -5163,6 +5163,7 @@ fn combinations_generic(items: &[T], k: usize) -> Vec> { #[cfg(test)] mod tests { + use crate::types::game_state::TargetEffectDetail; use std::sync::Arc; use super::*; @@ -5861,6 +5862,8 @@ mod tests { legal_targets: vec![TargetRef::Object(target_a), TargetRef::Object(target_b)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(), diff --git a/crates/engine/src/ai_support/combat_withdrawal.rs b/crates/engine/src/ai_support/combat_withdrawal.rs index 9dda6f5e36..ddf1bf8587 100644 --- a/crates/engine/src/ai_support/combat_withdrawal.rs +++ b/crates/engine/src/ai_support/combat_withdrawal.rs @@ -193,6 +193,7 @@ fn single_member( #[cfg(test)] mod tests { + use crate::types::game_state::TargetEffectDetail; use std::collections::HashMap; use std::sync::Arc; @@ -200,6 +201,7 @@ mod tests { use crate::game::combat::{AttackTarget, AttackerInfo, CombatState}; use crate::game::engine::apply_as_current_for_simulation; use crate::game::zones::create_object; + use crate::types::ability::EffectKind; use crate::types::ability::{ AbilityDefinition, AbilityKind, Effect, PtValue, TargetFilter, TargetRef, TypedFilter, }; @@ -273,6 +275,8 @@ mod tests { legal_targets: legal_targets.clone(), optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: TargetSelectionProgress { diff --git a/crates/engine/src/ai_support/mod.rs b/crates/engine/src/ai_support/mod.rs index a57703c09f..b44edfde70 100644 --- a/crates/engine/src/ai_support/mod.rs +++ b/crates/engine/src/ai_support/mod.rs @@ -2090,6 +2090,7 @@ fn activatable_object_mana_actions(state: &GameState) -> Vec { #[cfg(test)] mod tests { + use crate::types::game_state::TargetEffectDetail; use std::collections::HashMap; use std::sync::Arc; @@ -2102,6 +2103,7 @@ mod tests { use crate::game::mana_sources; use crate::game::zones::create_object; use crate::parser::oracle::parse_oracle_text; + use crate::types::ability::EffectKind; use crate::types::ability::{ AbilityCost, AbilityDefinition, AbilityKind, ChoiceType, ContinuousModification, ControllerRef, Effect, FilterProp, ManaContribution, ManaProduction, QuantityExpr, @@ -5484,6 +5486,8 @@ mod tests { legal_targets: vec![target.clone()], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: crate::types::game_state::TargetSelectionProgress { @@ -5535,6 +5539,8 @@ mod tests { legal_targets: targets.clone(), optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: crate::types::game_state::TargetSelectionProgress { @@ -5638,6 +5644,8 @@ mod tests { legal_targets: vec![target], optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: crate::types::game_state::TargetSelectionProgress { diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index 1d42eb04e7..ed77913be4 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -3,16 +3,17 @@ use crate::types::ability::TapStateChange; use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AdditionalCost, CardTypeSetSource, CastManaSpentMetric, CombatRelationSubject, ControllerRef, - CounterMoveSelection, DamageSource, Effect, EffectScope, FilterProp, GameRestriction, - ModalChoice, ModalSelectionCondition, ModalSelectionConstraint, MultiTargetSpec, ObjectScope, - PlayerFilter, PlayerScope, QuantityExpr, QuantityRef, ResolvedAbility, RestrictionPlayerScope, - SpellContext, SubAbilityLink, TargetChoiceTiming, TargetFilter, TargetRef, TriggerDefinition, - TypeFilter, TypedFilter, + CounterMoveSelection, DamageSource, Effect, EffectKind, EffectScope, FilterProp, + GameRestriction, ModalChoice, ModalSelectionCondition, ModalSelectionConstraint, + MultiTargetSpec, ObjectScope, PlayerFilter, PlayerScope, PtValue, QuantityExpr, QuantityRef, + ResolvedAbility, RestrictionPlayerScope, SpellContext, SubAbilityLink, TargetChoiceTiming, + TargetFilter, TargetRef, TriggerDefinition, TypeFilter, TypedFilter, }; #[cfg(test)] use crate::types::counter::CounterType; use crate::types::game_state::{ - GameState, TargetSelectionConstraint, TargetSelectionProgress, TargetSelectionSlot, + GameState, PtDirection, TargetEffectDetail, TargetSelectionConstraint, TargetSelectionProgress, + TargetSelectionSlot, }; use crate::types::identifiers::ObjectId; use crate::types::player::PlayerId; @@ -363,7 +364,6 @@ pub fn find_first_target_filter_in_chain(ability: &ResolvedAbility) -> Option<&T /// given mode's collection inherits that mode's `current_label`. Non-modal /// collection leaves `current_label` `None`, so `labels` ends up all-`None` /// (callers that don't need labels read `slots` and discard `labels`). -#[derive(Default)] struct SlotAccumulator { slots: Vec, labels: Vec>, @@ -377,6 +377,36 @@ struct SlotAccumulator { /// announcer (the CR-601.2c default). Set/restored by `collect_target_slots` /// per link so each chained sub-ability stamps only its own slots. current_chooser: Option, + /// CR 115.1: effect kind of the link currently being recursed, applied to + /// every slot it pushes. Scoped exactly like `current_chooser`: set before + /// a link's slots are collected and restored afterwards, so a chained + /// sub-ability's slots carry that sub-ability's effect rather than the + /// head link's. `collect_target_slots` recurses per link, so every slot + /// pushed within one frame belongs to that frame's `ability.effect`. + current_effect_kind: EffectKind, + /// CR 115.1: the discriminating payload of the link currently being + /// recursed, read by `target_effect_detail`. Scoped exactly like + /// `current_effect_kind`. + current_effect_detail: TargetEffectDetail, +} + +impl Default for SlotAccumulator { + /// `current_effect_kind` seeds to `NoOp` purely to have a value: both + /// constructors (`build_target_slots`, `build_target_slots_labelled`) call + /// `collect_target_slots` before any push, and that sets the real kind, so + /// the seed can never reach a slot. `EffectKind` has no `Default` of its + /// own and should not gain one for this — a 231-variant effect tag has no + /// meaningful default outside this accumulator. + fn default() -> Self { + Self { + slots: Vec::new(), + labels: Vec::new(), + current_label: None, + current_chooser: None, + current_effect_kind: EffectKind::NoOp, + current_effect_detail: TargetEffectDetail::None, + } + } } impl SlotAccumulator { @@ -2237,11 +2267,76 @@ fn collect_target_slots( .filter(|&player| player != ability.controller) }); let previous_chooser = std::mem::replace(&mut acc.current_chooser, resolved_chooser); + // CR 115.1: stamp this link's own effect on the slots it is about to push. + // Restored below so a chained sub-ability does not leak its kind upward. + let previous_effect_kind = std::mem::replace( + &mut acc.current_effect_kind, + EffectKind::from(&ability.effect), + ); + let previous_effect_detail = std::mem::replace( + &mut acc.current_effect_detail, + target_effect_detail(&ability.effect), + ); let result = collect_target_slots_inner(state, ability, acc); acc.current_chooser = previous_chooser; + acc.current_effect_kind = previous_effect_kind; + acc.current_effect_detail = previous_effect_detail; result } +/// CR 115.1: Read the fact `EffectKind` cannot carry off the effect's own +/// payload, at the one point where the payload is in hand. +/// +/// Only the kinds whose unit tag is genuinely ambiguous about what happens to +/// the target are covered; everything else is [`TargetEffectDetail::None`], +/// which is also the honest answer whenever the deciding value is not +/// statically known. This runs at construction (not projection) because +/// `WaitingFor::TriggerTargetSelection` carries no ability reference, so a +/// projection-time read would resolve spell targeting and not trigger +/// targeting — labelling the same effect differently depending on how it +/// reached the stack. Construction is symmetric: both `build_target_slots` and +/// `build_target_slots_labelled` route through `collect_target_slots`. +fn target_effect_detail(effect: &Effect) -> TargetEffectDetail { + match effect { + // The zone family's tag says "a zone change happened", never which + // zone. Exile and return-to-hand share `EffectKind::ChangeZone`. + Effect::ChangeZone { destination, .. } | Effect::ChangeZoneAll { destination, .. } => { + TargetEffectDetail::Destination(*destination) + } + // CR 613.4: `Effect::Pump` is one kind for "+3/+3" and "-3/-3". + Effect::Pump { + power, toughness, .. + } + | Effect::PumpAll { + power, toughness, .. + } => pt_direction(power, toughness) + .map_or(TargetEffectDetail::None, TargetEffectDetail::Modification), + _ => TargetEffectDetail::None, + } +} + +/// CR 613.4: Direction of a P/T modification, or `None` when no single +/// direction is true. +/// +/// `None` covers two real populations rather than being a catch-all: a dynamic +/// magnitude (X or count-based, where the sign is not knowable at announcement +/// — CR 601.2c fixes targets before X is locked for many cards) and a genuinely +/// opposing modification such as "+2/-2", which is neither a buff nor a debuff. +/// A one-sided change like "-4/-0" IS directional and resolves. +fn pt_direction(power: &PtValue, toughness: &PtValue) -> Option { + let fixed = |value: &PtValue| match value { + PtValue::Fixed(amount) => Some(*amount), + PtValue::Variable(_) | PtValue::Quantity(_) => None, + }; + let (power, toughness) = (fixed(power)?, fixed(toughness)?); + match (power.signum(), toughness.signum()) { + (1, 0 | 1) | (0, 1) => Some(PtDirection::Increase), + (-1, 0 | -1) | (0, -1) => Some(PtDirection::Decrease), + // Both zero (no change) or opposing signs: no direction is true. + _ => None, + } +} + fn collect_target_slots_inner( state: &GameState, ability: &ResolvedAbility, @@ -2289,6 +2384,8 @@ fn collect_target_slots_inner( legal_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } } @@ -2313,6 +2410,8 @@ fn collect_target_slots_inner( legal_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } return Ok(()); @@ -2339,6 +2438,8 @@ fn collect_target_slots_inner( legal_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } return Ok(()); @@ -2377,6 +2478,8 @@ fn collect_target_slots_inner( legal_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } return Ok(()); @@ -2404,6 +2507,8 @@ fn collect_target_slots_inner( legal_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } } else if let Effect::Attach { attachment, target } = &ability.effect { @@ -2420,6 +2525,8 @@ fn collect_target_slots_inner( legal_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } } else if let Effect::CreateDamageReplacement { @@ -2461,6 +2568,8 @@ fn collect_target_slots_inner( legal_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } } else if let Effect::EachDealsDamageEqualToPower { @@ -2490,6 +2599,8 @@ fn collect_target_slots_inner( legal_targets: source_legal.clone(), optional: slot_index >= bounds.min, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } } else { @@ -2504,6 +2615,8 @@ fn collect_target_slots_inner( legal_targets: source_legal, optional: false, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } @@ -2520,6 +2633,8 @@ fn collect_target_slots_inner( legal_targets: extra_legal, optional: true, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } @@ -2535,6 +2650,8 @@ fn collect_target_slots_inner( legal_targets: recipient_legal, optional: false, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } } else { @@ -2581,6 +2698,8 @@ fn collect_target_slots_inner( legal_targets: player_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } if ability.target_choice_timing == TargetChoiceTiming::Stack @@ -2600,6 +2719,8 @@ fn collect_target_slots_inner( legal_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } if ability.target_choice_timing == TargetChoiceTiming::Stack @@ -2617,6 +2738,8 @@ fn collect_target_slots_inner( legal_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } if ability.target_choice_timing == TargetChoiceTiming::Stack @@ -2637,6 +2760,8 @@ fn collect_target_slots_inner( legal_targets: legal_targets.clone(), optional: slot_index >= bounds.min, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } } else { @@ -2649,6 +2774,8 @@ fn collect_target_slots_inner( legal_targets, optional: ability.optional_targeting, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } } @@ -3324,6 +3451,8 @@ fn collect_attach_attachment_target_slots( legal_targets: legal_targets.clone(), optional: slot_index >= bounds.min, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } } else { @@ -3331,6 +3460,8 @@ fn collect_attach_attachment_target_slots( legal_targets, optional: ability.targeting_is_optional(), chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } Ok(()) @@ -4729,11 +4860,15 @@ fn collect_per_opponent_target_fanout_slots( legal_targets: player_targets, optional: false, chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); acc.push(TargetSelectionSlot { legal_targets, optional: ability.targeting_is_optional(), chooser: None, + effect_kind: acc.current_effect_kind, + effect_detail: acc.current_effect_detail, }); } @@ -8411,6 +8546,185 @@ mod tests { ); } + /// CR 601.2c + CR 115.1: each announced slot carries its OWN link's effect, + /// not the head link's. + /// + /// This is the discriminating assertion for per-frame stamping: verified by + /// injecting a whole-chain stamp (sub-abilities inheriting the head link's + /// kind), which makes slot 1 report `DealDamage` and fails here. + /// + /// What it does NOT cover, stated so it is not assumed: the + /// `acc.current_effect_kind = previous_effect_kind` restore in + /// `collect_target_slots`. Deleting that restore leaves this test green, + /// because no reachable path in `collect_target_slots_inner` pushes a slot + /// after recursing into a sub-ability — the one mid-frame recursion + /// (`is_per_opponent_target_fanout`) returns immediately after. The restore + /// is defensive symmetry with `current_chooser`, which has the identical + /// set/restore structure, and would become load-bearing the moment a frame + /// pushes after recursing. A whole-chain stamp is the bug that + /// would silently re-label a pump prompt as a damage prompt. + #[test] + fn chained_ability_stamps_each_target_slot_with_its_own_links_effect() { + let mut state = GameState::new_two_player(42); + let source = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Chained Source".to_string(), + Zone::Stack, + ); + let creature = create_object( + &mut state, + CardId(2), + PlayerId(0), + "Bear".to_string(), + Zone::Battlefield, + ); + state + .objects + .get_mut(&creature) + .unwrap() + .card_types + .core_types + .push(CoreType::Creature); + + // "Deal 2 damage to target creature. Target creature gets +1/+1." + let ability = ResolvedAbility::new( + Effect::DealDamage { + amount: QuantityExpr::Fixed { value: 2 }, + target: TargetFilter::Typed(TypedFilter::creature()), + damage_source: None, + excess: None, + }, + vec![], + source, + PlayerId(0), + ) + .sub_ability(ResolvedAbility::new( + Effect::Pump { + power: PtValue::Fixed(1), + toughness: PtValue::Fixed(1), + target: TargetFilter::Typed(TypedFilter::creature()), + }, + vec![], + source, + PlayerId(0), + )); + + let slots = build_target_slots(&state, &ability).expect("target slots should build"); + assert_eq!(slots.len(), 2, "each link declares one creature target"); + assert_eq!( + slots[0].effect_kind, + EffectKind::DealDamage, + "slot 0 belongs to the damage link" + ); + assert_eq!( + slots[1].effect_kind, + EffectKind::Pump, + "slot 1 belongs to the pump link — a whole-chain stamp would report DealDamage here" + ); + } + + /// CR 115.1: the discriminating payload `EffectKind` cannot carry is read + /// off the effect at construction, where it is symmetric across the spell + /// and trigger paths. + /// + /// Both `build_target_slots` and `build_target_slots_labelled` route + /// through `collect_target_slots`, so this holds for a triggered ability + /// too — which is why the read is done here and NOT at projection time, + /// where `WaitingFor::TriggerTargetSelection` carries no ability reference + /// and the same effect would be labelled differently depending on whether + /// it arrived as a spell or a trigger. + #[test] + fn slot_detail_carries_the_fact_the_effect_kind_cannot() { + let mut state = GameState::new_two_player(42); + let source = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Payload Source".to_string(), + Zone::Stack, + ); + let creature = create_object( + &mut state, + CardId(2), + PlayerId(0), + "Bear".to_string(), + Zone::Battlefield, + ); + state + .objects + .get_mut(&creature) + .unwrap() + .card_types + .core_types + .push(CoreType::Creature); + + let pump = |power: i32, toughness: i32| { + ResolvedAbility::new( + Effect::Pump { + power: PtValue::Fixed(power), + toughness: PtValue::Fixed(toughness), + target: TargetFilter::Typed(TypedFilter::creature()), + }, + vec![], + source, + PlayerId(0), + ) + }; + + // "+3/+3" and "-3/-3" are the SAME `EffectKind`; only the detail + // separates them. This is the assertion that fails if the payload read + // is dropped. + let buff = build_target_slots(&state, &pump(3, 3)).expect("slots"); + assert_eq!(buff[0].effect_kind, EffectKind::Pump); + assert_eq!( + buff[0].effect_detail, + TargetEffectDetail::Modification(PtDirection::Increase) + ); + let debuff = build_target_slots(&state, &pump(-3, -3)).expect("slots"); + assert_eq!(debuff[0].effect_kind, EffectKind::Pump); + assert_eq!( + debuff[0].effect_detail, + TargetEffectDetail::Modification(PtDirection::Decrease) + ); + + // A one-sided reduction ("-4/-0") IS directional and must resolve. + let one_sided = build_target_slots(&state, &pump(-4, 0)).expect("slots"); + assert_eq!( + one_sided[0].effect_detail, + TargetEffectDetail::Modification(PtDirection::Decrease), + "-4/-0 is a debuff, not an undirected modification" + ); + + // A genuinely opposing modification claims no direction rather than + // guessing one. + let opposing = build_target_slots(&state, &pump(2, -2)).expect("slots"); + assert_eq!( + opposing[0].effect_detail, + TargetEffectDetail::None, + "+2/-2 is neither a buff nor a debuff" + ); + + // A dynamic magnitude is not knowable at announcement (CR 601.2c fixes + // targets before X is locked), so it also declines. + let dynamic = ResolvedAbility::new( + Effect::Pump { + power: PtValue::Variable("X".to_string()), + toughness: PtValue::Variable("X".to_string()), + target: TargetFilter::Typed(TypedFilter::creature()), + }, + vec![], + source, + PlayerId(0), + ); + assert_eq!( + build_target_slots(&state, &dynamic).expect("slots")[0].effect_detail, + TargetEffectDetail::None, + "an X-sized pump has no statically known direction" + ); + } + #[test] fn search_library_collects_later_independent_stack_targets() { let mut state = GameState::new_two_player(42); @@ -9172,6 +9486,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, TargetSelectionSlot { legal_targets: vec![ @@ -9180,6 +9496,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, ]; @@ -9543,6 +9861,8 @@ mod tests { legal_targets: vec![TargetRef::Player(PlayerId(1))], optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }]; let selected = auto_select_targets(&slots, &[]).expect("optional targeting stays legal"); @@ -9557,11 +9877,15 @@ mod tests { legal_targets: vec![TargetRef::Player(PlayerId(0))], optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, TargetSelectionSlot { legal_targets: vec![TargetRef::Player(PlayerId(0))], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, ]; @@ -9579,11 +9903,15 @@ mod tests { legal_targets: vec![TargetRef::Player(PlayerId(1))], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, TargetSelectionSlot { legal_targets: vec![TargetRef::Player(PlayerId(1))], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, ]; @@ -9603,6 +9931,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, TargetSelectionSlot { legal_targets: vec![ @@ -9611,6 +9941,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, ]; @@ -9646,11 +9978,15 @@ mod tests { legal_targets: vec![TargetRef::Player(PlayerId(1))], optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, TargetSelectionSlot { legal_targets: vec![TargetRef::Object(ObjectId(42))], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, ]; @@ -12387,11 +12723,15 @@ mod tests { legal_targets: vec![], optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, TargetSelectionSlot { legal_targets: vec![], optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, ] }; @@ -12725,6 +13065,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }; let chosen = random_select_targets_for_ability(&mut state, std::slice::from_ref(&slot), &[]) @@ -12746,6 +13088,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }; let mut state_a = GameState::new_two_player(1234); let mut state_b = GameState::new_two_player(1234); @@ -12768,6 +13112,8 @@ mod tests { legal_targets: vec![], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }; let result = random_select_targets_for_ability(&mut state, &[slot], &[]); assert!(result.is_err(), "empty legal-target set must error"); @@ -12783,6 +13129,8 @@ mod tests { legal_targets: vec![], optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }; let chosen = random_select_targets_for_ability(&mut state, &[slot], &[]) .expect("optional empty slot resolves to empty selection"); @@ -12803,6 +13151,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }; let slot_b = TargetSelectionSlot { legal_targets: vec![ @@ -12811,6 +13161,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }; let chosen = random_select_targets_for_ability(&mut state, &[slot_a.clone(), slot_b.clone()], &[]) @@ -12834,11 +13186,15 @@ mod tests { legal_targets: vec![shared.clone()], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }; let slot_optional = TargetSelectionSlot { legal_targets: vec![shared.clone()], optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }; // Required + required: second slot has no remaining legal target → error. let err = random_select_targets_for_ability( diff --git a/crates/engine/src/game/casting.rs b/crates/engine/src/game/casting.rs index b5155f545b..bbedb54252 100644 --- a/crates/engine/src/game/casting.rs +++ b/crates/engine/src/game/casting.rs @@ -3,10 +3,11 @@ use crate::types::ability::{ AbilityCost, AbilityDefinition, AbilityKind, AbilityTag, ActivationManaPaymentRestriction, AdditionalCost, CardPlayMode, CardSelectionMode, CastTimingPermission, CastingPermission, ChoiceType, ContinuousModification, CostObjectCount, CostPaidObjectSnapshot, - CounterCostSelection, Duration, Effect, FilterProp, GameRestriction, ModalSelectionCondition, - ObjectScope, PlayerFilter, PlayerScope, ProhibitedActivity, QuantityExpr, QuantityRef, - ResolvedAbility, RestrictionExpiry, RestrictionPlayerScope, StaticCondition, StaticDefinition, - SubAbilityLink, TapCreaturesRequirement, TargetFilter, TargetRef, + CounterCostSelection, Duration, Effect, EffectKind, FilterProp, GameRestriction, + ModalSelectionCondition, ObjectScope, PlayerFilter, PlayerScope, ProhibitedActivity, + QuantityExpr, QuantityRef, ResolvedAbility, RestrictionExpiry, RestrictionPlayerScope, + StaticCondition, StaticDefinition, SubAbilityLink, TapCreaturesRequirement, TargetFilter, + TargetRef, }; use crate::types::actions::AlternativeCastDecision; use crate::types::card::LayoutKind; @@ -16,7 +17,7 @@ use crate::types::game_state::{ CastingPermissionIndex, CastingVariant, CastingVariantChoiceOption, ConvokeMode, CostResume, GameState, ManaAbilityCostParent, ManaAbilityResume, NextSpellModifier, PayCostKind, PendingCast, PendingCostMoveResume, SneakPlacement, SpellCostSource, StackEntry, - StackEntryKind, TargetSelectionSlot, WaitingFor, + StackEntryKind, TargetEffectDetail, TargetSelectionSlot, WaitingFor, }; use crate::types::identifiers::{CardId, ObjectId, TrackedSetId}; use crate::types::keywords::{FlashbackCost, Keyword, KeywordKind}; @@ -11619,10 +11620,15 @@ fn continue_with_prepared( "No legal targets for Aura".to_string(), )); } + // CR 303.4a + CR 702.5a: the enchant-defined target is the + // permanent this Aura will attach to on resolution, so `Attach` is + // the effect the chosen target will be subject to. let target_slots = vec![crate::types::game_state::TargetSelectionSlot { legal_targets: legal, optional: false, chooser: None, + effect_kind: EffectKind::Attach, + effect_detail: TargetEffectDetail::None, }]; if let Some(targets) = auto_select_targets(&target_slots, &[])? { let mut resolved = resolved; @@ -11691,10 +11697,17 @@ fn continue_with_prepared( "No legal target for mutate".to_string(), )); } + // CR 702.140a: mutate is resolved by the casting pipeline, not by an + // `Effect`, so there is no effect tag to name here. `NoOp` is the + // honest "the engine has no effect kind for this target" marker and + // projects as the neutral `Choose` intent rather than claiming a + // semantic the engine does not have. let target_slots = vec![crate::types::game_state::TargetSelectionSlot { legal_targets: legal, optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }]; if let Some(targets) = auto_select_targets(&target_slots, &[])? { let mut resolved = resolved; @@ -12476,6 +12489,9 @@ fn legal_target_slots_for_castable_spell_in_flushed_state( ), optional: false, chooser: None, + // CR 303.4a + CR 702.5a: see the cast path above. + effect_kind: EffectKind::Attach, + effect_detail: TargetEffectDetail::None, }) } else { None @@ -12502,6 +12518,10 @@ fn legal_target_slots_for_castable_spell_in_flushed_state( legal_targets: legal, optional: false, chooser: None, + // CR 702.140a: see the cast path above — no `Effect` backs + // mutate, so no effect kind names it. + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }] }); } diff --git a/crates/engine/src/game/engine_trigger_target_tests.rs b/crates/engine/src/game/engine_trigger_target_tests.rs index efff440280..09650f6ee4 100644 --- a/crates/engine/src/game/engine_trigger_target_tests.rs +++ b/crates/engine/src/game/engine_trigger_target_tests.rs @@ -5,7 +5,7 @@ use crate::types::ability::{ QuantityExpr, ResolvedAbility, StaticCondition, TargetFilter, TargetRef, TypedFilter, }; use crate::types::card_type::CoreType; -use crate::types::game_state::TargetSelectionConstraint; +use crate::types::game_state::{TargetEffectDetail, TargetSelectionConstraint}; use crate::types::identifiers::CardId; #[test] @@ -128,6 +128,8 @@ fn trigger_target_selection_select_targets_pushes_to_stack() { legal_targets: legal_targets.clone(), optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], target_constraints: Vec::new(), selection: crate::game::ability_utils::begin_target_selection( @@ -135,6 +137,8 @@ fn trigger_target_selection_select_targets_pushes_to_stack() { legal_targets: legal_targets.clone(), optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], &[], ) @@ -238,6 +242,8 @@ fn trigger_target_selection_rejects_illegal_target() { legal_targets: vec![TargetRef::Object(legal_target)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(), @@ -823,6 +829,8 @@ fn trigger_target_selection_enforces_different_player_constraint() { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, crate::types::game_state::TargetSelectionSlot { legal_targets: vec![ @@ -831,6 +839,8 @@ fn trigger_target_selection_enforces_different_player_constraint() { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, ], mode_labels: Vec::new(), @@ -892,6 +902,8 @@ fn choose_target_action_advances_trigger_selection_from_engine_state() { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, crate::types::game_state::TargetSelectionSlot { legal_targets: vec![ @@ -900,6 +912,8 @@ fn choose_target_action_advances_trigger_selection_from_engine_state() { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }, ]; let target_constraints = vec![TargetSelectionConstraint::DifferentTargetPlayers]; diff --git a/crates/engine/src/game/interaction.rs b/crates/engine/src/game/interaction.rs index 53cc15c07f..d04c96feaa 100644 --- a/crates/engine/src/game/interaction.rs +++ b/crates/engine/src/game/interaction.rs @@ -32,7 +32,7 @@ use crate::types::game_state::{ ActionResult, AutoMayChoice, CastPaymentMode, CastingVariant, CombatDamageAssignmentMode, ConvokeMode, CounterCostChoice, CounterMoveChoice, CounterRemoveChoice, GameState, ManaChoice, ManaChoiceContext, ManaChoicePrompt, OutsideGameChoiceSource, PayCostKind, PileSide, - ShardChoice, ShardOptions, WaitingFor, + PtDirection, ShardChoice, ShardOptions, TargetEffectDetail, WaitingFor, }; use crate::types::identifiers::ObjectId; use crate::types::interaction::{ @@ -983,6 +983,12 @@ struct TargetSequenceProjection { max: usize, unique: bool, action: TargetSequenceAction, + /// CR 115.1: what the announcing spell/ability will do to the chosen + /// target, derived from the current slot's `effect_kind`. Only the two + /// slot-carrying states (`TargetSelection`, `TriggerTargetSelection`) can + /// answer this; every other state on this model is a "choose" without a + /// per-slot effect attribution and stays neutral. + intent: InteractionIntentCode, } #[derive(Debug, Clone)] @@ -1137,15 +1143,22 @@ fn target_sequence_projection( selection, .. } => { - let optional = target_slots - .get(selection.current_slot) - .is_some_and(|slot| slot.optional); + let slot = target_slots.get(selection.current_slot); + let optional = slot.is_some_and(|slot| slot.optional); TargetSequenceProjection { candidates: selection.current_legal_targets.clone(), min: usize::from(!optional), max: 1, unique: true, action: TargetSequenceAction::ChooseTarget, + // CR 601.2c: targets are announced one slot at a time, and each + // slot carries its own effect, so the label is per-slot rather + // than per-spell. A chained "destroy target creature. Draw a + // card. Target player gains 2 life" announces two slots with + // two different intents. + intent: slot.map_or(InteractionIntentCode::Choose, |slot| { + target_intent(slot.effect_kind, slot.effect_detail) + }), } } WaitingFor::TriggerTargetSelection { @@ -1153,15 +1166,22 @@ fn target_sequence_projection( selection, .. } => { - let optional = target_slots - .get(selection.current_slot) - .is_some_and(|slot| slot.optional); + let slot = target_slots.get(selection.current_slot); + let optional = slot.is_some_and(|slot| slot.optional); TargetSequenceProjection { candidates: selection.current_legal_targets.clone(), min: usize::from(!optional), max: 1, unique: true, action: TargetSequenceAction::ChooseTarget, + // CR 601.2c: targets are announced one slot at a time, and each + // slot carries its own effect, so the label is per-slot rather + // than per-spell. A chained "destroy target creature. Draw a + // card. Target player gains 2 life" announces two slots with + // two different intents. + intent: slot.map_or(InteractionIntentCode::Choose, |slot| { + target_intent(slot.effect_kind, slot.effect_detail) + }), } } WaitingFor::MultiTargetSelection { @@ -1179,6 +1199,7 @@ fn target_sequence_projection( max: *max_targets, unique: true, action: TargetSequenceAction::SelectObjects, + intent: InteractionIntentCode::Choose, }, WaitingFor::ChooseObjectsSelection { eligible, .. } => TargetSequenceProjection { candidates: eligible.clone(), @@ -1186,6 +1207,7 @@ fn target_sequence_projection( max: eligible.len(), unique: true, action: TargetSequenceAction::SelectTargets, + intent: InteractionIntentCode::Choose, }, WaitingFor::EachPlayerCopyChosenSelection { eligible, min, max, .. @@ -1195,6 +1217,7 @@ fn target_sequence_projection( max: *max as usize, unique: true, action: TargetSequenceAction::SelectTargets, + intent: InteractionIntentCode::Choose, }, WaitingFor::ProliferateChoice { eligible, .. } | WaitingFor::TimeTravelChoice { eligible, .. } => TargetSequenceProjection { @@ -1203,6 +1226,7 @@ fn target_sequence_projection( max: eligible.len(), unique: true, action: TargetSequenceAction::SelectTargets, + intent: InteractionIntentCode::Choose, }, WaitingFor::RetargetChoice { scope, @@ -1225,6 +1249,10 @@ fn target_sequence_projection( max: count, unique: false, action: TargetSequenceAction::Retarget, + // CR 115.7: retargeting changes an existing spell's targets. The + // intent belongs to that spell, not to this choice, and this + // state carries no slot to read it from. + intent: InteractionIntentCode::Choose, } } _ => return Ok(None), @@ -2603,6 +2631,127 @@ fn effect_zone_intent(effect_kind: EffectKind, destination: Option) -> Int } } +/// CR 115.1 + CR 601.2c: Label a target announcement with what the announcing +/// spell or ability will do to the thing being chosen. +/// +/// Mirrors `effect_zone_intent` above: the slot stores the game fact +/// (`EffectKind`), and this projection-layer function decides how to present +/// it. Nothing about the label is stored in `GameState`. +/// +/// The catch-all is deliberate and safe. `EffectKind` has 231 variants, the +/// overwhelming majority of which never reach a CR 115.1 target slot, and the +/// fallback is the NEUTRAL `Choose` — so an unmapped or newly-added kind reads +/// as "just pick one", which is an honest partial rather than a wrong claim. +/// An exhaustive match here would be 231 arms whose default answer is the same +/// value, and would make every unrelated effect addition edit this function. +fn target_intent(effect_kind: EffectKind, detail: TargetEffectDetail) -> InteractionIntentCode { + // The zone family's unit tag says only "a zone change"; the destination is + // the deciding fact and is stamped on the slot. Delegate to the existing + // zone labeller rather than reimplementing it, so `EffectZoneChoice` and a + // zone-change target announcement can never disagree. + if let TargetEffectDetail::Destination(destination) = detail { + return effect_zone_intent(effect_kind, Some(destination)); + } + match effect_kind { + // CR 120.1: damage. + EffectKind::DealDamage + | EffectKind::DamageAll + | EffectKind::DamageEachPlayer + | EffectKind::ApplyPostReplacementDamage + | EffectKind::EachDealsDamageEqualToPower + | EffectKind::EachSourceDealsDamage => InteractionIntentCode::Damage, + // CR 701.8: destroy. + EffectKind::Destroy | EffectKind::DestroyAll => InteractionIntentCode::Destroy, + // CR 701.19: regeneration shield. Unambiguously protective. + EffectKind::Regenerate | EffectKind::RemoveAllDamage => InteractionIntentCode::Regenerate, + // CR 701.6: counter a spell. + EffectKind::Counter | EffectKind::CounterAll => InteractionIntentCode::Counter, + // CR 701.21: sacrifice. + EffectKind::Sacrifice | EffectKind::ChooseAndSacrificeRest | EffectKind::Exploit => { + InteractionIntentCode::Sacrifice + } + // CR 701.26: tap / untap. + EffectKind::Tap | EffectKind::TapAll => InteractionIntentCode::Tap, + EffectKind::Untap | EffectKind::UntapAll => InteractionIntentCode::Untap, + // CR 701.17: mill. + EffectKind::Mill => InteractionIntentCode::Mill, + // CR 701.9: discard. + EffectKind::DiscardCard | EffectKind::Discard => InteractionIntentCode::Discard, + // CR 121.1: draw. + EffectKind::Draw => InteractionIntentCode::Draw, + // CR 119.3: life gain / loss. + EffectKind::GainLife => InteractionIntentCode::GainLife, + EffectKind::LoseLife => InteractionIntentCode::LoseLife, + // CR 701.14: fight. + EffectKind::Fight => InteractionIntentCode::Fight, + // CR 701.3: attach (Auras and Equipment). + EffectKind::Attach | EffectKind::AttachAll | EffectKind::ReturnAsAura => { + InteractionIntentCode::Attach + } + // CR 707: copy. + EffectKind::CopySpell + | EffectKind::EpicCopy + | EffectKind::CastCopyOfCard + | EffectKind::CopyTokenOf + | EffectKind::BecomeCopy => InteractionIntentCode::Copy, + // CR 613.1b: control change. + EffectKind::GainControl + | EffectKind::GainControlAll + | EffectKind::ControlNextTurn + | EffectKind::GiveControl + | EffectKind::ExchangeControl => InteractionIntentCode::GainControl, + // CR 701.20: reveal. + EffectKind::Reveal | EffectKind::RevealUntil => InteractionIntentCode::Reveal, + // CR 406.1: exile. Only the kinds that name exile in the tag itself + // qualify. Plain "exile target creature" is `EffectKind::ChangeZone`, + // whose destination lives in the `Effect` payload and not in the unit + // tag, so it cannot be recognized here and stays neutral below — the + // same shape as the `Pump` sign problem. `effect_zone_intent` solves + // this for `EffectZoneChoice` only because that state stores the + // destination alongside the kind. + EffectKind::ExileHaunting | EffectKind::ExileTop | EffectKind::HeistExile => { + InteractionIntentCode::Exile + } + // Return to hand ("bounce"). Not a keyword action, so no CR citation: + // it is an ordinary zone change to the owner's hand. + EffectKind::Bounce | EffectKind::BounceAll => InteractionIntentCode::Return, + // CR 613.4: characteristic modification. Covers both directions — + // `EffectKind` is a unit tag, so `Pump` is the same variant for + // "+3/+3" and "-3/-3" (`Effect::Pump` carries the sign in `PtValue`, + // and that sign can be dynamic). `Modify` therefore names the action + // and claims no disposition; see the adapter's mapping for the + // consequence at the protocol boundary. + // CR 613.4: direction is read off `Effect::Pump`'s `PtValue` payload at + // slot construction, because `EffectKind` cannot carry it. `Modify` + // survives for the two populations where no direction is true: a + // dynamic magnitude (X / count-based) and a genuinely opposing + // modification ("+2/-2"). + EffectKind::Pump | EffectKind::PumpAll => match detail { + TargetEffectDetail::Modification(PtDirection::Increase) => InteractionIntentCode::Buff, + TargetEffectDetail::Modification(PtDirection::Decrease) => { + InteractionIntentCode::Debuff + } + TargetEffectDetail::None | TargetEffectDetail::Destination(_) => { + InteractionIntentCode::Modify + } + }, + EffectKind::SwitchPT + | EffectKind::DoublePT + | EffectKind::DoublePTAll + | EffectKind::PutCounter + | EffectKind::PutCounterAll + | EffectKind::PutChosenCounter + | EffectKind::RemoveCounter + | EffectKind::MultiplyCounter + | EffectKind::MoveCounters + | EffectKind::Animate => InteractionIntentCode::Modify, + // Everything else is either not reachable from a target slot or has no + // effect-semantic disposition (e.g. `NoOp`, which the mutate and other + // pipeline-resolved slots carry). Neutral by construction. + _ => InteractionIntentCode::Choose, + } +} + fn pay_cost_intent(kind: &PayCostKind) -> InteractionIntentCode { match kind { PayCostKind::Discard | PayCostKind::Reveal | PayCostKind::Behold { .. } => { @@ -6249,9 +6398,22 @@ fn opportunity_for_slot( filtered_state, ), }, - surfaces: vec![InteractionPresentationSurface::Summary { - code: InteractionSummaryCode::Decision, - }], + surfaces: vec![ + InteractionPresentationSurface::Summary { + code: InteractionSummaryCode::Decision, + }, + // CR 115.1: the opportunity-level label for what this + // announcement will do to whatever is chosen. The + // per-candidate identity surfaces are emitted by + // `target_sequence_choices`; what was missing here was + // the intent, so a consumer could not tell a kill spell + // from a pump spell. + InteractionPresentationSurface::Selection { + intent: projection.intent, + constraint: SelectionConstraint::Count { min, max }, + confirm: ConfirmSemantics::Explicit, + }, + ], progress: InteractionProgress { selected: 0, minimum: min, diff --git a/crates/engine/src/game/visibility.rs b/crates/engine/src/game/visibility.rs index e82649e0b0..5f553576f7 100644 --- a/crates/engine/src/game/visibility.rs +++ b/crates/engine/src/game/visibility.rs @@ -1803,6 +1803,7 @@ mod tests { continue_replacement, replace_event, replacement_choice_waiting_for, ReplacementResult, }; use crate::game::zones::create_object; + use crate::types::ability::EffectKind; use crate::types::ability::{ AbilityDefinition, AbilityKind, BeholdCostAction, CostPaidObjectSnapshot, Effect, ReplacementDefinition, ResolvedAbility, TargetFilter, @@ -1817,6 +1818,7 @@ mod tests { ManaAbilityResume, MayTriggerAutoChoiceKey, MayTriggerOrigin, PendingBeginGameAbility, PendingCast, PendingCostMoveCompletion, PendingCostMoveResume, PendingManaAbility, PendingScopedLibrarySearch, PendingSearchFoundBatch, PreparedScopedLibrarySearchChoice, + TargetEffectDetail, }; use crate::types::identifiers::CardId; use crate::types::mana::ManaCost; @@ -3947,6 +3949,8 @@ mod tests { legal_targets: vec![crate::types::ability::TargetRef::Object(ObjectId(20))], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(), diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index cd54153b31..75ee62bbed 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -6182,6 +6182,94 @@ pub struct TargetSelectionSlot { /// (CR 115.1) regardless of who announced a slot. #[serde(default, skip_serializing_if = "Option::is_none")] pub chooser: Option, + /// CR 115.1: The kind of effect that will affect this target — the game + /// fact of *what the spell or ability does to the thing being chosen*. + /// Stamped at slot construction from the enclosing ability frame's own + /// `Effect`, because the slot is otherwise attribution-free: nothing here + /// references the effect, so a consumer cannot recover it later without + /// re-walking the effect tree in lockstep with the slot builder. + /// + /// This is deliberately the game fact and NOT a presentation intent. + /// Labelling (e.g. "this prompt is hostile") is a projection-layer + /// decision made by `target_intent` in `game::interaction`, mirroring how + /// `WaitingFor::EffectZoneChoice` stores `effect_kind` and lets + /// `effect_zone_intent` label it. + pub effect_kind: EffectKind, + /// CR 115.1: The discriminating fact that `effect_kind` does not carry. + /// + /// `EffectKind` is a unit tag, so two effects that do opposite things to a + /// target can share one variant: `Effect::ChangeZone` is the same kind + /// whether it exiles or returns to hand, and `Effect::Pump` is the same + /// kind for "+3/+3" and "-3/-3". Both hold the deciding value in their + /// payload, which is in hand at slot construction and unrecoverable + /// afterwards. + /// + /// One sum type rather than one `Option` field per lossy kind: the axis + /// is "what extra fact does this kind need", and a per-kind field would be + /// the sibling-cluster smell at struct level. `From<&Effect> for + /// EffectKind` already reads payloads this way for `SetTapState`, so + /// payload discrimination at this boundary is established practice. + #[serde(default)] + pub effect_detail: TargetEffectDetail, +} + +/// CR 613.4: Direction of a power/toughness modification. Typed rather than a +/// signed number so the stored game fact says which way the change goes even +/// when the magnitude is irrelevant, mirroring [`TapStateChange`] for the +/// tap/untap axis. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "type")] +pub enum PtDirection { + /// CR 613.4: A modification that raises power and/or toughness. + Increase, + /// CR 613.4: A modification that lowers power and/or toughness. + Decrease, +} + +/// CR 115.1: Effect-kind-specific payload captured alongside a target slot's +/// [`EffectKind`], for the kinds whose unit tag is ambiguous about what will +/// happen to the chosen target. +/// +/// Deliberately a *game fact* and not a presentation intent — a [`Zone`] and a +/// direction, both read straight off the effect. `target_intent` in +/// `game::interaction` decides how to label them. +/// +/// # When a new variant is justified +/// +/// This enum is a deliberate concession, and a sum type accretes one plausible +/// variant at a time just as easily as a struct grows `Option` fields. A new +/// variant must satisfy **all three** of: +/// +/// 1. It carries a fact the [`EffectKind`] unit tag genuinely **cannot** +/// express — not merely one it happens not to today. +/// 2. Its kind actually **reaches a CR 115.1 target announcement**. An effect +/// that never produces a target slot needs nothing here. +/// 3. The fact is **available at slot construction**, in +/// `collect_target_slots`. Anything recoverable later belongs at the +/// projection layer instead, and anything unknowable at announcement +/// belongs in [`TargetEffectDetail::None`] rather than being guessed. +/// +/// If a proposal fails **any** of the three, the answer is a finer +/// [`EffectKind`] fan-out — see `impl From<&Effect> for EffectKind`'s +/// `Effect::SetTapState` arm, which reads `scope` and `state` to produce four +/// distinct kinds — and **not** a new detail variant. Prefer that route +/// whenever the distinction is intrinsic to the effect rather than to this +/// one announcement. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(tag = "type")] +pub enum TargetEffectDetail { + /// The kind is self-describing; no extra fact is needed (the default, and + /// the honest answer whenever the deciding value is not statically known). + #[default] + None, + /// Destination of a zone-change effect (`ChangeZone`, `ChangeZoneAll`). + /// Consumed by reusing the existing `effect_zone_intent(kind, destination)` + /// rather than reimplementing zone labelling. + Destination(Zone), + /// CR 613.4: Direction of a P/T modification (`Pump`, `PumpAll`). Absent + /// when the modification is dynamic (X or count-based) or genuinely + /// opposing ("+2/-2"), because then no direction is true. + Modification(PtDirection), } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] @@ -21143,6 +21231,8 @@ mod tests { legal_targets: vec![TargetRef::Object(ObjectId(1))], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: vec![], @@ -21557,6 +21647,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: vec![], diff --git a/crates/engine/src/types/interaction.rs b/crates/engine/src/types/interaction.rs index bdc7a6f32d..e0d6f4ecbe 100644 --- a/crates/engine/src/types/interaction.rs +++ b/crates/engine/src/types/interaction.rs @@ -184,6 +184,55 @@ pub enum InteractionIntentCode { Pay, Attack, Block, + // CR 115.1 targeting vocabulary. Each of these names a distinct game + // action in its own CR section, so they stay flat siblings rather than one + // parameterized variant: unifying e.g. Destroy (CR 701.8), Counter + // (CR 701.6) and Mill (CR 701.17) under a single code with a "which + // action" axis would conflate rule sections the engine resolves + // separately, which the workspace categorical-boundary rule forbids. + /// CR 120.1: damage dealt to the chosen target. + Damage, + /// CR 701.8: destroy the chosen permanent. + Destroy, + /// CR 701.19: put a regeneration shield on the chosen permanent. + Regenerate, + /// CR 701.6: counter the chosen spell. + Counter, + /// CR 701.26: untap the chosen permanent. + Untap, + /// CR 701.17: mill from the chosen player's library. + Mill, + /// CR 701.9: the chosen player discards. + Discard, + /// CR 121.1: the chosen player draws. + Draw, + /// CR 119.3: the chosen player gains life. + GainLife, + /// CR 119.3: the chosen player loses life. + LoseLife, + /// CR 701.14: the chosen creature fights. + Fight, + /// CR 701.3: attach to the chosen permanent. + Attach, + /// CR 707: copy the chosen object. + Copy, + /// CR 613.1b: take control of the chosen permanent. + GainControl, + /// CR 701.20: reveal the chosen card. + Reveal, + /// CR 613.4: change the chosen object's characteristics (power/toughness, + /// counters, types) with NO claim about direction. Used when no single + /// direction is true — a dynamic magnitude (X / count-based) or a genuinely + /// opposing modification such as "+2/-2". + Modify, + /// CR 613.4: a modification that raises the chosen object's power and/or + /// toughness. Split from `Modify` because `TargetSelectionSlot` stamps the + /// direction read off the effect payload at construction; the unit + /// `EffectKind` tag alone cannot distinguish these three. + Buff, + /// CR 613.4: a modification that lowers the chosen object's power and/or + /// toughness. + Debuff, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] diff --git a/crates/manabrew-compat/src/lib.rs b/crates/manabrew-compat/src/lib.rs index 0056b3d67c..283fda182c 100644 --- a/crates/manabrew-compat/src/lib.rs +++ b/crates/manabrew-compat/src/lib.rs @@ -27,9 +27,10 @@ use engine::types::game_state::{ ShardChoice, StackEntryKind, WaitingFor, }; use engine::types::interaction::{ - InteractionChoice, InteractionOpportunity, InteractionOpportunityResponse, - InteractionPresentationSurface, InteractionResponse, InteractionResponseSpec, - InteractionRoleCode, InteractionSubmission, SelectionConstraint, ViewerInteraction, + InteractionChoice, InteractionIntentCode, InteractionOpportunity, + InteractionOpportunityResponse, InteractionPresentationSurface, InteractionResponse, + InteractionResponseSpec, InteractionRoleCode, InteractionSubmission, SelectionConstraint, + ViewerInteraction, }; use engine::types::mana::{ManaColor as EngineManaColor, ManaCost, ManaCostShard, ManaType}; use engine::types::phase::Phase; @@ -1409,10 +1410,10 @@ static UNSUPPORTED_PROTOCOL_CAPABILITIES: [UnsupportedCapability; 83] = [ suggested_protocol_extension: "None needed upstream for reveals — the prompt is already answerable, and raising it from labels to cards is local work: either a bespoke ChooseCards arm here (as DiscardChoice has, with its matching gate entry) or an engine projection that classifies the reveal as a card selection. For DiceRolled, treat it as a display event with audience and sequencing metadata rather than a prompt, since no engine pause backs it.", }, UnsupportedCapability { - code: "local.targeting-intent-unsourceable", + code: "local.targeting-intent-neutral-inexpressible", area: "prompts", - reason: "Fidelity, not coverage: target prompts are fully answerable, but ChooseBoardTargetsInput.intent is a placeholder rather than a computed value, so a client must not read it. The field is required (no serde default) and all 25 TargetingIntent variants are effect semantics — Damage, Destroy, Sacrifice, Exile, Bounce, Mill, Discard, Counter, Tap, Untap, Copy, Buff, Debuff, Heal, LoseLife, Reveal, Draw, Fetch, GainControl, Fight, Attach, Attack, Block, Hostile, Friendly — with no neutral or unknown member, so every ChooseBoardTargets must claim one. Phase has none to give. Both producing states (WaitingFor::TargetSelection and TriggerTargetSelection) project as TargetSequenceProjection (game/interaction.rs), whose only descriptor is TargetSequenceAction — ChooseTarget | SelectObjects | SelectTargets | Retarget — which is the MECHANISM of the pick, not what the spell does to what it picks. The engine's one intent vocabulary, InteractionIntentCode (14 variants: Choose, Keep, Sacrifice, Return, Exile, Tap, Crew, Saddle, Station, RingBearer, Blight, Pay, Attack, Block), hangs off SelectionProjection for non-targeting Select states and likewise names the action taken on the objects, not a disposition toward a CR 115.1 target; nothing equivalent exists on the targeting path. Deriving intent from the pending spell's effect would be the adapter inventing game semantics, which the thin-boundary rule forbids. Consequence, stated plainly rather than hidden: build_prompt_input has exactly one ChooseBoardTargets construction site, so EVERY target prompt is advertised as Hostile — including targeting your own creature to buff or regenerate it — and the emitted hostile: false contradicts it. The asymmetry is visible inside this same DTO: TargetRefDto.intent is Option, and target_ref_dto declines it with None for every candidate. The adapter already refuses to guess wherever refusing is expressible; the prompt-level field is the one place it cannot.", - suggested_protocol_extension: "Preferred: make ChooseBoardTargetsInput.intent an Option, exactly matching TargetRefDto.intent one field away. This asks for no new concept — the protocol already models a declinable targeting intent and already spells it `#[serde(default, skip_serializing_if = \"Option::is_none\")] Option`; the prompt-level field is simply inconsistent with it. Adding a neutral/unknown variant is the weaker fallback: it would let an engine say nothing, but it grows the semantic enum to encode absence, which the type system already expresses. Align hostile's optionality with whichever is chosen, since the two fields can currently contradict each other. Either shape is breaking for older readers (the transport envelope sets deny_unknown_fields and no family enum is non_exhaustive), so it belongs in a major bump.", + reason: "Fidelity, narrowed twice. ChooseBoardTargetsInput.intent is now COMPUTED, not a placeholder. TargetSelectionSlot carries the announcing ability's EffectKind AND the discriminating payload that the unit tag cannot hold (TargetEffectDetail: a zone-change Destination or a P/T Modification direction), both stamped per CR 601.2c announcement frame by collect_target_slots in game/ability_utils.rs; game/interaction.rs projects the pair with target_intent (delegating to the pre-existing effect_zone_intent for the zone family) into InteractionIntentCode; this adapter renames that answer with targeting_intent_dto. hostile is derived from the same value, so the two fields no longer contradict each other. The earlier claim that Phase cannot source targeting intent was WRONG and has been retracted. Measured against data/card-data.json: of 21,077 targeting links in the card corpus, 61.4% resolved from EffectKind alone, and stamping the destination lifts that to roughly 70% by resolving 1,016 exiles and 865 bounces that previously shared one ChangeZone tag. What survives is a protocol-shape gap, not an engine gap: TargetingIntent's 25 variants (Damage, Destroy, Sacrifice, Exile, Bounce, Mill, Discard, Counter, Tap, Untap, Copy, Buff, Debuff, Heal, LoseLife, Reveal, Draw, Fetch, GainControl, Fight, Attach, Attack, Block, Hostile, Friendly) contain NO neutral or unknown member, and the field is required (no serde default), so a choice with no effect-semantic disposition must still claim one. Two populations land here. (1) Genuinely neutral picks, which the engine projects as InteractionIntentCode::Choose - about 30% of targeting links, led by GenericEffect (1,560), Shuffle (871) and TargetOnly (424), plus mutate targets (CR 702.140a), which the casting pipeline resolves without an Effect at all and which therefore carry EffectKind::NoOp. These fall back to Hostile. That is a LEAST-WRONG choice and NOT a safe one: an unlabelled pick still reads as hostile, which is the residue of the original defect rather than a fix for it, and a client must not read Hostile as an assertion. (2) Modifications whose direction is genuinely unknowable - a dynamic X or count-based magnitude (193 links) or a genuinely opposing '+2/-2' (83 links), together about 16% of the 1,679 targeted pumps. These project as InteractionIntentCode::Modify and resolve to Debuff, the adverse member, on an asymmetric-loss argument: Buff would be right more often, since pump is more often positive, but it is the one direction whose error is unrecoverable - it would mark 'target creature gets -4/-4' as harmless, while a caution affordance on a genuine combat trick is recoverable by the player. The other 84% of pumps (1,079 buff, 324 debuff) now resolve to their true direction and do not reach this arm. Exhibited by a_damage_target_prompt_and_a_regenerate_target_prompt_carry_opposite_intents, an_unsigned_pump_target_prompt_fails_cautious_as_debuff, a_signed_pump_resolves_to_its_actual_direction and a_zone_change_target_resolves_by_destination rather than asserted here.", + suggested_protocol_extension: "Preferred, unchanged in shape and now much narrower in scope: make ChooseBoardTargetsInput.intent an Option, matching TargetRefDto.intent one field away - the protocol already models a declinable targeting intent and already spells it `#[serde(default, skip_serializing_if = \"Option::is_none\")] Option`. That would let population (1) decline rather than fall back to Hostile, which is the single highest-value change upstream could make here. Population (2) is better served by a direction-neutral modification member (e.g. Modify alongside Buff/Debuff) so an engine that knows the action but not the direction can say exactly that instead of guessing. Align hostile's optionality with whichever is chosen. Either shape is breaking for older readers (the transport envelope sets deny_unknown_fields and no family enum is non_exhaustive), so it belongs in a major bump.", }, UnsupportedCapability { code: "local.library-arrangement-reorder-unsupported", @@ -2134,6 +2135,7 @@ fn build_prompt_input( waiting_for_type: waiting_for_type(waiting_for), code: "local.target-slot-missing", })?; + let intent = targeting_intent_dto(projected_target_intent(prepared)); Ok(PromptInput::ChooseBoardTargets(ChooseBoardTargetsInput { // v2 removed the flat `label`; the slot's mode label is the // presentation title. @@ -2144,32 +2146,21 @@ fn build_prompt_input( .unwrap_or_else(|| "Choose target".to_string()), ), candidates: target_refs(&slot.legal_targets), - // PLACEHOLDERS, not computed values — declared as - // `local.targeting-intent-unsourceable`. + // COMPUTED, not placeholders. The engine stamps the announcing + // ability's `EffectKind` on each target slot (CR 601.2c: one + // slot per announced target) and `game::interaction`'s + // `target_intent` projects it to an `InteractionIntentCode`; + // this adapter only renames that answer into the wire + // vocabulary. `hostile` is derived from the same value, so the + // two fields can no longer contradict each other. // - // `intent` is a required field and every one of - // `TargetingIntent`'s 25 variants is effect semantics (Damage, - // Destroy, Buff, Heal, ...); none means "unknown". Phase projects - // no effect-semantic intent for a CR 115.1 target choice — the - // engine's `TargetSequenceProjection` describes only the - // MECHANISM of the pick (`TargetSequenceAction`: ChooseTarget | - // SelectObjects | SelectTargets | Retarget) — and deriving one - // from the pending spell's effect would be this adapter - // inventing game semantics. So the field is filled rather than - // computed, and there is no value that would make it honest. - // Contrast the candidates just above: `TargetRefDto.intent` is - // an `Option`, and `target_ref_dto` declines it with `None`. - // That is what this field would do if the protocol let it. - // - // The two fields read as contradictory, and neither is - // authoritative: `hostile` is `#[serde(default)]`, so leaving it - // false asserts nothing, while `intent` must name a variant and - // only `Hostile`/`Friendly` name a disposition rather than a - // specific action. A client must not read either as engine-derived - // — in particular, targeting your own creature to buff or - // regenerate it is advertised here exactly as a kill spell is. - hostile: false, - intent: TargetingIntent::Hostile, + // Two residues remain, both declared as + // `local.targeting-intent-neutral-inexpressible`: a genuinely + // neutral pick and an unsigned P/T modification have no honest + // `TargetingIntent`, because the protocol's 25 variants contain + // no neutral member. See `targeting_intent_dto`. + hostile: targeting_is_hostile(intent), + intent, min_targets: if slot.optional { 0 } else { 1 }, max_targets: 1, chosen_targets: 0, @@ -4154,6 +4145,143 @@ fn target_refs(targets: &[TargetRef]) -> Vec { targets.iter().filter_map(target_ref_dto).collect() } +/// The engine's projected intent for the viewer's single open target +/// opportunity, or `Choose` if there is none. +/// +/// Read from the interaction projection rather than from the slot's +/// `effect_kind` directly: mapping an `EffectKind` to a disposition is game +/// semantics, which belongs in `engine::game::interaction::target_intent`, not +/// in this adapter. The adapter's job is only to rename the engine's answer +/// into the wire vocabulary. +fn projected_target_intent(prepared: &PreparedManabrewSnapshot) -> InteractionIntentCode { + prepared + .interaction + .opportunities + .iter() + .find_map(|opportunity| { + opportunity.surfaces.iter().find_map(|surface| { + if let InteractionPresentationSurface::Selection { intent, .. } = surface { + Some(*intent) + } else { + None + } + }) + }) + .unwrap_or(InteractionIntentCode::Choose) +} + +/// Rename an engine intent into the wire's `TargetingIntent` vocabulary. +/// +/// Every arm below is a pure rename of a value the engine computed. The two +/// lossy arms are called out because the protocol forces a choice the engine +/// deliberately does not make: +/// +/// * `Modify` — a P/T change whose direction is genuinely not knowable at +/// announcement (a dynamic X / count-based magnitude, or an opposing +/// "+2/-2"). `TargetingIntent` has only `Buff` and `Debuff`, so this resolves +/// to the ADVERSE member on the asymmetric-loss argument the arm below +/// states. Declared as `local.targeting-intent-neutral-inexpressible`. +/// Directional modifications do NOT arrive here — the engine resolves those +/// to `Buff`/`Debuff` from the direction stamped on the slot at +/// construction, so this arm now serves only the ~16% of targeted pumps +/// whose direction is genuinely unknowable. +/// * `Choose` — a genuinely neutral pick. `TargetingIntent` has no neutral +/// member, so this falls back to `Hostile`. That is a LEAST-WRONG choice and +/// not a safe one: an unlabelled pick still reads as hostile, which is the +/// residue of the original defect rather than a fix for it. Same declaration. +fn targeting_intent_dto(intent: InteractionIntentCode) -> TargetingIntent { + match intent { + InteractionIntentCode::Damage => TargetingIntent::Damage, + InteractionIntentCode::Destroy => TargetingIntent::Destroy, + InteractionIntentCode::Sacrifice => TargetingIntent::Sacrifice, + InteractionIntentCode::Exile => TargetingIntent::Exile, + InteractionIntentCode::Return => TargetingIntent::Bounce, + InteractionIntentCode::Mill => TargetingIntent::Mill, + InteractionIntentCode::Discard => TargetingIntent::Discard, + InteractionIntentCode::Counter => TargetingIntent::Counter, + InteractionIntentCode::Tap => TargetingIntent::Tap, + InteractionIntentCode::Untap => TargetingIntent::Untap, + InteractionIntentCode::Copy => TargetingIntent::Copy, + InteractionIntentCode::GainLife => TargetingIntent::Heal, + InteractionIntentCode::LoseLife => TargetingIntent::LoseLife, + InteractionIntentCode::Reveal => TargetingIntent::Reveal, + InteractionIntentCode::Draw => TargetingIntent::Draw, + InteractionIntentCode::GainControl => TargetingIntent::GainControl, + InteractionIntentCode::Fight => TargetingIntent::Fight, + InteractionIntentCode::Attach => TargetingIntent::Attach, + InteractionIntentCode::Attack => TargetingIntent::Attack, + InteractionIntentCode::Block => TargetingIntent::Block, + // CR 701.19: a regeneration shield only ever helps its target, and + // `Friendly` is the protocol's disposition member for exactly that. + InteractionIntentCode::Regenerate => TargetingIntent::Friendly, + // Lossy, declared: see the doc comment above. `EffectKind` is a unit tag + // and `Effect::Pump` is the same variant for "+3/+3" and "-4/-4" (the + // sign lives in `PtValue` and may be dynamic), so this bucket cannot + // tell a combat trick from removal. It resolves to the ADVERSE member + // for the same asymmetric-loss reason `targeting_is_hostile` gives for + // the neutral bucket: a caution affordance on a genuine buff is + // recoverable, whereas marking "target creature gets -4/-4" as harmless + // is not. Mapping to `Buff` would be the more frequently correct guess + // and the one unrecoverable kind of wrong. + InteractionIntentCode::Modify => TargetingIntent::Debuff, + // CR 613.4: direction IS known for these — read off `Effect::Pump`'s + // `PtValue` payload at slot construction — so they are exact renames, + // not guesses. This is what shrinks the lossy arm above from the whole + // pump family to just its unknowable tail: 1,079 buff and 324 debuff + // targeted links in the card corpus now resolve correctly, where + // before every one of them took a single guess. + InteractionIntentCode::Buff => TargetingIntent::Buff, + InteractionIntentCode::Debuff => TargetingIntent::Debuff, + // These never reach a CR 115.1 target announcement — they belong to the + // board-selection and cost-payment models — but the match stays + // exhaustive so a new intent code cannot silently fall into `Hostile`. + InteractionIntentCode::Choose + | InteractionIntentCode::Keep + | InteractionIntentCode::Crew + | InteractionIntentCode::Saddle + | InteractionIntentCode::Station + | InteractionIntentCode::RingBearer + | InteractionIntentCode::Blight + | InteractionIntentCode::Pay => TargetingIntent::Hostile, + } +} + +/// CR 115.1: whether the announcement is adverse to the thing being chosen. +/// +/// Derived from the same engine intent that fills `intent`, so the two fields +/// agree instead of contradicting each other. The neutral bucket resolves to +/// `true` only because `Choose` was already renamed to `Hostile` one step +/// earlier — a least-wrong protocol fallback, NOT a safety property. +fn targeting_is_hostile(intent: TargetingIntent) -> bool { + match intent { + TargetingIntent::Damage + | TargetingIntent::Destroy + | TargetingIntent::Sacrifice + | TargetingIntent::Exile + | TargetingIntent::Bounce + | TargetingIntent::Mill + | TargetingIntent::Discard + | TargetingIntent::Counter + | TargetingIntent::Tap + | TargetingIntent::Debuff + | TargetingIntent::LoseLife + | TargetingIntent::GainControl + | TargetingIntent::Fight + | TargetingIntent::Attack + | TargetingIntent::Block + | TargetingIntent::Hostile => true, + TargetingIntent::Untap + | TargetingIntent::Copy + | TargetingIntent::Buff + | TargetingIntent::Heal + | TargetingIntent::Reveal + | TargetingIntent::Draw + | TargetingIntent::Fetch + | TargetingIntent::Attach + | TargetingIntent::Friendly => false, + } +} + fn combat_assignments(state: &GameState) -> Vec { state .combat @@ -5165,12 +5293,12 @@ mod tests { use engine::game::interaction::bind_interaction_authority; use engine::game::zones::create_object; - use engine::types::ability::{Effect, ResolvedAbility, TargetFilter}; + use engine::types::ability::{Effect, EffectKind, ResolvedAbility, TargetFilter}; use engine::types::counter::CounterType; use engine::types::game_state::{ MulliganDecisionEntry, MulliganDecisionPhase, OutsideGameChoiceEntry, - OutsideGameChoiceSource, PayableResource, PendingCast, PendingMulliganAction, - TargetSelectionProgress, TargetSelectionSlot, + OutsideGameChoiceSource, PayableResource, PendingCast, PendingMulliganAction, PtDirection, + TargetEffectDetail, TargetSelectionProgress, TargetSelectionSlot, }; use engine::types::identifiers::CardId; use engine::types::interaction::InteractionSessionId; @@ -5751,6 +5879,8 @@ mod tests { legal_targets: vec![TargetRef::Player(PlayerId(1))], optional: false, chooser: None, + effect_kind: EffectKind::DealDamage, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(), @@ -5786,6 +5916,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::DealDamage, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: TargetSelectionProgress::default(), @@ -5803,6 +5935,194 @@ mod tests { assert_eq!(json["input"]["presentation"]["title"], "Choose target"); } + /// Build a `TargetSelection` board-target prompt whose single slot carries + /// `effect_kind`, driving the real engine projection + /// (`derive_viewer_interaction` -> `target_intent`) and the real adapter + /// mapping. Returns the serialized prompt. + fn board_target_prompt_for(effect_kind: EffectKind) -> serde_json::Value { + board_target_prompt_detailed(effect_kind, TargetEffectDetail::None) + } + + /// As above, with the discriminating payload the effect kind cannot carry. + fn board_target_prompt_detailed( + effect_kind: EffectKind, + effect_detail: TargetEffectDetail, + ) -> serde_json::Value { + let mut state = GameState::new_two_player(7); + let target = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Bear".to_string(), + Zone::Battlefield, + ); + let legal = vec![TargetRef::Object(target)]; + state.waiting_for = WaitingFor::TargetSelection { + player: PlayerId(0), + pending_cast: dummy_pending_cast(), + target_slots: vec![TargetSelectionSlot { + legal_targets: legal.clone(), + optional: false, + chooser: None, + effect_kind, + effect_detail, + }], + mode_labels: Vec::new(), + selection: TargetSelectionProgress { + current_slot: 0, + selected_slots: Vec::new(), + current_legal_targets: legal, + }, + }; + // The intent rides the engine's interaction projection, which only + // produces opportunities once authority is bound. Without this the + // adapter sees no opportunity and falls back to neutral, which would + // make the assertions below pass for the wrong reason. + bind_interaction_authority(&mut state, InteractionSessionId("intent".to_string())) + .expect("valid interaction authority binding"); + let prepared = prepare_snapshot_with_prompt_id(&state, PlayerId(0), "game-a", 42).unwrap(); + assert!( + !prepared.interaction.opportunities.is_empty(), + "reach guard: the viewer must actually own an open target opportunity, \ + otherwise the intent assertions below are vacuous" + ); + serde_json::to_value(build_prompt(&prepared, &lookup).unwrap()).unwrap() + } + + /// CR 115.1: the defect this fixes — every target prompt used to be + /// advertised as `Hostile` with a contradicting `hostile: false`, so + /// targeting your own creature to regenerate it looked exactly like a kill + /// spell. + /// + /// Both halves of each pair flip if the derivation is reverted to the old + /// hardcoded `intent: TargetingIntent::Hostile, hostile: false`. + #[test] + fn a_damage_target_prompt_and_a_regenerate_target_prompt_carry_opposite_intents() { + // CR 120.1: damage is adverse to whatever is chosen. + let damage = board_target_prompt_for(EffectKind::DealDamage); + assert_eq!(damage["input"]["type"], "chooseBoardTargets"); + assert_eq!(damage["input"]["intent"], "damage"); + assert_eq!(damage["input"]["hostile"], true); + + // CR 701.19: a regeneration shield only ever helps its target. + let regenerate = board_target_prompt_for(EffectKind::Regenerate); + assert_eq!(regenerate["input"]["intent"], "friendly"); + assert_eq!(regenerate["input"]["hostile"], false); + + assert_ne!( + damage["input"]["intent"], regenerate["input"]["intent"], + "a kill spell and a protective spell must not advertise the same intent" + ); + } + + /// The declared residue of `local.targeting-intent-neutral-inexpressible`. + /// + /// `EffectKind` is a unit tag, so `Effect::Pump` is the same variant for + /// "+3/+3" and "-3/-3"; the engine honestly projects `Modify`, and the + /// protocol — which has only `Buff`/`Debuff` and no neutral member — forces + /// a guess. It resolves to `Debuff`, the ADVERSE member, for the same + /// asymmetric-loss reason the neutral bucket does: `Buff` would be right + /// more often, since pump is more often positive, but it is the direction + /// whose error cannot be recovered — it marks "target creature gets -4/-4" + /// as harmless. Pinning it here means the lossy step is visible rather than + /// discovered later by a client. + #[test] + fn an_unsigned_pump_target_prompt_fails_cautious_as_debuff() { + let pump = board_target_prompt_for(EffectKind::Pump); + assert_eq!(pump["input"]["intent"], "debuff"); + assert_eq!( + pump["input"]["hostile"], true, + "an unsigned modification must not claim to be harmless" + ); + + // The pair that makes "every prompt is Hostile" impossible to + // reintroduce: an unsigned modification and a burn spell are both + // adverse, so `hostile` agrees — but they must still be distinguishable, + // which is what a constant `intent` would destroy. + let damage = board_target_prompt_for(EffectKind::DealDamage); + assert_ne!(pump["input"]["intent"], damage["input"]["intent"]); + + // A genuinely neutral pick (mutate carries `NoOp` — no `Effect` backs + // it) has no honest protocol value at all. It resolves to `Hostile`, + // which is a least-wrong fallback and not a fix: an unlabelled pick + // still reads as hostile. + let neutral = board_target_prompt_for(EffectKind::NoOp); + assert_eq!(neutral["input"]["intent"], "hostile"); + assert_eq!(neutral["input"]["hostile"], true); + } + + /// CR 613.4: a signed modification resolves to its true direction. + /// + /// This is the payoff of stamping the discriminating payload alongside the + /// kind. Before it, every `Effect::Pump` — 1,679 targeted links in the card + /// corpus — took one guess; 324 of them were targeted DEBUFFS being shown + /// under a single label with 1,079 buffs. Both halves below flip to the + /// lossy `Modify` arm if `TargetEffectDetail::Modification` is dropped. + #[test] + fn a_signed_pump_resolves_to_its_actual_direction() { + let buff = board_target_prompt_detailed( + EffectKind::Pump, + TargetEffectDetail::Modification(PtDirection::Increase), + ); + assert_eq!(buff["input"]["intent"], "buff"); + assert_eq!( + buff["input"]["hostile"], false, + "a combat trick on your own creature is not adverse" + ); + + let debuff = board_target_prompt_detailed( + EffectKind::Pump, + TargetEffectDetail::Modification(PtDirection::Decrease), + ); + assert_eq!(debuff["input"]["intent"], "debuff"); + assert_eq!(debuff["input"]["hostile"], true); + + assert_ne!( + buff["input"]["intent"], debuff["input"]["intent"], + "+3/+3 and -3/-3 share one EffectKind; only the stamped direction \ + separates them" + ); + // And the unknowable tail still declines to claim a direction. + let unsigned = board_target_prompt_for(EffectKind::Pump); + assert_eq!(unsigned["input"]["intent"], "debuff"); + } + + /// CR 115.1: a zone-change target resolves by DESTINATION, reusing the + /// engine's existing `effect_zone_intent` rather than a second labeller. + /// + /// `EffectKind::ChangeZone` is the single largest targeting family (2,828 + /// links) and its tag says only "a zone change happened". Exile and + /// return-to-hand are opposite dispositions under one kind. + #[test] + fn a_zone_change_target_resolves_by_destination() { + let exile = board_target_prompt_detailed( + EffectKind::ChangeZone, + TargetEffectDetail::Destination(Zone::Exile), + ); + assert_eq!(exile["input"]["intent"], "exile"); + assert_eq!(exile["input"]["hostile"], true); + + let bounce = board_target_prompt_detailed( + EffectKind::ChangeZone, + TargetEffectDetail::Destination(Zone::Hand), + ); + assert_eq!(bounce["input"]["intent"], "bounce"); + + assert_ne!( + exile["input"]["intent"], bounce["input"]["intent"], + "both are EffectKind::ChangeZone; only the stamped destination \ + separates them" + ); + + // Destinations `effect_zone_intent` deliberately leaves unlabelled stay + // neutral rather than inventing a disposition. + let battlefield = board_target_prompt_detailed( + EffectKind::ChangeZone, + TargetEffectDetail::Destination(Zone::Battlefield), + ); + assert_eq!(battlefield["input"]["intent"], "hostile"); + } + /// Grounds the capability registry in behaviour rather than prose. /// /// Every claim of the form "X has no exact upstream shape" is falsifiable diff --git a/crates/phase-ai/src/policies/anti_self_harm.rs b/crates/phase-ai/src/policies/anti_self_harm.rs index 7cc782ee40..032f6c4f23 100644 --- a/crates/phase-ai/src/policies/anti_self_harm.rs +++ b/crates/phase-ai/src/policies/anti_self_harm.rs @@ -1051,12 +1051,12 @@ mod tests { use engine::game::zones::create_object; use engine::types::ability::{ AbilityCost, AbilityDefinition, AbilityKind, BounceSelection, CardSelectionMode, - ContinuousModification, ControllerRef, DiscardSelfScope, FilterProp, PtValue, QuantityRef, - ReplacementDefinition, ResolvedAbility, SacrificeCost, StaticDefinition, TargetFilter, - TriggerDefinition, TypeFilter, TypedFilter, + ContinuousModification, ControllerRef, DiscardSelfScope, EffectKind, FilterProp, PtValue, + QuantityRef, ReplacementDefinition, ResolvedAbility, SacrificeCost, StaticDefinition, + TargetFilter, TriggerDefinition, TypeFilter, TypedFilter, }; use engine::types::game_state::{ - CastingVariant, GameState, PendingCast, TargetSelectionSlot, WaitingFor, + CastingVariant, GameState, PendingCast, TargetEffectDetail, TargetSelectionSlot, WaitingFor, }; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::keywords::Keyword; @@ -1124,6 +1124,8 @@ mod tests { legal_targets, optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -1767,6 +1769,8 @@ mod tests { legal_targets: legal_targets.clone(), optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -1853,6 +1857,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -2723,6 +2729,8 @@ mod tests { legal_targets: vec![TargetRef::Object(target_id)], optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -2992,6 +3000,8 @@ mod tests { legal_targets, optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -3333,6 +3343,8 @@ mod tests { legal_targets: legal_targets.clone(), optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(), @@ -3518,6 +3530,8 @@ mod tests { legal_targets, optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), diff --git a/crates/phase-ai/src/policies/combat_withdrawal.rs b/crates/phase-ai/src/policies/combat_withdrawal.rs index 8ad0f77eea..3c5a7c127c 100644 --- a/crates/phase-ai/src/policies/combat_withdrawal.rs +++ b/crates/phase-ai/src/policies/combat_withdrawal.rs @@ -127,12 +127,12 @@ mod tests { use engine::game::combat::{AttackTarget, AttackerInfo, CombatState}; use engine::game::zones::create_object; use engine::types::ability::{ - AbilityDefinition, AbilityKind, Effect, PtValue, ResolvedAbility, TargetFilter, TargetRef, - TypedFilter, + AbilityDefinition, AbilityKind, Effect, EffectKind, PtValue, ResolvedAbility, TargetFilter, + TargetRef, TypedFilter, }; use engine::types::card_type::CoreType; use engine::types::game_state::{ - PendingCast, TargetSelectionProgress, TargetSelectionSlot, WaitingFor, + PendingCast, TargetEffectDetail, TargetSelectionProgress, TargetSelectionSlot, WaitingFor, }; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::mana::ManaCost; @@ -238,6 +238,8 @@ mod tests { legal_targets: legal_targets.clone(), optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: TargetSelectionProgress { diff --git a/crates/phase-ai/src/policies/context.rs b/crates/phase-ai/src/policies/context.rs index 389b9893aa..28d534265b 100644 --- a/crates/phase-ai/src/policies/context.rs +++ b/crates/phase-ai/src/policies/context.rs @@ -252,9 +252,9 @@ mod tests { use engine::ai_support::{ActionMetadata, TacticalClass}; use engine::game::zones::create_object; use engine::types::ability::{ - AbilityDefinition, AbilityKind, PtValue, QuantityExpr, TargetFilter, + AbilityDefinition, AbilityKind, EffectKind, PtValue, QuantityExpr, TargetFilter, }; - use engine::types::game_state::{PendingCast, TargetSelectionSlot}; + use engine::types::game_state::{PendingCast, TargetEffectDetail, TargetSelectionSlot}; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::mana::ManaCost; use engine::types::zones::Zone; @@ -283,6 +283,8 @@ mod tests { legal_targets: vec![], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -346,6 +348,8 @@ mod tests { legal_targets: vec![], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -560,6 +564,8 @@ mod tests { legal_targets: vec![], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -609,6 +615,8 @@ mod tests { legal_targets: vec![], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -659,6 +667,8 @@ mod tests { legal_targets: vec![], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), diff --git a/crates/phase-ai/src/policies/evasion_removal_priority.rs b/crates/phase-ai/src/policies/evasion_removal_priority.rs index d91d3afac3..676438a205 100644 --- a/crates/phase-ai/src/policies/evasion_removal_priority.rs +++ b/crates/phase-ai/src/policies/evasion_removal_priority.rs @@ -220,8 +220,8 @@ mod tests { }; use engine::types::format::FormatConfig; use engine::types::game_state::{ - CastPaymentMode, CopyTargetSlot, GameState, PendingCast, TargetSelectionProgress, - TargetSelectionSlot, WaitingFor, + CastPaymentMode, CopyTargetSlot, GameState, PendingCast, TargetEffectDetail, + TargetSelectionProgress, TargetSelectionSlot, WaitingFor, }; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::keywords::Keyword; @@ -635,6 +635,8 @@ mod tests { legal_targets: vec![TargetRef::Object(low), TargetRef::Object(high)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }; let trigger = AiDecisionContext { waiting_for: WaitingFor::TriggerTargetSelection { @@ -708,6 +710,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: TargetSelectionProgress::default(), @@ -774,6 +778,8 @@ mod tests { legal_targets: vec![TargetRef::Object(flyer)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -840,6 +846,8 @@ mod tests { legal_targets: vec![TargetRef::Object(ground_opp)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), diff --git a/crates/phase-ai/src/policies/hand_disruption.rs b/crates/phase-ai/src/policies/hand_disruption.rs index 64bc48883c..974102c77b 100644 --- a/crates/phase-ai/src/policies/hand_disruption.rs +++ b/crates/phase-ai/src/policies/hand_disruption.rs @@ -257,11 +257,13 @@ mod tests { use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, TacticalClass}; use engine::game::zones::create_object; use engine::types::ability::{ - AbilityDefinition, AbilityKind, ControllerRef, Effect, ResolvedAbility, TargetFilter, - TargetRef, TypeFilter, TypedFilter, + AbilityDefinition, AbilityKind, ControllerRef, Effect, EffectKind, ResolvedAbility, + TargetFilter, TargetRef, TypeFilter, TypedFilter, }; use engine::types::format::FormatConfig; - use engine::types::game_state::{GameState, PendingCast, TargetSelectionSlot, WaitingFor}; + use engine::types::game_state::{ + GameState, PendingCast, TargetEffectDetail, TargetSelectionSlot, WaitingFor, + }; use engine::types::identifiers::CardId; use engine::types::mana::ManaCost; use engine::types::player::PlayerId; @@ -442,6 +444,8 @@ mod tests { legal_targets: legal_targets.clone(), optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -569,6 +573,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -659,6 +665,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), diff --git a/crates/phase-ai/src/policies/recursion_awareness.rs b/crates/phase-ai/src/policies/recursion_awareness.rs index 44a95d9406..971110be5a 100644 --- a/crates/phase-ai/src/policies/recursion_awareness.rs +++ b/crates/phase-ai/src/policies/recursion_awareness.rs @@ -136,8 +136,10 @@ mod tests { use crate::config::AiConfig; use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, TacticalClass}; use engine::game::zones::create_object; - use engine::types::ability::{ResolvedAbility, TargetFilter}; - use engine::types::game_state::{PendingCast, TargetSelectionSlot, WaitingFor}; + use engine::types::ability::{EffectKind, ResolvedAbility, TargetFilter}; + use engine::types::game_state::{ + PendingCast, TargetEffectDetail, TargetSelectionSlot, WaitingFor, + }; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::keywords::Keyword; use engine::types::mana::ManaCost; @@ -194,6 +196,8 @@ mod tests { legal_targets: vec![TargetRef::Object(creature)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), @@ -286,6 +290,8 @@ mod tests { legal_targets: vec![TargetRef::Object(creature)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), diff --git a/crates/phase-ai/src/policies/stack_awareness.rs b/crates/phase-ai/src/policies/stack_awareness.rs index 5a48f49613..f1d7b465ae 100644 --- a/crates/phase-ai/src/policies/stack_awareness.rs +++ b/crates/phase-ai/src/policies/stack_awareness.rs @@ -317,10 +317,11 @@ mod tests { use crate::config::AiConfig; use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, TacticalClass}; use engine::game::zones::create_object; - use engine::types::ability::{BounceSelection, ResolvedAbility, TargetFilter}; + use engine::types::ability::{BounceSelection, EffectKind, ResolvedAbility, TargetFilter}; use engine::types::card_type::CoreType; use engine::types::game_state::{ - GameState, PendingCast, StackEntry, StackEntryKind, TargetSelectionSlot, WaitingFor, + GameState, PendingCast, StackEntry, StackEntryKind, TargetEffectDetail, + TargetSelectionSlot, WaitingFor, }; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::mana::ManaCost; @@ -384,6 +385,8 @@ mod tests { legal_targets: vec![TargetRef::Object(target_id)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), diff --git a/crates/phase-ai/src/policies/tests/removal_lethality.rs b/crates/phase-ai/src/policies/tests/removal_lethality.rs index b9ef7638af..acb46cec6d 100644 --- a/crates/phase-ai/src/policies/tests/removal_lethality.rs +++ b/crates/phase-ai/src/policies/tests/removal_lethality.rs @@ -15,13 +15,15 @@ use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, Tac use engine::game::game_object::GameObject; use engine::game::zones::create_object; use engine::types::ability::{ - DamageContextSnapshot, DamageSource, EachDamageRecipient, Effect, QuantityExpr, + DamageContextSnapshot, DamageSource, EachDamageRecipient, Effect, EffectKind, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef, }; use engine::types::actions::GameAction; use engine::types::card_type::{CardType, CoreType}; use engine::types::format::FormatConfig; -use engine::types::game_state::{GameState, PendingCast, TargetSelectionSlot, WaitingFor}; +use engine::types::game_state::{ + GameState, PendingCast, TargetEffectDetail, TargetSelectionSlot, WaitingFor, +}; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::keywords::Keyword; use engine::types::mana::ManaCost; @@ -237,6 +239,8 @@ fn with_pending( legal_targets: Vec::new(), optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: Default::default(), diff --git a/crates/phase-ai/src/search.rs b/crates/phase-ai/src/search.rs index e2daade100..19007799c2 100644 --- a/crates/phase-ai/src/search.rs +++ b/crates/phase-ai/src/search.rs @@ -3878,6 +3878,8 @@ mod tests { legal_targets: vec![TargetRef::Object(opp_creature)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: engine::types::game_state::TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(), @@ -4489,6 +4491,8 @@ mod tests { legal_targets: stale_slot_targets, optional, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: engine::types::game_state::TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: engine::types::game_state::TargetSelectionProgress { @@ -5185,6 +5189,8 @@ mod tests { ], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: engine::types::game_state::TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(), @@ -5224,6 +5230,8 @@ mod tests { legal_targets: Vec::new(), optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: engine::types::game_state::TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(), diff --git a/crates/phase-ai/src/tactical_gate.rs b/crates/phase-ai/src/tactical_gate.rs index 1771825f7e..ecc42f119d 100644 --- a/crates/phase-ai/src/tactical_gate.rs +++ b/crates/phase-ai/src/tactical_gate.rs @@ -683,10 +683,10 @@ mod tests { use engine::ai_support::{ActionMetadata, TacticalClass}; use engine::game::combat::{AttackerInfo, CombatState}; use engine::game::scenario::{GameScenario, P0, P1}; - use engine::types::ability::{BounceSelection, ResolvedAbility, TargetFilter}; + use engine::types::ability::{BounceSelection, EffectKind, ResolvedAbility, TargetFilter}; use engine::types::game_state::{ - PendingCast, StackEntry, StackEntryKind, TargetSelectionProgress, TargetSelectionSlot, - WaitingFor, + PendingCast, StackEntry, StackEntryKind, TargetEffectDetail, TargetSelectionProgress, + TargetSelectionSlot, WaitingFor, }; use engine::types::identifiers::CardId; use engine::types::keywords::WardCost; @@ -843,6 +843,8 @@ mod tests { legal_targets: vec![TargetRef::Object(creature)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: TargetSelectionProgress::default(), @@ -914,6 +916,8 @@ mod tests { legal_targets: vec![TargetRef::Object(creature)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: TargetSelectionProgress::default(), @@ -967,6 +971,8 @@ mod tests { legal_targets: vec![TargetRef::Object(creature)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), selection: TargetSelectionProgress::default(), diff --git a/crates/phase-ai/tests/scenarios.rs b/crates/phase-ai/tests/scenarios.rs index 4e14f6eff0..27f1189f17 100644 --- a/crates/phase-ai/tests/scenarios.rs +++ b/crates/phase-ai/tests/scenarios.rs @@ -5,14 +5,15 @@ use engine::game::combat::{AttackTarget, AttackerInfo, CombatState}; use engine::game::engine::apply_as_current; use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; use engine::types::ability::{ - ChoiceType, Effect, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef, + ChoiceType, Effect, EffectKind, QuantityExpr, ResolvedAbility, TargetFilter, TargetRef, }; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; use engine::types::events::GameEvent; use engine::types::game_state::CastPaymentMode; use engine::types::game_state::{ - StackEntry, StackEntryKind, TargetSelectionProgress, TargetSelectionSlot, WaitingFor, + StackEntry, StackEntryKind, TargetEffectDetail, TargetSelectionProgress, TargetSelectionSlot, + WaitingFor, }; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::log::{LogCategory, LogSegment}; @@ -36,6 +37,8 @@ fn scenario_prefers_opponent_target_over_self() { legal_targets: vec![TargetRef::Player(P0), TargetRef::Player(P1)], optional: false, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(), @@ -72,6 +75,8 @@ fn scenario_skips_optional_target_with_no_legal_choices() { legal_targets: Vec::new(), optional: true, chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, }], mode_labels: Vec::new(), target_constraints: Vec::new(),