Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swift End + Bonecrusher Giants with a Clover gives too much damage #6158

Closed
JayDi85 opened this issue Jan 9, 2020 · 6 comments · Fixed by #6251
Closed

Swift End + Bonecrusher Giants with a Clover gives too much damage #6158

JayDi85 opened this issue Jan 9, 2020 · 6 comments · Fixed by #6251
Assignees
Labels
bug Bugs and errors

Comments

@JayDi85
Copy link
Member

JayDi85 commented Jan 9, 2020

From reddit:

Just cast Swift End on 2 Bonecrusher Giants with a Clover out. Would expect to take 8 damage for that ((2+2)x2) but as you can see from the screenshot I took 16.

svoun6R

@Paerux
Copy link

Paerux commented Jan 9, 2020

in case anyone who thought copying spells targeted the original target first and then changed (like me)
here's the official ruling

706.10c Some effects copy a spell or ability and state that its controller may choose new targets for the copy. The player may leave any number of the targets unchanged, even if those targets would be illegal. If the player chooses to change some or all of the targets, the new targets must be legal. Once the player has decided what the copy’s targets will be, the copy is put onto the stack with those targets.

@jeffwadsworth jeffwadsworth added the bug Bugs and errors label Jan 15, 2020
@jeffwadsworth
Copy link
Contributor

This is related to the copied spell retaining the targeted object long enough for the targeted trigger to fire.

@Dilnu
Copy link
Contributor

Dilnu commented Feb 2, 2020

This is because chooseNewTarget fires off many more events than are required. You can actually trigger arbitrarily many targeted events if you play in certain ways.

I spent a few hours trying to find a solution but I had no luck.

The main problem is that we don't have a way of imposing additional requirements when retargeting a spell as a result the implementation of chooseNewTarget in StackObjectImpl has loops and those loops allow for the arbitrary number of events I mentioned.

The other work around would be to add a skip event parameter chooseTargets method in every Target and player subclass but that's also really painful.

The final possibility which occurs to me is to somehow push a blank gameState onto the game then discard it once chooseNewTargets has finalized the targets.

@Dilnu
Copy link
Contributor

Dilnu commented Feb 2, 2020

Actually that last solution works great now that I know bookmarks are a thing.

@Dilnu Dilnu assigned Dilnu and unassigned Dilnu Feb 2, 2020
@Dilnu
Copy link
Contributor

Dilnu commented Feb 2, 2020

Welp... that doesn't really work either it just looked like it did in some circumstances.

To be more specific, if you bookmark and restore during an instance method of something stored in the game state then you're operating on an obsolete version of that object.

@Dilnu Dilnu self-assigned this Feb 2, 2020
Dilnu added a commit to Dilnu/mage that referenced this issue Feb 3, 2020
Dilnu added a commit to Dilnu/mage that referenced this issue Feb 3, 2020
This specifically addresses changing the target of a spell or ability on the stack.
Fixes magefree#6158
@JayDi85
Copy link
Member Author

JayDi85 commented Feb 3, 2020

