From acf28d8aff7f54157bf872d249c62c697dbce1b6 Mon Sep 17 00:00:00 2001 From: LevelX2 Date: Mon, 24 Jul 2017 00:41:03 +0200 Subject: [PATCH] * Breath of Fury - Fixed that the enchnatment was no longer properly moved (fixes #3722). --- .../src/mage/cards/b/BoundDetermined.java | 1 + Mage.Sets/src/mage/cards/b/BreathOfFury.java | 22 +++--- .../cards/enchantments/BreathOfFuryTest.java | 72 +++++++++++++++++++ .../cards/watchers/FuryOfTheHordeTest.java | 39 +++++----- .../java/org/mage/test/player/TestPlayer.java | 18 ++--- .../common/DiesTriggeredAbility.java | 12 +++- .../mage/game/permanent/PermanentImpl.java | 7 +- 7 files changed, 129 insertions(+), 42 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/enchantments/BreathOfFuryTest.java diff --git a/Mage.Sets/src/mage/cards/b/BoundDetermined.java b/Mage.Sets/src/mage/cards/b/BoundDetermined.java index f70f4127ac0..2889105d90d 100644 --- a/Mage.Sets/src/mage/cards/b/BoundDetermined.java +++ b/Mage.Sets/src/mage/cards/b/BoundDetermined.java @@ -120,6 +120,7 @@ class BoundEffect extends OneShotEffect { Permanent toSacrifice = game.getPermanent(target.getFirstTarget()); if (toSacrifice != null) { toSacrifice.sacrifice(source.getSourceId(), game); + game.applyEffects(); int colors = toSacrifice.getColor(game).getColorCount(); if (colors > 0) { TargetCardInYourGraveyard targetCard = new TargetCardInYourGraveyard(0, colors, diff --git a/Mage.Sets/src/mage/cards/b/BreathOfFury.java b/Mage.Sets/src/mage/cards/b/BreathOfFury.java index 74c8da96e24..fe4e83d15f2 100644 --- a/Mage.Sets/src/mage/cards/b/BreathOfFury.java +++ b/Mage.Sets/src/mage/cards/b/BreathOfFury.java @@ -52,12 +52,14 @@ import mage.players.Player; import mage.target.Target; import mage.target.TargetPermanent; import mage.target.common.TargetControlledCreaturePermanent; + /** * @author duncant */ public class BreathOfFury extends CardImpl { + public BreathOfFury(UUID ownerId, CardSetInfo setInfo) { - super(ownerId,setInfo,new CardType[]{CardType.ENCHANTMENT},"{2}{R}{R}"); + super(ownerId, setInfo, new CardType[]{CardType.ENCHANTMENT}, "{2}{R}{R}"); this.subtype.add("Aura"); // Enchant creature you control @@ -103,11 +105,11 @@ class BreathOfFuryAbility extends TriggeredAbilityImpl { @Override public boolean checkTrigger(GameEvent event, Game game) { - DamagedPlayerEvent damageEvent = (DamagedPlayerEvent)event; + DamagedPlayerEvent damageEvent = (DamagedPlayerEvent) event; Permanent enchantment = game.getPermanent(getSourceId()); - if (damageEvent.isCombatDamage() && - enchantment != null && - enchantment.getAttachedTo().equals(event.getSourceId())) { + if (damageEvent.isCombatDamage() + && enchantment != null + && enchantment.getAttachedTo().equals(event.getSourceId())) { Permanent creature = game.getPermanent(enchantment.getAttachedTo()); if (creature != null) { for (Effect effect : getEffects()) { @@ -129,7 +131,7 @@ class BreathOfFuryEffect extends OneShotEffect { public BreathOfFuryEffect() { super(Outcome.Benefit); - staticText = "sacrifice enchanted creature and attach {this} to a creature you control. If you do, untap all creatures you control and after this phase, there is an additional combat phase."; + staticText = "sacrifice enchanted creature and attach {this} to a creature you control. If you do, untap all creatures you control and after this phase, there is an additional combat phase"; } public BreathOfFuryEffect(final BreathOfFuryEffect effect) { @@ -142,7 +144,7 @@ class BreathOfFuryEffect extends OneShotEffect { } @Override - public boolean apply(Game game, Ability source){ + public boolean apply(Game game, Ability source) { Permanent enchantment = game.getPermanent(source.getSourceId()); if (enchantment == null) { return false; @@ -156,8 +158,8 @@ class BreathOfFuryEffect extends OneShotEffect { // It's important to check that the creature was successfully sacrificed here. Effects that prevent sacrifice will also prevent Breath of Fury's effect from working. // Commanders going to the command zone and Rest in Peace style replacement effects don't make Permanent.sacrifice return false. if (enchantedCreature != null && controller != null - && enchantedCreature.sacrifice(source.getSourceId(), game) - && target.canChoose(source.getSourceId(), controller.getId(), game)) { + && enchantedCreature.sacrifice(source.getSourceId(), game) + && target.canChoose(source.getSourceId(), controller.getId(), game)) { controller.choose(outcome, target, source.getSourceId(), game); Permanent newCreature = game.getPermanent(target.getFirstTarget()); boolean success = false; @@ -168,7 +170,7 @@ class BreathOfFuryEffect extends OneShotEffect { success = true; } else { if (oldCreature.removeAttachment(enchantment.getId(), game) - && newCreature.addAttachment(enchantment.getId(), game)) { + && newCreature.addAttachment(enchantment.getId(), game)) { game.informPlayers(enchantment.getLogName() + " was unattached from " + oldCreature.getLogName() + " and attached to " + newCreature.getLogName()); success = true; } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/enchantments/BreathOfFuryTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/enchantments/BreathOfFuryTest.java new file mode 100644 index 00000000000..f99aa4036f6 --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/enchantments/BreathOfFuryTest.java @@ -0,0 +1,72 @@ +/* + * 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.enchantments; + +import mage.constants.PhaseStep; +import mage.constants.Zone; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +/** + * + * @author LevelX2 + */ +public class BreathOfFuryTest extends CardTestPlayerBase { + + @Test + public void testMoveEnchantment() { + + // Enchant creature you control + // When enchanted creature deals combat damage to a player, sacrifice it and attach Breath of Fury to a creature you control. + // If you do, untap all creatures you control and after this phase, there is an additional combat phase. + addCard(Zone.HAND, playerA, "Breath of Fury", 1); // Enchantment - Aura {2}{R}{R} + addCard(Zone.BATTLEFIELD, playerA, "Mountain", 4); + + addCard(Zone.BATTLEFIELD, playerA, "Silvercoat Lion"); + addCard(Zone.BATTLEFIELD, playerA, "Pillarfield Ox"); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Breath of Fury", "Silvercoat Lion"); + + attack(3, playerA, "Pillarfield Ox"); + attack(3, playerA, "Silvercoat Lion"); + + addTarget(playerA, "Pillarfield Ox"); + + setStopAt(3, PhaseStep.POSTCOMBAT_MAIN); + execute(); + + assertLife(playerA, 20); + assertLife(playerB, 16); + + assertGraveyardCount(playerA, "Silvercoat Lion", 1); + assertTappedCount("Pillarfield Ox", false, 1); + assertPermanentCount(playerA, "Breath of Fury", 1); + + } + +} diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/watchers/FuryOfTheHordeTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/watchers/FuryOfTheHordeTest.java index 99286d2cdd7..748ca3b637c 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/watchers/FuryOfTheHordeTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/watchers/FuryOfTheHordeTest.java @@ -10,16 +10,16 @@ import org.mage.test.serverside.base.CardTestPlayerBase; * @author BetaSteward */ public class FuryOfTheHordeTest extends CardTestPlayerBase { + /* * Fury of the Horde * Sorcery, 5RR (7) - * You may exile two red cards from your hand rather than pay Fury of the + * You may exile two red cards from your hand rather than pay Fury of the * Horde's mana cost. - * Untap all creatures that attacked this turn. After this main phase, there + * Untap all creatures that attacked this turn. After this main phase, there * is an additional combat phase followed by an additional main phase. * - */ - + */ // test that creatures attack twice @Test public void testCreaturesAttackTwice() { @@ -27,16 +27,18 @@ public class FuryOfTheHordeTest extends CardTestPlayerBase { addCard(Zone.BATTLEFIELD, playerA, "Craw Wurm"); addCard(Zone.BATTLEFIELD, playerA, "Goblin Roughrider"); addCard(Zone.HAND, playerA, "Fury of the Horde"); - + attack(3, playerA, "Craw Wurm"); attack(3, playerA, "Goblin Roughrider"); castSpell(3, PhaseStep.POSTCOMBAT_MAIN, playerA, "Fury of the Horde"); - + attack(3, playerA, "Craw Wurm"); + attack(3, playerA, "Goblin Roughrider"); + setStopAt(3, PhaseStep.END_TURN); execute(); - + this.assertLife(playerB, 2); - + } // test that creatures attack once @@ -46,17 +48,17 @@ public class FuryOfTheHordeTest extends CardTestPlayerBase { addCard(Zone.BATTLEFIELD, playerA, "Craw Wurm"); addCard(Zone.BATTLEFIELD, playerA, "Goblin Roughrider"); addCard(Zone.HAND, playerA, "Fury of the Horde"); - + attack(3, playerA, "Craw Wurm"); attack(3, playerA, "Goblin Roughrider"); - + setStopAt(3, PhaseStep.END_TURN); execute(); - + this.assertLife(playerB, 11); - + } - + // test that only creatures that attacked attack twice @Test public void testCreaturesThatAttacked() { @@ -64,15 +66,16 @@ public class FuryOfTheHordeTest extends CardTestPlayerBase { addCard(Zone.BATTLEFIELD, playerA, "Craw Wurm"); addCard(Zone.BATTLEFIELD, playerA, "Goblin Roughrider"); addCard(Zone.HAND, playerA, "Fury of the Horde"); - + attack(3, playerA, "Craw Wurm"); castSpell(3, PhaseStep.POSTCOMBAT_MAIN, playerA, "Fury of the Horde"); - + attack(3, playerA, "Craw Wurm"); + setStopAt(3, PhaseStep.END_TURN); execute(); - + this.assertLife(playerB, 8); - + } - + } diff --git a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java index b35298b2309..ce65580ab56 100644 --- a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java +++ b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java @@ -27,6 +27,10 @@ */ package org.mage.test.player; +import java.io.Serializable; +import java.util.*; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import mage.MageObject; import mage.MageObjectReference; import mage.abilities.*; @@ -50,7 +54,6 @@ import mage.filter.FilterPermanent; import mage.filter.common.*; import mage.filter.predicate.Predicates; import mage.filter.predicate.mageobject.NamePredicate; -import mage.filter.predicate.permanent.AttackingPredicate; import mage.filter.predicate.permanent.SummoningSicknessPredicate; import mage.game.Game; import mage.game.Graveyard; @@ -71,11 +74,6 @@ import mage.target.*; import mage.target.common.*; import org.junit.Ignore; -import java.io.Serializable; -import java.util.*; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - /** * @author BetaSteward_at_googlemail.com * @author Simown @@ -585,7 +583,9 @@ public class TestPlayer implements Player { public void selectAttackers(Game game, UUID attackingPlayerId) { // Loop through players and validate can attack/block this turn UUID defenderId = null; - for (PlayerAction action : actions) { + //List + for (Iterator it = actions.iterator(); it.hasNext();) { + PlayerAction action = it.next(); if (action.getTurnNum() == game.getTurnNum() && action.getAction().startsWith("attack:")) { String command = action.getAction(); command = command.substring(command.indexOf("attack:") + 7); @@ -623,14 +623,16 @@ public class TestPlayer implements Player { findPermanent(firstFilter, groups[0], computerPlayer.getId(), game); // Second check to filter creature for combat - less strict to workaround issue in #3038 FilterCreatureForCombat secondFilter = new FilterCreatureForCombat(); - secondFilter.add(Predicates.not(new AttackingPredicate())); + // secondFilter.add(Predicates.not(new AttackingPredicate())); secondFilter.add(Predicates.not(new SummoningSicknessPredicate())); // TODO: Cannot enforce legal attackers multiple times per combat. See issue #3038 Permanent attacker = findPermanent(secondFilter, groups[0], computerPlayer.getId(), game, false); if (attacker != null && attacker.canAttack(defenderId, game)) { computerPlayer.declareAttacker(attacker.getId(), defenderId, game, false); + it.remove(); } } + } } diff --git a/Mage/src/main/java/mage/abilities/common/DiesTriggeredAbility.java b/Mage/src/main/java/mage/abilities/common/DiesTriggeredAbility.java index b587bbc7bba..6496e8ade82 100644 --- a/Mage/src/main/java/mage/abilities/common/DiesTriggeredAbility.java +++ b/Mage/src/main/java/mage/abilities/common/DiesTriggeredAbility.java @@ -57,12 +57,20 @@ public class DiesTriggeredAbility extends ZoneChangeTriggeredAbility { public boolean isInUseableZone(Game game, MageObject source, GameEvent event) { // check it was previously on battlefield Permanent before = ((ZoneChangeEvent) event).getTarget(); + if (before == null) { + return false; + } if (!(before instanceof PermanentToken) && !this.hasSourceObjectAbility(game, before, event)) { return false; } // check now it is in graveyard - Zone after = game.getState().getZone(sourceId); - return before != null && after != null && Zone.GRAVEYARD.match(after); + if (before.getZoneChangeCounter(game) + 1 == game.getState().getZoneChangeCounter(source.getId())) { + Zone after = game.getState().getZone(sourceId); + return after != null && Zone.GRAVEYARD.match(after); + } else { + // Already moved to another zone, so guess it's ok + return true; + } } @Override diff --git a/Mage/src/main/java/mage/game/permanent/PermanentImpl.java b/Mage/src/main/java/mage/game/permanent/PermanentImpl.java index 194df0a44a7..9da60b0f793 100644 --- a/Mage/src/main/java/mage/game/permanent/PermanentImpl.java +++ b/Mage/src/main/java/mage/game/permanent/PermanentImpl.java @@ -27,6 +27,7 @@ */ package mage.game.permanent; +import java.util.*; import mage.MageObject; import mage.MageObjectReference; import mage.ObjectColor; @@ -55,8 +56,6 @@ import mage.players.Player; import mage.util.GameLog; import mage.util.ThreadLocalStringBuilder; -import java.util.*; - /** * @author BetaSteward_at_googlemail.com */ @@ -990,8 +989,8 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { game.informPlayers(player.getLogName() + " sacrificed " + this.getLogName()); } game.fireEvent(GameEvent.getEvent(EventType.SACRIFICED_PERMANENT, objectId, sourceId, controllerId)); - game.checkStateAndTriggered(); - game.applyEffects(); + // game.checkStateAndTriggered(); + // game.applyEffects(); return true; } return false;