[UNF] Implement Magar of the Magic Strings with new face down type by matoro · Pull Request #14772 · magefree/mage

matoro · 2026-04-18T19:20:19Z

Linked to #8491

JayDi85 · 2026-04-19T01:34:37Z

[[Magar of the Magic Strings]]

github-actions · 2026-04-19T01:34:46Z

Magar of the Magic Strings - (Gatherer) (Scryfall) (EDHREC)

{1}{B}{R}
Legendary Creature — Minotaur Performer
3/3
{1}{B}{R}: Note the name of target instant or sorcery card in your graveyard and put it onto the battlefield face down. It's a 3/3 creature with "Whenever this creature deals combat damage to a player, you may create a copy of the card with the noted name. You may cast the copy without paying its mana cost" and "If this creature would leave the battlefield, exile it instead of putting it anywhere else."

JayDi85

I don’t like current approach. No usage research (including GUI). No game engine changes in issue title-description. Cards repo hack for a copy. Without any line of comment/docs.

Did you try to use continues effect instead custom face down?

JayDi85 · 2026-04-19T01:51:28Z

+        if (!player.chooseUse(outcome, "Cast copy of " + card.getName() + " without paying its mana cost?", source, game)) {
+            return true;
+        }
+        final CardInfo cardInfo = CardRepository.instance.findCardWithPreferredSetAndNumber(card.getName(), card.getExpansionSetCode(), card.getCardNumber());


What are doing here? Do not touch cards repo from a card at all. If you need card copy then copy it by game.copyCard.

I tried to do this at first, but I couldn't because of this exception that prevents copying cards that are on the battlefield:

// 707.12. An effect that instructs a player to cast a copy of an object (and not just copy a spell) follows the rules for casting spells, except that the copy is created in the same zone the object is in and then cast while another spell or ability is resolving. Zone copyToZone = game.getState().getZone(mainCardToCopy.getId()); if (copyToZone == Zone.BATTLEFIELD) { throw new UnsupportedOperationException("Cards cannot be copied while on the Battlefield"); }

and LKI does not look back far enough to see what it would have looked like before it left the graveyard, which could have been many turns ago.

Also, the text just says to note the name of the card, which is therefore the only piece of information in state. All of the other card data is derived from the text on the card, so this is a reasonably accurate way to represent it.

See [[Garth One-Eye]], which is where I got this from.

well, I see -- you need only card name here, so it's ok to create it same way. But you must use card.getMainCard() to make sure it's a real card, not permanent card. Also make sure logs-gui really show a card name in choose dialog, not empty or face down name.

I did confirm that. I did a preliminary GUI test and everything seemed to work as intended.

JayDi85 · 2026-04-19T02:02:06Z

