diff --git a/crates/engine/src/analysis/ability_graph.rs b/crates/engine/src/analysis/ability_graph.rs index bbf63ae952..eb79b19df5 100644 --- a/crates/engine/src/analysis/ability_graph.rs +++ b/crates/engine/src/analysis/ability_graph.rs @@ -561,6 +561,7 @@ fn project_mana_production(p: &ManaProduction) -> (Vec<(usize, i64)>, AxisMagnit (vec![(slot, a)], mag) } ManaProduction::ChosenColor { count, .. } + | ManaProduction::NotedType { count, .. } | ManaProduction::OpponentLandColors { count, .. } | ManaProduction::AnyTypeProduceableBy { count, .. } | ManaProduction::AnyInCommandersColorIdentity { count, .. } @@ -969,6 +970,7 @@ fn effect_projection(effect: &Effect) -> Projection { | Effect::GrantCastingPermission { .. } | Effect::ChooseFromZone { .. } | Effect::RememberCard { .. } + | Effect::NoteManaSpent | Effect::ForEachCategory { .. } | Effect::ChooseObjectsIntoTrackedSet { .. } | Effect::ChooseAndSacrificeRest { .. } diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index 9c4c0af93e..44fb97291f 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -3409,6 +3409,7 @@ fn legacy_effect(x: &Effect) -> bool { | Effect::RegisterBending { .. } | Effect::Cleanup { .. } | Effect::Learn + | Effect::NoteManaSpent | Effect::Forage | Effect::Harness | Effect::CollectEvidence { .. } @@ -3743,6 +3744,7 @@ fn walk_ability( distribution: _, chosen_x: _, cost_paid_object: _, + noted_mana_payment: _, // concrete captured payment snapshot, no read/write effect cost_paid_object_ids: _, effect_context_object: _, amassed_army_object: _, @@ -5654,6 +5656,7 @@ fn rw_effect( | Effect::Harness | Effect::ChooseAndSacrificeRest { .. } | Effect::RememberCard { .. } + | Effect::NoteManaSpent | Effect::ForEachCategory { .. } | Effect::VentureInto { .. } | Effect::TakeTheInitiative diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index 729ca1c885..f41146baf8 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -229,6 +229,7 @@ fn resolved_ability_axes(a: &ResolvedAbility, mode: ScanMode) -> Axes { targets: _, // concrete announced target refs (already resolved) source_id: _, // object id source_incarnation: _, // self-transform epoch latch, no dynamic read + noted_mana_payment: _, // concrete activation-payment snapshot, no dynamic read trigger_source: _, // exact triggered-source authority, no dynamic read trigger_definition_ref: _, // exact trigger occurrence, no dynamic read force_block_attacker: _, // exact force-block referent, no dynamic read @@ -798,6 +799,8 @@ fn scan_effect(x: &Effect, mode: ScanMode) -> Axes { Effect::TimeTravel => Axes::NONE, Effect::BecomeMonarch => Axes::NONE, Effect::NoOp => Axes::NONE, + // Captured at activation time; no resolution-time dynamic read. + Effect::NoteManaSpent => Axes::NONE, Effect::Proliferate => Axes::NONE, Effect::ProliferateTarget { target } => { let mut acc = Axes::NONE; @@ -4999,6 +5002,14 @@ fn scan_mana_production(p: &ManaProduction, mode: ScanMode) -> Axes { | ManaProduction::AnyInCommandersColorIdentity { count, .. } => { scan_quantity_expr(count, mode) } + // `NotedManaSpent` is mutable per-object state written by a companion + // `Effect::NoteManaSpent`, so sibling activations can affect its value. + ManaProduction::NotedType { count } => Axes { + event: false, + sibling: true, + projected: false, + } + .or(scan_quantity_expr(count, mode)), // SCOPED-OBJECT (Omnath, Locus of All): a SINGLE scoped object's colors, // NOT a board aggregate — the scope's own read surface is the sole sibling // source (CR 202.2c). NO own sibling literal. @@ -5363,6 +5374,7 @@ fn effect_target_ctx(e: &Effect, mode: ScanMode) -> FilterReadContext { | Effect::TimeTravel | Effect::BecomeMonarch | Effect::NoOp + | Effect::NoteManaSpent | Effect::Proliferate | Effect::ProliferateTarget { .. } | Effect::Populate @@ -5767,6 +5779,7 @@ fn effect_census_role(e: &Effect) -> CensusRole { | Effect::TimeTravel | Effect::BecomeMonarch | Effect::NoOp + | Effect::NoteManaSpent | Effect::Proliferate | Effect::ProliferateTarget { .. } | Effect::Populate @@ -6000,6 +6013,7 @@ pub(crate) fn effect_is_randomness_bearing(e: &Effect) -> bool { | Effect::TimeTravel | Effect::BecomeMonarch | Effect::NoOp + | Effect::NoteManaSpent | Effect::Proliferate | Effect::ProliferateTarget { .. } | Effect::Populate @@ -7360,6 +7374,22 @@ mod tests { assert!(effect_is_randomness_bearing(&Effect::Clash)); } + #[test] + fn noted_mana_effect_is_read_free_and_deterministic() { + let effect = Effect::NoteManaSpent; + let axes = scan_effect(&effect, ScanMode::LoopFirewall); + assert!(!axes.event && !axes.sibling && !axes.projected); + assert_eq!( + effect_target_ctx(&effect, ScanMode::LoopFirewall), + FilterReadContext::SnapshotOrEvent + ); + assert_eq!( + effect_census_role(&effect), + CensusRole::Relax(RelaxReason::BoundedOrNoPopulation) + ); + assert!(!effect_is_randomness_bearing(&effect)); + } + #[test] fn spell_ability_randomness_ability_level_and_tree() { use crate::types::ability::{AbilityKind, TargetSelectionMode}; diff --git a/crates/engine/src/game/casting.rs b/crates/engine/src/game/casting.rs index 08a11f133d..c997937be8 100644 --- a/crates/engine/src/game/casting.rs +++ b/crates/engine/src/game/casting.rs @@ -15577,6 +15577,24 @@ fn pay_ability_mana_cost_with_choices_excluding_and_parent( parent, )?; + // CR 106.1b + CR 602.2b (issue #6504): stamp the mana type(s) just spent + // onto this ability's own source, mirroring `colors_spent_to_cast`'s + // cast-side stamp-then-read idiom. This is the single authority where an + // activated ability's mana sub-cost is paid (both the direct-activation + // and interactive/PendingCast routes funnel through here). PURELY A + // BRIDGE: `push_ability_entry` drains this field synchronously into + // THIS activation's own `ResolvedAbility::noted_mana_payment` snapshot + // moments later, before any later activation of the same permanent + // could occur — see `GameObject::mana_spent_to_activate` for why a + // companion "note the type of mana spent to pay this activation cost" + // effect (Jeweled Amulet) never reads this field directly. + let spent_units = match &payment { + ManaCostPayment::Paid(units) | ManaCostPayment::Paused { value: units, .. } => units, + }; + if let Some(obj) = state.objects.get_mut(&source_id) { + obj.mana_spent_to_activate = spent_units.iter().map(|unit| unit.color).collect(); + } + Ok(match payment { ManaCostPayment::Paid(_) => ManaCostPayment::Paid(()), ManaCostPayment::Paused { diff --git a/crates/engine/src/game/casting_costs.rs b/crates/engine/src/game/casting_costs.rs index b8a38e6646..dd8eb421e1 100644 --- a/crates/engine/src/game/casting_costs.rs +++ b/crates/engine/src/game/casting_costs.rs @@ -5,8 +5,8 @@ use crate::types::ability::{ is_chosen_remove_counter_cost_count, AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AdditionalCost, AdditionalCostInstance, AdditionalCostOrigin, AggregateFunction, BeholdCostAction, CastTimingPermission, Comparator, CostPaidObjectSnapshot, - CounterCostSelection, Effect, KickerVariant, ObjectProperty, QuantityExpr, QuantityRef, - ReplacementDefinition, ResolvedAbility, SacrificeCost, SacrificeRequirement, + CounterCostSelection, Effect, KickerVariant, NotedManaPayment, ObjectProperty, QuantityExpr, + QuantityRef, ReplacementDefinition, ResolvedAbility, SacrificeCost, SacrificeRequirement, SpellCastingOptionKind, SpellContext, SpellStackToGraveyardReplacement, StaticCondition, TapCreaturesAggregate, TargetFilter, ThisWayCause, TypeFilter, TypedFilter, EXILE_COST_X, }; @@ -5406,6 +5406,30 @@ pub(super) fn push_ability_entry( // also clears any stale value. state.announced_source_x = resolved.chosen_x.map(|x| (source_id, x)); + // CR 106.1b + CR 400.7 + CR 602.2b (issue #6504): consume the source's + // transient mana-spent-to-activate latch into THIS activation's own + // `ResolvedAbility` snapshot (and every sub/else branch — `Effect:: + // NoteManaSpent` is typically a `sub_ability`, which resolves as its own + // separate node, so the stamp must recurse; see + // `set_noted_mana_payment_recursive`), paired with the source's + // incarnation at this exact moment. Since `push_ability_entry` is the + // single authority where an activated ability reaches the stack, this + // capture happens synchronously, immediately after cost payment + // completed and before any later activation of the same permanent could + // occur — so a permanent untapped and reactivated while this ability + // still sits unresolved on the stack cannot corrupt what THIS instance + // observed. The latch is cleared immediately after, so it never appears + // to hold a stale value between activations. + if let Some(obj) = state.objects.get_mut(&source_id) { + if !obj.mana_spent_to_activate.is_empty() { + let payment = NotedManaPayment { + types: std::mem::take(&mut obj.mana_spent_to_activate), + source_incarnation: obj.incarnation, + }; + resolved.set_noted_mana_payment_recursive(payment); + } + } + // CR 603.4: Stamp the printed-ability index for per-turn resolution tracking. resolved.ability_index = Some(ability_index); stack::push_to_stack( @@ -11400,6 +11424,11 @@ pub(crate) fn production_override_for_option( } | crate::types::ability::ManaProduction::ChoiceAmongCombinations { .. } | crate::types::ability::ManaProduction::DistinctColorsAmongPermanents { .. } + // CR 106.1b + CR 106.5: like `ChosenColor { fixed_alternative: None }`, + // the produced type is fixed by engine-set state read at production + // time (`noted_mana_type_for`), not chosen per auto-tap option — no + // override needed, and CR 106.5 governs the no-noted-type case. + | crate::types::ability::ManaProduction::NotedType { .. } | crate::types::ability::ManaProduction::TriggerEventManaType => None, } } diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 9336379b5d..8edbec0429 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -1976,6 +1976,9 @@ fn fmt_mana_production(mp: &ManaProduction) -> String { ManaProduction::ChosenColor { count, .. } => { format!("{} of chosen color", fmt_quantity(count)) } + ManaProduction::NotedType { count } => { + format!("{} of noted type", fmt_quantity(count)) + } ManaProduction::OpponentLandColors { count } => { format!("{} of opponent land colors", fmt_quantity(count)) } @@ -3692,6 +3695,7 @@ fn effect_details(effect: &Effect) -> Vec<(String, String)> { | Effect::Forage | Effect::Harness | Effect::Learn + | Effect::NoteManaSpent | Effect::SwitchPT { .. } | Effect::Myriad | Effect::Encore diff --git a/crates/engine/src/game/effects/additional_phase.rs b/crates/engine/src/game/effects/additional_phase.rs index 0e6e4a613b..77a35200da 100644 --- a/crates/engine/src/game/effects/additional_phase.rs +++ b/crates/engine/src/game/effects/additional_phase.rs @@ -290,6 +290,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, diff --git a/crates/engine/src/game/effects/copy_spell.rs b/crates/engine/src/game/effects/copy_spell.rs index e2d5a040ff..7f6b510651 100644 --- a/crates/engine/src/game/effects/copy_spell.rs +++ b/crates/engine/src/game/effects/copy_spell.rs @@ -827,9 +827,20 @@ pub(crate) fn set_resolved_source_recursive(ability: &mut ResolvedAbility, sourc } } +/// CR 707.10 + CR 707.10b: Normalize a copied activated/triggered ability. +/// The copy keeps the original ability's source (unlike a spell copy, which +/// sources itself), so this only re-stamps `source_id` uniformly through the +/// chain — but it must ALSO clear `noted_mana_payment` (issue #6504): +/// CR 707.10 says a copy of an activated ability is not itself activated, so +/// it never paid a mana cost. A naive struct clone otherwise carries the +/// ORIGINAL activation's payment snapshot along for the ride, and +/// `Effect::NoteManaSpent` resolving on the copy (Jeweled Amulet's first +/// ability copied via Rings of Brighthearth or similar) would falsely note +/// mana colors the copy never paid. fn preserve_ability_copy_source_recursive(ability: &mut ResolvedAbility) { let source_id = ability.source_id; set_resolved_source_recursive(ability, source_id); + ability.clear_noted_mana_payment_recursive(); } /// CR 707.10d: Replace every object target on a copied spell with `new_target`. diff --git a/crates/engine/src/game/effects/double.rs b/crates/engine/src/game/effects/double.rs index a40ed06c01..54c941b695 100644 --- a/crates/engine/src/game/effects/double.rs +++ b/crates/engine/src/game/effects/double.rs @@ -342,6 +342,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, diff --git a/crates/engine/src/game/effects/extra_turn.rs b/crates/engine/src/game/effects/extra_turn.rs index 7e63dab193..a871aa0f48 100644 --- a/crates/engine/src/game/effects/extra_turn.rs +++ b/crates/engine/src/game/effects/extra_turn.rs @@ -95,6 +95,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, diff --git a/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs b/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs index 761be2b674..a0deac3b11 100644 --- a/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs +++ b/crates/engine/src/game/effects/grant_extra_loyalty_activations.rs @@ -113,6 +113,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, diff --git a/crates/engine/src/game/effects/mana.rs b/crates/engine/src/game/effects/mana.rs index e0cec7b2ce..ad9f8309c0 100644 --- a/crates/engine/src/game/effects/mana.rs +++ b/crates/engine/src/game/effects/mana.rs @@ -707,6 +707,23 @@ fn resolve_mana_types_impl( (None, None) => Vec::new(), } } + // CR 106.1b + CR 106.5: Jeweled Amulet — "Add one mana of this + // artifact's last noted type." Unlike `ChosenColor` (a + // player-prompted `ManaColor`), the noted value is engine-set + // (`Effect::NoteManaSpent`) and `ManaType`-valued (colorless is a + // real noted type per the card's ruling). A card in this class always + // notes exactly one type — its cost's own generic mana is spent as a + // single unit-worth of one type — so, mirroring `AnyOneColor`'s + // repeat-by-count idiom, the first noted type repeats `count` times. + // No noted type (never activated the noting ability, or a fresh + // incarnation after a zone change) produces no mana. + ManaProduction::NotedType { count } => { + let amount = resolve_count(count, state, ability, controller, source_id); + match noted_mana_type_for(state, source_id) { + Some(mana_type) => vec![mana_type; amount], + None => Vec::new(), + } + } // CR 106.7: Produce mana of any color that a land an opponent controls could produce. // Delegates to mana_sources::opponent_land_color_options for the shared computation. ManaProduction::OpponentLandColors { count } => { @@ -998,6 +1015,21 @@ pub(crate) fn chosen_color_for_mana( }) } +/// CR 106.1b: The first mana type noted by a past `Effect::NoteManaSpent` +/// resolution on `source_id` ("this artifact's last noted type" — Jeweled +/// Amulet). Unlike `chosen_color_for_mana`, this is never player-prompted — +/// engine-set state only, with no `last_named_choice` fallback. +pub(crate) fn noted_mana_type_for( + state: &GameState, + source_id: crate::types::identifiers::ObjectId, +) -> Option { + state + .objects + .get(&source_id) + .and_then(|obj| obj.noted_mana_spent()) + .and_then(|types| types.first().copied()) +} + /// Convert a ManaColor to the runtime ManaType. /// CR 106.1a: There are five colors of mana: white, blue, black, red, and green. /// CR 106.1b: There are six types of mana: white, blue, black, red, green, and colorless. diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index e1df72586b..d6580b949f 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -157,6 +157,7 @@ pub mod manifest_dread; pub mod mill; pub mod monstrosity; pub mod myriad; +pub mod note_mana_spent; pub mod opponent_guess; pub mod overload; pub mod pair_with; @@ -4410,6 +4411,7 @@ pub fn resolve_effect( Effect::GrantCastingPermission { .. } => grant_permission::resolve(state, ability, events), Effect::ChooseFromZone { .. } => choose_from_zone::resolve(state, ability, events), Effect::RememberCard { .. } => remember_card::resolve(state, ability, events), + Effect::NoteManaSpent => note_mana_spent::resolve(state, ability, events), Effect::ForEachCategory { .. } => { choose_from_zone::resolve_for_each_category(state, ability, events) } diff --git a/crates/engine/src/game/effects/note_mana_spent.rs b/crates/engine/src/game/effects/note_mana_spent.rs new file mode 100644 index 0000000000..d435e938ef --- /dev/null +++ b/crates/engine/src/game/effects/note_mana_spent.rs @@ -0,0 +1,73 @@ +use crate::types::ability::{ChosenAttribute, Effect, EffectError, ResolvedAbility}; +use crate::types::events::GameEvent; +use crate::types::game_state::GameState; + +/// CR 106.1b + CR 602.2b + CR 608.2c: `Effect::NoteManaSpent` — record the mana +/// type(s) spent to pay this resolving ability's own activation cost onto its +/// source as `ChosenAttribute::NotedManaSpent` ("Note the type of mana spent to +/// pay this activation cost" — Jeweled Amulet). Scoped to the singular-type +/// wording only: Ice Cauldron's sibling "note the type AND AMOUNT..." needs an +/// exact stored multiset plus a spend restriction, which this building block +/// does not model — that text is intentionally left unmatched by the parser +/// and still reports `Effect::unimplemented`. +/// +/// Composable building block: cost payment stays in the mana-payment funnel; +/// `push_ability_entry` (the single authority where an activated ability +/// reaches the stack) snapshots what was spent, paired with the source's +/// incarnation at that moment, directly onto THIS activation's own +/// `ResolvedAbility::noted_mana_payment` (issue #6504) — never a per-object +/// mutable field, so a permanent untapped and reactivated with a different +/// payment while this ability still sits unresolved on the stack cannot +/// corrupt what this instance observed. This effect is the persistent writer, +/// read back by `ManaProduction::NotedType`. Doing the write at resolution — +/// not at payment time — means a countered or otherwise removed-from-stack +/// ability never notes anything (CR 608.2c: instructions are followed only on +/// resolution). +/// +/// "The last noted type" is singular per card, so this replaces any prior +/// `ChosenAttribute::NotedManaSpent` before pushing (replace-on-rechoose). +/// +/// CR 400.7: a source that leaves and returns (bounce/flicker) while this +/// SAME activation is still unresolved on the stack becomes a new object at +/// the same storage id — a new incarnation with no memory of the old +/// payment. Refuses to write unless the object's current incarnation still +/// matches `noted_mana_payment.source_incarnation`, mirroring the engine's +/// existing incarnation-pairing idiom (`ResolvedAbility::source_is_current`, +/// `TargetFilter::SelfRef` resolution). +pub fn resolve( + state: &mut GameState, + ability: &ResolvedAbility, + _events: &mut Vec, +) -> Result<(), EffectError> { + let Effect::NoteManaSpent = &ability.effect else { + return Ok(()); + }; + + let Some(payment) = ability.noted_mana_payment.as_ref() else { + // Nothing was captured at activation (e.g. the cost had no mana + // component to observe) — nothing to note. + return Ok(()); + }; + + let Some(src) = state.objects.get(&ability.source_id) else { + // CR 608.2c: the source has left the zone it was in — nothing to note. + return Ok(()); + }; + if src.incarnation != payment.source_incarnation { + // CR 400.7: this activation's payment was captured on a prior + // incarnation of this object (bounced/flickered since). The CURRENT + // incarnation never paid anything itself — nothing to note. + return Ok(()); + } + let spent_types = payment.types.clone(); + + let Some(src) = state.objects.get_mut(&ability.source_id) else { + return Ok(()); + }; + src.chosen_attributes + .retain(|a| !matches!(a, ChosenAttribute::NotedManaSpent(_))); + src.chosen_attributes + .push(ChosenAttribute::NotedManaSpent(spent_types)); + + Ok(()) +} diff --git a/crates/engine/src/game/effects/player_counter.rs b/crates/engine/src/game/effects/player_counter.rs index bd7130cacc..07294a2528 100644 --- a/crates/engine/src/game/effects/player_counter.rs +++ b/crates/engine/src/game/effects/player_counter.rs @@ -356,6 +356,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, @@ -550,6 +551,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, diff --git a/crates/engine/src/game/effects/reverse_turn_order.rs b/crates/engine/src/game/effects/reverse_turn_order.rs index 0eea7a4426..199068496a 100644 --- a/crates/engine/src/game/effects/reverse_turn_order.rs +++ b/crates/engine/src/game/effects/reverse_turn_order.rs @@ -68,6 +68,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, diff --git a/crates/engine/src/game/effects/skip_next_step.rs b/crates/engine/src/game/effects/skip_next_step.rs index f7407178a5..55a5469d16 100644 --- a/crates/engine/src/game/effects/skip_next_step.rs +++ b/crates/engine/src/game/effects/skip_next_step.rs @@ -131,6 +131,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, diff --git a/crates/engine/src/game/effects/skip_next_turn.rs b/crates/engine/src/game/effects/skip_next_turn.rs index 83ef4a5629..8192587e53 100644 --- a/crates/engine/src/game/effects/skip_next_turn.rs +++ b/crates/engine/src/game/effects/skip_next_turn.rs @@ -109,6 +109,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, diff --git a/crates/engine/src/game/effects/vote.rs b/crates/engine/src/game/effects/vote.rs index 86b87fd47d..afb2f26433 100644 --- a/crates/engine/src/game/effects/vote.rs +++ b/crates/engine/src/game/effects/vote.rs @@ -389,6 +389,7 @@ pub fn resolve_tally( starting_with: per_choice_effect[idx].starting_with.clone(), chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, @@ -456,6 +457,7 @@ pub fn resolve_tally( starting_with: per_choice_effect[idx].starting_with.clone(), chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, @@ -700,6 +702,7 @@ fn resolved_from_def( starting_with: def.starting_with.clone(), chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, @@ -956,6 +959,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, @@ -1064,6 +1068,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, @@ -1497,6 +1502,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, @@ -1662,6 +1668,7 @@ mod tests { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 3adc387a86..54df0eb669 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -15668,9 +15668,11 @@ mod stage2_injector_tests { // because that is what makes a NEW mint a counted event; a function + // content-hash anchor would end the drift class while keeping that property, // and is offered as a follow-up rather than taken unannounced mid-review. - "game/effects/mod.rs:6210".to_string(), - "game/effects/mod.rs:6287".to_string(), - "game/effects/mod.rs:9475".to_string(), + // #6812 noted-mana support inserts two lines above all three producers: + // `:6210/:6287/:9475 => :6212/:6289/:9477`. The producers remain byte-identical. + "game/effects/mod.rs:6212".to_string(), + "game/effects/mod.rs:6289".to_string(), + "game/effects/mod.rs:9477".to_string(), // UNMOVED across the rebase, and that is itself evidence the SET did not // move: a census that had gained or lost a producer would not leave this // entry both byte-identical AND at the same coordinate. diff --git a/crates/engine/src/game/game_object.rs b/crates/engine/src/game/game_object.rs index 974f19aeb1..3919c7cc5a 100644 --- a/crates/engine/src/game/game_object.rs +++ b/crates/engine/src/game/game_object.rs @@ -21,7 +21,7 @@ use crate::types::game_state::{ }; use crate::types::identifiers::{CardId, ObjectId, ObjectIdentityBinding, ObjectIncarnationRef}; use crate::types::keywords::{Keyword, KeywordKind}; -use crate::types::mana::{ColoredManaCount, ManaColor, ManaCost, ManaPip}; +use crate::types::mana::{ColoredManaCount, ManaColor, ManaCost, ManaPip, ManaType}; use crate::types::player::PlayerId; use crate::types::stickers::AppliedSticker; use crate::types::zones::Zone; @@ -1186,6 +1186,25 @@ pub struct GameObject { /// all rules queries. Defaults to `PhasedIn` for replay compatibility. #[serde(default)] pub phase_status: PhaseStatus, + + /// CR 106.1b + CR 602.2b (issue #6504): Mana type(s) spent to pay this + /// object's own activated-ability mana cost, stamped by + /// `pay_ability_mana_cost_with_choices_excluding_and_parent` at + /// activation-time payment. PURELY A BRIDGE: `push_ability_entry` (the + /// single authority where an activated ability reaches the stack) + /// synchronously drains this field — via `std::mem::take` — into that + /// specific activation's own `ResolvedAbility::noted_mana_payment` + /// (paired with the source's live incarnation at that same moment) + /// immediately after cost payment completes, before any later activation + /// of this permanent could occur. Nothing reads this field at resolution + /// time; `Effect::NoteManaSpent` reads the per-activation snapshot + /// instead, so a permanent untapped and reactivated with a different + /// payment while an earlier activation still sits unresolved on the + /// stack cannot corrupt what that earlier instance observed. Always + /// empty except transiently between the payment stamp and the very next + /// `push_ability_entry` call for the same source. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub mana_spent_to_activate: Vec, } /// CR 104.4b compile-time totality guard for `objects_content_eq`/`object_content_eq` @@ -1345,6 +1364,11 @@ fn _gameobject_partition_is_total(o: &GameObject) { mana_spent_source_snapshots: _, phase_status: _, protection_start_exempt_attachments: _, + // Activation-cost-payment latch — same omission class as + // `mana_spent_to_cast`/`colors_spent_to_cast` above: drained + // synchronously by `push_ability_entry` into the resolving + // `ResolvedAbility`'s own `noted_mana_payment` snapshot (§5.2c). + mana_spent_to_activate: _, } = o; } @@ -2212,6 +2236,7 @@ impl GameObject { phyrexian_life_paid: 0, mana_spent_source_snapshots: Vec::new(), phase_status: PhaseStatus::PhasedIn, + mana_spent_to_activate: Vec::new(), } } @@ -2659,6 +2684,16 @@ impl GameObject { }) } + /// CR 106.1b: Look up the mana type(s) noted by a past `Effect::NoteManaSpent` + /// resolution on this permanent's own ability ("this artifact's last noted + /// type" — Jeweled Amulet). Read by `ManaProduction::NotedType`. + pub fn noted_mana_spent(&self) -> Option<&[ManaType]> { + self.chosen_attributes.iter().find_map(|a| match a { + ChosenAttribute::NotedManaSpent(types) => Some(types.as_slice()), + _ => None, + }) + } + /// CR 205.2: Look up a stored card-type choice (e.g. the card /// type chosen as this permanent entered the battlefield). /// diff --git a/crates/engine/src/game/mana_sources.rs b/crates/engine/src/game/mana_sources.rs index dfbe91a861..189bd2666d 100644 --- a/crates/engine/src/game/mana_sources.rs +++ b/crates/engine/src/game/mana_sources.rs @@ -1608,6 +1608,20 @@ pub fn display_land_mana_pips( } } }, + // CR 106.1b + CR 106.5: Engine-set noted type (Jeweled Amulet class). + // Unreachable in practice today — no printed land has this + // mechanic — but display the noted type when present, mirroring + // `ChoiceAmongExiledColors`'s "compute, push if non-empty" shape. + ManaProduction::NotedType { .. } => { + if let Some(mana_type) = super::effects::mana::noted_mana_type_for(state, object_id) + { + if let Some(color) = mana_type_to_color(mana_type) { + push(&mut pips, ManaPip::Color(color)); + } else { + push(&mut pips, ManaPip::Colorless); + } + } + } // CR 106.7: Dynamically computed from opponent lands. ManaProduction::OpponentLandColors { .. } => { let colors: Vec = opponent_land_color_options(state, controller) @@ -2929,6 +2943,13 @@ fn mana_options_from_production( ManaProduction::ChosenColor { fixed_alternative, .. } => chosen_color_mana_type_options(state, object_id, *fixed_alternative), + // CR 106.1b + CR 106.5: Engine-set noted type (Jeweled Amulet class). + // Unreachable in practice today — no printed land has this mechanic. + ManaProduction::NotedType { .. } => { + super::effects::mana::noted_mana_type_for(state, object_id) + .into_iter() + .collect() + } // CR 106.7: Compute colors dynamically from opponent-controlled lands. ManaProduction::OpponentLandColors { .. } => opponent_land_color_options(state, controller), // CR 106.7 + CR 106.1b: Compute the full type set (incl. Colorless) diff --git a/crates/engine/src/game/resolution_prompt.rs b/crates/engine/src/game/resolution_prompt.rs index 4be3c26823..64c03031f2 100644 --- a/crates/engine/src/game/resolution_prompt.rs +++ b/crates/engine/src/game/resolution_prompt.rs @@ -208,6 +208,8 @@ fn quantity_offers_up_to_choice(q: &QuantityExpr) -> bool { /// a clone for. fn effect_offers_choice(e: &Effect) -> bool { match e { + // Engine-set from the activation-payment snapshot, never a player prompt. + Effect::NoteManaSpent => false, // ---- SCOPE FILTER. DESTRUCTURED WITHOUT `..` on every arm, exactly as // HEAD's three allow arms are, so a new field on any of them forces // a re-audit of whether the class is still in scope. @@ -547,6 +549,7 @@ pub(crate) fn chain_offers_choice(a: &ResolvedAbility) -> bool { targets: _, // concrete announced target refs (already resolved) source_id: _, // object id source_incarnation: _, // self-transform epoch latch, no resolution-time choice + noted_mana_payment: _, // concrete activation-payment snapshot, no resolution-time choice trigger_source: _, // exact triggered-source authority, no choice trigger_definition_ref: _, // exact trigger occurrence, no choice force_block_attacker: _, // exact force-block referent, no choice @@ -1720,6 +1723,11 @@ mod tests { } } + #[test] + fn noted_mana_spent_never_offers_a_resolution_choice() { + assert!(!effect_offers_choice(&Effect::NoteManaSpent)); + } + /// STRUCTURAL INVARIANT: `game/ability_scan.rs` holds NO `GameState`. /// /// The module header defines it as a pure AST walk, and that contract is diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index 9663c44359..3d46b04937 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -3023,6 +3023,7 @@ fn self_counter_ability_is_batch_candidate(ability: &ResolvedAbility) -> bool { starting_with, chosen_x, cost_paid_object, + noted_mana_payment, cost_paid_object_ids, effect_context_object, amassed_army_object, @@ -3084,6 +3085,13 @@ fn self_counter_ability_is_batch_candidate(ability: &ResolvedAbility) -> bool { && starting_with.is_none() && chosen_x.is_none() && cost_paid_object.is_none() + // Issue #6504: a batched ability must not carry a per-activation + // noted-mana-payment snapshot either — two sibling copies of a + // "note the type of mana spent..." ability can carry DIFFERENT + // payments (that's the whole point of threading it per-activation + // rather than through a shared mutable latch), so they are never + // safe to merge into one batched resolution. + && noted_mana_payment.is_none() // CR 117.1 (issue #4948): a batched triggered ability must not carry // per-instance cost-paid-object state either — mirrors the // `cost_paid_object` gate above. Always empty for triggered @@ -3226,6 +3234,7 @@ fn fixed_controller_gain_life_ability_is_batch_candidate(ability: &ResolvedAbili starting_with, chosen_x, cost_paid_object, + noted_mana_payment, cost_paid_object_ids, effect_context_object, amassed_army_object, @@ -3278,6 +3287,7 @@ fn fixed_controller_gain_life_ability_is_batch_candidate(ability: &ResolvedAbili && starting_with.is_none() && chosen_x.is_none() && cost_paid_object.is_none() + && noted_mana_payment.is_none() && cost_paid_object_ids.is_empty() && effect_context_object.is_none() && amassed_army_object.is_none() @@ -3414,6 +3424,7 @@ fn fixed_opponent_lose_life_ability_is_batch_candidate(ability: &ResolvedAbility starting_with, chosen_x, cost_paid_object, + noted_mana_payment, cost_paid_object_ids, effect_context_object, amassed_army_object, @@ -3466,6 +3477,7 @@ fn fixed_opponent_lose_life_ability_is_batch_candidate(ability: &ResolvedAbility && starting_with.is_none() && chosen_x.is_none() && cost_paid_object.is_none() + && noted_mana_payment.is_none() && cost_paid_object_ids.is_empty() && effect_context_object.is_none() && amassed_army_object.is_none() @@ -4050,6 +4062,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( starting_with: a_starting_with, chosen_x: a_chosen_x, cost_paid_object: a_cost_paid_object, + noted_mana_payment: a_noted_mana_payment, cost_paid_object_ids: a_cost_paid_object_ids, effect_context_object: a_effect_context_object, amassed_army_object: a_amassed_army_object, @@ -4103,6 +4116,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( starting_with: b_starting_with, chosen_x: b_chosen_x, cost_paid_object: b_cost_paid_object, + noted_mana_payment: b_noted_mana_payment, cost_paid_object_ids: b_cost_paid_object_ids, effect_context_object: b_effect_context_object, amassed_army_object: b_amassed_army_object, @@ -4162,6 +4176,7 @@ fn inert_trigger_abilities_eq_ignoring_provenance( && a_starting_with == b_starting_with && a_chosen_x == b_chosen_x && a_cost_paid_object == b_cost_paid_object + && a_noted_mana_payment == b_noted_mana_payment && a_cost_paid_object_ids == b_cost_paid_object_ids && a_effect_context_object == b_effect_context_object && a_amassed_army_object == b_amassed_army_object diff --git a/crates/engine/src/game/trigger_index.rs b/crates/engine/src/game/trigger_index.rs index 643d7b7ec8..3147b746f5 100644 --- a/crates/engine/src/game/trigger_index.rs +++ b/crates/engine/src/game/trigger_index.rs @@ -902,6 +902,7 @@ fn keys_from_effect_kind(kind: EffectKind, push: &mut impl FnMut(TriggerEventKey | EffectKind::GrantCastingPermission | EffectKind::ChooseFromZone | EffectKind::RememberCard + | EffectKind::NoteManaSpent | EffectKind::ChooseObjectsIntoTrackedSet // CR 608.2d + CR 122.1: counter-kind choice / consume — the actual // counter placement fires `GameEvent::CounterAdded`, so no matcher diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index 5b65cd4be5..3af74f00e8 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -9720,6 +9720,42 @@ fn parse_behold_effect_ast(text: &str, lower: &str) -> Option OracleResult<'_, ()> { + value( + (), + ( + parse_note_instruction_prefix, + parse_noted_mana_subject, + parse_activation_cost_referent, + ), + ) + .parse(input) +} + +fn parse_note_instruction_prefix(input: &str) -> OracleResult<'_, ()> { + value( + (), + preceded(tag("note "), preceded(opt(tag("the ")), tag("type"))), + ) + .parse(input) +} + +fn parse_noted_mana_subject(input: &str) -> OracleResult<'_, ()> { + value((), tag(" of mana spent")).parse(input) +} + +fn parse_activation_cost_referent(input: &str) -> OracleResult<'_, ()> { + preceded( + tag(" to pay this "), + value((), alt((tag("activation cost"), tag("cost")))), + ) + .parse(input) +} + pub(super) fn parse_imperative_family_ast( text: &str, lower: &str, @@ -10067,6 +10103,13 @@ pub(super) fn parse_imperative_family_ast( return Some(ImperativeFamilyAst::GainKeyword(effect)); } + if all_consuming(terminated(parse_note_mana_spent_clause, opt(tag(".")))) + .parse(lower.trim()) + .is_ok() + { + return Some(ImperativeFamilyAst::NoteManaSpent); + } + // NOTE: when adding verbs here, also add them to IMPERATIVE_EXTRA_VERBS // in game/gap_analysis.rs so the parser gap analyzer can classify them. match first_word { @@ -12661,6 +12704,7 @@ fn lower_imperative_family_effect(ast: ImperativeFamilyAst) -> Effect { }, ImperativeFamilyAst::Investigate => Effect::Investigate, ImperativeFamilyAst::Learn => Effect::Learn, + ImperativeFamilyAst::NoteManaSpent => Effect::NoteManaSpent, // CR 701.40a: Default subject is the controller ("you manifest..."). Subject // lowering for "its controller manifests..." routes through the dedicated // subject-predicate arm in `lower_subject_predicate_ast` below, which diff --git a/crates/engine/src/parser/oracle_effect/mana.rs b/crates/engine/src/parser/oracle_effect/mana.rs index fe2225eca6..c4fad8f486 100644 --- a/crates/engine/src/parser/oracle_effect/mana.rs +++ b/crates/engine/src/parser/oracle_effect/mana.rs @@ -1452,6 +1452,20 @@ fn scan_mana_production_type( }, alt((tag("mana of the chosen color"), tag("mana of that color"))), ), + // CR 106.1b: "mana of ~'s last noted type" (Jeweled Amulet: "Add + // one mana of this artifact's last noted type" — `~` normalized + // from "this artifact" upstream). Engine-set (`Effect:: + // NoteManaSpent`), not player-prompted, so this is a separate + // variant from `ChosenColor` above rather than a shared phrase. + value( + ManaProduction::NotedType { + count: count.clone(), + }, + alt(( + tag("mana of ~'s last noted type"), + tag("mana of ~’s last noted type"), + )), + ), )) .parse(input) }) diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index f9ec5df88e..1aa5468cc2 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -27665,6 +27665,7 @@ fn mana_production_with_count( contribution: *contribution, fixed_alternative: *fixed_alternative, }), + ManaProduction::NotedType { .. } => Some(ManaProduction::NotedType { count }), ManaProduction::OpponentLandColors { .. } => { Some(ManaProduction::OpponentLandColors { count }) } diff --git a/crates/engine/src/parser/oracle_effect/sequence.rs b/crates/engine/src/parser/oracle_effect/sequence.rs index 06232d6160..4ef6ab9110 100644 --- a/crates/engine/src/parser/oracle_effect/sequence.rs +++ b/crates/engine/src/parser/oracle_effect/sequence.rs @@ -6346,6 +6346,7 @@ pub(super) fn clause_is_dig_lookback_transparent(effect: &Effect) -> bool { | Effect::Bolster { .. } | Effect::Adapt { .. } | Effect::Learn + | Effect::NoteManaSpent | Effect::Forage | Effect::Harness | Effect::CollectEvidence { .. } diff --git a/crates/engine/src/parser/oracle_ir/ast.rs b/crates/engine/src/parser/oracle_ir/ast.rs index b9e492e710..de661b6dd4 100644 --- a/crates/engine/src/parser/oracle_ir/ast.rs +++ b/crates/engine/src/parser/oracle_ir/ast.rs @@ -672,6 +672,13 @@ pub(crate) enum ImperativeFamilyAst { Behold(TargetFilter), /// CR 701.48a: Learn. Learn, + /// CR 106.1b + CR 602.2b: "note the type of mana spent to pay this + /// activation cost" (Jeweled Amulet). Field-less: there is nothing to + /// select — the payment already happened, so the effect is a pure + /// readback recorded at resolution. Scoped to the singular-type wording; + /// Ice Cauldron's "note the type AND AMOUNT..." sibling is intentionally + /// left unmatched (see `parse_imperative_family_ast`). + NoteManaSpent, /// CR 701.40a: Manifest the top card(s) of library. Manifest { target: TargetFilter, diff --git a/crates/engine/src/parser/oracle_ir/doc.rs b/crates/engine/src/parser/oracle_ir/doc.rs index c921afeacb..26cd4d288f 100644 --- a/crates/engine/src/parser/oracle_ir/doc.rs +++ b/crates/engine/src/parser/oracle_ir/doc.rs @@ -1596,6 +1596,7 @@ fn stamp_effect_printed_slot(effect: &mut Effect, slot: usize, kind: PrintedItem Effect::ProcessRadCounters => {} Effect::ChooseFromZone { .. } => {} Effect::RememberCard { .. } => {} + Effect::NoteManaSpent => {} Effect::ForEachCategory { .. } => {} Effect::ChooseObjectsIntoTrackedSet { .. } => {} Effect::ChooseAndSacrificeRest { .. } => {} diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 18606e2aca..bc8b1b8b9b 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -2330,6 +2330,7 @@ fn lift_mana_production_quantities_to_triggering_source(produced: &mut ManaProdu | ManaProduction::AnyOneColor { count, .. } | ManaProduction::AnyCombination { count, .. } | ManaProduction::ChosenColor { count, .. } + | ManaProduction::NotedType { count } | ManaProduction::OpponentLandColors { count } | ManaProduction::AnyCombinationOfObjectColors { count, .. } | ManaProduction::AnyTypeProduceableBy { count, .. } diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 4007783e56..4afaa0896a 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -1468,6 +1468,19 @@ pub enum ChosenAttribute { /// zones (CR 400.7), which is exactly the copy's lifetime. Boxed to keep /// the enum small (mirrors the copy-value box idiom). CopiableSnapshot(Box), + /// CR 106.1b + CR 400.7: The mana type(s) spent to pay a past activation of + /// this permanent's own ability ("Note the type of mana spent to pay this + /// activation cost" — Jeweled Amulet). Like `Card` and + /// `TributeOutcome`, this is ENGINE-SET (written by `Effect::NoteManaSpent` + /// from the source's transient `mana_spent_to_activate` payment latch, not + /// produced through `ChoiceType`/`from_choice`) — it is never a + /// player-prompted choice. Read by `ManaProduction::NotedType` at a + /// companion mana ability's resolution. Being stored in `chosen_attributes`, + /// it is cleared automatically when the source permanent changes zones (CR + /// 400.7), matching the ruling that a freshly entered amulet has no noted + /// type. Replace-on-rechoose: `Effect::NoteManaSpent` removes any prior + /// `NotedManaSpent` before pushing. + NotedManaSpent(Vec), } impl ChosenAttribute { @@ -1537,6 +1550,12 @@ impl ChosenAttribute { Self::CopiableSnapshot(_) => ChoiceType::Labeled { options: Vec::new(), }, + // Engine-set, never player-prompted (written by + // `Effect::NoteManaSpent` from the cost-payment latch, not via + // `from_choice`). Mirrors the `Card`/`CopiableSnapshot` placeholder. + Self::NotedManaSpent(_) => ChoiceType::Labeled { + options: Vec::new(), + }, } } @@ -2061,6 +2080,19 @@ pub enum ManaProduction { #[serde(default, skip_serializing_if = "Option::is_none")] fixed_alternative: Option, }, + /// CR 106.1b + CR 106.5: Produce N mana of the type noted by a companion + /// `Effect::NoteManaSpent` ("Add one mana of this artifact's last noted + /// type" — Jeweled Amulet). Unlike `ChosenColor` (a player-prompted + /// `ManaColor`), the noted value is engine-set, `ManaType`-valued (CR + /// 106.1b: colorless is a type, and Jeweled Amulet's ruling confirms a + /// {1} generic cost paid with colorless mana notes colorless), and read + /// from `ChosenAttribute::NotedManaSpent` rather than `ChosenAttribute:: + /// Color`. CR 106.5: no noted type (a freshly entered amulet, or one whose + /// first ability was never activated) produces no mana. + NotedType { + #[serde(default = "default_quantity_one")] + count: QuantityExpr, + }, /// CR 106.7: Produce mana of any color that a land an opponent controls could produce. /// Colors are computed dynamically at resolution time by inspecting opponent lands. OpponentLandColors { @@ -2183,6 +2215,7 @@ impl ManaProduction { | ManaProduction::AnyOneColor { count, .. } | ManaProduction::AnyCombination { count, .. } | ManaProduction::ChosenColor { count, .. } + | ManaProduction::NotedType { count } | ManaProduction::OpponentLandColors { count } | ManaProduction::AnyCombinationOfObjectColors { count, .. } | ManaProduction::AnyTypeProduceableBy { count, .. } @@ -2266,6 +2299,10 @@ impl<'de> serde::Deserialize<'de> for ManaProduction { #[serde(default)] fixed_alternative: Option, }, + NotedType { + #[serde(default = "default_quantity_one")] + count: QuantityExpr, + }, OpponentLandColors { #[serde(default = "default_quantity_one")] count: QuantityExpr, @@ -2348,6 +2385,9 @@ impl<'de> serde::Deserialize<'de> for ManaProduction { contribution, fixed_alternative, }, + ManaProductionHelper::NotedType { count } => { + ManaProduction::NotedType { count } + } ManaProductionHelper::OpponentLandColors { count } => { ManaProduction::OpponentLandColors { count } } @@ -6674,6 +6714,20 @@ pub struct CostPaidObjectSnapshot { pub lki: LKISnapshot, } +/// CR 106.1b + CR 400.7 + CR 602.2b (issue #6504): The mana type(s) spent to +/// pay one activated ability's own mana sub-cost, snapshotted onto +/// `ResolvedAbility::noted_mana_payment` at the moment that specific +/// activation reached the stack. `source_incarnation` is the source's +/// `GameObject::incarnation` at that same moment — a companion "note the +/// type of mana spent to pay this activation cost" effect (Jeweled Amulet) +/// must refuse to act if the object's live incarnation no longer matches +/// (CR 400.7: bounced/flickered since). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct NotedManaPayment { + pub types: Vec, + pub source_incarnation: u64, +} + /// CR 102.1 + CR 103.1: Seating direction relative to a player. The game's /// default turn order proceeds clockwise (CR 103.1); the next player in turn /// order is seated to the active player's left (CR 101.4). Thus walking @@ -12918,6 +12972,21 @@ pub enum Effect { RememberCard { target: TargetFilter, }, + /// CR 106.1b + CR 602.2b + CR 608.2c: Record the mana type(s) spent to pay + /// this resolving ability's OWN activation cost onto its source as + /// `ChosenAttribute::NotedManaSpent` ("Note the type of mana spent to pay + /// this activation cost" — Jeweled Amulet). Reads the + /// source's transient `mana_spent_to_activate` latch, which + /// `pay_ability_mana_cost_with_choices_excluding_and_parent` stamps at + /// activation-time payment (CR 602.2b: costs are paid before the ability + /// resolves) — never at resolution time. Doing the write here rather than + /// at payment time means a countered/removed-from-stack ability (which + /// never resolves) never notes anything, matching CR 608.2c ("follows its + /// instructions" only on resolution). No fields: unlike `RememberCard`, + /// there is nothing to select — the noted value is a readback of a payment + /// that already happened, not a choice among candidates. Replace-on- + /// rechoose: removes any prior `NotedManaSpent` before pushing. + NoteManaSpent, /// CR 608.2c + CR 105.1: For each member of a fixed category (the five colors, /// or CR 205.2a card types), perform a per-member action referencing the /// bound member ("that color/type"). Iterates members in printed order, @@ -15426,7 +15495,10 @@ impl Effect { | Effect::CreateDrawReplacement { .. } // CR 614.1a: CreatePlaneswalkReplacement is non-targeted — "a player // would planeswalk" scopes via the shield's player scope, no slot. - | Effect::CreatePlaneswalkReplacement { .. } => None, + | Effect::CreatePlaneswalkReplacement { .. } + // CR 106.1b: NoteManaSpent has no target field — it reads back a + // payment already made on its own source, nothing to target. + | Effect::NoteManaSpent => None, // CR 115.1 + CR 601.2c: "two target players each reveal the top card of // their library" (Parker Luck) needs a stack-time player target slot so // the multi_target spec expands to one slot per revealer. Scoped to the @@ -16072,6 +16144,7 @@ impl Effect { | Effect::GrantCastingPermission { .. } | Effect::ChooseFromZone { .. } | Effect::RememberCard { .. } + | Effect::NoteManaSpent | Effect::ChooseObjectsIntoTrackedSet { .. } | Effect::EachPlayerCopyChosen { .. } | Effect::Exploit { .. } @@ -16304,6 +16377,7 @@ impl Effect { | Effect::ChooseDamageSource { .. } | Effect::ChooseFromZone { .. } | Effect::RememberCard { .. } + | Effect::NoteManaSpent | Effect::ForEachCategory { .. } | Effect::ChooseObjectsIntoTrackedSet { .. } | Effect::ChooseOneOf { .. } @@ -16561,6 +16635,7 @@ impl Effect { | Effect::ChooseDamageSource { .. } | Effect::ChooseFromZone { .. } | Effect::RememberCard { .. } + | Effect::NoteManaSpent | Effect::ForEachCategory { .. } | Effect::ChooseObjectsIntoTrackedSet { .. } | Effect::ChooseOneOf { .. } @@ -16813,6 +16888,7 @@ pub fn effect_variant_name(effect: &Effect) -> &str { Effect::GrantCastingPermission { .. } => "GrantCastingPermission", Effect::ChooseFromZone { .. } => "ChooseFromZone", Effect::RememberCard { .. } => "RememberCard", + Effect::NoteManaSpent => "NoteManaSpent", Effect::ForEachCategory { .. } => "ForEachCategory", Effect::ChooseObjectsIntoTrackedSet { .. } => "ChooseObjectsIntoTrackedSet", Effect::ChooseAndSacrificeRest { .. } => "ChooseAndSacrificeRest", @@ -17058,6 +17134,7 @@ pub enum EffectKind { GrantCastingPermission, ChooseFromZone, RememberCard, + NoteManaSpent, ChooseObjectsIntoTrackedSet, ChooseCounterKind, PutChosenCounter, @@ -17330,6 +17407,7 @@ impl From<&Effect> for EffectKind { Effect::GrantCastingPermission { .. } => EffectKind::GrantCastingPermission, Effect::ChooseFromZone { .. } => EffectKind::ChooseFromZone, Effect::RememberCard { .. } => EffectKind::RememberCard, + Effect::NoteManaSpent => EffectKind::NoteManaSpent, // The per-member iteration parks `ChooseFromZoneChoice` prompts and // emits `ChooseFromZone` resolution events; it shares the kind. Effect::ForEachCategory { @@ -23306,6 +23384,23 @@ pub struct ResolvedAbility { /// inherently single-object even when the cost consumed several. #[serde(default, skip_serializing_if = "Option::is_none")] pub cost_paid_object: Option, + /// CR 106.1b + CR 400.7 + CR 602.2b (issue #6504): The mana type(s) spent + /// to pay THIS resolving ability's own mana sub-cost, plus the source's + /// incarnation at the moment this activation reached the stack. Captured + /// exactly once, synchronously, by `push_ability_entry` (the single + /// authority where an activated ability reaches the stack) immediately + /// after cost payment completes — before any later activation of the + /// SAME permanent could occur. Unlike a per-object mutable latch, this + /// snapshot travels with THIS activation instance, so a permanent + /// untapped and reactivated (with a different payment) while this + /// ability still sits unresolved on the stack cannot corrupt what this + /// instance observed. Read by `Effect::NoteManaSpent` ("note the type of + /// mana spent to pay this activation cost" — Jeweled Amulet), which also + /// compares `source_incarnation` against the object's live incarnation + /// before writing (CR 400.7: a bounced/flickered source is a new object + /// with no memory of this activation's payment). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub noted_mana_payment: Option, /// CR 601.2h + CR 602.2b (issue #4948): EVERY object paid as /// part of this resolving ability's own cost — unlike `cost_paid_object` /// above, not just the first. This engine pays non-self @@ -23459,6 +23554,7 @@ impl ResolvedAbility { starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, @@ -23989,6 +24085,44 @@ impl ResolvedAbility { } } + /// CR 106.1b + CR 400.7 + CR 602.2b (issue #6504): Stamp this activation's + /// noted-mana-payment snapshot across this ability and every sub/else + /// branch — mirrors `set_cost_paid_object_recursive`. Necessary because + /// `Effect::NoteManaSpent` is typically chained as a `sub_ability` (e.g. + /// Jeweled Amulet: `PutCounter { sub_ability: NoteManaSpent }`), which + /// resolves as its OWN separate `ResolvedAbility` node distinct from the + /// top-level ability `push_ability_entry` captured the payment onto; + /// without this recursive stamp the sub-ability would read the field's + /// `None` default and silently note nothing. + pub fn set_noted_mana_payment_recursive(&mut self, payment: NotedManaPayment) { + self.noted_mana_payment = Some(payment.clone()); + if let Some(sub) = self.sub_ability.as_mut() { + sub.set_noted_mana_payment_recursive(payment.clone()); + } + if let Some(else_branch) = self.else_ability.as_mut() { + else_branch.set_noted_mana_payment_recursive(payment); + } + } + + /// CR 707.10 (issue #6504): Clear a noted-mana-payment snapshot across + /// this ability and every sub/else branch. A copy of an activated + /// ability is not itself activated, so it never paid a mana cost — a + /// naive struct clone otherwise carries the ORIGINAL activation's + /// payment along, and `Effect::NoteManaSpent` resolving on the copy + /// would falsely note mana the copy never spent. Called when normalizing + /// a copied activated/triggered ability (`preserve_ability_copy_source_ + /// recursive`), mirroring `set_noted_mana_payment_recursive`'s recursion + /// shape in reverse. + pub fn clear_noted_mana_payment_recursive(&mut self) { + self.noted_mana_payment = None; + if let Some(sub) = self.sub_ability.as_mut() { + sub.clear_noted_mana_payment_recursive(); + } + if let Some(else_branch) = self.else_ability.as_mut() { + else_branch.clear_noted_mana_payment_recursive(); + } + } + /// CR 601.2h + CR 602.2b (issue #4948): Record EVERY object /// paid as part of this ability's own cost (mirrors /// `set_cost_paid_object_recursive`'s recursion into `sub_ability` / diff --git a/crates/engine/src/types/ability_visit.rs b/crates/engine/src/types/ability_visit.rs index 432416a88a..72fc54bb80 100644 --- a/crates/engine/src/types/ability_visit.rs +++ b/crates/engine/src/types/ability_visit.rs @@ -570,6 +570,7 @@ where | Effect::GrantCastingPermission { .. } | Effect::ChooseFromZone { .. } | Effect::RememberCard { .. } + | Effect::NoteManaSpent | Effect::ForEachCategory { .. } | Effect::ChooseObjectsIntoTrackedSet { .. } | Effect::ChooseAndSacrificeRest { .. } diff --git a/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs b/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs new file mode 100644 index 0000000000..d335e8d66a --- /dev/null +++ b/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs @@ -0,0 +1,613 @@ +//! Regression for GitHub issue #6504 — Jeweled Amulet's first ability places a +//! charge counter but must also note the mana type spent to pay its own {1} +//! activation cost (CR 106.1b), and its second (mana) ability must read that +//! noted type back to produce matching mana (CR 106.5: no noted type, no +//! mana). Before the fix, both clauses fell through to `Effect::Unimplemented` +//! and the second ability produced no mana at all regardless of what was +//! noted. + +use engine::game::effects::{bounce, copy_spell}; +use engine::game::scenario::{GameScenario, P0}; +use engine::types::ability::{ + BounceSelection, CopyRetargetPermission, Effect, ResolvedAbility, TargetFilter, TargetRef, +}; +use engine::types::actions::GameAction; +use engine::types::counter::CounterType; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaType, ManaUnit}; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const JEWELED_AMULET_ORACLE: &str = "{1}, {T}: Put a charge counter on this artifact. \ +Note the type of mana spent to pay this activation cost. Activate only if there are no \ +charge counters on this artifact.\n\ +{T}, Remove a charge counter from this artifact: Add one mana of this artifact's last \ +noted type."; + +const CHARGE: fn() -> CounterType = || CounterType::Generic("charge".to_string()); + +fn charge_counters(runner: &engine::game::scenario::GameRunner, obj: ObjectId) -> u32 { + runner + .state() + .objects + .get(&obj) + .and_then(|o| o.counters.get(&CHARGE()).copied()) + .unwrap_or(0) +} + +fn pool_color( + runner: &engine::game::scenario::GameRunner, + player: engine::types::player::PlayerId, + color: ManaType, +) -> usize { + runner + .state() + .players + .iter() + .find(|p| p.id == player) + .map(|p| p.mana_pool.count_color(color)) + .unwrap_or(0) +} + +fn pool_total( + runner: &engine::game::scenario::GameRunner, + player: engine::types::player::PlayerId, +) -> usize { + runner + .state() + .players + .iter() + .find(|p| p.id == player) + .map(|p| p.mana_pool.total()) + .unwrap_or(0) +} + +/// Activates ability 0 ({1}, {T}: put a counter, note the paid type) funded by +/// exactly one mana unit of `color`, then untaps the amulet so ability 1's own +/// {T} cost isn't blocked by ability 0's tap (they are unrelated costs on the +/// same permanent). +fn activate_note_ability( + runner: &mut engine::game::scenario::GameRunner, + amulet: ObjectId, + color: ManaType, +) { + runner + .state_mut() + .players + .iter_mut() + .find(|p| p.id == P0) + .unwrap() + .mana_pool + .add(ManaUnit::new(color, ObjectId(0), false, vec![])); + runner.activate(amulet, 0).resolve(); + runner + .state_mut() + .objects + .get_mut(&amulet) + .expect("amulet must exist") + .tapped = false; +} + +#[test] +fn jeweled_amulet_notes_and_produces_matching_mana_type() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let amulet = scenario + .add_creature_from_oracle(P0, "Jeweled Amulet", 0, 0, JEWELED_AMULET_ORACLE) + .as_artifact() + .id(); + let mut runner = scenario.build(); + + activate_note_ability(&mut runner, amulet, ManaType::Red); + assert_eq!( + charge_counters(&runner, amulet), + 1, + "first ability must place exactly one charge counter" + ); + + runner + .act(GameAction::ActivateAbility { + source_id: amulet, + ability_index: 1, + }) + .expect("activate the mana ability (CR 605.3b: resolves immediately, no stack)"); + + assert_eq!( + pool_color(&runner, P0, ManaType::Red), + 1, + "the noted type (red) must be produced, not zero mana" + ); + assert_eq!( + charge_counters(&runner, amulet), + 0, + "the mana ability must remove the charge counter it noted the type from" + ); +} + +/// Same round trip with a different paid color, proving the produced type +/// tracks whatever was actually spent rather than being hardcoded to one +/// color (CR 106.1b names white/blue/black/red/green/colorless — the noted +/// value must be read back verbatim, not defaulted). +#[test] +fn jeweled_amulet_tracks_a_different_noted_color() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let amulet = scenario + .add_creature_from_oracle(P0, "Jeweled Amulet", 0, 0, JEWELED_AMULET_ORACLE) + .as_artifact() + .id(); + let mut runner = scenario.build(); + + activate_note_ability(&mut runner, amulet, ManaType::Green); + + runner + .act(GameAction::ActivateAbility { + source_id: amulet, + ability_index: 1, + }) + .expect("activate the mana ability"); + + assert_eq!( + pool_color(&runner, P0, ManaType::Green), + 1, + "green was spent to pay the first ability, so green must be produced" + ); + assert_eq!( + pool_color(&runner, P0, ManaType::Red), + 0, + "no red should appear when green was the noted type" + ); +} + +/// CR 106.5: an ability that would produce mana of an undefined type produces +/// no mana instead. A freshly entered amulet (or one whose first ability was +/// never activated) has nothing noted, so the second ability must add zero +/// mana — never silently defaulting to some fixed color. +#[test] +fn jeweled_amulet_produces_no_mana_with_nothing_noted() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let amulet = scenario + .add_creature_from_oracle(P0, "Jeweled Amulet", 0, 0, JEWELED_AMULET_ORACLE) + .as_artifact() + .id(); + // A charge counter is placed directly (bypassing ability 0 entirely) so + // ability 1's "remove a charge counter" cost is payable while nothing has + // ever been noted on this incarnation of the object. + scenario.with_counter(amulet, CHARGE(), 1); + let mut runner = scenario.build(); + + assert_eq!( + pool_total(&runner, P0), + 0, + "reach-guard: pool must start empty" + ); + + runner + .act(GameAction::ActivateAbility { + source_id: amulet, + ability_index: 1, + }) + .expect("activate the mana ability"); + + assert_eq!( + pool_total(&runner, P0), + 0, + "CR 106.5: no noted type must produce no mana, not a default color" + ); + assert_eq!( + charge_counters(&runner, amulet), + 0, + "the charge counter is still removed even though no mana was produced" + ); +} + +/// CR 400.7: a source that leaves and returns while its OWN activation is +/// still unresolved on the stack becomes a new object at the same storage id +/// (see `GameObject::incarnation`). Bouncing Jeweled Amulet in response to +/// its own first ability — before that ability resolves — must NOT let the +/// note land on the new incarnation: that incarnation never paid anything +/// itself. Without the incarnation guard in `Effect::NoteManaSpent`, the +/// stale `mana_spent_to_activate` latch from the departed incarnation would +/// be promoted onto the returned card's `chosen_attributes` anyway. +#[test] +fn jeweled_amulet_bounced_mid_stack_does_not_note_on_new_incarnation() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let amulet = scenario + .add_creature_from_oracle(P0, "Jeweled Amulet", 0, 0, JEWELED_AMULET_ORACLE) + .as_artifact() + .id(); + scenario.with_mana_pool( + P0, + vec![ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])], + ); + let mut runner = scenario.build(); + + // (1) Activate ability 0. The {1} cost auto-pays from the single floating + // red unit with no ambiguity, so one action step lands the ability on the + // stack, unresolved, at the post-announcement Priority window. + runner + .act(GameAction::ActivateAbility { + source_id: amulet, + ability_index: 0, + }) + .expect("activating ability 0 must succeed"); + assert_eq!( + runner.state().stack.len(), + 1, + "reach-guard: the note ability must be sitting on the stack, unresolved" + ); + assert_eq!( + pool_total(&runner, P0), + 0, + "reach-guard: the activation cost must consume the red mana before the bounce" + ); + let incarnation_before = runner.state().objects[&amulet].incarnation; + + // (2) Interject: bounce the amulet through the real bounce resolver + // (not a raw zone-field flip) before its own ability resolves. + let bounce_ability = ResolvedAbility::new( + Effect::Bounce { + target: TargetFilter::Any, + destination: None, + selection: BounceSelection::Targeted, + }, + vec![TargetRef::Object(amulet)], + ObjectId(999), + P0, + ); + let mut events = Vec::new(); + bounce::resolve(runner.state_mut(), &bounce_ability, &mut events) + .expect("bouncing the amulet must succeed"); + assert_eq!( + runner.state().objects[&amulet].zone, + Zone::Hand, + "reach-guard: the amulet must actually be in hand now" + ); + assert!( + runner.state().objects[&amulet].incarnation > incarnation_before, + "reach-guard: the zone change must have bumped the object's incarnation" + ); + + // (3) CR 112.7a: the ability is independent of its departed source and + // still resolves. + runner.resolve_top(); + assert!( + runner.state().stack.is_empty(), + "the note ability must have resolved off the stack" + ); + + // (4) The new incarnation, sitting in hand, must have nothing noted. + assert!( + runner.state().objects[&amulet].noted_mana_spent().is_none(), + "a bounced-and-returned amulet must not inherit the departed \ + incarnation's payment as a noted type" + ); +} + +/// CR 608.2c: an ability's instructions are followed only when it RESOLVES, +/// not when it's activated/paid for. Activating ability 0 and leaving it +/// sitting unresolved on the stack must not produce a durable +/// `ChosenAttribute::NotedManaSpent` yet — the write happens inside +/// `Effect::NoteManaSpent`'s resolution, never at payment time. (A countered +/// ability never reaches this resolution step at all, so this is also the +/// discriminating half of the "a countered ability never notes anything" +/// claim in `note_mana_spent.rs`'s doc comment.) +#[test] +fn jeweled_amulet_notes_nothing_before_resolution() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let amulet = scenario + .add_creature_from_oracle(P0, "Jeweled Amulet", 0, 0, JEWELED_AMULET_ORACLE) + .as_artifact() + .id(); + scenario.with_mana_pool( + P0, + vec![ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])], + ); + let mut runner = scenario.build(); + + runner + .act(GameAction::ActivateAbility { + source_id: amulet, + ability_index: 0, + }) + .expect("activating ability 0 must succeed"); + assert_eq!( + runner.state().stack.len(), + 1, + "reach-guard: the note ability must be sitting on the stack, unresolved" + ); + + assert!( + runner.state().objects[&amulet].noted_mana_spent().is_none(), + "an unresolved activation must not have written a durable note yet" + ); +} + +/// "The last noted type" (singular) must REPLACE, not append: two note +/// cycles on the SAME object, each fully resolved before the next begins, +/// must leave exactly one `ChosenAttribute::NotedManaSpent` reflecting only +/// the most recent payment. +#[test] +fn jeweled_amulet_second_note_replaces_not_appends() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let amulet = scenario + .add_creature_from_oracle(P0, "Jeweled Amulet", 0, 0, JEWELED_AMULET_ORACLE) + .as_artifact() + .id(); + let mut runner = scenario.build(); + + // First note cycle: pay red, resolve fully. + activate_note_ability(&mut runner, amulet, ManaType::Red); + assert_eq!( + runner.state().objects[&amulet].noted_mana_spent(), + Some([ManaType::Red].as_slice()), + "reach-guard: the first cycle must have noted red" + ); + + // Remove the charge counter (ability 1) so ability 0 is activatable + // again, and drain the mana it produces so it doesn't interfere with + // the pool assertions below. + runner + .act(GameAction::ActivateAbility { + source_id: amulet, + ability_index: 1, + }) + .expect("removing the charge counter must succeed"); + runner.state_mut().players[P0.0 as usize] + .mana_pool + .mana + .clear(); + // Ability 1 taps the amulet as part of its own {T} cost; untap it so the + // second `activate_note_ability` call can pay ability 0's own {T} cost. + runner + .state_mut() + .objects + .get_mut(&amulet) + .expect("amulet must exist") + .tapped = false; + assert_eq!( + charge_counters(&runner, amulet), + 0, + "reach-guard: ability 0 must be activatable again" + ); + + // Second note cycle: pay green, resolve fully. + activate_note_ability(&mut runner, amulet, ManaType::Green); + + let noted = runner.state().objects[&amulet].noted_mana_spent(); + assert_eq!( + noted, + Some([ManaType::Green].as_slice()), + "the second note must REPLACE the first, not append to it \ + (got {noted:?})" + ); +} + +/// Issue #6504 review: the payment latch this effect reads must be captured +/// PER ACTIVATION, not as a single mutable per-object field a later +/// activation can overwrite. Sequence (all legal under CR 602.5: "Activate +/// only if there are no charge counters" is checked at ACTIVATION time, and +/// no charge counter exists yet while the first note ability is still +/// unresolved on the stack): +/// +/// 1. Activate ability 0 paying RED. It goes on the stack, unresolved. +/// 2. Something untaps the amulet (simulated directly — CR-correctness of +/// the untap effect itself is not what this test is about). +/// 3. Activate ability 0 AGAIN, paying GREEN. LIFO: this sits ABOVE the +/// first activation. +/// 4. Resolve the top (green activation) first — notes green, first +/// charge counter placed. +/// 5. Resolve the remaining (red activation) second — must note RED, its +/// OWN payment, not a bled-through read of the green activation's +/// (later, and by then long-overwritten) payment. +/// +/// Revert-probe: reading a shared `GameObject`-level mutable latch at +/// resolution time (rather than a per-activation `ResolvedAbility` snapshot) +/// makes step 5 observe green instead of red, since both activations' cost +/// payments write through the SAME field and the red activation resolves +/// after the field was already overwritten by the green activation's +/// payment. +#[test] +fn jeweled_amulet_lifo_stacked_activations_each_note_their_own_payment() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let amulet = scenario + .add_creature_from_oracle(P0, "Jeweled Amulet", 0, 0, JEWELED_AMULET_ORACLE) + .as_artifact() + .id(); + scenario.with_mana_pool( + P0, + vec![ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])], + ); + let mut runner = scenario.build(); + + // (1) Activate paying red. On the stack, unresolved. + runner + .act(GameAction::ActivateAbility { + source_id: amulet, + ability_index: 0, + }) + .expect("first activation (red) must succeed"); + assert_eq!(runner.state().stack.len(), 1); + + // (2) Simulate an external untap effect (e.g. Puppet Strings) resolving + // in response — CR-correctness of untapping itself is out of scope here. + runner + .state_mut() + .objects + .get_mut(&amulet) + .expect("amulet must exist") + .tapped = false; + + // (3) Activate again, paying green. LIFO: stacks ABOVE the red one. + runner + .state_mut() + .players + .iter_mut() + .find(|p| p.id == P0) + .unwrap() + .mana_pool + .add(ManaUnit::new(ManaType::Green, ObjectId(0), false, vec![])); + runner + .act(GameAction::ActivateAbility { + source_id: amulet, + ability_index: 0, + }) + .expect("second activation (green), still no charge counter yet, must succeed"); + assert_eq!( + runner.state().stack.len(), + 2, + "reach-guard: both note activations must be on the stack at once" + ); + + // (4) Resolve the top (green) first. + runner.resolve_top(); + assert_eq!(runner.state().stack.len(), 1); + assert_eq!( + runner.state().objects[&amulet].noted_mana_spent(), + Some([ManaType::Green].as_slice()), + "the green activation must note its own (green) payment" + ); + assert_eq!( + charge_counters(&runner, amulet), + 1, + "reach-guard: the green activation's PutCounter must have resolved" + ); + + // (5) Resolve the remaining (red) activation second. + runner.resolve_top(); + assert!(runner.state().stack.is_empty()); + assert_eq!( + runner.state().objects[&amulet].noted_mana_spent(), + Some([ManaType::Red].as_slice()), + "the red activation must note ITS OWN (red) payment, not the \ + green activation's — a shared mutable per-object latch would leak \ + green into this resolution instead" + ); +} + +/// CR 707.10: a copy of an activated ability is not itself activated, so it +/// never paid a mana cost. Copies Jeweled Amulet's first ability (through the +/// real `copy_spell::resolve` pipeline — same stack-entry lookup and LIFO +/// resolution a real "copy target activated or triggered ability" card like +/// Lithoform Engine drives) after the ORIGINAL paid red, and proves the copy +/// cannot note red (or anything) even though its `ResolvedAbility` chain was +/// cloned from an original that carried a live `noted_mana_payment` +/// snapshot. Both PutCounter placements still fire unconditionally (CR +/// 602.5/608.2c: "Activate only if..." gates ACTIVATION, never re-checked at +/// resolution, and a copy was never gated by it in the first place), so the +/// counter count alone can't distinguish correct from buggy behavior here — +/// only `noted_mana_spent()` can. +#[test] +fn jeweled_amulet_copied_activation_does_not_note_original_payment() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let amulet = scenario + .add_creature_from_oracle(P0, "Jeweled Amulet", 0, 0, JEWELED_AMULET_ORACLE) + .as_artifact() + .id(); + scenario.with_mana_pool( + P0, + vec![ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])], + ); + let mut runner = scenario.build(); + + // (1) Activate ability 0 paying red. On the stack, unresolved. + runner + .act(GameAction::ActivateAbility { + source_id: amulet, + ability_index: 0, + }) + .expect("activating ability 0 must succeed"); + let original_entry_id = runner + .state() + .stack + .back() + .expect("reach-guard: the activation must be on the stack") + .id; + let original_payment = runner + .state() + .stack + .iter() + .find(|entry| entry.id == original_entry_id) + .and_then(|entry| entry.ability()) + .and_then(|ability| ability.noted_mana_payment.as_ref()) + .expect("reach-guard: the original activation must retain its payment snapshot"); + assert_eq!( + original_payment.types, + vec![ManaType::Red], + "reach-guard: the original activation must carry its paid red mana" + ); + + // (2) Copy it — mirrors what Lithoform Engine's "{2}, {T}: Copy target + // activated or triggered ability you control" drives through the real + // engine pipeline, minus the copying permanent's own stack presence + // (the mechanism under test, `copy_spell::resolve` and its + // `preserve_ability_copy_source_recursive` call, is identical either + // way — this is the same direct-resolver-call idiom already used above + // for `bounce::resolve`). + let copy_ability = ResolvedAbility::new( + Effect::CopySpell { + target: TargetFilter::StackAbility { + controller: None, + tag: None, + kind: None, + }, + retarget: CopyRetargetPermission::KeepOriginalTargets, + copier: None, + additional_modifications: Vec::new(), + starting_loyalty_from_casualty_sacrifice: false, + }, + vec![TargetRef::Object(original_entry_id)], + ObjectId(999), + P0, + ); + let mut events = Vec::new(); + copy_spell::resolve(runner.state_mut(), ©_ability, &mut events) + .expect("copying the activation must succeed"); + assert_eq!( + runner.state().stack.len(), + 2, + "reach-guard: original + copy must both be on the stack" + ); + + // (3) LIFO: the copy resolves first. + runner.resolve_top(); + assert_eq!( + runner.state().stack.len(), + 1, + "reach-guard: exactly the original must remain" + ); + assert_eq!( + charge_counters(&runner, amulet), + 1, + "the copy's PutCounter still fires unconditionally" + ); + assert!( + runner.state().objects[&amulet].noted_mana_spent().is_none(), + "CR 707.10: the copy never paid a mana cost, so its NoteManaSpent \ + must not note the original's (red) payment" + ); + + // (4) The original resolves second — it DID pay, so it must still note + // correctly. This is the paired positive reach-guard proving the fix + // clears only the COPY's snapshot, not the original's. + runner.resolve_top(); + assert!(runner.state().stack.is_empty()); + assert_eq!( + charge_counters(&runner, amulet), + 2, + "the original's PutCounter also fires (a second charge counter — \ + CR 602.5: the no-counters restriction gates activation, not \ + resolution, and was never re-checked for the copy either)" + ); + assert_eq!( + runner.state().objects[&amulet].noted_mana_spent(), + Some([ManaType::Red].as_slice()), + "the original activation must still note its own (red) payment" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 80c1e5e2d8..2f0792f0ec 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -648,6 +648,7 @@ mod issue_6477_wandering_archaic_optional_payment; mod issue_6498_portent_of_calamity; mod issue_6499_flickering_ward_protection_exemption; mod issue_6500_loreseekers_stone_hand_cost; +mod issue_6504_jeweled_amulet_noted_mana; mod issue_654_stridehangar_automaton; mod issue_6566_granted_leave_exile; mod issue_6634_aven_courier; diff --git a/crates/engine/tests/integration/oracle_parser.rs b/crates/engine/tests/integration/oracle_parser.rs index d919062eca..d0b163c46d 100644 --- a/crates/engine/tests/integration/oracle_parser.rs +++ b/crates/engine/tests/integration/oracle_parser.rs @@ -935,3 +935,177 @@ fn brood_birthing_grant_static_unchanged_by_masking() { result.statics[0] ); } + +/// Issue #6504: Jeweled Amulet's "note the type of mana spent to pay this +/// activation cost" / "add one mana of this artifact's last noted type" pair +/// must parse to the typed `Effect::NoteManaSpent` / `ManaProduction::NotedType` +/// building block, not fall through to `Effect::Unimplemented`. +#[test] +fn jeweled_amulet_notes_and_reads_back_mana_type() { + use engine::types::ability::{ManaProduction, QuantityExpr}; + + let result = parse( + "{1}, {T}: Put a charge counter on this artifact. Note the type of mana \ + spent to pay this activation cost. Activate only if there are no charge \ + counters on this artifact.\n{T}, Remove a charge counter from this \ + artifact: Add one mana of this artifact's last noted type.", + "Jeweled Amulet", + &[], + &["Artifact"], + &[], + ); + + assert_eq!( + result.abilities.len(), + 2, + "Jeweled Amulet has two top-level activated abilities: {:#?}", + result.abilities + ); + + let note_ability = result + .abilities + .iter() + .find(|a| matches!(&*a.effect, Effect::PutCounter { .. })) + .unwrap_or_else(|| panic!("no PutCounter ability parsed: {:#?}", result.abilities)); + let note_sub = note_ability + .sub_ability + .as_deref() + .unwrap_or_else(|| panic!("PutCounter has no note sub-ability: {note_ability:#?}")); + assert!( + matches!(&*note_sub.effect, Effect::NoteManaSpent), + "expected Effect::NoteManaSpent, got {:#?}", + note_sub.effect + ); + + let add_ability = result + .abilities + .iter() + .find(|a| matches!(&*a.effect, Effect::Mana { .. })) + .unwrap_or_else(|| panic!("no Mana ability parsed: {:#?}", result.abilities)); + match &*add_ability.effect { + Effect::Mana { produced, .. } => match produced { + ManaProduction::NotedType { count } => { + assert_eq!( + count, + &QuantityExpr::Fixed { value: 1 }, + "expected 'one mana' to parse as count=1" + ); + } + other => panic!("expected ManaProduction::NotedType, got {other:#?}"), + }, + other => unreachable!("filtered to Effect::Mana above, got {other:#?}"), + } +} + +/// Issue #6504: the note clause is a composed grammar (article + cost-referent +/// axes independently optional/variable), not a Jeweled-Amulet-shaped sentence +/// tag — a hypothetical sibling printing the articleless "note type" and the +/// shorter "this cost" (rather than "this activation cost") must reach +/// `Effect::NoteManaSpent` with no new parser arm. `Amber Amulet` here is not +/// a real printed card; it exercises the grammar's structural variants that +/// `parse_note_mana_spent_clause`'s unit tests cover in isolation, end to end +/// through the full Oracle-text pipeline. +#[test] +fn note_mana_spent_grammar_accepts_a_hypothetical_sibling_wording() { + let result = parse( + "{1}, {T}: Put a charge counter on this artifact. Note type of mana \ + spent to pay this cost. Activate only if there are no charge \ + counters on this artifact.", + "Amber Amulet", + &[], + &["Artifact"], + &[], + ); + + let note_ability = result + .abilities + .iter() + .find(|a| matches!(&*a.effect, Effect::PutCounter { .. })) + .unwrap_or_else(|| panic!("no PutCounter ability parsed: {:#?}", result.abilities)); + let note_sub = note_ability + .sub_ability + .as_deref() + .unwrap_or_else(|| panic!("PutCounter has no note sub-ability: {note_ability:#?}")); + assert!( + matches!(&*note_sub.effect, Effect::NoteManaSpent), + "expected Effect::NoteManaSpent for the articleless/shorter-cost \ + sibling wording, got {:#?}", + note_sub.effect + ); +} + +/// Issue #6504 (coverage-honesty guard): Ice Cauldron prints Jeweled Amulet's +/// sibling "note the type AND AMOUNT of mana spent..." / "add ... last noted +/// type and amount of mana" pair, which `parse_note_mana_spent_clause` and +/// `ManaProduction::NotedType` deliberately do not model (see the doc comment +/// on `parse_note_mana_spent_clause`). A production-parser assertion — not +/// just the isolated `parse_note_mana_spent_clause` unit tests — is required +/// here: it proves the parser actually REACHES both noted-mana clauses +/// (rather than failing earlier in the sentence for an unrelated reason) and +/// that each still falls through to `Effect::Unimplemented`, so an upstream +/// routing/fallback change can't silently start reporting Ice Cauldron as +/// supported or partially supported. Paired with +/// `jeweled_amulet_notes_and_reads_back_mana_type`'s positive full-Oracle +/// assertion that the same grammar area reaches `Effect::NoteManaSpent` for +/// Jeweled Amulet's singular-type wording. +#[test] +fn ice_cauldron_note_type_and_amount_stays_unimplemented() { + let result = parse( + "{X}, {T}: You may exile a nonland card from your hand. You may cast that \ + card for as long as it remains exiled. Put a charge counter on this \ + artifact and note the type and amount of mana spent to pay this \ + activation cost. Activate only if there are no charge counters on this \ + artifact.\n{T}, Remove a charge counter from this artifact: Add this \ + artifact's last noted type and amount of mana. Spend this mana only to \ + cast the last card exiled with this artifact.", + "Ice Cauldron", + &[], + &["Artifact"], + &[], + ); + + // First ability: exile -> cast -> put counter -> note (type and amount). + // The note clause is reached only by walking through three chained + // sub-abilities, proving the parser gets all the way to the noted-mana + // subject rather than failing earlier for an unrelated reason. + let exile_ability = result + .abilities + .iter() + .find(|a| matches!(&*a.effect, Effect::ChangeZone { .. })) + .unwrap_or_else(|| panic!("no exile ability parsed: {:#?}", result.abilities)); + let cast_sub = exile_ability + .sub_ability + .as_deref() + .unwrap_or_else(|| panic!("exile has no cast sub-ability: {exile_ability:#?}")); + let counter_sub = cast_sub + .sub_ability + .as_deref() + .unwrap_or_else(|| panic!("cast has no counter sub-ability: {cast_sub:#?}")); + let note_sub = counter_sub + .sub_ability + .as_deref() + .unwrap_or_else(|| panic!("counter has no note sub-ability: {counter_sub:#?}")); + assert!( + matches!(&*note_sub.effect, Effect::Unimplemented { name, .. } if name == "note"), + "Ice Cauldron's 'type AND amount' noted-mana subject must stay \ + Unimplemented, not be swallowed by parse_note_mana_spent_clause's \ + singular-type grammar; got {:#?}", + note_sub.effect + ); + + // Second ability: the mana-producing "add ... last noted type and amount + // of mana" must not be matched by `ManaProduction::NotedType`'s + // singular-type pattern. + let mana_ability = result + .abilities + .iter() + .find(|a| matches!(&*a.effect, Effect::Unimplemented { name, .. } if name == "add")) + .unwrap_or_else(|| panic!("no 'add' mana ability parsed: {:#?}", result.abilities)); + assert!( + matches!(&*mana_ability.effect, Effect::Unimplemented { .. }), + "Ice Cauldron's 'last noted type and amount of mana' must stay \ + Unimplemented, not be matched by ManaProduction::NotedType's \ + singular-type pattern; got {:#?}", + mana_ability.effect + ); +} diff --git a/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs b/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs index c3cb0345ab..b54860f1d5 100644 --- a/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs +++ b/crates/engine/tests/integration/the_chain_veil_loyalty_grants.rs @@ -176,6 +176,7 @@ fn make_grant_ability(controller: PlayerId, source: ObjectId) -> ResolvedAbility starting_with: None, chosen_x: None, cost_paid_object: None, + noted_mana_payment: None, cost_paid_object_ids: Vec::new(), effect_context_object: None, amassed_army_object: None, diff --git a/crates/mtgish-import/src/convert/action.rs b/crates/mtgish-import/src/convert/action.rs index 45d6cd851b..bea9f8e2fc 100644 --- a/crates/mtgish-import/src/convert/action.rs +++ b/crates/mtgish-import/src/convert/action.rs @@ -311,6 +311,7 @@ fn rewrite_bound_x_in_mana_production( | ManaProduction::AnyCombination { count, .. } | ManaProduction::AnyCombinationOfObjectColors { count, .. } | ManaProduction::ChosenColor { count, .. } + | ManaProduction::NotedType { count } | ManaProduction::OpponentLandColors { count } | ManaProduction::AnyTypeProduceableBy { count, .. } | ManaProduction::AnyInCommandersColorIdentity { count, .. } => { diff --git a/crates/phase-ai/baselines/perf-baseline.json b/crates/phase-ai/baselines/perf-baseline.json index a975681cfd..e4dea495af 100644 --- a/crates/phase-ai/baselines/perf-baseline.json +++ b/crates/phase-ai/baselines/perf-baseline.json @@ -1,7 +1,7 @@ { "schema_version": 4, - "git_sha": "7f5f0b4a3656", - "card_data_hash": "670a4a14a501f5ae5df43f832676c4556104234d", + "git_sha": "dd95f1c809ed", + "card_data_hash": "d11502ce500bf1ed0392287cc0376248f37f993d", "base_seed": 2654435769, "action_cap": 3000, "sample_count": 5, @@ -11,26 +11,26 @@ "enchantress-mirror" ], "counters": { - "attackable_player_sweeps": 2640, + "attackable_player_sweeps": 1349, "auto_tap_source_cache_builds": 10, "cached_auto_tap_source_rejects": 0, "cached_auto_tap_source_reuses": 37, "combat_shadow_block_scans": 0, - "crew_eligibility_scans": 11015, + "crew_eligibility_scans": 12877, "granted_ability_provider_scans": 0, - "layers_escalated": 73, - "layers_full_eval": 5464, - "layers_incremental": 395, + "layers_escalated": 79, + "layers_full_eval": 6475, + "layers_incremental": 398, "legal_actions_spell_cost_sweeps": 10, - "legend_rule_mode_gate_scans": 19703, - "mana_aura_trigger_scans": 40836, - "mana_display_sweeps": 247, - "mana_display_swept_objects": 2455, + "legend_rule_mode_gate_scans": 23051, + "mana_aura_trigger_scans": 67313, + "mana_display_sweeps": 277, + "mana_display_swept_objects": 2852, "priority_cast_probe_builds": 10, "restriction_static_exact_scans": 0, - "restriction_static_mode_gate_scans": 110891, - "sba_battlefield_snapshot_builds": 19626, - "sba_empty_battlefield_short_circuits": 31, + "restriction_static_mode_gate_scans": 193496, + "sba_battlefield_snapshot_builds": 22666, + "sba_empty_battlefield_short_circuits": 34, "spell_keyword_grant_scans": 0, "stack_batch_candidates": 0, "stack_batch_observer_refusals": 0, @@ -38,8 +38,8 @@ "stack_batched_entries": 0, "stack_inert_noop_batches": 0, "stack_inert_noop_entries": 0, - "state_clone_for_legality": 11099, + "state_clone_for_legality": 37244, "static_full_scans": 0 }, - "wall_clock_ms": 20942 -} \ No newline at end of file + "wall_clock_ms": 44423 +} diff --git a/crates/phase-ai/src/features/devotion.rs b/crates/phase-ai/src/features/devotion.rs index 8d7e7505cb..e8f6d4b5c2 100644 --- a/crates/phase-ai/src/features/devotion.rs +++ b/crates/phase-ai/src/features/devotion.rs @@ -344,6 +344,7 @@ fn mana_production_count( | MP::AnyOneColor { count, .. } | MP::AnyCombination { count, .. } | MP::ChosenColor { count, .. } + | MP::NotedType { count } | MP::OpponentLandColors { count } | MP::AnyCombinationOfObjectColors { count, .. } | MP::AnyTypeProduceableBy { count, .. } diff --git a/crates/phase-ai/src/mana_colors.rs b/crates/phase-ai/src/mana_colors.rs index 7b7c3e4bfc..3897d13ea1 100644 --- a/crates/phase-ai/src/mana_colors.rs +++ b/crates/phase-ai/src/mana_colors.rs @@ -83,6 +83,7 @@ pub(crate) fn collect_mana_production_colors( } ManaProduction::Colorless { .. } | ManaProduction::ChosenColor { .. } + | ManaProduction::NotedType { .. } | ManaProduction::OpponentLandColors { .. } | ManaProduction::AnyTypeProduceableBy { .. } | ManaProduction::ChoiceAmongExiledColors { .. } diff --git a/crates/phase-ai/src/policies/effect_classify.rs b/crates/phase-ai/src/policies/effect_classify.rs index ee7a0c4a18..0f2f921de8 100644 --- a/crates/phase-ai/src/policies/effect_classify.rs +++ b/crates/phase-ai/src/policies/effect_classify.rs @@ -321,6 +321,7 @@ pub(crate) fn effect_polarity(effect: &Effect) -> EffectPolarity { | Effect::Monstrosity { .. } | Effect::Myriad | Effect::NoOp + | Effect::NoteManaSpent | Effect::OpenAttractions { .. } | Effect::OpponentGuess { .. } | Effect::PairWith { .. } diff --git a/crates/phase-ai/src/policies/redundancy_avoidance.rs b/crates/phase-ai/src/policies/redundancy_avoidance.rs index dd1b50d804..cc14cbe07f 100644 --- a/crates/phase-ai/src/policies/redundancy_avoidance.rs +++ b/crates/phase-ai/src/policies/redundancy_avoidance.rs @@ -677,6 +677,7 @@ fn redundancy_delta( | Effect::PutSticker { .. } | Effect::ApplySticker { .. } | Effect::RememberCard { .. } + | Effect::NoteManaSpent // CR 608.2d + CR 122.1: the counter-kind choice + its consume carry no // static redundancy signal (the value depends on the runtime choice). | Effect::ChooseCounterKind { .. }