[don't merge] Implement mutate mechanic

[ninthworld]

https://user-images.githubusercontent.com/1323803/177071507-49eb9a32-512e-4cc6-a3f0-f5af93ac5ed4.mp4

Mutate Ability

Mutate is an ability keyword which was first add to MTG in 2020 in the set Ikoria: Lair of Behemoths. Due to the complexity of implementing this mechanic, it has been missing in XMage even since. This PR is the most complete implementation of the mutate mechanic so far, with all common interactions (and most edge-case interactions) tested and working.

Drawbacks

As stated above, this is a complex mechanic to implement properly, and truthfully, XMage was not originally designed with this type of thing in mind. The majority of cards will work just fine with mutate, and there are several abilities and effects that I have gone through and manually updated to work with mutate. However, it is very likely that there are still cards/abilities/effects out there that implement some custom zone change code that will not inherently work with mutate. And going forward, it is possible that new cards/abilities/effects may not take mutate into account when implementing custom zone change code. Unfortunately, there is no easy fix for this, and a proper refactoring of the code-base to fix this may be too much work for such a limited-use ability.

Test Coverage:

Expand

• Cast creature without using mutate ability
• Targeted creature is killed before mutate spell resolves
• Mutate over existing creature to retain the creature's P/T, counters, tapped state, and gain all abilities
• Mutated creature is killed
• Mutated creature is bounced
• Mutated creature is sent to its owners library
• Mutated creature with a God-Eternal over or under dies
• Mutated creature is exiled with Oblivion Ring, then Oblivion Ring is destroyed
• Cast mutate with Volo, Guide to Monsters on the battlefield
• Mutate over creature with transform
• Mutate over creature with imprint
• Mutated creature is phased
• Mutate creature under a morphed creature
• Mutate with number of times mutated
• Mutate with Soulbond, Monstrous, Renowned, Attachments, Regenerate, Persist, Undying, Flip-cards
• Mutate with Mimic Vat, Etrata, Nightmare Shepherd abilities
• Mutate with commander, commander damage, Leadership Vacuum

Related issues fixed: #6390, #7623

[ninthworld]

I'm really not sure how to implement Brokkos without allowing it to be cast like normal from the graveyard (since MutateAbility is implemented as an alternate casting cost, but you cant alter the cost of an ability that cant be cast)

[Alex-Vasile]

I'm really not sure how to implement Brokkos without allowing it to be cast like normal from the graveyard (since MutateAbility is implemented as an alternate casting cost, but you cant alter the cost of an ability that cant be cast)

Could you extend MutateAbility inside Brookos, overriding isAvailable to also include Zone.GRAVEYARD?

Additionally, Flashback also is an static ability which provided an alternate cost for casting specifically from the graveyard, maybe something from there may help.

[theelk801]

you probably don't need to extend the mutate ability, just add an arg to the mutate ability itself

[Alex-Vasile]

Are you supposed to be able to cast for the mutate cost using effects like "exile, and you may play until X"? E.g. [Prosper, Tome-Bound]

[theelk801]

yeah, the only situations where you can't use it is when it specifies "without paying its mana cost" or "rather than paying its mana cost"

[Alex-Vasile]

yeah, the only situations where you can't use it is when it specifies "without paying its mana cost" or "rather than paying its mana cost"

isAvailable is going to have to be changed then. It can't be checking zones.

[awjackson]

I think MutateAbility needs to be implemented as a subclass of SpellAbility (compare to bestow) rather than as a subclass of AlternativeSourceCosts. The main reason is that casting a creature spell for its mutate cost changes the spell into a targetted spell, which has pretty widespread effects both from a Magic rules and xmage UI standpoint. It's better to leverage the core xmage functionality for targetting (e.g. checking whether a valid target exists before xmage highlights the card as usable and allows you to attempt to cast it) than try to reimplement it from scratch like you are doing here.

[ninthworld]

I think MutateAbility needs to be implemented as a subclass of SpellAbility (compare to bestow) rather than as a subclass of AlternativeSourceCosts. The main reason is that casting a creature spell for its mutate cost changes the spell into a targetted spell, which has pretty widespread effects both from a Magic rules and xmage UI standpoint. It's better to leverage the core xmage functionality for targetting (e.g. checking whether a valid target exists before xmage highlights the card as usable and allows you to attempt to cast it) than try to reimplement it from scratch like you are doing here.

Great suggestion! This simplified the logic of the MutateAbility significantly and allowed me to get Brokkos casting from the graveyard working (albeit with a very improper implementation that will still need to be fixed)

[teskogi]

Wow! Nice to see someone else taking a crack at mutate. I tried my hand at it a while ago. I got it working against a couple of test cases I made (sans blinking the mutated permanent), but I don't think the UI quite accepted what I was trying to do. Anyway, I'm attaching the patch files for it, hopefully they'll be of some use. mutatePatches.zip

[teskogi]

Hi ninthworld, I've had some issues getting your test cases to work on my local copy. Some of it seems to be some staleness of your branch, which the below patch should take care of. FixMutate.zip

But some of the tests fail sometimes and not others. Perhaps there are some race conditions at play somewhere.

[ninthworld]

Hi ninthworld, I've had some issues getting your test cases to work on my local copy. Some of it seems to be some staleness of your branch, which the below patch should take care of. FixMutate.zip

But some of the tests fail sometimes and not others. Perhaps there are some race conditions at play somewhere.

I believe the reason tests are failing sometimes (rarely) is the computer randomly using all of a single color of land before the card that needs that color has been cast.

[sprangg]

Is there any reason to not merge this into beta so that we can start playing around with it and seeing how it goes? And reporting any issues encountered, of course.

Great work btw, one of xmage's most requested features if not the most.

[ninthworld]

Is there any reason to not merge this into beta so that we can start playing around with it and seeing how it goes? And reporting any issues encountered, of course.

Great work btw, one of xmage's most requested features if not the most.

While I agree that mutate is an important feature that should be in XMage, as I've worked on this PR the one thing that keeps nagging at the back of my mind is: How can we ensure future support of this mechanic?

As its written now, most new cards will be automatically supported, even cards that have normal zone change effects. However, any new cards or mechanics that need a custom zone change effect may not take into account that the card being moved may be mutated and require special consideration. So far I've had to specifically modify the code for Transform, Persist, and Undying abilities and Regenerate and GodEternalDies effects. This doesn't even include all of the abilities, effects, and individual cards with custom code that already exist that I haven't tested yet.

I'm afraid that to properly implement this feature sustainably may require a rewrite of many of the underlying core functionality of the engine, leading to a refractor of nearly every card already implemented. I certainly don't have the time or patience for that, and I don't believe the lead developers will feel that a mechanic with only around 30 cards will warrant such a costly endeavor.

Its also possible that I'm being too critical of this implementation because I've seen too much under-the-hood and with a bit more work it could be merged. If you or anyone else want's to test out this branch or work on it from this PR, just use git fetch origin pull/8733/head:mutate and then git checkout mutate

[theelk801]

I haven't had a chance to look over this code yet but I'm definitely down to try and make it work. I've already spent some time refactoring exile effects and there are other things that need changing ("exile target permanent until this leaves the battlefield", meld, etc) so I think it's worth the effort if we organize it well. I'll probably be able to put even more focus into it after CLB is done.

[teskogi]

I got this to pass checks again. PersistAbility and UndyingAbility needed to be patched. I also changed MutateCommanderTest to fix the mana issues. I added Ignore to a few of the MutateTest tests, but they do need to be completed. I also fixed some issues with GameState due to the implementation of Aeon Engine.

8733fixed.zip

[ninthworld]

I got this to pass checks again. PersistAbility and UndyingAbility needed to be patched. I also changed MutateCommanderTest to fix the mana issues. I added Ignore to a few of the MutateTest tests, but they do need to be completed. I also fixed some issues with GameState due to the implementation of Aeon Engine.

8733fixed.zip

Thanks! I haven't worked on this PR in a bit since I've had some other stuff going on. Hopefully once I get some time I'll be able to come back to it with some fresh eyes.

[teskogi]

Some of the cards in the IkoriaLairOfBehemoths set also need NON_FULL_USE_VARIOUS to pass verification. I've updated the patch. 8733fixed.zip

[ninthworld]

After some time away, I decided to take another swing at implementing mutate. Then I ran into all the same issues that I had implementing this version and remembered why I had to make all the compromises I did...

So instead, I cleaned up this one, got the rest of the tests working, squashed a bunch of lingering bugs, and finally got it to pass all build checks. I'm sure there are abilities/effects that still don't work with mutate, but I don't know what they are and I think its going to take some community testing to find them.

[sprangg]

Thank you for your hard work! Mutate for beta testing let's goooo?

[Alex-Vasile]

Thank you for your hard work! Mutate for beta testing let's goooo?

As much as it pains me to say it. This one really should wait until after the next release (🤞) to be merged. Doing it now would only further push the release back.

[jeffwadsworth]

Thank you for your hard work! Mutate for beta testing let's goooo?

As much as it pains me to say it. This one really should wait until after the next release (🤞) to be merged. Doing it now would only further push the release back.

There are no new releases. The Beta is now the “release”. As the coder noted, the community can help find bugs.

[jeffwadsworth]

Great work on this ninthworld. Hopefully it works out.

[teskogi]

Any ideas why the tests fail now? I removed the calls to assertAllCommandsUsed(), but that didn't seem to solve the issue.

[awjackson]

The six conflicting files probably have something to do with the build failing...

I finally took a close look at this and I really don't like the approach of replacing a permanent that's already on the battlefield with a new permanent with a different UUID when you "mutate over". You have added workarounds in Target and TargetPointer, but between the core rules engine, the GUI, common abilities/effects, and card-specific code there are probably thousands and thousands of places where game objects are expected to have unchanging UUIDs and things will break if they don't. Attachments (Auras, equipment) are just the first one I came up with off the top of my head.

I think we need a new approach where the first creature on the battlefield is always the base permanent, and when you "mutate over", rather than replacing the base permanent with a new permanent, you just make the existing permanent get its characteristics from the new card, similar to how a TDFC transforming or a permanent becoming a copy of something works.

[ninthworld]

The six conflicting files probably have something to do with the build failing...

I finally took a close look at this and I really don't like the approach of replacing a permanent that's already on the battlefield with a new permanent with a different UUID when you "mutate over". You have added workarounds in Target and TargetPointer, but between the core rules engine, the GUI, common abilities/effects, and card-specific code there are probably thousands and thousands of places where game objects are expected to have unchanging UUIDs and things will break if they don't. Attachments (Auras, equipment) are just the first one I came up with off the top of my head.

I think we need a new approach where the first creature on the battlefield is always the base permanent, and when you "mutate over", rather than replacing the base permanent with a new permanent, you just make the existing permanent get its characteristics from the new card, similar to how a TDFC transforming or a permanent becoming a copy of something works.

I completely agree with you, however, due to the way permanents are implemented in the core engine, it is not possible as far I have tried. For example, if you have a token permanent on the battlefield, and you mutate over it with a regular permanent, the object on the battlefield would still be a TokenPermanent with your suggestion. There is no way to make it a CardPermanent without just making a new object.

And the reason the tests are failing is because some logic dealing with the ZoneChangeCounters is incorrect and causing sync issues between Card MageObjectReference and the zoneChangeCounters in GameState and through multiple tries I can't seem to understand how to get it to work correctly.

[ExpensiveKoala]

What is still pending on this PR? What is holding it back from merging (other than merge conflicts)

[thefnitalian]

What is the status on this?

[ExpensiveKoala]

This PR's implementation has some issues with copies of on-top mutated creatures. To replicate: Have a [[Scute Swarm]] and 5+ lands under your control. Mutate a [[Migratory Greathorn]] on TOP of the Scute Swarm. The Migratory Greathorn's mutate triggers and you get a land, which triggers Scute Swarm to create a copy of itself since you'd have 6+ lands. However, any subsequent lands you play will only trigger the landfall of the copies and not the original anymore. This doesn't seem to happen when you mutate the Migratory Greathorn on bottom of the Scute Swarm.

There's probably a dozen other issues with how mutate interacts with other effects.

[github-actions[bot]]

Scute Swarm - (Gatherer) (Scryfall) (EDHREC)

{2}{G} Creature — Insect 1/1 Landfall — Whenever a land enters the battlefield under your control, create a 1/1 green Insect creature token. If you control six or more lands, create a token that's a copy of Scute Swarm instead.

Migratory Greathorn - (Gatherer) (Scryfall) (EDHREC)

{3}{G} Creature — Beast 3/4 Mutate {2}{G} (If you cast this spell for its mutate cost, put it over or under target non-Human creature you own. They mutate into the creature on top plus all abilities from under it.) Whenever this creature mutates, search your library for a basic land card, put it onto the battlefield tapped, then shuffle.