From 87c057654979c0d15bba33b7c0ba74666bf843b3 Mon Sep 17 00:00:00 2001 From: LevelX2 Date: Wed, 19 Mar 2014 16:44:57 +0100 Subject: [PATCH] Fixed that modes of modal spells resolved also if all targeted modes of a spell had no more legal targets and there were targeted modes (fixes #385). Should also fix same problem for parts of a fused spell. Added test for Cryptic Command. --- .../counterspell/CrypticCommandTest.java | 80 ++++++++++ .../mage/abilities/ActivatedAbilityImpl.java | 137 ------------------ Mage/src/mage/game/stack/Spell.java | 53 +++++-- 3 files changed, 123 insertions(+), 147 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/abilities/oneshot/counterspell/CrypticCommandTest.java diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/oneshot/counterspell/CrypticCommandTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/oneshot/counterspell/CrypticCommandTest.java new file mode 100644 index 00000000000..e6c5b812a59 --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/oneshot/counterspell/CrypticCommandTest.java @@ -0,0 +1,80 @@ +/* +* Copyright 2010 BetaSteward_at_googlemail.com. All rights reserved. +* +* Redistribution and use in source and binary forms, with or without modification, are +* permitted provided that the following conditions are met: +* +* 1. Redistributions of source code must retain the above copyright notice, this list of +* conditions and the following disclaimer. +* +* 2. Redistributions in binary form must reproduce the above copyright notice, this list +* of conditions and the following disclaimer in the documentation and/or other materials +* provided with the distribution. +* +* THIS SOFTWARE IS PROVIDED BY BetaSteward_at_googlemail.com ``AS IS'' AND ANY EXPRESS OR IMPLIED +* WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND +* FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL BetaSteward_at_googlemail.com OR +* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON +* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +* NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF +* ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +* +* The views and conclusions contained in the software and documentation are those of the +* authors and should not be interpreted as representing official policies, either expressed +* or implied, of BetaSteward_at_googlemail.com. +*/ + +package org.mage.test.cards.abilities.oneshot.counterspell; + +import mage.constants.PhaseStep; +import mage.constants.Zone; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +/** + * Cryptic Command + * Instant, 1UUU + * Choose two — Counter target spell; or return target permanent to its owner's hand; or tap all creatures your opponents control; or draw a card. + * + * @author LevelX2 + */ +public class CrypticCommandTest extends CardTestPlayerBase { + + /** + * Test that if command has only one target and that targets is not valid on resolution, Cryptic Command fizzeles + * The player does not draw a card + */ + @Test + public void testCommand() { + addCard(Zone.HAND, playerA, "Thoughtseize"); + addCard(Zone.HAND, playerA, "Remand"); + addCard(Zone.BATTLEFIELD, playerA, "Swamp", 1); + addCard(Zone.BATTLEFIELD, playerA, "Island", 2); + + addCard(Zone.HAND, playerB, "Cryptic Command"); + addCard(Zone.BATTLEFIELD, playerB, "Island", 4); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Thoughtseize", playerB); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Cryptic Command", "Thoughtseize"); + setModeChoice(playerB, "0"); // Counter target spell + setModeChoice(playerB, "3"); // Draw a card + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Remand", "Thoughtseize", "Cryptic Command"); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertLife(playerA, 20); + assertLife(playerB, 20); + + assertHandCount(playerA, 2); + assertGraveyardCount(playerA, 1); + assertHandCount(playerB, 0); // Because Cryptic Command has no legal target playerB does not draw a card and has 0 cards in hand + assertGraveyardCount(playerB, 1); + + } + +} diff --git a/Mage/src/mage/abilities/ActivatedAbilityImpl.java b/Mage/src/mage/abilities/ActivatedAbilityImpl.java index 93185bb4df2..18be8da6ba3 100644 --- a/Mage/src/mage/abilities/ActivatedAbilityImpl.java +++ b/Mage/src/mage/abilities/ActivatedAbilityImpl.java @@ -31,25 +31,17 @@ package mage.abilities; import java.util.UUID; import mage.constants.AbilityType; -import mage.constants.SpellAbilityType; import mage.constants.TimingRule; import mage.constants.Zone; -import mage.MageObject; -import mage.abilities.costs.AlternativeSourceCosts; import mage.abilities.costs.Cost; import mage.abilities.costs.Costs; -import mage.abilities.costs.OptionalAdditionalSourceCosts; import mage.abilities.costs.mana.ManaCosts; import mage.abilities.costs.mana.PhyrexianManaCost; import mage.abilities.effects.Effect; import mage.abilities.effects.Effects; import mage.cards.Card; -import mage.choices.Choice; import mage.constants.TargetController; import mage.game.Game; -import mage.game.stack.Spell; -import mage.game.stack.StackAbility; -import mage.target.Target; /** @@ -207,135 +199,6 @@ public abstract class ActivatedAbilityImpl> ex return false; } -// @Override -// public String getGameLogMessage(Game game) { -// if (game.isSimulation()) { -// return ""; -// } -// MageObject object = game.getObject(this.sourceId); -// return new StringBuilder(" activates: ") -// .append(object != null ? this.formatRule(modes.getText(), object.getName()) :modes.getText()) -// .append(" from ") -// .append(getMessageText(game)).toString(); -// } -// -// @Override -// protected String getMessageText(Game game) { -// StringBuilder sb = new StringBuilder(); -// MageObject object = game.getObject(this.sourceId); -// if (object == null) { -// object = game.getLastKnownInformation(this.sourceId, Zone.BATTLEFIELD); -// } -// if (object != null) { -// if (object instanceof StackAbility) { -// Card card = game.getCard(((StackAbility) object).getSourceId()); -// if (card != null) { -// sb.append(card.getName()); -// } else { -// sb.append(object.getName()); -// } -// } else { -// if (object instanceof Spell) { -// Spell spell = (Spell) object; -// String castText = spell.getSpellAbility().toString(); -// sb.append((castText.startsWith("Cast ") ? castText.substring(5):castText)); -// if (spell.getFromZone() == Zone.GRAVEYARD) { -// sb.append(" from graveyard"); -// } -// sb.append(getOptionalTextSuffix(game, spell)); -// } else { -// sb.append(object.getName()); -// } -// } -// } else { -// sb.append("unknown"); -// } -// if (object instanceof Spell && ((Spell) object).getSpellAbilities().size() > 1) { -// if (((Spell) object).getSpellAbility().getSpellAbilityType().equals(SpellAbilityType.SPLIT_FUSED)) { -// Spell spell = (Spell) object; -// int i = 0; -// for (SpellAbility spellAbility : spell.getSpellAbilities()) { -// i++; -// String half; -// if (i == 1) { -// half = " left"; -// } else { -// half = " right"; -// } -// if (spellAbility.getTargets().size() > 0) { -// sb.append(half).append(" half targeting "); -// for (Target target: spellAbility.getTargets()) { -// sb.append(target.getTargetedName(game)); -// } -// } -// } -// } else { -// Spell spell = (Spell) object; -// int i = 0; -// for (SpellAbility spellAbility : spell.getSpellAbilities()) { -// i++; -// if ( i > 1) { -// sb.append(" splicing "); -// if (spellAbility.name.length() > 5 && spellAbility.name.startsWith("Cast ")) { -// sb.append(spellAbility.name.substring(5)); -// } else { -// sb.append(spellAbility.name); -// } -// } -// appendTargetDescriptionForLog(sb, spellAbility.getTargets(), game); -// } -// } -// } else if (object instanceof Spell && ((Spell) object).getSpellAbility().getModes().size() > 1) { -// Modes spellModes = ((Spell) object).getSpellAbility().getModes(); -// int item = 0; -// for (Mode mode : spellModes.values()) { -// item++; -// if (spellModes.getSelectedModes().contains(mode.getId())) { -// spellModes.setMode(mode); -// sb.append(" (mode ").append(item).append(")"); -// appendTargetDescriptionForLog(sb, getTargets(), game); -// } -// } -// } else { -// appendTargetDescriptionForLog(sb, getTargets(), game); -// } -// for (Choice choice :this.getChoices()) { -// sb.append(" - ").append(choice.getMessage()).append(": ").append(choice.getChoice()); -// } -// return sb.toString(); -// } - -// private void appendTargetDescriptionForLog(StringBuilder sb, Targets targets, Game game) { -// if (targets.size() > 0) { -// String usedVerb = null; -// for (Target target: targets) { -// if (!target.isNotTarget()) { -// if (usedVerb == null || usedVerb.equals(" choosing ")) { -// usedVerb = " targeting "; -// sb.append(usedVerb); -// } -// } else if (target.isNotTarget() && (usedVerb == null || usedVerb.equals(" targeting "))) { -// usedVerb = " choosing "; -// sb.append(usedVerb); -// } -// sb.append(target.getTargetedName(game)); -// } -// } -// } - -// private String getOptionalTextSuffix(Game game, Spell spell) { -// StringBuilder sb = new StringBuilder(); -// for (Ability ability : (Abilities) spell.getAbilities()) { -// if (ability instanceof OptionalAdditionalSourceCosts) { -// sb.append(((OptionalAdditionalSourceCosts) ability).getCastMessageSuffix()); -// } -// if (ability instanceof AlternativeSourceCosts) { -// sb.append(((AlternativeSourceCosts) ability).getCastMessageSuffix()); -// } -// } -// return sb.toString(); -// } - public void setMayActivate(TargetController mayActivate) { this.mayActivate = mayActivate; } diff --git a/Mage/src/mage/game/stack/Spell.java b/Mage/src/mage/game/stack/Spell.java index cb31bd8c880..5c86b1e0c26 100644 --- a/Mage/src/mage/game/stack/Spell.java +++ b/Mage/src/mage/game/stack/Spell.java @@ -154,20 +154,31 @@ public class Spell> implements StackObject, Card { int index = 0; result = false; boolean legalParts = false; + // check for legal parts for(SpellAbility spellAbility: this.spellAbilities) { - for (UUID modeId :spellAbility.getModes().getSelectedModes()) { - spellAbility.getModes().setMode(spellAbility.getModes().get(modeId)); - if (spellAbility.getTargets().stillLegal(spellAbility, game)) { - legalParts = true; - if (!spellAbility.getSpellAbilityType().equals(SpellAbilityType.SPLICE)) { - updateOptionalCosts(index); + // if muliple modes are selected, and there are modes with targets, then at least one mode has to have a legal target or + // When resolving a fused split spell with multiple targets, treat it as you would any spell with multiple targets. + // If all targets are illegal when the spell tries to resolve, the spell is countered and none of its effects happen. + // If at least one target is still legal at that time, the spell resolves, but an illegal target can’t perform any actions + // or have any actions performed on it. + legalParts |= spellAbilityHasLegalParts(spellAbility, game); + } + // resolve if legal parts + if (legalParts) { + for(SpellAbility spellAbility: this.spellAbilities) { + if (spellAbilityHasLegalParts(spellAbility, game)) { + for (UUID modeId :spellAbility.getModes().getSelectedModes()) { + spellAbility.getModes().setMode(spellAbility.getModes().get(modeId)); + if (spellAbility.getTargets().stillLegal(spellAbility, game)) { + if (!spellAbility.getSpellAbilityType().equals(SpellAbilityType.SPLICE)) { + updateOptionalCosts(index); + } + result |= spellAbility.resolve(game); + } } - result |= spellAbility.resolve(game); + index++; } } - index++; - } - if (legalParts) { if (!copiedSpell) { for (Effect effect : ability.getEffects()) { if (effect instanceof PostResolveEffect) { @@ -213,6 +224,28 @@ public class Spell> implements StackObject, Card { } } + private boolean spellAbilityHasLegalParts(SpellAbility spellAbility, Game game) { + if (spellAbility.getModes().getSelectedModes().size() > 1) { + boolean targetedMode = false; + boolean legalTargetedMode = false; + for (UUID modeId :spellAbility.getModes().getSelectedModes()) { + spellAbility.getModes().setMode(spellAbility.getModes().get(modeId)); + if (spellAbility.getTargets().size() > 0) { + targetedMode = true; + if (spellAbility.getTargets().stillLegal(spellAbility, game)) { + legalTargetedMode = true; + } + } + } + if (targetedMode) { + return legalTargetedMode; + } + return true; + } else { + return spellAbility.getTargets().stillLegal(spellAbility, game); + } + } + /** * As we have ability in the stack, we need to update optional costs in original card. * This information will be used later by effects, e.g. to determine whether card was kicked or not.