mage icon indicating copy to clipboard operation
mage copied to clipboard

Rework RemoveCounterCost

Open Susucre opened this issue 1 year ago • 2 comments

RemoveCounterCost is a cost for removing a number of (specific type) counters from permanents or cards.

Currently, it does take a non-specific Target. Notably there is no constraint/check that the target has at least one counter. Which results in an unintuitive UI that rollbacks the payment if selecting one with none of the counter.

It would make a lot more sense to provide Filter(s) instead of Target, and add the extra minimum constraint to the filter to target permanents/cards with at least 1 matching counter there. It could also make sense to split the class for FilterCard (rare, e.g. [[Mari, the Killing Quill]]) and FilterPermanent (more common, e.g. [[Falco Spara]]).

Another solution (worst in my opinion), would be to always make sure that the target is filtering permanents/cards with at least 1 of the matching counter, but that seems like a lot of extra code in userrs that should be shared by the common effect.

Susucre avatar May 02 '24 15:05 Susucre

Mari, the Killing Quill - (Gatherer) (Scryfall) (EDHREC)

{1}{B}{B} Legendary Creature — Vampire Assassin 3/2 Whenever a creature an opponent controls dies, exile it with a hit counter on it. Assassins, Mercenaries, and Rogues you control have deathtouch and "Whenever this creature deals combat damage to a player, you may remove a hit counter from a card that player owns in exile. If you do, draw a card and create two Treasure tokens."

Falco Spara, Pactweaver - (Gatherer) (Scryfall) (EDHREC)

{1}{G}{W}{U} Legendary Creature — Bird Demon 3/3 Flying, trample Falco Spara, Pactweaver enters the battlefield with a shield counter on it. You may look at the top card of your library any time. You may cast spells from the top of your library by removing a counter from a creature you control in addition to paying their other costs.

github-actions[bot] avatar May 02 '24 15:05 github-actions[bot]

Yes, it must be improved. Each related cost usage must be carefully reviewed -- if you put filter to current target (CounterType already has predicates) then it will fail due unsupported canPay method (see zero counters use case on X = 0):

shot_240502_202553 shot_240502_202603

P.S. Not related to issue -- looks like MariTheKillingQuill and OblivionSower uses wrongly face-up predicate (I don't see any requirements for it):

shot_240502_203219

shot_240502_203030

JayDi85 avatar May 02 '24 16:05 JayDi85