magefree / mage

Magic Another Game Engine
http://xmage.today
MIT License
1.87k stars 767 forks source link

Refactor: Several refactors that should be done found during implementing cards #9553

Open Alex-Vasile opened 2 years ago

Alex-Vasile commented 2 years ago

Leftover TODOs from #9328:

jeffwadsworth commented 2 years ago

[[Kami of Celebration]] [[Sunbird's Invocation]] [[Ob Nixilis, the Adversary]]

github-actions[bot] commented 2 years ago

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.

awjackson commented 2 years ago

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.

github-actions[bot] commented 2 years ago

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.