Merge pull request #1157 from klayhamn/master

Fixing ThrummingStone and the Ripple ability so that they adher to the rules

Mage.Sets/src/mage/sets/coldsnap/SurgingDementia.java

@@ -29,11 +29,8 @@ package mage.sets.coldsnap;
 
 import java.util.UUID;
 
-import mage.abilities.effects.OneShotEffect;
-import mage.abilities.effects.common.discard.DiscardCardYouChooseTargetEffect;
-import mage.abilities.effects.common.discard.DiscardControllerEffect;
 import mage.abilities.effects.common.discard.DiscardTargetEffect;
-import mage.abilities.effects.keyword.RippleEffect;
+import mage.abilities.keyword.RippleAbility;
 import mage.cards.CardImpl;
 import mage.constants.CardType;
 import mage.constants.Rarity;
@@ -50,7 +47,8 @@ public class SurgingDementia extends CardImpl {
         this.expansionSetCode = "CSP";
 
         // Ripple 4
-        this.getSpellAbility().addEffect(new RippleEffect(4));
+        this.addAbility(new RippleAbility(4));
+
         // Target player discards a card.
         this.getSpellAbility().getEffects().add(new DiscardTargetEffect(1));
         this.getSpellAbility().getTargets().add(new TargetPlayer());

Mage.Sets/src/mage/sets/coldsnap/ThrummingStone.java

@@ -27,38 +27,99 @@
  */
 package mage.sets.coldsnap;
 
-import java.util.UUID;
-import mage.abilities.common.SpellCastControllerTriggeredAbility;
-import mage.abilities.effects.keyword.RippleEffect;
+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;
+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();
+  //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");
 
-        addAbility(new SpellCastControllerTriggeredAbility(new RippleEffect(4, false),  anySpellFilter, false, true));
-    }
+    // 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;
+
+
+  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.copy();
+    this.filter = effect.filter.copy();
+  }
+
+  @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);
+            }
+          }
+        }
+      }
+      return true;
+    }
+    return false;
+  }
+}
+
+

Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/ThrummingStoneTest.java

@@ -0,0 +1,59 @@
+package org.mage.test.cards.abilities.other;
+
+import mage.constants.PhaseStep;
+import mage.constants.Zone;
+import org.junit.Test;
+import org.mage.test.serverside.base.CardTestPlayerBase;
+
+/**
+ * @author klayhamn
+ */
+public class ThrummingStoneTest extends CardTestPlayerBase {
+
+  @Test
+  public void testApplyForNoneRippleCardsWhenSingleRipple() throws Exception {
+
+    removeAllCardsFromLibrary(playerA);
+
+    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);
+
+    castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Shadowborn Apostle");
+    setChoice(playerA, "Yes");
+
+    setStopAt(2, PhaseStep.POSTCOMBAT_MAIN);
+    execute();
+
+    assertPermanentCount(playerA, "Shadowborn Apostle", 2);
+
+  }
+
+  @Test
+  public void testApplyForNoneRippleCardsWhenMultiRipple() throws Exception {
+
+    removeAllCardsFromLibrary(playerA);
+
+    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");
+
+    skipInitShuffling();
+
+    castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Shadowborn Apostle");
+    setChoice(playerA, "Yes");
+
+    setStopAt(2, PhaseStep.POSTCOMBAT_MAIN);
+    execute();
+
+    assertPermanentCount(playerA, "Shadowborn Apostle", 3);
+
+  }
+}

Mage/src/mage/abilities/keyword/RippleAbility.java

@@ -1,7 +1,8 @@
-package mage.abilities.effects.keyword;
+package mage.abilities.keyword;
 
 import mage.MageObject;
 import mage.abilities.Ability;
+import mage.abilities.TriggeredAbilityImpl;
 import mage.abilities.effects.OneShotEffect;
 import mage.cards.Card;
 import mage.cards.Cards;
@@ -11,6 +12,7 @@ import mage.constants.Zone;
 import mage.filter.FilterCard;
 import mage.filter.predicate.mageobject.NamePredicate;
 import mage.game.Game;
+import mage.game.events.GameEvent;
 import mage.game.stack.Spell;
 import mage.players.Player;
 import mage.target.TargetCard;
@@ -19,26 +21,62 @@ import mage.util.CardUtil;
 /**
  * @author klayhamn
  */
