Unbound Flourishing's X doubling should be a triggered ability (and related refactors) (#12597)

Complete rework of Unbound Flourishing, removing the multiplier code for casting X spells.
Adds ActivateAbilityTriggeredAbility, NotManaAbilityPredicate, AbilitySourceAttachedPredicate
CopyStackObjectEffect now uses a MOR.
OrTriggeredAbility now works with target pointer setting abilities.

Mage/src/main/java/mage/abilities/AbilityImpl.java

@@ -278,8 +278,6 @@ public abstract class AbilityImpl implements Ability {
                 int xValue = CardUtil.getSourceCostsTag(game, this, "X", 0);
                 this.clearManaCostsToPay();
                 VariableManaCost xCosts = new VariableManaCost(VariableCostType.ADDITIONAL);
-                // no x events - rules from Unbound Flourishing:
-                // - Spells with additional costs that include X won't be affected by Unbound Flourishing. X must be in the spell's mana cost.
                 xCosts.setAmount(xValue, xValue, false);
                 addManaCostsToPay(xCosts);
             } else {
@@ -617,8 +615,6 @@ public abstract class AbilityImpl implements Ability {
                 Cost fixedCost = variableCost.getFixedCostsFromAnnouncedValue(xValue);
                 addCost(fixedCost);
                 // set the xcosts to paid
-                // no x events - rules from Unbound Flourishing:
-                // - Spells with additional costs that include X won't be affected by Unbound Flourishing. X must be in the spell's mana cost.
                 variableCost.setAmount(xValue, xValue, false);
                 ((Cost) variableCost).setPaid();
                 String message = controller.getLogName() + " announces a value of " + xValue + " (" + variableCost.getActionText() + ')'
@@ -653,13 +649,6 @@ public abstract class AbilityImpl implements Ability {
         }
     }
 
-    public int handleManaXMultiplier(Game game, int value) {
-        // some spells can change X value without new pays (Unbound Flourishing doubles X)
-        GameEvent xEvent = GameEvent.getEvent(GameEvent.EventType.X_MANA_ANNOUNCE, this.getId(), this, getControllerId(), value);
-        game.replaceEvent(xEvent, this);
-        return xEvent.getAmount();
-    }
-
     /**
      * Handles X mana costs and sets manaCostsToPay.
      *
@@ -695,9 +684,8 @@ public abstract class AbilityImpl implements Ability {
         if (variableManaCost != null) {
             if (!variableManaCost.isPaid()) { // should only happen for human players
                 int xValue;
-                int xValueMultiplier = handleManaXMultiplier(game, 1);
                 if (!noMana || variableManaCost.getCostType().canUseAnnounceOnFreeCast()) {
-                    xValue = controller.announceXMana(variableManaCost.getMinX(), variableManaCost.getMaxX(), xValueMultiplier,
+                    xValue = controller.announceXMana(variableManaCost.getMinX(), variableManaCost.getMaxX(),
                             "Announce the value for " + variableManaCost.getText(), game, this);
                     int amountMana = xValue * variableManaCost.getXInstancesCount();
                     StringBuilder manaString = threadLocalBuilder.get();
@@ -728,8 +716,8 @@ public abstract class AbilityImpl implements Ability {
                         }
                     }
                     addManaCostsToPay(new ManaCostsImpl<>(manaString.toString()));
-                    getManaCostsToPay().setX(xValue * xValueMultiplier, amountMana);
-                    setCostsTag("X", xValue * xValueMultiplier);
+                    getManaCostsToPay().setX(xValue, amountMana);
+                    setCostsTag("X", xValue);
                 }
                 variableManaCost.setPaid();
             }

Mage/src/main/java/mage/abilities/common/ActivateAbilityTriggeredAbility.java

@@ -0,0 +1,73 @@
+package mage.abilities.common;
+
+import mage.abilities.TriggeredAbilityImpl;
+import mage.abilities.effects.Effect;
+import mage.constants.SetTargetPointer;
+import mage.constants.Zone;
+import mage.filter.FilterStackObject;
+import mage.game.Game;
+import mage.game.events.GameEvent;
+import mage.game.stack.StackAbility;
+import mage.target.targetpointer.FixedTarget;
+
+public class ActivateAbilityTriggeredAbility extends TriggeredAbilityImpl {
+
+    private final FilterStackObject filter;
+    protected final SetTargetPointer setTargetPointer;
+
+    public ActivateAbilityTriggeredAbility(Effect effect, FilterStackObject filter, SetTargetPointer setTargetPointer) {
+        this(Zone.BATTLEFIELD, effect, filter, setTargetPointer);
+    }
+
+    public ActivateAbilityTriggeredAbility(Zone zone, Effect effect, FilterStackObject filter, SetTargetPointer setTargetPointer) {
+        super(zone, effect, false);
+        this.filter = filter;
+        this.setTargetPointer = setTargetPointer;
+        setTriggerPhrase("Whenever you activate " + filter.getMessage() + ", ");
+    }
+
+    private ActivateAbilityTriggeredAbility(final ActivateAbilityTriggeredAbility ability) {
+        super(ability);
+        this.filter = ability.filter;
+        this.setTargetPointer = ability.setTargetPointer;
+    }
+
+    @Override
+    public ActivateAbilityTriggeredAbility copy() {
+        return new ActivateAbilityTriggeredAbility(this);
+    }
+
+    @Override
+    public boolean checkEventType(GameEvent event, Game game) {
+        return event.getType() == GameEvent.EventType.ACTIVATED_ABILITY;
+    }
+
+    @Override
+    public boolean checkTrigger(GameEvent event, Game game) {
+        if (!event.getPlayerId().equals(getControllerId())) {
+            return false;
+        }
+        StackAbility stackAbility = (StackAbility) game.getStack().getStackObject(event.getTargetId());
+        if (stackAbility == null) {
+            return false;
+        }
+
+        if (!filter.match(stackAbility, event.getPlayerId(), this, game)) {
+            return false;
+        }
+
+        switch (setTargetPointer) {
+            case NONE:
+                break;
+            case PLAYER:
+                getAllEffects().setTargetPointer(new FixedTarget(getControllerId(), game));
+                break;
+            case SPELL:
+                getAllEffects().setTargetPointer(new FixedTarget(event.getTargetId(), game));
+                break;
+            default:
+                throw new UnsupportedOperationException("Unexpected setTargetPointer in ActivateAbilityTriggeredAbility: " + setTargetPointer);
+        }
+        return true;
+    }
+}

Mage/src/main/java/mage/abilities/common/ActivatePlaneswalkerLoyaltyAbilityTriggeredAbility.java

@@ -3,26 +3,31 @@ package mage.abilities.common;
 import mage.abilities.LoyaltyAbility;
 import mage.abilities.TriggeredAbilityImpl;
 import mage.abilities.effects.Effect;
+import mage.constants.SetTargetPointer;
 import mage.constants.SubType;
 import mage.constants.Zone;
 import mage.game.Game;
 import mage.game.events.GameEvent;
 import mage.game.permanent.Permanent;
 import mage.game.stack.StackAbility;
+import mage.target.targetpointer.FixedTarget;
 
 public class ActivatePlaneswalkerLoyaltyAbilityTriggeredAbility extends TriggeredAbilityImpl {
 
     private final SubType planeswalkerSubType;
+    protected final SetTargetPointer setTargetPointer;
 
-    public ActivatePlaneswalkerLoyaltyAbilityTriggeredAbility(Effect effect, SubType planeswalkerSubType) {
+    public ActivatePlaneswalkerLoyaltyAbilityTriggeredAbility(Effect effect, SubType planeswalkerSubType, SetTargetPointer setTargetPointer) {
         super(Zone.BATTLEFIELD, effect, false);
         this.planeswalkerSubType = planeswalkerSubType;
+        this.setTargetPointer = setTargetPointer;
         setTriggerPhrase("Whenever you activate a loyalty ability of a " + planeswalkerSubType.getDescription() + " planeswalker, ");
     }
 
     private ActivatePlaneswalkerLoyaltyAbilityTriggeredAbility(final ActivatePlaneswalkerLoyaltyAbilityTriggeredAbility ability) {
         super(ability);
         this.planeswalkerSubType = ability.planeswalkerSubType;
+        this.setTargetPointer = ability.setTargetPointer;
     }
 
     @Override
@@ -49,7 +54,22 @@ public class ActivatePlaneswalkerLoyaltyAbilityTriggeredAbility extends Triggere
                 || !permanent.hasSubtype(planeswalkerSubType, game)) {
             return false;
         }
-        this.getEffects().setValue("stackObject", stackAbility);
+
+        switch (setTargetPointer) {
+            case NONE:
+                break;
+            case PLAYER:
+                getAllEffects().setTargetPointer(new FixedTarget(getControllerId(), game));
+                break;
+            case SPELL:
+                getAllEffects().setTargetPointer(new FixedTarget(event.getTargetId(), game));
+                break;
+            case PERMANENT:
+                getAllEffects().setTargetPointer(new FixedTarget(event.getSourceId(), game));
+                break;
+            default:
+                throw new UnsupportedOperationException("Unexpected setTargetPointer in ActivatePlaneswalkerLoyaltyAbilityTriggeredAbility: " + setTargetPointer);
+        }
         return true;
     }
 }

Mage/src/main/java/mage/abilities/costs/mana/ManaCostsImpl.java

@@ -418,7 +418,6 @@ public class ManaCostsImpl<T extends ManaCost> extends ArrayList<T> implements M
                     game.undo(playerId);
                     this.clearPaid();
 
-                    // TODO: checks Word of Command with Unbound Flourishing's X multiplier
                     // TODO: checks Word of Command with {X}{X} cards
                     int amount = 0;
                     List<VariableCost> variableCosts = getVariableCosts();

Mage/src/main/java/mage/abilities/effects/common/CopyStackObjectEffect.java

@@ -3,18 +3,25 @@ package mage.abilities.effects.common;
 import mage.abilities.Ability;
 import mage.abilities.effects.OneShotEffect;
 import mage.constants.Outcome;
+import mage.constants.Zone;
 import mage.game.Game;
 import mage.game.stack.StackObject;
 import mage.players.Player;
 
+import java.util.UUID;
+
 /**
  * @author TheElk801
  */
 public class CopyStackObjectEffect extends OneShotEffect {
 
     public CopyStackObjectEffect() {
+        this("that ability");
+    }
+
+    public CopyStackObjectEffect(String name) {
         super(Outcome.Copy);
-        staticText = "copy that ability. You may choose new targets for the copy";
+        staticText = "copy "+ name + ". You may choose new targets for the copy";
     }
 
     private CopyStackObjectEffect(final CopyStackObjectEffect effect) {
@@ -29,11 +36,15 @@ public class CopyStackObjectEffect extends OneShotEffect {
     @Override
     public boolean apply(Game game, Ability source) {
         Player controller = game.getPlayer(source.getControllerId());
-        StackObject ability = (StackObject) getValue("stackObject");
-        if (controller == null || ability == null) {
+        UUID id = getTargetPointer().getFirst(game, source);
+        StackObject object = game.getStack().getStackObject(id);
+        if (object == null) {
+            object = (StackObject) game.getLastKnownInformation(id, Zone.STACK);
+        }
+        if (controller == null || object == null) {
             return false;
         }
-        ability.createCopyOnStack(game, source, source.getControllerId(), true);
+        object.createCopyOnStack(game, source, source.getControllerId(), true);
         return true;
     }
 }

Mage/src/main/java/mage/abilities/meta/OrTriggeredAbility.java

@@ -72,9 +72,13 @@ public class OrTriggeredAbility extends TriggeredAbilityImpl {
     public boolean checkTrigger(GameEvent event, Game game) {
         boolean toRet = false;
         for (TriggeredAbility ability : triggeredAbilities) {
+            for (Effect e : getEffects()) { //Add effects to the sub-abilities so that they can set target pointers
+                ability.addEffect(e);
+            }
             if (ability.checkEventType(event, game) && ability.checkTrigger(event, game)) {
                 toRet = true;
             }
+            ability.getEffects().clear(); //Remove afterwards, ensures that they remain synced even with copying
         }
         return toRet;
     }

Mage/src/main/java/mage/filter/predicate/other/AbilitySourceAttachedPredicate.java

@@ -0,0 +1,30 @@
+package mage.filter.predicate.other;
+
+import mage.MageObject;
+import mage.abilities.Ability;
+import mage.filter.predicate.ObjectSourcePlayer;
+import mage.filter.predicate.ObjectSourcePlayerPredicate;
+import mage.game.Game;
+import mage.game.stack.StackAbility;
+import mage.game.stack.StackObject;
+
+/**
+ * @author notgreat
+ */
+public enum AbilitySourceAttachedPredicate implements ObjectSourcePlayerPredicate<StackObject> {
+    instance;
+
+    @Override
+    public boolean apply(ObjectSourcePlayer<StackObject> input, Game game) {
+        MageObject obj = input.getObject();
+        Ability source = input.getSource();
+
+        return obj instanceof StackAbility
+                && ((StackAbility) obj).getSourceId().equals(source.getSourcePermanentOrLKI(game).getAttachedTo());
+    }
+
+    @Override
+    public String toString() {
+        return "Attached activated/triggered";
+    }
+}

Mage/src/main/java/mage/filter/predicate/other/NotManaAbilityPredicate.java

@@ -0,0 +1,26 @@
+package mage.filter.predicate.other;
+
+import mage.abilities.Ability;
+import mage.filter.predicate.Predicate;
+import mage.game.Game;
+import mage.game.stack.StackObject;
+
+/**
+ * @author notgreat
+ */
+public enum NotManaAbilityPredicate implements Predicate<StackObject> {
+    instance;
+
+    @Override
+    public boolean apply(StackObject input, Game game) {
+        if (!(input instanceof Ability)) {
+            return false;
+        }
+        return !((Ability) input).isManaAbility();
+    }
+
+    @Override
+    public String toString() {
+        return "isn't a mana ability";
+    }
+}

Mage/src/main/java/mage/game/command/emblems/RowanKenrithEmblem.java

@@ -1,22 +1,28 @@
 package mage.game.command.emblems;
 
-import mage.abilities.TriggeredAbilityImpl;
+import mage.abilities.common.ActivateAbilityTriggeredAbility;
 import mage.abilities.effects.common.CopyStackObjectEffect;
+import mage.constants.SetTargetPointer;
 import mage.constants.Zone;
-import mage.game.Game;
+import mage.filter.FilterStackObject;
+import mage.filter.common.FilterActivatedOrTriggeredAbility;
+import mage.filter.predicate.other.NotManaAbilityPredicate;
 import mage.game.command.Emblem;
-import mage.game.events.GameEvent;
-import mage.game.stack.StackAbility;
 
 /**
  * @author TheElk801
  */
 public final class RowanKenrithEmblem extends Emblem {
     // Target player gets an emblem with "Whenever you activate an ability that isn't a mana ability, copy it. You may choose new targets for the copy."
+    private static final FilterStackObject filter = new FilterActivatedOrTriggeredAbility("an ability that isn't a mana ability");
+
+    static {
+        filter.add(NotManaAbilityPredicate.instance);
+    }
 
     public RowanKenrithEmblem() {
         super("Emblem Rowan Kenrith");
-        this.getAbilities().add(new RowanKenrithEmblemTriggeredAbility());
+        this.getAbilities().add(new ActivateAbilityTriggeredAbility(Zone.COMMAND, new CopyStackObjectEffect("it"), filter, SetTargetPointer.SPELL));
     }
 
     private RowanKenrithEmblem(final RowanKenrithEmblem card) {
@@ -28,42 +34,3 @@ public final class RowanKenrithEmblem extends Emblem {
         return new RowanKenrithEmblem(this);
     }
 }
-
-class RowanKenrithEmblemTriggeredAbility extends TriggeredAbilityImpl {
-
-    RowanKenrithEmblemTriggeredAbility() {
-        super(Zone.COMMAND, new CopyStackObjectEffect(), false);
-    }
-
-    private RowanKenrithEmblemTriggeredAbility(final RowanKenrithEmblemTriggeredAbility ability) {
-        super(ability);
-    }
-
-    @Override
-    public RowanKenrithEmblemTriggeredAbility copy() {
-        return new RowanKenrithEmblemTriggeredAbility(this);
-    }
-
-    @Override
-    public boolean checkEventType(GameEvent event, Game game) {
-        return event.getType() == GameEvent.EventType.ACTIVATED_ABILITY;
-    }
-
-    @Override
-    public boolean checkTrigger(GameEvent event, Game game) {
-        if (!event.getPlayerId().equals(getControllerId())) {
-            return false;
-        }
-        StackAbility stackAbility = (StackAbility) game.getStack().getStackObject(event.getSourceId());
-        if (stackAbility == null || stackAbility.getStackAbility().isManaActivatedAbility()) {
-            return false;
-        }
-        this.getEffects().setValue("stackObject", stackAbility);
-        return true;
-    }
-
-    @Override
-    public String getRule() {
-        return "Whenever you activate an ability that isn't a mana ability, copy it. You may choose new targets for the copy.";
-    }
-}

Mage/src/main/java/mage/game/events/GameEvent.java

@@ -219,13 +219,6 @@ public class GameEvent implements Serializable {
          sourceId    sourceId of the mount
          playerId    the id of the controlling player
          */
-        X_MANA_ANNOUNCE,
-        /* X_MANA_ANNOUNCE
-         mana x-costs announced by players (X value can be changed by replace events like Unbound Flourishing)
-         targetId    id of the spell that's cast
-         playerId    player that casts the spell or ability
-         amount      X multiplier to change X value, default 1
-         */
         CAST_SPELL,
         CAST_SPELL_LATE,
         /* SPELL_CAST, CAST_SPELL_LATE

Mage/src/main/java/mage/game/stack/SpellStack.java

@@ -1,18 +1,19 @@
 
 package mage.game.stack;
 
-import java.util.ArrayDeque;
-import java.util.Date;
-import java.util.UUID;
-
 import mage.MageObject;
 import mage.abilities.Ability;
 import mage.constants.PutCards;
+import mage.constants.Zone;
 import mage.game.Game;
 import mage.game.events.GameEvent;
 import mage.util.CardUtil;
 import org.apache.log4j.Logger;
 
+import java.util.ArrayDeque;
+import java.util.Date;
+import java.util.UUID;
+
 /**
  * @author BetaSteward_at_googlemail.com
  */
@@ -82,6 +83,7 @@ public class SpellStack extends ArrayDeque<StackObject> {
             if (!game.replaceEvent(GameEvent.getEvent(GameEvent.EventType.COUNTER, objectId, source, stackObject.getControllerId()))) {
                 if (!(stackObject instanceof Spell)) { // spells are removed from stack by the card movement
                     this.remove(stackObject, game);
+                    game.rememberLKI(Zone.STACK, stackObject);
                 }
                 stackObject.counter(source, game, putCard);
                 if (!game.isSimulation()) {

Mage/src/main/java/mage/players/Player.java

@@ -731,11 +731,7 @@ public interface Player extends MageItem, Copyable<Player> {
     boolean shuffleCardsToLibrary(Card card, Game game, Ability source);
 
     // set the value for X mana spells and abilities
-    default int announceXMana(int min, int max, String message, Game game, Ability ability) {
-        return announceXMana(min, max, 1, message, game, ability);
-    }
-
-    int announceXMana(int min, int max, int multiplier, String message, Game game, Ability ability);
+    int announceXMana(int min, int max, String message, Game game, Ability ability);
 
     // set the value for non mana X costs
     int announceXCost(int min, int max, String message, Game game, Ability ability, VariableCost variableCost);

Mage/src/main/java/mage/players/StubPlayer.java

@@ -156,7 +156,7 @@ public class StubPlayer extends PlayerImpl {
     }
 
     @Override
-    public int announceXMana(int min, int max, int multiplier, String message, Game game, Ability ability) {
+    public int announceXMana(int min, int max, String message, Game game, Ability ability) {
         return 0;
     }