refactored code that calculates if the cardpanels fit playarea with a given cardWidth: the "rows" member is never modified, so there won't be any "concurrent modification" exceptions; lists or lands, tokens, creatures and others are not modified during calculation, so they don't need to be cloned from reference for each trial of card width.

src/main/java/forge/view/arcane/PlayArea.java

@@ -23,11 +23,9 @@ import java.awt.Rectangle;
 import java.awt.event.MouseEvent;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Iterator;
 import java.util.List;
 
 import javax.swing.JScrollPane;
-
 import forge.Card;
 import forge.view.arcane.util.CardPanelMouseListener;
 
@@ -76,6 +74,7 @@ public class PlayArea extends CardPanelContainer implements CardPanelMouseListen
      *            a {@link javax.swing.JScrollPane} object.
      * @param mirror
      *            a boolean.
+     * @param modelRef 
      */
     public PlayArea(final JScrollPane scrollPane, final boolean mirror) {
         super(scrollPane);
@@ -196,46 +195,40 @@ public class PlayArea extends CardPanelContainer implements CardPanelMouseListen
         this.playAreaWidth = rect.width;
         this.playAreaHeight = rect.height;
 
-        final CardStackRow allLands = collectAllLands();
+        final CardStackRow lands = collectAllLands();
-        final CardStackRow allTokens = collectAllTokens();
+        final CardStackRow tokens = collectAllTokens();
-        final CardStackRow allCreatures = new CardStackRow(this.getCardPanels(), RowType.creatureNonToken);
+        final CardStackRow creatures = new CardStackRow(this.getCardPanels(), RowType.CreatureNonToken);
-        final CardStackRow allOthers = new CardStackRow(this.getCardPanels(), RowType.other);
+        final CardStackRow others = new CardStackRow(this.getCardPanels(), RowType.Other);
 
         // should find an appropriate width of card
-        this.cardWidth = this.getCardWidthMax();
         int maxCardWidth = this.getCardWidthMax();
+        setCardWidth(maxCardWidth);
         int minCardWidth = this.getCardWidthMin();
         int lastGoodCardWidth = minCardWidth;
         int deltaCardWidth = (maxCardWidth - minCardWidth) / 2;
-        boolean workedLastTime = false;
+        List<CardStackRow> lastTemplate = null;
-        //boolean isFirstRun = true;
 
         while (deltaCardWidth > 0) {
-            final CardStackRow creatures = (CardStackRow) allCreatures.clone();
+            List<CardStackRow> template = tryArrangePilesOfWidth(lands, tokens, creatures, others);
-            final CardStackRow tokens = (CardStackRow) allTokens.clone();
-            final CardStackRow lands = (CardStackRow) allLands.clone();
-            CardStackRow others = (CardStackRow) allOthers.clone();
-            workedLastTime = canAdjustWidth(lands, tokens, creatures, others);
             
-            deltaCardWidth = (cardWidth - lastGoodCardWidth) / 2;
+            deltaCardWidth = (getCardWidth() - lastGoodCardWidth) / 2;
-            if (workedLastTime) {
+            if (template != null) {
-                lastGoodCardWidth = cardWidth;
+                lastTemplate = template;
-                cardWidth += deltaCardWidth;
+                lastGoodCardWidth = getCardWidth();
+                setCardWidth(getCardWidth() + deltaCardWidth);
                 if (lastGoodCardWidth == maxCardWidth) {
                     break;
                 }
             }
             else {
-                cardWidth -= deltaCardWidth;
+                setCardWidth(getCardWidth() - deltaCardWidth);
             }
         }
-        cardWidth = lastGoodCardWidth;
+        setCardWidth(lastGoodCardWidth);
-        final CardStackRow creatures = (CardStackRow) allCreatures.clone();
+        if ( null == lastTemplate ) 
-        final CardStackRow tokens = (CardStackRow) allTokens.clone();
+            lastTemplate = tryArrangePilesOfWidth(lands, tokens, creatures, others);
-        final CardStackRow lands = (CardStackRow) allLands.clone();
-        CardStackRow others = (CardStackRow) allOthers.clone();
-        workedLastTime = canAdjustWidth(lands, tokens, creatures, others);
 
+        this.rows = lastTemplate;
         // Get size of all the rows.
         int x, y = PlayArea.GUTTER_Y;
         int maxRowWidth = 0;
@@ -252,25 +245,26 @@ public class PlayArea extends CardPanelContainer implements CardPanelMouseListen
         }
         this.setPreferredSize(new Dimension(maxRowWidth - this.cardSpacingX, y - this.cardSpacingY));
         this.revalidate();
-        positionAllCards();
+        positionAllCards(lastTemplate);
     }
 
