From 648756d1b8cc49cfaf4d6b1721fad4bfa517341d Mon Sep 17 00:00:00 2001 From: Myrd Date: Wed, 31 Dec 2014 03:19:41 +0000 Subject: [PATCH] More use of Visitor pattern - allowing iteration over all cards in the game without allocating a temporary collection. --- .gitattributes | 1 + .../src/main/java/forge/util/Visitor.java | 11 +++++++ forge-game/src/main/java/forge/game/Game.java | 29 +++++++++++----- .../src/main/java/forge/game/GameAction.java | 33 ++++++++++--------- .../src/main/java/forge/game/card/Card.java | 19 +++++------ .../game/replacement/ReplacementHandler.java | 31 +++++++++-------- .../forge/game/trigger/TriggerHandler.java | 31 +++++++++-------- 7 files changed, 94 insertions(+), 61 deletions(-) create mode 100644 forge-core/src/main/java/forge/util/Visitor.java diff --git a/.gitattributes b/.gitattributes index 8e461bc6660..eff21f7ffd0 100644 --- a/.gitattributes +++ b/.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 diff --git a/forge-core/src/main/java/forge/util/Visitor.java b/forge-core/src/main/java/forge/util/Visitor.java new file mode 100644 index 00000000000..608fae2af85 --- /dev/null +++ b/forge-core/src/main/java/forge/util/Visitor.java @@ -0,0 +1,11 @@ +package forge.util; + +public abstract class Visitor { + public abstract void visit(T object); + + public void visitAll(Iterable objects) { + for (T obj : objects) { + visit(obj); + } + } +} diff --git a/forge-game/src/main/java/forge/game/Game.java b/forge-game/src/main/java/forge/game/Game.java index 82846a81df0..4b5e10d4954 100644 --- a/forge-game/src/main/java/forge/game/Game.java +++ b/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 single game, 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 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()) { - all.addAll(player.getZone(ZoneType.Graveyard).getCards()); - all.addAll(player.getZone(ZoneType.Hand).getCards()); - all.addAll(player.getZone(ZoneType.Library).getCards()); - all.addAll(player.getZone(ZoneType.Battlefield).getCards(false)); - all.addAll(player.getZone(ZoneType.Exile).getCards()); - all.addAll(player.getZone(ZoneType.Command).getCards()); - } - all.addAll(getStackZone().getCards()); + Visitor visitor = new Visitor() { + @Override + public void visit(Card card) { + all.add(card); + } + }; + forEachCardInGame(visitor); return all; } diff --git a/forge-game/src/main/java/forge/game/GameAction.java b/forge-game/src/main/java/forge/game/GameAction.java index 953cfd297ea..0c6cdf01f9f 100644 --- a/forge-game/src/main/java/forge/game/GameAction.java +++ b/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 staticAbilities = new ArrayList(); final List staticList = new ArrayList(); - for (final Card c : allCards) { - for (int i = 0; i < c.getStaticAbilities().size(); i++) { - StaticAbility stAb = c.getStaticAbilities().get(i); - if (stAb.getMapParams().get("Mode").equals("Continuous")) { - staticAbilities.add(stAb); - } - if (stAb.isTemporary()) { - c.removeStaticAbility(stAb); - i--; - } + game.forEachCardInGame(new Visitor() { + @Override + public void visit(Card c) { + for (int i = 0; i < c.getStaticAbilities().size(); i++) { + StaticAbility stAb = c.getStaticAbilities().get(i); + if (stAb.getMapParams().get("Mode").equals("Continuous")) { + staticAbilities.add(stAb); + } + if (stAb.isTemporary()) { + c.removeStaticAbility(stAb); + i--; + } + } + if (!c.getStaticCommandList().isEmpty()) { + staticList.add(c); + } } - if (!c.getStaticCommandList().isEmpty()) { - staticList.add(c); - } - } + }); final Comparator comp = new Comparator() { @Override diff --git a/forge-game/src/main/java/forge/game/card/Card.java b/forge-game/src/main/java/forge/game/card/Card.java index 0d4bd24cddf..440d52b2a16 100644 --- a/forge-game/src/main/java/forge/game/card/Card.java +++ b/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 { 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 visitor) { visitUnhiddenKeywords(state, visitor); visitHiddenExtreinsicKeywords(visitor); } @@ -2847,7 +2850,7 @@ public class Card extends GameEntity implements Comparable { } return keywords; } - private void visitUnhiddenKeywords(CardState state, KeywordVisitor visitor) { + private void visitUnhiddenKeywords(CardState state, Visitor 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 { visitHiddenExtreinsicKeywords(visitor); return visitor.getKeywords(); } - private void visitHiddenExtreinsicKeywords(KeywordVisitor visitor) { + private void visitHiddenExtreinsicKeywords(Visitor visitor) { for (String keyword : hiddenExtrinsicKeyword) { if (keyword == null) { continue; @@ -6397,14 +6400,8 @@ public class Card extends GameEntity implements Comparable { 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 { private String keyword; private int count; @@ -6426,7 +6423,7 @@ public class Card extends GameEntity implements Comparable { } // Collects all the keywords into a list. - private static final class ListKeywordVisitor implements KeywordVisitor { + private static final class ListKeywordVisitor extends Visitor { private ArrayList keywords = new ArrayList<>(); @Override diff --git a/forge-game/src/main/java/forge/game/replacement/ReplacementHandler.java b/forge-game/src/main/java/forge/game/replacement/ReplacementHandler.java index 4278a6565a8..862b0471fa0 100644 --- a/forge-game/src/main/java/forge/game/replacement/ReplacementHandler.java +++ b/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(); - for (final Card c : absolutelyAllCards) { - for (int i = 0; i < c.getReplacementEffects().size(); i++) { - ReplacementEffect rep = c.getReplacementEffects().get(i); - if (rep.isTemporary()) { - c.removeReplacementEffect(rep); - i--; + game.forEachCardInGame(new Visitor() { + @Override + public void visit(Card c) { + for (int i = 0; i < c.getReplacementEffects().size(); i++) { + ReplacementEffect rep = c.getReplacementEffects().get(i); + if (rep.isTemporary()) { + c.removeReplacementEffect(rep); + i--; + } } } - } - for (final Card c : absolutelyAllCards) { - for (int i = 0; i < c.getReplacementEffects().size(); i++) { - c.getReplacementEffects().get(i).setTemporarilySuppressed(false); + }); + game.forEachCardInGame(new Visitor() { + @Override + public void visit(Card c) { + for (int i = 0; i < c.getReplacementEffects().size(); i++) { + c.getReplacementEffects().get(i).setTemporarilySuppressed(false); + } } - } + }); } } diff --git a/forge-game/src/main/java/forge/game/trigger/TriggerHandler.java b/forge-game/src/main/java/forge/game/trigger/TriggerHandler.java index b8662520b9c..426deb86322 100644 --- a/forge-game/src/main/java/forge/game/trigger/TriggerHandler.java +++ b/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(); - for (final Card c : absolutelyAllCards) { - for (int i = 0; i < c.getTriggers().size(); i++) { - Trigger trigger = c.getTriggers().get(i); - if (trigger.isTemporary()) { - c.removeTrigger(trigger); - i--; + game.forEachCardInGame(new Visitor() { + @Override + public void visit(Card c) { + for (int i = 0; i < c.getReplacementEffects().size(); i++) { + Trigger trigger = c.getTriggers().get(i); + if (trigger.isTemporary()) { + c.removeTrigger(trigger); + i--; + } } } - } - for (final Card c : absolutelyAllCards) { - for (int i = 0; i < c.getTriggers().size(); i++) { - c.getTriggers().get(i).setTemporarilySuppressed(false); + }); + game.forEachCardInGame(new Visitor() { + @Override + public void visit(Card c) { + for (int i = 0; i < c.getReplacementEffects().size(); i++) { + c.getTriggers().get(i).setTemporarilySuppressed(false); + } } - } + }); } public final boolean hasDelayedTriggers() {