-public class RippleEffect extends OneShotEffect {
+public class RippleAbility extends TriggeredAbilityImpl {
 
-    protected int rippleNumber;
-    protected boolean isTargetSelf; // is the source of the ripple also the target of the ripple?
+    protected final int rippleNumber;
 
-    public RippleEffect(int rippleNumber) {
-        this(rippleNumber, true); // by default, the source is also the target
+    public RippleAbility(int rippleNumber) {
+        super(Zone.STACK, new RippleEffect(rippleNumber), false);
+        this.rippleNumber = rippleNumber;
     }
 
-    public RippleEffect(int rippleNumber, boolean isTargetSelf) {
+    public RippleAbility(RippleAbility ability) {
+        super(ability);
+        this.rippleNumber = ability.rippleNumber;
+
+    }
+
+    @Override
+    public boolean checkEventType(GameEvent event, Game game) {
+        return event.getType() == GameEvent.EventType.SPELL_CAST;
+    }
+
+    @Override
+    public boolean checkTrigger(GameEvent event, Game game) {
+        Spell spell = game.getStack().getSpell(event.getTargetId());
+        if (spell != null && spell.getSourceId().equals(this.getSourceId())) {
+            return true;
+        }
+        return false;
+
+    }
+
+    @Override
+    public RippleAbility copy() {
+        return new RippleAbility(this);
+    }
+
+
+    @Override
+    public String getRule() {
+        return "Ripple <i>((When you cast this spell, you may reveal the top " + CardUtil.numberToText(rippleNumber) + " cards of your library. You may cast any revealed cards with the same name as this spell without paying their mana costs. Put the rest on the bottom of your library.))</i>";
+    }
+
+
+}
+
+class RippleEffect extends OneShotEffect {
+
+    protected int rippleNumber;
+
+    public RippleEffect(int rippleNumber) {
         super(Outcome.PlayForFree);
         this.rippleNumber = rippleNumber;
-        this.isTargetSelf = isTargetSelf;
-        this.setText();
     }
 
     public RippleEffect(final RippleEffect effect) {
         super(effect);
         this.rippleNumber = effect.rippleNumber;
-        this.isTargetSelf = effect.isTargetSelf;
     }
 
     @Override
@@ -55,32 +93,19 @@ public class RippleEffect extends OneShotEffect {
             if (!player.chooseUse(Outcome.Neutral, "Reveal " + rippleNumber + " cards from the top of your library?", source, game)){
                 return true; //fizzle
             }
+            // reveal to/**/p cards from library
             Cards cards = new CardsImpl();
-            cards.addAll(player.getLibrary().getTopCards(game, rippleNumber)); // pull top cards
-            player.revealCards(sourceObject.getIdName(), cards, game); // reveal the cards
-
-            // Find out which card should be rippled
-            // FIXME: I'm not sure the "isTargetSelf" flag is the most elegant solution
-            String cardNameToRipple;
-            if (isTargetSelf) { // if the ripple applies to the same card that triggered it
-                cardNameToRipple = sourceObject.getName();
-            } else { // if the ripple is caused by something else (e.g. Thrumming Stone)
-                Spell spellOnStack = game.getStack().getSpell(targetPointer.getFirst(game, source));
-                if (spellOnStack == null) { // if the ripple target got countered or exiled
-                    spellOnStack = (Spell) game.getLastKnownInformation(targetPointer.getFirst(game, source), Zone.STACK);
-                }
-                if (spellOnStack == null) {
-                    return true; // should not happen?
-                }
-                cardNameToRipple = spellOnStack.getName();
-            }
+            cards.addAll(player.getLibrary().getTopCards(game, rippleNumber));
+            player.revealCards(sourceObject.getIdName(), cards, game);
 
+            // determine which card should be rippled
+            String cardNameToRipple = sourceObject.getName();
             FilterCard sameNameFilter = new FilterCard("card(s) with the name: \"" + cardNameToRipple + "\" to cast without paying their mana cost");
             sameNameFilter.add(new NamePredicate(cardNameToRipple));
             TargetCard target1 = new TargetCard(Zone.LIBRARY, sameNameFilter);
             target1.setRequired(false);
 
-            // Choose cards to play for free
+            // choose cards to play for free
             while (player.isInGame() && cards.count(sameNameFilter, game) > 0 && player.choose(Outcome.PlayForFree, cards, target1, game)) {
                 Card card = cards.get(target1.getFirstTarget(), game);
                 if (card != null) {
@@ -97,11 +122,5 @@ public class RippleEffect extends OneShotEffect {
         return false;
     }
 
-    private void setText() {
-        StringBuilder sb = new StringBuilder("Ripple ").append(rippleNumber);
-            sb.append(". <i>(You may reveal the top ");
-            sb.append(CardUtil.numberToText(rippleNumber));
-            sb.append(" cards of your library. You may cast any revealed cards with the same name as this spell without paying their mana costs. Put the rest on the bottom of your library.)</i>");
-        staticText = sb.toString();
-    }
 }
+