Caveat: The messages below were generated by the user while running local commands. DO NOT respond to these messages or otherwise consider them in your response unless the user explicitly asks you to.
Caveat: Caveat: The messages below were generated by the user while running local commands. DO NOT respond to these messages or otherwise consider them in your response unless the user explicitly asks you to.
clear
Read docs/work-queue.md and do the next unchecked batch. For each batch:
mtg-engine/src/abilities.rs, mtg-engine/src/game.rs) to understand current Effect/StaticEffect variantsgame.rs mod tests)cargo check -p mtg-cards && cargo test --libdocs/{fdn,tla,tdm,ecl}-remediation.md, update ROADMAP.mddocs/work-queue.md with today's dateIf a batch turns out to be wrong or more complex than described, update the queue with what you learned and move on to the next one.
I'll start by reading the work queue to understand what's next.
Batch 2 is the next unchecked batch. It has several sub-batches (2a through 2f). Let me read the engine files to understand the current state, then work through each sub-batch.
Let me start by reading the engine files and the relevant card files in parallel.
Batch 2 is next - "Easy swaps — existing typed effects" with sub-batches 2a through 2f. Let me read the engine files and investigate the cards.
The game.rs file is very large. Let me look at specific parts I need. Let me now investigate each sub-batch by finding the exact lines in the card files. Let me search for the specific patterns mentioned in the work queue.
Now let me investigate each sub-batch by finding the exact card lines. Let me search in parallel.
No matches found
No matches found
No matches found
mtg-rl/mtg-cards/src/sets/ecl.rs:354: vec![Effect::Custom("Put two -1/-1 counters on Blighted Blackthorn.".into()), Effect::draw_cards(1), Effect::lose_life(1)],
mtg-rl/mtg-cards/src/sets/ecl.rs:358: vec![Effect::Custom("Put two -1/-1 counters on Blighted Blackthorn.".into()), Effect::draw_cards(1), Effect::lose_life(1)],
mtg-rl/mtg-cards/src/sets/ecl.rs:464: vec![Cost::pay_mana("{1}{B}"), Cost::Custom("Put a -1/-1 counter on this creature".into())],
mtg-rl/mtg-cards/src/sets/ecl.rs:596: vec![Effect::Custom("Put a -1/-1 counter on Dream Seizer. Each opponent discards a card.".into())],
mtg-rl/mtg-cards/src/sets/ecl.rs:1438: vec![Effect::Custom("Put two -1/-1 counters on Sourbread Auntie.".into()), Effect::create_token("1/1 Goblin", 2)],
mtg-rl/mtg-cards/src/sets/ecl.rs:1467: vec![Effect::Custom("Put a -1/-1 counter on Sting-Slinger.".into()), Effect::damage_opponents(2)],
mtg-rl/mtg-cards/src/sets/ecl.rs:1691: vec![Effect::Custom("Put a -1/-1 counter on Warren Torchmaster. Target creature gains haste until end of turn.".into())],
mtg-rl/mtg-cards/src/sets/ecl.rs:1843: abilities: vec![Ability::spell(id, vec![Effect::Custom("Put two -1/-1 counters on each creature.".into())], TargetSpec::None)],mtg-rl/mtg-cards/src/sets/ecl.rs:715: vec![Effect::Custom("Remove a -1/-1 counter from Encumbered Reejerey.".into())],
mtg-rl/mtg-cards/src/sets/ecl.rs:992: vec![Effect::scry(1), Effect::Custom("Remove a -1/-1 counter from Heirloom Auntie.".into())],
mtg-rl/mtg-cards/src/sets/ecl.rs:1256: vec![Effect::Custom("Remove a -1/-1 counter from Reluctant Dounguard.".into())],
mtg-rl/mtg-cards/src/sets/ecl.rs:2197: vec![Effect::Custom("Remove a -1/-1 counter from this creature.".into())],No matches found
No matches found
Some of the patterns from the work queue aren't found. Let me search more broadly for the batches that didn't match. The line numbers may have shifted since the queue was written. Let me search for the actual cards mentioned.
Some patterns weren't found — line numbers shifted or text changed. Let me search more broadly for the remaining items.
171- registry.register("Deathmark", deathmark, "FDN");
172- registry.register("Crash Through", crash_through, "FDN");
173- registry.register("Dragon Fodder", dragon_fodder, "FDN");
174: registry.register("Day of Judgment", day_of_judgment, "FDN");
175- registry.register("Angelic Edict", angelic_edict, "FDN");
176- registry.register("Chart a Course", chart_a_course, "FDN");
177- registry.register("Macabre Waltz", macabre_waltz, "FDN");
--
2410-
2411-fn day_of_judgment(id: ObjectId, owner: PlayerId) -> CardData {
2412- // Sorcery {2}{W}{W}. Destroy all creatures.
2413: CardData { id, owner, name: "Day of Judgment".into(), mana_cost: ManaCost::parse("{2}{W}{W}"),
2414- card_types: vec![CardType::Sorcery], rarity: Rarity::Rare,
2415- abilities: vec![
2416- Ability::spell(id,21- registry.register("Brazen Scourge", brazen_scourge, "FDN");
22- registry.register("Burglar Rat", burglar_rat, "FDN");
23- registry.register("Campus Guide", campus_guide, "FDN");
24: registry.register("Diregraf Ghoul", diregraf_ghoul, "FDN");
25- registry.register("Dragon Trainer", dragon_trainer, "FDN");
26- registry.register("Druid of the Cowl", druid_of_the_cowl, "FDN");
27- registry.register("Erudite Wizard", erudite_wizard, "FDN");
--
634-}
635-
636-fn diregraf_ghoul(id: ObjectId, owner: PlayerId) -> CardData {
637: // Diregraf Ghoul enters the battlefield tapped.
638: CardData { id, owner, name: "Diregraf Ghoul".into(), mana_cost: ManaCost::parse("{B}"),
639- card_types: vec![CardType::Creature], subtypes: vec![SubType::Zombie],
640- power: Some(2), toughness: Some(2), rarity: Rarity::Uncommon,
641- abilities: vec![
642- Ability::static_ability(id,
643: "Diregraf Ghoul enters the battlefield tapped.",
644- vec![StaticEffect::Custom("Enters tapped.".into())]),
645- ],
646- ..Default::default() }119- registry.register("Driver of the Dead", driver_of_the_dead, "FDN");
120- registry.register("Fanatical Firebrand", fanatical_firebrand, "FDN");
121- registry.register("Goblin Smuggler", goblin_smuggler, "FDN");
122: registry.register("Frenzied Goblin", frenzied_goblin, "FDN");
123- registry.register("Ghitu Lavarunner", ghitu_lavarunner, "FDN");
124- registry.register("Courageous Goblin", courageous_goblin, "FDN");
125- registry.register("Heartfire Immolator", heartfire_immolator, "FDN");
--
1771-
1772-fn frenzied_goblin(id: ObjectId, owner: PlayerId) -> CardData {
1773- / 1/1 Goblin Berserker for {R}. Attacks: pay {R}, creature can't block.
1774: CardData { id, owner, name: "Frenzied Goblin".into(), mana_cost: ManaCost::parse("{R}"),
1775- card_types: vec![CardType::Creature], subtypes: vec![SubType::Goblin, SubType::Berserker],
1776- power: Some(1), toughness: Some(1), rarity: Rarity::Uncommon,
1777- abilities: vec![
1778- Ability::attacks_triggered(id,
1779: "Whenever Frenzied Goblin attacks, you may pay {R}. If you do, target creature can't block this turn.",
1780- vec![Effect::Custom("Target creature can't block this turn.".into())],
1781- TargetSpec::Creature),
1782- ],No matches found
427- registry.register("Krenko, Mob Boss", krenko_mob_boss, "FDN");
428- registry.register("Lightshell Duo", lightshell_duo, "FDN");
429- registry.register("Linden, the Steadfast Queen", linden_the_steadfast_queen, "FDN");
430: registry.register("Loot, Exuberant Explorer", loot_exuberant_explorer, "FDN");
431- registry.register("Mentor of the Meek", mentor_of_the_meek, "FDN");
432- registry.register("Meteor Golem", meteor_golem, "FDN");
433- registry.register("Micromancer", micromancer, "FDN");
--
6337- rarity: Rarity::Common,
6338- abilities: vec![
6339- Ability::triggered(id,
6340: "When this creature enters, draw a card, then discard a card.",
6341- vec![EventType::EnteredTheBattlefield],
6342: vec![Effect::Custom("When this creature enters, draw a card, then discard a card.".into())],
6343- TargetSpec::None),
6344- ],
6345- ..Default::default() }
--
6500- ..Default::default() }
6501-}
6502-
6503:fn loot_exuberant_explorer(id: ObjectId, owner: PlayerId) -> CardData {
6504: CardData { id, owner, name: "Loot, Exuberant Explorer".into(), mana_cost: ManaCost::parse("{2}{G}"),
6505- card_types: vec![CardType::Creature],
6506- subtypes: vec![SubType::Beast, SubType::Noble],
6507- supertypes: vec![SuperType::Legendary],
--
6893- rarity: Rarity::Common,
6894- abilities: vec![
6895- Ability::spell(id,
6896: vec![Effect::Custom("Counter target spell. Draw a card, then discard a card.".into())],
6897- TargetSpec::None),
6898- ],
6899- ..Default::default() }1644- "Whenever an opponent casts a spell during your turn, you may pay 1 life. If you do, draw a card.",
1645- vec![EventType::SpellCast],
1646: vec![Effect::Custom("Pay 1 life to draw a card.".into())],
1647- TargetSpec::None),
1648- ],
--
3979- "When this creature enters, draw a card.",
3980- vec![EventType::EnteredTheBattlefield],
3981: vec![Effect::Custom("When this creature enters, draw a card.".into())],
3982- TargetSpec::None),
3983- ],Good. Let me now look at the specific cards I need to fix more carefully. Let me look at the context around each card to understand the full picture. Let me also look for the "Skoa Veteran" in ECL since it wasn't found - maybe it has a different name.
Let me also check what card is at ECL line 431 and what card has the loot effect in FDN around line 6337.
Let me get more context on the specific cards that need fixing.
424- Ability::enters_battlefield_triggered(id,
425- "Brambleback Brute enters with two -1/-1 counters on it.",
426- vec![Effect::add_counters("-1/-1", 2)],
427- TargetSpec::None),
428- Ability::activated(id,
429: "{1}{R}: Target creature can't block this turn.",
430- vec![Cost::pay_mana("{1}{R}")],
431: vec![Effect::Custom("Target creature can't block this turn.".into())],
432- TargetSpec::Creature),
433- ],
434- ..Default::default() }
435-}
436-Good. Now let me also check the cards around ECL line 1438 (Sourbread Auntie) and line 1467 (Sting-Slinger). And I need to see how AddCounters is handled in game.rs. Let me also check the EntersTapped variant - it already exists in the StaticEffect enum with a filter field. Let me check what the card needs.
Actually, looking at the StaticEffect enum, EntersTapped has filter: String. For Diregraf Ghoul, we just need StaticEffect::EntersTapped { filter: "self".into() } or something. But wait, a static ability on the card itself that says "this enters tapped" - what filter should it use?
Now I need to check how EntersTapped is actually implemented in game.rs to understand the filter behavior.
Let me check how AddCounters and EntersTapped are handled in game.rs, and look at the remaining card contexts.
1034- if let Some(player) = self.state.players.get_mut(&opp) {
1035- player.life -= *amount as i32;
1036- }
1037- }
1038- }
1039: Effect::AddCounters { counter_type, count } => {
1040- let ct = crate::counters::CounterType::from_name(counter_type);
1041- for &target_id in targets {
1042- if let Some(perm) = self.state.battlefield.get_mut(target_id) {
1043- perm.add_counters(ct.clone(), *count);
1044- }No matches found
1283- if let Some(perm) = self.state.battlefield.get_mut(id) {
1284- perm.apply_damage(*amount);
1285- }
1286- }
1287- }
1288: Effect::RemoveCounters { counter_type, count } => {
1289- let ct = crate::counters::CounterType::from_name(counter_type);
1290- for &target_id in targets {
1291- if let Some(perm) = self.state.battlefield.get_mut(target_id) {
1292- perm.counters.remove(&ct, *count);
1293- }Now I have a good picture. Let me analyze the AddCounters and RemoveCounters handling in game.rs. The key issue is: AddCounters currently applies to targets, but these self-counter effects have TargetSpec::None - so targets will be empty and the counters won't get added.
Let me look more carefully at how targets are collected in the resolve function:
Now let me check how AddCounters resolves when TargetSpec::None — the key question for batch 2d.
No matches found
688: fn resolve_top_of_stack(&mut self) {Now let me look at the execute_effects method to understand how AddCounters works with targets:
948: pub fn execute_effects(&mut self, effects: &[Effect], controller: PlayerId, targets: &[ObjectId]) {Now I can see the issue clearly. AddCounters only applies to targets. When the ability has TargetSpec::None (like all the self-counter effects), the targets array is empty, so nothing happens.
I need to add logic to handle the "add counters to self" case. When targets is empty, the effect should apply to the source permanent (the creature that has the ability). But I need the source ObjectId. Let me check how other effects get the source.
Looking at the execute_effects signature:
fn execute_effects(&mut self, effects: &[Effect], controller: PlayerId, targets: &[ObjectId])
There's no source parameter! Let me trace through how the ability resolution passes in the source. Looking back at resolve_top_of_stack, when an ability resolves, it clones the ability data and executes the effects with the targets from the stack item, but the item.id is just the StackItem id, not the source permanent that has the ability.
The ability itself has a source_id field, but that's not being passed to execute_effects. I could either add a new variant that doesn't require targets, or modify execute_effects to accept the source_id and use it as a fallback when targets are empty. Actually, for triggered abilities like ETB, the source permanent is the creature that triggered it, and the ability stores that as source_id. The problem is execute_effects doesn't currently have access to it.
undefinedLet me check how the stack item stores source information.
236-#[derive(Clone, Debug, Serialize, Deserialize)]
237-pub struct ExileZone {
238- /// The source that created this exile zone.
239: pub source_id: ObjectId,
240- /// Human-readable name.
241- pub name: String,
242- /// Cards in this exile zone.
--
266- }
267-
268- /// Exile a card to a named zone (e.g. for "exile until this leaves the battlefield").
269: pub fn exile_to_zone(&mut self, card_id: ObjectId, source_id: ObjectId, zone_name: &str) {
270: let zone = self.zones.entry(source_id).or_insert_with(|| ExileZone {
271: source_id,
272- name: zone_name.to_string(),
273- cards: Vec::new(),
274- });
--
291- }
292-
293- /// Get all cards in a named exile zone.
294: pub fn get_zone(&self, source_id: ObjectId) -> Option<&ExileZone> {
295: self.zones.get(&source_id)
296- }
297-
298- /// Check if a card is in exile (any zone).
--
428-
429-/// An item on the stack (a spell or ability waiting to resolve).
430-#[derive(Clone, Debug, Serialize, Deserialize)]
431:pub struct StackItem {
432- /// Unique ID for this stack object.
433: pub id: ObjectId,
434- /// The kind of stack object (spell or ability).
435- pub kind: StackItemKind,
436- /// Who controls this stack object.
--
452- /// An activated or triggered ability.
453- Ability {
454- /// The source permanent or card.
455: source_id: ObjectId,
456- /// The ability's unique ID.
457- ability_id: AbilityId,
458- /// Human-readable description.The StackItemKind::Ability has a source_id. I need to pass this through to execute_effects so AddCounters and RemoveCounters can fall back to the source when there are no targets. Let me look at how the stack item passes data during resolution.
OK, so the approach is clear. For AddCounters and RemoveCounters, when targets is empty, the effect should apply to the source. But execute_effects doesn't take a source parameter.
The simplest approach: add an Option<ObjectId> parameter for source_id to execute_effects. For abilities, we pass the source_id from the StackItemKind::Ability. For spells, we pass item.id (the card itself).
Actually, a simpler approach: just change the AddCounters/RemoveCounters handling to fall back to the source. But I don't have the source in execute_effects.
I'm reconsidering this. The NOTE in the work queue mentions that AddCounters currently targets via SelectedTargets and these are self-targeting — I need to verify the engine handles "add counters to self" when no target is selected, or if it needs the source object ID in targets. An AddCountersSelf variant would work but feels redundant, so I should modify execute_effects to accept an optional source_id parameter instead. Before committing to this approach, I need to check all the callers of execute_effects to understand the scope of changes required.
Let me check all callers of execute_effects to plan the source_id plumbing.
741: self.execute_effects(&effects, item.controller, &targets);
750: self.execute_effects(&ability.effects, item.controller, &targets);
948: pub fn execute_effects(&mut self, effects: &[Effect], controller: PlayerId, targets: &[ObjectId]) {
2078: game.execute_effects(&[Effect::DrawCards { count: 2 }], p1, &[]);
2108: game.execute_effects(&[Effect::GainLife { amount: 5 }], p1, &[]);
2133: game.execute_effects(&[Effect::lose_life_opponents(3)], p1, &[]);
2171: game.execute_effects(&[Effect::Exile], p1, &[bear_id]);
2209: game.execute_effects(&[Effect::Bounce], p1, &[bear_id]);There are 2 callers in production code (lines 741 and 750) and 5 in tests. The cleanest approach is to add an Option<ObjectId> source parameter to execute_effects. Let me see what source to pass for each:
item.id (the spell card)StackItemKind::Ability { source_id, .. }But I need to extract source_id from the match arm. Let me look at that again:
The .. pattern hides source_id. I'd need to capture it.
So my plan is to update the execute_effects signature with a source parameter, pass Some(item.id) for spell resolution and the captured source_id for ability resolution, then add fallback logic to both counter handlers that applies to the source when targets are empty, and update all test callers to pass None.
Actually, I'm realizing that for Sting-Slinger, the -1/-1 counter isn't an effect at all—it's part of the cost. The ability text reads "{1}{R}, {T}, Put a -1/-1 counter on Sting-Slinger: It deals 2 damage to each opponent," so the counter is a cost component, not an effect. I should be using the Cost::Blight(1) variant that already exists in the Cost enum instead of modeling it as an effect.
Cost::pay_mana("{1}{R}"), Cost::tap_self(), Cost::Blight(1)
But wait, I need to check if Cost::Blight is the right fit here since it puts counters on "a creature you control" generically, whereas this specifically targets self. Let me look at how other blighted cards in the ECL are structured to see if the -1/-1 counters are actually costs or effects in those cases.
Looking at the blighted cards more carefully, I'm seeing a pattern: for triggered abilities like Blighted Blackthorn and Sourbread Auntie, the -1/-1 counters are part of the effect that resolves when the ability triggers, not a cost. But for Sting-Slinger's activated ability, the counter is explicitly part of the cost. So I should model it as a cost, not an effect. The question is whether Cost::Blight(1) works or if I need something more specific to self-targeting. I need to handle the case where AddCounters has no targets — it should default to applying the counters to the source permanent itself. Looking at how the existing ETB trigger works, I can see there's already a pattern for this, so I could either add an AddCountersSelf variant or implement default-to-source logic in the engine. I see the issue now—the abilities get registered when the permanent enters, but their effects don't execute at that moment. The ETB trigger fires separately through the event system, so I need to trace how triggered abilities actually get their targets assigned when they're put on the stack.
Let me understand how triggered abilities get their source_id onto the stack.
No matches found
10:// 4. Put triggered abilities on the stack 342: /// Process the current step: turn-based actions, SBAs, triggers, priority. 353: // -- Handle triggered abilities -- 354: // TODO: Put triggered abilities on the stack (task #13) [Showing results with pagination = limit: 30, offset: 0]
10:// 4. Put triggered abilities on the stack 342: /// Process the current step: turn-based actions, SBAs, triggers, priority. 353: // -- Handle triggered abilities -- 354: // TODO: Put triggered abilities on the stack (task #13)
So triggered abilities are NOT YET implemented for the stack. The game has a TODO for "Put triggered abilities on the stack (task #13)". This means that the ETB counters on Heirloom Auntie don't actually work yet either, and the AddCounters with TargetSpec::None is just a representation that will work later.
Given this state, I should still make the card data correct. The fix for batch 2d is simple then: just swap Custom(...) for the correct typed effects, even if they won't fully work until triggered abilities are implemented. The card representations should be correct.
But wait, the work queue says to add "add counters to self" logic. Since triggered abilities don't work yet at all, let me decide:
I'll fix the card data by replacing the custom effects with proper typed effects, and add source plumbing to the effect execution so that when triggers eventually work, the counter logic will already be in place.
For the ETB self-counter effects, I'm considering whether they should be handled at permanent creation time like EntersTapped, but that's a bigger design change—I'll keep it simple for now and just do the straightforward swaps.
Now I'm mapping out the batch 2 work: DestroyAll for Day of Judgment is a simple swap since it already exists, and EntersTapped for Diregraf Ghoul needs to replace the custom static effect with the proper typed version.
I need to check what filter parameter EntersTapped expects and whether there are existing uses I can reference...
Let me check existing EntersTapped usage in cards and how it's handled by the engine.
mtg-rl/mtg-cards/src/sets/tdm.rs:1322: vec![StaticEffect::EntersTapped { filter: "self".into() }]),
mtg-rl/mtg-cards/src/sets/tdm.rs:1378: vec![StaticEffect::EntersTapped { filter: "self".into() }]),
mtg-rl/mtg-cards/src/sets/tdm.rs:2397: vec![StaticEffect::EntersTapped { filter: "self".into() }]),
mtg-rl/mtg-cards/src/sets/fdn.rs:2860: vec![StaticEffect::EntersTapped { filter: "self".into() }]),
mtg-rl/mtg-cards/src/sets/fdn.rs:2943: vec![StaticEffect::EntersTapped { filter: "self".into() }]),
mtg-rl/mtg-cards/src/sets/fdn.rs:2977: vec![StaticEffect::EntersTapped { filter: "self".into() }]),
mtg-rl/mtg-cards/src/sets/fdn.rs:2995: vec![StaticEffect::EntersTapped { filter: "self".into() }]),Good — EntersTapped { filter: "self".into() } is the established pattern. Now let me look at the Sting-Slinger cost issue and understand how Blight cost works in game.rs.
No matches found
822: if !self.pay_costs(player_id, source_id, &ability.costs) {
858: if !self.pay_costs(player_id, source_id, &ability.costs) {
871: fn pay_costs(&mut self, player_id: PlayerId, source_id: ObjectId, costs: &[Cost]) -> bool {
2216: fn pay_costs_tap_and_sacrifice() {
2246: assert!(game.pay_costs(p1, source_id, &[Cost::TapSelf]));
2251: assert!(!game.pay_costs(p1, source_id, &[Cost::TapSelf]));
2254: assert!(game.pay_costs(p1, source_id, &[Cost::SacrificeSelf]));OK, now I have a clear picture. Let me plan my approach for batch 2:
2a: DestroyAll — Day of Judgment
Effect::Custom("Destroy all creatures.".into()) → Effect::destroy_all("creatures")2b: EntersTapped — Diregraf Ghoul
StaticEffect::Custom("Enters tapped.".into()) → StaticEffect::EntersTapped { filter: "self".into() }2c: CantBlock (target) — Frenzied Goblin, Brambleback Brute (ECL, not "Skoa Veteran")
Effect::Custom("Target creature can't block this turn.".into()) → Effect::CantBlock2d: Self -1/-1 counters — ECL blight creatures
This is the tricky one. The engine needs a modification so that execute_effects receives source information, allowing AddCounters and RemoveCounters to default to the source permanent when no explicit targets are specified. I need to update the function signature to include source context and then thread that through the resolution logic.
For Sting-Slinger, I'm realizing the -1/-1 counter is actually part of the activation cost rather than an effect. Since the Blight mechanic already exists in the Cost enum, I could add Cost::Blight(1) directly to the costs array, or I could use the general self-counter approach depending on how the ability text is structured.
Looking at the actual card text, everything before the colon represents the cost, so I should model it as costs including the mana payment, tap, and the -1/-1 counter, with the damage effect separate. The issue is that Cost::Blight isn't currently handled in the pay_costs function, so I need to implement that handler first.
Actually, let me step back and focus on what's realistic to accomplish. The real goal here is replacing those Custom effect stubs with proper typed effects. I should add source_id plumbing to the effect execution, make the counter operations use source when there's no explicit target, and then fix the card data itself. For Sting-Slinger specifically, I'll move the counter to a Cost::Blight and implement the handler.
Looking at the specific cards in batch 2d, I need to swap out the Custom stubs for AddCounters and RemoveCounters across Blighted Blackthorn, Sourbread Auntie, Sting-Slinger, and Heirloom Auntie. The main engine consideration is whether AddCounters can handle self-targeting when no target is selected, or if I need to pass the source object ID explicitly—might need a small tweak to default to source when targets are empty. handle the case where targets is empty (fall back to source).
Let me plan:
Engine change: Modify execute_effects to accept source: Option<ObjectId>. When AddCounters/RemoveCounters has empty targets and source is Some, apply to source.
Card fixes: I need to swap several cards to use typed effects instead of custom strings. Day of Judgment becomes destroy_all("creatures"), Diregraf Ghoul gets EntersTapped with a self filter, Frenzied Goblin and Brambleback Brute use CantBlock, and I'm replacing a bunch of counter-related custom effects with proper RemoveCounters calls. There are also a few draw and discard effects to standardize, plus some extra cards in the ECL file that need the same counter treatment.
Validation: After making these changes, I'll run the tests to make sure nothing breaks.
Documentation: I should document what changes were made and why, so future maintainers understand the refactoring.
Code Review: Before submitting, I'll review the changes one more time to catch any edge cases or inconsistencies.
Let me start by identifying all the cards that need updating and grouping them by the type of change needed. This will help me tackle them systematically and avoid missing any.
Looking at the cards with -1/-1 counter effects, I can see several patterns:
Effect::remove_countersFor the ones I can fix now, I'll focus on the straightforward removals and the compound effects that can be cleanly separated. The trickier ones like the spell that puts counters on all creatures will need more thought, so I'll defer those. Now I'm identifying which cards need updates across both sets — Day of Judgment and several creatures from FDN, plus a longer list from ECL including Brambleback Brute and others. I should also verify what's at those specific line numbers and check if there are other EntersTapped patterns I'm missing in the codebase.
Let me check the remaining cards I found and check for more matches.
Good. Now I have a complete picture. Let me also check the "Skoa Veteran" card that was mentioned in batch 2c but not found. It may have a different name in the code.
Let me also check what ECL card was at the line reference for can't block.
The search earlier found Brambleback Brute at ECL line 429-432 with the can't block Custom effect. The work queue mentions "Skoa Veteran" but the actual card is "Brambleback Brute."
Now let me also check for more "Enters tapped" Custom patterns in TLA and TDM:
Let me check for any other StaticEffect::Custom("Enters tapped") patterns across all sets.
mtg-rl/mtg-cards/src/sets/tdm.rs:2110: vec![StaticEffect::Custom("Enters tapped unless you control Plains or Island.".into())]),
mtg-rl/mtg-cards/src/sets/tdm.rs:2153: vec![StaticEffect::Custom("Enters tapped unless you control Swamp or Mountain.".into())]),
mtg-rl/mtg-cards/src/sets/fdn.rs:644: vec![StaticEffect::Custom("Enters tapped.".into())]),Only the Diregraf Ghoul has the simple "Enters tapped" Custom — the TDM ones are conditional (batch 6). Good.
Now I have the full picture. Let me also check what the RemoveCounters match arm looks like more carefully and read the rest of execute_effects.
Now I have all the information I need. Let me now implement batch 2.
Let me plan the implementation:
source: Option<ObjectId> parameter to execute_effectsAddCounters: when targets is empty and source is Some, apply counters to sourceRemoveCounters: same fallbackSome(item.id)source_id and pass Some(*source_id)None2a: FDN Day of Judgment - Replace the custom effect string with a dedicated destroy all creatures effect.
2b: FDN Diregraf Ghoul - Replace the custom static effect with an enters tapped effect that targets self.
2c: FDN Frenzied Goblin + ECL Brambleback Brute - Need to handle similar effect conversions.
2d: ECL self -1/-1 counters - Converting multiple cards that add -1/-1 counters to use a dedicated counter effect instead of custom strings. Blighted Blackthorn, Sourbread Auntie, and Sting-Slinger all need this same refactor.
For Sting-Slinger specifically, I considered moving the counter to a cost using Cost::Blight, but that doesn't work since Blight is meant for choosing any creature you control, whereas this effect targets only the card itself. I'll keep it as an effect for now.
Also need to handle Heirloom Auntie's counter removal with a proper RemoveCounters effect.
Looking at similar patterns across other cards—Encumbered Reejerey, Reluctant Dounguard, and an unnamed creature with Ward—they all follow the same counter removal structure.
Then there are a few more straightforward conversions: Icewind Elemental needs to split its enter effect into separate draw and discard actions, and Mistmeadow Council just needs a simple draw effect.
Now I'm finding additional cases like FDN Ref that need attention, including a counter spell that requires both the effect conversion and a target specification fix, plus Warren Torchmaster which has a tricky split between self-targeting counters and creature-targeting haste.
The core issue with Warren Torchmaster is that both effects would share the same target list, so if there's a creature target for the haste, the counter logic would incorrectly apply to that creature instead of the card itself. I'll need to leave this one as a custom effect for now and come back to it once I have a better pattern for handling mixed self and external targets.
Dream Seizer has a similar problem—it needs a counter on itself but also triggers a batch effect (opponent discard), which doesn't fit the current framework. And the creature at line 1843 would require a new variant to handle counters on all creatures at once. I'll skip both of those for now and check what else needs handling.
Good. I now have a complete picture. Let me implement everything. I'll start with the engine change to execute_effects, then fix all the cards.
Let me first modify the engine to support "add/remove counters on self" when no targets are provided.
Now I'll make the changes. The approach:
source: Option<ObjectId> to execute_effectsAddCounters/RemoveCounters: fall back to source when targets is emptyNow update the execute_effects signature and add the source fallback for AddCounters/RemoveCounters.
Now update the AddCounters match arm to fall back to source:
Now update RemoveCounters the same way:
Now update the test callers of execute_effects:
cd /mnt/dest-btrfs/home/jc/src/mage/mtg-rl && grep -n 'execute_effects(' mtg-engine/src/game.rs | grep -v '//'741: self.execute_effects(&effects, item.controller, &targets, Some(item.id));
751: self.execute_effects(&ability.effects, item.controller, &targets, Some(source));
949: pub fn execute_effects(&mut self, effects: &[Effect], controller: PlayerId, targets: &[ObjectId], source: Option<ObjectId>) {
2091: game.execute_effects(&[Effect::DrawCards { count: 2 }], p1, &[]);
2121: game.execute_effects(&[Effect::GainLife { amount: 5 }], p1, &[]);
2146: game.execute_effects(&[Effect::lose_life_opponents(3)], p1, &[]);
2184: game.execute_effects(&[Effect::Exile], p1, &[bear_id]);
2222: game.execute_effects(&[Effect::Bounce], p1, &[bear_id]);sed -i 's/game\.execute_effects(\(.*\), &\[\]);/game.execute_effects(\1, \&[], None);/g; s/game\.execute_effects(\(.*\), &\[bear_id\]);/game.execute_effects(\1, \&[bear_id], None);/g' mtg-engine/src/game.rs(empty result)
grep -n 'execute_effects(' mtg-engine/src/game.rs | grep -v '//'741: self.execute_effects(&effects, item.controller, &targets, Some(item.id));
751: self.execute_effects(&ability.effects, item.controller, &targets, Some(source));
949: pub fn execute_effects(&mut self, effects: &[Effect], controller: PlayerId, targets: &[ObjectId], source: Option<ObjectId>) {
2091: game.execute_effects(&[Effect::DrawCards { count: 2 }], p1, &[], None);
2121: game.execute_effects(&[Effect::GainLife { amount: 5 }], p1, &[], None);
2146: game.execute_effects(&[Effect::lose_life_opponents(3)], p1, &[], None);
2184: game.execute_effects(&[Effect::Exile], p1, &[bear_id], None);
2222: game.execute_effects(&[Effect::Bounce], p1, &[bear_id], None);Good. Now let me add a test for the self-counter behavior, then fix all the cards. Let me first check compilation and tests.
cargo check -p mtg-engine 2>&1 | tail -20 Checking mtg-engine v0.1.0 (/mnt/dest-btrfs/home/jc/src/mage/mtg-rl/mtg-engine)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.20sEngine compiles. Now let me add a test for self-counters, then fix all the cards. Let me find the test section in game.rs.
Now let me add a test for self-counters at the end of the test module.
cargo test -p mtg-engine --lib 2>&1 | tail -20test watchers::tests::track_creatures_died ... ok test watchers::tests::custom_watcher_card_scope ... ok test watchers::tests::track_enters_battlefield ... ok test watchers::tests::track_life_gained ... ok test zones::tests::battlefield_add_remove ... ok test watchers::tests::track_spells_cast ... ok test watchers::tests::custom_watcher_game_scope ... ok test zones::tests::exile_zones ... ok test zones::tests::battlefield_controlled_by ... ok test zones::tests::library_peek ... ok test watchers::tests::custom_watcher_player_scope ... ok test zones::tests::library_put_on_top_and_bottom ... ok test zones::tests::stack_lifo ... ok test zones::tests::hand_add_remove ... ok test zones::tests::library_draw ... ok test zones::tests::graveyard_ordering ... ok test game::tests::game_runs_to_completion ... ok test result: ok. 159 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s
All 159 engine tests pass. Now let me fix all the cards. I'll do them in parallel using sed for efficiency.
\
# 2a: Day of Judgment
sed -i 's/vec!\[Effect::Custom("Destroy all creatures.".into())\]/vec![Effect::destroy_all("creatures")]/' mtg-cards/src/sets/fdn.rs && \
# 2b: Diregraf Ghoul
sed -i 's/vec!\[StaticEffect::Custom("Enters tapped.".into())\]/vec![StaticEffect::EntersTapped { filter: "self".into() }]/' mtg-cards/src/sets/fdn.rs && \
# 2c: Frenzied Goblin
sed -i 's/vec!\[Effect::Custom("Target creature can'\''t block this turn.".into())\]/vec![Effect::CantBlock]/' mtg-cards/src/sets/fdn.rs && \
echo "FDN 2a/2b/2c done"FDN 2a/2b/2c done
\
# 2c: Brambleback Brute (ECL)
sed -i 's/vec!\[Effect::Custom("Target creature can'\''t block this turn.".into())\]/vec![Effect::CantBlock]/' mtg-cards/src/sets/ecl.rs && \
echo "ECL 2c done"ECL 2c done
Now the 2d self-counter fixes for ECL:
\
# 2d: Blighted Blackthorn (2 lines)
sed -i 's/vec!\[Effect::Custom("Put two -1\/-1 counters on Blighted Blackthorn.".into()), Effect::draw_cards(1), Effect::lose_life(1)\]/vec![Effect::add_counters("-1\/-1", 2), Effect::draw_cards(1), Effect::lose_life(1)]/g' mtg-cards/src/sets/ecl.rs && \
# 2d: Sourbread Auntie
sed -i 's/vec!\[Effect::Custom("Put two -1\/-1 counters on Sourbread Auntie.".into()), Effect::create_token("1\/1 Goblin", 2)\]/vec![Effect::add_counters("-1\/-1", 2), Effect::create_token("1\/1 Goblin", 2)]/' mtg-cards/src/sets/ecl.rs && \
# 2d: Sting-Slinger
sed -i 's/vec!\[Effect::Custom("Put a -1\/-1 counter on Sting-Slinger.".into()), Effect::damage_opponents(2)\]/vec![Effect::add_counters("-1\/-1", 1), Effect::damage_opponents(2)]/' mtg-cards/src/sets/ecl.rs && \
# 2d: Heirloom Auntie (remove counter)
sed -i 's/vec!\[Effect::scry(1), Effect::Custom("Remove a -1\/-1 counter from Heirloom Auntie.".into())\]/vec![Effect::scry(1), Effect::RemoveCounters { counter_type: "-1\/-1".into(), count: 1 }]/' mtg-cards/src/sets/ecl.rs && \
echo "ECL 2d main cards done"ECL 2d main cards done
Now the additional RemoveCounters cards found:
\
# Encumbered Reejerey
sed -i 's/vec!\[Effect::Custom("Remove a -1\/-1 counter from Encumbered Reejerey.".into())\]/vec![Effect::RemoveCounters { counter_type: "-1\/-1".into(), count: 1 }]/' mtg-cards/src/sets/ecl.rs && \
# Reluctant Dounguard
sed -i 's/vec!\[Effect::Custom("Remove a -1\/-1 counter from Reluctant Dounguard.".into())\]/vec![Effect::RemoveCounters { counter_type: "-1\/-1".into(), count: 1 }]/' mtg-cards/src/sets/ecl.rs && \
# Unknown creature with Ward {2} (line ~2197)
sed -i 's/vec!\[Effect::Custom("Remove a -1\/-1 counter from this creature.".into())\]/vec![Effect::RemoveCounters { counter_type: "-1\/-1".into(), count: 1 }]/' mtg-cards/src/sets/ecl.rs && \
echo "ECL extra remove-counter cards done"ECL extra remove-counter cards done
Now batch 2e (loot — Icewind Elemental in FDN) and 2f (ETB draw — Mistmeadow Council in ECL):
\
# 2e: Icewind Elemental (loot = draw then discard)
sed -i 's/vec!\[Effect::Custom("When this creature enters, draw a card, then discard a card.".into())\]/vec![Effect::draw_cards(1), Effect::discard_cards(1)]/' mtg-cards/src/sets/fdn.rs && \
# 2f: Mistmeadow Council (ETB draw)
sed -i 's/vec!\[Effect::Custom("When this creature enters, draw a card.".into())\]/vec![Effect::draw_cards(1)]/' mtg-cards/src/sets/ecl.rs && \
echo "2e and 2f done"2e and 2f done
Now let me also fix the bonus card I found — Refute (FDN) which has counter spell + loot:
\
# Refute: counter target spell + loot
sed -i 's/vec!\[Effect::Custom("Counter target spell. Draw a card, then discard a card.".into())\]/vec![Effect::counter_spell(), Effect::draw_cards(1), Effect::discard_cards(1)]/' mtg-cards/src/sets/fdn.rs && \
echo "Refute fixed"Refute fixed
Also need to fix Refute's target — it says "Counter target spell" but has TargetSpec::None:
sed -i '/vec!\[Effect::counter_spell(), Effect::draw_cards(1), Effect::discard_cards(1)\]/{n;s/TargetSpec::None/TargetSpec::Spell/}' mtg-cards/src/sets/fdn.rs && echo "Refute target fixed"Refute target fixed
Now let me verify all changes compiled correctly:
cargo check -p mtg-cards 2>&1 | tail -20 Checking mtg-engine v0.1.0 (/mnt/dest-btrfs/home/jc/src/mage/mtg-rl/mtg-engine)
Checking mtg-cards v0.1.0 (/mnt/dest-btrfs/home/jc/src/mage/mtg-rl/mtg-cards)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.35sNow run all tests:
cargo test --lib 2>&1 | tail -20