Order of resolve and graveyard

[Rob Cashwalker]

I'm having a problem dealing with Feast of Flesh. (Testing my new damage spell code)

It needs to count the number of cards named Feast of Flesh in all graveyards. However when the first one in the game resolves, the X is calculated as 2 (1 + 1) because of the following code:

Code: Select all

    //special consideration for "Beacon of Unrest" and other "Beacon" cards
    if((c.isInstant() || c.isSorcery())     &&
       (! c.getName().startsWith("Beacon")) &&
       (! c.getName().startsWith("Pulse")) &&
       !AllZone.GameAction.isCardRemovedFromGame(c)) //hack to make flashback work
    {
      AllZone.GameAction.moveToGraveyard(c);
    }
   
    if (sa.getSourceCard().getKeyword().contains("Cantrip"))
         AllZone.GameAction.drawCard(sa.getSourceCard().getController());
    //resolve() needs to be call AFTER stuff is moved to the graveyard
    //because cards like Elvish Fury are returned to hand after resolving
    sa.resolve();
    AllZone.GameAction.checkStateEffects();


    //update all zones, something things arent' updated for some reason
    AllZone.Human_Hand.updateObservers();
    AllZone.Human_Play.updateObservers();
    AllZone.Computer_Play.updateObservers();


The comment in there specifically mentions that the card needs to be moved to the graveyard before the resolve is executed. However, according to a rules question I posted at MTG Salvation (no one asked it before) the spell resolves THEN the card goes to the yard.

If there was explicit card code for Feast of Flesh, then it would take the number of cards and explicitly subtract 1 from it, then add the "plus 1".
But to do this generically, when the counting function shouldn't have to know about Feast of Flesh or the other similarly worded cards, that's a problem.

Any thoughts?

[DennisBergkamp]

Huh, really ? What about cards like Life Burst?

EDIT: Oh I see, looking at the code of Life Burst, they just do a lifegain of count * 4 (where count = number in graveyards).

[DennisBergkamp]

This brings up other interesting questions, I've added Tarmogoyf for instance, and apparently when it's a 0/1 it should still survive a Tremor (as a 1/2 with 1 damage). So how does that work, if the Tremor resolves and THEN gets put into the graveyard?

[Rob Cashwalker]

re: Life Burst - Is kinda correct. The explicit code for Kjeldoran War Cry is kinda correct, because it just returns the CardList.size. the grouped code, which includes the code for Overrun, and Tromp the Domains is wrong, because it returns the CardList.size +1. (Yes it's my code, and now it's clear to me why it's wrong)

Tarmogoyf, when I was its victim, took on the new characteristic even during combat. So the opponent would play an instant and then the goyf was suddenly bigger.

[DennisBergkamp]

Yes, but combat is an entire phase. I guess the reason it survives is because state based checks take place after Tremor goes to the graveyard.

Anyway, I'm not sure what the best solution is in this case. It seems to only have been added for cards with buyback, we could add some hack where we reverse switch the moveToGrave and Resolve if (c.hasBuyBack). It would be pretty hacky, but currently as it turns out, every case is completely wrong...

[Rob Cashwalker]

I think the answer is to change the method of buyback.... Instead of moving it from the graveyard back to its owners hand, it should be moved from whatever zone its in while being played, (still in the hand?) to the hand.

[DennisBergkamp]

I don't think that's possible, because after it resolves, it will always be put into the graveyard. We'd end up moving it back to hand, then after it resolves it still hits the graveyard.
Another option would be switching the order, but adding another clause to the the if:

Code: Select all

...
//resolve happens first
sa.resolve();
...
if((c.isInstant() || c.isSorcery())     &&
       (! c.getName().startsWith("Beacon")) &&
       (! c.getName().startsWith("Pulse")) &&
       !AllZone.GameAction.isCardRemovedFromGame(c) &&
>     !sa.isBuyBackAbility())
             AllZone.GameAction.moveToGraveyard(c);


All we'd have to do is add a boolean isBuyBackAbility in SpellAbility.java.

[Rob Cashwalker]

That will work for buyback. But there may end up being other reasons a card may go back to your hand after it resolves....

How about a default command object member in the Card, that when executed does the actual moving of the card to the graveyard. If the card rules move the card someplace instead of the graveyard, then the CardFactory (or other code repository system) overrides the command object. (or just creates a new command and uses Card.setCmdMoveToYard() or something)
This is as close to a replacement effect (which I think buyback is) as we could get right now.

Edit - fixed some late-night grammar and clarified a couple items.

[Marek14]

This brings up other interesting questions, I've added Tarmogoyf for instance, and apparently when it's a 0/1 it should still survive a Tremor (as a 1/2 with 1 damage). So how does that work, if the Tremor resolves and THEN gets put into the graveyard?

Well, the trick is that state-based effects only check between resolution of spells/abilities, not during. So Tremor first deals 1 damage to Tarmogoyf, which is 0/1 with 1 damage now, then it's put into graveyard which causes Tarmogoyf's power/toughness to update. When the resolution is complete, it's 1/2 with 1 damage and survives. It won't die because he was momentally mortally wounded, the same way Maro won't die if you cast Wheel of Fortune, despite having toughness 0 for a while.

[DennisBergkamp]

That will work for buyback. But there may end up being other reasons a card may go back to your hand after it resolves....

How about a default command object member in the Card, that when executed does the actual moving of the card to the graveyard. If the card rules move the card someplace instead of the graveyard, then the CardFactory (or other code repository system) overrides the command object. (or just creates a new command and uses Card.setCmdMoveToYard() or something)
This is as close to a replacement effect (which I think buyback is) as we could get right now.

Edit - fixed some late-night grammar and clarified a couple items.

Ok, that makes sense I guess. I'll play around with it and see if I can make that work.

However, after making the change of reversing "hitting graveyard" and resolving, all of the Burst cards will have to be updated (Life Burst, Muscle Burst, ... ). Question is, are there any more cards with similar checks ?

[Rob Cashwalker]

search for ".size()" and it should reveal them.

Ultimately, I think the command object model for card effects when some particular action occurs is the best way to handle just about everything in the game.
Each card has a ComesIntoPlay Command, and it works great.
Each card has a destroy Command, and it also works out nice.
Each card should have a stateEffects command, which, for each card in play, the command is executed.
And so forth.
Better than this, the card should really have an array of command objects for each action... because other cards may need to create additional effects when a card which already has a CIP effect, for example. The tricky part is cases where another card creates a replacement effect....

[DennisBergkamp]

Well .size() will return a LOT of things, but in addition to the Burst cards and Kjeldoran War Cry I don't think there are that many...

In agree with the fact they should have arrays of CIP/destroy/leavePlay/... commands, I've seen it happen sometimes that certain cards get their original ("intrinsic") commands overridden (Adarkar Valkyrie for instance does this a lot - causes some really interesting bugs).