AI Attack Timeout

forge-ai/src/main/java/forge/ai/AiAttackController.java

@@ -50,6 +50,10 @@ import forge.util.collect.FCollectionView;
 import org.apache.commons.lang3.tuple.Pair;
 
 import java.util.*;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 
 
 /**
@@ -169,7 +173,7 @@ public class AiAttackController {
     }
 
     public void removeBlocker(Card blocker) {
-    	this.oppList.remove(blocker);
+        this.oppList.remove(blocker);
         this.blockers.remove(blocker);
     }
 
@@ -286,7 +290,7 @@ public class AiAttackController {
         }
 
         if ("TRUE".equals(attacker.getSVar("HasAttackEffect"))) {
-        	return true;
+            return true;
         }
 
         // Damage opponent if unblocked
@@ -388,7 +392,7 @@ public class AiAttackController {
         // reduce the search space
         final List<Card> opponentsAttackers = CardLists.filter(ai.getOpponents().getCreaturesInPlay(), c -> !c.hasSVar("EndOfTurnLeavePlay")
                 && (c.toughnessAssignsDamage() || c.getNetCombatDamage() > 0 // performance shortcuts
-                        || c.getNetCombatDamage() + ComputerUtilCombat.predictPowerBonusOfAttacker(c, null, null, true) > 0)
+                || c.getNetCombatDamage() + ComputerUtilCombat.predictPowerBonusOfAttacker(c, null, null, true) > 0)
                 && ComputerUtilCombat.canAttackNextTurn(c));
 
         // don't hold back creatures that can't block any of the human creatures
@@ -884,7 +888,7 @@ public class AiAttackController {
         // TODO: detect Season of the Witch by presence of a card with a specific trigger
         final boolean seasonOfTheWitch = ai.getGame().isCardInPlay("Season of the Witch");
 
-        List<Card> attackersLeft = new ArrayList<>(this.attackers);
+        final Queue<Card> attackersLeft = new ConcurrentLinkedQueue<>(this.attackers);
 
         // TODO probably use AttackConstraints instead of only GlobalAttackRestrictions?
         GlobalAttackRestrictions restrict = GlobalAttackRestrictions.getGlobalRestrictions(ai, combat.getDefenders());
@@ -900,63 +904,71 @@ public class AiAttackController {
         }
 
         // Attackers that don't really have a choice
-        int numForcedAttackers = 0;
+        final AtomicInteger numForcedAttackers = new AtomicInteger(0);
         // nextTurn is now only used by effect from Oracle en-Vec, which can skip check must attack,
         // because creatures not chosen can't attack.
+        List<CompletableFuture<Integer>> futures = new ArrayList<>();
         if (!nextTurn) {
             for (final Card attacker : this.attackers) {
-                GameEntity mustAttackDef = null;
+                final GameEntity finalDefender = defender;
-                if (attacker.getSVar("MustAttack").equals("True")) {
+                futures.add(CompletableFuture.supplyAsync(()-> {
-                    mustAttackDef = defender;
+                    GameEntity mustAttackDef = null;
-                } else if (attacker.hasSVar("EndOfTurnLeavePlay")
+                    if (attacker.getSVar("MustAttack").equals("True")) {
-                        && isEffectiveAttacker(ai, attacker, combat, defender)) {
+                        mustAttackDef = finalDefender;
-                    mustAttackDef = defender;
+                    } else if (attacker.hasSVar("EndOfTurnLeavePlay")
-                } else if (seasonOfTheWitch) {
+                            && isEffectiveAttacker(ai, attacker, combat, finalDefender)) {
-                    //TODO: if there are other ways to tap this creature (like mana creature), then don't need to attack
+                        mustAttackDef = finalDefender;
-                    mustAttackDef = defender;
+                    } else if (seasonOfTheWitch) {
-                } else {
+                        //TODO: if there are other ways to tap this creature (like mana creature), then don't need to attack
-                    if (combat.getAttackConstraints().getRequirements().get(attacker) == null) continue;
+                        mustAttackDef = finalDefender;
-                    // check defenders in order of maximum requirements
+                    } else {
-                    List<Pair<GameEntity, Integer>> reqs = combat.getAttackConstraints().getRequirements().get(attacker).getSortedRequirements();
+                        if (combat.getAttackConstraints().getRequirements().get(attacker) == null) return 0;
-                    final GameEntity def = defender;
+                        // check defenders in order of maximum requirements
-                    reqs.sort((r1, r2) -> {
+                        List<Pair<GameEntity, Integer>> reqs = combat.getAttackConstraints().getRequirements().get(attacker).getSortedRequirements();
-                        if (r1.getValue() == r2.getValue()) {
+                        final GameEntity def = finalDefender;
-                            // try to attack the designated defender
+                        reqs.sort((r1, r2) -> {
-                            if (r1.getKey().equals(def) && !r2.getKey().equals(def)) {
+                            if (r1.getValue() == r2.getValue()) {
-                                return -1;
+                                // try to attack the designated defender
+                                if (r1.getKey().equals(def) && !r2.getKey().equals(def)) {
+                                    return -1;
+                                }
+                                if (r2.getKey().equals(def) && !r1.getKey().equals(def)) {
+                                    return 1;
+                                }
+                                // otherwise PW
+                                if (r1.getKey() instanceof Card && r2.getKey() instanceof Player) {
+                                    return -1;
+                                }
+                                if (r2.getKey() instanceof Card && r1.getKey() instanceof Player) {
+                                    return 1;
+                                }
+                                // or weakest player
+                                if (r1.getKey() instanceof Player && r2.getKey() instanceof Player) {
+                                    return ((Player) r1.getKey()).getLife() - ((Player) r2.getKey()).getLife();
+                                }
                             }
-                            if (r2.getKey().equals(def) && !r1.getKey().equals(def)) {
+                            return r2.getValue() - r1.getValue();
-                                return 1;
+                        });
+                        for (Pair<GameEntity, Integer> e : reqs) {
+                            if (e.getRight() == 0) continue;
+                            GameEntity mustAttackDefMaybe = e.getLeft();
+                            if (canAttackWrapper(attacker, mustAttackDefMaybe) && CombatUtil.getAttackCost(ai.getGame(), attacker, mustAttackDefMaybe) == null) {
+                                mustAttackDef = mustAttackDefMaybe;
+                                break;
                             }
-                            // otherwise PW
-                            if (r1.getKey() instanceof Card && r2.getKey() instanceof Player) {
-                                return -1;
-                            }
-                            if (r2.getKey() instanceof Card && r1.getKey() instanceof Player) {
-                                return 1;
-                            }
-                            // or weakest player
-                            if (r1.getKey() instanceof Player && r2.getKey() instanceof Player) {
-                                return ((Player) r1.getKey()).getLife() - ((Player) r2.getKey()).getLife();
-                            }
-                        }
-                        return r2.getValue() - r1.getValue();
-                    });
-                    for (Pair<GameEntity, Integer> e : reqs) {
-                        if (e.getRight() == 0) continue;
-                        GameEntity mustAttackDefMaybe = e.getLeft();
-                        if (canAttackWrapper(attacker, mustAttackDefMaybe) && CombatUtil.getAttackCost(ai.getGame(), attacker, mustAttackDefMaybe) == null) {
-                            mustAttackDef = mustAttackDefMaybe;
-                            break;
                         }
                     }
-                }
+                    if (mustAttackDef != null) {
-                if (mustAttackDef != null) {
+                        combat.addAttacker(attacker, mustAttackDef);
-                    combat.addAttacker(attacker, mustAttackDef);
+                        attackersLeft.remove(attacker);
-                    attackersLeft.remove(attacker);
+                        numForcedAttackers.incrementAndGet();
-                    numForcedAttackers++;
+                    }
-                }
+                    return 0;
+                }));
             }
+            CompletableFuture<?>[] futuresArray = futures.toArray(new CompletableFuture<?>[0]);
+            CompletableFuture.allOf(futuresArray).completeOnTimeout(null, 5, TimeUnit.SECONDS).join();
+            futures.clear();
             if (attackersLeft.isEmpty()) {
                 return aiAggression;
             }
@@ -964,18 +976,19 @@ public class AiAttackController {
 
         // Lightmine Field: make sure the AI doesn't wipe out its own creatures
         if (lightmineField) {
-            doLightmineFieldAttackLogic(attackersLeft, numForcedAttackers, playAggro);
+            doLightmineFieldAttackLogic(attackersLeft, numForcedAttackers.get(), playAggro);
         }
         // Revenge of Ravens: make sure the AI doesn't kill itself and doesn't damage itself unnecessarily
-        if (!doRevengeOfRavensAttackLogic(defender, attackersLeft, numForcedAttackers, attackMax)) {
+        if (!doRevengeOfRavensAttackLogic(defender, attackersLeft, numForcedAttackers.get(), attackMax)) {
             return aiAggression;
         }
 
         if (bAssault && defender == defendingOpponent) { // in case we are forced to attack someone else
             if (LOG_AI_ATTACKS)
                 System.out.println("Assault");
-            CardLists.sortByPowerDesc(attackersLeft);
+            List<Card> left = new ArrayList<>(attackersLeft);
-            for (Card attacker : attackersLeft) {
+            CardLists.sortByPowerDesc(left);
+            for (Card attacker : left) {
                 // reached max, breakup
                 if (attackMax != -1 && combat.getAttackers().size() >= attackMax)
                     return aiAggression;
@@ -1227,7 +1240,7 @@ public class AiAttackController {
         if (ratioDiff > 0 && doAttritionalAttack) {
             aiAggression = 5; // attack at all costs
         } else if ((ratioDiff >= 1 && this.attackers.size() > 1 && (humanLifeToDamageRatio < 2 || outNumber > 0))
-        		|| (playAggro && MyRandom.percentTrue(chanceToAttackToTrade) && humanLifeToDamageRatio > 1)) {
+                || (playAggro && MyRandom.percentTrue(chanceToAttackToTrade) && humanLifeToDamageRatio > 1)) {
             aiAggression = 4; // attack expecting to trade or damage player.
         } else if (MyRandom.percentTrue(chanceToAttackToTrade) && humanLifeToDamageRatio > 1
                 && defendingOpponent != null
@@ -1237,7 +1250,7 @@ public class AiAttackController {
                 && (ComputerUtilMana.getAvailableManaEstimate(ai) > 0) || tradeIfTappedOut
                 && (ComputerUtilMana.getAvailableManaEstimate(defendingOpponent) == 0) || MyRandom.percentTrue(extraChanceIfOppHasMana)
                 && (!tradeIfLowerLifePressure || (ai.getLifeLostLastTurn() + ai.getLifeLostThisTurn() <
-                        defendingOpponent.getLifeLostThisTurn() + defendingOpponent.getLifeLostThisTurn()))) {
+                defendingOpponent.getLifeLostThisTurn() + defendingOpponent.getLifeLostThisTurn()))) {
             aiAggression = 4; // random (chance-based) attack expecting to trade or damage player.
         } else if (ratioDiff >= 0 && this.attackers.size() > 1) {
             aiAggression = 3; // attack expecting to make good trades or damage player.
@@ -1265,19 +1278,20 @@ public class AiAttackController {
         if ( LOG_AI_ATTACKS )
             System.out.println("Normal attack");
 
-        attackersLeft = notNeededAsBlockers(combat.getAttackers(), attackersLeft);
+        List<Card> left = new ArrayList<>(attackersLeft);
-        attackersLeft = sortAttackers(attackersLeft);
+        left = notNeededAsBlockers(combat.getAttackers(), left);
+        left = sortAttackers(left);
 
         if ( LOG_AI_ATTACKS )
-            System.out.println("attackersLeft = " + attackersLeft);
+            System.out.println("attackersLeft = " + left);
 
         FCollection<GameEntity> possibleDefenders = new FCollection<>(defendingOpponent);
         possibleDefenders.addAll(defendingOpponent.getPlaneswalkersInPlay());
 
-        while (!attackersLeft.isEmpty()) {
+        while (!left.isEmpty()) {
             CardCollection attackersAssigned = new CardCollection();
-            for (int i = 0; i < attackersLeft.size(); i++) {
+            for (int i = 0; i < left.size(); i++) {
-                final Card attacker = attackersLeft.get(i);
+                final Card attacker = left.get(i);
                 if (aiAggression < 5 && !attacker.hasFirstStrike() && !attacker.hasDoubleStrike()
                         && ComputerUtilCombat.getTotalFirstStrikeBlockPower(attacker, defendingOpponent)
                         >= ComputerUtilCombat.getDamageToKill(attacker, false)) {
@@ -1291,7 +1305,7 @@ public class AiAttackController {
                     attackersAssigned.add(attacker);
 
                     // check if attackers are enough to finish the attacked planeswalker
-                    if (i < attackersLeft.size() - 1 && defender instanceof Card) {
+                    if (i < left.size() - 1 && defender instanceof Card) {
                         final int blockNum = this.blockers.size();
                         int attackNum = 0;
                         int damage = 0;
@@ -1312,9 +1326,9 @@ public class AiAttackController {
                 }
             }
 
-            attackersLeft.removeAll(attackersAssigned);
+            left.removeAll(attackersAssigned);
             possibleDefenders.remove(defender);
-            if (attackersLeft.isEmpty() || possibleDefenders.isEmpty()) {
+            if (left.isEmpty() || possibleDefenders.isEmpty()) {
                 break;
             }
             CardCollection pwDefending = new CardCollection(Iterables.filter(possibleDefenders, Card.class));
@@ -1496,51 +1510,51 @@ public class AiAttackController {
 
         // decide if the creature should attack based on the prevailing strategy choice in aiAggression
         switch (aiAggression) {
-        case 6: // Exalted: expecting to at least kill a creature of equal value or not be blocked
+            case 6: // Exalted: expecting to at least kill a creature of equal value or not be blocked
-            if ((saf.canKillAll && saf.isWorthLessThanAllKillers) || !saf.canBeBlocked()) {
+                if ((saf.canKillAll && saf.isWorthLessThanAllKillers) || !saf.canBeBlocked()) {
+                    if (LOG_AI_ATTACKS)
+                        System.out.println(attacker.getName() + " = attacking expecting to kill creature, or is unblockable");
+                    return true;
+                }
+                break;
+            case 5: // all out attacking
                 if (LOG_AI_ATTACKS)
-                    System.out.println(attacker.getName() + " = attacking expecting to kill creature, or is unblockable");
+                    System.out.println(attacker.getName() + " = all out attacking");
                 return true;
-            }
+            case 4: // expecting to at least trade with something, or can attack "for free", expecting no counterattack
-            break;
+                if (saf.canKillAll || (saf.dangerousBlockersPresent && saf.canKillAllDangerous && !saf.canBeKilledByOne) || !saf.canBeBlocked()
-        case 5: // all out attacking
+                        || saf.defPower == 0) {
-            if (LOG_AI_ATTACKS)
+                    if (LOG_AI_ATTACKS)
-                System.out.println(attacker.getName() + " = all out attacking");
+                        System.out.println(attacker.getName() + " = attacking expecting to at least trade with something");
-            return true;
+                    return true;
-        case 4: // expecting to at least trade with something, or can attack "for free", expecting no counterattack
+                }
-            if (saf.canKillAll || (saf.dangerousBlockersPresent && saf.canKillAllDangerous && !saf.canBeKilledByOne) || !saf.canBeBlocked()
+                break;
-                    || saf.defPower == 0) {
+            case 3: // expecting to at least kill a creature of equal value or not be blocked
-                if (LOG_AI_ATTACKS)
+                if ((saf.canKillAll && saf.isWorthLessThanAllKillers)
-                    System.out.println(attacker.getName() + " = attacking expecting to at least trade with something");
+                        || (((saf.dangerousBlockersPresent && saf.canKillAllDangerous) || saf.hasAttackEffect || saf.hasCombatEffect) && !saf.canBeKilledByOne)
-                return true;
+                        || !saf.canBeBlocked()) {
-            }
+                    if (LOG_AI_ATTACKS)
-            break;
+                        System.out.println(attacker.getName() + " = attacking expecting to kill creature or cause damage, or is unblockable");
-        case 3: // expecting to at least kill a creature of equal value or not be blocked
+                    return true;
-            if ((saf.canKillAll && saf.isWorthLessThanAllKillers)
+                }
-                    || (((saf.dangerousBlockersPresent && saf.canKillAllDangerous) || saf.hasAttackEffect || saf.hasCombatEffect) && !saf.canBeKilledByOne)
+                break;
-                    || !saf.canBeBlocked()) {
+            case 2: // attack expecting to attract a group block or destroying a single blocker and surviving
-                if (LOG_AI_ATTACKS)
+                if (!saf.canBeBlocked() || ((saf.canKillAll || saf.hasAttackEffect || saf.hasCombatEffect) && !saf.canBeKilledByOne &&
-                    System.out.println(attacker.getName() + " = attacking expecting to kill creature or cause damage, or is unblockable");
+                        ((saf.dangerousBlockersPresent && saf.canKillAllDangerous) || !saf.canBeKilled))) {
-                return true;
+                    if (LOG_AI_ATTACKS)
-            }
+                        System.out.println(attacker.getName() + " = attacking expecting to survive or attract group block");
-            break;
+                    return true;
-        case 2: // attack expecting to attract a group block or destroying a single blocker and surviving
+                }
-            if (!saf.canBeBlocked() || ((saf.canKillAll || saf.hasAttackEffect || saf.hasCombatEffect) && !saf.canBeKilledByOne &&
+                break;
-                    ((saf.dangerousBlockersPresent && saf.canKillAllDangerous) || !saf.canBeKilled))) {
+            case 1: // unblockable creatures only
-                if (LOG_AI_ATTACKS)
+                if (!saf.canBeBlocked() || (saf.numberOfPossibleBlockers == 1 && saf.canKillAll && !saf.canBeKilledByOne)) {
-                    System.out.println(attacker.getName() + " = attacking expecting to survive or attract group block");
+                    if (LOG_AI_ATTACKS)
-                return true;
+                        System.out.println(attacker.getName() + " = attacking expecting not to be blocked");
-            }
+                    return true;
-            break;
+                }
-        case 1: // unblockable creatures only
+                break;
-            if (!saf.canBeBlocked() || (saf.numberOfPossibleBlockers == 1 && saf.canKillAll && !saf.canBeKilledByOne)) {
+            default:
-                if (LOG_AI_ATTACKS)
+                break;
-                    System.out.println(attacker.getName() + " = attacking expecting not to be blocked");
-                return true;
-            }
-            break;
-        default:
-            break;
         }
         return false; // don't attack
     }
@@ -1657,31 +1671,31 @@ public class AiAttackController {
             }
             if (color != null) {
                 switch (color) {
-                case "black":
+                    case "black":
-                    if (!c.isBlack()) {
+                        if (!c.isBlack()) {
-                        color = null;
+                            color = null;
-                    }
+                        }
-                    break;
+                        break;
-                case "blue":
+                    case "blue":
-                    if (!c.isBlue()) {
+                        if (!c.isBlue()) {
-                        color = null;
+                            color = null;
-                    }
+                        }
-                    break;
+                        break;
-                case "green":
+                    case "green":
-                    if (!c.isGreen()) {
+                        if (!c.isGreen()) {
-                        color = null;
+                            color = null;
-                    }
+                        }
-                    break;
+                        break;
-                case "red":
+                    case "red":
-                    if (!c.isRed()) {
+                        if (!c.isRed()) {
-                        color = null;
+                            color = null;
-                    }
+                        }
-                    break;
+                        break;
-                case "white":
+                    case "white":
-                    if (!c.isWhite()) {
+                        if (!c.isWhite()) {
-                        color = null;
+                            color = null;
-                    }
+                        }
-                    break;
+                        break;
                 }
             }
             if (color == null && artifact == null) { //nothing can make the attacker unblockable
@@ -1697,7 +1711,7 @@ public class AiAttackController {
         return null; //should never get here
     }
 
-    private void doLightmineFieldAttackLogic(final List<Card> attackersLeft, int numForcedAttackers, boolean playAggro) {
+    private void doLightmineFieldAttackLogic(final Queue<Card> attackersLeft, int numForcedAttackers, boolean playAggro) {
         CardCollection attSorted = new CardCollection(attackersLeft);
         CardCollection attUnsafe = new CardCollection();
         CardLists.sortByToughnessDesc(attSorted);
@@ -1727,7 +1741,7 @@ public class AiAttackController {
         attackersLeft.removeAll(attUnsafe);
     }
 
-    private boolean doRevengeOfRavensAttackLogic(final GameEntity defender, final List<Card> attackersLeft, int numForcedAttackers, int maxAttack) {
+    private boolean doRevengeOfRavensAttackLogic(final GameEntity defender, final Queue<Card> attackersLeft, int numForcedAttackers, int maxAttack) {
         // TODO: detect Revenge of Ravens by the trigger instead of by name
         boolean revengeOfRavens = false;
         if (defender instanceof Player) {

forge-ai/src/main/java/forge/ai/AiController.java

@@ -61,6 +61,7 @@ import forge.game.trigger.WrappedAbility;
 import forge.game.zone.ZoneType;
 import forge.item.PaperCard;
 import forge.util.Aggregates;
+import forge.util.CollectionUtil;
 import forge.util.ComparatorUtil;
 import forge.util.Expressions;
 import forge.util.MyRandom;
@@ -68,6 +69,9 @@ import io.sentry.Breadcrumb;
 import io.sentry.Sentry;
 
 import java.util.*;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.TimeUnit;
 
 /**
  * <p>
@@ -88,6 +92,7 @@ public class AiController {
     private SpellAbilityPicker simPicker;
     private int lastAttackAggression;
     private boolean useLivingEnd;
+    private List<SpellAbility> skipped;
 
     public AiController(final Player computerPlayer, final Game game0) {
         player = computerPlayer;
@@ -397,10 +402,27 @@ public class AiController {
     private static List<SpellAbility> getPlayableCounters(final CardCollection l) {
         final List<SpellAbility> spellAbility = Lists.newArrayList();
         for (final Card c : l) {
-            for (final SpellAbility sa : c.getNonManaAbilities()) {
+            if (c.isForetold() && c.getAlternateState() != null) {
-                // Check if this AF is a Counterspell
+                try {
-                if (sa.getApi() == ApiType.Counter) {
+                    for (final SpellAbility sa : c.getAlternateState().getNonManaAbilities()) {
-                    spellAbility.add(sa);
+                        // Check if this AF is a Counterspell
+                        if (sa.getApi() == ApiType.Counter) {
+                            spellAbility.add(sa);
+                        } else {
+                            if (sa.getApi() != null && sa.getApi().toString().contains("Foretell") && c.getAlternateState().getName().equalsIgnoreCase("Saw It Coming"))
+                                spellAbility.add(sa);
+                        }
+                    }
+                } catch (Exception e) {
+                    // facedown and alternatestate counters should be accessible
+                    e.printStackTrace();
+                }
+            } else {
+                for (final SpellAbility sa : c.getNonManaAbilities()) {
+                    // Check if this AF is a Counterspell
+                    if (sa.getApi() == ApiType.Counter) {
+                        spellAbility.add(sa);
+                    }
                 }
             }
         }
@@ -1323,9 +1345,7 @@ public class AiController {
 
         for (final Card element : combat.getAttackers()) {
             // tapping of attackers happens after Propaganda is paid for
-            final StringBuilder sb = new StringBuilder();
+            Log.debug("Computer just assigned " + element.getName() + " as an attacker.");
-            sb.append("Computer just assigned ").append(element.getName()).append(" as an attacker.");
-            Log.debug(sb.toString());
         }
     }
 
@@ -1369,7 +1389,7 @@ public class AiController {
             if (landsWannaPlay != null && !landsWannaPlay.isEmpty()) {
                 // TODO search for other land it might want to play?
                 Card land = chooseBestLandToPlay(landsWannaPlay);
-                if ((!player.canLoseLife() || player.cantLoseForZeroOrLessLife() || ComputerUtil.getDamageFromETB(player, land) < player.getLife())
+                if (land != null && (!player.canLoseLife() || player.cantLoseForZeroOrLessLife() || ComputerUtil.getDamageFromETB(player, land) < player.getLife())
                         && (!game.getPhaseHandler().is(PhaseType.MAIN1) || !isSafeToHoldLandDropForMain2(land))) {
                     final List<SpellAbility> abilities = land.getAllPossibleAbilities(player, true);
                     // skip non Land Abilities
@@ -1502,6 +1522,13 @@ public class AiController {
     }
 
     private SpellAbility getSpellAbilityToPlay() {
+        if (skipped != null) {
+            //FIXME: this is for failed SA to skip temporarily, don't know why AI computation for mana fails, maybe due to auto mana compute?
+            for (SpellAbility sa : skipped) {
+                //System.out.println("Unskip: " + sa.toString() + " (" +  sa.getHostCard().getName() + ").");
+                sa.setSkip(false);
+            }
+        }
         CardCollection cards = ComputerUtilAbility.getAvailableCards(game, player);
         cards = ComputerUtilCard.dedupeCards(cards);
         List<SpellAbility> saList = Lists.newArrayList();
@@ -1547,8 +1574,12 @@ public class AiController {
 
         Iterables.removeIf(saList, spellAbility -> { //don't include removedAI cards if somehow the AI can play the ability or gain control of unsupported card
             // TODO allow when experimental profile?
-            return spellAbility.isLandAbility() || (spellAbility.getHostCard() != null && ComputerUtilCard.isCardRemAIDeck(spellAbility.getHostCard()));
+            return spellAbility.isLandAbility() || (spellAbility.getHostCard() != null && ComputerUtilCard.isCardRemAIDeck(spellAbility.getHostCard()))  || !spellAbility.canCastTiming(player);
         });
+        //removed skipped SA
+        skipped = Lists.newArrayList(Iterables.filter(saList, SpellAbility::isSkip));
+        if (!skipped.isEmpty())
+            saList.removeAll(skipped);
         //update LivingEndPlayer
         useLivingEnd = Iterables.any(player.getZone(ZoneType.Library), CardPredicates.nameEquals("Living End"));
 
@@ -1565,79 +1596,94 @@ public class AiController {
         if (all == null || all.isEmpty())
             return null;
 
-        try {
-            all.sort(ComputerUtilAbility.saEvaluator); // put best spells first
-            ComputerUtilAbility.sortCreatureSpells(all);
-        } catch (IllegalArgumentException ex) {
-            System.err.println(ex.getMessage());
-            String assertex = ComparatorUtil.verifyTransitivity(ComputerUtilAbility.saEvaluator, all);
-            Sentry.captureMessage(ex.getMessage() + "\nAssertionError [verifyTransitivity]: " + assertex);
-        }
         //avoid ComputerUtil.aiLifeInDanger in loops as it slows down a lot.. call this outside loops will generally be fast...
         boolean isLifeInDanger = useLivingEnd && ComputerUtil.aiLifeInDanger(player, true, 0);
+        List<CompletableFuture<Integer>> futures = new ArrayList<>();
+        Queue<SpellAbility> spells = new ConcurrentLinkedQueue<>();
         for (final SpellAbility sa : ComputerUtilAbility.getOriginalAndAltCostAbilities(all, player)) {
-            // Don't add Counterspells to the "normal" playcard lookups
+            futures.add(CompletableFuture.supplyAsync(()-> {
-            if (skipCounter && sa.getApi() == ApiType.Counter) {
+                // Don't add Counterspells to the "normal" playcard lookups
-                continue;
+                if (skipCounter && sa.getApi() == ApiType.Counter) {
-            }
+                    return 0;
-
-            if (sa.getHostCard().hasKeyword(Keyword.STORM)
-                    && sa.getApi() != ApiType.Counter // AI would suck at trying to deliberately proc a Storm counterspell
-                    && player.getZone(ZoneType.Hand).contains(Predicates.not(Predicates.or(CardPredicates.Presets.LANDS, CardPredicates.hasKeyword("Storm"))))) {
-                if (game.getView().getStormCount() < this.getIntProperty(AiProps.MIN_COUNT_FOR_STORM_SPELLS)) {
-                    // skip evaluating Storm unless we reached the minimum Storm count
-                    continue;
                 }
-            }
+
-            // living end AI decks
+                if (sa.getHostCard().hasKeyword(Keyword.STORM)
-            // TODO: generalize the implementation so that superfluous logic-specific checks for life, library size, etc. aren't needed
+                        && sa.getApi() != ApiType.Counter // AI would suck at trying to deliberately proc a Storm counterspell
-            AiPlayDecision aiPlayDecision = AiPlayDecision.CantPlaySa;
+                        && player.getZone(ZoneType.Hand).contains(Predicates.not(Predicates.or(CardPredicates.Presets.LANDS, CardPredicates.hasKeyword("Storm"))))) {
-            if (useLivingEnd) {
+                    if (game.getView().getStormCount() < this.getIntProperty(AiProps.MIN_COUNT_FOR_STORM_SPELLS)) {
-                if (sa.isCycling() && sa.canCastTiming(player) && player.getCardsIn(ZoneType.Library).size() >= 10) {
+                        // skip evaluating Storm unless we reached the minimum Storm count
-                    if (ComputerUtilCost.canPayCost(sa, player, sa.isTrigger())) {
+                        return 0;
-                        if (sa.getPayCosts() != null && sa.getPayCosts().hasSpecificCostType(CostPayLife.class)
+                    }
-                                && !player.cantLoseForZeroOrLessLife()
+                }
-                                && player.getLife() <= sa.getPayCosts().getCostPartByType(CostPayLife.class).getAbilityAmount(sa) * 2) {
+                // living end AI decks
-                            aiPlayDecision = AiPlayDecision.CantAfford;
+                // TODO: generalize the implementation so that superfluous logic-specific checks for life, library size, etc. aren't needed
+                AiPlayDecision aiPlayDecision = AiPlayDecision.CantPlaySa;
+                if (useLivingEnd) {
+                    if (sa.isCycling() && sa.canCastTiming(player) && player.getCardsIn(ZoneType.Library).size() >= 10) {
+                        if (ComputerUtilCost.canPayCost(sa, player, sa.isTrigger())) {
+                            if (sa.getPayCosts() != null && sa.getPayCosts().hasSpecificCostType(CostPayLife.class)
+                                    && !player.cantLoseForZeroOrLessLife()
+                                    && player.getLife() <= sa.getPayCosts().getCostPartByType(CostPayLife.class).getAbilityAmount(sa) * 2) {
+                                aiPlayDecision = AiPlayDecision.CantAfford;
+                            } else {
+                                aiPlayDecision = AiPlayDecision.WillPlay;
+                            }
+                        }
+                    } else if (sa.getHostCard().hasKeyword(Keyword.CASCADE)) {
+                        if (isLifeInDanger) { //needs more tune up for certain conditions
+                            aiPlayDecision = player.getCreaturesInPlay().size() >= 4 ? AiPlayDecision.CantPlaySa : AiPlayDecision.WillPlay;
+                        } else if (CardLists.filter(player.getZone(ZoneType.Graveyard).getCards(), CardPredicates.Presets.CREATURES).size() > 4) {
+                            if (player.getCreaturesInPlay().size() >= 4) // it's good minimum
+                                return 0;
+                            else if (!sa.getHostCard().isPermanent() && sa.canCastTiming(player) && ComputerUtilCost.canPayCost(sa, player, sa.isTrigger()))
+                                aiPlayDecision = AiPlayDecision.WillPlay;// needs tuneup for bad matchups like reanimator and other things to check on opponent graveyard
                         } else {
-                            aiPlayDecision = AiPlayDecision.WillPlay;
+                            return 0;
                         }
                     }
-                } else if (sa.getHostCard().hasKeyword(Keyword.CASCADE)) {
-                    if (isLifeInDanger) { //needs more tune up for certain conditions
-                        aiPlayDecision = player.getCreaturesInPlay().size() >= 4 ? AiPlayDecision.CantPlaySa : AiPlayDecision.WillPlay;
-                    } else if (CardLists.filter(player.getZone(ZoneType.Graveyard).getCards(), CardPredicates.Presets.CREATURES).size() > 4) {
-                        if (player.getCreaturesInPlay().size() >= 4) // it's good minimum
-                            continue;
-                        else if (!sa.getHostCard().isPermanent() && sa.canCastTiming(player) && ComputerUtilCost.canPayCost(sa, player, sa.isTrigger()))
-                            aiPlayDecision = AiPlayDecision.WillPlay;// needs tuneup for bad matchups like reanimator and other things to check on opponent graveyard
-                    } else {
-                        continue;
-                    }
                 }
-            }
 
-            sa.setActivatingPlayer(player, true);
+                sa.setActivatingPlayer(player, true);
-            SpellAbility root = sa.getRootAbility();
+                SpellAbility root = sa.getRootAbility();
 
-            if (root.isSpell() || root.isTrigger() || root.isReplacementAbility()) {
+                if (root.isSpell() || root.isTrigger() || root.isReplacementAbility()) {
-                sa.setLastStateBattlefield(game.getLastStateBattlefield());
+                    sa.setLastStateBattlefield(game.getLastStateBattlefield());
-                sa.setLastStateGraveyard(game.getLastStateGraveyard());
+                    sa.setLastStateGraveyard(game.getLastStateGraveyard());
-            }
+                }
-            //override decision for living end player
+                //override decision for living end player
-            AiPlayDecision opinion = useLivingEnd && AiPlayDecision.WillPlay.equals(aiPlayDecision) ? aiPlayDecision : canPlayAndPayFor(sa);
+                AiPlayDecision opinion = useLivingEnd && AiPlayDecision.WillPlay.equals(aiPlayDecision) ? aiPlayDecision : canPlayAndPayFor(sa);
 
-            // reset LastStateBattlefield
+                // reset LastStateBattlefield
-            sa.clearLastState();
+                sa.clearLastState();
-            // PhaseHandler ph = game.getPhaseHandler();
+                // PhaseHandler ph = game.getPhaseHandler();
-            // System.out.printf("Ai thinks '%s' of %s -> %s @ %s %s >>> \n", opinion, sa.getHostCard(), sa, Lang.getPossesive(ph.getPlayerTurn().getName()), ph.getPhase());
+                // System.out.printf("Ai thinks '%s' of %s -> %s @ %s %s >>> \n", opinion, sa.getHostCard(), sa, Lang.getPossesive(ph.getPlayerTurn().getName()), ph.getPhase());
 
-            if (opinion != AiPlayDecision.WillPlay)
+                if (opinion != AiPlayDecision.WillPlay)
-                continue;
+                    return 0;
 
-            return sa;
+                spells.add(sa);
+                return 0;
+            }));
         }
+        //timeout 5 seconds? even the AI don't acquire all, there should be SA to cast if valid
+        CompletableFuture<?>[] futuresArray = futures.toArray(new CompletableFuture<?>[0]);
+        CompletableFuture.allOf(futuresArray).completeOnTimeout(null, 5, TimeUnit.SECONDS).join();
+        futures.clear();
 
+        if (!spells.isEmpty()) {
+            List<SpellAbility> spellAbilities = new ArrayList<>(spells);
+            if (spellAbilities.size() == 1)
+                return spellAbilities.get(0);
+            try {
+                spellAbilities.sort(ComputerUtilAbility.saEvaluator); // put best spells first
+                ComputerUtilAbility.sortCreatureSpells(spellAbilities);
+            } catch (IllegalArgumentException ex) {
+                System.err.println(ex.getMessage());
+                String assertex = ComparatorUtil.verifyTransitivity(ComputerUtilAbility.saEvaluator, spellAbilities);
+                Sentry.captureMessage(ex.getMessage() + "\nAssertionError [verifyTransitivity]: " + assertex);
+            }
+            return spellAbilities.get(0);
+        }
         return null;
     }
 
@@ -2205,7 +2251,7 @@ public class AiController {
         result.addAll(activePlayerSAs);
 
         //need to reverse because of magic stack
-        Collections.reverse(result);
+        CollectionUtil.reverse(result);
         return result;
     }
 

forge-ai/src/main/java/forge/ai/AiCostDecision.java

@@ -15,6 +15,7 @@ import forge.game.spellability.SpellAbility;
 import forge.game.spellability.SpellAbilityStackInstance;
 import forge.game.zone.ZoneType;
 import forge.util.Aggregates;
+import forge.util.CollectionUtil;
 import forge.util.TextUtil;
 import forge.util.collect.FCollectionView;
 import org.apache.commons.lang3.ObjectUtils;
@@ -155,7 +156,7 @@ public class AiCostDecision extends CostDecisionMakerBase {
         List<Player> res = cost.getPotentialPlayers(player, ability);
         // I should only choose one of these right?
         // TODO Choose the "worst" player.
-        Collections.shuffle(res);
+        CollectionUtil.shuffle(res);
 
         return PaymentDecision.players(res.subList(0, 1));
     }
@@ -179,7 +180,7 @@ public class AiCostDecision extends CostDecisionMakerBase {
             CardCollection chosen = new CardCollection();
 
             CardLists.sortByCmcDesc(valid);
-            Collections.reverse(valid);
+            CollectionUtil.reverse(valid);
 
             int totalCMC = 0;
             for (Card card : valid) {

forge-ai/src/main/java/forge/ai/ComputerUtil.java

@@ -68,6 +68,7 @@ import forge.game.trigger.WrappedAbility;
 import forge.game.zone.Zone;
 import forge.game.zone.ZoneType;
 import forge.util.Aggregates;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.TextUtil;
 import forge.util.collect.FCollection;
@@ -87,6 +88,8 @@ public class ComputerUtil {
     }
     public static boolean handlePlayingSpellAbility(final Player ai, SpellAbility sa, final Game game, Runnable chooseTargets) {
         final Card source = sa.getHostCard();
+        final Card host = sa.getHostCard();
+        final Zone hz = host.isCopiedSpell() ? null : host.getZone();
         source.setSplitStateToPlayAbility(sa);
 
         if (sa.isSpell() && !source.isCopiedSpell()) {
@@ -144,8 +147,15 @@ public class ComputerUtil {
                 return true;
             }
         }
-        //Should not arrive here
+        // FIXME: Should not arrive here, though the card seems to be stucked on stack zone and invalidated and nowhere to be found, try to put back to original zone and maybe try to cast again if possible at later time?
-        System.out.println("AI failed to play " + sa.getHostCard());
+        System.out.println("[" + sa.getActivatingPlayer() + "] AI failed to play " + sa.getHostCard() + " [" + sa.getHostCard().getZone() + "]");
+        sa.setSkip(true);
+        if (host != null && hz != null) {
+            Card c = game.getAction().moveTo(hz.getZoneType(), host, null, null);
+            for (SpellAbility csa : c.getSpellAbilities()) {
+                csa.setSkip(true);
+            }
+        }
         return false;
     }
 
@@ -673,7 +683,7 @@ public class ComputerUtil {
 
         // FIXME: This is suboptimal, maybe implement a single comparator that'll take care of all of this?
         CardLists.sortByCmcDesc(typeList);
-        Collections.reverse(typeList);
+        CollectionUtil.reverse(typeList);
 
 
         // TODO AI needs some improvements here
@@ -727,7 +737,7 @@ public class ComputerUtil {
 
             // FIXME: This is suboptimal, maybe implement a single comparator that'll take care of all of this?
             CardLists.sortByCmcDesc(typeList);
-            Collections.reverse(typeList);
+            CollectionUtil.reverse(typeList);
             typeList.sort((a, b) -> {
                 if (!a.isInPlay() && b.isInPlay()) return -1;
                 else if (!b.isInPlay() && a.isInPlay()) return 1;
@@ -757,7 +767,7 @@ public class ComputerUtil {
         final CardCollection list = new CardCollection();
 
         if (zone != ZoneType.Hand) {
-            Collections.reverse(typeList);
+            CollectionUtil.reverse(typeList);
         }
 
         for (int i = 0; i < amount; i++) {
@@ -807,7 +817,7 @@ public class ComputerUtil {
             typeList.remove(activate);
         }
         ComputerUtilCard.sortByEvaluateCreature(typeList);
-        Collections.reverse(typeList);
+        CollectionUtil.reverse(typeList);
 
         final CardCollection tapList = new CardCollection();
 
@@ -1759,7 +1769,7 @@ public class ComputerUtil {
 
         // align threatened with resolve order
         // matters if stack contains multiple activations (e.g. Temur Sabertooth)
-        Collections.reverse(objects);
+        CollectionUtil.reverse(objects);
         return objects;
     }
 

forge-ai/src/main/java/forge/ai/ComputerUtilMana.java

@@ -38,6 +38,7 @@ import forge.game.trigger.Trigger;
 import forge.game.trigger.TriggerType;
 import forge.game.zone.Zone;
 import forge.game.zone.ZoneType;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.TextUtil;
 import org.apache.commons.lang3.StringUtils;
@@ -466,18 +467,18 @@ public class ComputerUtilMana {
     public static String predictManafromSpellAbility(SpellAbility saPayment, Player ai, ManaCostShard toPay) {
         Card hostCard = saPayment.getHostCard();
 
-        String manaProduced = predictManaReplacement(saPayment, ai, toPay);
+        StringBuilder manaProduced = new StringBuilder(predictManaReplacement(saPayment, ai, toPay));
-        String originalProduced = manaProduced;
+        String originalProduced = manaProduced.toString();
 
         if (originalProduced.isEmpty()) {
-            return manaProduced;
+            return manaProduced.toString();
         }
 
         // Run triggers like Nissa
         final Map<AbilityKey, Object> runParams = AbilityKey.mapFromCard(hostCard);
         runParams.put(AbilityKey.Activator, ai); // assuming AI would only ever gives itself mana
         runParams.put(AbilityKey.AbilityMana, saPayment);
-        runParams.put(AbilityKey.Produced, manaProduced);
+        runParams.put(AbilityKey.Produced, manaProduced.toString());
         for (Trigger tr : ai.getGame().getTriggerHandler().getActiveTrigger(TriggerType.TapsForMana, runParams)) {
             SpellAbility trSA = tr.ensureAbility();
             if (trSA == null) {
@@ -489,7 +490,7 @@ public class ComputerUtilMana {
                 if (produced.equals("Chosen")) {
                     produced = MagicColor.toShortString(trSA.getHostCard().getChosenColor());
                 }
-                manaProduced += " " + StringUtils.repeat(produced, " ", pAmount);
+                manaProduced.append(" ").append(StringUtils.repeat(produced, " ", pAmount));
             } else if (ApiType.ManaReflected.equals(trSA.getApi())) {
                 final String colorOrType = trSA.getParamOrDefault("ColorOrType", "Color");
                 // currently Color or Type, Type is colors + colorless
@@ -498,11 +499,11 @@ public class ComputerUtilMana {
                 if (reflectProperty.equals("Produced") && !originalProduced.isEmpty()) {
                     // check if a colorless shard can be paid from the trigger
                     if (toPay.equals(ManaCostShard.COLORLESS) && colorOrType.equals("Type") && originalProduced.contains("C")) {
-                        manaProduced += " " + "C";
+                        manaProduced.append(" " + "C");
                     } else if (originalProduced.length() == 1) {
                         // if length is only one, and it either is equal C == Type
                         if (colorOrType.equals("Type") || !originalProduced.equals("C")) {
-                            manaProduced += " " + originalProduced;
+                            manaProduced.append(" ").append(originalProduced);
                         }
                     } else {
                         // should it look for other shards too?
@@ -510,7 +511,7 @@ public class ComputerUtilMana {
                         for (String s : originalProduced.split(" ")) {
                             if (colorOrType.equals("Type") || !s.equals("C") && toPay.canBePaidWithManaOfColor(MagicColor.fromName(s))) {
                                 found = true;
-                                manaProduced += " " + s;
+                                manaProduced.append(" ").append(s);
                                 break;
                             }
                         }
@@ -518,7 +519,7 @@ public class ComputerUtilMana {
                         if (!found) {
                             for (String s : originalProduced.split(" ")) {
                                 if (colorOrType.equals("Type") || !s.equals("C")) {
-                                    manaProduced += " " + s;
+                                    manaProduced.append(" ").append(s);
                                     break;
                                 }
                             }
@@ -527,7 +528,7 @@ public class ComputerUtilMana {
                 }
             }
         }
-        return manaProduced;
+        return manaProduced.toString();
     }
 
     public static CardCollection getManaSourcesToPayCost(final ManaCostBeingPaid cost, final SpellAbility sa, final Player ai) {
@@ -826,7 +827,8 @@ public class ComputerUtilMana {
             if (test) {
                 resetPayment(paymentList);
             } else {
-                System.out.println("ComputerUtilMana: payManaCost() cost was not paid for " + sa.toString() + " (" +  sa.getHostCard().getName() + "). Didn't find what to pay for " + toPay);
+                System.out.println("ComputerUtilMana: payManaCost() cost was not paid for " + sa + " (" +  sa.getHostCard().getName() + "). Didn't find what to pay for " + toPay);
+                sa.setSkip(true);
             }
             return false;
         }
@@ -1518,11 +1520,11 @@ public class ComputerUtilMana {
         sortedManaSources.addAll(sortedManaSources.size(), anyColorManaSources);
         //use better creatures later
         ComputerUtilCard.sortByEvaluateCreature(otherManaSources);
-        Collections.reverse(otherManaSources);
+        CollectionUtil.reverse(otherManaSources);
         sortedManaSources.addAll(sortedManaSources.size(), otherManaSources);
         // This should be things like sacrifice other stuff.
         ComputerUtilCard.sortByEvaluateCreature(useLastManaSources);
-        Collections.reverse(useLastManaSources);
+        CollectionUtil.reverse(useLastManaSources);
         sortedManaSources.addAll(sortedManaSources.size(), useLastManaSources);
 
         if (DEBUG_MANA_PAYMENT) {

forge-ai/src/main/java/forge/ai/PlayerControllerAi.java

@@ -40,6 +40,7 @@ import forge.game.zone.PlayerZone;
 import forge.game.zone.ZoneType;
 import forge.item.PaperCard;
 import forge.util.Aggregates;
+import forge.util.CollectionUtil;
 import forge.util.ITriggerEvent;
 import forge.util.MyRandom;
 import forge.util.collect.FCollection;
@@ -650,7 +651,7 @@ public class PlayerControllerAi extends PlayerController {
             if(source == null || !source.hasParam("LibraryPosition")
                     || AbilityUtils.calculateAmount(source.getHostCard(), source.getParam("LibraryPosition"), source) >= 0) {
                 //Cards going to the top of a deck are returned in reverse order.
-                Collections.reverse(reordered);
+                CollectionUtil.reverse(reordered);
             }
 
             assert(reordered.size() == cards.size());
@@ -1565,8 +1566,13 @@ public class PlayerControllerAi extends PlayerController {
             }
         }
 
-        int i = MyRandom.getRandom().nextInt(dungeonNames.size());
+        try {
-        return Card.fromPaperCard(dungeonCards.get(i), ai);
+            // if this fail somehow add fallback to get any from dungeonCards
+            int i = MyRandom.getRandom().nextInt(dungeonNames.size());
+            return Card.fromPaperCard(dungeonCards.get(i), ai);
+        } catch (Exception e) {
+            return Card.fromPaperCard(Aggregates.random(dungeonCards), ai);
+        }
     }
 
     @Override

forge-ai/src/main/java/forge/ai/SpecialCardAi.java

@@ -47,6 +47,7 @@ import forge.game.staticability.StaticAbility;
 import forge.game.trigger.Trigger;
 import forge.game.zone.ZoneType;
 import forge.util.Aggregates;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.TextUtil;
 import forge.util.maps.LinkedHashMapToAmount;
@@ -319,7 +320,7 @@ public class SpecialCardAi {
             best = ComputerUtilCard.getBestCreatureAI(cardlist);
             if (best == null) {
                 // If nothing on the battlefield has a nonmana ability choose something
-                Collections.shuffle(cardlist);
+                CollectionUtil.shuffle(cardlist);
                 best = cardlist.getFirst();
             }
 

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

@@ -1,6 +1,5 @@
 package forge.ai.ability;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -18,6 +17,7 @@ import forge.game.player.Player;
 import forge.game.spellability.AbilitySub;
 import forge.game.spellability.SpellAbility;
 import forge.util.Aggregates;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.collect.FCollection;
 
@@ -52,7 +52,7 @@ public class CharmAi extends SpellAbilityAi {
         } else {
             // only randomize if not all possible together
             if (num < choices.size()) {
-                Collections.shuffle(choices);
+                CollectionUtil.shuffle(choices);
             }
 
             /*
@@ -101,7 +101,7 @@ public class CharmAi extends SpellAbilityAi {
         // Pawprint
         final int pawprintLimit = sa.hasParam("Pawprint") ? AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("Pawprint"), sa) : 0;
         if (pawprintLimit > 0) {
-            Collections.reverse(choices); // try to pay for the more expensive subs first
+            CollectionUtil.reverse(choices); // try to pay for the more expensive subs first
         }
         int pawprintAmount = 0;
 

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

@@ -16,8 +16,8 @@ import forge.game.player.PlayerPredicates;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
 import forge.util.Aggregates;
+import forge.util.CollectionUtil;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -226,7 +226,7 @@ public class ChooseCardAi extends SpellAbilityAi {
                 choice = ComputerUtilCard.getWorstAI(options);
             } else {
                 CardLists.sortByCmcDesc(creats);
-                Collections.reverse(creats);
+                CollectionUtil.reverse(creats);
                 choice = creats.get(0);
             }
         } else if ("NegativePowerFirst".equals(logic)) {

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

@@ -1,6 +1,5 @@
 package forge.ai.ability;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -10,6 +9,7 @@ import forge.ai.SpellAbilityAi;
 import forge.game.card.Card;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
+import forge.util.CollectionUtil;
 
 public class ChooseCompanionAi extends SpellAbilityAi {
 
@@ -23,7 +23,7 @@ public class ChooseCompanionAi extends SpellAbilityAi {
             return null;
         }
 
-        Collections.shuffle(cards);
+        CollectionUtil.shuffle(cards);
         return cards.get(0);
     }
 }

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

@@ -1,6 +1,5 @@
 package forge.ai.ability;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -23,6 +22,7 @@ import forge.game.player.PlayerCollection;
 import forge.game.player.PlayerPredicates;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 
 public class DiscardAi extends SpellAbilityAi {
@@ -148,7 +148,7 @@ public class DiscardAi extends SpellAbilityAi {
 
     private boolean discardTargetAI(final Player ai, final SpellAbility sa) {
         final PlayerCollection opps = ai.getOpponents();
-        Collections.shuffle(opps);
+        CollectionUtil.shuffle(opps);
         for (Player opp : opps) {
             if (opp.getCardsIn(ZoneType.Hand).isEmpty() && !ComputerUtil.activateForCost(sa, ai)) {
                 continue;

forge-ai/src/main/java/forge/ai/simulation/SimulationController.java

@@ -1,7 +1,6 @@
 package forge.ai.simulation;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import forge.ai.simulation.GameStateEvaluator.Score;
@@ -9,6 +8,7 @@ import forge.game.GameObject;
 import forge.game.card.Card;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
+import forge.util.CollectionUtil;
 
 public class SimulationController {
     private static boolean DEBUG = false;
@@ -106,7 +106,7 @@ public class SimulationController {
             sequence.add(current);
             current = current.prevDecision;
         }
-        Collections.reverse(sequence);
+        CollectionUtil.reverse(sequence);
         // Merge targets & choices into their parents.
         int writeIndex = 0;
         for (int i = 0; i < sequence.size(); i++) {

forge-core/src/main/java/forge/StaticData.java

@@ -7,6 +7,7 @@ import forge.card.CardRules;
 import forge.card.PrintSheet;
 import forge.item.*;
 import forge.token.TokenDb;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.ImageUtil;
 import forge.util.TextUtil;
@@ -195,7 +196,7 @@ public class StaticData {
                 sortedEditions.add(set);
             }
             Collections.sort(sortedEditions);
-            Collections.reverse(sortedEditions); //put newer sets at the top
+            CollectionUtil.reverse(sortedEditions); //put newer sets at the top
         }
         return sortedEditions;
     }

forge-core/src/main/java/forge/card/CardDb.java

@@ -28,6 +28,7 @@ import forge.deck.generation.IDeckGenPool;
 import forge.item.IPaperCard;
 import forge.item.PaperCard;
 import forge.util.CollectionSuppliers;
+import forge.util.CollectionUtil;
 import forge.util.Lang;
 import forge.util.TextUtil;
 import forge.util.lang.LangEnglish;
@@ -842,7 +843,7 @@ public final class CardDb implements ICardDatabase, IDeckGenPool {
         if (acceptedEditions.size() > 1) {
             Collections.sort(acceptedEditions);  // CardEdition correctly sort by (release) date
             if (artPref.latestFirst)
-                Collections.reverse(acceptedEditions);  // newest editions first
+                CollectionUtil.reverse(acceptedEditions);  // newest editions first
         }
 
         final Iterator<CardEdition> editionIterator = acceptedEditions.iterator();

forge-core/src/main/java/forge/card/CardEdition.java

@@ -490,7 +490,7 @@ public final class CardEdition implements Comparable<CardEdition> {
             return null;
         }
 
-        Collections.shuffle(boosterTypes);
+        CollectionUtil.shuffle(boosterTypes);
         return boosterTypes.get(0);
     }
 
@@ -802,7 +802,7 @@ public final class CardEdition implements Comparable<CardEdition> {
         public Iterable<CardEdition> getOrderedEditions() {
             List<CardEdition> res = Lists.newArrayList(this);
             Collections.sort(res);
-            Collections.reverse(res);
+            CollectionUtil.reverse(res);
             return res;
         }
 

forge-core/src/main/java/forge/deck/CardPool.java

@@ -27,6 +27,7 @@ import forge.card.CardEdition;
 import forge.item.IPaperCard;
 import forge.item.PaperCard;
 import forge.util.CollectionSuppliers;
+import forge.util.CollectionUtil;
 import forge.util.ItemPool;
 import forge.util.ItemPoolSorter;
 import forge.util.MyRandom;
@@ -349,7 +350,7 @@ public class CardPool extends ItemPool<PaperCard> {
         pivotCandidates.sort(CardEdition::compareTo);
         boolean searchPolicyAndPoolAreCompliant = isLatestCardArtPreference == this.isModern();
         if (!searchPolicyAndPoolAreCompliant)
-            Collections.reverse(pivotCandidates);  // reverse to have latest-first.
+            CollectionUtil.reverse(pivotCandidates);  // reverse to have latest-first.
         return pivotCandidates.get(0);
     }
 

forge-core/src/main/java/forge/item/generation/BoosterGenerator.java

@@ -28,6 +28,7 @@ import forge.card.CardEdition.FoilType;
 import forge.item.*;
 import forge.item.IPaperCard.Predicates.Presets;
 import forge.util.Aggregates;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.TextUtil;
 import org.apache.commons.lang3.StringUtils;
@@ -58,7 +59,7 @@ public class BoosterGenerator {
     }
 
     private static PaperCard generateFoilCard(List<PaperCard> cardList) {
-        Collections.shuffle(cardList, MyRandom.getRandom());
+        CollectionUtil.shuffle(cardList, MyRandom.getRandom());
         PaperCard randomCard = cardList.get(0);
         return randomCard.getFoiled();
     }

forge-core/src/main/java/forge/util/CollectionUtil.java

@@ -0,0 +1,65 @@
+package forge.util;
+
+import forge.util.collect.FCollection;
+
+import java.util.*;
+
+public class CollectionUtil {
+    public static <T> void shuffle(List<T> list) {
+        shuffle(list, MyRandom.getRandom());
+    }
+
+    public static <T> void shuffle(List<T> list, Random random) {
+        if (list instanceof FCollection) {
+            //FCollection -> copyonwritearraylist is not compatible, use different method
+            shuffleList(list, random);
+        } else {
+            //use Collections -> shuffle(LIST, RANDOM) since it's not FCollection
+            Collections.shuffle(list, random);
+        }
+    }
+
+    public static <T> void shuffleList(List<T> a, Random r) {
+        int n = a.size();
+        for (int i = 0; i < n; i++) {
+            int change = i + r.nextInt(n - i);
+            swap(a, i, change);
+        }
+    }
+
+    private static <T> void swap(List<T> a, int i, int change) {
+        T helper = a.get(i);
+        a.set(i, a.get(change));
+        a.set(change, helper);
+    }
+
+    public static <T> void reverse(List<T> list) {
+        if (list == null || list.isEmpty())
+            return;
+        if (list instanceof FCollection) {
+            //FCollection -> copyonwritearraylist is not compatible, use different method
+            reverseWithRecursion(list, 0, list.size() - 1);
+        } else {
+            Collections.reverse(list);
+        }
+    }
+
+    public static <T> void reverseWithRecursion(List<T> list) {
+        if (list.size() > 1) {
+            T value = list.remove(0);
+            reverseWithRecursion(list);
+            list.add(value);
+        }
+    }
+
+    public static <T> void reverseWithRecursion(List<T> list, int startIndex, int lastIndex) {
+        if (startIndex < lastIndex) {
+            T t = list.get(lastIndex);
+            list.set(lastIndex, list.get(startIndex));
+            list.set(startIndex, t);
+            startIndex++;
+            lastIndex--;
+            reverseWithRecursion(list, startIndex, lastIndex);
+        }
+    }
+}

forge-core/src/main/java/forge/util/collect/FCollection.java

@@ -6,7 +6,6 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.ListIterator;
 import java.util.NoSuchElementException;
@@ -43,12 +42,38 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
     /**
      * The {@link Set} representation of this collection.
      */
