More use of Visitor pattern - allowing iteration over all cards in the game without allocating a temporary collection.

.gitattributes

@@ -250,6 +250,7 @@ forge-core/src/main/java/forge/util/PredicateString.java -text
 forge-core/src/main/java/forge/util/ReflectionUtil.java -text
 forge-core/src/main/java/forge/util/TextUtil.java -text
 forge-core/src/main/java/forge/util/ThreadUtil.java -text
+forge-core/src/main/java/forge/util/Visitor.java -text
 forge-core/src/main/java/forge/util/maps/EnumMapOfLists.java -text
 forge-core/src/main/java/forge/util/maps/EnumMapToAmount.java -text
 forge-core/src/main/java/forge/util/maps/HashMapOfLists.java -text

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

@@ -0,0 +1,11 @@
+package forge.util;
+
+public abstract class Visitor<T> {
+    public abstract void visit(T object);
+
+    public void visitAll(Iterable<? extends T> objects) {
+        for (T obj : objects) {
+            visit(obj);
+        }
+    }
+}

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

@@ -66,6 +66,7 @@ import forge.game.zone.ZoneType;
 import forge.util.Aggregates;
 import forge.util.FCollection;
 import forge.util.FCollectionView;
+import forge.util.Visitor;
 
 /**
  * Represents the state of a <i>single game</i>, a new instance is created for each game.
@@ -429,17 +430,27 @@ public class Game {
         return card;
     }
 
+    // Allows visiting cards in game without allocating a temporary list.
+    public void forEachCardInGame(Visitor<Card> visitor) {
+        for (final Player player : getPlayers()) {
+            visitor.visitAll(player.getZone(ZoneType.Graveyard).getCards());
+            visitor.visitAll(player.getZone(ZoneType.Hand).getCards());
+            visitor.visitAll(player.getZone(ZoneType.Library).getCards());
+            visitor.visitAll(player.getZone(ZoneType.Battlefield).getCards(false));
+            visitor.visitAll(player.getZone(ZoneType.Exile).getCards());
+            visitor.visitAll(player.getZone(ZoneType.Command).getCards());
+        }
+        visitor.visitAll(getStackZone().getCards());
+    }
     public CardCollectionView getCardsInGame() {
         final CardCollection all = new CardCollection();
-        for (final Player player : getPlayers()) {
+        Visitor<Card> visitor = new Visitor<Card>() {
-            all.addAll(player.getZone(ZoneType.Graveyard).getCards());
+            @Override
-            all.addAll(player.getZone(ZoneType.Hand).getCards());
+            public void visit(Card card) {
-            all.addAll(player.getZone(ZoneType.Library).getCards());
+                all.add(card);
-            all.addAll(player.getZone(ZoneType.Battlefield).getCards(false));
+            }
-            all.addAll(player.getZone(ZoneType.Exile).getCards());
+        };
-            all.addAll(player.getZone(ZoneType.Command).getCards());
+        forEachCardInGame(visitor);
-        }
-        all.addAll(getStackZone().getCards());
         return all;
     }
 

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

@@ -53,6 +53,7 @@ import forge.util.CollectionSuppliers;
 import forge.util.Expressions;
 import forge.util.FCollectionView;
 import forge.util.ThreadUtil;
+import forge.util.Visitor;
 import forge.util.maps.HashMapOfLists;
 import forge.util.maps.MapOfLists;
 
@@ -547,24 +548,26 @@ public class GameAction {
         }
 
         // search for cards with static abilities
-        final CardCollectionView allCards = game.getCardsInGame();
         final ArrayList<StaticAbility> staticAbilities = new ArrayList<StaticAbility>();
         final List<Card> staticList = new ArrayList<Card>();
-        for (final Card c : allCards) {
+        game.forEachCardInGame(new Visitor<Card>() {
-            for (int i = 0; i < c.getStaticAbilities().size(); i++) {
+            @Override
-               StaticAbility stAb = c.getStaticAbilities().get(i);
+            public void visit(Card c) {
-               if (stAb.getMapParams().get("Mode").equals("Continuous")) {
+                for (int i = 0; i < c.getStaticAbilities().size(); i++) {
-                   staticAbilities.add(stAb);
+                    StaticAbility stAb = c.getStaticAbilities().get(i);
-               }
+                    if (stAb.getMapParams().get("Mode").equals("Continuous")) {
-               if (stAb.isTemporary()) {
+                        staticAbilities.add(stAb);
-                   c.removeStaticAbility(stAb);
+                    }
-                   i--;
+                    if (stAb.isTemporary()) {
-               }
+                        c.removeStaticAbility(stAb);
+                        i--;
+                    }
+                 }
+                 if (!c.getStaticCommandList().isEmpty()) {
+                     staticList.add(c);
+                 }
             }
-            if (!c.getStaticCommandList().isEmpty()) {
+        });
-                staticList.add(c);
-            }
-        }
 
         final Comparator<StaticAbility> comp = new Comparator<StaticAbility>() {
             @Override

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

@@ -67,6 +67,7 @@ import forge.util.FCollection;
 import forge.util.FCollectionView;
 import forge.util.Lang;
 import forge.util.TextUtil;
+import forge.util.Visitor;
 import forge.util.maps.HashMapOfLists;
 import forge.util.maps.MapOfLists;
 
@@ -2746,7 +2747,9 @@ public class Card extends GameEntity implements Comparable<Card> {
         visitKeywords(state, visitor);
         return visitor.getKeywords();
     }
-    public final void visitKeywords(CardState state, KeywordVisitor visitor) {
+    // Allows traversing the card's keywords without needing to concat a bunch
+    // of lists. Optimizes common operations such as hasKeyword().
+    public final void visitKeywords(CardState state, Visitor<String> visitor) {
         visitUnhiddenKeywords(state, visitor);
         visitHiddenExtreinsicKeywords(visitor);
     }
@@ -2847,7 +2850,7 @@ public class Card extends GameEntity implements Comparable<Card> {
         }
         return keywords;
     }
-    private void visitUnhiddenKeywords(CardState state, KeywordVisitor visitor) {
+    private void visitUnhiddenKeywords(CardState state, Visitor<String> visitor) {
         if (changedCardKeywords.isEmpty()) {
             // Fast path that doesn't involve temp allocations.
             for (String kw : state.getIntrinsicKeywords()) {
@@ -3063,7 +3066,7 @@ public class Card extends GameEntity implements Comparable<Card> {
         visitHiddenExtreinsicKeywords(visitor);
         return visitor.getKeywords();
     }
-    private void visitHiddenExtreinsicKeywords(KeywordVisitor visitor) {
+    private void visitHiddenExtreinsicKeywords(Visitor<String> visitor) {
         for (String keyword : hiddenExtrinsicKeyword) {
             if (keyword == null) {
                 continue;
@@ -6397,14 +6400,8 @@ public class Card extends GameEntity implements Comparable<Card> {
         return view;
     }
 
-    // Interface that allows traversing the card's keywords without needing to
-    // concat a bunch of lists. Optimizes common operations such as hasKeyword().
-    private interface KeywordVisitor {
-        public void visit(String keyword);
-    }
-
     // Counts number of instances of a given keyword.
-    private static final class CountKeywordVisitor implements KeywordVisitor {
+    private static final class CountKeywordVisitor extends Visitor<String> {
         private String keyword;
         private int count;
 
@@ -6426,7 +6423,7 @@ public class Card extends GameEntity implements Comparable<Card> {
     }
 
     // Collects all the keywords into a list.
-    private static final class ListKeywordVisitor implements KeywordVisitor {
+    private static final class ListKeywordVisitor extends Visitor<String> {
         private ArrayList<String> keywords = new ArrayList<>();
 
         @Override

forge-game/src/main/java/forge/game/replacement/ReplacementHandler.java

@@ -23,11 +23,11 @@ import forge.game.GameLogEntryType;
 import forge.game.ability.AbilityFactory;
 import forge.game.ability.AbilityUtils;
 import forge.game.card.Card;
-import forge.game.card.CardCollectionView;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
 import forge.util.FileSection;
+import forge.util.Visitor;
 
 import org.apache.commons.lang3.StringUtils;
 
@@ -280,20 +280,25 @@ public class ReplacementHandler {
     }
 
     public void cleanUpTemporaryReplacements() {
-        final CardCollectionView absolutelyAllCards = game.getCardsInGame();
+        game.forEachCardInGame(new Visitor<Card>() {
-        for (final Card c : absolutelyAllCards) {
+            @Override
-            for (int i = 0; i < c.getReplacementEffects().size(); i++) {
+            public void visit(Card c) {
-                ReplacementEffect rep = c.getReplacementEffects().get(i);
+                for (int i = 0; i < c.getReplacementEffects().size(); i++) {
-                if (rep.isTemporary()) {
+                    ReplacementEffect rep = c.getReplacementEffects().get(i);
-                    c.removeReplacementEffect(rep);
+                    if (rep.isTemporary()) {
-                    i--;
+                        c.removeReplacementEffect(rep);
+                        i--;
+                    }
                 }
             }
-        }
+        });
-        for (final Card c : absolutelyAllCards) {
+        game.forEachCardInGame(new Visitor<Card>() {
-            for (int i = 0; i < c.getReplacementEffects().size(); i++) {
+            @Override
-                c.getReplacementEffects().get(i).setTemporarilySuppressed(false);
+            public void visit(Card c) {
+                for (int i = 0; i < c.getReplacementEffects().size(); i++) {
+                    c.getReplacementEffects().get(i).setTemporarilySuppressed(false);
+                }
             }
-        }
+        });
     }
 }

forge-game/src/main/java/forge/game/trigger/TriggerHandler.java

@@ -25,7 +25,6 @@ import forge.game.ability.AbilityUtils;
 import forge.game.ability.ApiType;
 import forge.game.ability.effects.CharmEffect;
 import forge.game.card.Card;
-import forge.game.card.CardCollectionView;
 import forge.game.card.CardUtil;
 import forge.game.phase.PhaseType;
 import forge.game.player.Player;
@@ -34,6 +33,7 @@ import forge.game.spellability.AbilitySub;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
+import forge.util.Visitor;
 
 import java.util.*;
 
@@ -57,21 +57,26 @@ public class TriggerHandler {
     }
 
     public final void cleanUpTemporaryTriggers() {
-        final CardCollectionView absolutelyAllCards = game.getCardsInGame();
+        game.forEachCardInGame(new Visitor<Card>() {
-        for (final Card c : absolutelyAllCards) {
+            @Override
-            for (int i = 0; i < c.getTriggers().size(); i++) {
+            public void visit(Card c) {
-                Trigger trigger = c.getTriggers().get(i);
+                for (int i = 0; i < c.getReplacementEffects().size(); i++) {
-                if (trigger.isTemporary()) {
+                    Trigger trigger = c.getTriggers().get(i);
-                    c.removeTrigger(trigger);
+                    if (trigger.isTemporary()) {
-                    i--;
+                        c.removeTrigger(trigger);
+                        i--;
+                    }
                 }
             }
-        }
+        });
-        for (final Card c : absolutelyAllCards) {
+        game.forEachCardInGame(new Visitor<Card>() {
-            for (int i = 0; i < c.getTriggers().size(); i++) {
+            @Override
-                c.getTriggers().get(i).setTemporarilySuppressed(false);
+            public void visit(Card c) {
+                for (int i = 0; i < c.getReplacementEffects().size(); i++) {
+                    c.getTriggers().get(i).setTemporarilySuppressed(false);
+                }
             }
-        }
+        });
     }
 
     public final boolean hasDelayedTriggers() {