Try that tests:

    @Test
    public void BonecrusherGiantChangeTargets_BoneTargetBoth() {
        // Whenever Bonecrusher Giant becomes the target of a spell, Bonecrusher Giant deals 2 damage to that spell’s controller.
        addCard(Zone.BATTLEFIELD, playerA, "Bonecrusher Giant");
        //
        // Target creature gets +2/+2 until end of turn.
        // Conspire (As you cast this spell, you may tap two untapped creatures you control that share a color with it. When you do, copy it and you may choose a new target for the copy.)
        addCard(Zone.HAND, playerA, "Barkshell Blessing");
        addCard(Zone.BATTLEFIELD, playerA, "Plains");
        addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears");
        addCard(Zone.BATTLEFIELD, playerA, "Savannah Lions");

        castSpell(1, PhaseStep.UPKEEP, playerA, "Barkshell Blessing");
        setChoice(playerA, "Yes"); // use Conspire
        addTarget(playerA, "Bonecrusher Giant"); // target bone
        setChoice(playerA, "Grizzly Bears"); // pay for conspire
        setChoice(playerA, "Savannah Lions"); // pay for conspire
        setChoice(playerA, "No"); // both spells target bone
        setChoice(playerA, "When {this} becomes the target of a spell"); // two triggers

        setStrictChooseMode(true);
        setStopAt(1, PhaseStep.PRECOMBAT_MAIN);
        execute();
        assertAllCommandsUsed();

        assertPowerToughness(playerA, "Bonecrusher Giant", 4 + 2 * 2, 3 + 2 * 2);
        assertPowerToughness(playerA, "Grizzly Bears", 2, 2);
        assertPowerToughness(playerA, "Savannah Lions", 2, 1);
        assertLife(playerA, 20 - 2 * 2); // bone trigger from both spells
    }

    @Test
    public void BonecrusherGiantChangeTargets_BoneTargetFirst() {
        // Whenever Bonecrusher Giant becomes the target of a spell, Bonecrusher Giant deals 2 damage to that spell’s controller.
        addCard(Zone.BATTLEFIELD, playerA, "Bonecrusher Giant");
        //
        // Target creature gets +2/+2 until end of turn.
        // Conspire (As you cast this spell, you may tap two untapped creatures you control that share a color with it. When you do, copy it and you may choose a new target for the copy.)
        addCard(Zone.HAND, playerA, "Barkshell Blessing");
        addCard(Zone.BATTLEFIELD, playerA, "Plains");
        addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears");
        addCard(Zone.BATTLEFIELD, playerA, "Savannah Lions");

        castSpell(1, PhaseStep.UPKEEP, playerA, "Barkshell Blessing");
        setChoice(playerA, "Yes"); // use Conspire
        addTarget(playerA, "Bonecrusher Giant"); // target bone
        setChoice(playerA, "Grizzly Bears"); // pay for conspire
        setChoice(playerA, "Savannah Lions"); // pay for conspire
        setChoice(playerA, "Yes"); // new targt for copy: bear
        addTarget(playerA, "Grizzly Bears");

        setStrictChooseMode(true);
        setStopAt(1, PhaseStep.PRECOMBAT_MAIN);
        execute();
        assertAllCommandsUsed();

        assertPowerToughness(playerA, "Bonecrusher Giant", 4 + 2, 3 + 2);
        assertPowerToughness(playerA, "Grizzly Bears", 2 + 2, 2 + 2);
        assertPowerToughness(playerA, "Savannah Lions", 2, 1);
        assertLife(playerA, 20 - 2); // one trigger
    }

    @Test
    public void BonecrusherGiantChangeTargets_BoneTargetSecond() {
        // Whenever Bonecrusher Giant becomes the target of a spell, Bonecrusher Giant deals 2 damage to that spell’s controller.
        addCard(Zone.BATTLEFIELD, playerA, "Bonecrusher Giant");
        //
        // Target creature gets +2/+2 until end of turn.
        // Conspire (As you cast this spell, you may tap two untapped creatures you control that share a color with it. When you do, copy it and you may choose a new target for the copy.)
        addCard(Zone.HAND, playerA, "Barkshell Blessing");
        addCard(Zone.BATTLEFIELD, playerA, "Plains");
        addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears");
        addCard(Zone.BATTLEFIELD, playerA, "Savannah Lions");

        castSpell(1, PhaseStep.UPKEEP, playerA, "Barkshell Blessing");
        setChoice(playerA, "Yes"); // use Conspire
        addTarget(playerA, "Grizzly Bears"); // target bear
        setChoice(playerA, "Grizzly Bears"); // pay for conspire
        setChoice(playerA, "Savannah Lions"); // pay for conspire
        setChoice(playerA, "Yes"); // new target for copy: bone
        addTarget(playerA, "Bonecrusher Giant");
        // setChoice(playerA, "When {this} becomes the target of a spell"); // must be one trigger from bone, not two

        setStrictChooseMode(true);
        setStopAt(1, PhaseStep.PRECOMBAT_MAIN);
        execute();
        assertAllCommandsUsed();

        assertPowerToughness(playerA, "Bonecrusher Giant", 4 + 2, 3 + 2);
        assertPowerToughness(playerA, "Grizzly Bears", 2 + 2, 2 + 2);
        assertPowerToughness(playerA, "Savannah Lions", 2, 1);
        assertLife(playerA, 20 - 2); // one trigger
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs and errors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants