From cac6a03e7ff78a37a3537d1e19113605950d46bf Mon Sep 17 00:00:00 2001 From: klayhamn Date: Tue, 28 Jul 2015 17:38:50 +0300 Subject: [PATCH] * fix potential bug where the copy constructor of the continuous effect of ThrummingStone did not copy its object parameters * unignore the thrumming stone test --- .../mage/sets/coldsnap/ThrummingStone.java | 113 +++++++++--------- .../abilities/other/ThrummingStoneTest.java | 62 +++++----- 2 files changed, 89 insertions(+), 86 deletions(-) diff --git a/Mage.Sets/src/mage/sets/coldsnap/ThrummingStone.java b/Mage.Sets/src/mage/sets/coldsnap/ThrummingStone.java index c7f1a281d01..a7a7b45af73 100644 --- a/Mage.Sets/src/mage/sets/coldsnap/ThrummingStone.java +++ b/Mage.Sets/src/mage/sets/coldsnap/ThrummingStone.java @@ -27,14 +27,18 @@ */ package mage.sets.coldsnap; -import java.util.UUID; - import mage.abilities.Ability; import mage.abilities.common.SimpleStaticAbility; import mage.abilities.effects.ContinuousEffectImpl; import mage.abilities.keyword.RippleAbility; import mage.cards.CardImpl; -import mage.constants.*; +import mage.constants.CardType; +import mage.constants.Duration; +import mage.constants.Layer; +import mage.constants.Outcome; +import mage.constants.Rarity; +import mage.constants.SubLayer; +import mage.constants.Zone; import mage.filter.FilterSpell; import mage.game.Game; import mage.game.permanent.Permanent; @@ -42,79 +46,80 @@ import mage.game.stack.Spell; import mage.game.stack.StackObject; import mage.players.Player; +import java.util.UUID; + /** - * * @author klayhamn */ public class ThrummingStone extends CardImpl { - //applies to all spells - private static final FilterSpell anySpellFilter = new FilterSpell("Spells you cast"); + //applies to all spells + private static final FilterSpell anySpellFilter = new FilterSpell("Spells you cast"); - public ThrummingStone(UUID ownerId) { - super(ownerId, 142, "Thrumming Stone", Rarity.RARE, new CardType[]{CardType.ARTIFACT}, "{5}"); - this.expansionSetCode = "CSP"; - this.supertype.add("Legendary"); + public ThrummingStone(UUID ownerId) { + super(ownerId, 142, "Thrumming Stone", Rarity.RARE, new CardType[]{CardType.ARTIFACT}, "{5}"); + this.expansionSetCode = "CSP"; + this.supertype.add("Legendary"); - // spells you cast have Ripple 4 - this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new ThrummingStoneGainAbilitySpellsEffect(new RippleAbility(4), anySpellFilter))); - } + // spells you cast have Ripple 4 + this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new ThrummingStoneGainAbilitySpellsEffect(new RippleAbility(4), anySpellFilter))); + } - public ThrummingStone(final ThrummingStone card) { - super(card); - } + public ThrummingStone(final ThrummingStone card) { + super(card); + } - @Override - public ThrummingStone copy() { - return new ThrummingStone(this); - } + @Override + public ThrummingStone copy() { + return new ThrummingStone(this); + } } class ThrummingStoneGainAbilitySpellsEffect extends ContinuousEffectImpl { - private final Ability ability; - private final FilterSpell filter; + private final Ability ability; + private final FilterSpell filter; - public ThrummingStoneGainAbilitySpellsEffect(Ability ability, FilterSpell filter) { - super(Duration.WhileOnBattlefield, Layer.AbilityAddingRemovingEffects_6, SubLayer.NA, Outcome.AddAbility); - this.ability = ability; - this.filter = filter; - staticText = filter.getMessage() + " have " + ability.getRule(); - } + public ThrummingStoneGainAbilitySpellsEffect(Ability ability, FilterSpell filter) { + super(Duration.WhileOnBattlefield, Layer.AbilityAddingRemovingEffects_6, SubLayer.NA, Outcome.AddAbility); + this.ability = ability; + this.filter = filter; + staticText = filter.getMessage() + " have " + ability.getRule(); + } - public ThrummingStoneGainAbilitySpellsEffect(final ThrummingStoneGainAbilitySpellsEffect effect) { - super(effect); - this.ability = effect.ability; - this.filter = effect.filter; - } + public ThrummingStoneGainAbilitySpellsEffect(final ThrummingStoneGainAbilitySpellsEffect effect) { + super(effect); + this.ability = effect.ability.copy(); + this.filter = effect.filter.copy(); + } - @Override - public ThrummingStoneGainAbilitySpellsEffect copy() { - return new ThrummingStoneGainAbilitySpellsEffect(this); - } + @Override + public ThrummingStoneGainAbilitySpellsEffect copy() { + return new ThrummingStoneGainAbilitySpellsEffect(this); + } - @Override - public boolean apply(Game game, Ability source) { - Player player = game.getPlayer(source.getControllerId()); - Permanent permanent = game.getPermanent(source.getSourceId()); - if (player != null && permanent != null) { - for (StackObject stackObject : game.getStack()) { - // only spells cast, so no copies of spells - if ((stackObject instanceof Spell) && !stackObject.isCopy() && stackObject.getControllerId().equals(source.getControllerId())) { - Spell spell = (Spell) stackObject; - if (filter.match(spell, game)) { - if (!spell.getAbilities().contains(ability)) { - game.getState().addOtherAbility(spell.getCard(), ability); - } - } - } + @Override + public boolean apply(Game game, Ability source) { + Player player = game.getPlayer(source.getControllerId()); + Permanent permanent = game.getPermanent(source.getSourceId()); + if (player != null && permanent != null) { + for (StackObject stackObject : game.getStack()) { + // only spells cast, so no copies of spells + if ((stackObject instanceof Spell) && !stackObject.isCopy() && stackObject.getControllerId().equals(source.getControllerId())) { + Spell spell = (Spell) stackObject; + if (filter.match(spell, game)) { + if (!spell.getAbilities().contains(ability)) { + game.getState().addOtherAbility(spell.getCard(), ability); } - return true; + } } - return false; + } + return true; } + return false; + } } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/ThrummingStoneTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/ThrummingStoneTest.java index 0776f50e703..1b84ec26f63 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/ThrummingStoneTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/ThrummingStoneTest.java @@ -1,6 +1,5 @@ package org.mage.test.cards.abilities.other; -import jdk.nashorn.internal.ir.annotations.Ignore; import mage.constants.PhaseStep; import mage.constants.Zone; import org.junit.Test; @@ -11,51 +10,50 @@ import org.mage.test.serverside.base.CardTestPlayerBase; */ public class ThrummingStoneTest extends CardTestPlayerBase { - @Test - public void testApplyForNoneRippleCardsWhenSingleRipple() throws Exception { + @Test + public void testApplyForNoneRippleCardsWhenSingleRipple() throws Exception { - removeAllCardsFromLibrary(playerA); + removeAllCardsFromLibrary(playerA); - addCard(Zone.BATTLEFIELD, playerA, "Thrumming Stone"); - addCard(Zone.BATTLEFIELD, playerA, "Swamp"); - addCard(Zone.HAND, playerA, "Shadowborn Apostle"); + addCard(Zone.BATTLEFIELD, playerA, "Thrumming Stone"); + addCard(Zone.BATTLEFIELD, playerA, "Swamp"); + addCard(Zone.HAND, playerA, "Shadowborn Apostle"); - addCard(Zone.LIBRARY, playerA, "Shadowborn Apostle", 1); - addCard(Zone.LIBRARY, playerA, "Swamp", 3); + addCard(Zone.LIBRARY, playerA, "Shadowborn Apostle", 1); + addCard(Zone.LIBRARY, playerA, "Swamp", 3); - castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Shadowborn Apostle"); - setChoice(playerA, "Yes"); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Shadowborn Apostle"); + setChoice(playerA, "Yes"); - setStopAt(2, PhaseStep.POSTCOMBAT_MAIN); - execute(); + setStopAt(2, PhaseStep.POSTCOMBAT_MAIN); + execute(); - assertPermanentCount(playerA, "Shadowborn Apostle", 2); + assertPermanentCount(playerA, "Shadowborn Apostle", 2); - } + } - @Test - @Ignore // FIXME: This still fails - public void testApplyForNoneRippleCardsWhenMultiRipple() throws Exception { + @Test + public void testApplyForNoneRippleCardsWhenMultiRipple() throws Exception { - removeAllCardsFromLibrary(playerA); + removeAllCardsFromLibrary(playerA); - addCard(Zone.BATTLEFIELD, playerA, "Thrumming Stone"); - addCard(Zone.BATTLEFIELD, playerA, "Swamp"); - addCard(Zone.HAND, playerA, "Shadowborn Apostle"); + addCard(Zone.BATTLEFIELD, playerA, "Thrumming Stone"); + addCard(Zone.BATTLEFIELD, playerA, "Swamp"); + addCard(Zone.HAND, playerA, "Shadowborn Apostle"); - addCard(Zone.LIBRARY, playerA, "Shadowborn Apostle"); - addCard(Zone.LIBRARY, playerA, "Swamp", 3); - addCard(Zone.LIBRARY, playerA, "Shadowborn Apostle"); + addCard(Zone.LIBRARY, playerA, "Shadowborn Apostle"); + addCard(Zone.LIBRARY, playerA, "Swamp", 3); + addCard(Zone.LIBRARY, playerA, "Shadowborn Apostle"); - skipInitShuffling(); + skipInitShuffling(); - castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Shadowborn Apostle"); - setChoice(playerA, "Yes"); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Shadowborn Apostle"); + setChoice(playerA, "Yes"); - setStopAt(2, PhaseStep.POSTCOMBAT_MAIN); - execute(); + setStopAt(2, PhaseStep.POSTCOMBAT_MAIN); + execute(); - assertPermanentCount(playerA, "Shadowborn Apostle", 3); + assertPermanentCount(playerA, "Shadowborn Apostle", 3); - } + } }