-    private final Set<T> set = Sets.newHashSet();
+    private Set<T> SET;
+    private Set<T> set() {
+        Set<T> result = SET;
+        if (result == null) {
+            synchronized (this) {
+                result = SET;
+                if (result == null) {
+                    result = Sets.newConcurrentHashSet();
+                    SET = result;
+                }
+            }
+        }
+        return SET;
+    }
 
     /**
      * The {@link List} representation of this collection.
      */
-    private final LinkedList<T> list = Lists.newLinkedList();
+    private List<T> LIST;
+    private List<T> list() {
+        List<T> result = LIST;
+        if (result == null) {
+            synchronized (this) {
+                result = LIST;
+                if (result == null) {
+                    result = Lists.newCopyOnWriteArrayList();
+                    LIST = result;
+                }
+            }
+        }
+        return LIST;
+    }
 
     /**
      * Create an empty {@link FCollection}.
@@ -139,7 +164,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public int hashCode() {
-        return list.hashCode();
+        return list().hashCode();
     }
 
     /**
@@ -149,7 +174,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public String toString() {
-        return list.toString();
+        return list().toString();
     }
 
     /**
@@ -158,7 +183,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public final FCollection<T> clone() {
-        return new FCollection<>(list);
+        return new FCollection<>(list());
     }
 
     /**
@@ -169,7 +194,10 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public T getFirst() {
-        return list.getFirst();
+        if (list().isEmpty())
+            return null;
+        return list().get(0);
+        //return list.getFirst();
     }
 
     /**
@@ -180,7 +208,10 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public T getLast() {
-        return list.getLast();
+        if (list().isEmpty())
+            return null;
+        return list().get(list().size() - 1);
+        //return list.getLast();
     }
 
     /**
@@ -188,7 +219,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public int size() {
-        return set.size();
+        return set().size();
     }
 
     /**
@@ -196,11 +227,11 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public boolean isEmpty() {
-        return set.isEmpty();
+        return set().isEmpty();
     }
 
     public Set<T> asSet() {
-        return set;
+        return set();
     }
 
     /**
@@ -211,7 +242,9 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public boolean contains(final Object o) {
-        return set.contains(o);
+        if (o == null)
+            return false;
+        return set().contains(o);
     }
 
     /**
@@ -219,7 +252,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public Iterator<T> iterator() {
-        return list.iterator();
+        return list().iterator();
     }
 
     /**
@@ -227,7 +260,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public Object[] toArray() {
-        return list.toArray();
+        return list().toArray();
     }
 
     /**
@@ -236,7 +269,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
     @Override
     @SuppressWarnings("hiding")
     public <T> T[] toArray(final T[] a) {
-        return list.toArray(a);
+        return list().toArray(a);
     }
 
     /**
@@ -248,8 +281,10 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public boolean add(final T e) {
-        if (set.add(e)) {
+        if (e == null)
-            list.add(e);
+            return false;
+        if (set().add(e)) {
+            list().add(e);
             return true;
         }
         return false;
@@ -264,8 +299,10 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public boolean remove(final Object o) {
-        if (set.remove(o)) {
+        if (o == null)
-            list.remove(o);
+            return false;
+        if (set().remove(o)) {
+            list().remove(o);
             return true;
         }
         return false;
@@ -273,8 +310,8 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
 
     @Override
     public boolean removeIf(Predicate<? super T> filter) {
-        if (list.removeIf(filter)) {
+        if (list().removeIf(filter)) {
-            set.removeIf(filter);
+            set().removeIf(filter);
             return true;
         }
         return false;
@@ -285,7 +322,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public boolean containsAll(final Collection<?> c) {
-        return set.containsAll(c);
+        return set().containsAll(c);
     }
 
     /**
@@ -307,6 +344,8 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     public boolean addAll(final Iterable<? extends T> i) {
         boolean changed = false;
+        if (i == null)
+            return false;
         for (final T e : i) {
             changed |= add(e);
         }
@@ -370,6 +409,8 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     public boolean removeAll(final Iterable<?> c) {
         boolean changed = false;
+        if (c == null)
+            return false;
         for (final Object o : c) {
             changed |= remove(o);
         }
@@ -381,8 +422,8 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public boolean retainAll(final Collection<?> c) {
-        if (set.retainAll(c)) {
+        if (set().retainAll(c)) {
-            list.retainAll(c);
+            list().retainAll(c);
             return true;
         }
         return false;
@@ -393,9 +434,9 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public void clear() {
-        if (set.isEmpty()) { return; }
+        if (set().isEmpty()) { return; }
-        set.clear();
+        set().clear();
-        list.clear();
+        list().clear();
     }
 
     /**
@@ -403,7 +444,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public T get(final int index) {
-        return list.get(index);
+        return list().get(index);
     }
 
     /**
@@ -413,7 +454,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public T set(final int index, final T element) { //assume this isn't called except when changing list order, so don't worry about updating set
-        return list.set(index, element);
+        return list().set(index, element);
     }
 
     /**
@@ -434,12 +475,12 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      * @return whether this collection changed as a result of this method call.
      */
     private boolean insert(int index, final T element) {
-        if (set.add(element)) {
+        if (set().add(element)) {
-            list.add(index, element);
+            list().add(index, element);
             return true;
         }
         //re-position in list if needed
-        final int oldIndex = list.indexOf(element);
+        final int oldIndex = list().indexOf(element);
         if (index == oldIndex) {
             return false;
         }
@@ -447,8 +488,8 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
         if (index > oldIndex) {
             index--; //account for being removed
         }
-        list.remove(oldIndex);
+        list().remove(oldIndex);
-        list.add(index, element);
+        list().add(index, element);
         return true;
     }
 
@@ -457,9 +498,9 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public T remove(final int index) {
-        final T removedItem = list.remove(index);
+        final T removedItem = list().remove(index);
         if (removedItem != null) {
-            set.remove(removedItem);
+            set().remove(removedItem);
         }
         return removedItem;
     }
@@ -469,7 +510,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public int indexOf(final Object o) {
-        return list.indexOf(o);
+        return list().indexOf(o);
     }
 
     /**
@@ -477,7 +518,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public int lastIndexOf(final Object o) {
-        return list.lastIndexOf(o);
+        return list().lastIndexOf(o);
     }
 
     /**
@@ -485,7 +526,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public ListIterator<T> listIterator() {
-        return list.listIterator();
+        return list().listIterator();
     }
 
     /**
@@ -493,7 +534,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public ListIterator<T> listIterator(final int index) {
-        return list.listIterator(index);
+        return list().listIterator(index);
     }
 
     /**
@@ -506,7 +547,7 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public List<T> subList(final int fromIndex, final int toIndex) {
-        return ImmutableList.copyOf(list.subList(fromIndex, toIndex));
+        return ImmutableList.copyOf(list().subList(fromIndex, toIndex));
     }
 
     /**
@@ -525,7 +566,11 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      * {@inheritDoc}
      */
     public void sort(final Comparator<? super T> comparator) {
-        list.sort(comparator);
+        try {
+            list().sort(comparator);
+        } catch (Exception e) {
+            System.err.println("FCollection failed to sort: \n" + comparator + "\n" + e.getMessage());
+        }
     }
 
     /**
@@ -533,8 +578,9 @@ public class FCollection<T> implements List<T>, /*Set<T>,*/ FCollectionView<T>,
      */
     @Override
     public Iterable<T> threadSafeIterable() {
+        return list();
         //create a new linked list for iterating to make it thread safe and avoid concurrent modification exceptions
-        return Iterables.unmodifiableIterable(new LinkedList<>(list));
+        //return Iterables.unmodifiableIterable(new LinkedList<>(list));
     }
 
     @Override

forge-game/src/main/java/forge/game/Game.java

@@ -47,6 +47,7 @@ import forge.game.zone.Zone;
 import forge.game.zone.ZoneType;
 import forge.trackable.Tracker;
 import forge.util.Aggregates;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.Visitor;
 import forge.util.collect.FCollection;
@@ -398,7 +399,7 @@ public class Game {
             return ingamePlayers;
         }
         final PlayerCollection players = new PlayerCollection(ingamePlayers);
-        Collections.reverse(players);
+        CollectionUtil.reverse(players);
         return players;
     }
 