-    private void positionAllCards()  {
+    private void positionAllCards(List<CardStackRow> template)  {
 
         // Position all card panels.
         int x = 0;
         int y = PlayArea.GUTTER_Y;
 
-        for (final CardStackRow row : this.rows) {
+        for (final CardStackRow row : template) {
             int rowBottom = 0;
             x = PlayArea.GUTTER_X;
             for (int stackIndex = 0, stackCount = row.size(); stackIndex < stackCount; stackIndex++) {
                 final CardStack stack = row.get(stackIndex);
                 // Align others to the right.
-                if (RowType.other.isType(stack.get(0).getGameCard())) {
+                if (RowType.Other.isGoodFor(stack.get(0).getGameCard())) {
                     x = (this.playAreaWidth - PlayArea.GUTTER_X) + this.extraCardSpacingX;
                     for (int i = stackIndex, n = row.size(); i < n; i++) {
-                        x -= row.get(i).getWidth();
+                        CardStack r = row.get(i);
+                        x -= r.getWidth();
                     }
                 }
                 for (int panelIndex = 0, panelCount = stack.size(); panelIndex < panelCount; panelIndex++) {
@@ -279,68 +273,62 @@ public class PlayArea extends CardPanelContainer implements CardPanelMouseListen
                     this.setComponentZOrder(panel, panelIndex);
                     final int panelX = x + (stackPosition * this.stackSpacingX);
                     final int panelY = y + (stackPosition * this.stackSpacingY);
-                    panel.setCardBounds(panelX, panelY, this.cardWidth, this.cardHeight);
+                    panel.setCardBounds(panelX, panelY, this.getCardWidth(), this.cardHeight);
                 }
                 rowBottom = Math.max(rowBottom, y + stack.getHeight());
-                x += stack.getWidth();
+                x += stack.getWidth() + cardSpacingX;
             }
             y = rowBottom;
         }
     }
 
-    private boolean canAdjustWidth(final CardStackRow lands,  final CardStackRow tokens, final CardStackRow creatures, CardStackRow others) {
+    private List<CardStackRow> tryArrangePilesOfWidth(final CardStackRow lands, final CardStackRow tokens, final CardStackRow creatures, CardStackRow others) {
-        this.rows.clear();
+        List<CardStackRow> template = new ArrayList<PlayArea.CardStackRow>();
-        this.cardHeight = Math.round(this.cardWidth * CardPanel.ASPECT_RATIO);
-        this.extraCardSpacingX = Math.round(this.cardWidth * PlayArea.EXTRA_CARD_SPACING_X);
-        this.cardSpacingX = (this.cardHeight - this.cardWidth) + this.extraCardSpacingX;
-        this.cardSpacingY = Math.round(this.cardHeight * PlayArea.CARD_SPACING_Y);
-        this.stackSpacingX = Math.round(this.cardWidth * PlayArea.STACK_SPACING_X);
-        this.stackSpacingY = Math.round(this.cardHeight * PlayArea.STACK_SPACING_Y);
         
         int afterFirstRow;
 
+        boolean landsFit, tokensFit, creaturesFit;
         if (this.mirror) {
             // Wrap all creatures and lands.
-            this.wrap(lands, this.rows, -1);
+            landsFit = this.planRow(lands, template, -1);
-            afterFirstRow = this.rows.size();
+            afterFirstRow = template.size();
-            this.wrap(tokens, this.rows, afterFirstRow);
+            tokensFit = this.planRow(tokens, template, afterFirstRow);
-            this.wrap(creatures, this.rows, this.rows.size());
+            creaturesFit = this.planRow(creatures, template, template.size());
         } else {
             // Wrap all creatures and lands.
-            this.wrap(creatures, this.rows, -1);
+            creaturesFit = this.planRow(creatures, template, -1);
-            afterFirstRow = this.rows.size();
+            afterFirstRow = template.size();
-            this.wrap(tokens, this.rows, afterFirstRow);
+            tokensFit = this.planRow(tokens, template, afterFirstRow);
-            this.wrap(lands, this.rows, this.rows.size());
+            landsFit = this.planRow(lands, template, template.size());
         }
-        // Store the current rows and others.
+
-        final List<CardStackRow> storedRows = new ArrayList<CardStackRow>(this.rows.size());
+        if ( !landsFit || !creaturesFit || !tokensFit ) 
-        for (final CardStackRow row : this.rows) {
+            return null;
-            try {
+        
-                storedRows.add((CardStackRow) row.clone());
+        // Other cards may be stored at end of usual rows or on their own row.
+        int cntOthers = others.size();
+
+        // Copy the template for the case 1st approach won't work
+        final List<CardStackRow> templateCopy = new ArrayList<CardStackRow>(template.size());
+        for (final CardStackRow row : template) {
+            templateCopy.add((CardStackRow) row.clone());
         }
-            catch (NullPointerException e) {
+
-                System.out.println("Null pointer exception in Row Spacing. Possibly also part of the issue.");
-            }
-        }
-        final CardStackRow storedOthers = (CardStackRow) others.clone();
         // Fill in all rows with others.
-        for (final CardStackRow row : this.rows) {
+        int nextOther = 0;
-            this.fillRow(others, this.rows, row);
+        for (final CardStackRow row : template) {
+            nextOther = this.planOthersRow(others, nextOther, template, row);
+            if ( nextOther == cntOthers )
+                return template; // everything was successfully placed
         }
-        // Stop if everything fits, otherwise revert back to the stored
+
-        // values.
+        template = templateCopy;
-        if (creatures.isEmpty() && tokens.isEmpty() && lands.isEmpty() && others.isEmpty()) {
+        // Try to put others on their own row(s)
-            return true;
+        if ( this.planRow(others, template, afterFirstRow) ) 
-        }
+            return template;
-        this.rows = storedRows;
+        
-        others = storedOthers;
+        
-        // Try to put others on their own row(s) and fill in the rest.
+        return null; // Cannot fit everything with that width;
-        this.wrap(others, this.rows, afterFirstRow);
-        for (final CardStackRow row : this.rows) {
-            this.fillRow(others, this.rows, row);
-        }
-        // If that still doesn't fit, scale down.
-        return creatures.isEmpty() && tokens.isEmpty() && lands.isEmpty() && others.isEmpty();
     }
 
     /**
@@ -350,69 +338,53 @@ public class PlayArea extends CardPanelContainer implements CardPanelMouseListen
      * 
      * @param sourceRow
      *            a {@link forge.view.arcane.PlayArea.CardStackRow} object.
-     * @param rows
+     * @param template
      *            a {@link java.util.List} object.
      * @param insertIndex
      *            a int.
      * @return a int.
      */
-//    private int cntRepaints = 0;
+    // Won't modify the first parameter
-    private int wrap(final CardStackRow sourceRow, final List<CardStackRow> rows, final int insertIndex) {
+    private boolean planRow(final CardStackRow sourceRow, final List<CardStackRow> template, final int insertIndex) {
         // The cards are sure to fit (with vertical scrolling) at the minimum
         // card width.
-        final boolean allowHeightOverflow = this.cardWidth == this.getCardWidthMin();
+        final boolean isMinimalSize = this.getCardWidth() == this.getCardWidthMin();
 
 //        System.err.format("[%d] @ %d - Repaint playarea - %s %n", new Date().getTime(), cntRepaints++, mirror ? "MIRROR" : "DIRECT");
 
         CardStackRow currentRow = new CardStackRow();
-        for (int i = 0, n = sourceRow.size() - 1; i <= n; i++) {
+        for (final CardStack stack : sourceRow) {
-            final CardStack stack = sourceRow.get(i);
-            // If the row is not empty and this stack doesn't fit, add the row.
             final int rowWidth = currentRow.getWidth();
-            if (!currentRow.isEmpty() && ((rowWidth + stack.getWidth()) > this.playAreaWidth)) {
+            // If the row is not empty and this stack doesn't fit, add the row.
+            if (rowWidth + stack.getWidth() > this.playAreaWidth && !currentRow.isEmpty() ) {
+
                 // Stop processing if the row is too wide or tall.
-                if (!allowHeightOverflow && (rowWidth > this.playAreaWidth)) {
+                if (rowWidth > this.playAreaWidth || this.getRowsHeight(template) + sourceRow.getHeight() > this.playAreaHeight) {
-                    break;
+                    if ( !isMinimalSize ) 
-                }
+                        return false;
-                if (!allowHeightOverflow && ((this.getRowsHeight(rows) + sourceRow.getHeight()) > this.playAreaHeight)) {
-                    break;
-                }
-                try {
-                    rows.add(insertIndex == -1 ? rows.size() : insertIndex, currentRow);
-                }
-                catch (ArrayIndexOutOfBoundsException e) {
-                    System.out.println("ArrayIndex Out of Bounds when trying to add row in PlayArea. Someone fix this logic, "
-                            + " I believe it causes the no cards loading in issue we've noticed.");
-                    // TODO: There's a crash here, maybe when rows == [null] and currentRow == [[Plant Wall]] and insertIndex is 0
                 }
+
+                if ( insertIndex == -1)
+                    template.add(currentRow);
+                else 
+                    template.add(insertIndex, currentRow);
+                
                 currentRow = new CardStackRow();
             }
+
             currentRow.add(stack);
         }
         // Add the last row if it is not empty and it fits.
         if (!currentRow.isEmpty()) {
             final int rowWidth = currentRow.getWidth();
-            if (allowHeightOverflow
+            if (isMinimalSize || rowWidth <= this.playAreaWidth && this.getRowsHeight(template) + sourceRow.getHeight() <= this.playAreaHeight) {
-                    || (rowWidth <= this.playAreaWidth)
+                if ( insertIndex == -1)
-                    && (allowHeightOverflow || ((this.getRowsHeight(rows) + sourceRow.getHeight()) <= this.playAreaHeight))) {
+                    template.add(currentRow);
-                try {
+                else 
-                    rows.add(insertIndex == -1 ? rows.size() : insertIndex, currentRow);
+                    template.add(insertIndex, currentRow);
+            } else return false;
         }
-                catch (ArrayIndexOutOfBoundsException e) {
+        return true;
-                    System.out.println("ArrayIndex Out of Bounds when trying to add row in PlayArea. Someone fix this logic, "
-                            + " I believe it causes the no cards loading in issue we've noticed.");
-                    // TODO: There's a crash here, maybe when rows == [null] and currentRow == [[Plant Wall]] and insertIndex is 0
-                }
-            }
-        }
-        // Remove the wrapped stacks from the source row.
-        for (int iRow = 0; iRow < rows.size(); iRow++) {
-            CardStackRow row = rows.get(iRow);
-            if (row != null) {
-                sourceRow.removeAll(row);
-            }
-        }
-        return insertIndex;
     }
 
 
@@ -423,32 +395,30 @@ public class PlayArea extends CardPanelContainer implements CardPanelMouseListen
      * 
      * @param sourceRow
      *            a {@link forge.view.arcane.PlayArea.CardStackRow} object.
-     * @param rows
+     * @param template
      *            a {@link java.util.List} object.
-     * @param rows
+     * @param template
      *            a {@link java.util.List} object.
-     * @param row
+     * @param rowToFill
      *            a {@link forge.view.arcane.PlayArea.CardStackRow} object.
      */
-    private void fillRow(final CardStackRow sourceRow, final List<CardStackRow> rows, final CardStackRow row) {
+    private int planOthersRow(final List<CardStack> sourceRow, final int firstPile, final List<CardStackRow> template, final CardStackRow rowToFill) {
-        int rowWidth = row.getWidth();
+        int rowWidth = rowToFill.getWidth();
 
-        final Iterator<CardStack> itr = sourceRow.iterator();
+        for (int i = firstPile; i < sourceRow.size(); i++ ) {
-
+            CardStack stack = sourceRow.get(i);
-        while (itr.hasNext()) {
-            final CardStack stack = itr.next();
 
             rowWidth += stack.getWidth();
-            if (rowWidth > this.playAreaWidth) {
+            if (rowWidth > this.playAreaWidth) return i; // cannot add any more piles in a row 
-                break;
+            
+            if (stack.getHeight() > rowToFill.getHeight()) { // if row becomes taller
+                int newAllRowsHeight = this.getRowsHeight(template) - rowToFill.getHeight() + stack.getHeight();
+                if ( newAllRowsHeight > this.playAreaHeight) 
+                    return i; // refuse to add here because it won't fit in height
             }
-            if (stack.getHeight() > row.getHeight()
+            rowToFill.add(stack);
-                    && (((this.getRowsHeight(rows) - row.getHeight()) + stack.getHeight()) > this.playAreaHeight)) {
-                break;
-            }
-            row.add(stack);
-            itr.remove();
         }
+        return sourceRow.size();
     }
 
     /**
@@ -509,20 +479,18 @@ public class PlayArea extends CardPanelContainer implements CardPanelMouseListen
     }
 
     private static enum RowType {
-        land, creature, creatureNonToken, other;
+        Land,
+        Creature,
+        CreatureNonToken,
+        Other;
 
-        public boolean isType(final Card card) {
+        public boolean isGoodFor(final Card card) {
             switch (this) {
-            case land:
+            case Land:              return card.isLand();
-                return card.isLand();
+            case Creature:          return card.isCreature();
-            case creature:
+            case CreatureNonToken:  return card.isCreature() && !card.isToken();
-                return card.isCreature();
+            case Other:             return !card.isLand() && !card.isCreature();
-            case creatureNonToken:
+            default:                throw new RuntimeException("Unhandled type: " + this);
-                return card.isCreature() && !card.isToken();
-            case other:
-                return !card.isLand() && !card.isCreature();
-            default:
-                throw new RuntimeException("Unhandled type: " + this);
             }
         }
     }
@@ -541,7 +509,7 @@ public class PlayArea extends CardPanelContainer implements CardPanelMouseListen
 
         private void addAll(final List<CardPanel> cardPanels, final RowType type) {
             for (final CardPanel panel : cardPanels) {
-                if (!type.isType(panel.getGameCard()) || (panel.getAttachedToPanel() != null)) {
+                if (!type.isGoodFor(panel.getGameCard()) || (panel.getAttachedToPanel() != null)) {
                     continue;
                 }
                 final CardStack stack = new CardStack();
@@ -606,4 +574,17 @@ public class PlayArea extends CardPanelContainer implements CardPanelMouseListen
                     + PlayArea.this.cardSpacingY;
         }
     }
+    private int getCardWidth() {
+        return cardWidth;
+    }
+
+    private void setCardWidth(int cardWidth0) {
+        this.cardWidth = cardWidth0;
+        this.cardHeight = Math.round(this.cardWidth * CardPanel.ASPECT_RATIO);
+        this.extraCardSpacingX = Math.round(this.cardWidth * PlayArea.EXTRA_CARD_SPACING_X);
+        this.cardSpacingX = (this.cardHeight - this.cardWidth) + this.extraCardSpacingX;
+        this.cardSpacingY = Math.round(this.cardHeight * PlayArea.CARD_SPACING_Y);
+        this.stackSpacingX = Math.round(this.cardWidth * PlayArea.STACK_SPACING_X);
+        this.stackSpacingY = Math.round(this.cardHeight * PlayArea.STACK_SPACING_Y);
+    }    
 }