+        CLOAKED,
+        CUSTOM_33 {
+            @Override
+            public int getPower() {


All that workarounds smells bad. Why you can’t to use the standard face down with PT and ability change by ContinuousEffectImpl?

I tried this, but it did not work with clone effects. When you clone one of Magar's face-down cards it should be a 3/3 (with no combat damage trigger), but the current code in CopyPermanent uses makeFaceDownObject() unconditionally when copying a face-down card. So BecomesFaceDownCreatureEffect needs to be aware of this possible flavor.

JayDi85 · 2026-04-19T02:06:28Z

                break;
            case MANUAL:
            case MANIFESTED:
+            case CUSTOM_33:


MANUAL_33 not custom_33

JayDi85 · 2026-04-19T02:09:12Z

        } else if (permanent.isFaceDown(game)) {
+            if (permanent.getPower().getModifiedBaseValue() == 3 && permanent.getPower().getModifiedBaseValue() == 3) {
+                return BecomesFaceDownCreatureEffect.FaceDownType.CUSTOM_33;
+            }


Miss other places with manual usage. If you introduce new type then must update all usages too. Example with miss 33 code:

xenohedron · 2026-04-19T02:31:08Z

I'm not liking the approach, and I also recommend this NOT be merged until #13486 gets through

matoro · 2026-04-19T02:43:43Z

I'm not liking the approach, and I also recommend this NOT be merged until #13486 gets through

...are there any actual plans to merge that after 18 months? What is the blocker?

xenohedron · 2026-04-19T02:45:53Z

The main blocker is it takes a lot of work to review (and the originator isn't particularly active anymore either). But it's a lot more important than an individual card implementation

JayDi85 · 2026-04-19T02:54:22Z

+            return false;
+        }
+        final Card cardCopy = game.copyCard(newCard, source, source.getControllerId());
+        cardCopy.setZone(Zone.GRAVEYARD, game);


Must use Zone.OUTSIDE here

This is how Garth does it, but unfortunately the ruling for Magar is that the copies are created in the graveyard, so they trigger cast-from-graveyard e.g. [[Ignite the Future]] or [[Drannith Magistrate]]

Well, you must add all related rules to the custom code too.

707.13. One card (Garth One-Eye) instructs a player to create a copy of a card defined by name rather than by indicating an object to be copied. To do so, the player uses the Oracle card reference to determine the characteristics of the copy and creates the copy outside of the game.

It's card name from repo, not real copy of the card with card link. Must use game.loadCards and setZone(Outsize).

707.14. One card (Magar of the Magic Strings) instructs a player to note the name of a particular card in a graveyard and create a copy of the card with the noted name. To do so, use the characteristics of that card as it last existed in the graveyard to determine the copiable values of the copy. (See rule 608.2h.)

It's copy of the card, not card's name repo. You must save graveyard blueprint on first usage/resolve and use it in all other copy triggers. Not repo's card. Like blueprint = target->card->main.copyCard (or like that, so it will keep some grave's gained ability). It's interesting use case.

Gained abilities are not copyable values, so anything that gives cards in the graveyard new abilities would not apply to these copies.

I reviewed this carefully and asked about it in the discord - as far as I can tell there are no effects which modify the copiable values of instant/sorcery cards in the graveyard. So this is functionally equivalent from a game state perspective.

It would be a significantly more invasive change to try and actually implement this exactly reflecting how the rule works, and require a bunch more engine changes, since there is no other card that works like this. That's why I though implementing it in a much simpler way, that behaves the same according to the rules, was the better choice.

JayDi85 · 2026-04-19T02:58:36Z

-        object.getPower().setModifiedBaseValue(2);
-        object.getToughness().setModifiedBaseValue(2);
+        object.getPower().setModifiedBaseValue(faceDownType.getPower());
+        object.getToughness().setModifiedBaseValue(faceDownType.getToughness());


miss code changes/sync in BecomesFaceDownCreatureAllEffect

Linked to magefree#8491

matoro · 2026-04-20T23:17:19Z

I've implemented all requested changes, and also added an extra test to ensure that cast-from-graveyard triggers work using [[The Final Days]].

github-actions · 2026-04-20T23:17:30Z

The Final Days - (Gatherer) (Scryfall) (EDHREC)

{2}{B}{B}
Sorcery
Create two tapped 2/2 black Horror creature tokens. If this spell was cast from a graveyard, instead create X of those tokens, where X is the number of creature cards in your graveyard.
Flashback {4}{B}{B} (You may cast this card from your graveyard for its flashback cost. Then exile it.)

github-actions Bot added cards engine tests labels Apr 18, 2026

matoro force-pushed the magar branch from 5f0138c to 87e4bc7 Compare April 18, 2026 19:28

matoro changed the title ~~[UNF} Implement Magar of the Magic Strings~~ [UNF] Implement Magar of the Magic Strings Apr 18, 2026

Grath approved these changes Apr 18, 2026

View reviewed changes

JayDi85 changed the title ~~[UNF] Implement Magar of the Magic Strings~~ [UNF] Implement Magar of the Magic Strings with new face down type Apr 19, 2026

JayDi85 requested changes Apr 19, 2026

View reviewed changes

JayDi85 reviewed Apr 19, 2026

View reviewed changes

matoro added 2 commits April 20, 2026 18:42

[UNF] Implement Magar of the Magic Strings

6d746c4

Linked to magefree#8491

Magar of the Magic Strings review comments

b67601d

matoro force-pushed the magar branch from 87e4bc7 to b67601d Compare April 20, 2026 23:16

Conversation

matoro commented Apr 18, 2026

Uh oh!

JayDi85 commented Apr 19, 2026

Uh oh!

github-actions Bot commented Apr 19, 2026

Uh oh!

JayDi85 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xenohedron commented Apr 19, 2026

Uh oh!

matoro commented Apr 19, 2026

Uh oh!

xenohedron commented Apr 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JayDi85 Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matoro commented Apr 20, 2026

Uh oh!

github-actions Bot commented Apr 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JayDi85 Apr 19, 2026 •

edited

Loading