From e2f747c488dcc3a12c1b7d7d3b424626d2843dae Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 07:03:45 -0700 Subject: [PATCH 1/3] fix(engine): pay Braid of Fire cumulative upkeep --- crates/engine/src/game/costs.rs | 78 +++++++++++-------- crates/engine/src/game/effects/mod.rs | 30 ++++++- .../engine/src/game/engine_payment_choices.rs | 22 +++--- crates/engine/src/types/ability.rs | 39 ++++++---- .../integration/issue_4395_braid_of_fire.rs | 68 ++++++++++++++++ crates/engine/tests/integration/main.rs | 1 + 6 files changed, 174 insertions(+), 64 deletions(-) create mode 100644 crates/engine/tests/integration/issue_4395_braid_of_fire.rs diff --git a/crates/engine/src/game/costs.rs b/crates/engine/src/game/costs.rs index 655adee0bf..de5ec38112 100644 --- a/crates/engine/src/game/costs.rs +++ b/crates/engine/src/game/costs.rs @@ -43,7 +43,7 @@ use std::collections::HashSet; use crate::types::ability::{ - AbilityCost, Effect, EffectKind, TargetFilter, TypedFilter, REMOVE_COUNTER_COST_ALL, + AbilityCost, EffectKind, TargetFilter, TypedFilter, REMOVE_COUNTER_COST_ALL, }; use crate::types::events::GameEvent; use crate::types::game_state::{ @@ -1319,8 +1319,9 @@ fn pay_ability_cost_inner( // Waterbend cost was already paid via ManaPayment before reaching pay_ability_cost. AbilityCost::Waterbend { .. } => {} // CR 118.3: An effect performed as a cost. Resolve the effect on the - // source before the ability's own effect fires. Currently handles - // PutCounter on self (Devoted Druid, Chainbreaker, etc.). + // source before the ability's own effect fires. The shared support + // predicate admits only deterministic source-counter and fixed-mana + // forms, so this never opens a player-choice prompt mid-payment. AbilityCost::EffectCost { effect } => { use crate::types::ability::Effect; match effect.as_ref() { @@ -1365,6 +1366,42 @@ fn pay_ability_cost_inner( }); } } + // CR 106.3 + CR 106.4: A Braid of Fire-style cost performs + // fixed mana production directly into the payer's pool. This + // uses the ordinary replacement-aware mana primitive but does + // not resolve a separate ability or change priority mid-cost. + Effect::Mana { + produced: + produced @ crate::types::ability::ManaProduction::Fixed { colors, .. }, + restrictions, + grants, + expiry, + target: None, + } => { + let restrictions = super::effects::mana::resolve_restrictions( + restrictions, + state, + source_id, + ); + let source_could_produce_two_or_more_colors = + super::mana_sources::mana_production_could_produce_two_or_more_colors( + state, player, source_id, produced, + ); + for color in colors { + super::mana_payment::produce_mana_with_attributes_from_source_quality( + state, + source_id, + super::mana_sources::mana_color_to_type(color), + player, + false, + source_could_produce_two_or_more_colors, + &restrictions, + grants, + *expiry, + events, + ); + } + } _ => { return Ok(payment_failed(format!( "Effect-as-cost not yet resolvable: {effect:?}" @@ -1857,20 +1894,9 @@ pub(crate) fn supported_at_resolution(cost: &AbilityCost) -> bool { // "exile two creature cards from graveyards"). The interactive choice is // surfaced via WaitingFor::PayCost before this resume runs. AbilityCost::Exile { filter, .. } if !matches!(filter, Some(TargetFilter::SelfRef)) => true, - // CR 702.24a + CR 122.1: An effect-cost counter placement on the - // source is the deterministic cumulative-upkeep payment shape. The - // actual addition still flows through `add_counter_with_replacement`. - AbilityCost::EffectCost { effect } - if matches!( - effect.as_ref(), - Effect::PutCounter { - target: TargetFilter::SelfRef, - .. - } - ) => - { - true - } + // CR 118.3: The shared effect-cost predicate admits only deterministic + // payment effects that the authority resolves directly. + AbilityCost::EffectCost { .. } if cost.supports_effect_cost_payment() => true, AbilityCost::Discard { .. } | AbilityCost::Tap | AbilityCost::Untap @@ -2033,20 +2059,10 @@ fn can_pay_resolution( // limit on giving yourself more counters (poison's ten-or-more loss // condition is a separate SBA, not a payment-time affordability gate). AbilityCost::GetPlayerCounters { .. } => true, - // CR 702.24a + CR 122.1: The concrete source-counter effect cost is - // always offerable; replacement handling determines whether its - // actual placement completes, exactly as for other counter costs. - AbilityCost::EffectCost { effect } - if matches!( - effect.as_ref(), - Effect::PutCounter { - target: TargetFilter::SelfRef, - .. - } - ) => - { - true - } + // CR 118.3: Every deterministic effect-cost payment admitted by the + // shared support predicate is offerable; its resolver handles any + // replacement effects while paying it. + AbilityCost::EffectCost { .. } if cost.supports_effect_cost_payment() => true, // Variants below have no resolution-time payment arm // (`supported_at_resolution` is the shared membership authority). // Refusing here is the conservative affordability answer (treat as diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index e76a16c5a8..82b2d0d8a1 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -12,10 +12,10 @@ use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityKind, CardPlayMode, CardTypeSetSource, ControllerRef, CopyRetargetPermission, CostPaidObjectSnapshot, EachDamageRecipient, Effect, EffectError, EffectKind, EffectOutcomeSignal, EffectResolutionResult, EffectScope, FilterProp, - OpponentMayScope, PlayerFilter, PlayerScope, QuantityExpr, QuantityRef, RepeatContinuation, - ResolvedAbility, RevealUntilDisposition, SacrificeCost, SacrificeRequirement, SharedQuality, - SharedQualityRelation, SiblingCondition, SubAbilityLink, TapStateChange, TargetChoiceTiming, - TargetFilter, TargetRef, ThisWayCause, + ManaProduction, OpponentMayScope, PlayerFilter, PlayerScope, QuantityExpr, QuantityRef, + RepeatContinuation, ResolvedAbility, RevealUntilDisposition, SacrificeCost, + SacrificeRequirement, SharedQuality, SharedQualityRelation, SiblingCondition, SubAbilityLink, + TapStateChange, TargetChoiceTiming, TargetFilter, TargetRef, ThisWayCause, }; #[cfg(test)] use crate::types::ability::{AttackScope, AttackSubject}; @@ -13105,6 +13105,28 @@ fn expand_per_counter(base: &AbilityCost, n: u32) -> AbilityCost { target: TargetFilter::SelfRef, }), }, + // CR 702.24a: Every age counter requires a separate instance of + // the fixed mana-producing cost. Combining its fixed color vector + // keeps the result a single deterministic EffectCost while adding + // the same number of mana units as N separate resolutions. + Effect::Mana { + produced: ManaProduction::Fixed { .. }, + target: None, + .. + } => { + let mut scaled_effect = effect.as_ref().clone(); + let Effect::Mana { + produced: ManaProduction::Fixed { colors, .. }, + .. + } = &mut scaled_effect + else { + unreachable!("matched fixed mana effect cost") + }; + *colors = colors.repeat(n as usize); + AbilityCost::EffectCost { + effect: Box::new(scaled_effect), + } + } _ => AbilityCost::Composite { costs: vec![base.clone(); n as usize], }, diff --git a/crates/engine/src/game/engine_payment_choices.rs b/crates/engine/src/game/engine_payment_choices.rs index ab1c48b282..b510bf11da 100644 --- a/crates/engine/src/game/engine_payment_choices.rs +++ b/crates/engine/src/game/engine_payment_choices.rs @@ -1193,20 +1193,14 @@ pub(super) fn handle_unless_payment( // CR 118.12a: "unless [target's controller] has [~] deal N damage to // them" — the payer takes damage from the ability source instead of // the primary effect (Blazing Salvo, Lava Blister, Barbarian Bully). - AbilityCost::EffectCost { effect } => match effect.as_ref() { - // CR 702.24a + CR 122.1: Cumulative upkeep can require a - // source-counter effect cost (Aboroth). Route it through the - // same resolution payment authority as its activation-cost - // form, including the replacement-aware continuation. - Effect::PutCounter { - target: TargetFilter::SelfRef, - .. - } => match costs::pay_ability_cost_for_resolution( + // CR 118.3: Deterministic effect-cost payments use the single + // resolution payment authority. Its shared support predicate + // covers source counters and fixed mana without a prompt. + AbilityCost::EffectCost { .. } if cost.supports_effect_cost_payment() => { + match costs::pay_ability_cost_for_resolution( state, player, - &AbilityCost::EffectCost { - effect: effect.clone(), - }, + &cost, pending_effect.as_ref(), events, )? { @@ -1223,7 +1217,9 @@ pub(super) fn handle_unless_payment( }); return Ok(action_result(events, state.waiting_for.clone())); } - }, + } + } + AbilityCost::EffectCost { effect } => match effect.as_ref() { Effect::DealDamage { .. } => { let mut damage_ability = pending_effect.as_ref().clone(); damage_ability.effect = *effect.clone(); diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index caf3fd59cb..44a5493940 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -9492,22 +9492,7 @@ impl AbilityCost { filter: None, .. } => true, - // CR 702.24a + CR 122.1: The existing resolution payment - // authority can place a counter on the source through the - // replacement pipeline. This covers cumulative-upkeep costs such - // as Aboroth's "put a -1/-1 counter on this creature" without - // admitting arbitrary effect-as-cost shapes. - AbilityCost::EffectCost { effect } - if matches!( - effect.as_ref(), - Effect::PutCounter { - target: TargetFilter::SelfRef, - .. - } - ) => - { - true - } + AbilityCost::EffectCost { .. } if self.supports_effect_cost_payment() => true, // CR 118.12a: OneOf at the base must be a disjunction of mana // costs; mixed-shape disjunctions are not yet expanded into a // payable per-counter form. @@ -9524,6 +9509,28 @@ impl AbilityCost { } } + /// CR 118.3: Effect-as-cost forms the payment authority can resolve without + /// a player choice. This is shared by cumulative-upkeep synthesis and the + /// resolution-time payment gate so supported cards never install a trigger + /// whose cost will later be rejected. + pub fn supports_effect_cost_payment(&self) -> bool { + matches!( + self, + AbilityCost::EffectCost { effect } + if matches!( + effect.as_ref(), + Effect::PutCounter { + target: TargetFilter::SelfRef, + .. + } | Effect::Mana { + produced: ManaProduction::Fixed { .. }, + target: None, + .. + } + ) + ) + } + /// CR 118: Classify this cost into one or more `CostCategory` buckets. /// /// `Composite` recurses, flattening every sub-cost. Variants that pay diff --git a/crates/engine/tests/integration/issue_4395_braid_of_fire.rs b/crates/engine/tests/integration/issue_4395_braid_of_fire.rs new file mode 100644 index 0000000000..bc1621f1be --- /dev/null +++ b/crates/engine/tests/integration/issue_4395_braid_of_fire.rs @@ -0,0 +1,68 @@ +//! GitHub issue #4395 — Braid of Fire's cumulative upkeep is a mana-producing +//! effect cost, not an unsupported trigger. + +use engine::game::scenario::{GameScenario, P0}; +use engine::types::ability::{AbilityCost, Effect, ManaProduction}; +use engine::types::actions::GameAction; +use engine::types::counter::CounterType; +use engine::types::game_state::WaitingFor; +use engine::types::mana::ManaType; +use engine::types::phase::Phase; +use engine::types::zones::Zone; + +const BRAID_OF_FIRE_ORACLE: &str = "Cumulative upkeep—Add {R}. (At the beginning of your upkeep, put an age counter on this permanent, then sacrifice it unless you pay its upkeep cost for each age counter on it.)"; + +/// CR 702.24a + CR 106.4: After the upkeep tick creates two age counters, +/// paying Braid of Fire's cumulative cost adds two red mana rather than +/// sacrificing it. The exact Oracle pipeline guards the synthesized trigger, +/// per-counter expansion, and resolution-time effect-cost payment together. +#[test] +fn braid_of_fire_cumulative_upkeep_adds_red_for_each_age_counter() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::Untap); + let braid = scenario + .add_enchantment_from_oracle(P0, "Braid of Fire", BRAID_OF_FIRE_ORACLE) + .id(); + let mut runner = scenario.build(); + runner + .state_mut() + .objects + .get_mut(&braid) + .expect("Braid of Fire exists") + .counters + .insert(CounterType::Age, 1); + + runner.auto_advance_to_main_phase(); + runner.advance_until_stack_empty(); + + match &runner.state().waiting_for { + WaitingFor::UnlessPayment { cost, .. } => assert!(matches!( + cost, + AbilityCost::EffectCost { effect } + if matches!( + effect.as_ref(), + Effect::Mana { + produced: ManaProduction::Fixed { colors, .. }, + target: None, + .. + } if colors == &vec![engine::types::mana::ManaColor::Red; 2] + ) + )), + other => panic!("expected Braid of Fire cumulative-upkeep prompt, got {other:?}"), + } + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("Braid of Fire's mana-producing cost is payable"); + + let braid_object = runner + .state() + .objects + .get(&braid) + .expect("Braid of Fire remains"); + assert_eq!(braid_object.zone, Zone::Battlefield); + assert_eq!(braid_object.counters.get(&CounterType::Age), Some(&2)); + let pool = &runner.state().players[P0.0 as usize].mana_pool.mana; + assert_eq!(pool.len(), 2); + assert!(pool.iter().all(|unit| unit.color == ManaType::Red)); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 6155c250ea..24257508a7 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -562,6 +562,7 @@ mod issue_4358_hama_pashar_dungeon; mod issue_4379_convoke_cancel_untap; mod issue_4384_airbend_stack_spell; mod issue_4388_mana_on_opponents_turn; +mod issue_4395_braid_of_fire; mod issue_4420_lava_blister_unless_deal_damage; mod issue_4459_decoy_gambit_unless_have_you_draw; mod issue_4503_incremental_growth; From 8358ca595b033755c62dd3241e843e4143e02c06 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 07:15:00 -0700 Subject: [PATCH 2/3] test(engine): cover fixed mana effect costs --- .../integration/issue_4395_braid_of_fire.rs | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/crates/engine/tests/integration/issue_4395_braid_of_fire.rs b/crates/engine/tests/integration/issue_4395_braid_of_fire.rs index bc1621f1be..3de34d2900 100644 --- a/crates/engine/tests/integration/issue_4395_braid_of_fire.rs +++ b/crates/engine/tests/integration/issue_4395_braid_of_fire.rs @@ -2,11 +2,14 @@ //! effect cost, not an unsupported trigger. use engine::game::scenario::{GameScenario, P0}; -use engine::types::ability::{AbilityCost, Effect, ManaProduction}; +use engine::types::ability::{ + AbilityCost, Effect, ManaContribution, ManaProduction, QuantityExpr, ResolvedAbility, + TargetFilter, +}; use engine::types::actions::GameAction; use engine::types::counter::CounterType; use engine::types::game_state::WaitingFor; -use engine::types::mana::ManaType; +use engine::types::mana::{ManaColor, ManaType}; use engine::types::phase::Phase; use engine::types::zones::Zone; @@ -66,3 +69,50 @@ fn braid_of_fire_cumulative_upkeep_adds_red_for_each_age_counter() { assert_eq!(pool.len(), 2); assert!(pool.iter().all(|unit| unit.color == ManaType::Red)); } + +/// CR 118.3 + CR 106.4: A deterministic, untargeted fixed-mana effect cost +/// resolves through the normal unless-payment flow into the payer's mana pool. +#[test] +fn fixed_mana_effect_cost_pays_into_the_unless_payers_mana_pool() { + let mut scenario = GameScenario::new(); + let source = scenario.add_creature(P0, "Fixed Mana Cost Source", 1, 1).id(); + let mut runner = scenario.build(); + let pending_effect = ResolvedAbility::new( + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + vec![], + source, + P0, + ); + runner.state_mut().waiting_for = WaitingFor::UnlessPayment { + player: P0, + cost: AbilityCost::EffectCost { + effect: Box::new(Effect::Mana { + produced: ManaProduction::Fixed { + colors: vec![ManaColor::Blue, ManaColor::Red], + contribution: ManaContribution::Base, + }, + restrictions: vec![], + grants: vec![], + expiry: None, + target: None, + }), + }, + pending_effect: Box::new(pending_effect), + trigger_event: None, + effect_description: None, + remaining: vec![], + }; + + runner + .act(GameAction::PayUnlessCost { pay: true }) + .expect("fixed mana effect cost is payable"); + + let pool = &runner.state().players[P0.0 as usize].mana_pool.mana; + assert_eq!( + pool.iter().map(|unit| unit.color).collect::>(), + vec![ManaType::Blue, ManaType::Red] + ); +} From 22d22d7a5df0a141d72cce2c8cfc37af94cb8863 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 07:19:36 -0700 Subject: [PATCH 3/3] docs(engine): cite unless payment in effect cost test --- .../engine/tests/integration/issue_4395_braid_of_fire.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/engine/tests/integration/issue_4395_braid_of_fire.rs b/crates/engine/tests/integration/issue_4395_braid_of_fire.rs index 3de34d2900..c69a5f2073 100644 --- a/crates/engine/tests/integration/issue_4395_braid_of_fire.rs +++ b/crates/engine/tests/integration/issue_4395_braid_of_fire.rs @@ -70,12 +70,15 @@ fn braid_of_fire_cumulative_upkeep_adds_red_for_each_age_counter() { assert!(pool.iter().all(|unit| unit.color == ManaType::Red)); } -/// CR 118.3 + CR 106.4: A deterministic, untargeted fixed-mana effect cost -/// resolves through the normal unless-payment flow into the payer's mana pool. +/// CR 118.3 + CR 118.12a + CR 106.4: A deterministic, untargeted fixed-mana +/// effect cost resolves through the normal unless-payment flow into the payer's +/// mana pool. #[test] fn fixed_mana_effect_cost_pays_into_the_unless_payers_mana_pool() { let mut scenario = GameScenario::new(); - let source = scenario.add_creature(P0, "Fixed Mana Cost Source", 1, 1).id(); + let source = scenario + .add_creature(P0, "Fixed Mana Cost Source", 1, 1) + .id(); let mut runner = scenario.build(); let pending_effect = ResolvedAbility::new( Effect::GainLife {