mage
mage copied to clipboard
Refactor: Several refactors that should be done found during implementing cards
Leftover TODOs from #9328:
- [ ] Several very similar
MoveCounterFromTargetToTargetEffect
should be refactored to a common class - [ ] Change cards to make use of new
SpellCastControllerTriggeredAbility
'sfromZone
(e.g. Kami of Celebration, Sunbird's Invocation) - [ ] Make use of new non-token creature static filter
- [ ] Change Ob Nixilis, the Adversary to use standard Casualty ability rather than custom implementation
- [ ] Refactor
TargetOfOpponentsSpellOrAbilityTriggeredAbility
andBecomesTargetControlledPermanentTriggeredAbility
into a single class for "Whenever {a} becomes the target of a {b}" that takes filters for both {a} and {b}. - [x] ~~Refactor
MageObject.getColor(game)
so that it handles the intricacies of MDFC and split cards internally rather than needing to be wrapped externally.~~ Clean up and double check by https://github.com/magefree/mage/pull/9328#issuecomment-1255699247 - [ ] Refactor Replicate cost show the final cost of the spell at once when paying. Currently the replicate cost is only shown after the regular cost has been paid.
[[Kami of Celebration]] [[Sunbird's Invocation]] [[Ob Nixilis, the Adversary]]
Kami of Celebration - (Gatherer) (Scryfall) (EDHREC)
{4}{R} Creature — Spirit 3/3 Whenever a modified creature you control attacks, exile the top card of your library. You may play that card this turn. (Equipment, Auras you control, and counters are modifications.) Whenever you cast a spell from exile, put a +1/+1 counter on target creature you control.
Sunbird's Invocation - (Gatherer) (Scryfall) (EDHREC)
{5}{R} Enchantment Whenever you cast a spell from your hand, reveal the top X cards of your library, where X is that spell's mana value. You may cast a spell with mana value X or less from among cards revealed this way without paying its mana cost. Put the rest on the bottom of your library in a random order.
Ob Nixilis, the Adversary - (Gatherer) (Scryfall) (EDHREC)
{1}{B}{R} Legendary Planeswalker — Nixilis 3 Casualty X. The copy isn't legendary and has starting loyalty X. (As you cast this spell, you may sacrifice a creature with power X. When you do, copy this spell. The copy becomes a token.) +1: Each opponent loses 2 life unless they discard a card. If you control a Demon or Devil, you gain 2 life. −2: Create a 1/1 red Devil creature token with "When this creature dies, it deals 1 damage to any target." −7: Target player draws seven cards and loses 7 life.
Regarding the apparently unnecessary split/MDFC code in MulticoloredPredicate
, I came up with a hypothesis as to why it exists. The rules for split cards were greatly simplified in 2017, with the release of Amonkhet. Before Amonkhet, split cards that weren't on the stack actually had three sets of characteristics at once: those of the left side, of the right side, and the union of the two sides. Which characteristics you used depended on what kind of comparison you were doing. For example, if you revealed [[Wear // Tear]] to [[Counterbalance]] it would counter a 1-mana spell or a 2-mana spell but not a 3-mana spell, whereas if you revealed it to Dark Confidant you would lose 3 life. In the current rules, split cards not on the stack always and only have the characteristics of the union of both sides.
The code in question might be a leftover from when xmage implemented (or attempted to implement) the complex pre-Amonkhet rules. Under the modern rules, there's no reason a SplitCardHalf
that isn't on the stack should ever be getting passed to a filter, but under the pre-Amonkhet rules there was because the characteristics of the separate halves mattered.
EDIT: I just checked the Amonkhet rules update diff and the rule quoted in the comment in MulticoloredPredicate.java
is a rule that was removed in the update. It was condensed along with a whole bunch of other rules into "In every zone except the stack, the characteristics of a split card are those of its two halves combined. This is a change from previous rules." So I think my hypothesis for why that code exists is almost confirmed.
Wear // Tear - (Gatherer) (Scryfall) (EDHREC)
{1}{R} Instant Destroy target artifact. Fuse (You may cast one or both halves of this card from your hand.) :arrows_counterclockwise: {W} Instant Destroy target enchantment. Fuse (You may cast one or both halves of this card from your hand.)
Counterbalance - (Gatherer) (Scryfall) (EDHREC)
{U}{U} Enchantment Whenever an opponent casts a spell, you may reveal the top card of your library. If you do, counter that spell if it has the same mana value as the revealed card.