Timestamp rework

forge-ai/src/main/java/forge/ai/ability/UntapAi.java

@@ -319,15 +319,14 @@ private boolean untapTargetList(final Card source, final TargetRestrictions tgt,
 
     @Override
     public Card chooseSingleCard(Player ai, SpellAbility sa, Iterable<Card> list, boolean isOptional, Player targetedPlayer, Map<String, Object> params) {
-        CardCollection pref = CardLists.filterControlledBy(list, ai.getYourTeam());
-        if (pref.isEmpty()) {
-            if (isOptional) {
-                return null;
-            }
-        } else {
-            list = pref;
+        CardCollection filteredList = CardLists.filterControlledBy(list, ai.getYourTeam());
+        if (!filteredList.isEmpty()) {
+            return ComputerUtilCard.getBestAI(filteredList);
+        }
+        if (isOptional) {
+            return null;
         }
-        return ComputerUtilCard.getBestAI(list);
+        return ComputerUtilCard.getWorstAI(list);
     }
 
     private static Card detectPriorityUntapTargets(final List<Card> untapList) {

forge-game/src/main/java/forge/game/ability/ApiType.java

@@ -167,7 +167,6 @@ public enum ApiType {
     Reveal (RevealEffect.class),
     RevealHand (RevealHandEffect.class),
     ReverseTurnOrder (ReverseTurnOrderEffect.class),
-
     RingTemptsYou (RingTemptsYouEffect.class),
     RollDice (RollDiceEffect.class),
     RollPlanarDice (RollPlanarDiceEffect.class),

forge-game/src/main/java/forge/game/ability/effects/AlterAttributeEffect.java

@@ -34,54 +34,62 @@ public void resolve(SpellAbility sa) {
         }
 
         for (Card c : defined) {
+            final Card gameCard = c.getGame().getCardState(c, null);
+            // gameCard is LKI in that case, the card is not in game anymore
+            // or the timestamp did change
+            // this should check Self too
+            if (gameCard == null || !c.equalsWithGameTimestamp(gameCard) || gameCard.isPhasedOut()) {
+                continue;
+            }
+
             for (String attr : attributes) {
                 boolean altered = false;
 
                 switch (attr.trim()) {
                     case "Plotted":
-                        altered = c.setPlotted(activate);
+                        altered = gameCard.setPlotted(activate);
                         break;
                     case "Solve":
                     case "Solved":
-                        altered = c.setSolved(activate);
+                        altered = gameCard.setSolved(activate);
                         if (altered) {
-                            Map<AbilityKey, Object> runParams = AbilityKey.mapFromCard(c);
+                            Map<AbilityKey, Object> runParams = AbilityKey.mapFromCard(gameCard);
                             runParams.put(AbilityKey.Player, sa.getActivatingPlayer());
                             c.getGame().getTriggerHandler().runTrigger(TriggerType.CaseSolved, runParams, false);
                         }
                         break;
                     case "Suspect":
                     case "Suspected":
-                        altered = c.setSuspected(activate);
+                        altered = gameCard.setSuspected(activate);
                         break;
                     case "Saddle":
                     case "Saddled":
                         // currently clean up in Card manually
-                        altered = c.setSaddled(activate);
+                        altered = gameCard.setSaddled(activate);
                         if (altered) {
                             CardCollection saddlers = sa.getPaidList("TappedCards", true);
-                            c.addSaddledByThisTurn(saddlers);
-                            Map<AbilityKey, Object> runParams = AbilityKey.mapFromCard(c);
+                            gameCard.addSaddledByThisTurn(saddlers);
+                            Map<AbilityKey, Object> runParams = AbilityKey.mapFromCard(gameCard);
                             runParams.put(AbilityKey.Crew, saddlers);
                             c.getGame().getTriggerHandler().runTrigger(TriggerType.BecomesSaddled, runParams, false);
                         }
                         break;
                     case "Commander":
                         //This implementation doesn't let a card make someone else's creature your commander. But that's an edge case among edge cases.
-                        Player p = c.getOwner();
-                        if (c.isCommander() == activate || p.getCommanders().contains(c) == activate)
+                        Player p = gameCard.getOwner();
+                        if (gameCard.isCommander() == activate || p.getCommanders().contains(gameCard) == activate)
                             break; //Isn't changing status.
                         if (activate) {
-                            if (!c.getGame().getRules().hasCommander()) {
+                            if (!gameCard.getGame().getRules().hasCommander()) {
                                 System.out.println("Commander status applied in non-commander format. Applying Commander variant.");
-                                c.getGame().getRules().addAppliedVariant(GameType.Commander);
+                                gameCard.getGame().getRules().addAppliedVariant(GameType.Commander);
                             }
-                            p.addCommander(c);
+                            p.addCommander(gameCard);
                             //Seems important enough to mention in the game log.
-                            c.getGame().getGameLog().add(GameLogEntryType.STACK_RESOLVE, String.format("%s is now %s's commander.", c.getPaperCard().getName(), p));
+                            gameCard.getGame().getGameLog().add(GameLogEntryType.STACK_RESOLVE, String.format("%s is now %s's commander.", gameCard.getPaperCard().getName(), p));
                         } else {
-                            p.removeCommander(c);
-                            c.getGame().getGameLog().add(GameLogEntryType.STACK_RESOLVE, String.format("%s is no longer %s's commander.", c.getPaperCard().getName(), p));
+                            p.removeCommander(gameCard);
+                            gameCard.getGame().getGameLog().add(GameLogEntryType.STACK_RESOLVE, String.format("%s is no longer %s's commander.", gameCard.getPaperCard().getName(), p));
                         }
                         altered = true;
                         break;
@@ -93,10 +101,10 @@ public void resolve(SpellAbility sa) {
                 }
 
                 if (altered && sa.hasParam("RememberAltered")) {
-                    sa.getHostCard().addRemembered(c);
+                    sa.getHostCard().addRemembered(gameCard);
                 }
             }
-            c.updateAbilityTextForView();
+            gameCard.updateAbilityTextForView();
         }
     }
 }

forge-game/src/main/java/forge/game/ability/effects/AnimateEffect.java

@@ -163,41 +163,49 @@ public void resolve(final SpellAbility sa) {
             }
         }
 
-        for (final Card c : tgts) {
+        for (final Card tgtC : tgts) {
             // CR 702.26e
-            if (c.isPhasedOut()) {
+            if (tgtC.isPhasedOut()) {
                 continue;
             }
 
-            doAnimate(c, sa, power, toughness, types, removeTypes, finalColors,
+            final Card gameCard = game.getCardState(tgtC, null);
+            // gameCard is LKI in that case, the card is not in game anymore
+            // or the timestamp did change
+            // this should check Self too
+            if (gameCard == null || !tgtC.equalsWithGameTimestamp(gameCard) || gameCard.isPhasedOut()) {
+                continue;
+            }
+
+            doAnimate(gameCard, sa, power, toughness, types, removeTypes, finalColors,
                     keywords, removeKeywords, hiddenKeywords,
                     abilities, triggers, replacements, stAbs, timestamp, duration);
 
             if (sa.hasParam("Name")) {
-                c.addChangedName(sa.getParam("Name"), false, timestamp, 0);
+                gameCard.addChangedName(sa.getParam("Name"), false, timestamp, 0);
             }
 
             // give sVars
             if (!sVarsMap.isEmpty()) {
-                c.addChangedSVars(sVarsMap, timestamp, 0);
+                gameCard.addChangedSVars(sVarsMap, timestamp, 0);
             }
 
             // give Remembered
             if (animateRemembered != null) {
-                c.addRemembered(AbilityUtils.getDefinedObjects(source, animateRemembered, sa));
+                gameCard.addRemembered(AbilityUtils.getDefinedObjects(source, animateRemembered, sa));
             }
 
             // give Imprinted
             if (animateImprinted != null) {
-                c.addImprintedCards(AbilityUtils.getDefinedCards(source, animateImprinted, sa));
+                gameCard.addImprintedCards(AbilityUtils.getDefinedCards(source, animateImprinted, sa));
             }
 
             if (sa.isCrew()) {
-                c.becomesCrewed(sa);
-                c.updatePowerToughnessForView();
+                gameCard.becomesCrewed(sa);
+                gameCard.updatePowerToughnessForView();
             }
 
-            game.fireEvent(new GameEventCardStatsChanged(c));
+            game.fireEvent(new GameEventCardStatsChanged(gameCard));
         }
 
         if (sa.hasParam("AtEOT") && !tgts.isEmpty()) {

forge-game/src/main/java/forge/game/ability/effects/BecomesBlockedEffect.java

@@ -29,6 +29,9 @@ protected String getStackDescription(SpellAbility sa) {
     @Override
     public void resolve(SpellAbility sa) {
         final Game game = sa.getActivatingPlayer().getGame();
+        if (game.getCombat() == null) {
+            return;
+        }
         List<Card> blocked = Lists.newArrayList();
         for (final Card c : getTargetCards(sa)) {
             game.getCombat().setBlocked(c, true);

forge-game/src/main/java/forge/game/ability/effects/ChooseCardEffect.java

@@ -19,6 +19,7 @@
 import forge.game.card.CardPredicates;
 import forge.game.player.Player;
 import forge.game.player.PlayerActionConfirmMode;
+import forge.game.player.PlayerCollection;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
 import forge.util.Aggregates;
@@ -60,7 +61,7 @@ public void resolve(SpellAbility sa) {
         final Game game = activator.getGame();
         CardCollection allChosen = new CardCollection();
 
-        final List<Player> tgtPlayers = getDefinedPlayersOrTargeted(sa);
+        final PlayerCollection tgtPlayers = getDefinedPlayersOrTargeted(sa);
 
         List<ZoneType> choiceZone = Lists.newArrayList(ZoneType.Battlefield);
         if (sa.hasParam("ChoiceZone")) {
@@ -71,7 +72,7 @@ public void resolve(SpellAbility sa) {
             choices = CardLists.getValidCards(choices, sa.getParam("Choices"), activator, host, sa);
         }
         if (sa.hasParam("TargetControls")) {
-            choices = CardLists.filterControlledBy(choices, tgtPlayers.get(0));
+            choices = CardLists.filterControlledBy(choices, tgtPlayers);
         }
         if (sa.hasParam("DefinedCards")) {
             choices = AbilityUtils.getDefinedCards(host, sa.getParam("DefinedCards"), sa);

forge-game/src/main/java/forge/game/ability/effects/ChoosePlayerEffect.java

@@ -20,7 +20,7 @@ protected String getStackDescription(SpellAbility sa) {
 
         sb.append(Lang.joinHomogenous(getTargetPlayers(sa)));
 
-        sb.append("chooses a player.");
+        sb.append(" chooses a player.");
 
         return sb.toString();
     }

forge-game/src/main/java/forge/game/ability/effects/ChooseTypeEffect.java

@@ -1,6 +1,5 @@
 package forge.game.ability.effects;
 
-import java.security.InvalidParameterException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -152,7 +151,7 @@ public void resolve(SpellAbility sa) {
                 }
             }
         } else {
-            throw new InvalidParameterException(sa.getHostCard() + "'s ability resulted in no types to choose from");
+            throw new RuntimeException(sa.getHostCard() + "'s ability resulted in no types to choose from");
         }
     }
 

forge-game/src/main/java/forge/game/ability/effects/CloneEffect.java

@@ -21,7 +21,6 @@
 import java.util.List;
 
 public class CloneEffect extends SpellAbilityEffect {
-    // TODO update this method
 
     @Override
     protected String getStackDescription(SpellAbility sa) {

forge-game/src/main/java/forge/game/ability/effects/CountersRemoveEffect.java

@@ -123,14 +123,13 @@ public void resolve(SpellAbility sa) {
             }
         }
 
-        CardCollectionView srcCards = null;
+        CardCollectionView srcCards;
 
         String typeforPrompt = counterType == null ? "" : counterType.getName();
         String title = Localizer.getInstance().getMessage("lblChooseCardsToTakeTargetCounters", typeforPrompt);
         title = title.replace("  ", " ");
         if (sa.hasParam("ValidSource")) {
-            srcCards = game.getCardsIn(ZoneType.Battlefield);
-            srcCards = CardLists.getValidCards(srcCards, sa.getParam("ValidSource"), activator, card, sa);
+            srcCards = CardLists.getValidCards(game.getCardsIn(ZoneType.Battlefield), sa.getParam("ValidSource"), activator, card, sa);
             if (num.equals("Any")) {
                 Map<String, Object> params = Maps.newHashMap();
                 params.put("CounterType", counterType);

forge-game/src/main/java/forge/game/ability/effects/DamageDealEffect.java

@@ -255,13 +255,10 @@ public void resolve(SpellAbility sa) {
                 if (o instanceof Card) {
                     final Card c = (Card) o;
                     final Card gc = game.getCardState(c, null);
-                    if (gc == null || !c.equalsWithGameTimestamp(gc) || !gc.isInPlay()) {
+                    if (gc == null || !c.equalsWithGameTimestamp(gc) || !gc.isInPlay() || gc.isPhasedOut()) {
                         // timestamp different or not in play
                         continue;
                     }
-                    if (c.isPhasedOut()) {
-                        continue;
-                    }
                     internalDamageDeal(sa, sourceLKI, gc, dmg, damageMap);
                 } else if (o instanceof Player) {
                     final Player p = (Player) o;

forge-game/src/main/java/forge/game/ability/effects/DamageEachEffect.java

@@ -93,20 +93,29 @@ public void resolve(SpellAbility sa) {
                     }
                 }
             }
-        } else for (final GameEntity ge : getTargetEntities(sa)) {
+        } else for (GameEntity ge : getTargetEntities(sa)) {
+            // check before checking sources
+            if (ge instanceof Card) {
+                final Card c = (Card) ge;
+                if (!c.isInPlay() || c.isPhasedOut()) {
+                    continue;
+                }
+                // check if the object is still in game or if it was moved
+                Card gameCard = game.getCardState(c, null);
+                // gameCard is LKI in that case, the card is not in game anymore
+                // or the timestamp did change
+                // this should check Self too
+                if (gameCard == null || !c.equalsWithGameTimestamp(gameCard)) {
+                    continue;
+                }
+                ge = gameCard;
+            }
+
             for (final Card source : sources) {
                 final Card sourceLKI = game.getChangeZoneLKIInfo(source);
-
                 final int dmg = AbilityUtils.calculateAmount(source, num, sa);
 
-                if (ge instanceof Card) {
-                    final Card c = (Card) ge;
-                    if (c.isInPlay() && !c.isPhasedOut()) {
-                        damageMap.put(sourceLKI, c, dmg);
-                    }
-                } else {
-                    damageMap.put(sourceLKI, ge, dmg);
-                }
+                damageMap.put(sourceLKI, ge, dmg);
             }
         }
 

forge-game/src/main/java/forge/game/ability/effects/DebuffEffect.java

@@ -87,11 +87,10 @@ public void resolve(SpellAbility sa) {
 
             final List<String> addedKW = Lists.newArrayList();
             final List<String> removedKW = Lists.newArrayList();
-
             if (sa.hasParam("AllSuffixKeywords")) {
                 // this only for walk abilities, may to try better
                 if (sa.getParam("AllSuffixKeywords").equals("walk")) {
-                    for (final KeywordInterface kw : tgtC.getKeywords(Keyword.LANDWALK)) {
+                    for (final KeywordInterface kw : gameCard.getKeywords(Keyword.LANDWALK)) {
                         removedKW.add(kw.getOriginal());
                     }
                 }
@@ -109,7 +108,7 @@ public void resolve(SpellAbility sa) {
                         continue;
                     }
                     final String wardString = StringUtils.capitalize(colString) + ":" + colString;
-                    for (final KeywordInterface inst : tgtC.getKeywords(Keyword.PROTECTION)) {
+                    for (final KeywordInterface inst : gameCard.getKeywords(Keyword.PROTECTION)) {
                         // special for the Ward Auras Protection:Card.<Color>:<color>:*
                         String keyword = inst.getOriginal();
                         if (keyword.startsWith("Protection:") && keyword.contains(wardString)) {
@@ -122,11 +121,12 @@ public void resolve(SpellAbility sa) {
             if (ProtectionFromColor) {
                 // Split "Protection from each color" into extra Protection from <color>
                 String allColors = "Protection from each color";
-                if (tgtC.hasKeyword(allColors)) {
+                if (gameCard.hasKeyword(allColors)) {
                     final List<String> allColorsProtect = Lists.newArrayList();
 
                     for (byte col : MagicColor.WUBRG) {
                         allColorsProtect.add("Protection from " + MagicColor.toLongString(col));
+
                     }
                     allColorsProtect.removeAll(kws);
                     addedKW.addAll(allColorsProtect);
@@ -135,7 +135,7 @@ public void resolve(SpellAbility sa) {
 
                 // Extra for Spectra Ward
                 allColors = "Protection:Card.nonColorless:each color:Aura";
-                if (tgtC.hasKeyword(allColors)) {
+                if (gameCard.hasKeyword(allColors)) {
                     final List<String> allColorsProtect = Lists.newArrayList();
 
                     for (byte col : MagicColor.WUBRG) {
@@ -150,15 +150,15 @@ public void resolve(SpellAbility sa) {
             }
 
             removedKW.addAll(kws);
-            tgtC.addChangedCardKeywords(addedKW, removedKW, false, timestamp, null);
+            gameCard.addChangedCardKeywords(addedKW, removedKW, false, timestamp, null);
 
             if (!"Permanent".equals(sa.getParam("Duration"))) {
                 final GameCommand until = new GameCommand() {
                     private static final long serialVersionUID = 5387486776282932314L;
 
                     @Override
                     public void run() {
-                        tgtC.removeChangedCardKeywords(timestamp, 0);
+                        gameCard.removeChangedCardKeywords(timestamp, 0);
                     }
                 };
                 addUntilCommand(sa, until);