multiple changes:

* refactor: improved target pointer init code and logic, added docs and runtime checks;
* game: fixed miss or wrong init calls in some continuous effects;
* game: fixed wrong usage of target pointers (miss copy code, miss npe checks);

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

@@ -15,11 +15,7 @@ import java.util.stream.Collectors;
 
 public class EachTargetPointer extends TargetPointerImpl {
 
-    private Map<UUID, Integer> zoneChangeCounter = new HashMap<>();
-
-    public static EachTargetPointer getInstance() {
-        return new EachTargetPointer();
-    }
+    private final Map<UUID, Integer> zoneChangeCounter = new HashMap<>();
 
     public EachTargetPointer() {
         super();
@@ -27,15 +23,16 @@ public class EachTargetPointer extends TargetPointerImpl {
 
     protected EachTargetPointer(final EachTargetPointer targetPointer) {
         super(targetPointer);
-
-        this.zoneChangeCounter = new HashMap<>();
-        for (Map.Entry<UUID, Integer> entry : targetPointer.zoneChangeCounter.entrySet()) {
-            this.zoneChangeCounter.put(entry.getKey(), entry.getValue());
-        }
+        this.zoneChangeCounter.putAll(targetPointer.zoneChangeCounter);
     }
 
     @Override
     public void init(Game game, Ability source) {
+        if (isInitialized()) {
+            return;
+        }
+        this.setInitialized();
+
         if (!source.getTargets().isEmpty()) {
             for (UUID target : source
                     .getTargets()
@@ -99,17 +96,6 @@ public class EachTargetPointer extends TargetPointerImpl {
         return new EachTargetPointer(this);
     }
 
-    @Override
-    public FixedTarget getFixedTarget(Game game, Ability source) {
-        this.init(game, source);
-        UUID firstId = getFirst(game, source);
-        if (firstId != null) {
-            return new FixedTarget(firstId, game.getState().getZoneChangeCounter(firstId));
-        }
-        return null;
-
-    }
-
     @Override
     public Permanent getFirstTargetPermanentOrLKI(Game game, Ability source) {
         UUID targetId = source.getFirstTarget();

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

@@ -16,10 +16,11 @@ import java.util.UUID;
 public class FixedTarget extends TargetPointerImpl {
 
     private final UUID targetId;
-    private int zoneChangeCounter;
-    private boolean initialized;
+    private int zoneChangeCounter = 0;
 
     /**
+     * Dynamic ZCC (not recommended)
+     * <p>
      * Use this best only to target to a player or spells on the stack. Try to
      * avoid this method to set the target to a specific card or permanent if
      * possible. Because the zoneChangeCounter is not set immediately, it can be
@@ -32,7 +33,6 @@ public class FixedTarget extends TargetPointerImpl {
     public FixedTarget(UUID target) {
         super();
         this.targetId = target;
-        this.initialized = false;
     }
 
     public FixedTarget(MageObjectReference mor) {
@@ -40,7 +40,9 @@ public class FixedTarget extends TargetPointerImpl {
     }
 
     /**
-     * Target counter is immediatly initialised with current zoneChangeCounter
+     * Static ZCC
+     * <p>
+     * Target counter is immediately initialised with current zoneChangeCounter
      * value from the GameState Sets fixed the currect zoneChangeCounter
      *
      * @param card used to get the objectId
@@ -50,10 +52,13 @@ public class FixedTarget extends TargetPointerImpl {
         super();
         this.targetId = card.getId();
         this.zoneChangeCounter = card.getZoneChangeCounter(game);
-        this.initialized = true;
+
+        this.setInitialized(); // no need dynamic init
     }
 
     /**
+     * Static ZCC
+     * <p>
      * Target counter is immediately initialized with current zoneChangeCounter
      * value from the given permanent
      *
@@ -65,6 +70,8 @@ public class FixedTarget extends TargetPointerImpl {
     }
 
     /**
+     * Static ZCC
+     * <p>
      * Use this if you already want to fix the target object to the known zone
      * now (otherwise the zone will be set if the ability triggers or not at
      * all) If not initialized, the object of the current zone then will be
@@ -76,11 +83,14 @@ public class FixedTarget extends TargetPointerImpl {
     public FixedTarget(UUID targetId, int zoneChangeCounter) {
         super();
         this.targetId = targetId;
-        this.initialized = true;
         this.zoneChangeCounter = zoneChangeCounter;
+
+        this.setInitialized(); // no need dynamic init
     }
 
     /**
+     * Static ZCC
+     * <p>
      * Use this to set the target to exactly the zone the target is currently in
      *
      * @param targetId
@@ -89,8 +99,9 @@ public class FixedTarget extends TargetPointerImpl {
     public FixedTarget(UUID targetId, Game game) {
         super();
         this.targetId = targetId;
-        this.initialized = true;
         this.zoneChangeCounter = game.getState().getZoneChangeCounter(targetId);
+
+        this.setInitialized(); // no need dynamic init
     }
 
     protected FixedTarget(final FixedTarget targetPointer) {
@@ -98,15 +109,16 @@ public class FixedTarget extends TargetPointerImpl {
 
         this.targetId = targetPointer.targetId;
         this.zoneChangeCounter = targetPointer.zoneChangeCounter;
-        this.initialized = targetPointer.initialized;
     }
 
     @Override
     public void init(Game game, Ability source) {
-        if (!initialized) {
-            initialized = true;
-            this.zoneChangeCounter = game.getState().getZoneChangeCounter(targetId);
+        if (isInitialized()) {
+            return;
         }
+        setInitialized();
+
+        this.zoneChangeCounter = game.getState().getZoneChangeCounter(targetId);
     }
 
     /**
@@ -161,12 +173,6 @@ public class FixedTarget extends TargetPointerImpl {
         return zoneChangeCounter;
     }
 
-    @Override
-    public FixedTarget getFixedTarget(Game game, Ability source) {
-        init(game, source);
-        return this;
-    }
-
     @Override
     public Permanent getFirstTargetPermanentOrLKI(Game game, Ability source) {
         init(game, source);

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

@@ -14,80 +14,61 @@ import java.util.*;
 import java.util.stream.Collectors;
 
 /**
+ * Targets list with static ZCC
+ *
  * @author LevelX2
  */
 public class FixedTargets extends TargetPointerImpl {
 
     final ArrayList<MageObjectReference> targets = new ArrayList<>();
-    final ArrayList<UUID> targetsNotInitialized = new ArrayList<>();
 
-    private boolean initialized;
-
-    public FixedTargets(UUID targetId) {
-        super();
-
-        targetsNotInitialized.add(targetId);
-        this.initialized = false;
+    public FixedTargets(List<Permanent> objects, Game game) {
+        this(objects
+                .stream()
+                .map(o -> new MageObjectReference(o.getId(), game))
+                .collect(Collectors.toList()));
     }
 
-    public FixedTargets(Cards cards, Game game) {
-        super();
-        if (cards != null) {
-            for (UUID targetId : cards) {
-                MageObjectReference mor = new MageObjectReference(targetId, game);
-                targets.add(mor);
-            }
-        }
-        this.initialized = true;
+    public FixedTargets(Set<Card> objects, Game game) {
+        this(objects
+                .stream()
+                .map(o -> new MageObjectReference(o.getId(), game))
+                .collect(Collectors.toList()));
+    }
+
+    public FixedTargets(Cards objects, Game game) {
+        this(objects.getCards(game)
+                .stream()
+                .map(o -> new MageObjectReference(o.getId(), game))
+                .collect(Collectors.toList()));
     }
 
     public FixedTargets(Token token, Game game) {
-        this(token.getLastAddedTokenIds().stream().map(game::getPermanent).collect(Collectors.toList()), game);
+        this(token.getLastAddedTokenIds()
+                .stream()
+                .map(game::getPermanent)
+                .collect(Collectors.toList()), game);
     }
 
-    public FixedTargets(List<Permanent> permanents, Game game) {
+    public FixedTargets(List<MageObjectReference> morList) {
         super();
-
-        for (Permanent permanent : permanents) {
-            MageObjectReference mor = new MageObjectReference(permanent.getId(), permanent.getZoneChangeCounter(game), game);
-            targets.add(mor);
-        }
-        this.initialized = true;
+        targets.addAll(morList);
+        this.setInitialized(); // no need dynamic init
     }
 
-    public FixedTargets(Set<Card> cards, Game game) {
-        super();
-
-        for (Card card : cards) {
-            MageObjectReference mor = new MageObjectReference(card.getId(), card.getZoneChangeCounter(game), game);
-            targets.add(mor);
-        }
-        this.initialized = true;
-    }
-
-    public FixedTargets(Collection<MageObjectReference> morSet, Game game) {
-        super();
-
-        targets.addAll(morSet);
-        this.initialized = true;
-    }
-
-    private FixedTargets(final FixedTargets targetPointer) {
-        super(targetPointer);
-
-        this.targets.addAll(targetPointer.targets);
-        this.targetsNotInitialized.addAll(targetPointer.targetsNotInitialized);
-        this.initialized = targetPointer.initialized;
+    private FixedTargets(final FixedTargets pointer) {
+        super(pointer);
+        this.targets.addAll(pointer.targets);
     }
 
     @Override
     public void init(Game game, Ability source) {
-        if (!initialized) {
-            initialized = true;
-            for (UUID targetId : targetsNotInitialized) {
-                targets.add(new MageObjectReference(targetId, game.getState().getZoneChangeCounter(targetId), game));
-            }
+        if (isInitialized()) {
+            return;
         }
+
+        // impossible use case
+        throw new IllegalArgumentException("Wrong code usage: FixedTargets support only static ZCC, you can't get here");
     }
 
     @Override
@@ -118,23 +99,6 @@ public class FixedTargets extends TargetPointerImpl {
         return new FixedTargets(this);
     }
 
-    /**
-     * Returns a fixed target for (and only) the first taget
-     *
-     * @param game
-     * @param source
-     * @return
-     */
-    @Override
-    public FixedTarget getFixedTarget(Game game, Ability source) {
-        this.init(game, source);
-        UUID firstId = getFirst(game, source);
-        if (firstId != null) {
-            return new FixedTarget(firstId, game.getState().getZoneChangeCounter(firstId));
-        }
-        return null;
-    }
-
     @Override
     public Permanent getFirstTargetPermanentOrLKI(Game game, Ability source) {
         UUID targetId = null;
@@ -143,8 +107,6 @@ public class FixedTargets extends TargetPointerImpl {
             MageObjectReference mor = targets.get(0);
             targetId = mor.getSourceId();
             zoneChangeCounter = mor.getZoneChangeCounter();
-        } else if (!targetsNotInitialized.isEmpty()) {
-            targetId = targetsNotInitialized.get(0);
         }
         if (targetId != null) {
             Permanent permanent = game.getPermanent(targetId);

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

@@ -1,6 +1,6 @@
 package mage.target.targetpointer;
 
-import mage.MageObject;
+import mage.MageObjectReference;
 import mage.abilities.Ability;
 import mage.cards.Card;
 import mage.constants.Zone;
@@ -15,154 +15,148 @@ import java.util.*;
  */
 public abstract class NthTargetPointer extends TargetPointerImpl {
 
-    private static final Map<UUID, Integer> emptyZoneChangeCounter = Collections.unmodifiableMap(new HashMap<>(0));
     private static final List<UUID> emptyTargets = Collections.unmodifiableList(new ArrayList<>(0));
 
-    private Map<UUID, Integer> zoneChangeCounter;
-    private final int targetNumber;
+    // TODO: rework to list of MageObjectReference instead zcc
+    private final Map<UUID, Integer> zoneChangeCounter = new HashMap<>();
+    private final int targetIndex; // zero-based target numbers (1 -> 0, 2 -> 1, 3 -> 2, etc)
 
     public NthTargetPointer(int targetNumber) {
         super();
-        this.targetNumber = targetNumber;
+        this.targetIndex = targetNumber - 1;
     }
 
     protected NthTargetPointer(final NthTargetPointer nthTargetPointer) {
         super(nthTargetPointer);
-        this.targetNumber = nthTargetPointer.targetNumber;
-
-        if (nthTargetPointer.zoneChangeCounter != null) {
-            this.zoneChangeCounter = new HashMap<>(nthTargetPointer.zoneChangeCounter.size());
-            for (Map.Entry<UUID, Integer> entry : nthTargetPointer.zoneChangeCounter.entrySet()) {
-                addToZoneChangeCounter(entry.getKey(), entry.getValue());
-            }
-        }
+        this.targetIndex = nthTargetPointer.targetIndex;
+        this.zoneChangeCounter.putAll(nthTargetPointer.zoneChangeCounter);
     }
 
     @Override
     public void init(Game game, Ability source) {
-        if (source.getTargets().size() < targetNumber) {
+        if (isInitialized()) {
+            return;
+        }
+        this.setInitialized();
+
+        if (source.getTargets().size() <= this.targetIndex) {
+            wrongTargetsUsage(source);
             return;
         }
 
-        for (UUID target : source.getTargets().get(targetIndex()).getTargets()) {
+        for (UUID target : source.getTargets().get(this.targetIndex).getTargets()) {
             Card card = game.getCard(target);
             if (card != null) {
-                addToZoneChangeCounter(target, card.getZoneChangeCounter(game));
+                this.zoneChangeCounter.put(target, card.getZoneChangeCounter(game));
             }
         }
     }
 
+    private void wrongTargetsUsage(Ability source) {
+        if (this.targetIndex > 0) {
+            // first target pointer is default, so must be ignored
+            throw new IllegalStateException("Wrong code usage: source ability miss targets setup for target pointer - "
+                    + this.getClass().getSimpleName() + " - " + source.getClass().getSimpleName() + " - " + source);
+        }
+    }
+
     @Override
     public List<UUID> getTargets(Game game, Ability source) {
-        if (source.getTargets().size() < targetNumber) {
+        // can be used before effect's init (example: checking spell targets on stack before resolve like HeroicAbility)
+
+        if (source.getTargets().size() <= this.targetIndex) {
+            wrongTargetsUsage(source);
             return emptyTargets;
         }
 
-        List<UUID> targetIds = source.getTargets().get(targetIndex()).getTargets();
-        List<UUID> finalTargetIds = new ArrayList<>(targetIds.size());
-
-        for (UUID targetId : targetIds) {
-            Card card = game.getCard(targetId);
-            if (card != null
-                    && getZoneChangeCounter().containsKey(targetId)
-            && card.getZoneChangeCounter(game) != getZoneChangeCounter().get(targetId)) {
-                // But no longer if new permanent is already on the battlefield
-                Permanent permanent = game.getPermanentOrLKIBattlefield(targetId);
-                if (permanent == null || permanent.getZoneChangeCounter(game) != getZoneChangeCounter().get(targetId)) {
-                    continue;
-                }
+        List<UUID> res = new ArrayList<>();
+        for (UUID targetId : source.getTargets().get(this.targetIndex).getTargets()) {
+            if (!isOutdatedTarget(game, targetId)) {
+                res.add(targetId);
             }
-
-            finalTargetIds.add(targetId);
         }
-        return finalTargetIds;
+        return res;
+    }
+
+    private boolean isOutdatedTarget(Game game, UUID targetId) {
+        int needZcc = this.zoneChangeCounter.getOrDefault(targetId, 0);
+        if (needZcc == 0) {
+            // any zcc (target not init yet here)
+            return false;
+        }
+
+        // card
+        Card card = game.getCard(targetId);
+        if (card != null && card.getZoneChangeCounter(game) == needZcc) {
+            return false;
+        }
+
+        // permanent
+        Permanent permanent = game.getPermanentOrLKIBattlefield(targetId);
+        if (permanent != null && permanent.getZoneChangeCounter(game) == needZcc) {
+            return false;
+        }
+
+        // TODO: if no bug reports with die triggers and new code then remove it, 2024-02-18
+        //  if you catch bugs then add code like if permanent.getZoneChangeCounter(game) == needZcc + 1 then return false
+        // old comments:
+        // Because if dies trigger has to trigger as permanent has already moved zone, we have to check if target
+        // was on the battlefield immed. before, but no longer if new permanent is already on the battlefield
+
+        // outdated
+        return true;
     }
 
     @Override
     public UUID getFirst(Game game, Ability source) {
-        if (source.getTargets().size() < targetNumber) {
+        if (source.getTargets().size() <= this.targetIndex) {
+            wrongTargetsUsage(source);
             return null;
         }
 
-        UUID targetId = source.getTargets().get(targetIndex()).getFirstTarget();
-        if (getZoneChangeCounter().containsKey(targetId)) {
-            Card card = game.getCard(targetId);
-            if (card != null && getZoneChangeCounter().containsKey(targetId)
-                    && card.getZoneChangeCounter(game) != getZoneChangeCounter().get(targetId)) {
-
-                // Because if dies trigger has to trigger as permanent has already moved zone, we have to check if target was on the battlefield immed. before
-                // but no longer if new permanent is already on the battlefield
-                Permanent permanent = game.getPermanentOrLKIBattlefield(targetId);
-                if (permanent == null || permanent.getZoneChangeCounter(game) != zoneChangeCounter.get(targetId)) {
-                    return null;
-                }
-            }
+        UUID targetId = source.getTargets().get(this.targetIndex).getFirstTarget();
+        if (isOutdatedTarget(game, targetId)) {
+            return null;
         }
+
         return targetId;
-
-    }
-
-    @Override
-    public FixedTarget getFixedTarget(Game game, Ability source) {
-        this.init(game, source);
-        UUID firstId = getFirst(game, source);
-        if (firstId != null) {
-            return new FixedTarget(firstId, game.getState().getZoneChangeCounter(firstId));
-        }
-
-        return null;
     }
 
     @Override
     public Permanent getFirstTargetPermanentOrLKI(Game game, Ability source) {
-        if (source.getTargets().size() < targetNumber) {
+        if (source.getTargets().size() < this.targetIndex) {
+            wrongTargetsUsage(source);
             return null;
         }
+        UUID targetId = source.getTargets().get(this.targetIndex).getFirstTarget();
 
-        Permanent permanent;
-        UUID targetId = source.getTargets().get(targetIndex()).getFirstTarget();
-
-        if (getZoneChangeCounter().containsKey(targetId)) {
-            permanent = game.getPermanent(targetId);
-            if (permanent != null && permanent.getZoneChangeCounter(game) == getZoneChangeCounter().get(targetId)) {
-                return permanent;
-            }
-            MageObject mageObject = game.getLastKnownInformation(targetId, Zone.BATTLEFIELD, getZoneChangeCounter().get(targetId));
-            if (mageObject instanceof Permanent) {
-                return (Permanent) mageObject;
-            }
-
+        if (this.zoneChangeCounter.containsKey(targetId)) {
+            // need static zcc
+            MageObjectReference needRef = new MageObjectReference(targetId, this.zoneChangeCounter.getOrDefault(targetId, 0), game);
+            return game.getPermanentOrLKIBattlefield(needRef);
         } else {
-            permanent = game.getPermanent(targetId);
+            // need any zcc
+            // TODO: must research, is it used at all?! Init code must fill all static zcc data before go here
+            Permanent permanent = game.getPermanent(targetId);
             if (permanent == null) {
                 permanent = (Permanent) game.getLastKnownInformation(targetId, Zone.BATTLEFIELD);
             }
+            return permanent;
         }
-        return permanent;
     }
 
     @Override
     public String describeTargets(Targets targets, String defaultDescription) {
-        return targets.size() < targetNumber ? defaultDescription : targets.get(targetIndex()).getDescription();
+        if (targets.size() <= this.targetIndex) {
+            // TODO: need research, is it used for non setup targets ?!
+            return defaultDescription;
+        } else {
+            return targets.get(this.targetIndex).getDescription();
+        }
     }
 
     @Override
     public boolean isPlural(Targets targets) {
-        return targets.size() > targetIndex() && targets.get(targetIndex()).getMaxNumberOfTargets() > 1;
-    }
-
-    private int targetIndex() {
-        return targetNumber - 1;
-    }
-
-    private Map<UUID, Integer> getZoneChangeCounter() {
-        return zoneChangeCounter != null ? zoneChangeCounter : emptyZoneChangeCounter;
-    }
-
-    private void addToZoneChangeCounter(UUID key, Integer value) {
-        if (zoneChangeCounter == null) {
-            zoneChangeCounter = new HashMap<>();
-        }
-        getZoneChangeCounter().put(key, value);
+        return targets.size() > this.targetIndex && targets.get(this.targetIndex).getMaxNumberOfTargets() > 1;
     }
 }

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

@@ -12,15 +12,30 @@ import java.util.UUID;
 
 public interface TargetPointer extends Serializable, Copyable<TargetPointer> {
 
+    /**
+     * Init dynamic targets (must save current targets zcc to fizzle it later on outdated targets)
+     * - one shot effects: no needs to init
+     * - continues effects: must use init logic
+     */
     void init(Game game, Ability source);
 
+    boolean isInitialized();
+
+    void setInitialized();
+
     List<UUID> getTargets(Game game, Ability source);
 
+    /**
+     * Return first actual target id (null on outdated targets)
+     */
     UUID getFirst(Game game, Ability source);
 
-    TargetPointer copy();
+    /**
+     * Return first actual target data (null on outdated targets)
+     */
+    FixedTarget getFirstAsFixedTarget(Game game, Ability source);
 
-    FixedTarget getFixedTarget(Game game, Ability source);
+    TargetPointer copy();
 
     /**
      * Retrieves the permanent according the first targetId and

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

@@ -1,7 +1,11 @@
 package mage.target.targetpointer;
 
+import mage.abilities.Ability;
+import mage.game.Game;
+
 import java.util.HashMap;
 import java.util.Map;
+import java.util.UUID;
 
 /**
  * @author JayDi85
@@ -11,6 +15,8 @@ public abstract class TargetPointerImpl implements TargetPointer {
     // Store custom data here. Use it to keep unique values for ability instances on stack (example: Gruul Ragebeast)
     private Map<String, String> data;
 
+    private boolean initialized = false;
+
     public TargetPointerImpl() {
         super();
     }
@@ -21,6 +27,17 @@ public abstract class TargetPointerImpl implements TargetPointer {
             this.data = new HashMap<>();
             this.data.putAll(targetPointer.data);
         }
+        this.initialized = targetPointer.initialized;
+    }
+
+    @Override
+    public boolean isInitialized() {
+        return this.initialized;
+    }
+
+    @Override
+    public void setInitialized() {
+        this.initialized = true;
     }
 
     @Override
@@ -39,4 +56,14 @@ public abstract class TargetPointerImpl implements TargetPointer {
         data.put(key, value);
         return this;
     }
+
+    @Override
+    public final FixedTarget getFirstAsFixedTarget(Game game, Ability source) {
+        UUID firstId = this.getFirst(game, source);
+        if (firstId != null) {
+            return new FixedTarget(firstId, game.getState().getZoneChangeCounter(firstId));
+        }
+
+        return null;
+    }
 }