- Saving a deck now properly updates the deck in the lobby

- Prevent NPE when loading a deck with nonexistent cards
- Clean up warning messages about deck conformity
- Lots of code cleanup and simplifications

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

@@ -204,12 +204,15 @@ public class Deck extends DeckBase implements Iterable<Entry<DeckSection, CardPo
     }
 
     public CardPool getAllCardsInASinglePool() {
-        CardPool allCards = new CardPool(); // will count cards in this pool to enforce restricted
+        return getAllCardsInASinglePool(true);
+    }
+    public CardPool getAllCardsInASinglePool(final boolean includeCommander) {
+        final CardPool allCards = new CardPool(); // will count cards in this pool to enforce restricted
         allCards.addAll(this.getMain());
         if (this.has(DeckSection.Sideboard)) {
             allCards.addAll(this.get(DeckSection.Sideboard));
         }
-        if (this.has(DeckSection.Commander)) {
+        if (includeCommander && this.has(DeckSection.Commander)) {
             allCards.addAll(this.get(DeckSection.Commander));
         }
         // do not include schemes / avatars and any non-regular cards

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

@@ -17,6 +17,18 @@
  */
 package forge.deck;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map.Entry;
+
+import org.apache.commons.lang3.Range;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableSet;
+
 import forge.StaticData;
 import forge.card.CardRules;
 import forge.card.CardType;
@@ -28,17 +40,6 @@ import forge.item.IPaperCard;
 import forge.item.PaperCard;
 import forge.util.Aggregates;
 
-import org.apache.commons.lang3.Range;
-import org.apache.commons.lang3.tuple.ImmutablePair;
-
-import com.google.common.base.Predicate;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map.Entry;
-
 /**
  * GameType is an enum to determine the type of current game. :)
  */
@@ -66,8 +67,7 @@ public enum DeckFormat {
             return true;
         }
     }) {
-        private final HashSet<String> bannedCommanders = new HashSet<String>(Arrays.asList(
+        private final ImmutableSet<String> bannedCommanders = ImmutableSet.of("Derevi, Empyrial Tactician", "Erayo, Soratami Ascendant", "Rofellos, Llanowar Emissary");
-                "Derevi, Empyrial Tactician", "Erayo, Soratami Ascendant", "Rofellos, Llanowar Emissary"));
 
         @Override
         public boolean isLegalCommander(CardRules rules) {
@@ -102,6 +102,10 @@ public enum DeckFormat {
         cardPoolFilter = cardPoolFilter0;
     }
 
+    private boolean hasCommander() {
+        return this == Commander || this == TinyLeaders;
+    }
+
     /**
      * Smart value of.
      *
@@ -159,58 +163,40 @@ public enum DeckFormat {
         // Adjust minimum base on number of Advantageous Proclamation or similar cards
 
         if (deckSize < min) {
-            return String.format("should have a minimum of %d cards", min);
+            return String.format("should have at least %d cards", min);
         }
 
         if (deckSize > max) {
-            return String.format("should not exceed a maximum of %d cards", max);
+            return String.format("should have no more than %d cards", max);
         }
 
-        if (this == Commander || this == TinyLeaders) { //Must contain exactly 1 legendary Commander and a sideboard of 10 or zero cards.
+        if (hasCommander()) { //Must contain exactly 1 legendary Commander and a sideboard of 10 or zero cards.
-        	final CardPool cmd = deck.get(DeckSection.Commander);
+            final CardPool cmd = deck.get(DeckSection.Commander);
-        	if (cmd == null || cmd.isEmpty()) {
+            if (cmd == null || cmd.isEmpty()) {
-        		return "is missing a commander";
+                return "is missing a commander";
-        	}
+            }
-        	if (!isLegalCommander(cmd.get(0).getRules())) {
+            if (!isLegalCommander(cmd.get(0).getRules())) {
-        		return "has an illegal commander";
+                return "has an illegal commander";
-        	}
+            }
 
-        	ColorSet cmdCI = cmd.get(0).getRules().getColorIdentity();
+            final ColorSet cmdCI = cmd.get(0).getRules().getColorIdentity();
-        	List<PaperCard> erroneousCI = new ArrayList<PaperCard>();
+            final List<PaperCard> erroneousCI = new ArrayList<PaperCard>();
 
-        	for (Entry<PaperCard, Integer> cp : deck.get(DeckSection.Main)) {
+            for (final Entry<PaperCard, Integer> cp : deck.get(DeckSection.Main)) {
-        		if (!cp.getKey().getRules().getColorIdentity().hasNoColorsExcept(cmdCI.getColor())) {
+                if (!cp.getKey().getRules().getColorIdentity().hasNoColorsExcept(cmdCI.getColor())) {
-        			erroneousCI.add(cp.getKey());
-        		}
-        	}
-        	if (deck.has(DeckSection.Sideboard)) {
-        		for (Entry<PaperCard, Integer> cp : deck.get(DeckSection.Sideboard)) {
-        			if (!cp.getKey().getRules().getColorIdentity().hasNoColorsExcept(cmdCI.getColor())) {
-        				erroneousCI.add(cp.getKey());
-        			}
-        		}
-        	}
-
-        	if (erroneousCI.size() > 0) {
-        		StringBuilder sb = new StringBuilder("contains card that do not match the commanders color identity:");
-
-        		for (PaperCard cp : erroneousCI) {
-        			sb.append("\n").append(cp.getName());
-        		}
-
-        		return sb.toString();
-        	}
-        }
-
-        if (cardPoolFilter != null) {
-            List<PaperCard> erroneousCI = new ArrayList<PaperCard>();
-            for (Entry<PaperCard, Integer> cp : deck.getAllCardsInASinglePool()) {
-                if (!cardPoolFilter.apply(cp.getKey().getRules())) {
                     erroneousCI.add(cp.getKey());
                 }
             }
+            if (deck.has(DeckSection.Sideboard)) {
+                for (final Entry<PaperCard, Integer> cp : deck.get(DeckSection.Sideboard)) {
+                    if (!cp.getKey().getRules().getColorIdentity().hasNoColorsExcept(cmdCI.getColor())) {
+                        erroneousCI.add(cp.getKey());
+                    }
+                }
+            }
+
             if (erroneousCI.size() > 0) {
-                StringBuilder sb = new StringBuilder("contains the following illegal cards:\n");
+                StringBuilder sb = new StringBuilder("contains one or more cards that do not match the commanders color identity:");
 
                 for (PaperCard cp : erroneousCI) {
                     sb.append("\n").append(cp.getName());
@@ -220,29 +206,43 @@ public enum DeckFormat {
             }
         }
 
-        int maxCopies = getMaxCardCopies();
+        if (cardPoolFilter != null) {
+            final List<PaperCard> erroneousCI = new ArrayList<PaperCard>();
+            for (final Entry<PaperCard, Integer> cp : deck.getAllCardsInASinglePool()) {
+                if (!cardPoolFilter.apply(cp.getKey().getRules())) {
+                    erroneousCI.add(cp.getKey());
+                }
+            }
+            if (erroneousCI.size() > 0) {
+                final StringBuilder sb = new StringBuilder("contains the following illegal cards:\n");
+
+                for (final PaperCard cp : erroneousCI) {
+                    sb.append("\n").append(cp.getName());
+                }
+
+                return sb.toString();
+            }
+        }
+
+        final int maxCopies = getMaxCardCopies();
         if (maxCopies < Integer.MAX_VALUE) {
             //Must contain no more than 4 of the same card
             //shared among the main deck and sideboard, except
             //basic lands, Shadowborn Apostle and Relentless Rats
 
-            CardPool tmp = new CardPool(deck.getMain());
+            final CardPool allCards = deck.getAllCardsInASinglePool(hasCommander());
-            if (deck.has(DeckSection.Sideboard)) {
+            final ImmutableSet<String> limitExceptions = ImmutableSet.of("Relentless Rats", "Shadowborn Apostle");
-                tmp.addAll(deck.get(DeckSection.Sideboard));
-            }
-            if (deck.has(DeckSection.Commander) && this == Commander) {
-                tmp.addAll(deck.get(DeckSection.Commander));
-            }
-
-            List<String> limitExceptions = Arrays.asList(new String[]{"Relentless Rats", "Shadowborn Apostle"});
 
             // should group all cards by name, so that different editions of same card are really counted as the same card
-            for (Entry<String, Integer> cp : Aggregates.groupSumBy(tmp, PaperCard.FN_GET_NAME)) {
+            for (final Entry<String, Integer> cp : Aggregates.groupSumBy(allCards, PaperCard.FN_GET_NAME)) {
-                IPaperCard simpleCard = StaticData.instance().getCommonCards().getCard(cp.getKey());
+                final IPaperCard simpleCard = StaticData.instance().getCommonCards().getCard(cp.getKey());
-                boolean canHaveMultiple = simpleCard.getRules().getType().isBasicLand() || limitExceptions.contains(cp.getKey());
+                if (simpleCard == null) {
+                    return String.format("contains the nonexisting card %s", cp.getKey());
+                }
 
+                final boolean canHaveMultiple = simpleCard.getRules().getType().isBasicLand() || limitExceptions.contains(cp.getKey());
                 if (!canHaveMultiple && cp.getValue() > maxCopies) {
-                    return String.format("must not contain more than %d of '%s' card", maxCopies, cp.getKey());
+                    return String.format("must not contain more than %d copies of the card %s", maxCopies, cp.getKey());
                 }
             }
         }

forge-gui-desktop/src/main/java/forge/deckchooser/FDeckChooser.java

@@ -276,8 +276,7 @@ public class FDeckChooser extends JPanel implements IDecksComboBoxListener {
             lstDecksContainer = new ItemManagerContainer(lstDecks);
             decksComboBox.addListener(this);
             restoreSavedState();
-        }
+        } else {
-        else {
             removeAll();
         }
         this.setLayout(new MigLayout("insets 0, gap 0"));
@@ -330,6 +329,7 @@ public class FDeckChooser extends JPanel implements IDecksComboBoxListener {
     }
 
     private void refreshDecksList(DeckType deckType, boolean forceRefresh, DecksComboBoxEvent ev) {
+        if (decksComboBox == null) { return; } // Not yet populated
         if (selectedDeckType == deckType && !forceRefresh) { return; }
         selectedDeckType = deckType;
 
@@ -406,7 +406,7 @@ public class FDeckChooser extends JPanel implements IDecksComboBoxListener {
         }
     }
 
-    private void restoreSavedState() {
+    public void restoreSavedState() {
         DeckType oldDeckType = selectedDeckType;
         if (stateSetting == null) {
             //if can't restore saved state, just refresh deck list

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

@@ -17,15 +17,16 @@
  */
 package forge.screens.deckeditor.controllers;
 
+import org.apache.commons.lang3.StringUtils;
+
 import com.google.common.base.Supplier;
 
 import forge.deck.DeckBase;
 import forge.screens.deckeditor.menus.DeckFileMenu;
 import forge.screens.deckeditor.views.VCurrentDeck;
+import forge.screens.home.sanctioned.VSubmenuConstructed;
 import forge.util.storage.IStorage;
 
-import org.apache.commons.lang3.StringUtils;
-
 /**
  * TODO: Write javadoc for this type.
  *
@@ -199,6 +200,8 @@ public class DeckController<T extends DeckBase> {
         this.currentFolder.add((T) this.model.copyTo(this.model.getName()));
         this.modelInStorage = true;
         this.setSaved(true);
+
+        VSubmenuConstructed.SINGLETON_INSTANCE.getLobby().updateDeckPanel();
     }
 
     /**

forge-gui-desktop/src/main/java/forge/screens/home/CLobby.java

@@ -8,6 +8,9 @@ import javax.swing.SwingUtilities;
 
 import forge.deck.Deck;
 import forge.deck.DeckType;
+import forge.deckchooser.DecksComboBoxEvent;
+import forge.deckchooser.FDeckChooser;
+import forge.deckchooser.IDecksComboBoxListener;
 import forge.model.CardCollections;
 import forge.model.FModel;
 import forge.properties.ForgePreferences;
@@ -102,14 +105,16 @@ public class CLobby {
     }
 
     public void initialize() {
-        view.getDeckChooser(0).initialize(FPref.CONSTRUCTED_P1_DECK_STATE, DeckType.PRECONSTRUCTED_DECK);
+        for (int iSlot = 0; iSlot < VLobby.MAX_PLAYERS; iSlot++) {
-        view.getDeckChooser(1).initialize(FPref.CONSTRUCTED_P2_DECK_STATE, DeckType.COLOR_DECK);
+            final FDeckChooser fdc = view.getDeckChooser(iSlot);
-        view.getDeckChooser(2).initialize(FPref.CONSTRUCTED_P3_DECK_STATE, DeckType.COLOR_DECK);
+            fdc.initialize(FPref.CONSTRUCTED_DECK_STATES[iSlot], defaultDeckTypeForSlot(iSlot));
-        view.getDeckChooser(3).initialize(FPref.CONSTRUCTED_P4_DECK_STATE, DeckType.COLOR_DECK);
+            fdc.populate();
-        view.getDeckChooser(4).initialize(FPref.CONSTRUCTED_P5_DECK_STATE, DeckType.COLOR_DECK);
+            fdc.getDecksComboBox().addListener(new IDecksComboBoxListener() {
-        view.getDeckChooser(5).initialize(FPref.CONSTRUCTED_P6_DECK_STATE, DeckType.COLOR_DECK);
+                @Override public final void deckTypeSelected(final DecksComboBoxEvent ev) {
-        view.getDeckChooser(6).initialize(FPref.CONSTRUCTED_P7_DECK_STATE, DeckType.COLOR_DECK);
+                    view.focusOnAvatar();
-        view.getDeckChooser(7).initialize(FPref.CONSTRUCTED_P8_DECK_STATE, DeckType.COLOR_DECK);
+                }
+            });
+        }
 
         final ForgePreferences prefs = FModel.getPreferences();
         // Checkbox event handling
@@ -134,4 +139,7 @@ public class CLobby {
         view.getCbArtifacts().setSelected(prefs.getPrefBoolean(FPref.DECKGEN_ARTIFACTS));
     }
 
+    private static DeckType defaultDeckTypeForSlot(final int iSlot) {
+        return iSlot == 0 ? DeckType.PRECONSTRUCTED_DECK : DeckType.COLOR_DECK;
+    }
 }

forge-gui-desktop/src/main/java/forge/screens/home/PlayerPanel.java

@@ -346,12 +346,16 @@ public class PlayerPanel extends FPanel {
     @Override
     public void paintComponent(Graphics g) {
         super.paintComponent(g);
-        if (lobby.getPlayerPanelWithFocus() != this) {
+        if (!hasFocusInLobby()) {
             FSkin.setGraphicsColor(g, unfocusedPlayerOverlay);
             g.fillRect(0, 0, this.getWidth(), this.getHeight());
         }
     }
 
+    private boolean hasFocusInLobby() {
+        return lobby.hasFocus(index);
+    }
+
     int getIndex() {
         return index;
     }

forge-gui-desktop/src/main/java/forge/screens/home/VLobby.java

@@ -33,9 +33,7 @@ import forge.deck.DeckProxy;
 import forge.deck.DeckSection;
 import forge.deck.DeckType;
 import forge.deck.DeckgenUtil;
-import forge.deckchooser.DecksComboBoxEvent;
 import forge.deckchooser.FDeckChooser;
-import forge.deckchooser.IDecksComboBoxListener;
 import forge.game.GameType;
 import forge.game.card.CardView;
 import forge.gui.CardDetailPanel;
@@ -80,7 +78,6 @@ public class VLobby implements IUpdateable {
     private final LblHeader lblTitle = new LblHeader("Sanctioned Format: Constructed");
     private int activePlayersNum = 0;
     private int playerWithFocus = 0; // index of the player that currently has focus
-    private PlayerPanel playerPanelWithFocus;
 
     private final StartButton btnStart  = new StartButton();
     private final JPanel pnlStart = new JPanel(new MigLayout("insets 0, gap 0, wrap 2"));
@@ -113,13 +110,12 @@ public class VLobby implements IUpdateable {
     private final Deck[] decks = new Deck[MAX_PLAYERS];
 
     // Variants
-    private final List<FList<Object>> schemeDeckLists = new ArrayList<FList<Object>>();
-    private final List<FPanel> schemeDeckPanels = new ArrayList<FPanel>(MAX_PLAYERS);
-    private int lastArchenemy = 0;
-
     private final List<FList<Object>> commanderDeckLists = new ArrayList<FList<Object>>();
     private final List<FPanel> commanderDeckPanels = new ArrayList<FPanel>(MAX_PLAYERS);
 
+    private final List<FList<Object>> schemeDeckLists = new ArrayList<FList<Object>>();
+    private final List<FPanel> schemeDeckPanels = new ArrayList<FPanel>(MAX_PLAYERS);
+
     private final List<FList<Object>> planarDeckLists = new ArrayList<FList<Object>>();
     private final List<FPanel> planarDeckPanels = new ArrayList<FPanel>(MAX_PLAYERS);
 
@@ -193,17 +189,15 @@ public class VLobby implements IUpdateable {
         }
     }
 
-    public void populate() {
+    public void updateDeckPanel() {
-        for (final FDeckChooser fdc : deckChoosers) {
+        for (int iPlayer = 0; iPlayer < activePlayersNum; iPlayer++) {
-            fdc.populate();
+            final FDeckChooser fdc = getDeckChooser(iPlayer);
-            fdc.getDecksComboBox().addListener(new IDecksComboBoxListener() {
+            fdc.restoreSavedState();
-                @Override
-                public void deckTypeSelected(DecksComboBoxEvent ev) {
-                    playerPanelWithFocus.focusOnAvatar();
-                }
-            });
         }
-        populateDeckPanel(GameType.Constructed);
+    }
+
+    public void focusOnAvatar() {
+        getPlayerPanelWithFocus().focusOnAvatar();
     }
 
     public void update(final boolean fullUpdate) {
@@ -221,6 +215,7 @@ public class VLobby implements IUpdateable {
             if (i < activePlayersNum) {
                 // visible panels
                 final LobbySlot slot = lobby.getSlot(i);
+                final FDeckChooser deckChooser = getDeckChooser(i);
                 final PlayerPanel panel;
                 final boolean isNewPanel;
                 if (hasPanel) {
@@ -234,6 +229,7 @@ public class VLobby implements IUpdateable {
                         constraints += ", gaptop 5px";
                     }
                     playersScroll.add(panel, constraints);
+                    deckChooser.restoreSavedState();
                     if (i == 0) {
                         changePlayerFocus(0);
                     }
@@ -253,7 +249,6 @@ public class VLobby implements IUpdateable {
                 panel.setMayRemove(lobby.mayRemove(i));
                 panel.update();
 
-                final FDeckChooser deckChooser = getDeckChooser(i);
                 deckChooser.setIsAi(slot.getType() == LobbySlotType.AI);
                 if (fullUpdate && (type == LobbySlotType.LOCAL || type == LobbySlotType.AI)) {
                     selectDeck(i);
@@ -325,8 +320,8 @@ public class VLobby implements IUpdateable {
      * These are added to a list which can be referenced to populate the deck panel appropriately. */
     @SuppressWarnings("serial")
     private void buildDeckPanel(final int playerIndex) {
-        String sectionConstraints = "insets 0, gap 0, wrap";
+        final String sectionConstraints = "insets 0, gap 0, wrap";
-        String labelConstraints = "gaptop 10px, gapbottom 5px";
+        final String labelConstraints = "gaptop 10px, gapbottom 5px";
 
         // Main deck
         final FDeckChooser mainChooser = new FDeckChooser(null, false);
@@ -637,14 +632,14 @@ public class VLobby implements IUpdateable {
     FCheckBox getVntTinyLeaders()     { return vntTinyLeaders; }
     FCheckBox getVntVanguard()        { return vntVanguard; }
 
-    public int getLastArchenemy() { return lastArchenemy; }
-    public void setLastArchenemy(final int archenemy) { lastArchenemy = archenemy; }
-
     public final List<PlayerPanel> getPlayerPanels() {
         return playerPanels;
     }
-    public final PlayerPanel getPlayerPanelWithFocus() {
+    private PlayerPanel getPlayerPanelWithFocus() {
-        return playerPanelWithFocus;
+        return getPlayerPanels().get(playerWithFocus);
+    }
+    boolean hasFocus(final int iPlayer) {
+        return iPlayer == playerWithFocus;
     }
 
     public final FDeckChooser getDeckChooser(int playernum) {
@@ -684,14 +679,15 @@ public class VLobby implements IUpdateable {
     }
 
     void changePlayerFocus(int newFocusOwner, GameType gType) {
-        if (playerPanelWithFocus != null) {
+        final PlayerPanel oldFocus = getPlayerPanelWithFocus();
-            playerPanelWithFocus.setFocused(false);
+        if (oldFocus != null) {
+            oldFocus.setFocused(false);
         }
         playerWithFocus = newFocusOwner;
-        playerPanelWithFocus = playerPanels.get(playerWithFocus);
+        final PlayerPanel newFocus = getPlayerPanelWithFocus();
-        playerPanelWithFocus.setFocused(true);
+        newFocus.setFocused(true);
 
-        playersScroll.getViewport().scrollRectToVisible(playerPanelWithFocus.getBounds());
+        playersScroll.getViewport().scrollRectToVisible(newFocus.getBounds());
         populateDeckPanel(gType);
 
         refreshPanels(true, true);

forge-gui-desktop/src/main/java/forge/screens/home/online/CSubmenuOnlineLobby.java

@@ -84,7 +84,6 @@ public enum CSubmenuOnlineLobby implements ICDoc, IMenuProvider {
             }
         });
 
-        view.populate();
         view.update(true);
 
         Singletons.getControl().setCurrentScreen(FScreen.ONLINE_LOBBY);

forge-gui-desktop/src/main/java/forge/screens/home/online/VOnlineLobby.java

@@ -61,7 +61,7 @@ public enum VOnlineLobby implements IVDoc<COnlineLobby>, IVTopLevelUI {
             fdc.populate();
             fdc.getDecksComboBox().addListener(new IDecksComboBoxListener() {
                 @Override public final void deckTypeSelected(final DecksComboBoxEvent ev) {
-                    lobby.getPlayerPanelWithFocus().focusOnAvatar();
+                    lobby.focusOnAvatar();
                 }
             });
         }

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

@@ -131,7 +131,7 @@ public enum VSubmenuConstructed implements IVSubmenu<CSubmenuConstructed> {
             fdc.populate();
             fdc.getDecksComboBox().addListener(new IDecksComboBoxListener() {
                 @Override public final void deckTypeSelected(final DecksComboBoxEvent ev) {
-                    vLobby.getPlayerPanelWithFocus().focusOnAvatar();
+                    vLobby.focusOnAvatar();
                 }
             });
         }

forge-gui/src/main/java/forge/properties/ForgePreferences.java

@@ -167,6 +167,12 @@ public class ForgePreferences extends PreferencesStore<ForgePreferences.FPref> {
         public String getDefault() {
             return strDefaultVal;
         }
+
+        public static FPref[] CONSTRUCTED_DECK_STATES = {
+            CONSTRUCTED_P1_DECK_STATE, CONSTRUCTED_P2_DECK_STATE,
+            CONSTRUCTED_P3_DECK_STATE, CONSTRUCTED_P4_DECK_STATE,
+            CONSTRUCTED_P5_DECK_STATE, CONSTRUCTED_P6_DECK_STATE,
+            CONSTRUCTED_P7_DECK_STATE, CONSTRUCTED_P8_DECK_STATE };
     }
 
     public static enum CardSizeType {