@@ -418,7 +419,7 @@ public class Game {
         final PlayerCollection players = new PlayerCollection(ingamePlayers);
         players.remove(phaseHandler.getPlayerTurn());
         if (!getTurnOrder().isDefaultDirection()) {
-            Collections.reverse(players);
+            CollectionUtil.reverse(players);
         }
         return players;
     }

forge-game/src/main/java/forge/game/GameAction.java

@@ -2041,7 +2041,7 @@ public class GameAction {
 
         //shuffle
         List<Card> shuffledCards = Lists.newArrayList(p1.getZone(ZoneType.Library).getCards().threadSafeIterable());
-        Collections.shuffle(shuffledCards);
+        CollectionUtil.shuffle(shuffledCards);
 
         //check a second hand
         List<Card> hand2 = shuffledCards.subList(0,p1.getMaxHandSize());
@@ -2171,7 +2171,7 @@ public class GameAction {
 
         if (!powerPlayers.isEmpty()) {
             List<Player> players = Lists.newArrayList(powerPlayers);
-            Collections.shuffle(players, MyRandom.getRandom());
+            CollectionUtil.shuffle(players, MyRandom.getRandom());
             return players.get(0);
         }
 
@@ -2409,7 +2409,7 @@ public class GameAction {
             int numLookedAt = 0;
             if (toTop != null) {
                 numLookedAt += toTop.size();
-                Collections.reverse(toTop); // reverse to get the correct order
+                CollectionUtil.reverse(toTop); // reverse to get the correct order
                 for (Card c : toTop) {
                     moveToLibrary(c, cause, null);
                 }

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

@@ -17,6 +17,7 @@ import forge.game.player.PlayerView;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
 import forge.util.CardTranslation;
+import forge.util.CollectionUtil;
 import forge.util.Lang;
 import forge.util.Localizer;
 import forge.util.TextUtil;
@@ -174,7 +175,7 @@ public class DigEffect extends SpellAbilityEffect {
             CardCollection all = new CardCollection(p.getCardsIn(srcZone));
 
             if (sa.hasParam("FromBottom")) {
-                Collections.reverse(all);
+                CollectionUtil.reverse(all);
             }
 
             int numToDig = Math.min(digNum, all.size());
@@ -356,7 +357,7 @@ public class DigEffect extends SpellAbilityEffect {
                 if (sa.hasParam("ForgetOtherRemembered")) {
                     host.clearRemembered();
                 }
-                Collections.reverse(movedCards);
+                CollectionUtil.reverse(movedCards);
 
                 if (destZone1.equals(ZoneType.Battlefield) || destZone1.equals(ZoneType.Library)) {
                     if (sa.hasParam("GainControl")) {

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

@@ -1,6 +1,5 @@
 package forge.game.ability.effects;
 
-import java.util.Collections;
 import java.util.Map;
 
 import com.google.common.collect.Maps;
@@ -18,6 +17,7 @@ import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.PlayerZone;
 import forge.game.zone.ZoneType;
+import forge.util.CollectionUtil;
 import forge.util.Localizer;
 
 public class DigMultipleEffect extends SpellAbilityEffect {
@@ -160,7 +160,7 @@ public class DigMultipleEffect extends SpellAbilityEffect {
                     }
                     if (libraryPosition2 != -1) {
                         // Closest to top
-                        Collections.reverse(afterOrder);
+                        CollectionUtil.reverse(afterOrder);
                     }
                     for (final Card c : afterOrder) {
                         final ZoneType origin = c.getZone().getZoneType();

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

@@ -1,6 +1,5 @@
 package forge.game.ability.effects;
 
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.Map;
 
@@ -18,6 +17,7 @@ import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.PlayerZone;
 import forge.game.zone.ZoneType;
+import forge.util.CollectionUtil;
 import forge.util.Lang;
 import forge.util.Localizer;
 import forge.util.MyRandom;
@@ -261,7 +261,7 @@ public class DigUntilEffect extends SpellAbilityEffect {
             }
 
             if (sa.hasParam("RevealRandomOrder")) {
-                Collections.shuffle(revealed, MyRandom.getRandom());
+                CollectionUtil.shuffle(revealed, MyRandom.getRandom());
             }
 
             if (sa.hasParam("NoMoveRevealed") || sequential) {

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

@@ -11,6 +11,7 @@ import forge.game.card.CardZoneTable;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
+import forge.util.CollectionUtil;
 import forge.util.Localizer;
 
 import java.util.*;
@@ -51,7 +52,7 @@ import java.util.*;
          CardCollection drafted = new CardCollection();
 
          for (int i = 0; i < numToDraft; i++) {
-             Collections.shuffle(spellbook);
+             CollectionUtil.shuffle(spellbook);
              List<Card> draftOptions = new ArrayList<>();
              for (String name : spellbook.subList(0, 3)) {
                  // Cardnames that include "," must use ";" instead in Spellbook$ (i.e. Tovolar; Dire Overlord)

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

@@ -1,6 +1,5 @@
 package forge.game.ability.effects;
 
-import java.util.Collections;
 import java.util.List;
 
 import forge.game.ability.SpellAbilityEffect;
@@ -9,6 +8,7 @@ import forge.game.card.CardCollectionView;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
+import forge.util.CollectionUtil;
 import forge.util.Lang;
 import forge.util.MyRandom;
 
@@ -34,7 +34,7 @@ public class ReorderZoneEffect extends SpellAbilityEffect {
 
             CardCollection list = new CardCollection(p.getCardsIn(zone));
             if (shuffle) {
-                Collections.shuffle(list, MyRandom.getRandom());
+                CollectionUtil.shuffle(list, MyRandom.getRandom());
                 p.getZone(zone).setCards(list);
             } else {
                 CardCollectionView orderedCards = p.getController().orderMoveToZoneList(list, zone, sa);

forge-game/src/main/java/forge/game/card/CardLists.java

@@ -32,6 +32,7 @@ import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.TargetRestrictions;
 import forge.game.staticability.StaticAbilityCrewValue;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.collect.FCollectionView;
 
@@ -153,7 +154,7 @@ public class CardLists {
     }
 
     public static void shuffle(List<Card> list) {
-        Collections.shuffle(list, MyRandom.getRandom());
+        CollectionUtil.shuffle(list, MyRandom.getRandom());
     }
 
     public static CardCollection filterControlledBy(Iterable<Card> cardList, Player player) {

forge-game/src/main/java/forge/game/card/CardProperty.java

@@ -28,6 +28,7 @@ import forge.game.spellability.SpellAbilityStackInstance;
 import forge.game.zone.Zone;
 import forge.game.zone.ZoneType;
 import forge.item.PaperCard;
+import forge.util.CollectionUtil;
 import forge.util.Expressions;
 import forge.util.TextUtil;
 import forge.util.collect.FCollection;
@@ -623,13 +624,13 @@ public class CardProperty {
             }
         } else if (property.startsWith("TopGraveyardCreature")) {
             CardCollection cards = CardLists.filter(card.getOwner().getCardsIn(ZoneType.Graveyard), CardPredicates.Presets.CREATURES);
-            Collections.reverse(cards);
+            CollectionUtil.reverse(cards);
             if (cards.isEmpty() || !card.equals(cards.get(0))) {
                 return false;
             }
         } else if (property.startsWith("TopGraveyard")) {
             final CardCollection cards = new CardCollection(card.getOwner().getCardsIn(ZoneType.Graveyard));
-            Collections.reverse(cards);
+            CollectionUtil.reverse(cards);
             if (property.substring(12).matches("[0-9][0-9]?")) {
                 int n = Integer.parseInt(property.substring(12));
                 int num = Math.min(n, cards.size());
@@ -665,7 +666,7 @@ public class CardProperty {
             if (property.startsWith("BottomLibrary_")) {
                 cards = CardLists.getValidCards(cards, property.substring(14), sourceController, source, spellAbility);
             }
-            Collections.reverse(cards);
+            CollectionUtil.reverse(cards);
             if (cards.isEmpty() || !card.equals(cards.get(0))) {
                 return false;
             }

forge-game/src/main/java/forge/game/combat/Combat.java

@@ -53,19 +53,122 @@ public class Combat {
     private boolean legacyOrderCombatants;
     private AttackConstraints attackConstraints;
     // Defenders, as they are attacked by hostile forces
-    private final FCollection<GameEntity> attackableEntries = new FCollection<>();
+    private FCollection<GameEntity> _attackableEntries;
-
+    private FCollection<GameEntity> attackableEntries() {
+        FCollection<GameEntity> result = _attackableEntries;
+        if (result == null) {
+            synchronized (this) {
+                result = _attackableEntries;
+                if (result == null) {
+                    result = new FCollection<>();
+                    _attackableEntries = result;
+                }
+            }
+        }
+        return _attackableEntries;
+    }
     // Keyed by attackable defender (player or planeswalker or battle)
-    private final Multimap<GameEntity, AttackingBand> attackedByBands = Multimaps.synchronizedMultimap(ArrayListMultimap.create());
+    private Multimap<GameEntity, AttackingBand> _attackedByBands;
-    private final Multimap<AttackingBand, Card> blockedBands = Multimaps.synchronizedMultimap(ArrayListMultimap.create());
+    private Multimap<GameEntity, AttackingBand> attackedByBands() {
+        Multimap<GameEntity, AttackingBand> result = _attackedByBands;
+        if (result == null) {
+            synchronized (this) {
+                result = _attackedByBands;
+                if (result == null) {
+                    result = Multimaps.synchronizedMultimap(ArrayListMultimap.create());
+                    _attackedByBands = result;
+                }
+            }
+        }
+        return _attackedByBands;
+    }
+    private Multimap<AttackingBand, Card> _blockedBands;
+    private Multimap<AttackingBand, Card> blockedBands() {
+        Multimap<AttackingBand, Card> result = _blockedBands;
+        if (result == null) {
+            synchronized (this) {
+                result = _blockedBands;
+                if (result == null) {
+                    result = Multimaps.synchronizedMultimap(ArrayListMultimap.create());
+                    _blockedBands = result;
+                }
+            }
+        }
+        return _blockedBands;
+    }
 
-    private final Map<Card, CardCollection> attackersOrderedForDamageAssignment = Maps.newHashMap();
+    private Map<Card, CardCollection> _attackersOrderedForDamageAssignment;
-    private final Map<Card, CardCollection> blockersOrderedForDamageAssignment = Maps.newHashMap();
+    private Map<Card, CardCollection> attackersOrderedForDamageAssignment() {
-    private CardCollection lkiCache = new CardCollection();
+        Map<Card, CardCollection> result = _attackersOrderedForDamageAssignment;
-    private CardDamageMap damageMap = new CardDamageMap();
+        if (result == null) {
+            synchronized (this) {
+                result = _attackersOrderedForDamageAssignment;
+                if (result == null) {
+                    result = Maps.newHashMap();
+                    _attackersOrderedForDamageAssignment = result;
+                }
+            }
+        }
+        return _attackersOrderedForDamageAssignment;
+    }
+    private Map<Card, CardCollection> _blockersOrderedForDamageAssignment;
+    private Map<Card, CardCollection> blockersOrderedForDamageAssignment() {
+        Map<Card, CardCollection> result = _blockersOrderedForDamageAssignment;
+        if (result == null) {
+            synchronized (this) {
+                result = _blockersOrderedForDamageAssignment;
+                if (result == null) {
+                    result = Maps.newHashMap();
+                    _blockersOrderedForDamageAssignment = result;
+                }
+            }
+        }
+        return _blockersOrderedForDamageAssignment;
+    }
+    private CardCollection _lkiCache;
+    private CardCollection lkiCache() {
+        CardCollection result = _lkiCache;
+        if (result == null) {
+            synchronized (this) {
+                result = _lkiCache;
+                if (result == null) {
+                    result = new CardCollection();
+                    _lkiCache = result;
+                }
+            }
+        }
+        return _lkiCache;
+    }
+    private CardDamageMap _damageMap;
+    private CardDamageMap damageMap() {
+        CardDamageMap result = _damageMap;
+        if (result == null) {
+            synchronized (this) {
+                result = _damageMap;
+                if (result == null) {
+                    result = new CardDamageMap();
+                    _damageMap = result;
+                }
+            }
+        }
+        return _damageMap;
+    }
 
     // List holds creatures who have dealt 1st strike damage to disallow them deal damage on regular basis (unless they have double-strike KW)
-    private CardCollection combatantsThatDealtFirstStrikeDamage = new CardCollection();
+    private CardCollection _combatantsThatDealtFirstStrikeDamage;
+    private CardCollection combatantsThatDealtFirstStrikeDamage() {
+        CardCollection result = _combatantsThatDealtFirstStrikeDamage;
+        if (result == null) {
+            synchronized (this) {
+                result = _combatantsThatDealtFirstStrikeDamage;
+                if (result == null) {
+                    result = new CardCollection();
+                    _combatantsThatDealtFirstStrikeDamage = result;
+                }
+            }
+        }
+        return _combatantsThatDealtFirstStrikeDamage;
+    }
 
     public Combat(final Player attacker) {
         playerWhoAttacks = attacker;
@@ -75,12 +178,12 @@ public class Combat {
 
     public Combat(Combat combat, IEntityMap map) {
         playerWhoAttacks = map.map(combat.playerWhoAttacks);
-        for (GameEntity entry : combat.attackableEntries) {
+        for (GameEntity entry : combat.attackableEntries()) {
-            attackableEntries.add(map.map(entry));
+            attackableEntries().add(map.map(entry));
         }
 
         HashMap<AttackingBand, AttackingBand> bandsMap = new HashMap<>();
-        for (Entry<GameEntity, AttackingBand> entry : combat.attackedByBands.entries()) {
+        for (Entry<GameEntity, AttackingBand> entry : combat.attackedByBands().entries()) {
             AttackingBand origBand = entry.getValue();
             ArrayList<Card> attackers = new ArrayList<>();
             for (Card c : origBand.getAttackers()) {
@@ -92,37 +195,37 @@ public class Combat {
                 newBand.setBlocked(blocked);
             }
             bandsMap.put(origBand, newBand);
-            attackedByBands.put(map.map(entry.getKey()), newBand);
+            attackedByBands().put(map.map(entry.getKey()), newBand);
         }
-        for (Entry<AttackingBand, Card> entry : combat.blockedBands.entries()) {
+        for (Entry<AttackingBand, Card> entry : combat.blockedBands().entries()) {
-            blockedBands.put(bandsMap.get(entry.getKey()), map.map(entry.getValue()));
+            blockedBands().put(bandsMap.get(entry.getKey()), map.map(entry.getValue()));
         }
 
-        for (Entry<Card, CardCollection> entry : combat.attackersOrderedForDamageAssignment.entrySet()) {
+        for (Entry<Card, CardCollection> entry : combat.attackersOrderedForDamageAssignment().entrySet()) {
-            attackersOrderedForDamageAssignment.put(map.map(entry.getKey()), map.mapCollection(entry.getValue()));
+            attackersOrderedForDamageAssignment().put(map.map(entry.getKey()), map.mapCollection(entry.getValue()));
         }
-        for (Entry<Card, CardCollection> entry : combat.blockersOrderedForDamageAssignment.entrySet()) {
+        for (Entry<Card, CardCollection> entry : combat.blockersOrderedForDamageAssignment().entrySet()) {
-            blockersOrderedForDamageAssignment.put(map.map(entry.getKey()), map.mapCollection(entry.getValue()));
+            blockersOrderedForDamageAssignment().put(map.map(entry.getKey()), map.mapCollection(entry.getValue()));
         }
         // Note: Doesn't currently set up lkiCache, since it's just a cache and not strictly needed...
-        for (Table.Cell<Card, GameEntity, Integer> entry : combat.damageMap.cellSet()) {
+        for (Table.Cell<Card, GameEntity, Integer> entry : combat.damageMap().cellSet()) {
-            damageMap.put(map.map(entry.getRowKey()), map.map(entry.getColumnKey()), entry.getValue());
+            damageMap().put(map.map(entry.getRowKey()), map.map(entry.getColumnKey()), entry.getValue());
         }
 
         attackConstraints = new AttackConstraints(this);
     }
 
     public void initConstraints() {
-        attackableEntries.clear();
+        attackableEntries().clear();
         // Create keys for all possible attack targets
-        attackableEntries.addAll(CombatUtil.getAllPossibleDefenders(playerWhoAttacks));
+        attackableEntries().addAll(CombatUtil.getAllPossibleDefenders(playerWhoAttacks));
         attackConstraints = new AttackConstraints(this);
     }
 
     @Override
     public String toString() {
         StringBuilder sb = new StringBuilder();
-        for (GameEntity defender : attackableEntries) {
+        for (GameEntity defender : attackableEntries()) {
             CardCollection attackers = getAttackersOf(defender);
             if (attackers.isEmpty()) {
                 continue;
@@ -148,13 +251,13 @@ public class Combat {
         CardCollection blockers = getAllBlockers();
 
         //clear all combat-related collections
-        attackableEntries.clear();
+        attackableEntries().clear();
-        attackedByBands.clear();
+        attackedByBands().clear();
-        blockedBands.clear();
+        blockedBands().clear();
-        attackersOrderedForDamageAssignment.clear();
+        attackersOrderedForDamageAssignment().clear();
-        blockersOrderedForDamageAssignment.clear();
+        blockersOrderedForDamageAssignment().clear();
-        lkiCache.clear();
+        lkiCache().clear();
-        combatantsThatDealtFirstStrikeDamage.clear();
+        combatantsThatDealtFirstStrikeDamage().clear();
 
         //clear tracking for cards that care about "this combat"
         Game game = playerWhoAttacks.getGame();
@@ -186,7 +289,7 @@ public class Combat {
         return attackConstraints;
     }
     public final FCollectionView<GameEntity> getDefenders() {
-        return attackableEntries;
+        return attackableEntries();
     }
 
     //gets attacked player opponents (ignores planeswalkers)
@@ -204,7 +307,7 @@ public class Combat {
 
     public final FCollection<GameEntity> getDefendersControlledBy(Player who) {
         FCollection<GameEntity> res = new FCollection<>();
-        for (GameEntity ge : attackableEntries) {
+        for (GameEntity ge : attackableEntries()) {
             // if defender is the player himself or his cards
             if (ge == who || ge instanceof Card && ((Card) ge).getController() == who) {
                 res.add(ge);
@@ -214,15 +317,15 @@ public class Combat {
     }
 
     public final FCollectionView<Player> getDefendingPlayers() {
-        return new FCollection<>(Iterables.filter(attackableEntries, Player.class));
+        return new FCollection<>(Iterables.filter(attackableEntries(), Player.class));
     }
 
     public final CardCollection getDefendingPlaneswalkers() {
-        return CardLists.filter(Iterables.filter(attackableEntries, Card.class), CardPredicates.isType("Planeswalker"));
+        return CardLists.filter(Iterables.filter(attackableEntries(), Card.class), CardPredicates.isType("Planeswalker"));
     }
 
     public final CardCollection getDefendingBattles() {
-        return CardLists.filter(Iterables.filter(attackableEntries, Card.class), CardPredicates.isType("Battle"));
+        return CardLists.filter(Iterables.filter(attackableEntries(), Card.class), CardPredicates.isType("Battle"));
     }
 
     public final Map<Card, GameEntity> getAttackersAndDefenders() {
@@ -230,14 +333,14 @@ public class Combat {
     }
 
     public final List<AttackingBand> getAttackingBandsOf(GameEntity defender) {
-        return Lists.newArrayList(attackedByBands.get(defender));
+        return Lists.newArrayList(attackedByBands().get(defender));
     }
 
     public final CardCollection getAttackersOf(GameEntity defender) {
         CardCollection result = new CardCollection();
-        if (!attackedByBands.containsKey(defender))
+        if (!attackedByBands().containsKey(defender))
             return result;
-        for (AttackingBand v : attackedByBands.get(defender)) {
+        for (AttackingBand v : attackedByBands().get(defender)) {
             result.addAll(v.getAttackers());
         }
         return result;
@@ -247,7 +350,7 @@ public class Combat {
         addAttacker(c, defender, null);
     }
     public final void addAttacker(final Card c, GameEntity defender, AttackingBand band) {
-        Collection<AttackingBand> attackersOfDefender = attackedByBands.get(defender);
+        Collection<AttackingBand> attackersOfDefender = attackedByBands().get(defender);
         if (attackersOfDefender == null) {
             System.out.println("Trying to add Attacker " + c + " to missing defender " + defender);
             return;
@@ -272,7 +375,7 @@ public class Combat {
         return getDefenderByAttacker(getBandOfAttacker(c));
     }
     public final GameEntity getDefenderByAttacker(final AttackingBand c) {
-        for (Entry<GameEntity, AttackingBand> e : attackedByBands.entries()) {
+        for (Entry<GameEntity, AttackingBand> e : attackedByBands().entries()) {
             if (e.getValue() == c) {
                 return e.getKey();
             }
@@ -305,12 +408,12 @@ public class Combat {
         if (c == null) {
             return null;
         }
-        for (AttackingBand ab : attackedByBands.values()) {
+        for (AttackingBand ab : attackedByBands().values()) {
             if (ab.contains(c)) {
                 return ab;
             }
         }
-        CombatLki lki = lkiCache.get(c).getCombatLKI();
+        CombatLki lki = lkiCache().get(c).getCombatLKI();
         return lki == null || !lki.isAttacker ? null : lki.getFirstBand();
     }
 
@@ -323,12 +426,12 @@ public class Combat {
     }
 
     public final List<AttackingBand> getAttackingBands() {
-        return Lists.newArrayList(attackedByBands.values());
+        return Lists.newArrayList(attackedByBands().values());
     }
 
     public boolean isAttacking(Card card, GameEntity defender) {
         AttackingBand ab = getBandOfAttacker(card);
-        for (Entry<GameEntity, AttackingBand> ee : attackedByBands.entries()) {
+        for (Entry<GameEntity, AttackingBand> ee : attackedByBands().entries()) {
             if (ee.getValue() == ab) {
                 return ee.getKey() == defender;
             }
@@ -340,7 +443,7 @@ public class Combat {
      * Checks if a card is currently attacking, returns false if the card is not currently attacking, even if its LKI was.
      */
     public final boolean isAttacking(Card card) {
-        for (AttackingBand ab : attackedByBands.values()) {
+        for (AttackingBand ab : attackedByBands().values()) {
             if (ab.contains(card)) {
                 return true;
             }
@@ -350,7 +453,7 @@ public class Combat {
 
     public final CardCollection getAttackers() {
         CardCollection result = new CardCollection();
-        for (AttackingBand ab : attackedByBands.values()) {
+        for (AttackingBand ab : attackedByBands().values()) {
             result.addAll(ab.getAttackers());
         }
         return result;
@@ -368,9 +471,9 @@ public class Combat {
 
     public final void addBlocker(final Card attacker, final Card blocker) {
         final AttackingBand band = getBandOfAttackerNotNull(attacker);
-        blockedBands.put(band, blocker);
+        blockedBands().put(band, blocker);
         // If damage is already assigned, add this blocker as a "late entry"
-        if (blockersOrderedForDamageAssignment.containsKey(attacker)) {
+        if (blockersOrderedForDamageAssignment().containsKey(attacker)) {
             addBlockerToDamageAssignmentOrder(attacker, blocker);
         }
         blocker.updateBlockingForView();
@@ -379,7 +482,7 @@ public class Combat {
     // remove blocker from specific attacker
     public final void removeBlockAssignment(final Card attacker, final Card blocker) {
         AttackingBand band = getBandOfAttackerNotNull(attacker);
-        Collection<Card> cc = blockedBands.get(band);
+        Collection<Card> cc = blockedBands().get(band);
         if (cc != null) {
             cc.remove(blocker);
         }
@@ -389,13 +492,13 @@ public class Combat {
     // remove blocker from everywhere
     public final void undoBlockingAssignment(final Card blocker) {
         CardCollection toRemove = new CardCollection(blocker);
-        blockedBands.values().removeAll(toRemove);
+        blockedBands().values().removeAll(toRemove);
         blocker.updateBlockingForView();
     }
 
     public final CardCollection getAllBlockers() {
         CardCollection result = new CardCollection();
-        for (Card blocker : blockedBands.values()) {
+        for (Card blocker : blockedBands().values()) {
             if (!result.contains(blocker)) {
                 result.add(blocker);
             }
@@ -406,8 +509,11 @@ public class Combat {
     public final CardCollection getDefendersCreatures() {
         CardCollection result = new CardCollection();
         for (Card attacker : getAttackers()) {
-            CardCollection cc = getDefenderPlayerByAttacker(attacker).getCreaturesInPlay();
+            Player defender = getDefenderPlayerByAttacker(attacker);
-            result.addAll(cc);
+            if (defender != null) {
+                CardCollection cc = defender.getCreaturesInPlay();
+                result.addAll(cc);
+            }
         }
         return result;
     }
@@ -417,13 +523,13 @@ public class Combat {
         return getBlockers(getBandOfAttacker(card));
     }
     public final CardCollection getBlockers(final AttackingBand band) {
-        Collection<Card> blockers = blockedBands.get(band);
+        Collection<Card> blockers = blockedBands().get(band);
         return blockers == null ? new CardCollection() : new CardCollection(blockers);
     }
 
     public final CardCollection getAttackersBlockedBy(final Card blocker) {
         CardCollection blocked =  new CardCollection();
-        for (Entry<AttackingBand, Card> s : blockedBands.entries()) {
+        for (Entry<AttackingBand, Card> s : blockedBands().entries()) {
             if (s.getValue().equals(blocker)) {
                 blocked.addAll(s.getKey().getAttackers());
             }
@@ -433,7 +539,7 @@ public class Combat {
 
     public final FCollectionView<AttackingBand> getAttackingBandsBlockedBy(Card blocker) {
         FCollection<AttackingBand> bands = new FCollection<>();
-        for (Entry<AttackingBand, Card> kv : blockedBands.entries()) {
+        for (Entry<AttackingBand, Card> kv : blockedBands().entries()) {
             if (kv.getValue().equals(blocker)) {
                 bands.add(kv.getKey());
             }
@@ -457,10 +563,10 @@ public class Combat {
     /** If there are multiple blockers, the Attacker declares the Assignment Order */
     public void orderBlockersForDamageAssignment() { // this method performs controller's role
         List<Pair<Card, CardCollection>> blockersNeedManualOrdering = new ArrayList<>();
-        for (AttackingBand band : attackedByBands.values()) {
+        for (AttackingBand band : attackedByBands().values()) {
             if (band.isEmpty()) continue;
 
-            Collection<Card> blockers = blockedBands.get(band);
+            Collection<Card> blockers = blockedBands().get(band);
             if (blockers == null || blockers.isEmpty()) {
                 continue;
             }
@@ -484,13 +590,13 @@ public class Combat {
     /** If there are multiple blockers, the Attacker declares the Assignment Order */
     public void orderBlockersForDamageAssignment(Card attacker, CardCollection blockers) { // this method performs controller's role
         if (blockers.size() <= 1 || !this.legacyOrderCombatants) {
-            blockersOrderedForDamageAssignment.put(attacker, new CardCollection(blockers));
+            blockersOrderedForDamageAssignment().put(attacker, new CardCollection(blockers));
             return;
         }
 
         // Damage Ordering needs to take cards like Melee into account, is that happening?
         CardCollection orderedBlockers = playerWhoAttacks.getController().orderBlockers(attacker, blockers); // we know there's a list
-        blockersOrderedForDamageAssignment.put(attacker, orderedBlockers);
+        blockersOrderedForDamageAssignment().put(attacker, orderedBlockers);
 
         // Display the chosen order of blockers in the log
         // TODO: this is best done via a combat panel update
@@ -517,15 +623,15 @@ public class Combat {
      * @param blocker the blocking creature.
      */
     public void addBlockerToDamageAssignmentOrder(Card attacker, Card blocker) {
-        final CardCollection oldBlockers = blockersOrderedForDamageAssignment.get(attacker);
+        final CardCollection oldBlockers = blockersOrderedForDamageAssignment().get(attacker);
-    	if (oldBlockers == null || oldBlockers.isEmpty()) {
+        if (oldBlockers == null || oldBlockers.isEmpty()) {
-   			blockersOrderedForDamageAssignment.put(attacker, new CardCollection(blocker));
+            blockersOrderedForDamageAssignment().put(attacker, new CardCollection(blocker));
-    	} else if (this.legacyOrderCombatants) {
+        } else if (this.legacyOrderCombatants) {
             CardCollection orderedBlockers = playerWhoAttacks.getController().orderBlocker(attacker, blocker, oldBlockers);
-            blockersOrderedForDamageAssignment.put(attacker, orderedBlockers);
+            blockersOrderedForDamageAssignment().put(attacker, orderedBlockers);
-    	} else {
+        } else {
             oldBlockers.add(blocker);
-            blockersOrderedForDamageAssignment.put(attacker, oldBlockers);
+            blockersOrderedForDamageAssignment().put(attacker, oldBlockers);
         }
     }
 
@@ -544,19 +650,19 @@ public class Combat {
         CardCollection orderedAttacker = attackers.size() <= 1 || !this.legacyOrderCombatants ? attackers : blockerCtrl.getController().orderAttackers(blocker, attackers);
 
         // Damage Ordering needs to take cards like Melee into account, is that happening?
-        attackersOrderedForDamageAssignment.put(blocker, orderedAttacker);
+        attackersOrderedForDamageAssignment().put(blocker, orderedAttacker);
     }
 
     // removes references to this attacker from all indices and orders
     public void unregisterAttacker(final Card c, AttackingBand ab) {
-        blockersOrderedForDamageAssignment.remove(c);
+        blockersOrderedForDamageAssignment().remove(c);
 
-        Collection<Card> blockers = blockedBands.get(ab);
+        Collection<Card> blockers = blockedBands().get(ab);
         if (blockers != null) {
             for (Card b : blockers) {
                 // Clear removed attacker from assignment order
-                if (attackersOrderedForDamageAssignment.containsKey(b)) {
+                if (attackersOrderedForDamageAssignment().containsKey(b)) {
-                    attackersOrderedForDamageAssignment.get(b).remove(c);
+                    attackersOrderedForDamageAssignment().get(b).remove(c);
                 }
             }
         }
@@ -582,10 +688,10 @@ public class Combat {
 
     // removes references to this defender from all indices and orders
     public void unregisterDefender(final Card c, AttackingBand bandBeingBlocked) {
-        attackersOrderedForDamageAssignment.remove(c);
+        attackersOrderedForDamageAssignment().remove(c);
         for (Card atk : bandBeingBlocked.getAttackers()) {
-            if (blockersOrderedForDamageAssignment.containsKey(atk)) {
+            if (blockersOrderedForDamageAssignment().containsKey(atk)) {
-                blockersOrderedForDamageAssignment.get(atk).remove(c);
+                blockersOrderedForDamageAssignment().get(atk).remove(c);
             }
         }
     }
@@ -601,16 +707,16 @@ public class Combat {
         }
 
         // if not found in attackers, look for this card in blockers
-        for (Entry<AttackingBand, Card> be : blockedBands.entries()) {
+        for (Entry<AttackingBand, Card> be : blockedBands().entries()) {
             if (be.getValue().equals(c)) {
                 unregisterDefender(c, be.getKey());
             }
         }
 
-        for (Card battleOrPW : Iterables.filter(attackableEntries, Card.class)) {
+        for (Card battleOrPW : Iterables.filter(attackableEntries(), Card.class)) {
             if (battleOrPW.equals(c)) {
                 Multimap<GameEntity, AttackingBand> attackerBuffer = ArrayListMultimap.create();
-                Collection<AttackingBand> bands = attackedByBands.get(c);
+                Collection<AttackingBand> bands = attackedByBands().get(c);
                 for (AttackingBand abDef : bands) {
                     unregisterDefender(c, abDef);
                     // Rule 506.4c workaround to keep creatures in combat
@@ -620,20 +726,20 @@ public class Combat {
                     attackerBuffer.put(fake, abDef);
                 }
                 bands.clear();
-                attackedByBands.putAll(attackerBuffer);
+                attackedByBands().putAll(attackerBuffer);
                 break;
             }
         }
 
         // remove card from map
-        while (blockedBands.values().remove(c));
+        while (blockedBands().values().remove(c));
         c.updateBlockingForView();
     }
 
     public final boolean removeAbsentCombatants() {
         // CR 506.4 iterate all attackers and remove illegal declarations
         CardCollection missingCombatants = new CardCollection();
-        for (Entry<GameEntity, AttackingBand> ee : attackedByBands.entries()) {
+        for (Entry<GameEntity, AttackingBand> ee : attackedByBands().entries()) {
             for (Card c : ee.getValue().getAttackers()) {
                 if (!c.isInPlay() || !c.isCreature()) {
                     missingCombatants.add(c);
@@ -647,7 +753,7 @@ public class Combat {
             }
         }
 
-        for (Entry<AttackingBand, Card> be : blockedBands.entries()) {
+        for (Entry<AttackingBand, Card> be : blockedBands().entries()) {
             Card blocker = be.getValue();
             if (!blocker.isInPlay() || !blocker.isCreature()) {
                 missingCombatants.add(blocker);
@@ -666,8 +772,8 @@ public class Combat {
     public final void fireTriggersForUnblockedAttackers(final Game game) {
         boolean bFlag = false;
         List<GameEntity> defenders = Lists.newArrayList();
-        for (AttackingBand ab : attackedByBands.values()) {
+        for (AttackingBand ab : attackedByBands().values()) {
-            Collection<Card> blockers = blockedBands.get(ab);
+            Collection<Card> blockers = blockedBands().get(ab);
             boolean isBlocked = blockers != null && !blockers.isEmpty();
             ab.setBlocked(isBlocked);
 
@@ -706,23 +812,23 @@ public class Combat {
             }
 
             if (firstStrikeDamage) {
-                combatantsThatDealtFirstStrikeDamage.add(blocker);
+                combatantsThatDealtFirstStrikeDamage().add(blocker);
             }
 
             // Run replacement effects
             blocker.getGame().getReplacementHandler().run(ReplacementType.AssignDealDamage, AbilityKey.mapFromAffected(blocker));
 
-            CardCollection attackers = attackersOrderedForDamageAssignment.get(blocker);
+            CardCollection attackers = attackersOrderedForDamageAssignment().get(blocker);
 
             final int damage = blocker.getNetCombatDamage();
 
-            if (!attackers.isEmpty()) {
+            if (attackers != null && !attackers.isEmpty()) {
                 Player attackingPlayer = getAttackingPlayer();
                 Player assigningPlayer = blocker.getController();
 
                 Player defender = null;
                 boolean divideCombatDamageAsChoose = blocker.hasKeyword("You may assign CARDNAME's combat damage divided as you choose among " +
-                                "defending player and/or any number of creatures they control.")
+                        "defending player and/or any number of creatures they control.")
                         && blocker.getController().getController().confirmStaticApplication(blocker, PlayerActionConfirmMode.AlternativeDamageAssignment,
                         Localizer.getInstance().getMessage("lblAssignCombatDamageAsChoose",
                                 CardTranslation.getTranslatedName(blocker.getName())), null);
@@ -740,10 +846,10 @@ public class Combat {
                 for (Entry<Card, Integer> dt : map.entrySet()) {
                     // Butcher Orgg
                     if (dt.getKey() == null && dt.getValue() > 0) {
-                        damageMap.put(blocker, defender, dt.getValue());
+                        damageMap().put(blocker, defender, dt.getValue());
                     } else {
                         dt.getKey().addAssignedDamage(dt.getValue(), blocker);
-                        damageMap.put(blocker, dt.getKey(), dt.getValue());
+                        damageMap().put(blocker, dt.getKey(), dt.getValue());
                     }
                 }
             }
@@ -764,7 +870,7 @@ public class Combat {
             }
 
             if (firstStrikeDamage) {
-                combatantsThatDealtFirstStrikeDamage.add(attacker);
+                combatantsThatDealtFirstStrikeDamage().add(attacker);
             }
 
             // Run replacement effects
@@ -785,7 +891,7 @@ public class Combat {
 
             GameEntity defender = getDefenderByAttacker(band);
             Player assigningPlayer = getAttackingPlayer();
-            orderedBlockers = blockersOrderedForDamageAssignment.get(attacker);
+            orderedBlockers = blockersOrderedForDamageAssignment().get(attacker);
             // Defensive Formation is very similar to Banding with Blockers
             // It allows the defending player to assign damage instead of the attacking player
             if (defender instanceof Player && defender.hasKeyword("You assign combat damage of each creature attacking you.")) {
@@ -814,8 +920,8 @@ public class Combat {
                         attacker.hasKeyword("You may assign CARDNAME's combat damage divided as you choose among " +
                                 "defending player and/or any number of creatures they control.")
                         && assigningPlayer.getController().confirmStaticApplication(attacker, PlayerActionConfirmMode.AlternativeDamageAssignment,
-                                Localizer.getInstance().getMessage("lblAssignCombatDamageAsChoose",
+                        Localizer.getInstance().getMessage("lblAssignCombatDamageAsChoose",
-                                        CardTranslation.getTranslatedName(attacker.getName())), null);
+                                CardTranslation.getTranslatedName(attacker.getName())), null);
                 if (defender instanceof Card && divideCombatDamageAsChoose) {
                     defender = getDefenderPlayerByAttacker(attacker);
                 }
@@ -849,7 +955,7 @@ public class Combat {
             }
             if (assignToPlayer) {
                 attackers.remove(attacker);
-                damageMap.put(attacker, defender, damageDealt);
+                damageMap().put(attacker, defender, damageDealt);
             }
             else if (orderedBlockers == null || orderedBlockers.isEmpty()) {
                 attackers.remove(attacker);
@@ -857,9 +963,14 @@ public class Combat {
                     final SpellAbility emptySA = new SpellAbility.EmptySa(ApiType.Cleanup, attacker);
                     Card chosen = attacker.getController().getController().chooseCardsForEffect(getDefendersCreatures(),
                             emptySA, Localizer.getInstance().getMessage("lblChooseCreature"), 1, 1, false, null).get(0);
-                    damageMap.put(attacker, chosen, damageDealt);
+                    damageMap().put(attacker, chosen, damageDealt);
                 } else if (trampler || !band.isBlocked()) { // this is called after declare blockers, no worries 'bout nulls in isBlocked
-                    damageMap.put(attacker, defender, damageDealt);
+                    if (defender == null) {
+                        defender = getDefenderPlayerByAttacker(attacker);
+                        System.err.println("[COMBAT] defender is null, getDefenderPlayerByAttacker(attacker) result: " + defender);
+                    }
+                    // this will fail if defender is null, and it doesn't allow null values..
+                    damageMap().put(attacker, defender, damageDealt);
                 } // No damage happens if blocked but no blockers left
             } else {
                 Map<Card, Integer> map = assigningPlayer.getController().assignCombatDamage(attacker, orderedBlockers, attackers,
@@ -879,11 +990,11 @@ public class Combat {
                             if (defender instanceof Card) {
                                 ((Card) defender).addAssignedDamage(dt.getValue(), attacker);
                             }
-                            damageMap.put(attacker, defender, dt.getValue());
+                            damageMap().put(attacker, defender, dt.getValue());
                         }
                     } else {
                         dt.getKey().addAssignedDamage(dt.getValue(), attacker);
-                        damageMap.put(attacker, dt.getKey(), dt.getValue());
+                        damageMap().put(attacker, dt.getKey(), dt.getValue());
                     }
                 }
             } // if !hasFirstStrike ...
@@ -900,7 +1011,7 @@ public class Combat {
         if (firstStrikeDamage && combatant.hasFirstStrike()) {
             return true;
         }
-        return !firstStrikeDamage && !combatantsThatDealtFirstStrikeDamage.contains(combatant);
+        return !firstStrikeDamage && !combatantsThatDealtFirstStrikeDamage().contains(combatant);
     }
 
     public final boolean assignCombatDamage(boolean firstStrikeDamage) {
@@ -908,7 +1019,7 @@ public class Combat {
         assignedDamage |= assignBlockersDamage(firstStrikeDamage);
         if (!firstStrikeDamage) {
             // Clear first strike damage list since it doesn't matter anymore
-            combatantsThatDealtFirstStrikeDamage.clear();
+            combatantsThatDealtFirstStrikeDamage().clear();
         }
         return assignedDamage;
     }
@@ -920,7 +1031,7 @@ public class Combat {
         CardDamageMap preventMap = new CardDamageMap();
         GameEntityCounterTable counterTable = new GameEntityCounterTable();
 
-        game.getAction().dealDamage(true, damageMap, preventMap, counterTable, null);
+        game.getAction().dealDamage(true, damageMap(), preventMap, counterTable, null);
 
         // copy last state again for dying replacement effects
         game.copyLastState();
@@ -933,7 +1044,7 @@ public class Combat {
 
     public final CardCollection getUnblockedAttackers() {
         CardCollection unblocked = new CardCollection();
-        for (AttackingBand ab : attackedByBands.values()) {
+        for (AttackingBand ab : attackedByBands().values()) {
             if (Boolean.FALSE.equals(ab.isBlocked())) {
                 unblocked.addAll(ab.getAttackers());
             }
@@ -942,13 +1053,13 @@ public class Combat {
     }
 
     public boolean isPlayerAttacked(Player who) {
-        for (GameEntity defender : attackedByBands.keySet()) {
+        for (GameEntity defender : attackedByBands().keySet()) {
             Card defenderAsCard = defender instanceof Card ? (Card)defender : null;
             if ((null != defenderAsCard && (defenderAsCard.getController() != who && defenderAsCard.getProtectingPlayer() != who)) ||
-                (null == defenderAsCard && defender != who)) {
+                    (null == defenderAsCard && defender != who)) {
                 continue; // defender is not related to player 'who'
             }
-            for (AttackingBand ab : attackedByBands.get(defender)) {
+            for (AttackingBand ab : attackedByBands().get(defender)) {
                 if (!ab.isEmpty()) {
                     return true;
                 }
@@ -958,7 +1069,7 @@ public class Combat {
     }
 
     public boolean isBlocking(Card blocker) {
-        if (blockedBands.containsValue(blocker)) {
+        if (blockedBands().containsValue(blocker)) {
             return true; // is blocking something at the moment
         }
 
@@ -966,13 +1077,13 @@ public class Combat {
             return false;
         }
 
-        CombatLki lki = lkiCache.get(blocker).getCombatLKI();
+        CombatLki lki = lkiCache().get(blocker).getCombatLKI();
         return null != lki && !lki.isAttacker; // was blocking something anyway
     }
 
     public boolean isBlocking(Card blocker, Card attacker) {
         AttackingBand ab = getBandOfAttacker(attacker);
-        Collection<Card> blockers = blockedBands.get(ab);
+        Collection<Card> blockers = blockedBands().get(ab);
         if (blockers != null && blockers.contains(blocker)) {
             return true; // is blocking the attacker's band at the moment
         }
@@ -981,7 +1092,7 @@ public class Combat {
             return false;
         }
 
-        CombatLki lki = lkiCache.get(blocker).getCombatLKI();
+        CombatLki lki = lkiCache().get(blocker).getCombatLKI();
         return null != lki && !lki.isAttacker && lki.relatedBands.contains(ab); // was blocking that very band
     }
 
@@ -994,7 +1105,7 @@ public class Combat {
         final boolean isAttacker = attackingBand != null;
         if (isAttacker) {
             boolean found = false;
-            for (AttackingBand ab : attackedByBands.values()) {
+            for (AttackingBand ab : attackedByBands().values()) {
                 if (ab.contains(lki)) {
                     found = true;
                     break;
@@ -1009,7 +1120,7 @@ public class Combat {
                 return null; // card was not even in combat
             }
         }
-        lkiCache.add(lki);
+        lkiCache().add(lki);
         final FCollectionView<AttackingBand> relatedBands = isAttacker ? new FCollection<>(attackingBand) : attackersBlocked;
         return new CombatLki(isAttacker, relatedBands);
     }

forge-game/src/main/java/forge/game/cost/CostPayment.java

@@ -229,8 +229,8 @@ public class CostPayment extends ManaConversionMatrix {
      * @return a {@link forge.game.mana.Mana} object.
      */
     public static Mana getMana(final Player player, final ManaCostShard shard, final SpellAbility saBeingPaidFor,
-            final byte colorsPaid, Map<String, Integer> xManaCostPaidByColor) {
+            final byte colorsPaid, Map<String, Integer> xManaCostPaidByColor) { // player.getManaPool() is not threadsafe if this is called somwewhere concurrently
-        final List<Pair<Mana, Integer>> weightedOptions = selectManaToPayFor(player.getManaPool(), shard,
+        final List<Pair<Mana, Integer>> weightedOptions = selectManaToPayFor(new ManaPool(player), shard,
             saBeingPaidFor, colorsPaid, xManaCostPaidByColor);
 
         // Exclude border case

forge-game/src/main/java/forge/game/mana/ManaPool.java

@@ -23,6 +23,8 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Multimaps;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Lists;
 
@@ -51,7 +53,20 @@ import forge.game.staticability.StaticAbilityUnspentMana;
  */
 public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
     private final Player owner;
-    private final ArrayListMultimap<Byte, Mana> floatingMana = ArrayListMultimap.create();
+    private ListMultimap<Byte, Mana> _floatingMana;
+    private ListMultimap<Byte, Mana> floatingMana() {
+        ListMultimap<Byte, Mana> result = _floatingMana;
+        if (result == null) {
+            synchronized (this) {
+                result = _floatingMana;
+                if (result == null) {
+                    result = Multimaps.synchronizedListMultimap(ArrayListMultimap.create());
+                    _floatingMana = result;
+                }
+            }
+        }
+        return _floatingMana;
+    }
 
     public ManaPool(final Player player) {
         owner = player;
@@ -59,7 +74,7 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
     }
 
     public final int getAmountOfColor(final byte color) {
-        Collection<Mana> ofColor = floatingMana.get(color);
+        Collection<Mana> ofColor = floatingMana().get(color);
         return ofColor == null ? 0 : ofColor.size();
     }
 
@@ -67,7 +82,7 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
         addMana(mana, true);
     }
     public void addMana(final Mana mana, boolean updateView) {
-        floatingMana.put(mana.getColor(), mana);
+        floatingMana().put(mana.getColor(), mana);
         if (updateView) {
             owner.updateManaForView();
             owner.getGame().fireEvent(new GameEventManaPool(owner, EventValueChangeType.Added, mana));
@@ -88,7 +103,7 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
      * </p>
      */
     public final boolean willManaBeLostAtEndOfPhase() {
-        if (floatingMana.isEmpty()) {
+        if (floatingMana().isEmpty()) {
             return false;
         }
 
@@ -114,13 +129,13 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
 
     public final void resetPool() {
         // This should only be used to reset the pool to empty by things like restores.
-        floatingMana.clear();
+        floatingMana().clear();
     }
 
     public final List<Mana> clearPool(boolean isEndOfPhase) {
         // isEndOfPhase parameter: true = end of phase, false = mana drain effect
         List<Mana> cleared = Lists.newArrayList();
-        if (floatingMana.isEmpty()) { return cleared; }
+        if (floatingMana().isEmpty()) { return cleared; }
 
         Byte convertTo = null;
 
@@ -128,17 +143,17 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
         final Map<AbilityKey, Object> runParams = AbilityKey.mapFromAffected(owner);
         runParams.put(AbilityKey.Mana, "C");
         switch (owner.getGame().getReplacementHandler().run(ReplacementType.LoseMana, runParams)) {
-        case NotReplaced:
+            case NotReplaced:
-            break;
+                break;
-        case Skipped:
+            case Skipped:
-            return cleared;
+                return cleared;
-        default:
+            default:
-            convertTo = ManaAtom.fromName((String) runParams.get(AbilityKey.Mana));
+                convertTo = ManaAtom.fromName((String) runParams.get(AbilityKey.Mana));
-            break;
+                break;
 
         }
 
-        final List<Byte> keys = Lists.newArrayList(floatingMana.keySet());
+        final List<Byte> keys = Lists.newArrayList(floatingMana().keySet());
         if (isEndOfPhase) {
             keys.removeAll(StaticAbilityUnspentMana.getManaToKeep(owner));
         }
@@ -147,7 +162,7 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
         }
 
         for (Byte b : keys) {
-            Collection<Mana> cm = floatingMana.get(b);
+            Collection<Mana> cm = floatingMana().get(b);
             final List<Mana> pMana = Lists.newArrayList();
             if (isEndOfPhase && !owner.getGame().getPhaseHandler().is(PhaseType.CLEANUP)) {
                 for (final Mana mana : cm) {
@@ -163,7 +178,7 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
             } else {
                 cleared.addAll(cm);
                 cm.clear();
-                floatingMana.putAll(b, pMana);
+                floatingMana().putAll(b, pMana);
             }
         }
 
@@ -174,12 +189,12 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
 
     private void convertManaColor(final byte originalColor, final byte toColor) {
         List<Mana> convert = Lists.newArrayList();
-        Collection<Mana> cm = floatingMana.get(originalColor);
+        Collection<Mana> cm = floatingMana().get(originalColor);
         for (Mana m : cm) {
             convert.add(new Mana(toColor, m.getSourceCard(), m.getManaAbility()));
         }
         cm.clear();
-        floatingMana.putAll(toColor, convert);
+        floatingMana().putAll(toColor, convert);
         owner.updateManaForView();
     }
 
@@ -187,7 +202,7 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
         return removeMana(mana, true);
     }
     public boolean removeMana(final Mana mana, boolean updateView) {
-        boolean result = floatingMana.remove(mana.getColor(), mana);
+        boolean result = floatingMana().remove(mana.getColor(), mana);
         if (result && updateView) {
             owner.updateManaForView();
             owner.getGame().fireEvent(new GameEventManaPool(owner, EventValueChangeType.Removed, mana));
@@ -216,7 +231,7 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
 
     public boolean tryPayCostWithColor(byte colorCode, SpellAbility saPaidFor, ManaCostBeingPaid manaCost, List<Mana> manaSpentToPay) {
         Mana manaFound = null;
-        Collection<Mana> cm = floatingMana.get(colorCode);
+        Collection<Mana> cm = floatingMana().get(colorCode);
 
         for (final Mana mana : cm) {
             if (mana.getManaAbility() != null && !mana.getManaAbility().meetsManaRestrictions(saPaidFor)) {
@@ -253,11 +268,11 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
     }
 
     public final boolean isEmpty() {
-        return floatingMana.isEmpty();
+        return floatingMana().isEmpty();
     }
 
     public final int totalMana() {
-        return floatingMana.values().size();
+        return floatingMana().values().size();
     }
 
     //Account for mana part of ability when undoing it
@@ -265,7 +280,7 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
         if (ma == null) {
             return false;
         }
-        if (floatingMana.isEmpty()) {
+        if (floatingMana().isEmpty()) {
             return false;
         }
 
@@ -274,7 +289,7 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
         boolean manaNotAccountedFor = false;
         // loop over mana produced by mana ability
         for (Mana mana : ma.getLastManaProduced()) {
-            Collection<Mana> poolLane = floatingMana.get(mana.getColor());
+            Collection<Mana> poolLane = floatingMana().get(mana.getColor());
 
             if (poolLane != null && poolLane.contains(mana)) {
                 removeFloating.add(mana);
@@ -321,7 +336,6 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
      * Checks if the given mana cost can be paid from floating mana.
      * @param cost mana cost to pay for
      * @param sa ability to pay for
-     * @param player activating player
      * @param test actual payment is made if this is false
      * @param manaSpentToPay list of mana spent
      * @return whether the floating mana is sufficient to pay the cost fully
@@ -358,7 +372,10 @@ public class ManaPool extends ManaConversionMatrix implements Iterable<Mana> {
 
     @Override
     public Iterator<Mana> iterator() {
-        return floatingMana.values().iterator();
+        // use synchronizedListMultimap
+        synchronized (floatingMana()) {
+            return floatingMana().values().iterator();
+        }
     }
 
 }

forge-game/src/main/java/forge/game/mana/ManaRefundService.java

@@ -6,8 +6,8 @@ import forge.game.event.GameEventZone;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
+import forge.util.CollectionUtil;
 
-import java.util.Collections;
 import java.util.List;
 
 public class ManaRefundService {
@@ -39,7 +39,7 @@ public class ManaRefundService {
         List<SpellAbility> payingAbilities = sa.getPayingManaAbilities();
 
         // start with the most recent
-        Collections.reverse(payingAbilities);
+        CollectionUtil.reverse(payingAbilities);
 
         for (final SpellAbility am : payingAbilities) {
             // What if am is owned by a different player?

forge-game/src/main/java/forge/game/player/Player.java

@@ -1142,7 +1142,7 @@ public class Player extends GameEntity implements Comparable<Player> {
             }
 
             if (toTop != null) {
-                Collections.reverse(toTop); // the last card in list will become topmost in library, have to revert thus.
+                CollectionUtil.reverse(toTop); // the last card in list will become topmost in library, have to revert thus.
                 for (Card c : toTop) {
                     getGame().getAction().moveToLibrary(c, cause, params);
                     numToTop++;
@@ -1669,7 +1669,7 @@ public class Player extends GameEntity implements Comparable<Player> {
         final CardCollection list = new CardCollection(getCardsIn(ZoneType.Library));
 
         // Note: Shuffling once is sufficient.
-        Collections.shuffle(list, MyRandom.getRandom());
+        CollectionUtil.shuffle(list, MyRandom.getRandom());
 
         getZone(ZoneType.Library).setCards(getController().cheatShuffle(list));
 

forge-game/src/main/java/forge/game/spellability/SpellAbility.java

@@ -118,7 +118,23 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     private boolean optionalTrigger = false;
     private ReplacementEffect replacementEffect = null;
     private int sourceTrigger = -1;
-    private List<Object> triggerRemembered = Lists.newArrayList();
+    private List<Object> _triggerRemembered;
+    private List<Object> triggerRemembered() {
+        List<Object> result = _triggerRemembered;
+        if (result == null) {
+            synchronized (this) {
+                result = _triggerRemembered;
+                if (result == null) {
+                    result = Lists.newArrayList();
+                    _triggerRemembered = result;
+                }
+            }
+        }
+        return _triggerRemembered;
+    }
+    private void _setTriggerRemembered(List<Object> tr) {
+        _triggerRemembered = tr;
+    }
 
     private AlternativeCost altCost = null;
     private EnumSet<OptionalCost> optionalCosts = EnumSet.noneOf(OptionalCost.class);
@@ -126,30 +142,153 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
 
     private boolean aftermath = false;
 
+    private boolean skip = false;
     /** The pay costs. */
     private Cost payCosts;
     private SpellAbilityRestriction restrictions;
-    private SpellAbilityCondition conditions = new SpellAbilityCondition();
+    private SpellAbilityCondition _conditions;
+    private SpellAbilityCondition conditions() {
+        SpellAbilityCondition result = _conditions;
+        if (result == null) {
+            synchronized (this) {
+                result = _conditions;
+                if (result == null) {
+                    result = new SpellAbilityCondition();
+                    _conditions = result;
+                }
+            }
+        }
+        return _conditions;
+    }
+    private void _setConditions(SpellAbilityCondition c) {
+        _conditions = c;
+    }
     private AbilitySub subAbility;
 
-    private Map<String, SpellAbility> additionalAbilities = Maps.newHashMap();
+    private Map<String, SpellAbility> _additionalAbilities;
-    private Map<String, List<AbilitySub>> additionalAbilityLists = Maps.newHashMap();
+    private Map<String, SpellAbility> additionalAbilities() {
+        Map<String, SpellAbility> result = _additionalAbilities;
+        if (result == null) {
+            synchronized (this) {
+                result = _additionalAbilities;
+                if (result == null) {
+                    result = Maps.newHashMap();
+                    _additionalAbilities = result;
+                }
+            }
+        }
+        return _additionalAbilities;
+    }
+    private void _setAdditionalAbilities(Map<String, SpellAbility> aa) {
+        _additionalAbilities = aa;
+    }
+    private Map<String, List<AbilitySub>> _additionalAbilityLists;
+    private Map<String, List<AbilitySub>> additionalAbilityLists() {
+        Map<String, List<AbilitySub>> result = _additionalAbilityLists;
+        if (result == null) {
+            synchronized (this) {
+                result = _additionalAbilityLists;
+                if (result == null) {
+                    result = Maps.newHashMap();
+                    _additionalAbilityLists = result;
+                }
+            }
+        }
+        return _additionalAbilityLists;
+    }
+    private void _setAdditionalAbilityLists(Map<String, List<AbilitySub>> al) {
+        _additionalAbilityLists = al;
+    }
 
     protected ApiType api = null;
 
-    private List<Mana> payingMana = Lists.newArrayList();
+    private List<Mana> _payingMana;
-    private List<SpellAbility> paidAbilities = Lists.newArrayList();
+    private List<Mana> payingMana() {
+        List<Mana> result = _payingMana;
+        if (result == null) {
+            synchronized (this) {
+                result = _payingMana;
+                if (result == null) {
+                    result = Lists.newArrayList();
+                    _payingMana = result;
+                }
+            }
+        }
+        return _payingMana;
+    }
+    private void _setPayingMana(List<Mana> m) {
+        _payingMana = m;
+    }
+    private List<SpellAbility> _paidAbilities;
+    private List<SpellAbility> paidAbilities() {
+        List<SpellAbility> result = _paidAbilities;
+        if (result == null) {
+            synchronized (this) {
+                result = _paidAbilities;
+                if (result == null) {
+                    result = Lists.newArrayList();
+                    _paidAbilities = result;
+                }
+            }
+        }
+        return _paidAbilities;
+    }
+    private void _setPaidAbilities(List<SpellAbility> p) {
+        _paidAbilities = p;
+    }
     private Integer xManaCostPaid = null;
 
-    private TreeBasedTable<String, Boolean, CardCollection> paidLists = TreeBasedTable.create();
+    private TreeBasedTable<String, Boolean, CardCollection> _paidLists;
+    private TreeBasedTable<String, Boolean, CardCollection> paidLists() {
+        TreeBasedTable<String, Boolean, CardCollection> result = _paidLists;
+        if (result == null) {
+            synchronized (this) {
+                result = _paidLists;
+                if (result == null) {
+                    result = TreeBasedTable.create();
+                    _paidLists = result;
+                }
+            }
+        }
+        return _paidLists;
+    }
+    private void _setPaidLists(TreeBasedTable<String, Boolean, CardCollection> p) {
+        _paidLists = p;
+    }
 
     private EnumMap<AbilityKey, Object> triggeringObjects = AbilityKey.newMap();
     private EnumMap<AbilityKey, Object> replacingObjects = AbilityKey.newMap();
 
-    private final List<String> pipsToReduce = new ArrayList<>();
+    private List<String> _pipsToReduce;
+    private List<String> pipsToReduce() {
+        List<String> result = _pipsToReduce;
+        if (result == null) {
+            synchronized (this) {
+                result = _pipsToReduce;
+                if (result == null) {
+                    result = new ArrayList<>();
+                    _pipsToReduce = result;
+                }
+            }
+        }
+        return _pipsToReduce;
+    }
 
     private List<AbilitySub> chosenList = null;
-    private CardCollection tappedForConvoke = new CardCollection();
+    private CardCollection _tappedForConvoke;
+    private CardCollection tappedForConvoke() {
+        CardCollection result = _tappedForConvoke;
+        if (result == null) {
+            synchronized (this) {
+                result = _tappedForConvoke;
+                if (result == null) {
+                    result = new CardCollection();
+                    _tappedForConvoke = result;
+                }
+            }
+        }
+        return _tappedForConvoke;
+    }
     private Card sacrificedAsOffering;
     private Card sacrificedAsEmerge;
 
@@ -163,7 +302,26 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     private boolean isCastFromPlayEffect = false;
 
     private TargetRestrictions targetRestrictions;
-    private TargetChoices targetChosen = new TargetChoices();
+    private TargetChoices _targetChosen;
+    private TargetChoices targetChosen() {
+        TargetChoices result = _targetChosen;
+        if (result == null) {
+            synchronized (this) {
+                result = _targetChosen;
+                if (result == null) {
+                    result = new TargetChoices();
+                    _targetChosen = result;
+                }
+            }
+        }
+        return _targetChosen;
+    }
+    private void _setTargetChosen(TargetChoices c) {
+        _targetChosen = c;
+    }
+    private void resetTargetChosen() {
+        _targetChosen = new TargetChoices();
+    }
 
     private Integer dividedValue = null;
 
@@ -174,7 +332,20 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     private CardCollection lastStateBattlefield;
     private CardCollection lastStateGraveyard;
 
-    private CardCollection rollbackEffects = new CardCollection();
+    private CardCollection _rollbackEffects;
+    private CardCollection rollbackEffects() {
+        CardCollection result = _rollbackEffects;
+        if (result == null) {
+            synchronized (this) {
+                result = _rollbackEffects;
+                if (result == null) {
+                    result = new CardCollection();
+                    _rollbackEffects = result;
+                }
+            }
+        }
+        return _rollbackEffects;
+    }
 
     private CardDamageMap damageMap;
     private CardDamageMap preventMap;
@@ -242,12 +413,12 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         if (subAbility != null) {
             subAbility.setHostCard(c);
         }
-        for (SpellAbility sa : additionalAbilities.values()) {
+        for (SpellAbility sa : additionalAbilities().values()) {
             if (sa.getHostCard() != c) {
                 sa.setHostCard(c);
             }
         }
-        for (List<AbilitySub> list : additionalAbilityLists.values()) {
+        for (List<AbilitySub> list : additionalAbilityLists().values()) {
             for (AbilitySub sa : list) {
                 if (sa.getHostCard() != c) {
                     sa.setHostCard(c);
@@ -266,10 +437,10 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         if (subAbility != null) {
             subAbility.setKeyword(kw);
         }
-        for (SpellAbility sa : additionalAbilities.values()) {
+        for (SpellAbility sa : additionalAbilities().values()) {
             sa.setKeyword(kw);
         }
-        for (List<AbilitySub> list : additionalAbilityLists.values()) {
+        for (List<AbilitySub> list : additionalAbilityLists().values()) {
             for (AbilitySub sa : list) {
                 sa.setKeyword(kw);
             }
@@ -467,10 +638,10 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         if (subAbility != null) {
             updated |= subAbility.setActivatingPlayer(player, lki);
         }
-        for (SpellAbility sa : additionalAbilities.values()) {
+        for (SpellAbility sa : additionalAbilities().values()) {
             updated |= sa.setActivatingPlayer(player, lki);
         }
-        for (List<AbilitySub> list : additionalAbilityLists.values()) {
+        for (List<AbilitySub> list : additionalAbilityLists().values()) {
             for (AbilitySub sa : list) {
                 updated |= sa.setActivatingPlayer(player, lki);
             }
@@ -648,10 +819,10 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     }
 
     public SpellAbilityCondition getConditions() {
-        return conditions;
+        return conditions();
     }
     public final void setConditions(final SpellAbilityCondition condition) {
-        conditions = condition;
+        _setConditions(condition);
     }
 
     public boolean metConditions() {
@@ -659,13 +830,13 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     }
 
     public List<Mana> getPayingMana() {
-        return payingMana;
+        return payingMana();
     }
     public void setPayingMana(List<Mana> paying) {
-        payingMana = Lists.newArrayList(paying);
+        _setPayingMana(Lists.newArrayList(paying));
     }
     public final void clearManaPaid() {
-        payingMana.clear();
+        payingMana( ).clear();
     }
 
     public final int getSpendPhyrexianMana() {
@@ -727,42 +898,42 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
 
     public ColorSet getPayingColors() {
         byte colors = 0;
-        for (Mana m : payingMana) {
+        for (Mana m : payingMana()) {
             colors |= m.getColor();
         }
         return ColorSet.fromMask(colors);
     }
 
     public List<SpellAbility> getPayingManaAbilities() {
-        return paidAbilities;
+        return paidAbilities();
     }
 
     // Combined PaidLists
     public TreeBasedTable<String, Boolean, CardCollection> getPaidHash() {
-        return paidLists;
+        return paidLists();
     }
     public void setPaidHash(final TreeBasedTable<String, Boolean, CardCollection> hash) {
-        paidLists = TreeBasedTable.create(hash);
+        _setPaidLists(TreeBasedTable.create(hash));
     }
 
     // use if it doesn't matter if payment was caused by extrinsic cost modifier
     public Iterable<Card> getPaidList(final String str) {
-        return Iterables.concat(paidLists.row(str).values());
+        return Iterables.concat(paidLists().row(str).values());
     }
 
     public CardCollection getPaidList(final String str, final boolean intrinsic) {
-        return paidLists.get(str, intrinsic);
+        return paidLists().get(str, intrinsic);
     }
 
     public void addCostToHashList(final Card c, final String str, final boolean intrinsic) {
-        if (!paidLists.contains(str, intrinsic)) {
+        if (!paidLists().contains(str, intrinsic)) {
-            paidLists.put(str, intrinsic, new CardCollection());
+            paidLists().put(str, intrinsic, new CardCollection());
         }
-        paidLists.get(str, intrinsic).add(c);
+        paidLists().get(str, intrinsic).add(c);
     }
 
     public void resetPaidHash() {
-        paidLists.clear();
+        paidLists().clear();
     }
 
     public Iterable<OptionalCost> getOptionalCosts() {
@@ -774,7 +945,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         optionalCosts = EnumSet.copyOf(optionalCosts);
         optionalCosts.add(cost);
         if (!cost.getPip().isEmpty()) {
-            pipsToReduce.add(cost.getPip());
+            pipsToReduce().add(cost.getPip());
         }
     }
 
@@ -788,7 +959,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
 
     public boolean isKicked() {
         return isOptionalCostPaid(OptionalCost.Kicker1) || isOptionalCostPaid(OptionalCost.Kicker2) ||
-            getRootAbility().getOptionalKeywordAmount(Keyword.MULTIKICKER) > 0;
+                getRootAbility().getOptionalKeywordAmount(Keyword.MULTIKICKER) > 0;
     }
 
     public boolean isEntwine() {
@@ -834,13 +1005,13 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
 
     @Override
     public List<Object> getTriggerRemembered() {
-        return triggerRemembered;
+        return triggerRemembered();
     }
     public void setTriggerRemembered(List<Object> list) {
-        triggerRemembered = list;
+        _setTriggerRemembered(list);
     }
     public void resetTriggerRemembered() {
-        triggerRemembered = Lists.newArrayList();
+        _setTriggerRemembered(Lists.newArrayList());
     }
 
     public Map<AbilityKey, Object> getReplacingObjects() {
@@ -866,7 +1037,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
 
     public void resetOnceResolved() {
         //resetPaidHash(); // FIXME: if uncommented, breaks Dragon Presence, e.g. Orator of Ojutai + revealing a Dragon from hand.
-                           // Is it truly necessary at this point? The paid hash seems to be reset on all SA instance operations.
+        // Is it truly necessary at this point? The paid hash seems to be reset on all SA instance operations.
         // Epic spell keeps original targets
         if (!isEpic()) {
             resetTargets();
@@ -898,7 +1069,9 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     public void setStackDescription(final String s) {
         originalStackDescription = s;
         stackDescription = originalStackDescription;
-        if (StringUtils.isEmpty(description) && StringUtils.isEmpty(hostCard.getView().getText())) {
+        // FIXME: why would the view is null? freezed tracker and the view is not composed yet?
+        String compareHostText = hostCard.getView() == null ? "" : hostCard.getView().getText();
+        if (StringUtils.isEmpty(description) && StringUtils.isEmpty(compareHostText)) {
             setDescription(s);
         }
     }
@@ -944,7 +1117,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
                 }
                 sb.append(payCosts.toString());
                 sb.append(" or ").append(altOnlyMana ? alternateCost.toString() :
-                    StringUtils.uncapitalize(alternateCost.toString()));
+                        StringUtils.uncapitalize(alternateCost.toString()));
                 sb.append(equip && !altOnlyMana ? "." : "");
             } else {
                 sb.append(payCosts.toString());
@@ -959,13 +1132,12 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     }
 
     public void rebuiltDescription() {
-        final StringBuilder sb = new StringBuilder();
 
         // SubAbilities don't have Costs or Cost descriptors
-        sb.append(getCostDescription());
 
-        sb.append(getParam("SpellDescription"));
+        String sb = getCostDescription() +
-        setDescription(sb.toString());
+                getParam("SpellDescription");
+        setDescription(sb);
     }
 
     /** {@inheritDoc} */
@@ -1015,37 +1187,37 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     }
 
     public Map<String, SpellAbility> getAdditionalAbilities() {
-        return additionalAbilities;
+        return additionalAbilities();
     }
     public SpellAbility getAdditionalAbility(final String name) {
         if (hasAdditionalAbility(name)) {
-            return additionalAbilities.get(name);
+            return additionalAbilities().get(name);
         }
         return null;
     }
 
     public boolean hasAdditionalAbility(final String name) {
-        return additionalAbilities.containsKey(name);
+        return additionalAbilities().containsKey(name);
     }
 
     public void setAdditionalAbility(final String name, final SpellAbility sa) {
         if (sa == null) {
-            additionalAbilities.remove(name);
+            additionalAbilities().remove(name);
         } else {
             if (sa instanceof AbilitySub) {
                 ((AbilitySub)sa).setParent(this);
             }
-            additionalAbilities.put(name, sa);
+            additionalAbilities().put(name, sa);
         }
         view.updateDescription(this); //description changes when sub-abilities change
     }
 
     public Map<String, List<AbilitySub>> getAdditionalAbilityLists() {
-        return additionalAbilityLists;
+        return additionalAbilityLists();
     }
     public List<AbilitySub> getAdditionalAbilityList(final String name) {
-        if (additionalAbilityLists.containsKey(name)) {
+        if (additionalAbilityLists().containsKey(name)) {
-            return additionalAbilityLists.get(name);
+            return additionalAbilityLists().get(name);
         } else {
             return ImmutableList.of();
         }
@@ -1053,13 +1225,13 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
 
     public void setAdditionalAbilityList(final String name, final List<AbilitySub> list) {
         if (list == null || list.isEmpty()) {
-            additionalAbilityLists.remove(name);
+            additionalAbilityLists().remove(name);
         } else {
             List<AbilitySub> result = Lists.newArrayList(list);
             for (AbilitySub sa : result) {
                 sa.setParent(this);
             }
-            additionalAbilityLists.put(name, result);
+            additionalAbilityLists().put(name, result);
         }
         view.updateDescription(this);
     }
@@ -1199,18 +1371,18 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
                 clone.changeZoneTable = new CardZoneTable(changeZoneTable);
             }
 
-            clone.payingMana = Lists.newArrayList(payingMana);
+            clone._setPayingMana(Lists.newArrayList(payingMana()));
-            clone.paidAbilities = Lists.newArrayList();
+            clone._setPaidAbilities(Lists.newArrayList());
             clone.setPaidHash(getPaidHash());
 
             if (usesTargeting()) {
                 // the targets need to be cloned, otherwise they might be cleared
-                clone.targetChosen = getTargets().clone();
+                clone._setTargetChosen(getTargets().clone());
             }
 
             // clear maps for copy, the values will be added later
-            clone.additionalAbilities = Maps.newHashMap();
+            clone._setAdditionalAbilities(Maps.newHashMap());
-            clone.additionalAbilityLists = Maps.newHashMap();
+            clone._setAdditionalAbilityLists(Maps.newHashMap());
             // run special copy Ability to make a deep copy
             CardFactory.copySpellAbility(this, clone, host, activ, lki, keepTextChanges);
         } catch (final CloneNotSupportedException e) {
@@ -1381,15 +1553,15 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
                     return false;
                 }
                 switch (related) {
-                case "LEPower" :
+                    case "LEPower" :
-                    if (c.getNetPower() > parentTarget.getNetPower()) {
+                        if (c.getNetPower() > parentTarget.getNetPower()) {
-                        return false;
+                            return false;
-                    }
+                        }
-                    break;
+                        break;
-                case "LECMC" :
+                    case "LECMC" :
-                    if (c.getCMC() > parentTarget.getCMC()) {
+                        if (c.getCMC() > parentTarget.getCMC()) {
-                        return false;
+                            return false;
-                    }
+                        }
                 }
             }
 
@@ -1562,25 +1734,20 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     }
 
     public List<String> getPipsToReduce() {
-        return pipsToReduce;
+        return pipsToReduce();
     }
     public final void clearPipsToReduce() {
-        pipsToReduce.clear();
+        pipsToReduce().clear();
     }
 
     public CardCollection getTappedForConvoke() {
-        return tappedForConvoke;
+        return tappedForConvoke();
     }
     public void addTappedForConvoke(final Card c) {
-        if (tappedForConvoke == null) {
+        tappedForConvoke().add(c);
-            tappedForConvoke = new CardCollection();
-        }
-        tappedForConvoke.add(c);
     }
     public void clearTappedForConvoke() {
-        if (tappedForConvoke != null) {
+        tappedForConvoke().clear();
-            tappedForConvoke.clear();
-        }
     }
 
     public boolean isEmerge() {
@@ -1754,16 +1921,16 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
      * @return the chosenTarget
      */
     public TargetChoices getTargets() {
-        return targetChosen;
+        return targetChosen();
     }
 
     public void setTargets(TargetChoices targets) {
         // TODO should copy the target choices?
-        targetChosen = targets;
+        _setTargetChosen(targets);
     }
 
     public void resetTargets() {
-        targetChosen = new TargetChoices();
+        resetTargetChosen();
     }
 
     /**
@@ -1879,7 +2046,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     }
 
     public Card getTargetCard() {
-        return targetChosen.getFirstTargetedCard();
+        return targetChosen().getFirstTargetedCard();
     }
 
     /**
@@ -1898,7 +2065,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         }
 
         resetTargets();
-        targetChosen.add(card);
+        targetChosen().add(card);
         setStackDescription(getHostCard().getName() + " - targeting " + card);
     }
 
@@ -2258,11 +2425,11 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
                 subAbility.changeText();
             }
         }
-        for (SpellAbility sa : additionalAbilities.values()) {
+        for (SpellAbility sa : additionalAbilities().values()) {
             sa.changeText();
         }
 
-        for (List<AbilitySub> list : additionalAbilityLists.values()) {
+        for (List<AbilitySub> list : additionalAbilityLists().values()) {
             for (AbilitySub sa : list) {
                 sa.changeText();
             }
@@ -2283,11 +2450,11 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
                 subAbility.changeTextIntrinsic(colorMap, typeMap);
             }
         }
-        for (SpellAbility sa : additionalAbilities.values()) {
+        for (SpellAbility sa : additionalAbilities().values()) {
             sa.changeTextIntrinsic(colorMap, typeMap);
         }
 
-        for (List<AbilitySub> list : additionalAbilityLists.values()) {
+        for (List<AbilitySub> list : additionalAbilityLists().values()) {
             for (AbilitySub sa : list) {
                 sa.changeTextIntrinsic(colorMap, typeMap);
             }
@@ -2300,12 +2467,12 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         if (subAbility != null) {
             subAbility.setIntrinsic(i);
         }
-        for (SpellAbility sa : additionalAbilities.values()) {
+        for (SpellAbility sa : additionalAbilities().values()) {
             if (sa.isIntrinsic() != i) {
                 sa.setIntrinsic(i);
             }
         }
-        for (List<AbilitySub> list : additionalAbilityLists.values()) {
+        for (List<AbilitySub> list : additionalAbilityLists().values()) {
             for (AbilitySub sa : list) {
                 if (sa.isIntrinsic() != i) {
                     sa.setIntrinsic(i);
@@ -2505,6 +2672,12 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         return sb.toString();
     }
 
+    public boolean isSkip() {
+        return skip;
+    }
+    public void setSkip(boolean val) {
+        skip = val;
+    }
     public boolean canCastTiming(Player activator) {
         return canCastTiming(getHostCard(), activator);
     }
@@ -2555,14 +2728,14 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     }
 
     public void addRollbackEffect(Card eff) {
-        rollbackEffects.add(eff);
+        rollbackEffects().add(eff);
     }
 
     public void rollback() {
-        for (Card c : rollbackEffects) {
+        for (Card c : rollbackEffects()) {
             c.getGame().getAction().ceaseToExist(c, true);
         }
-        rollbackEffects.clear();
+        rollbackEffects().clear();
     }
 
     public boolean isHidden() {

forge-game/src/main/java/forge/game/zone/Zone.java

@@ -18,7 +18,6 @@
 package forge.game.zone;
 
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -36,6 +35,7 @@ import forge.game.event.EventValueChangeType;
 import forge.game.event.GameEventZone;
 import forge.game.player.Player;
 import forge.util.CollectionSuppliers;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.maps.EnumMapOfLists;
 import forge.util.maps.MapOfLists;
@@ -278,7 +278,7 @@ public class Zone implements java.io.Serializable, Iterable<Card> {
     }
 
     public void shuffle() {
-        Collections.shuffle(cardList, MyRandom.getRandom());
+        CollectionUtil.shuffle(cardList, MyRandom.getRandom());
         onChanged();
     }
 

forge-gui-desktop/src/main/java/forge/itemmanager/CardManager.java

@@ -18,6 +18,7 @@ import forge.screens.home.quest.DialogChooseFormats;
 import forge.screens.home.quest.DialogChooseSets;
 import forge.screens.match.controllers.CDetailPicture;
 import forge.util.CollectionSuppliers;
+import forge.util.CollectionUtil;
 import forge.util.Localizer;
 
 import javax.swing.*;
@@ -98,7 +99,7 @@ public class CardManager extends ItemManager<PaperCard> {
         // Use standard sort + index, for better performance!
         Collections.sort(acceptedEditions);
         if (StaticData.instance().cardArtPreferenceIsLatest())
-            Collections.reverse(acceptedEditions);
+            CollectionUtil.reverse(acceptedEditions);
         Iterator<CardEdition> editionIterator = acceptedEditions.iterator();
         Entry<PaperCard, Integer> candidateEntry = null;
         Entry<PaperCard, Integer> firstCandidateEntryFound = null;

forge-gui-desktop/src/main/java/forge/screens/deckeditor/controllers/CProbabilities.java

@@ -1,7 +1,6 @@
 package forge.screens.deckeditor.controllers;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -14,6 +13,7 @@ import forge.item.InventoryItem;
 import forge.item.PaperCard;
 import forge.screens.deckeditor.CDeckEditorUI;
 import forge.screens.deckeditor.views.VProbabilities;
+import forge.util.CollectionUtil;
 import forge.util.ItemPool;
 import forge.util.MyRandom;
 
@@ -63,7 +63,7 @@ public enum CProbabilities implements ICDoc {
         final List<String> cardProbabilities = new ArrayList<>();
 
         final List<PaperCard> shuffled = deck.toFlatList();
-        Collections.shuffle(shuffled, MyRandom.getRandom());
+        CollectionUtil.shuffle(shuffled, MyRandom.getRandom());
 
         // Log totals of each card for decrementing
         final Map<PaperCard, Integer> cardTotals = new HashMap<>();

forge-gui-desktop/src/main/java/forge/screens/home/quest/DialogChooseSets.java

@@ -13,6 +13,7 @@ import forge.game.GameFormat;
 import forge.gui.SOverlayUtils;
 import forge.localinstance.skin.FSkinProp;
 import forge.model.FModel;
+import forge.util.CollectionUtil;
 import forge.util.Localizer;
 import net.miginfocom.swing.MigLayout;
 import forge.toolbox.FCheckBoxTree.FTreeNode;
@@ -467,7 +468,7 @@ public class DialogChooseSets {
 				FTreeNode setTypeNode = checkBoxTree.getNodeByKey(editionType);
 				if (setTypeNode != null){
 					List<FTreeNode> activeChildNodes = checkBoxTree.getActiveChildNodes(setTypeNode);
-					Collections.shuffle(activeChildNodes);
+					CollectionUtil.shuffle(activeChildNodes);
 					for (int i = 0; i < totalToSelect; i++)
 						checkBoxTree.setNodeCheckStatus(activeChildNodes.get(i), true);
 				}

forge-gui-desktop/src/main/java/forge/screens/home/sanctioned/CSubmenuDraft.java

@@ -26,12 +26,12 @@ import forge.screens.deckeditor.controllers.CEditorDraftingProcess;
 import forge.screens.deckeditor.views.VProbabilities;
 import forge.screens.deckeditor.views.VStatistics;
 import forge.toolbox.FOptionPane;
+import forge.util.CollectionUtil;
 import forge.util.Localizer;
 
 import javax.swing.*;
 import java.awt.event.ActionListener;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -163,7 +163,7 @@ public enum CSubmenuDraft implements ICDoc {
             for(int i = 0; i < maxDecks; i++) {
                 aiIndices.add(i);
             }
-            Collections.shuffle(aiIndices);
+            CollectionUtil.shuffle(aiIndices);
             aiIndices = aiIndices.subList(0, numOpponents);
 
             for(int i : aiIndices) {

forge-gui-desktop/src/main/java/forge/view/SimulateMatch.java

@@ -2,7 +2,6 @@ package forge.view;
 
 import java.io.File;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.List;
@@ -10,6 +9,7 @@ import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
+import forge.util.CollectionUtil;
 import org.apache.commons.lang3.time.StopWatch;
 
 import forge.LobbyPlayer;
@@ -201,7 +201,7 @@ public class SimulateMatch {
         } else {
             log = g1.getGameLog().getLogEntries(GameLogEntryType.MATCH_RESULTS);
         }
-        Collections.reverse(log);
+        CollectionUtil.reverse(log);
         for (GameLogEntry l : log) {
             System.out.println(l);
         }

forge-gui/src/main/java/forge/deck/DeckgenUtil.java

@@ -2,13 +2,13 @@ package forge.deck;
 
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import forge.util.CollectionUtil;
 import org.apache.commons.lang3.tuple.Pair;
 
 import com.google.common.base.Predicate;
@@ -701,7 +701,7 @@ public class DeckgenUtil {
             }else {
                 String matrixKey = (format.equals(DeckFormat.TinyLeaders) ? DeckFormat.Commander : format).toString(); //use Commander for Tiny Leaders
                 List<Map.Entry<PaperCard, Integer>> potentialCards = new ArrayList<>(CardRelationMatrixGenerator.cardPools.get(matrixKey).get(commander.getName()));
-                Collections.shuffle(potentialCards, MyRandom.getRandom());
+                CollectionUtil.shuffle(potentialCards, MyRandom.getRandom());
                 for(Map.Entry<PaperCard,Integer> pair:potentialCards){
                     if(format.isLegalCard(pair.getKey())) {
                         preSelectedCards.add(pair.getKey());
@@ -800,7 +800,7 @@ public class DeckgenUtil {
                 break;
             }
             List<PaperCard> cardList = Lists.newArrayList(colorList);
-            Collections.shuffle(cardList, MyRandom.getRandom());
+            CollectionUtil.shuffle(cardList, MyRandom.getRandom());
             int shortlistlength=400;
             if(cardList.size()<shortlistlength){
                 shortlistlength=cardList.size();

forge-gui/src/main/java/forge/deck/NetDeckArchiveBlock.java

@@ -7,6 +7,7 @@ import forge.gui.GuiBase;
 import forge.gui.download.GuiDownloadZipService;
 import forge.gui.util.SGuiChoose;
 import forge.localinstance.properties.ForgeConstants;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.WaitCallback;
 import forge.util.storage.StorageBase;
@@ -71,7 +72,7 @@ public class NetDeckArchiveBlock extends StorageBase<Deck> {
         }
 
         List<NetDeckArchiveBlock> category = new ArrayList<>(categories.values());
-        Collections.reverse(category);
+        CollectionUtil.reverse(category);
 
         final NetDeckArchiveBlock c = SGuiChoose.oneOrNone("Select a Net Deck Archive Block category", category);
         if (c == null) { return null; }

forge-gui/src/main/java/forge/deck/NetDeckArchiveLegacy.java

@@ -7,6 +7,7 @@ import forge.gui.GuiBase;
 import forge.gui.download.GuiDownloadZipService;
 import forge.gui.util.SGuiChoose;
 import forge.localinstance.properties.ForgeConstants;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.WaitCallback;
 import forge.util.storage.StorageBase;
@@ -71,7 +72,7 @@ public class NetDeckArchiveLegacy extends StorageBase<Deck> {
         }
 
         List<NetDeckArchiveLegacy> category = new ArrayList<>(categories.values());
-        Collections.reverse(category);
+        CollectionUtil.reverse(category);
 
         final NetDeckArchiveLegacy c = SGuiChoose.oneOrNone("Select a Net Deck Archive Legacy category", category);
         if (c == null) { return null; }

forge-gui/src/main/java/forge/deck/NetDeckArchiveModern.java

@@ -7,6 +7,7 @@ import forge.gui.GuiBase;
 import forge.gui.download.GuiDownloadZipService;
 import forge.gui.util.SGuiChoose;
 import forge.localinstance.properties.ForgeConstants;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.WaitCallback;
 import forge.util.storage.StorageBase;
@@ -71,7 +72,7 @@ public class NetDeckArchiveModern extends StorageBase<Deck> {
         }
 
         List<NetDeckArchiveModern> category = new ArrayList<>(categories.values());
-        Collections.reverse(category);
+        CollectionUtil.reverse(category);
 
         final NetDeckArchiveModern c = SGuiChoose.oneOrNone("Select a Net Deck Archive Modern category", category);
         if (c == null) { return null; }

forge-gui/src/main/java/forge/deck/NetDeckArchivePauper.java

@@ -7,6 +7,7 @@ import forge.gui.GuiBase;
 import forge.gui.download.GuiDownloadZipService;
 import forge.gui.util.SGuiChoose;
 import forge.localinstance.properties.ForgeConstants;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.WaitCallback;
 import forge.util.storage.StorageBase;
@@ -71,7 +72,7 @@ public class NetDeckArchivePauper extends StorageBase<Deck> {
         }
 
         List<NetDeckArchivePauper> category = new ArrayList<>(categories.values());
-        Collections.reverse(category);
+        CollectionUtil.reverse(category);
 
         final NetDeckArchivePauper c = SGuiChoose.oneOrNone("Select a Net Deck Archive Pauper category", category);
         if (c == null) { return null; }

forge-gui/src/main/java/forge/deck/NetDeckArchivePioneer.java

@@ -7,6 +7,7 @@ import forge.gui.GuiBase;
 import forge.gui.download.GuiDownloadZipService;
 import forge.gui.util.SGuiChoose;
 import forge.localinstance.properties.ForgeConstants;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.WaitCallback;
 import forge.util.storage.StorageBase;
@@ -71,7 +72,7 @@ public class NetDeckArchivePioneer extends StorageBase<Deck> {
         }
 
         List<NetDeckArchivePioneer> category = new ArrayList<>(categories.values());
-        Collections.reverse(category);
+        CollectionUtil.reverse(category);
 
         final NetDeckArchivePioneer c = SGuiChoose.oneOrNone("Select a Net Deck Archive Pioneer category", category);
         if (c == null) { return null; }

forge-gui/src/main/java/forge/deck/NetDeckArchiveStandard.java

@@ -7,6 +7,7 @@ import forge.gui.GuiBase;
 import forge.gui.download.GuiDownloadZipService;
 import forge.gui.util.SGuiChoose;
 import forge.localinstance.properties.ForgeConstants;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.WaitCallback;
 import forge.util.storage.StorageBase;
@@ -71,7 +72,7 @@ public class NetDeckArchiveStandard extends StorageBase<Deck> {
         }
 
         List<NetDeckArchiveStandard> category = new ArrayList<>(categories.values());
-        Collections.reverse(category);
+        CollectionUtil.reverse(category);
 
         final NetDeckArchiveStandard c = SGuiChoose.oneOrNone("Select a Net Deck Archive Standard category",category);
         if (c == null) { return null; }

forge-gui/src/main/java/forge/deck/NetDeckArchiveVintage.java

@@ -7,6 +7,7 @@ import forge.gui.GuiBase;
 import forge.gui.download.GuiDownloadZipService;
 import forge.gui.util.SGuiChoose;
 import forge.localinstance.properties.ForgeConstants;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.WaitCallback;
 import forge.util.storage.StorageBase;
@@ -71,7 +72,7 @@ public class NetDeckArchiveVintage extends StorageBase<Deck> {
         }
 
         List<NetDeckArchiveVintage> category = new ArrayList<>(categories.values());
-        Collections.reverse(category);
+        CollectionUtil.reverse(category);
 
         final NetDeckArchiveVintage c = SGuiChoose.oneOrNone("Select a Net Deck Archive Vintage category", category);
         if (c == null) { return null; }

forge-gui/src/main/java/forge/gamemodes/limited/BoosterDraft.java

@@ -36,6 +36,7 @@ import forge.localinstance.properties.ForgeConstants;
 import forge.localinstance.properties.ForgePreferences;
 import forge.model.CardBlock;
 import forge.model.FModel;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.ItemPool;
 import forge.util.Localizer;
@@ -474,7 +475,7 @@ public class BoosterDraft implements IBoosterDraft {
                         // Maybe the AI could have more knowledge about the other players.
                         // Like don't pass to players that have revealed certain cards or colors
                         // But random is probably fine for now
-                        Collections.shuffle(dredgers);
+                        CollectionUtil.shuffle(dredgers);
                         passToPlayer = dredgers.get(0);
                     } else {
                         // Human player, so we need to ask them
@@ -556,7 +557,7 @@ public class BoosterDraft implements IBoosterDraft {
             }
         }
 
-        Collections.shuffle(brokers);
+        CollectionUtil.shuffle(brokers);
         for(LimitedPlayer pl : brokers) {
             pl.activateBrokers(this.players);
         }

forge-gui/src/main/java/forge/gamemodes/limited/CardThemedDeckBuilder.java

@@ -1,7 +1,6 @@
 package forge.gamemodes.limited;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -34,6 +33,7 @@ import forge.item.IPaperCard;
 import forge.item.PaperCard;
 import forge.localinstance.properties.ForgePreferences;
 import forge.model.FModel;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 
 /**
@@ -628,7 +628,7 @@ public class CardThemedDeckBuilder extends DeckGeneratorBase {
             possibleList.removeAll(StaticData.instance().getCommonCards().getAllCards(secondKeyCard.getName()));
         }
         //Iterator<PaperCard> iRandomPool = CardRanker.rankCardsInDeck(possibleList.subList(0, targetSize <= possibleList.size() ? targetSize : possibleList.size())).iterator();
-        Collections.shuffle(possibleList);
+        CollectionUtil.shuffle(possibleList);
         Iterator<PaperCard> iRandomPool = possibleList.iterator();
         while (deckList.size() < targetSize) {
             if (logToConsole) {
@@ -864,7 +864,7 @@ public class CardThemedDeckBuilder extends DeckGeneratorBase {
         if (secondKeyCard != null) {
             possibleList.removeAll(StaticData.instance().getCommonCards().getAllCards(secondKeyCard.getName()));
         }
-        Collections.shuffle(possibleList);
+        CollectionUtil.shuffle(possibleList);
         //addManaCurveCards(CardRanker.rankCardsInDeck(possibleList.subList(0, targetSize*3 <= possibleList.size() ? targetSize*3 : possibleList.size())),
                 //num, "Random Card");
         addManaCurveCards(possibleList, num, "Random Card");

forge-gui/src/main/java/forge/gamemodes/limited/LimitedPlayer.java

@@ -11,6 +11,7 @@ import forge.deck.DeckSection;
 import forge.gui.util.SGuiChoose;
 import forge.item.PaperCard;
 import forge.model.FModel;
+import forge.util.CollectionUtil;
 import forge.util.TextUtil;
 
 import java.util.*;
@@ -723,7 +724,7 @@ public class LimitedPlayer {
     }
 
     public PaperCard pickFromArchdemonCurse(DraftPack chooseFrom) {
-        Collections.shuffle(chooseFrom);
+        CollectionUtil.shuffle(chooseFrom);
         reduceArchdemonOfPalianoCurse();
         return chooseFrom.get(0);
     }

forge-gui/src/main/java/forge/gamemodes/limited/LimitedPlayerAI.java

@@ -10,10 +10,10 @@ import forge.deck.DeckSection;
 import forge.deck.generation.DeckGeneratorBase;
 import forge.item.PaperCard;
 import forge.localinstance.properties.ForgePreferences;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import org.apache.commons.lang3.tuple.Pair;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -81,7 +81,7 @@ public class LimitedPlayerAI extends LimitedPlayer {
             // For Paliano, if player has revealed anything, try to avoid that color
             // For Regicide, don't choose one of my colors
         }
-        Collections.shuffle(colors);
+        CollectionUtil.shuffle(colors);
         return colors.get(0);
     }
 
@@ -89,7 +89,7 @@ public class LimitedPlayerAI extends LimitedPlayer {
     protected String removeWithAny(PaperCard bestPick, List<String> options) {
         // If we have multiple remove from draft options, do none of them for now
 
-        Collections.shuffle(options);
+        CollectionUtil.shuffle(options);
         if (options.get(0).equals("Animus of Predation")) {
             if (removeWithAnimus(bestPick)) {
                 return "Animus of Predation";
@@ -254,7 +254,7 @@ public class LimitedPlayerAI extends LimitedPlayer {
 
     @Override
     protected CardEdition chooseEdition(List<CardEdition> possibleEditions) {
-        Collections.shuffle(possibleEditions);
+        CollectionUtil.shuffle(possibleEditions);
         return possibleEditions.get(0);
     }
 

forge-gui/src/main/java/forge/gamemodes/limited/SealedCardPoolGenerator.java

@@ -37,6 +37,7 @@ import forge.localinstance.skin.FSkinProp;
 import forge.model.CardBlock;
 import forge.model.FModel;
 import forge.model.UnOpenedMeta;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.Localizer;
 import forge.util.MyRandom;
@@ -173,7 +174,7 @@ public class SealedCardPoolGenerator {
             case Prerelease:
                 ArrayList<CardEdition> editions = Lists.newArrayList(StaticData.instance().getEditions().getPrereleaseEditions());
                 Collections.sort(editions);
-                Collections.reverse(editions);
+                CollectionUtil.reverse(editions);
 
                 CardEdition chosenEdition = SGuiChoose.oneOrNone(Localizer.getInstance().getMessage("lblChooseAnEdition"), editions);
                 if (chosenEdition == null) {

forge-gui/src/main/java/forge/gamemodes/limited/WinstonDraft.java

@@ -1,7 +1,6 @@
 package forge.gamemodes.limited;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Stack;
 
@@ -12,6 +11,7 @@ import com.google.common.collect.Iterables;
 import forge.deck.CardPool;
 import forge.deck.Deck;
 import forge.item.PaperCard;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 
 public class WinstonDraft extends BoosterDraft {
@@ -47,7 +47,7 @@ public class WinstonDraft extends BoosterDraft {
                 }
             }
         }
-        Collections.shuffle(this.deck, MyRandom.getRandom());
+        CollectionUtil.shuffle(this.deck, MyRandom.getRandom());
 
         // Create three Winston piles, adding the top card from the Winston deck to start each pile
         this.piles = new ArrayList<>();

forge-gui/src/main/java/forge/gamemodes/planarconquest/ConquestData.java

@@ -42,6 +42,7 @@ import forge.localinstance.properties.ForgeConstants;
 import forge.localinstance.skin.ISkinImage;
 import forge.model.FModel;
 import forge.util.CardTranslation;
+import forge.util.CollectionUtil;
 import forge.util.FileUtil;
 import forge.util.Localizer;
 import forge.util.XmlReader;
@@ -589,7 +590,7 @@ public final class ConquestData {
                         path.add(current.loc);
                         current = current.came_from;
                     }
-                    Collections.reverse(path); //reverse path so it begins with start location
+                    CollectionUtil.reverse(path); //reverse path so it begins with start location
                     return path;
                 }
 

forge-gui/src/main/java/forge/gamemodes/quest/BoosterUtils.java

@@ -20,10 +20,10 @@ package forge.gamemodes.quest;
 import static forge.gamemodes.quest.QuestUtilCards.isLegalInQuestFormat;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 
+import forge.util.CollectionUtil;
 import org.apache.commons.lang3.StringUtils;
 
 import com.google.common.base.Predicate;
@@ -236,7 +236,7 @@ public final class BoosterUtils {
                     preferredColors.clear();
                     int numberOfColors = COLOR_COUNT_PROBABILITIES[(int) (MyRandom.getRandom().nextDouble() * COLOR_COUNT_PROBABILITIES.length)];
                     if (numberOfColors < 6) {
-                        Collections.shuffle(possibleColors);
+                        CollectionUtil.shuffle(possibleColors);
                         for (int i = 0; i < numberOfColors; i++) {
                             preferredColors.add(possibleColors.get(i));
                         }
@@ -391,7 +391,7 @@ public final class BoosterUtils {
 
         final int size = allowedColors == null ? 0 : allowedColors.size();
         if (allowedColors != null) {
-            Collections.shuffle(allowedColors);
+            CollectionUtil.shuffle(allowedColors);
         }
 
         int cntMade = 0, iAttempt = 0;

forge-gui/src/main/java/forge/gamemodes/quest/MainWorldEventDuelManager.java

@@ -3,7 +3,6 @@ package forge.gamemodes.quest;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 
 import forge.gamemodes.quest.data.QuestPreferences;
@@ -12,6 +11,7 @@ import forge.gamemodes.quest.data.QuestPreferences.QPref;
 import forge.gamemodes.quest.io.MainWorldDuelReader;
 import forge.model.FModel;
 import forge.util.CollectionSuppliers;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.maps.EnumMapOfLists;
 import forge.util.maps.MapOfLists;
@@ -259,7 +259,7 @@ public class MainWorldEventDuelManager implements QuestEventDuelManagerInterface
     public void randomizeOpponents() {
         for (QuestEventDifficulty qd : sortedDuels.keySet()) {
             List<QuestEventDuel> list = (List<QuestEventDuel>) sortedDuels.get(qd);
-            Collections.shuffle(list, MyRandom.getRandom());
+            CollectionUtil.shuffle(list, MyRandom.getRandom());
         }
     }
     

forge-gui/src/main/java/forge/gamemodes/quest/QuestController.java

@@ -19,7 +19,6 @@ package forge.gamemodes.quest;
 
 import java.io.File;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -51,6 +50,7 @@ import forge.item.PreconDeck;
 import forge.localinstance.properties.ForgeConstants;
 import forge.model.FModel;
 import forge.player.GamePlayerUtil;
+import forge.util.CollectionUtil;
 import forge.util.storage.IStorage;
 import forge.util.storage.StorageBase;
 
@@ -612,7 +612,7 @@ public class QuestController {
                 }
             }
 
-            Collections.shuffle(unlockedChallengeIds);
+            CollectionUtil.shuffle(unlockedChallengeIds);
 
             maxChallenges = Math.min(maxChallenges, unlockedChallengeIds.size());
 

forge-gui/src/main/java/forge/gamemodes/quest/QuestEventCommanderDuelManager.java

@@ -1,7 +1,6 @@
 package forge.gamemodes.quest;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import forge.deck.CardPool;
@@ -12,6 +11,7 @@ import forge.deck.DeckProxy;
 import forge.gamemodes.quest.data.QuestPreferences;
 import forge.item.PaperCard;
 import forge.model.FModel;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 
 /**
@@ -198,6 +198,6 @@ public class QuestEventCommanderDuelManager implements QuestEventDuelManagerInte
      * Randomizes the list of Commander Duels.
      */
     public void randomizeOpponents(){
-        Collections.shuffle(commanderDuels);
+        CollectionUtil.shuffle(commanderDuels);
     }
 }

forge-gui/src/main/java/forge/gamemodes/quest/QuestEventDraft.java

@@ -46,6 +46,7 @@ import forge.item.PaperCard;
 import forge.model.CardBlock;
 import forge.model.FModel;
 import forge.player.GamePlayerUtil;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.NameGenerator;
 import forge.util.TextUtil;
@@ -856,7 +857,7 @@ public class QuestEventDraft implements IQuestEvent {
             return null;
         }
 
-        Collections.shuffle(possibleFormats);
+        CollectionUtil.shuffle(possibleFormats);
         return getDraftOrNull(quest, possibleFormats.get(0));
 
     }
@@ -885,7 +886,7 @@ public class QuestEventDraft implements IQuestEvent {
                 System.err.println("Warning: no valid set combinations were detected when trying to generate a draft tournament for the format: " + format);
                 return null;
             }
-            Collections.shuffle(possibleSetCombinations);
+            CollectionUtil.shuffle(possibleSetCombinations);
             event.boosterConfiguration = possibleSetCombinations.get(0);
         }
 
@@ -902,7 +903,7 @@ public class QuestEventDraft implements IQuestEvent {
         players.add("6");
         players.add("7");
 
-        Collections.shuffle(players);
+        CollectionUtil.shuffle(players);
 
         // Initialize tournament
         for (int i = 0; i < players.size(); i++) {

forge-gui/src/main/java/forge/gamemodes/quest/QuestEventDuelManager.java

@@ -20,7 +20,6 @@ package forge.gamemodes.quest;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 
 import forge.gamemodes.quest.data.QuestPreferences;
@@ -29,6 +28,7 @@ import forge.gamemodes.quest.data.QuestPreferences.QPref;
 import forge.gamemodes.quest.io.QuestDuelReader;
 import forge.model.FModel;
 import forge.util.CollectionSuppliers;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.maps.EnumMapOfLists;
 import forge.util.maps.MapOfLists;
@@ -236,7 +236,7 @@ public class QuestEventDuelManager implements QuestEventDuelManagerInterface {
     public void randomizeOpponents() {
         for (QuestEventDifficulty qd : sortedDuels.keySet()) {
             List<QuestEventDuel> list = (List<QuestEventDuel>) sortedDuels.get(qd);
-            Collections.shuffle(list, MyRandom.getRandom());
+            CollectionUtil.shuffle(list, MyRandom.getRandom());
         }
     }
 

forge-gui/src/main/java/forge/gamemodes/quest/QuestEventLDADuelManager.java

@@ -19,7 +19,6 @@ package forge.gamemodes.quest;
 
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 
 import forge.deck.CardArchetypeLDAGenerator;
@@ -30,6 +29,7 @@ import forge.gamemodes.quest.data.QuestPreferences.DifficultyPrefs;
 import forge.gamemodes.quest.data.QuestPreferences.QPref;
 import forge.model.FModel;
 import forge.util.CollectionSuppliers;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.maps.EnumMapOfLists;
 import forge.util.maps.MapOfLists;
@@ -236,7 +236,7 @@ public class QuestEventLDADuelManager implements QuestEventDuelManagerInterface
     public void randomizeOpponents() {
         for (QuestEventDifficulty qd : sortedDuels.keySet()) {
             List<QuestEventDuel> list = (List<QuestEventDuel>) sortedDuels.get(qd);
-            Collections.shuffle(list, MyRandom.getRandom());
+            CollectionUtil.shuffle(list, MyRandom.getRandom());
         }
     }
 }

forge-gui/src/main/java/forge/gamemodes/quest/QuestUtilCards.java

@@ -42,13 +42,13 @@ import forge.item.generation.UnOpenedProduct;
 import forge.localinstance.properties.ForgePreferences.FPref;
 import forge.model.FModel;
 import forge.util.Aggregates;
+import forge.util.CollectionUtil;
 import forge.util.ItemPool;
 import forge.util.MyRandom;
 import org.apache.commons.lang3.tuple.Pair;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map.Entry;
 
@@ -678,7 +678,7 @@ public final class QuestUtilCards {
             editions.add(e);
         }
 
-        Collections.shuffle(editions);
+        CollectionUtil.shuffle(editions);
 
         int numberOfBoxes = Math.min(Math.max(count / 2, 1), editions.size());
 

forge-gui/src/main/java/forge/gamemodes/quest/QuestUtilUnlockSets.java

@@ -17,12 +17,6 @@
  */
 package forge.gamemodes.quest;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.EnumSet;
-import java.util.List;
-import java.util.Map;
-
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
@@ -36,6 +30,7 @@ import forge.item.PaperCard;
 import forge.item.SealedTemplate;
 import forge.item.generation.UnOpenedProduct;
 import forge.model.FModel;
+import forge.util.CollectionUtil;
 import forge.util.TextUtil;
 import forge.util.storage.IStorage;
 import org.apache.commons.lang3.tuple.ImmutablePair;
@@ -176,7 +171,7 @@ public class QuestUtilUnlockSets {
             options.add(set.left);
             // System.out.println("Padded with: " + fillers.get(i).getName());
         }
-        Collections.reverse(options);
+        CollectionUtil.reverse(options);
 
         if (FModel.getQuestPreferences().getPrefInt(QPref.UNLIMITED_UNLOCKING) == 0) {
             return options.subList(0, Math.min(options.size(), Math.min(8, 2 + ((qData.getAchievements().getWin()) / 50))));

forge-gui/src/main/java/forge/gamemodes/quest/setrotation/QueueRandomRotation.java

@@ -1,10 +1,10 @@
 package forge.gamemodes.quest.setrotation;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Random;
 import forge.model.FModel;
+import forge.util.CollectionUtil;
 
 import static java.lang.Integer.min;
 
@@ -27,7 +27,7 @@ public class QueueRandomRotation implements ISetRotation {
         int seed = FModel.getQuest().getName().hashCode();
         Random rnd = new Random(seed);
         List<String> shuffledSets = new ArrayList<>(allSets);
-        Collections.shuffle(shuffledSets, rnd);
+        CollectionUtil.shuffle(shuffledSets, rnd);
 
         List<String> currentCodes = new ArrayList<>();
         int outRotations = FModel.getQuest().getAchievements().getWin() / rotateAfterWins;

forge-gui/src/main/java/forge/gamemodes/tournament/system/AbstractTournament.java

@@ -2,7 +2,6 @@ package forge.gamemodes.tournament.system;
 
 import java.io.Serializable;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import com.google.common.collect.Lists;
@@ -12,6 +11,7 @@ import forge.LobbyPlayer;
 import forge.deck.DeckGroup;
 import forge.game.player.RegisteredPlayer;
 import forge.player.GamePlayerUtil;
+import forge.util.CollectionUtil;
 import forge.util.MyRandom;
 import forge.util.TextUtil;
 
@@ -44,7 +44,7 @@ public abstract class AbstractTournament implements Serializable {
 
     public void initializeTournament() {
         // "Randomly" seed players to start tournament
-        Collections.shuffle(remainingPlayers, MyRandom.getRandom());
+        CollectionUtil.shuffle(remainingPlayers, MyRandom.getRandom());
         generateActivePairings();
         initialized = true;
     }

forge-gui/src/main/java/forge/gamemodes/tournament/system/TournamentSwiss.java

@@ -1,13 +1,13 @@
 package forge.gamemodes.tournament.system;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 
 import com.google.common.collect.Lists;
+import forge.util.CollectionUtil;
 
 @SuppressWarnings("serial")
 public class TournamentSwiss extends AbstractTournament {
@@ -40,7 +40,7 @@ public class TournamentSwiss extends AbstractTournament {
         activeRound++;
 
         // Randomize players, then sort by scores
-        Collections.shuffle(allPlayers);
+        CollectionUtil.shuffle(allPlayers);
         sortAllPlayers("swiss");
 
         if (allPlayers.size() % 2 == 1) {

forge-gui/src/main/java/forge/itemmanager/GroupDef.java

@@ -15,6 +15,7 @@ import forge.deck.DeckProxy;
 import forge.item.InventoryItem;
 import forge.item.PaperCard;
 import forge.model.FModel;
+import forge.util.CollectionUtil;
 import forge.util.Localizer;
 
 public enum GroupDef {
@@ -239,7 +240,7 @@ public enum GroupDef {
         //build sorted list of sets
         List<CardEdition> sortedSets = Lists.newArrayList(FModel.getMagicDb().getEditions());
         Collections.sort(sortedSets);
-        Collections.reverse(sortedSets);
+        CollectionUtil.reverse(sortedSets);
 
         int groupNum = 0;
         String[] setGroups = new String[sortedSets.size()];

forge-gui/src/main/java/forge/player/HumanCostDecision.java

@@ -465,7 +465,7 @@ public class HumanCostDecision extends CostDecisionMakerBase {
     }
 
     private PaymentDecision exileFromTopGraveType(final int nNeeded, final CardCollection typeList) {
-        Collections.reverse(typeList);
+        CollectionUtil.reverse(typeList);
         return PaymentDecision.card(Iterables.limit(typeList, nNeeded));
     }
 

forge-gui/src/main/java/forge/player/PlayerControllerHuman.java

@@ -1141,7 +1141,7 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
         }
         endTempShowCards();
         if(topOfDeck)
-            Collections.reverse(choices);
+            CollectionUtil.reverse(choices);
         CardCollection result = new CardCollection();
         gameCacheMove.addToList(choices, result);
         return result;
@@ -1663,7 +1663,7 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
 
         Collections.sort(sortedResults);
         if (!call) {
-            Collections.reverse(sortedResults);
+            CollectionUtil.reverse(sortedResults);
         }
         return getGui().one(sa.getHostCard().getName() + " - " + localizer.getMessage("lblChooseAResult"), sortedResults).equals(labelsSrc[0]);
     }