Effect withTargetDescription, other fixes for discrepencies with "target" text with reference (#13867)

* Add withTargetDescription support to all Effects

* use withTargetDescription for effects incorrectly with "target" in text

* CommanderStormAbility add support for non-target rules text

* Heliods Emissary use OrTriggeredAbility

* support all effects withTargetDescription, rename overwriteTargetDescription to targetDescription

* copy target description from old pointer when using setTargetPointer

* Text cleanup/simplifications

* Remove outdated warning

Mage/src/main/java/mage/abilities/decorator/ConditionalOneShotEffect.java

@@ -132,6 +132,13 @@ public class ConditionalOneShotEffect extends OneShotEffect {
         return this;
     }
 
+    @Override
+    public ConditionalOneShotEffect withTargetDescription(String target) {
+        effects.forEach(effect -> effect.withTargetDescription(target));
+        otherwiseEffects.forEach(effect -> effect.withTargetDescription(target));
+        return this;
+    }
+
     @Override
     public Condition getCondition() {
         return condition;

Mage/src/main/java/mage/abilities/effects/ContinuousEffectImpl.java

@@ -448,6 +448,12 @@ public abstract class ContinuousEffectImpl extends EffectImpl implements Continu
         return this;
     }
 
+    @Override
+    public ContinuousEffect withTargetDescription(String target) {
+        super.withTargetDescription(target);
+        return this;
+    }
+
     /**
      * Auto-generates dependencies on different effects (what's apply first and
      * what's apply second)

Mage/src/main/java/mage/abilities/effects/Effect.java

@@ -59,6 +59,11 @@ public interface Effect extends Serializable, Copyable<Effect> {
 
     TargetPointer getTargetPointer();
 
+    /**
+     * Sets the target pointer's description to the given string.
+     */
+    Effect withTargetDescription(String target);
+
     void setValue(String key, Object value);
 
     Object getValue(String key);

Mage/src/main/java/mage/abilities/effects/EffectImpl.java

@@ -93,7 +93,7 @@ public abstract class EffectImpl implements Effect {
             // first target pointer is default
             throw new IllegalArgumentException("Wrong code usage: target pointer can't be set to null: " + this);
         }
-
+        targetPointer.setTargetDescription(this.targetPointer.getTargetDescription()); // copies the null if not set
         this.targetPointer = targetPointer;
         initNewTargetPointer();
         return this;
@@ -104,6 +104,12 @@ public abstract class EffectImpl implements Effect {
         return this.targetPointer;
     }
 
+    @Override
+    public Effect withTargetDescription(String target) {
+        this.targetPointer.setTargetDescription(target);
+        return this;
+    }
+
     @Override
     public void newId() {
         if (!(this instanceof MageSingleton)) {

Mage/src/main/java/mage/abilities/effects/OneShotEffect.java

@@ -42,6 +42,12 @@ public abstract class OneShotEffect extends EffectImpl {
         return this;
     }
 
+    @Override
+    public OneShotEffect withTargetDescription(String target) {
+        super.withTargetDescription(target);
+        return this;
+    }
+
     @Override
     abstract public OneShotEffect copy();
 }

Mage/src/main/java/mage/abilities/effects/OneShotNonTargetEffect.java

@@ -72,6 +72,12 @@ public class OneShotNonTargetEffect extends OneShotEffect {
         return super.setTargetPointer(targetPointer);
     }
 
+    @Override
+    public OneShotEffect withTargetDescription(String target) {
+        effect.withTargetDescription(target);
+        return super.withTargetDescription(target);
+    }
+
     @Override
     public void setValue(String key, Object value) {
         effect.setValue(key, value);

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

@@ -96,4 +96,11 @@ public class CreateDelayedTriggeredAbilityEffect extends OneShotEffect {
         this.copyToPointer = copyToPointer;
         return this;
     }
+
+    @Override
+    public CreateDelayedTriggeredAbilityEffect withTargetDescription(String target) {
+        ability.getEffects().forEach(effect -> effect.withTargetDescription(target));
+        super.withTargetDescription(target);
+        return this;
+    }
 }

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

@@ -160,4 +160,13 @@ public class RollDieWithResultTableEffect extends OneShotEffect {
         super.setTargetPointer(targetPointer);
         return this;
     }
+
+
+    @Override
+    public RollDieWithResultTableEffect withTargetDescription(String target) {
+        resultsTable.forEach(tableEntry -> tableEntry.effects.forEach(
+                effect -> effect.withTargetDescription(target)));
+        super.withTargetDescription(target);
+        return this;
+    }
 }

Mage/src/main/java/mage/abilities/keyword/CommanderStormAbility.java

@@ -20,8 +20,11 @@ import mage.watchers.common.CommanderPlaysCountWatcher;
  */
 public class CommanderStormAbility extends TriggeredAbilityImpl {
 
-    public CommanderStormAbility() {
-        super(Zone.STACK, new CommanderStormEffect());
+    public CommanderStormAbility(boolean newTargetsText) {
+        super(Zone.STACK, new CommanderStormEffect().setText("copy it for each time you've "
+                + "cast your commander from the command zone this game."
+                + (newTargetsText ? " You may choose new targets for the copies." : "")));
+        this.setTriggerPhrase("When you cast this spell, ");
         this.setRuleAtTheTop(true);
     }
 
@@ -54,13 +57,6 @@ public class CommanderStormAbility extends TriggeredAbilityImpl {
         }
         return true;
     }
-
-    @Override
-    public String getRule() {
-        return "When you cast this spell, copy it for each time you've "
-                + "cast your commander from the command zone this game. "
-                + "You may choose new targets for the copies.";
-    }
 }
 
 class CommanderStormEffect extends OneShotEffect {

Mage/src/main/java/mage/constants/ModeChoice.java

@@ -18,7 +18,7 @@ public enum ModeChoice {
     SULTAI("Sultai"),
 
     MIRRAN("Mirran"),
-    PHYREXIAN("Phyrexian "),
+    PHYREXIAN("Phyrexian"),
 
     ODD("odd"),
     EVEN("even"),

Mage/src/main/java/mage/target/targetpointer/EachTargetPointer.java

@@ -120,6 +120,7 @@ public class EachTargetPointer extends TargetPointerImpl {
 
     @Override
     public String describeTargets(Targets targets, String defaultDescription) {
+        if (targetDescription != null) return targetDescription;
         if (targets.isEmpty()) {
             return defaultDescription;
         }

Mage/src/main/java/mage/target/targetpointer/NthTargetPointer.java

@@ -148,6 +148,7 @@ public abstract class NthTargetPointer extends TargetPointerImpl {
 
     @Override
     public String describeTargets(Targets targets, String defaultDescription) {
+        if (targetDescription != null) return targetDescription;
         if (targets.size() <= this.targetIndex) {
             // TODO: need research, is it used for non setup targets ?!
             // Typical usage example: trigger sets fixed target pointer

Mage/src/main/java/mage/target/targetpointer/TargetPointer.java

@@ -57,9 +57,13 @@ public interface TargetPointer extends Serializable, Copyable<TargetPointer> {
     /**
      * Describes the appropriate subset of targets for ability text.
      */
-    default String describeTargets(Targets targets, String defaultDescription) {
-        return defaultDescription;
-    }
+    String describeTargets(Targets targets, String defaultDescription);
+
+    /**
+     * Overwrite the default target description
+     */
+    void setTargetDescription(String description);
+    String getTargetDescription();
 
     default boolean isPlural(Targets targets) {
         return false;

Mage/src/main/java/mage/target/targetpointer/TargetPointerImpl.java

@@ -5,6 +5,7 @@ import mage.game.Game;
 import mage.game.permanent.Permanent;
 import mage.game.stack.Spell;
 import mage.players.Player;
+import mage.target.Targets;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -18,6 +19,7 @@ public abstract class TargetPointerImpl implements TargetPointer {
     private Map<String, String> data;
 
     private boolean initialized = false;
+    protected String targetDescription = null;
 
     protected TargetPointerImpl() {
         super();
@@ -30,6 +32,7 @@ public abstract class TargetPointerImpl implements TargetPointer {
             this.data.putAll(targetPointer.data);
         }
         this.initialized = targetPointer.initialized;
+        this.targetDescription = targetPointer.targetDescription;
     }
 
     @Override
@@ -74,4 +77,18 @@ public abstract class TargetPointerImpl implements TargetPointer {
         return this;
     }
 
+    @Override
+    public String describeTargets(Targets targets, String defaultDescription) {
+        return targetDescription != null ? targetDescription : defaultDescription;
+    }
+
+    @Override
+    public void setTargetDescription(String description) {
+        targetDescription = description;
+    }
+    @Override
+    public String getTargetDescription() {
+        return targetDescription;
+    }
+
 }