From b72b109a68d064110e12382a331fa55f4665b2f3 Mon Sep 17 00:00:00 2001 From: Chris Swenson <1323803+ninthworld@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:35:13 -0700 Subject: [PATCH] fix(engine): fifteen M19 cards did not match the card that was printed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Diffed every one of the 299 catalog definitions against the real set data: printed characteristics mechanically, ability text by reading each definition beside its Oracle text. Fifteen cards were wrong and every one of them passed `make check` (#819). Twelve had wrong printed characteristics — mana cost, power/toughness, subtypes, colours. Regal Bloodlord was the worst of them: a mono-black card in the catalog and a white-black gold card in the set, so it was castable off the wrong mana. Eight had wrong costs, amounts, or trigger scopes. Dismissive Pyromancer was missing the {R} from one ability's cost and the {T} from the other's; Goblin Trashmaster carried a {1}{R} the card does not print; Ravenous Harpy cost {B} and gained a life the card does not give; Gift of Paradise gained two; the Axe granted +2/+1; Regal Bloodlord watched only its controller's end step and made a 2/2; Mistcaller had a flash it never had; the Sigiled Sword's Knight arrived tapped where the card says only "attacking". The tests asserted against the definitions rather than against the cards, so they cemented the bugs rather than catching them — one was named `..._gains_two_life` for a card that gains three. Each is corrected here, not relaxed, and where a test used a wrong number as a convenient fixture the numbers moved with it: the Twins are a 7/7 now, so the combat-spread tests that leaned on 5 power say 7, and the two that needed an attacker bigger than the blocker use an 8/8. Six new tests state the printed cost directly, because an offer is where a wrong cost is visible — a missing symbol makes an ability cheaper, and cheap is the direction nothing else complains about. Closes #820. Part of #819. Co-Authored-By: Claude Opus 5 (1M context) --- .../data/catalog/dismissive_pyromancer.json | 11 +- .../data/catalog/enigma_drake.json | 2 +- .../data/catalog/gargoyle_sentinel.json | 2 +- .../data/catalog/ghastbark_twins.json | 6 +- .../data/catalog/gift_of_paradise.json | 2 +- .../data/catalog/goblin_trashmaster.json | 7 +- .../catalog/goreclaw_terror_of_qal_sisma.json | 4 +- .../sage-engine/data/catalog/hired_blade.json | 2 +- .../data/catalog/magistrate_s_scepter.json | 2 +- .../data/catalog/marauder_s_axe.json | 4 +- .../sage-engine/data/catalog/mistcaller.json | 10 +- .../data/catalog/ravenous_harpy.json | 7 +- .../data/catalog/regal_bloodlord.json | 11 +- .../data/catalog/runic_armasaur.json | 4 +- .../catalog/sigiled_sword_of_valeron.json | 1 - crates/sage-engine/src/card/attachment.rs | 2 +- .../tests/blocking_an_additional_creature.rs | 53 ++++---- .../sage-engine/tests/m19_activation_costs.rs | 114 +++++++++++++++++- .../sage-engine/tests/m19_attacking_tokens.rs | 39 ++++++ crates/sage-engine/tests/m19_equipment.rs | 4 +- .../tests/m19_granted_abilities.rs | 6 +- crates/sage-engine/tests/m19_hired_blade.rs | 4 +- .../tests/m19_life_gained_this_turn.rs | 54 ++++++++- crates/sage-engine/tests/m19_mistcaller.rs | 42 ++++--- crates/sage-server/src/rules_text/tests.rs | 25 ++-- .../src/view/payment/activation.rs | 6 + 26 files changed, 317 insertions(+), 107 deletions(-) diff --git a/crates/sage-engine/data/catalog/dismissive_pyromancer.json b/crates/sage-engine/data/catalog/dismissive_pyromancer.json index 9f5bda56..6bf8a8d7 100644 --- a/crates/sage-engine/data/catalog/dismissive_pyromancer.json +++ b/crates/sage-engine/data/catalog/dismissive_pyromancer.json @@ -9,16 +9,20 @@ "Human", "Wizard" ], - "mana_cost": "{2}{R}", + "mana_cost": "{1}{R}", "colors": [ "red" ], "power": 2, - "toughness": 1, + "toughness": 2, "abilities": [ { "type": "activated", "cost": [ + { + "kind": "mana", + "mana": "{R}" + }, { "kind": "tap" }, @@ -41,6 +45,9 @@ "kind": "mana", "mana": "{2}{R}" }, + { + "kind": "tap" + }, { "kind": "sacrifice_this" } diff --git a/crates/sage-engine/data/catalog/enigma_drake.json b/crates/sage-engine/data/catalog/enigma_drake.json index b10a4eb2..246f410a 100644 --- a/crates/sage-engine/data/catalog/enigma_drake.json +++ b/crates/sage-engine/data/catalog/enigma_drake.json @@ -8,7 +8,7 @@ "subtypes": [ "Drake" ], - "mana_cost": "{2}{U}{R}", + "mana_cost": "{1}{U}{R}", "colors": [ "blue", "red" diff --git a/crates/sage-engine/data/catalog/gargoyle_sentinel.json b/crates/sage-engine/data/catalog/gargoyle_sentinel.json index 5d03fbde..b632f4e4 100644 --- a/crates/sage-engine/data/catalog/gargoyle_sentinel.json +++ b/crates/sage-engine/data/catalog/gargoyle_sentinel.json @@ -12,7 +12,7 @@ "mana_cost": "{3}", "colors": [], "power": 3, - "toughness": 4, + "toughness": 3, "keywords": [ "defender" ], diff --git a/crates/sage-engine/data/catalog/ghastbark_twins.json b/crates/sage-engine/data/catalog/ghastbark_twins.json index 7f0e49e4..87fe94b7 100644 --- a/crates/sage-engine/data/catalog/ghastbark_twins.json +++ b/crates/sage-engine/data/catalog/ghastbark_twins.json @@ -8,12 +8,12 @@ "subtypes": [ "Treefolk" ], - "mana_cost": "{4}{G}", + "mana_cost": "{5}{G}{G}", "colors": [ "green" ], - "power": 5, - "toughness": 5, + "power": 7, + "toughness": 7, "keywords": [ "trample" ], diff --git a/crates/sage-engine/data/catalog/gift_of_paradise.json b/crates/sage-engine/data/catalog/gift_of_paradise.json index 3c080846..bd81cb09 100644 --- a/crates/sage-engine/data/catalog/gift_of_paradise.json +++ b/crates/sage-engine/data/catalog/gift_of_paradise.json @@ -41,7 +41,7 @@ { "kind": "gain_life", "player_ref": "controller", - "amount": 2 + "amount": 3 } ] } diff --git a/crates/sage-engine/data/catalog/goblin_trashmaster.json b/crates/sage-engine/data/catalog/goblin_trashmaster.json index 5edd6075..2adfe13c 100644 --- a/crates/sage-engine/data/catalog/goblin_trashmaster.json +++ b/crates/sage-engine/data/catalog/goblin_trashmaster.json @@ -6,7 +6,8 @@ "creature" ], "subtypes": [ - "Goblin" + "Goblin", + "Warrior" ], "mana_cost": "{2}{R}{R}", "colors": [ @@ -31,10 +32,6 @@ { "type": "activated", "cost": [ - { - "kind": "mana", - "mana": "{1}{R}" - }, { "kind": "sacrifice", "subtype": "Goblin" diff --git a/crates/sage-engine/data/catalog/goreclaw_terror_of_qal_sisma.json b/crates/sage-engine/data/catalog/goreclaw_terror_of_qal_sisma.json index 27b5c9f2..6aacc85f 100644 --- a/crates/sage-engine/data/catalog/goreclaw_terror_of_qal_sisma.json +++ b/crates/sage-engine/data/catalog/goreclaw_terror_of_qal_sisma.json @@ -11,12 +11,12 @@ "subtypes": [ "Bear" ], - "mana_cost": "{4}{G}", + "mana_cost": "{3}{G}", "colors": [ "green" ], "power": 4, - "toughness": 4, + "toughness": 3, "abilities": [ { "type": "cost_modifier", diff --git a/crates/sage-engine/data/catalog/hired_blade.json b/crates/sage-engine/data/catalog/hired_blade.json index 0fce6b8f..a4894279 100644 --- a/crates/sage-engine/data/catalog/hired_blade.json +++ b/crates/sage-engine/data/catalog/hired_blade.json @@ -13,7 +13,7 @@ "colors": [ "black" ], - "power": 2, + "power": 3, "toughness": 2, "keywords": [ "flash" diff --git a/crates/sage-engine/data/catalog/magistrate_s_scepter.json b/crates/sage-engine/data/catalog/magistrate_s_scepter.json index 62c593c8..3b607004 100644 --- a/crates/sage-engine/data/catalog/magistrate_s_scepter.json +++ b/crates/sage-engine/data/catalog/magistrate_s_scepter.json @@ -5,7 +5,7 @@ "types": [ "artifact" ], - "mana_cost": "{4}", + "mana_cost": "{3}", "colors": [], "abilities": [ { diff --git a/crates/sage-engine/data/catalog/marauder_s_axe.json b/crates/sage-engine/data/catalog/marauder_s_axe.json index 7446c576..bae88362 100644 --- a/crates/sage-engine/data/catalog/marauder_s_axe.json +++ b/crates/sage-engine/data/catalog/marauder_s_axe.json @@ -8,13 +8,13 @@ "subtypes": [ "Equipment" ], - "mana_cost": "{3}", + "mana_cost": "{2}", "colors": [], "attachment": { "kind": "equipment", "attach_to": "any_creature_you_control", "equip": "{2}", "power": 2, - "toughness": 1 + "toughness": 0 } } diff --git a/crates/sage-engine/data/catalog/mistcaller.json b/crates/sage-engine/data/catalog/mistcaller.json index 6259dfaa..ff936b03 100644 --- a/crates/sage-engine/data/catalog/mistcaller.json +++ b/crates/sage-engine/data/catalog/mistcaller.json @@ -6,17 +6,15 @@ "creature" ], "subtypes": [ - "Spirit" + "Merfolk", + "Wizard" ], - "mana_cost": "{1}{U}", + "mana_cost": "{U}", "colors": [ "blue" ], - "power": 2, + "power": 1, "toughness": 1, - "keywords": [ - "flash" - ], "abilities": [ { "type": "activated", diff --git a/crates/sage-engine/data/catalog/ravenous_harpy.json b/crates/sage-engine/data/catalog/ravenous_harpy.json index 306229ab..fd09b743 100644 --- a/crates/sage-engine/data/catalog/ravenous_harpy.json +++ b/crates/sage-engine/data/catalog/ravenous_harpy.json @@ -23,7 +23,7 @@ "cost": [ { "kind": "mana", - "mana": "{B}" + "mana": "{1}" }, { "kind": "sacrifice", @@ -36,11 +36,6 @@ "kind": "put_counters_on_self", "counter": "plus_one_plus_one", "count": 1 - }, - { - "kind": "gain_life", - "player_ref": "controller", - "amount": 1 } ] } diff --git a/crates/sage-engine/data/catalog/regal_bloodlord.json b/crates/sage-engine/data/catalog/regal_bloodlord.json index 8135f504..7f5bdf43 100644 --- a/crates/sage-engine/data/catalog/regal_bloodlord.json +++ b/crates/sage-engine/data/catalog/regal_bloodlord.json @@ -7,10 +7,11 @@ ], "subtypes": [ "Vampire", - "Noble" + "Soldier" ], - "mana_cost": "{3}{B}{B}", + "mana_cost": "{3}{W}{B}", "colors": [ + "white", "black" ], "power": 2, @@ -24,7 +25,7 @@ "event": { "beginning_of_step": { "step": "end_step", - "whose_turn": "yours" + "whose_turn": "each" } }, "effects": [ @@ -48,8 +49,8 @@ "colors": [ "black" ], - "power": 2, - "toughness": 2, + "power": 1, + "toughness": 1, "keywords": [ "flying" ] diff --git a/crates/sage-engine/data/catalog/runic_armasaur.json b/crates/sage-engine/data/catalog/runic_armasaur.json index 3812d9f5..862ac5f6 100644 --- a/crates/sage-engine/data/catalog/runic_armasaur.json +++ b/crates/sage-engine/data/catalog/runic_armasaur.json @@ -8,11 +8,11 @@ "subtypes": [ "Dinosaur" ], - "mana_cost": "{2}{G}", + "mana_cost": "{1}{G}{G}", "colors": [ "green" ], - "power": 1, + "power": 2, "toughness": 5, "abilities": [ { diff --git a/crates/sage-engine/data/catalog/sigiled_sword_of_valeron.json b/crates/sage-engine/data/catalog/sigiled_sword_of_valeron.json index 02f20b83..a9211eac 100644 --- a/crates/sage-engine/data/catalog/sigiled_sword_of_valeron.json +++ b/crates/sage-engine/data/catalog/sigiled_sword_of_valeron.json @@ -30,7 +30,6 @@ { "kind": "create_token", "count": 1, - "tapped": true, "attacking": true, "token": { "name": "Knight", diff --git a/crates/sage-engine/src/card/attachment.rs b/crates/sage-engine/src/card/attachment.rs index 500a2aba..fc73dfcd 100644 --- a/crates/sage-engine/src/card/attachment.rs +++ b/crates/sage-engine/src/card/attachment.rs @@ -275,7 +275,7 @@ mod tests { attach_to: TargetSpec::AnyCreatureYouControl, equip: Some("{2}".to_string()), power: 2, - toughness: 1, + toughness: 0, count_of: None, keywords: vec![], restrictions: vec![], diff --git a/crates/sage-engine/tests/blocking_an_additional_creature.rs b/crates/sage-engine/tests/blocking_an_additional_creature.rs index fdd87a8b..a0572dc8 100644 --- a/crates/sage-engine/tests/blocking_an_additional_creature.rs +++ b/crates/sage-engine/tests/blocking_an_additional_creature.rs @@ -307,11 +307,11 @@ fn ghastbark_twins_records_its_blocks_in_the_order_it_declared_them() { #[test] fn a_blocker_spreads_one_pool_of_power_across_the_attackers_it_blocks() { - // CR 510.1c/e from the blocker's side: the Twins' 5 power is *one* pool, assigned + // CR 510.1c/e from the blocker's side: the Twins' 7 power is *one* pool, assigned // just-lethal to the first attacker in its order and the remainder to the last — - // not 5 to each, which is what an attacker-driven loop would deal. + // not 7 to each, which is what an attacker-driven loop would deal. // - // A 3/3 first and a 6/6 second: 3 is lethal to the courser, and the 2 left over + // A 3/3 first and a 6/6 second: 3 is lethal to the courser, and the 4 left over // reaches the dreadmaw and stays marked on it. let db = db(); let mut state = main_phase(); @@ -345,21 +345,22 @@ fn a_blocker_spreads_one_pool_of_power_across_the_attackers_it_blocks() { ); assert_eq!( damage(&after, dreadmaw), - Some(2), - "and the remaining 2 reached the second, which survived it" + Some(4), + "and the remaining 4 reached the second, which survived it" ); assert_eq!( damage(&after, twins), None, - "both attackers struck the one blocker back, which is 9 on a 5/5" + "both attackers struck the one blocker back, which is 9 on a 7/7" ); } #[test] fn the_blockers_order_decides_which_attacker_the_damage_reaches() { // The mirror of the test above with the declaration order reversed. The 6/6 is now - // first and soaks all 5 — nothing is left for the 3/3, which walks away untouched. - // Nothing but the order changed, which is what makes the order load-bearing. + // first, so 6 of the Twins' 7 is lethal to *it* and the single point left over is all + // the 3/3 behind it takes. Nothing but the order changed, and it moved the death from + // one attacker to the other, which is what makes the order load-bearing. let db = db(); let mut state = main_phase(); let courser = place(&mut state, &db, "centaur_courser", PlayerId(0)); @@ -387,13 +388,13 @@ fn the_blockers_order_decides_which_attacker_the_damage_reaches() { assert_eq!( damage(&after, dreadmaw), - Some(5), - "the first in the order takes everything it can absorb" + None, + "the first in the order took lethal damage and died (CR 704.5g)" ); assert_eq!( damage(&after, courser), - Some(0), - "and the second, behind a creature that never took lethal, takes nothing" + Some(1), + "and the second took only what the first could not absorb" ); } @@ -404,16 +405,18 @@ fn one_blocked_attacker_still_takes_the_whole_of_its_blockers_power() { // there is something to spread across. let db = db(); let mut state = main_phase(); - let dreadmaw = place(&mut state, &db, "colossal_dreadmaw", PlayerId(0)); + // An 8/8 rather than the 6/6 the spread tests use: the whole pool is only *visible* + // on an attacker that survives it, and 7 kills a 6/6. + let mammoth = place(&mut state, &db, "aggressive_mammoth", PlayerId(0)); let twins = place(&mut state, &db, "ghastbark_twins", PlayerId(1)); - let state = attack_with(&state, &db, &[dreadmaw]); + let state = attack_with(&state, &db, &[mammoth]); let state = apply_action( &state, &Action::DeclareBlockers { blocks: vec![Block { blocker: twins, - attacker: dreadmaw, + attacker: mammoth, }], }, &db, @@ -421,9 +424,9 @@ fn one_blocked_attacker_still_takes_the_whole_of_its_blockers_power() { let after = settle_until(&state, &db, |s| s.step == Step::EndCombat); assert_eq!( - damage(&after, dreadmaw), - Some(5), - "all 5 power on the one attacker it blocked" + damage(&after, mammoth), + Some(7), + "all 7 power on the one attacker it blocked" ); } @@ -435,11 +438,13 @@ fn blocking_two_attackers_leaves_neither_of_them_unblocked() { let db = db(); let mut state = main_phase(); let courser = place(&mut state, &db, "centaur_courser", PlayerId(0)); - let dreadmaw = place(&mut state, &db, "colossal_dreadmaw", PlayerId(0)); + // An 8/8 trampler, because 7 of anything less is swallowed whole by a 7/7 blocker and + // there would be no overflow left to be the point of this test. + let mammoth = place(&mut state, &db, "aggressive_mammoth", PlayerId(0)); let twins = place(&mut state, &db, "ghastbark_twins", PlayerId(1)); let before = state.players[1].life; - let state = attack_with(&state, &db, &[courser, dreadmaw]); + let state = attack_with(&state, &db, &[courser, mammoth]); let state = apply_action( &state, &Action::DeclareBlockers { @@ -450,7 +455,7 @@ fn blocking_two_attackers_leaves_neither_of_them_unblocked() { }, Block { blocker: twins, - attacker: dreadmaw, + attacker: mammoth, }, ], }, @@ -458,7 +463,9 @@ fn blocking_two_attackers_leaves_neither_of_them_unblocked() { ); let after = settle_until(&state, &db, |s| s.step == Step::EndCombat); - // The courser is blocked and has no trample, so it reaches nobody; the dreadmaw - // assigns 5 to the 5/5 blocker and tramples the 1 that is left (CR 702.19e). + // The courser has 3 power against a 7-toughness blocker, so it reaches nobody — the + // trample the Mammoth grants it changes nothing, because there is no lethal-and-then- + // some to spill. The Mammoth itself assigns 7 to the blocker and tramples the 1 that + // is left (CR 702.19e), and that single point is the whole of the life lost. assert_eq!(after.players[1].life, before - 1); } diff --git a/crates/sage-engine/tests/m19_activation_costs.rs b/crates/sage-engine/tests/m19_activation_costs.rs index 0ab6176d..74038024 100644 --- a/crates/sage-engine/tests/m19_activation_costs.rs +++ b/crates/sage-engine/tests/m19_activation_costs.rs @@ -96,6 +96,16 @@ fn on_battlefield(state: &GameState, id: PermanentId) -> bool { state.battlefield.iter().any(|perm| perm.id == id) } +/// The permanent `id`, to stage a board a cost has to refuse — tapped, or freshly +/// arrived. +fn perm_mut(state: &mut GameState, id: PermanentId) -> &mut Permanent { + state + .battlefield + .iter_mut() + .find(|perm| perm.id == id) + .expect("the permanent is on the battlefield") +} + /// Activate `permanent`'s ability `index` with `payment`, and let the ability resolve. fn activate( state: &GameState, @@ -159,6 +169,108 @@ fn a_discard_cost_is_not_offered_from_an_empty_hand() { assert!(valid_actions(&state, &db).contains(&offer(pyromancer, 0))); } +// ----- the printed cost, exactly --------------------------------------------- +// +// Four cards whose authored cost drifted from the printed one and which every gate +// passed anyway (#819, #820). Each test drives the offer, because an offer is where a +// wrong cost is visible: a missing symbol makes an ability *cheaper*, and cheap is the +// direction nothing else complains about. + +#[test] +fn issue_820_the_pyromancers_draw_ability_needs_the_red_in_its_cost() { + // `{R}, {T}, Discard a card: Draw a card.` — the mana half was missing, so the + // ability was offered off an empty pool. The comparison is the same board with one + // red in it, which is what separates "the cost is enforced" from "nothing was ever + // offered". + let db = db(); + let mut state = main_phase(&db); + to_hand(&mut state, &db, "shock"); + let pyromancer = place(&mut state, &db, "dismissive_pyromancer", PlayerId(0)); + state.players[0].mana_pool = sage_engine::ManaPool::default(); + + assert!( + !valid_actions(&state, &db).contains(&offer(pyromancer, 0)), + "an empty pool cannot pay the red pip" + ); + + state.players[0].mana_pool.add(Color::Red, 1); + assert!( + valid_actions(&state, &db).contains(&offer(pyromancer, 0)), + "and one red is enough" + ); +} + +#[test] +fn issue_820_the_pyromancers_damage_ability_taps_as_part_of_its_cost() { + // `{2}{R}, {T}, Sacrifice this creature: …` — the `{T}` was missing, which made the + // ability usable while tapped and on the turn the creature arrived. Both are the same + // omission seen from two sides (CR 302.6). + let db = db(); + let mut state = main_phase(&db); + place(&mut state, &db, "diamond_mare", PlayerId(1)); + let pyromancer = place(&mut state, &db, "dismissive_pyromancer", PlayerId(0)); + assert!( + valid_actions(&state, &db).contains(&offer(pyromancer, 1)), + "untapped and settled in, it is on offer" + ); + + let mut tapped = state.clone(); + perm_mut(&mut tapped, pyromancer).tapped = true; + assert!( + !valid_actions(&tapped, &db).contains(&offer(pyromancer, 1)), + "a tapped creature has no tap symbol left to pay" + ); + + let mut sick = state.clone(); + perm_mut(&mut sick, pyromancer).entered_turn = sick.turn; + assert!( + !valid_actions(&sick, &db).contains(&offer(pyromancer, 1)), + "and one that arrived this turn cannot tap either (CR 302.6)" + ); +} + +#[test] +fn issue_820_the_trashmaster_destroys_an_artifact_for_a_goblin_and_nothing_else() { + // `Sacrifice a Goblin: Destroy target artifact.` — the whole cost is the sacrifice. + // An authored `{1}{R}` beside it made the ability unofferable off an empty pool, which + // is a card that simply does less than it prints. + let db = db(); + let mut state = main_phase(&db); + let trashmaster = place(&mut state, &db, "goblin_trashmaster", PlayerId(0)); + place(&mut state, &db, "diamond_mare", PlayerId(1)); + state.players[0].mana_pool = sage_engine::ManaPool::default(); + + assert!( + valid_actions(&state, &db).contains(&offer(trashmaster, 1)), + "a Goblin to spend is the only thing it asks for" + ); +} + +#[test] +fn issue_820_the_harpy_costs_one_generic_and_gains_no_life() { + // `{1}, Sacrifice another creature: Put a +1/+1 counter on this creature.` — the cost + // was authored `{B}` and the ability carried a life gain the card does not print. One + // colorless in the pool proves the first; the life total proves the second. + let db = db(); + let mut state = main_phase(&db); + let harpy = place(&mut state, &db, "ravenous_harpy", PlayerId(0)); + let food = place(&mut state, &db, "centaur_courser", PlayerId(0)); + state.players[0].mana_pool = sage_engine::ManaPool::default(); + state.players[0].mana_pool.add_colorless(1); + let life = state.players[0].life; + + assert!( + valid_actions(&state, &db).contains(&offer(harpy, 0)), + "one generic pays it — no black required" + ); + + let after = activate(&state, &db, harpy, 0, vec![CostPayment::Sacrifice(food)]); + assert_eq!( + after.players[0].life, life, + "and nothing on the card gains life" + ); +} + // ----- what the cost accepts ------------------------------------------------ #[test] @@ -169,7 +281,6 @@ fn the_sacrifice_is_charged_and_the_ability_still_does_what_it_says() { let mut state = main_phase(&db); let harpy = place(&mut state, &db, "ravenous_harpy", PlayerId(0)); let food = place(&mut state, &db, "centaur_courser", PlayerId(0)); - let life = state.players[0].life; let after = activate(&state, &db, harpy, 0, vec![CostPayment::Sacrifice(food)]); @@ -184,7 +295,6 @@ fn the_sacrifice_is_charged_and_the_ability_still_does_what_it_says() { 1, "the ability resolved" ); - assert_eq!(after.players[0].life, life + 1); } #[test] diff --git a/crates/sage-engine/tests/m19_attacking_tokens.rs b/crates/sage-engine/tests/m19_attacking_tokens.rs index 9a3a4e85..3c3e7ac9 100644 --- a/crates/sage-engine/tests/m19_attacking_tokens.rs +++ b/crates/sage-engine/tests/m19_attacking_tokens.rs @@ -271,6 +271,45 @@ fn issue_734_a_created_attacker_can_be_blocked() { ); } +/// The other half of the pair: `create a 2/2 white Knight creature token with vigilance +/// **that's attacking**` — attacking and **untapped**, because that is all the card says +/// (#819, #820). The Knight was authored tapped, which is a token that arrives already +/// spent for a card whose whole point is that it does not tap. +#[test] +fn issue_820_the_sigiled_swords_knight_arrives_attacking_and_untapped() { + let db = db(); + let mut state = main_phase(); + let bearer = place(&mut state, &db, "onakke_ogre", PlayerId(0)); + let sword = place(&mut state, &db, "sigiled_sword_of_valeron", PlayerId(0)); + if let Some(perm) = state.battlefield.iter_mut().find(|perm| perm.id == sword) { + perm.attached_to = Some(bearer); + } + + let state = declare_attack(&state, &db, bearer); + assert_eq!( + state.stack.len(), + 1, + "the granted attack trigger is on the stack" + ); + let state = resolve_stack(&state, &db); + + let knights = tokens(&state); + assert_eq!(knights.len(), 1, "one Knight token"); + let knight = knights[0]; + assert_eq!( + attack_target_of(&state, knight.id), + attack_target_of(&state, bearer), + "it joins the attack the equipped creature declared" + ); + assert!( + !knight.tapped, + "and it is untapped — the card says attacking, and nothing else" + ); + let current = characteristics(&state, knight.id, &db); + assert_eq!((current.power, current.toughness), (Some(2), Some(2))); + assert!(current.keywords.contains(&Keyword::Vigilance)); +} + // ----- what it is *not* ----------------------------------------------------- /// CR 506.3c: a token put onto the battlefield attacking was never **declared** as an diff --git a/crates/sage-engine/tests/m19_equipment.rs b/crates/sage-engine/tests/m19_equipment.rs index 32887a29..776fad0f 100644 --- a/crates/sage-engine/tests/m19_equipment.rs +++ b/crates/sage-engine/tests/m19_equipment.rs @@ -185,8 +185,8 @@ fn issue_728_an_equipment_enters_attached_to_nothing_and_equips_a_creature() { power(&state, &db, bear), characteristics(&state, bear, &db).toughness ), - (Some(5), Some(4)), - "a 3/3 holding a +2/+1 Axe is a 5/4 (CR 613 layers 7c and 6)" + (Some(5), Some(3)), + "a 3/3 holding a +2/+0 Axe is a 5/3 (CR 613 layers 7c and 6)" ); } diff --git a/crates/sage-engine/tests/m19_granted_abilities.rs b/crates/sage-engine/tests/m19_granted_abilities.rs index aa27ecac..cb67a008 100644 --- a/crates/sage-engine/tests/m19_granted_abilities.rs +++ b/crates/sage-engine/tests/m19_granted_abilities.rs @@ -166,7 +166,7 @@ fn offered_indices(state: &GameState, db: &CardDatabase, permanent: PermanentId) // ----- an Aura on a land ---------------------------------------------------- #[test] -fn issue_740_gift_of_paradise_enchants_a_land_and_gains_two_life() { +fn issue_740_gift_of_paradise_enchants_a_land_and_gains_three_life() { // CR 303.4a: an Aura's enchant restriction is whatever class its card names, and a // land is one of them. The whole cast goes through the ordinary path — one required // target slot, chosen at announcement — and the entry trigger resolves behind it. @@ -191,8 +191,8 @@ fn issue_740_gift_of_paradise_enchants_a_land_and_gains_two_life() { assert_eq!(attached_to(&state, aura), Some(land)); assert_eq!( state.players[0].life, - life + 2, - "the enters-the-battlefield trigger gained two life" + life + 3, + "the enters-the-battlefield trigger gained three life" ); } diff --git a/crates/sage-engine/tests/m19_hired_blade.rs b/crates/sage-engine/tests/m19_hired_blade.rs index 93b7144b..e83fb51d 100644 --- a/crates/sage-engine/tests/m19_hired_blade.rs +++ b/crates/sage-engine/tests/m19_hired_blade.rs @@ -159,7 +159,7 @@ fn issue_748_a_creature_without_flash_is_refused_by_apply_action_itself() { #[test] fn issue_748_flash_changes_only_the_timing() { // What the keyword does *not* do is as much of the card as what it does: the - // creature that arrives is an ordinary 2/2 with no abilities, entering the way any + // creature that arrives is an ordinary 3/2 with no abilities, entering the way any // other creature spell's would. let db = db(); let mut state = stocked(); @@ -177,6 +177,6 @@ fn issue_748_flash_changes_only_the_timing() { .find(|p| p.instance == blade.id) .expect("the creature resolved onto the battlefield"); let face = perm.printed.face(&db).expect("a printed face"); - assert_eq!((face.power(), face.toughness()), (Some(2), Some(2))); + assert_eq!((face.power(), face.toughness()), (Some(3), Some(2))); assert!(state.stack.is_empty(), "and nothing is left over"); } diff --git a/crates/sage-engine/tests/m19_life_gained_this_turn.rs b/crates/sage-engine/tests/m19_life_gained_this_turn.rs index ac45a327..00fdf5bc 100644 --- a/crates/sage-engine/tests/m19_life_gained_this_turn.rs +++ b/crates/sage-engine/tests/m19_life_gained_this_turn.rs @@ -8,10 +8,11 @@ //! still answers yes — and the events stop counting at the turn boundary, which only a //! walk that crosses one can show. //! -//! Regal Bloodlord asks the plain question at its controller's end step; Resplendent -//! Angel asks it with a threshold of five at every end step. Sovereign's Bite is how a -//! seat takes life away from itself: it is printed as "target player loses 3 life and -//! you gain 3 life", and nothing stops the caster from being the target. +//! Regal Bloodlord and Resplendent Angel both ask at **every** end step, whoever's turn +//! it is; the Angel's question carries a threshold of five and the Bloodlord's does not. +//! Sovereign's Bite is how a seat takes life away from itself: it is printed as "target +//! player loses 3 life and you gain 3 life", and nothing stops the caster from being the +//! target. #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] use sage_engine::{ @@ -177,6 +178,51 @@ fn tokens(state: &GameState, db: &CardDatabase, name: &str) -> usize { .count() } +/// The trigger is `at the beginning of **each** end step` (#819, #820): it was authored +/// as the controller's own, so an opponent's turn made no Bat however much life the +/// Bloodlord's controller had gained in it. The Bat is a **1/1**, not the 2/2 it was +/// authored as — a token's printed size is the only thing on it, so a wrong one is wrong +/// everywhere it is read. +#[test] +fn issue_820_the_bloodlord_asks_at_every_end_step_and_makes_a_one_one_bat() { + let db = db(); + let mut state = fresh_game(&db); + place(&mut state, &db, "regal_bloodlord", PlayerId(0)); + + // Turn 2 belongs to seat 1. Seat 0 holds the Bloodlord and gains the life on their + // opponent's turn, with an instant — which is the only turn this trigger's scope is + // observable on, since the life-gain window closes at the turn boundary. + let state = walk_until(&state, &db, |s| { + s.turn == 2 && s.step == Step::PrecombatMain + }); + let state = cast_and_resolve(&state, &db, "revitalize", Vec::new()); + + let state = walk_until(&state, &db, |s| s.turn == 3 && s.step == Step::Upkeep); + assert_eq!( + tokens(&state, &db, "Bat"), + 1, + "the opponent's end step is an end step, and the Bloodlord watched it" + ); + + let bat = state + .battlefield + .iter() + .find(|perm| { + perm.printed.card().is_none() + && perm + .printed + .face(&db) + .is_some_and(|face| face.name() == "Bat") + }) + .expect("the Bat is on the battlefield"); + let face = bat.printed.face(&db).expect("a printed face"); + assert_eq!( + (face.power(), face.toughness()), + (Some(1), Some(1)), + "a 1/1 black Bat creature token with flying" + ); +} + /// A turn with no life gained in it makes no Bat: the trigger fires either way, and the /// condition is what decides whether anything happens. #[test] diff --git a/crates/sage-engine/tests/m19_mistcaller.rs b/crates/sage-engine/tests/m19_mistcaller.rs index a3bcc086..8415b8fc 100644 --- a/crates/sage-engine/tests/m19_mistcaller.rs +++ b/crates/sage-engine/tests/m19_mistcaller.rs @@ -1,13 +1,15 @@ -//! Mistcaller (M19 #62): flash (CR 702.8), and a **one-shot replacement effect** -//! (CR 614.1b) that exiles the next nontoken creature to enter the battlefield this turn -//! without being cast. +//! Mistcaller (M19 #62): a **one-shot replacement effect** (CR 614.1b) that exiles the +//! next nontoken creature to enter the battlefield this turn without being cast. +//! +//! The card prints **no keyword** — it is a `{U}` 1/1 Merfolk Wizard whose whole text is +//! the sacrifice ability. It is not a flash creature and never was; that is Hired Blade, +//! whose file owns the keyword's tests. //! //! Every test drives the **real** [`apply_action`] pipeline over the bundled catalog. -//! What is under test is not that the definition parses — it is that the card can be held -//! up like an instant, that sacrificing it arms a replacement, that the replacement -//! catches a reanimation and misses a cast, and that it is spent by the first entry it -//! catches. Cards are named by their authored `functional_id`, never by an interned -//! handle (ADR 0008 §3). +//! What is under test is not that the definition parses — it is that sacrificing the card +//! arms a replacement, that the replacement catches a reanimation and misses a cast, and +//! that it is spent by the first entry it catches. Cards are named by their authored +//! `functional_id`, never by an interned handle (ADR 0008 §3). #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] use sage_engine::{ @@ -113,21 +115,24 @@ fn reanimate_skeleton( (apply_action(&state, &Action::PassPriority, db), card) } -// ----- flash ---------------------------------------------------------------- +// ----- the printed card ----------------------------------------------------- #[test] -fn issue_731_mistcaller_is_castable_at_instant_speed_on_an_opponents_turn() { - // CR 702.8: flash lifts the sorcery-speed gate and nothing else. The comparison is a - // creature without it, in the same hand at the same moment. +fn mistcaller_has_no_flash_and_is_bound_by_the_sorcery_speed_gate() { + // The card carries no keyword at all, so the ordinary CR 302.1 timing rule applies to + // it exactly as it does to any other creature spell. The comparison is a creature that + // *does* have flash, in the same hand at the same moment — the same shape Hired Blade's + // file uses, run the other way round, so "the gate is there" is distinguishable from + // "nothing was offered for some other reason". let db = db(); let mut state = main_phase(); // Player 1's turn, player 0 holding priority: sorcery speed is unavailable to them. state.active_player = PlayerId(1); state.priority = PlayerId(0); let mistcaller = state.new_instance(cid(&db, "mistcaller")); - let vanilla = state.new_instance(cid(&db, "tolarian_scholar")); + let flashed = state.new_instance(cid(&db, "hired_blade")); state.players[0].hand.push(mistcaller); - state.players[0].hand.push(vanilla); + state.players[0].hand.push(flashed); let offers = valid_actions(&state, &db); let castable = |card: CardInstance| { @@ -139,10 +144,13 @@ fn issue_731_mistcaller_is_castable_at_instant_speed_on_an_opponents_turn() { payment: Vec::new(), }) }; - assert!(castable(mistcaller), "flash ignores the sorcery-speed gate"); assert!( - !castable(vanilla), - "a creature without flash is still bound by it" + !castable(mistcaller), + "Mistcaller prints no flash, so it waits for its own main phase" + ); + assert!( + castable(flashed), + "and a creature that does print it is offered at the same moment" ); } diff --git a/crates/sage-server/src/rules_text/tests.rs b/crates/sage-server/src/rules_text/tests.rs index e642f67a..108c85de 100644 --- a/crates/sage-server/src/rules_text/tests.rs +++ b/crates/sage-server/src/rules_text/tests.rs @@ -489,7 +489,7 @@ fn issue_728_an_equipment_states_its_grant_and_its_equip_ability() { let db = bundled(); assert_eq!( text_of(&db, "marauder_s_axe"), - "Equipped creature gets +2/+1.\n\ + "Equipped creature gets +2/+0.\n\ {2}: Attach Marauder's Axe to target creature you control." ); @@ -1247,8 +1247,8 @@ fn a_life_gained_condition_states_its_threshold_only_when_there_is_one() { let db = bundled(); assert_eq!( text_of(&db, "regal_bloodlord"), - "Flying\nAt the beginning of your end step, if you gained life this turn, \ - you create a 2/2 black Bat creature token with flying." + "Flying\nAt the beginning of each end step, if you gained life this turn, \ + you create a 1/1 black Bat creature token with flying." ); assert_eq!( text_of(&db, "resplendent_angel"), @@ -1497,20 +1497,19 @@ fn issue_721_an_activation_cost_states_what_the_player_must_spend() { let db = bundled(); assert_eq!( text_of(&db, "ravenous_harpy"), - "Flying\n{B}, Sacrifice another creature: Put a +1/+1 counter on Ravenous Harpy \ - and you gain 1 life." + "Flying\n{1}, Sacrifice another creature: Put a +1/+1 counter on Ravenous Harpy." ); // A subtype names the class on its own: a Goblin is a Goblin whatever else it is, and // the Trashmaster is one, so with no *another* it is a legal payment for its own cost. assert_eq!( text_of(&db, "goblin_trashmaster"), "Other Goblins you control get +1/+1.\n\ - {1}{R}, Sacrifice a Goblin: Destroy target artifact." + Sacrifice a Goblin: Destroy target artifact." ); assert_eq!( text_of(&db, "dismissive_pyromancer"), - "{T}, Discard a card: Draw a card.\n\ - {2}{R}, Sacrifice this permanent: Dismissive Pyromancer deals 4 damage to \ + "{R}, {T}, Discard a card: Draw a card.\n\ + {2}{R}, {T}, Sacrifice this permanent: Dismissive Pyromancer deals 4 damage to \ target creature." ); } @@ -1545,15 +1544,13 @@ fn issue_721_a_costs_size_and_the_amount_that_reads_it_are_both_stated() { } /// A created replacement reads as the sentence a card prints it in: the event, the turn -/// it lasts, the qualifier on the event, and what happens instead (CR 614.1b). The -/// keyword line above it is the flash the card is held up with (CR 702.8). +/// it lasts, the qualifier on the event, and what happens instead (CR 614.1b). #[test] fn issue_731_a_created_replacement_reads_as_the_next_time_this_turn() { let db = bundled(); assert_eq!( text_of(&db, "mistcaller"), - "Flash\n\ - Sacrifice this permanent: The next time a nontoken creature would enter the \ + "Sacrifice this permanent: The next time a nontoken creature would enter the \ battlefield this turn without being cast, exile it instead." ); } @@ -1664,7 +1661,7 @@ fn issue_740_a_granted_ability_is_quoted_on_the_card_that_grants_it() { let db = bundled(); assert_eq!( text_of(&db, "gift_of_paradise"), - "When Gift of Paradise enters the battlefield, you gain 2 life.\n\ + "When Gift of Paradise enters the battlefield, you gain 3 life.\n\ Enchant land.\n\ Enchanted land has \"{T}: Add two mana of any one color.\"" ); @@ -1893,7 +1890,7 @@ fn issue_706_animating_reads_as_what_it_becomes_and_for_how_long() { text_of(&db, "sigiled_sword_of_valeron"), "Equipped creature gets +2/+0.\nEquipped creature has vigilance.\nEquipped \ creature is a Knight in addition to its other types.\nEquipped creature has \ - \"Whenever this creature attacks, you create a tapped 2/2 white Knight creature \ + \"Whenever this creature attacks, you create a 2/2 white Knight creature \ token with vigilance that's attacking.\"\n{3}: Attach Sigiled Sword of Valeron \ to target creature you control." ); diff --git a/crates/sage-server/src/view/payment/activation.rs b/crates/sage-server/src/view/payment/activation.rs index aadb3b89..7a621b4f 100644 --- a/crates/sage-server/src/view/payment/activation.rs +++ b/crates/sage-server/src/view/payment/activation.rs @@ -415,6 +415,9 @@ mod tests { false, false, ); + // `{R}, {T}, Discard a card` — the mana half has to be payable for the + // activation to be offered at all. + state.players[0].mana_pool.add(sage_engine::Color::Red, 1); let action = activation(&state, &db, pyromancer, "Discard"); let (prompt, zone, count, candidates) = zone_slot(&action, DISCARD_SLOT); @@ -469,6 +472,9 @@ mod tests { false, false, ); + // `{R}, {T}, Discard a card` — the mana half has to be payable for the + // activation to be offered at all. + state.players[0].mana_pool.add(sage_engine::Color::Red, 1); let action = activation(&state, &db, pyromancer, "Discard"); let bound = resolve_action(