* Delay triggered abilities - fixed that copied spell with multiple delayed triggered abilities can freeze the game or missing from stack (#5437);

Mage.Common/src/main/java/mage/view/CardsView.java

@@ -13,14 +13,18 @@ import mage.game.permanent.Permanent;
 import mage.game.permanent.PermanentToken;
 import mage.target.targetpointer.TargetPointer;
 import mage.util.GameLog;
+import org.apache.log4j.Logger;
 
 import java.util.*;
+import java.util.stream.Collectors;
 
 /**
  * @author BetaSteward_at_googlemail.com
  */
 public class CardsView extends LinkedHashMap<UUID, CardView> {
 
+    private static final Logger LOGGER = Logger.getLogger(CardsView.class);
+
     public CardsView() {
     }
 
@@ -151,6 +155,11 @@ public class CardsView extends LinkedHashMap<UUID, CardView> {
                 this.put(ability.getId(), abilityView);
             }
         }
+
+        if (this.size() != abilities.size()) {
+            LOGGER.error("Can't translate abilities list to cards view (need " + abilities.size() + ", but get " + this.size() + "). Abilities:\n"
+                    + abilities.stream().map(a -> a.getClass().getSimpleName() + " - " + a.getRule()).collect(Collectors.joining("\n")));
+        }
     }
 
     public CardsView(Collection<? extends Ability> abilities, GameState state) {

Mage.Common/src/main/java/mage/view/GameView.java

@@ -2,11 +2,6 @@ package mage.view;
 
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
-import java.io.Serializable;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
 import mage.MageObject;
 import mage.abilities.costs.Cost;
 import mage.cards.Card;
@@ -30,6 +25,12 @@ import mage.players.Player;
 import mage.watchers.common.CastSpellLastTurnWatcher;
 import org.apache.log4j.Logger;
 
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
 /**
  * @author BetaSteward_at_googlemail.com
  */
@@ -124,7 +125,6 @@ public class GameView implements Serializable {
                         } else {
                             LOGGER.fatal("Designation object not found: " + object.getName() + ' ' + object.toString() + ' ' + object.getClass().toString());
                         }
-
                     } else if (object instanceof StackAbility) {
                         StackAbility stackAbility = ((StackAbility) object);
                         stackAbility.newId();

Mage/src/main/java/mage/game/GameImpl.java

@@ -1,9 +1,5 @@
 package mage.game;
 
-import java.io.IOException;
-import java.io.Serializable;
-import java.util.*;
-import java.util.Map.Entry;
 import mage.MageException;
 import mage.MageObject;
 import mage.abilities.*;
@@ -68,6 +64,11 @@ import mage.util.functions.ApplyToPermanent;
 import mage.watchers.common.*;
 import org.apache.log4j.Logger;
 
+import java.io.IOException;
+import java.io.Serializable;
+import java.util.*;
+import java.util.Map.Entry;
+
 public abstract class GameImpl implements Game, Serializable {
 
     private static final int ROLLBACK_TURNS_MAX = 4;
@@ -515,6 +516,13 @@ public abstract class GameImpl implements Game, Serializable {
         if (card == null) {
             card = this.getMeldCard(cardId);
         }
+
+        // copied cards removes, but delayed triggered possible from it, see https://github.com/magefree/mage/issues/5437
+        // TODO: remove that workround after LKI rework, see GameState.copyCard
+        if (card == null) {
+            card = (Card) state.getValue(GameState.COPIED_FROM_CARD_KEY + cardId.toString());
+        }
+
         return card;
     }
 
@@ -1502,7 +1510,7 @@ public abstract class GameImpl implements Game, Serializable {
     /**
      * @param emblem
      * @param sourceObject
-     * @param toPlayerId controller and owner of the emblem
+     * @param toPlayerId   controller and owner of the emblem
      */
     @Override
     public void addEmblem(Emblem emblem, MageObject sourceObject, UUID toPlayerId) {
@@ -1520,8 +1528,8 @@ public abstract class GameImpl implements Game, Serializable {
     /**
      * @param plane
      * @param sourceObject
-     * @param toPlayerId controller and owner of the plane (may only be one per
+     * @param toPlayerId   controller and owner of the plane (may only be one per
-     * game..)
+     *                     game..)
      * @return boolean - whether the plane was added successfully or not
      */
     @Override
@@ -1750,7 +1758,7 @@ public abstract class GameImpl implements Game, Serializable {
                     break;
                 }
                 // triggered abilities that don't use the stack have to be executed first (e.g. Banisher Priest Return exiled creature
-                for (Iterator<TriggeredAbility> it = abilities.iterator(); it.hasNext();) {
+                for (Iterator<TriggeredAbility> it = abilities.iterator(); it.hasNext(); ) {
                     TriggeredAbility triggeredAbility = it.next();
                     if (!triggeredAbility.isUsesStack()) {
                         state.removeTriggeredAbility(triggeredAbility);
@@ -1806,10 +1814,11 @@ public abstract class GameImpl implements Game, Serializable {
         while (copiedCards.hasNext()) {
             Card card = copiedCards.next();
             if (card instanceof SplitCardHalf || card instanceof AdventureCardSpell) {
-                continue; // only the main card is moves, not the halves
+                continue; // only the main card is moves, not the halves (cause halfes is not copied - it uses original card -- TODO: need to fix (bugs with same card copy)?
             }
             Zone zone = state.getZone(card.getId());
             if (zone != Zone.BATTLEFIELD && zone != Zone.STACK) {
+                // TODO: remember LKI of copied cards here after LKI rework
                 switch (zone) {
                     case GRAVEYARD:
                         for (Player player : getPlayers().values()) {
@@ -2484,7 +2493,7 @@ public abstract class GameImpl implements Game, Serializable {
         }
         //20100423 - 800.4a
         Set<Card> toOutside = new HashSet<>();
-        for (Iterator<Permanent> it = getBattlefield().getAllPermanents().iterator(); it.hasNext();) {
+        for (Iterator<Permanent> it = getBattlefield().getAllPermanents().iterator(); it.hasNext(); ) {
             Permanent perm = it.next();
             if (perm.isOwnedBy(playerId)) {
                 if (perm.getAttachedTo() != null) {
@@ -2526,7 +2535,7 @@ public abstract class GameImpl implements Game, Serializable {
         player.moveCards(toOutside, Zone.OUTSIDE, null, this);
         // triggered abilities that don't use the stack have to be executed
         List<TriggeredAbility> abilities = state.getTriggered(player.getId());
-        for (Iterator<TriggeredAbility> it = abilities.iterator(); it.hasNext();) {
+        for (Iterator<TriggeredAbility> it = abilities.iterator(); it.hasNext(); ) {
             TriggeredAbility triggeredAbility = it.next();
             if (!triggeredAbility.isUsesStack()) {
                 state.removeTriggeredAbility(triggeredAbility);
@@ -2546,7 +2555,7 @@ public abstract class GameImpl implements Game, Serializable {
 
         // Remove cards from the player in all exile zones
         for (ExileZone exile : this.getExile().getExileZones()) {
-            for (Iterator<UUID> it = exile.iterator(); it.hasNext();) {
+            for (Iterator<UUID> it = exile.iterator(); it.hasNext(); ) {
                 Card card = this.getCard(it.next());
                 if (card != null && card.isOwnedBy(playerId)) {
                     it.remove();
@@ -2556,7 +2565,7 @@ public abstract class GameImpl implements Game, Serializable {
 
         //Remove all commander/emblems/plane the player controls
         boolean addPlaneAgain = false;
-        for (Iterator<CommandObject> it = this.getState().getCommand().iterator(); it.hasNext();) {
+        for (Iterator<CommandObject> it = this.getState().getCommand().iterator(); it.hasNext(); ) {
             CommandObject obj = it.next();
             if (obj.isControlledBy(playerId)) {
                 if (obj instanceof Emblem) {
@@ -2779,9 +2788,7 @@ public abstract class GameImpl implements Game, Serializable {
             Map<UUID, CardState> lkiCardStateMap = lkiCardState.get(zone);
             if (lkiCardStateMap != null) {
                 CardState cardState = lkiCardStateMap.get(objectId);
-                if (cardState != null) {
+                return cardState;
-                    return cardState;
-                }
             }
         }
         return null;

Mage/src/main/java/mage/game/GameState.java

@@ -1,8 +1,5 @@
 package mage.game;
 
-import java.io.Serializable;
-import java.util.*;
-import java.util.stream.Collectors;
 import mage.MageObject;
 import mage.abilities.*;
 import mage.abilities.effects.ContinuousEffect;
@@ -37,6 +34,10 @@ import mage.util.ThreadLocalStringBuilder;
 import mage.watchers.Watcher;
 import mage.watchers.Watchers;
 
+import java.io.Serializable;
+import java.util.*;
+import java.util.stream.Collectors;
+
 /**
  * @author BetaSteward_at_googlemail.com
  * <p>
@@ -49,6 +50,8 @@ public class GameState implements Serializable, Copyable<GameState> {
 
     private static final ThreadLocalStringBuilder threadLocalBuilder = new ThreadLocalStringBuilder(1024);
 
+    public static final String COPIED_FROM_CARD_KEY = "CopiedFromCard";
+
     private final Players players;
     private final PlayerList playerList;
     private UUID choosingPlayerId; // player that makes a choice at game start
@@ -601,6 +604,7 @@ public class GameState implements Serializable, Copyable<GameState> {
 //    public void addMessage(String message) {
 //        this.messages.add(message);
 //    }
+
     /**
      * Returns a list of all players of the game ignoring range or if a player
      * has lost or left the game.
@@ -774,7 +778,7 @@ public class GameState implements Serializable, Copyable<GameState> {
         for (Map.Entry<ZoneChangeData, List<GameEvent>> entry : eventsByKey.entrySet()) {
             Set<Card> movedCards = new LinkedHashSet<>();
             Set<PermanentToken> movedTokens = new LinkedHashSet<>();
-            for (Iterator<GameEvent> it = entry.getValue().iterator(); it.hasNext();) {
+            for (Iterator<GameEvent> it = entry.getValue().iterator(); it.hasNext(); ) {
                 GameEvent event = it.next();
                 ZoneChangeEvent castEvent = (ZoneChangeEvent) event;
                 UUID targetId = castEvent.getTargetId();
@@ -1007,7 +1011,7 @@ public class GameState implements Serializable, Copyable<GameState> {
      * @param attachedTo
      * @param ability
      * @param copyAbility copies non MageSingleton abilities before adding to
-     * state
+     *                    state
      */
     public void addOtherAbility(Card attachedTo, Ability ability, boolean copyAbility) {
         Ability newAbility;
@@ -1165,7 +1169,7 @@ public class GameState implements Serializable, Copyable<GameState> {
         copiedCards.put(copiedCard.getId(), copiedCard);
         addCard(copiedCard);
         if (copiedCard.isSplitCard()) {
-            Card leftCard = ((SplitCard) copiedCard).getLeftHalfCard();
+            Card leftCard = ((SplitCard) copiedCard).getLeftHalfCard(); // TODO: must be new ID (bugs with same card copy)?
             copiedCards.put(leftCard.getId(), leftCard);
             addCard(leftCard);
             Card rightCard = ((SplitCard) copiedCard).getRightHalfCard();
@@ -1177,6 +1181,11 @@ public class GameState implements Serializable, Copyable<GameState> {
             copiedCards.put(spellCard.getId(), spellCard);
             addCard(spellCard);
         }
+
+        // copied cards removes from game after battlefield/stack leaves, so remember it here as workaround to fix freeze, see https://github.com/magefree/mage/issues/5437
+        // TODO: remove that workaround after LKI will be rewritten to support cross-steps/turns data transition and support copied cards
+        this.setValue(COPIED_FROM_CARD_KEY + copiedCard.getId(), cardToCopy.copy());
+
         return copiedCard;
     }