Use caching for CombatView (plus some random NPE and concurrency fixes).

.gitattributes

@@ -496,6 +496,7 @@ forge-game/src/main/java/forge/game/event/GameEventCardRegenerated.java -text
 forge-game/src/main/java/forge/game/event/GameEventCardSacrificed.java -text
 forge-game/src/main/java/forge/game/event/GameEventCardStatsChanged.java -text
 forge-game/src/main/java/forge/game/event/GameEventCardTapped.java -text
+forge-game/src/main/java/forge/game/event/GameEventCombatChanged.java -text
 forge-game/src/main/java/forge/game/event/GameEventCombatEnded.java -text
 forge-game/src/main/java/forge/game/event/GameEventFlipCoin.java -text
 forge-game/src/main/java/forge/game/event/GameEventGameFinished.java -text

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

@@ -440,7 +440,7 @@ public class Game implements IGameStateObject {
         return card.getZone();
     }
 
-    public List<Card> getCardsIn(final ZoneType zone) {
+    public synchronized List<Card> getCardsIn(final ZoneType zone) {
         if (zone == ZoneType.Stack) {
             return getStackZone().getCards();
         }

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

@@ -4,9 +4,11 @@ import forge.game.Game;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.card.CardFactoryUtil;
+import forge.game.event.GameEventCombatChanged;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.TargetRestrictions;
 import forge.game.trigger.TriggerType;
+
 import org.apache.commons.lang3.StringUtils;
 
 import java.util.ArrayList;
@@ -29,13 +31,14 @@ public class BecomesBlockedEffect extends SpellAbilityEffect {
 
     @Override
     public void resolve(SpellAbility sa) {
-
+        boolean isCombatChanged = false;
         final Game game = sa.getActivatingPlayer().getGame();
         final TargetRestrictions tgt = sa.getTargetRestrictions();
         for (final Card c : getTargetCards(sa)) {
             if ((tgt == null) || c.canBeTargetedBy(sa)) {
                 game.getCombat().setBlocked(c, true);
                 if (!c.getDamageHistory().getCreatureGotBlockedThisCombat()) {
+                    isCombatChanged = true;
                     final HashMap<String, Object> runParams = new HashMap<String, Object>();
                     runParams.put("Attacker", c);
                     runParams.put("Blockers", new ArrayList<Card>());
@@ -50,5 +53,8 @@ public class BecomesBlockedEffect extends SpellAbilityEffect {
             }
         }
 
+        if (isCombatChanged) {
+            game.fireEvent(new GameEventCombatChanged());
+        }
     }
 }

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

@@ -11,6 +11,7 @@ import forge.game.ability.AbilityUtils;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.*;
 import forge.game.combat.Combat;
+import forge.game.event.GameEventCombatChanged;
 import forge.game.player.Player;
 import forge.game.player.PlayerActionConfirmMode;
 import forge.game.spellability.AbilitySub;
@@ -511,6 +512,7 @@ public class ChangeZoneEffect extends SpellAbilityEffect {
                             // Blockeres are already declared, set this to unblocked
                             game.getCombat().addAttacker(tgtC, defenders.get(0));
                             game.getCombat().getBandOfAttacker(tgtC).setBlocked(false);
+                            game.fireEvent(new GameEventCombatChanged());
                         }
                     }
                     if (sa.hasParam("Tapped") || sa.hasParam("Ninjutsu")) {
@@ -880,6 +882,7 @@ public class ChangeZoneEffect extends SpellAbilityEffect {
                         final List<GameEntity> e = combat.getDefenders();
                         final GameEntity defender = player.getController().chooseSingleEntityForEffect(e, sa, "Declare " + c);
                         combat.addAttacker(c, defender);
+                        game.fireEvent(new GameEventCombatChanged());
                     }
                 }
                 if (sa.hasParam("Blocking")) {
@@ -891,6 +894,7 @@ public class ChangeZoneEffect extends SpellAbilityEffect {
                             if (combat.isAttacking(attacker)) {
                                 combat.addBlocker(attacker, c);
                                 combat.orderAttackersForDamageAssignment(c);
+                                game.fireEvent(new GameEventCombatChanged());
                             }
                         }
                     }

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

@@ -15,6 +15,7 @@ import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.card.CardFactory;
 import forge.game.card.CardLists;
+import forge.game.event.GameEventCombatChanged;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.TargetRestrictions;
@@ -189,6 +190,7 @@ public class CopyPermanentEffect extends SpellAbilityEffect {
                     if (sa.hasParam("CopyAttacking") && game.getPhaseHandler().inCombat()) {
                         final GameEntity defender = AbilityUtils.getDefinedPlayers(hostCard, sa.getParam("CopyAttacking"), sa).get(0);
                         game.getCombat().addAttacker(copyInPlay, defender);
+                        game.fireEvent(new GameEventCombatChanged());
                     }
 
                     if (sa.hasParam("AttachedTo")) {

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

@@ -27,6 +27,7 @@ import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.card.CardFactory;
 import forge.game.combat.Combat;
+import forge.game.event.GameEventCombatChanged;
 import forge.game.event.GameEventTokenCreated;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
@@ -297,6 +298,7 @@ public class TokenEffect extends SpellAbilityEffect {
                     }
                 }
 
+                boolean combatChanged = false;
                 final Game game = controller.getGame();
                 for (final Card c : tokens) {
                     if (this.tokenAttacking && game.getPhaseHandler().inCombat()) {
@@ -304,6 +306,7 @@ public class TokenEffect extends SpellAbilityEffect {
                         final List<GameEntity> defs = combat.getDefenders();
                         final GameEntity defender = c.getController().getController().chooseSingleEntityForEffect(defs, sa, "Choose which defender to attack with " + c, false);
                         combat.addAttacker(c, defender);
+                        combatChanged = true;
                     }
                     if (this.tokenBlocking != null && game.getPhaseHandler().inCombat()) {
                         Combat combat = game.getPhaseHandler().getCombat();
@@ -315,6 +318,7 @@ public class TokenEffect extends SpellAbilityEffect {
                             } else {
                                 // TODO Flash Foliage: set blocked; attackerBlocked trigger; damage 
                             }
+                            combatChanged = true;
                         }
                     }
                     if (remember) {
@@ -336,6 +340,10 @@ public class TokenEffect extends SpellAbilityEffect {
                         }
                     }
                 }
+
+                if (combatChanged) {
+                    game.fireEvent(new GameEventCombatChanged());
+                }
             }
         }
     }

forge-game/src/main/java/forge/game/event/GameEventCombatChanged.java

@@ -0,0 +1,13 @@
+package forge.game.event;
+
+public class GameEventCombatChanged extends GameEvent {
+
+    public GameEventCombatChanged() {
+    }
+
+    @Override
+    public <T> T visit(IGameEventVisitor<T> visitor) {
+        return visitor.visit(this);
+    }
+
+}

forge-game/src/main/java/forge/game/event/IGameEventVisitor.java

@@ -18,6 +18,7 @@ public interface IGameEventVisitor<T> {
     T visit(GameEventCardTapped event);
     T visit(GameEventCardStatsChanged event);
     T visit(GameEventCardCounters event);
+    T visit(GameEventCombatChanged event);
     T visit(GameEventCombatEnded event);
     T visit(GameEventGameFinished event);
     T visit(GameEventGameOutcome event);
@@ -60,6 +61,8 @@ public interface IGameEventVisitor<T> {
         public T visit(GameEventCardStatsChanged event) { return null; }
         public T visit(GameEventCardCounters event) { return null; }
         public T visit(GameEventCardPhased event) { return null; }
+        public T visit(GameEventCombatChanged event) { return null; }
+        public T visit(GameEventCombatEnded event) { return null; }
         public T visit(GameEventGameFinished event) { return null; }
         public T visit(GameEventGameOutcome event) { return null; }
         public T visit(GameEventFlipCoin event) { return null; }
@@ -84,7 +87,6 @@ public interface IGameEventVisitor<T> {
         public T visit(GameEventTurnPhase event) { return null; }
         public T visit(GameEventPlayerDamaged event) { return null; }
         public T visit(GameEventZone event) { return null; }
-        public T visit(GameEventCombatEnded event) { return null; }
     }
 }
 

forge-game/src/main/java/forge/game/phase/PhaseHandler.java

@@ -473,7 +473,7 @@ public class PhaseHandler implements java.io.Serializable, IGameStateObject {
         }
     }
 
-    private Combat declareAttackersTurnBasedAction() {
+    private void declareAttackersTurnBasedAction() {
         Player whoDeclares = playerDeclaresAttackers == null || playerDeclaresAttackers.hasLost() ? playerTurn : playerDeclaresAttackers;
 
         if (CombatUtil.canAttack(playerTurn)) {
@@ -481,7 +481,7 @@ public class PhaseHandler implements java.io.Serializable, IGameStateObject {
         }
 
         if (game.isGameOver()) { // they just like to close window at any moment
-            return null;
+            return;
         }
 
         combat.removeAbsentCombatants();
@@ -544,7 +544,8 @@ public class PhaseHandler implements java.io.Serializable, IGameStateObject {
         for (final Card c : combat.getAttackers()) {
             CombatUtil.checkDeclaredAttacker(game, c, combat);
         }
-        return combat;
+
+        game.fireEvent(new GameEventCombatChanged());
     }
 
 
@@ -686,6 +687,8 @@ public class PhaseHandler implements java.io.Serializable, IGameStateObject {
 
             a.getDamageHistory().setCreatureGotBlockedThisCombat(true);
         }
+
+        game.fireEvent(new GameEventCombatChanged());
     }
 
 

forge-gui-desktop/src/main/java/forge/screens/match/TargetingOverlay.java

@@ -45,6 +45,7 @@ import forge.view.CardView;
 import forge.view.CombatView;
 import forge.view.FView;
 import forge.view.GameEntityView;
+import forge.view.IGameView;
 import forge.view.arcane.CardPanel;
 
 /**
@@ -364,7 +365,10 @@ public enum TargetingOverlay {
             if (overlaystate == 0) { return; }
 
             // Arc drawing
-            assembleArcs(MatchUtil.getGameView().getCombat());
+            final IGameView gameView = MatchUtil.getGameView();
+            if (gameView != null) {
+                assembleArcs(gameView.getCombat());
+            }
 
             if (arcsCombat.isEmpty() && arcsOther.isEmpty()) { return; }
 

forge-gui-desktop/src/main/java/forge/screens/match/VMatchUI.java

@@ -8,6 +8,7 @@ import forge.sound.MusicPlaylist;
 import forge.sound.SoundSystem;
 import forge.toolbox.FButton;
 import forge.view.FView;
+import forge.view.IGameView;
 
 import javax.swing.*;
 
@@ -207,7 +208,8 @@ public enum VMatchUI implements IVTopLevelUI {
      */
     @Override
     public boolean onClosing(FScreen screen) {
-        if (!MatchUtil.getGameView().isGameOver()) {
+        final IGameView gameView = MatchUtil.getGameView();
+        if (gameView != null && !gameView.isGameOver()) {
             MatchUtil.concede();
             return false; //delay hiding tab even if concede successful
         }

forge-gui/src/main/java/forge/control/FControlGameEventHandler.java

@@ -28,6 +28,7 @@ import forge.game.event.GameEventCardDamaged;
 import forge.game.event.GameEventCardPhased;
 import forge.game.event.GameEventCardStatsChanged;
 import forge.game.event.GameEventCardTapped;
+import forge.game.event.GameEventCombatChanged;
 import forge.game.event.GameEventCombatEnded;
 import forge.game.event.GameEventGameFinished;
 import forge.game.event.GameEventGameOutcome;
@@ -97,8 +98,13 @@ public class FControlGameEventHandler extends IGameEventVisitor.Base<Void> {
     private final AtomicBoolean combatUpdPlanned = new AtomicBoolean(false);
     @Override
     public Void visit(GameEventPlayerPriority event) {
-        if (!isMainHandler || combatUpdPlanned.getAndSet(true)) { return null; }
+        updateCombat();
+        return null;
+    }
 
+    public void updateCombat() {
+        if (!isMainHandler || combatUpdPlanned.getAndSet(true)) { return; }
+
         FThreads.invokeInEdtNowOrLater(gameView.getGui(), new Runnable() {
             @Override
             public void run() {
@@ -106,7 +112,6 @@ public class FControlGameEventHandler extends IGameEventVisitor.Base<Void> {
                 MatchUtil.getController().showCombat(gameView.getCombat());
             }
         });
-        return null;
     }
 
     private final AtomicBoolean turnUpdPlanned = new AtomicBoolean(false);
@@ -352,9 +357,19 @@ public class FControlGameEventHandler extends IGameEventVisitor.Base<Void> {
         return super.visit(event);
     }
 
+    @Override
+    public Void visit(GameEventCombatChanged event) {
+        gameView.refreshCombat();
+        updateCombat();
+        return null;
+    }
+
     @Override
     public Void visit(GameEventCombatEnded event) {
         if (!isMainHandler) { return null; }
+
+        gameView.refreshCombat();
+        updateCombat();
 
         // This should remove sword/shield icons from combatants by the time game moves to M2
         updateManyCards(gameView.getCardViews(event.attackers));

forge-gui/src/main/java/forge/match/MatchUtil.java

@@ -60,6 +60,7 @@ import forge.util.NameGenerator;
 import forge.util.gui.SOptionPane;
 import forge.view.Cache;
 import forge.view.CardView;
+import forge.view.CombatView;
 import forge.view.GameEntityView;
 import forge.view.LocalGameView;
 import forge.view.PlayerView;
@@ -80,6 +81,8 @@ public class MatchUtil {
     public static final Cache<SpellAbility, SpellAbilityView> spabs = new Cache<>();
     /** Cache of stack items. */
     public static final Cache<SpellAbilityStackInstance, StackItemView> stackItems = new Cache<>();
+    /** Cache of combat. */
+    public static CombatView cachedCombatView = null;
 
     private static int humanCount;
     private static final EventBus uiEvents;

forge-gui/src/main/java/forge/match/input/InputAttack.java

@@ -313,6 +313,6 @@ public class InputAttack extends InputSyncronizedBase {
         showMessage(message);
 
         updatePrompt();
-        MatchUtil.getController().showCombat(getController().getCombat(combat)); // redraw sword icons
+        MatchUtil.getController().showCombat(getController().getCombat()); // redraw sword icons
     }
 }

forge-gui/src/main/java/forge/match/input/InputBlock.java

@@ -24,6 +24,7 @@ import forge.game.card.CardLists;
 import forge.game.card.CardPredicates.Presets;
 import forge.game.combat.Combat;
 import forge.game.combat.CombatUtil;
+import forge.game.event.GameEventCombatChanged;
 import forge.game.player.Player;
 import forge.game.zone.ZoneType;
 import forge.match.MatchUtil;
@@ -86,7 +87,7 @@ public class InputBlock extends InputSyncronizedBase {
             showMessage(message);
         }
 
-        MatchUtil.getController().showCombat(getController().getCombat(combat));
+        MatchUtil.getController().showCombat(getController().getCombat());
     }
 
     /** {@inheritDoc} */
@@ -147,7 +148,9 @@ public class InputBlock extends InputSyncronizedBase {
                 }
             }
 
-            if (!isCorrectAction) {
+            if (isCorrectAction) {
+                card.getGame().fireEvent(new GameEventCombatChanged());
+            } else {
                 flashIncorrectAction();
             }
         }

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

@@ -1080,11 +1080,11 @@ public class PlayerControllerHuman extends PlayerController {
      */
     @Override
     public List<AbilitySub> chooseModeForAbility(SpellAbility sa, int min, int num) {
-        List<AbilitySub> choices = CharmEffect.makePossibleOptions(sa);
+        List<SpellAbilityView> choices = getSpellAbilityViews(CharmEffect.makePossibleOptions(sa));
         String modeTitle = String.format("%s activated %s - Choose a mode", sa.getActivatingPlayer(), sa.getHostCard());
-        List<AbilitySub> chosen = new ArrayList<AbilitySub>();
+        List<AbilitySub> chosen = Lists.newArrayListWithCapacity(num);
         for (int i = 0; i < num; i++) {
-            AbilitySub a;
+            SpellAbilityView a;
             if (i < min) {
                 a = SGuiChoose.one(getGui(), modeTitle, choices);
             }
@@ -1096,7 +1096,7 @@ public class PlayerControllerHuman extends PlayerController {
             }
 
             choices.remove(a);
-            chosen.add(a);
+            chosen.add((AbilitySub) getSpellAbility(a));
         }
         return chosen;
     }
@@ -1556,8 +1556,8 @@ public class PlayerControllerHuman extends PlayerController {
      * @return
      * @see forge.view.LocalGameView#getCombat(forge.game.combat.Combat)
      */
-    public CombatView getCombat(Combat c) {
+    public CombatView getCombat() {
-        return gameView.getCombat(c);
+        return gameView.getCombat();
     }
 
     /**
@@ -1663,8 +1663,8 @@ public class PlayerControllerHuman extends PlayerController {
      * @see forge.view.LocalGameView#getSpellAbilityViews(java.util.List)
      */
     public final List<SpellAbilityView> getSpellAbilityViews(
-            List<SpellAbility> cards) {
+            final List<? extends SpellAbility> spabs) {
-        return gameView.getSpellAbilityViews(cards);
+        return gameView.getSpellAbilityViews(spabs);
     }
 
     /**

forge-gui/src/main/java/forge/view/LocalGameView.java

@@ -186,30 +186,39 @@ public abstract class LocalGameView implements IGameView {
 
     @Override
     public CombatView getCombat() {
-        return getCombat(game.getCombat());
+        synchronized (MatchUtil.class) {
+            if (MatchUtil.cachedCombatView != null) {
+                return MatchUtil.cachedCombatView;
+            }
+
+            final Combat combat = game.getCombat();
+            final CombatView combatView;
+            if (combat == null) {
+                combatView = null;
+            } else {
+                combatView = new CombatView();
+                for (final AttackingBand b : combat.getAttackingBands()) {
+                    if (b == null) continue;
+                    final GameEntity defender = combat.getDefenderByAttacker(b);
+                    final List<Card> blockers = combat.getBlockers(b);
+                    final boolean isBlocked = b.isBlocked() == Boolean.TRUE;
+                    combatView.addAttackingBand(
+                            getCardViews(b.getAttackers()),
+                            getGameEntityView(defender),
+                            blockers == null || !isBlocked ? null : getCardViews(blockers),
+                                    blockers == null ? null : getCardViews(blockers));
+                }
+            }
+            MatchUtil.cachedCombatView = combatView;
+            return combatView;
+        }
     }
 
-    /* (non-Javadoc)
+    public final void refreshCombat() {
-     * @see forge.view.IGameView#getCombat()
+        synchronized (MatchUtil.class) {
-     */
+            MatchUtil.cachedCombatView = null;
-    public CombatView getCombat(final Combat combat) {
+            this.getCombat();
-        if (combat == null) {
-            return null;
         }
-
-        final CombatView combatView = new CombatView();
-        for (final AttackingBand b : combat.getAttackingBands()) {
-            if (b == null) continue;
-            final GameEntity defender = combat.getDefenderByAttacker(b);
-            final List<Card> blockers = combat.getBlockers(b);
-            final boolean isBlocked = b.isBlocked() == Boolean.TRUE;
-            combatView.addAttackingBand(
-                    getCardViews(b.getAttackers()),
-                    getGameEntityView(defender),
-                    blockers == null || !isBlocked ? null : getCardViews(blockers),
-                    blockers == null ? null : getCardViews(blockers));
-        }
-        return combatView;
     }
 
     @Override
@@ -559,7 +568,7 @@ public abstract class LocalGameView implements IGameView {
         }
     };
 
-    public final List<SpellAbilityView> getSpellAbilityViews(final List<SpellAbility> cards) {
+    public final List<SpellAbilityView> getSpellAbilityViews(final List<? extends SpellAbility> cards) {
         return ViewUtil.transformIfNotNull(cards, FN_GET_SPAB_VIEW);
     }
 

forge-gui/src/main/java/forge/view/ViewUtil.java

@@ -135,7 +135,7 @@ public final class ViewUtil {
         return view;
     }
 
-    public static <T,V> List<V> transformIfNotNull(final Iterable<T> input, final Function<T, V> transformation) {
+    public static <T,V> List<V> transformIfNotNull(final Iterable<? extends T> input, final Function<T, V> transformation) {
         final List<V> ret = Lists.newLinkedList();
         synchronized (input) {
             for (final T t : input) {