From 4555043a1d9c5a1f44235fd95b79588d9e12032f Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 30 Jul 2026 05:21:54 -0500 Subject: [PATCH 1/9] fix(engine): Jeweled Amulet notes and reproduces spent mana type (#6504) Jeweled Amulet's first ability placed a charge counter but never noted the mana type spent to pay its own activation cost, so the second ability ("Add one mana of this artifact's last noted type") always produced zero mana. Both clauses fell through to Effect::Unimplemented. Adds the noted-mana-type building block shared by this small card class (Jeweled Amulet, Ice Cauldron): a transient GameObject payment latch stamped at ability-mana-cost payment, a ChosenAttribute::NotedManaSpent slot written by a new Effect::NoteManaSpent at resolution (so a countered ability never notes anything), and a ManaProduction::NotedType variant that reads it back at CR 106.5-correct empty-set behavior. --- crates/engine/src/analysis/ability_graph.rs | 2 + crates/engine/src/game/ability_rw.rs | 2 + crates/engine/src/game/ability_scan.rs | 8 + crates/engine/src/game/casting.rs | 16 ++ crates/engine/src/game/casting_costs.rs | 5 + crates/engine/src/game/coverage.rs | 4 + crates/engine/src/game/effects/mana.rs | 32 +++ crates/engine/src/game/effects/mod.rs | 2 + .../src/game/effects/note_mana_spent.rs | 44 ++++ crates/engine/src/game/game_object.rs | 36 +++- crates/engine/src/game/mana_sources.rs | 21 ++ crates/engine/src/game/trigger_index.rs | 1 + .../src/parser/oracle_effect/imperative.rs | 44 ++++ .../engine/src/parser/oracle_effect/mana.rs | 14 ++ crates/engine/src/parser/oracle_effect/mod.rs | 1 + .../src/parser/oracle_effect/sequence.rs | 1 + crates/engine/src/parser/oracle_ir/ast.rs | 5 + crates/engine/src/parser/oracle_ir/doc.rs | 1 + crates/engine/src/parser/oracle_trigger.rs | 1 + crates/engine/src/types/ability.rs | 66 +++++- .../issue_6504_jeweled_amulet_noted_mana.rs | 198 ++++++++++++++++++ crates/engine/tests/integration/main.rs | 1 + .../engine/tests/integration/oracle_parser.rs | 61 ++++++ crates/mtgish-import/src/convert/action.rs | 1 + crates/phase-ai/src/features/devotion.rs | 1 + crates/phase-ai/src/mana_colors.rs | 1 + .../phase-ai/src/policies/effect_classify.rs | 1 + .../src/policies/redundancy_avoidance.rs | 1 + 28 files changed, 569 insertions(+), 2 deletions(-) create mode 100644 crates/engine/src/game/effects/note_mana_spent.rs create mode 100644 crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs 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..bdb24451c6 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 { .. } @@ -5654,6 +5655,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..3279492230 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -4999,6 +4999,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. diff --git a/crates/engine/src/game/casting.rs b/crates/engine/src/game/casting.rs index 08a11f133d..edd4850b9c 100644 --- a/crates/engine/src/game/casting.rs +++ b/crates/engine/src/game/casting.rs @@ -15577,6 +15577,22 @@ fn pay_ability_mana_cost_with_choices_excluding_and_parent( parent, )?; + // CR 106.1b + CR 602.2b: 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), so it is where a companion "note the type of + // mana spent to pay this activation cost" effect (Jeweled Amulet, Ice + // Cauldron) reads back what was spent — at that effect's OWN resolution, + // not here. Never cleared: overwritten by this source's next ability-mana- + // cost payment (see `GameObject::mana_spent_to_activate`). + 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..bc53acce4c 100644 --- a/crates/engine/src/game/casting_costs.rs +++ b/crates/engine/src/game/casting_costs.rs @@ -11400,6 +11400,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/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..07b1c4f787 --- /dev/null +++ b/crates/engine/src/game/effects/note_mana_spent.rs @@ -0,0 +1,44 @@ +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, Ice Cauldron). +/// +/// Composable building block: cost payment stays in the mana-payment funnel +/// (`pay_ability_mana_cost_with_choices_excluding_and_parent`, which stamps the +/// transient `GameObject::mana_spent_to_activate` latch); 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). +pub fn resolve( + state: &mut GameState, + ability: &ResolvedAbility, + _events: &mut Vec, +) -> Result<(), EffectError> { + let Effect::NoteManaSpent = &ability.effect else { + 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(()); + }; + let spent_types = src.mana_spent_to_activate.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/game_object.rs b/crates/engine/src/game/game_object.rs index 974f19aeb1..3d824bb64e 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,23 @@ pub struct GameObject { /// all rules queries. Defaults to `PhasedIn` for replay compatibility. #[serde(default)] pub phase_status: PhaseStatus, + + /// CR 106.1b + CR 602.2b: Mana type(s) spent to pay this object's own + /// most recent activated-ability mana cost, stamped by + /// `pay_ability_mana_cost_with_choices_excluding_and_parent` at + /// activation-time payment. Bridges cost payment (activation) to + /// `Effect::NoteManaSpent` (resolution) for "note the type of mana spent + /// to pay this activation cost" (Jeweled Amulet, Ice Cauldron). Transient: + /// overwritten by the object's next ability-mana-cost payment, with no + /// other clearing logic — safe because every known card in this class + /// gates its "note" ability behind a `{T}` cost component, so the same + /// source cannot pay a second activation's mana cost before the first + /// resolves and consumes this latch. Mirrors `colors_spent_to_cast`'s + /// cast-side stamp-then-read idiom, adapted for ability activation, where + /// the resolving stack entry is a separate `ResolvedAbility` rather than + /// this same object. + #[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 +1362,12 @@ 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: consumed + // synchronously by `Effect::NoteManaSpent` into the compared + // `chosen_attributes` accumulator, or overwritten by the object's + // next ability-mana-cost payment (§5.2c). + mana_spent_to_activate: _, } = o; } @@ -2212,6 +2235,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 +2683,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, Ice Cauldron). 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/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..80c3434924 100644 --- a/crates/engine/src/parser/oracle_ir/ast.rs +++ b/crates/engine/src/parser/oracle_ir/ast.rs @@ -672,6 +672,11 @@ pub(crate) enum ImperativeFamilyAst { Behold(TargetFilter), /// CR 701.48a: Learn. Learn, + /// CR 106.1b + CR 602.2b: "note the type [and amount] of mana spent to pay + /// this activation cost" (Jeweled Amulet, Ice Cauldron). Field-less: there + /// is nothing to select — the payment already happened, so the effect is + /// a pure readback recorded at resolution. + 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..9d8efab196 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, Ice Cauldron). 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 } } @@ -12918,6 +12958,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, Ice Cauldron). 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 +15481,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 +16130,7 @@ impl Effect { | Effect::GrantCastingPermission { .. } | Effect::ChooseFromZone { .. } | Effect::RememberCard { .. } + | Effect::NoteManaSpent | Effect::ChooseObjectsIntoTrackedSet { .. } | Effect::EachPlayerCopyChosen { .. } | Effect::Exploit { .. } @@ -16304,6 +16363,7 @@ impl Effect { | Effect::ChooseDamageSource { .. } | Effect::ChooseFromZone { .. } | Effect::RememberCard { .. } + | Effect::NoteManaSpent | Effect::ForEachCategory { .. } | Effect::ChooseObjectsIntoTrackedSet { .. } | Effect::ChooseOneOf { .. } @@ -16561,6 +16621,7 @@ impl Effect { | Effect::ChooseDamageSource { .. } | Effect::ChooseFromZone { .. } | Effect::RememberCard { .. } + | Effect::NoteManaSpent | Effect::ForEachCategory { .. } | Effect::ChooseObjectsIntoTrackedSet { .. } | Effect::ChooseOneOf { .. } @@ -16813,6 +16874,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 +17120,7 @@ pub enum EffectKind { GrantCastingPermission, ChooseFromZone, RememberCard, + NoteManaSpent, ChooseObjectsIntoTrackedSet, ChooseCounterKind, PutChosenCounter, @@ -17330,6 +17393,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 { 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..83d325e359 --- /dev/null +++ b/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs @@ -0,0 +1,198 @@ +//! 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::scenario::{GameScenario, P0}; +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; + +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" + ); +} 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..bc7cdc7aa7 100644 --- a/crates/engine/tests/integration/oracle_parser.rs +++ b/crates/engine/tests/integration/oracle_parser.rs @@ -935,3 +935,64 @@ 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:#?}"), + } +} 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/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 { .. } From 5a533ac70df6f68af7d8a5325d7b229f000626fd Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 30 Jul 2026 06:25:30 -0500 Subject: [PATCH 2/9] =?UTF-8?q?fix(engine):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20narrow=20noted-mana=20class,=20fix=20incarnation=20staleness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on PR #6812 (issue #6504): - The parser accepted Ice Cauldron's "note the type AND AMOUNT of mana spent..." wording and routed it through the same Effect::NoteManaSpent as Jeweled Amulet's singular-type wording, but the resolver only ever stores/reproduces the first noted type — it doesn't model Ice Cauldron's exact stored multiset or its spend restriction. Narrowed the parser (and every doc comment) to Jeweled Amulet's exact "note the type of mana spent..." wording only; Ice Cauldron's text is now intentionally left unmatched and still reports Unimplemented. This also fixes a parser-combinator-gate violation: the narrowed recognizer moved out of the `match first_word { "note" => .. }` string-literal dispatch (flagged as new bare-string-match-arm code) into an anchored nom `all_consuming` guard, mirroring the existing "end the turn" precedent in the same function. - `Effect::NoteManaSpent` read/wrote its source purely via `ability. source_id`, with no check that the object at that storage id was still the same incarnation that made the payment (CR 400.7). A source bounced or flickered while its OWN "note" ability sat unresolved on the stack would have the OLD incarnation's payment silently promoted onto the NEW incarnation's `chosen_attributes`. `GameObject::mana_spent_to_activate` is now paired with the incarnation captured at payment time (`mana_spent_to_activate_incarnation`); the resolver refuses to write unless the object's live incarnation still matches, mirroring the engine's existing incarnation-pairing idiom used by `ResolvedAbility::source_is_current` / `TargetFilter::SelfRef` resolution. Added a stack-level bounce regression test, verified revert-failing. --- crates/engine/src/game/casting.rs | 23 ++++-- .../src/game/effects/note_mana_spent.rs | 21 ++++- crates/engine/src/game/game_object.rs | 20 ++++- crates/engine/src/parser/oracle_ir/ast.rs | 10 ++- crates/engine/src/types/ability.rs | 4 +- .../issue_6504_jeweled_amulet_noted_mana.rs | 82 +++++++++++++++++++ 6 files changed, 142 insertions(+), 18 deletions(-) diff --git a/crates/engine/src/game/casting.rs b/crates/engine/src/game/casting.rs index edd4850b9c..e29b35f74b 100644 --- a/crates/engine/src/game/casting.rs +++ b/crates/engine/src/game/casting.rs @@ -15577,20 +15577,25 @@ fn pay_ability_mana_cost_with_choices_excluding_and_parent( parent, )?; - // CR 106.1b + CR 602.2b: 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), so it is where a companion "note the type of - // mana spent to pay this activation cost" effect (Jeweled Amulet, Ice - // Cauldron) reads back what was spent — at that effect's OWN resolution, - // not here. Never cleared: overwritten by this source's next ability-mana- - // cost payment (see `GameObject::mana_spent_to_activate`). + // CR 106.1b + CR 602.2b + CR 400.7: 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), so it is where a + // companion "note the type of mana spent to pay this activation cost" + // effect (Jeweled Amulet) reads back what was spent — at that effect's + // OWN resolution, not here. Never cleared: overwritten by this source's + // next ability-mana-cost payment (see `GameObject::mana_spent_to_activate`). + // The source's CURRENT incarnation is captured alongside the spent types + // so the reader can refuse a stale payment from a since-departed + // incarnation (bounce/flicker while this same ability sits unresolved on + // the stack) — see `GameObject::mana_spent_to_activate_incarnation`. 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(); + obj.mana_spent_to_activate_incarnation = obj.incarnation; } Ok(match payment { diff --git a/crates/engine/src/game/effects/note_mana_spent.rs b/crates/engine/src/game/effects/note_mana_spent.rs index 07b1c4f787..670b26e838 100644 --- a/crates/engine/src/game/effects/note_mana_spent.rs +++ b/crates/engine/src/game/effects/note_mana_spent.rs @@ -5,7 +5,11 @@ 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, Ice Cauldron). +/// 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 /// (`pay_ability_mana_cost_with_choices_excluding_and_parent`, which stamps the @@ -17,6 +21,15 @@ use crate::types::game_state::GameState; /// /// "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 the one captured when `mana_spent_to_activate` was stamped (see +/// `GameObject::mana_spent_to_activate_incarnation`), mirroring the engine's +/// existing incarnation-pairing idiom (`ResolvedAbility::source_is_current`, +/// `TargetFilter::SelfRef` resolution). pub fn resolve( state: &mut GameState, ability: &ResolvedAbility, @@ -30,6 +43,12 @@ pub fn resolve( // CR 608.2c: the source has left the zone it was in — nothing to note. return Ok(()); }; + if src.mana_spent_to_activate_incarnation != src.incarnation { + // CR 400.7: the payment was stamped on a prior incarnation of this + // object (bounced/flickered since). This incarnation never paid + // anything itself — nothing to note. + return Ok(()); + } let spent_types = src.mana_spent_to_activate.clone(); let Some(src) = state.objects.get_mut(&ability.source_id) else { diff --git a/crates/engine/src/game/game_object.rs b/crates/engine/src/game/game_object.rs index 3d824bb64e..35a5ddf628 100644 --- a/crates/engine/src/game/game_object.rs +++ b/crates/engine/src/game/game_object.rs @@ -1192,7 +1192,7 @@ pub struct GameObject { /// `pay_ability_mana_cost_with_choices_excluding_and_parent` at /// activation-time payment. Bridges cost payment (activation) to /// `Effect::NoteManaSpent` (resolution) for "note the type of mana spent - /// to pay this activation cost" (Jeweled Amulet, Ice Cauldron). Transient: + /// to pay this activation cost" (Jeweled Amulet). Transient: /// overwritten by the object's next ability-mana-cost payment, with no /// other clearing logic — safe because every known card in this class /// gates its "note" ability behind a `{T}` cost component, so the same @@ -1201,8 +1201,21 @@ pub struct GameObject { /// cast-side stamp-then-read idiom, adapted for ability activation, where /// the resolving stack entry is a separate `ResolvedAbility` rather than /// this same object. + /// + /// Paired with `mana_spent_to_activate_incarnation` (CR 400.7): a source + /// that leaves and returns (bounce/flicker) while its OWN ability is + /// still unresolved on the stack becomes a new object at this same + /// storage id (see `GameObject::incarnation`). `Effect::NoteManaSpent` + /// must not promote this latch to durable `ChosenAttribute:: + /// NotedManaSpent` state unless the object's current incarnation still + /// matches the one captured here — otherwise the new incarnation would + /// silently inherit a payment it never made. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub mana_spent_to_activate: Vec, + /// See `mana_spent_to_activate`. The object's `incarnation` at the moment + /// that latch was stamped. + #[serde(default)] + pub mana_spent_to_activate_incarnation: u64, } /// CR 104.4b compile-time totality guard for `objects_content_eq`/`object_content_eq` @@ -1368,6 +1381,8 @@ fn _gameobject_partition_is_total(o: &GameObject) { // `chosen_attributes` accumulator, or overwritten by the object's // next ability-mana-cost payment (§5.2c). mana_spent_to_activate: _, + // Paired with `mana_spent_to_activate` above — same omission class. + mana_spent_to_activate_incarnation: _, } = o; } @@ -2236,6 +2251,7 @@ impl GameObject { mana_spent_source_snapshots: Vec::new(), phase_status: PhaseStatus::PhasedIn, mana_spent_to_activate: Vec::new(), + mana_spent_to_activate_incarnation: 0, } } @@ -2685,7 +2701,7 @@ 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, Ice Cauldron). Read by `ManaProduction::NotedType`. + /// 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()), diff --git a/crates/engine/src/parser/oracle_ir/ast.rs b/crates/engine/src/parser/oracle_ir/ast.rs index 80c3434924..de661b6dd4 100644 --- a/crates/engine/src/parser/oracle_ir/ast.rs +++ b/crates/engine/src/parser/oracle_ir/ast.rs @@ -672,10 +672,12 @@ pub(crate) enum ImperativeFamilyAst { Behold(TargetFilter), /// CR 701.48a: Learn. Learn, - /// CR 106.1b + CR 602.2b: "note the type [and amount] of mana spent to pay - /// this activation cost" (Jeweled Amulet, Ice Cauldron). Field-less: there - /// is nothing to select — the payment already happened, so the effect is - /// a pure readback recorded at resolution. + /// 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 { diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 9d8efab196..2b97e53e8f 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -1470,7 +1470,7 @@ pub enum ChosenAttribute { 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, Ice Cauldron). Like `Card` and + /// 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 @@ -12961,7 +12961,7 @@ pub enum Effect { /// 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, Ice Cauldron). Reads the + /// 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 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 index 83d325e359..c7a7a2259e 100644 --- a/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs +++ b/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs @@ -6,12 +6,15 @@ //! and the second ability produced no mana at all regardless of what was //! noted. +use engine::game::effects::bounce; use engine::game::scenario::{GameScenario, P0}; +use engine::types::ability::{BounceSelection, 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 \ @@ -196,3 +199,82 @@ fn jeweled_amulet_produces_no_mana_with_nothing_noted() { "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" + ); + 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" + ); +} From 4055837c94fb3b352d563420412ee1d90bc10c40 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 30 Jul 2026 10:49:06 -0500 Subject: [PATCH 3/9] fix(engine): thread noted-mana payment per-activation, not per-object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second round of review on PR #6812 (issue #6504): - game/casting.rs / game/casting_costs.rs / effects/note_mana_spent.rs: GameObject::mana_spent_to_activate was a per-source mutable latch that the NEXT activation's cost payment silently overwrote. Since the incarnation guard added in the prior round only protects against a bounced/flickered SOURCE, not a same-incarnation permanent untapped and reactivated while an earlier "note" activation still sits unresolved on the stack (legal: "Activate only if no charge counters" is checked at activation, and no counter exists yet while the first note ability is still unresolved), two stacked activations could both observe the LATER payment. `push_ability_entry` — the single authority where an activated ability reaches the stack — now drains that latch synchronously into THIS activation's own `ResolvedAbility:: noted_mana_payment` snapshot (a new `NotedManaPayment { types, source_incarnation }`, propagated through `sub_ability`/`else_ability` via a new `set_noted_mana_payment_recursive`, since `Effect:: NoteManaSpent` is chained as a sub-ability with its own separate ResolvedAbility node) immediately after cost payment completes, before any later activation of the same permanent can occur. `Effect::NoteManaSpent` now reads that per-activation snapshot instead of the shared object field. Added a LIFO stacked-activation regression (`jeweled_amulet_lifo_stacked_activations_each_note_their_own_payment`) that activates twice with different colors before either resolves and asserts each resolution notes its own payment. - game/ability_scan.rs: `ManaProduction::NotedType` read `ChosenAttribute::NotedManaSpent`, a per-object value a SIBLING copy of `Effect::NoteManaSpent` can mutate before this production resolves — exactly the race above — so it must self-assert `sibling: true` (CR 603.3b ordering-relevance) rather than sit in the count-only bucket alongside `ChosenColor` (whose as-enters choice is effectively fixed for the object's lifetime and has no such sibling-mutation risk). Added `noted_mana_type_self_asserts_sibling`. - issue_6504_jeweled_amulet_noted_mana.rs: added `jeweled_amulet_notes_nothing_before_resolution` (CR 608.2c: no durable note before the ability actually resolves) and `jeweled_amulet_second_note_replaces_not_appends` (two full note cycles on the SAME object leave exactly one entry, not two) — the prior round's red/green tests used separate objects and an always-resolved helper, so neither claim was actually exercised. `noted_mana_payment` is a new `ResolvedAbility` field; every exhaustive match/destructure/literal across the crate (batch-candidate proofs in stack.rs, inert-trigger equality, several no-target `ResolvedAbility` literals in effects/*.rs) is updated in lockstep, including two new "must not batch a per-activation payment snapshot" gates mirroring the existing `cost_paid_object` ones. --- crates/engine/src/game/ability_rw.rs | 1 + crates/engine/src/game/casting.rs | 23 +- crates/engine/src/game/casting_costs.rs | 28 ++- .../src/game/effects/additional_phase.rs | 1 + crates/engine/src/game/effects/double.rs | 1 + crates/engine/src/game/effects/extra_turn.rs | 1 + .../grant_extra_loyalty_activations.rs | 1 + .../src/game/effects/note_mana_spent.rs | 38 ++-- .../engine/src/game/effects/player_counter.rs | 2 + .../src/game/effects/reverse_turn_order.rs | 1 + .../engine/src/game/effects/skip_next_step.rs | 1 + .../engine/src/game/effects/skip_next_turn.rs | 1 + crates/engine/src/game/effects/vote.rs | 7 + crates/engine/src/game/game_object.rs | 51 ++--- crates/engine/src/game/stack.rs | 15 ++ crates/engine/src/types/ability.rs | 51 +++++ .../issue_6504_jeweled_amulet_noted_mana.rs | 204 ++++++++++++++++++ .../the_chain_veil_loyalty_grants.rs | 1 + 18 files changed, 366 insertions(+), 62 deletions(-) diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index bdb24451c6..44fb97291f 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -3744,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: _, diff --git a/crates/engine/src/game/casting.rs b/crates/engine/src/game/casting.rs index e29b35f74b..c997937be8 100644 --- a/crates/engine/src/game/casting.rs +++ b/crates/engine/src/game/casting.rs @@ -15577,25 +15577,22 @@ fn pay_ability_mana_cost_with_choices_excluding_and_parent( parent, )?; - // CR 106.1b + CR 602.2b + CR 400.7: 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), so it is where a + // 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) reads back what was spent — at that effect's - // OWN resolution, not here. Never cleared: overwritten by this source's - // next ability-mana-cost payment (see `GameObject::mana_spent_to_activate`). - // The source's CURRENT incarnation is captured alongside the spent types - // so the reader can refuse a stale payment from a since-departed - // incarnation (bounce/flicker while this same ability sits unresolved on - // the stack) — see `GameObject::mana_spent_to_activate_incarnation`. + // 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(); - obj.mana_spent_to_activate_incarnation = obj.incarnation; } Ok(match payment { diff --git a/crates/engine/src/game/casting_costs.rs b/crates/engine/src/game/casting_costs.rs index bc53acce4c..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( 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/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/note_mana_spent.rs b/crates/engine/src/game/effects/note_mana_spent.rs index 670b26e838..d435e938ef 100644 --- a/crates/engine/src/game/effects/note_mana_spent.rs +++ b/crates/engine/src/game/effects/note_mana_spent.rs @@ -11,13 +11,18 @@ use crate::types::game_state::GameState; /// 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 -/// (`pay_ability_mana_cost_with_choices_excluding_and_parent`, which stamps the -/// transient `GameObject::mana_spent_to_activate` latch); 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). +/// 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). @@ -26,8 +31,7 @@ use crate::types::game_state::GameState; /// 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 the one captured when `mana_spent_to_activate` was stamped (see -/// `GameObject::mana_spent_to_activate_incarnation`), mirroring the engine's +/// matches `noted_mana_payment.source_incarnation`, mirroring the engine's /// existing incarnation-pairing idiom (`ResolvedAbility::source_is_current`, /// `TargetFilter::SelfRef` resolution). pub fn resolve( @@ -39,17 +43,23 @@ pub fn resolve( 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.mana_spent_to_activate_incarnation != src.incarnation { - // CR 400.7: the payment was stamped on a prior incarnation of this - // object (bounced/flickered since). This incarnation never paid - // anything itself — nothing to note. + 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 = src.mana_spent_to_activate.clone(); + let spent_types = payment.types.clone(); let Some(src) = state.objects.get_mut(&ability.source_id) else { return 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/game_object.rs b/crates/engine/src/game/game_object.rs index 35a5ddf628..3919c7cc5a 100644 --- a/crates/engine/src/game/game_object.rs +++ b/crates/engine/src/game/game_object.rs @@ -1187,35 +1187,24 @@ pub struct GameObject { #[serde(default)] pub phase_status: PhaseStatus, - /// CR 106.1b + CR 602.2b: Mana type(s) spent to pay this object's own - /// most recent activated-ability mana cost, stamped by + /// 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. Bridges cost payment (activation) to - /// `Effect::NoteManaSpent` (resolution) for "note the type of mana spent - /// to pay this activation cost" (Jeweled Amulet). Transient: - /// overwritten by the object's next ability-mana-cost payment, with no - /// other clearing logic — safe because every known card in this class - /// gates its "note" ability behind a `{T}` cost component, so the same - /// source cannot pay a second activation's mana cost before the first - /// resolves and consumes this latch. Mirrors `colors_spent_to_cast`'s - /// cast-side stamp-then-read idiom, adapted for ability activation, where - /// the resolving stack entry is a separate `ResolvedAbility` rather than - /// this same object. - /// - /// Paired with `mana_spent_to_activate_incarnation` (CR 400.7): a source - /// that leaves and returns (bounce/flicker) while its OWN ability is - /// still unresolved on the stack becomes a new object at this same - /// storage id (see `GameObject::incarnation`). `Effect::NoteManaSpent` - /// must not promote this latch to durable `ChosenAttribute:: - /// NotedManaSpent` state unless the object's current incarnation still - /// matches the one captured here — otherwise the new incarnation would - /// silently inherit a payment it never made. + /// 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, - /// See `mana_spent_to_activate`. The object's `incarnation` at the moment - /// that latch was stamped. - #[serde(default)] - pub mana_spent_to_activate_incarnation: u64, } /// CR 104.4b compile-time totality guard for `objects_content_eq`/`object_content_eq` @@ -1376,13 +1365,10 @@ fn _gameobject_partition_is_total(o: &GameObject) { phase_status: _, protection_start_exempt_attachments: _, // Activation-cost-payment latch — same omission class as - // `mana_spent_to_cast`/`colors_spent_to_cast` above: consumed - // synchronously by `Effect::NoteManaSpent` into the compared - // `chosen_attributes` accumulator, or overwritten by the object's - // next ability-mana-cost payment (§5.2c). + // `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: _, - // Paired with `mana_spent_to_activate` above — same omission class. - mana_spent_to_activate_incarnation: _, } = o; } @@ -2251,7 +2237,6 @@ impl GameObject { mana_spent_source_snapshots: Vec::new(), phase_status: PhaseStatus::PhasedIn, mana_spent_to_activate: Vec::new(), - mana_spent_to_activate_incarnation: 0, } } 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/types/ability.rs b/crates/engine/src/types/ability.rs index 2b97e53e8f..6620a3529e 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -6714,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 @@ -23370,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 @@ -23523,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, @@ -24053,6 +24085,25 @@ 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 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/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs b/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs index c7a7a2259e..8736d73634 100644 --- a/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs +++ b/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs @@ -278,3 +278,207 @@ fn jeweled_amulet_bounced_mid_stack_does_not_note_on_new_incarnation() { 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" + ); +} 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, From 0dcc7e033bc8083fdb2d010df157c9376acb68d8 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Thu, 30 Jul 2026 12:20:03 -0500 Subject: [PATCH 4/9] fix(engine): clear noted-mana payment when copying an activated ability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third round of review on PR #6812 (issue #6504): copy_spell.rs's ActivatedAbility/TriggeredAbility copy arm only called preserve_ability_copy_source_recursive to re-stamp source_id; the rest of the cloned ResolvedAbility chain, including the new noted_mana_payment field, rode along unchanged. CR 707.10: a copy of an activated ability is not itself activated, so it never paid a mana cost — but a copied Jeweled Amulet activation still carried the ORIGINAL's captured payment, and Effect::NoteManaSpent resolving on the copy would falsely note colors the copy never spent. preserve_ability_copy_source_recursive now also calls a new ResolvedAbility::clear_noted_mana_payment_recursive (mirrors set_noted_mana_payment_recursive's recursion shape in reverse) so the copy's whole chain — including any sub_ability/else_ability nodes — loses the inherited snapshot. Added jeweled_amulet_copied_activation_does_not_note_original_payment, which exercises the real copy pipeline (copy_spell::resolve targeting the original activation's own stack-entry id, the same stack-entry lookup and LIFO resolution a real "copy target activated or triggered ability" card like Lithoform Engine drives) rather than only unit -testing the clearing method: activates paying red, copies it, resolves the copy first (asserting no note) and the original second (asserting it still notes red — the paired positive reach-guard proving the fix clears only the copy's snapshot). Verified revert-failing. --- crates/engine/src/game/effects/copy_spell.rs | 11 ++ crates/engine/src/types/ability.rs | 19 +++ .../issue_6504_jeweled_amulet_noted_mana.rs | 115 +++++++++++++++++- 3 files changed, 143 insertions(+), 2 deletions(-) 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/types/ability.rs b/crates/engine/src/types/ability.rs index 6620a3529e..4afaa0896a 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -24104,6 +24104,25 @@ impl ResolvedAbility { } } + /// 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/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs b/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs index 8736d73634..3b947ea096 100644 --- a/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs +++ b/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs @@ -6,9 +6,11 @@ //! and the second ability produced no mana at all regardless of what was //! noted. -use engine::game::effects::bounce; +use engine::game::effects::{bounce, copy_spell}; use engine::game::scenario::{GameScenario, P0}; -use engine::types::ability::{BounceSelection, Effect, ResolvedAbility, TargetFilter, TargetRef}; +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; @@ -482,3 +484,112 @@ fn jeweled_amulet_lifo_stacked_activations_each_note_their_own_payment() { 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; + + // (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" + ); +} From 6ccbcbca251f57853ed96be9aa3f7f3ecb43f21f Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Fri, 31 Jul 2026 14:03:16 -0500 Subject: [PATCH 5/9] test(engine): guard Ice Cauldron's noted-mana boundary through the production parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-5 review: the intentionally unsupported "type and amount" clause (Ice Cauldron) was only exercised by parse_note_mana_spent_clause's own unit tests and a fabricated end-to-end sibling ("Amber Amulet") — neither path parses Ice Cauldron's actual printed Oracle text, so its Unimplemented status wasn't protected against an upstream routing/fallback change that could start treating it as supported. Added ice_cauldron_note_type_and_amount_stays_unimplemented, parsing Ice Cauldron's full two-ability Oracle text (verified against MTGJSON) through the production parser. It walks the exile -> cast -> counter -> note sub-ability chain to confirm the parser actually reaches the "type and amount" clause (not failing earlier for an unrelated reason) before asserting it still lands on Effect::Unimplemented, and separately asserts the second ability's "last noted type and amount of mana" is not matched by ManaProduction::NotedType's singular-type pattern. Paired with the existing jeweled_amulet_notes_and_reads_back_mana_type positive assertion, this distinguishes the intentional boundary from a parser that simply fails before reaching either clause. --- .../engine/tests/integration/oracle_parser.rs | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/crates/engine/tests/integration/oracle_parser.rs b/crates/engine/tests/integration/oracle_parser.rs index bc7cdc7aa7..d0b163c46d 100644 --- a/crates/engine/tests/integration/oracle_parser.rs +++ b/crates/engine/tests/integration/oracle_parser.rs @@ -996,3 +996,116 @@ fn jeweled_amulet_notes_and_reads_back_mana_type() { 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 + ); +} From a201be9ba04e994bc1886898b721138c552deffd Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 7 Aug 2026 13:56:29 -0700 Subject: [PATCH 6/9] fix(PR-6812): resolve review blockers --- .../issue_6504_jeweled_amulet_noted_mana.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 index 3b947ea096..d335e8d66a 100644 --- a/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs +++ b/crates/engine/tests/integration/issue_6504_jeweled_amulet_noted_mana.rs @@ -238,6 +238,11 @@ fn jeweled_amulet_bounced_mid_stack_does_not_note_on_new_incarnation() { 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 @@ -524,6 +529,19 @@ fn jeweled_amulet_copied_activation_does_not_note_original_payment() { .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 From c8346c4de203621269c9d474b3ca28d06ce2b3f8 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 7 Aug 2026 14:34:10 -0700 Subject: [PATCH 7/9] fix(PR-6812): classify noted mana effect --- crates/engine/src/game/ability_scan.rs | 22 +++++++++++++++++++++ crates/engine/src/game/resolution_prompt.rs | 8 ++++++++ crates/engine/src/types/ability_visit.rs | 1 + 3 files changed, 31 insertions(+) diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index 3279492230..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; @@ -5371,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 @@ -5775,6 +5779,7 @@ fn effect_census_role(e: &Effect) -> CensusRole { | Effect::TimeTravel | Effect::BecomeMonarch | Effect::NoOp + | Effect::NoteManaSpent | Effect::Proliferate | Effect::ProliferateTarget { .. } | Effect::Populate @@ -6008,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 @@ -7368,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/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/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 { .. } From dd95f1c809ed1a9e82663b39c250319e3661ba8b Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 7 Aug 2026 15:05:30 -0700 Subject: [PATCH 8/9] test(engine): refresh prompt census coordinates --- crates/engine/src/game/engine.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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. From 2243c0834e0307f4f9c204c73cbb5f580e106e57 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Fri, 7 Aug 2026 16:35:14 -0700 Subject: [PATCH 9/9] chore(ai): refresh decision-cost baseline --- crates/phase-ai/baselines/perf-baseline.json | 34 ++++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) 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 +}