From b426c6570d38d94e3d01cb77a8929631a1d9bf24 Mon Sep 17 00:00:00 2001
From: Brent <90901032@westernsydney.edu.au>
Date: Sat, 27 Jul 2024 08:46:41 +1000
Subject: [PATCH 1/2] 13505 - Prevent GameRefresher moving Decks with duplicate
 names

---
 .../VASSAL/build/module/GameRefresher.java    | 127 +++++++++++-------
 1 file changed, 77 insertions(+), 50 deletions(-)

diff --git a/vassal-app/src/main/java/VASSAL/build/module/GameRefresher.java b/vassal-app/src/main/java/VASSAL/build/module/GameRefresher.java
index 7f35453716..5c89e3f796 100644
--- a/vassal-app/src/main/java/VASSAL/build/module/GameRefresher.java
+++ b/vassal-app/src/main/java/VASSAL/build/module/GameRefresher.java
@@ -417,59 +417,43 @@ public void execute(Set<String> options, Command command) throws IllegalBuildExc
           for (final GamePiece pieceOrStack : map.getPieces()) {
             if (pieceOrStack instanceof Deck) {
               final Deck deck = (Deck) pieceOrStack;
-              // Match with a DrawPile if possible
-              boolean deckFound = false;
-              for (final DrawPile drawPile : drawPiles) {
-                final String deckName = deck.getDeckName();
-                if (deckName.equals(drawPile.getAttributeValueString(SetupStack.NAME))) {
-
-                  // If drawPile is owned by a specific board, then we can only match it if that board is active in this game
-                  if (drawPile.getOwningBoardName() != null) {
-                    if (map.getBoardByName(drawPile.getOwningBoardName()) == null) {
-                      continue;
-                    }
-
-                    // If the drawPile is on a map that doesn't have its current owning board active, then we
-                    // cannot match that drawPile.
-                    if (drawPile.getMap().getBoardByName(drawPile.getOwningBoardName()) == null) {
-                      continue;
-                    }
-                  }
-
-                  deckFound = true;
-                  foundDrawPiles.add(drawPile);
-
-                  final String drawPileName = drawPile.getAttributeValueString(SetupStack.NAME);
-
-                  if (!options.contains(SUPPRESS_INFO_REPORTS))
-                    log(Resources.getString("GameRefresher.refreshing_deck", deckName, drawPileName));
-
-                  // This refreshes the existing deck with all the up-to-date drawPile fields from the module
-                  deck.removeListeners();
-                  deck.myRefreshType(drawPile.getDeckType());
-                  deck.addListeners();
-
-                  // Make sure the deck is in the right place
-                  final Point pt = drawPile.getPosition();
-                  final Map newMap = drawPile.getMap();
-                  if (newMap != map) {
-                    map.removePiece(deck);
-                    newMap.addPiece(deck);
-                  }
-                  deck.setPosition(pt);
-                  for (final GamePiece piece : deck.asList()) {
-                    piece.setMap(newMap);
-                    piece.setPosition(pt);
-                  }
-
-                  refreshable++;
-                  break;
-                }
-              }
-              if (!deckFound) {
+              final String deckName = deck.getDeckName();
+
+              // Match with a DrawPile if possible. Take into account duplicate Deck names and moved
+              final DrawPile drawPile = findDrawPile(deck);
+              if (drawPile == null) {
                 deletable++;
                 decksToDelete.add(deck);
               }
+              else {
+                foundDrawPiles.add(drawPile);
+
+                final String drawPileName = drawPile.getAttributeValueString(SetupStack.NAME);
+
+                if (!options.contains(SUPPRESS_INFO_REPORTS))
+                  log(Resources.getString("GameRefresher.refreshing_deck", deckName, drawPileName));
+
+                // This refreshes the existing deck with all the up-to-date drawPile fields from the module
+                deck.removeListeners();
+                deck.myRefreshType(drawPile.getDeckType());
+                deck.addListeners();
+
+                // Make sure the deck is in the right place
+                final Point pt = drawPile.getPosition();
+                final Map newMap = drawPile.getMap();
+                if (newMap != map) {
+                  map.removePiece(deck);
+                  newMap.addPiece(deck);
+                }
+                deck.setPosition(pt);
+                for (final GamePiece piece : deck.asList()) {
+                  piece.setMap(newMap);
+                  piece.setPosition(pt);
+                }
+
+                refreshable++;
+                break;
+              }
             }
           }
         }
@@ -611,6 +595,49 @@ else if (!decksToDelete.isEmpty()) {
   }
 
 
+  /**
+   * Find the Vassal DrawPile that matches a given Deck.
+   * Deck Names may have been repeated (ill-advised). DrawPiles may have been added to or removed from the module
+   * or may exist on Boards that are not active in the current game
+   * If a DrawPile exists with a matching Deck Name on the same Map, return it.
+   * Otherwise, return the first DrawPile found with a matching name
+   *
+   * @param deck Deck to search for
+   * @return     Matching DrawPile
+   */
+  private DrawPile findDrawPile(Deck deck) {
+    DrawPile first = null;
+    DrawPile matching = null;
+    final String deckName = deck.getDeckName();
+    for (final DrawPile pile : getModuleDrawPiles()) {
+      if (deckName.equals(pile.getAttributeValueString(SetupStack.NAME))) {
+
+        // If drawPile is owned by a specific board, then we can only match it if that board is active in this game
+        final String pileBoardName = pile.getOwningBoardName();
+        if (pileBoardName != null) {
+          if (deck.getMap().getBoardByName(pileBoardName) == null) {
+            continue;
+          }
+
+          // If the drawPile is on a map that doesn't have its current owning board active, then we
+          // cannot match that drawPile.
+          if (pile.getMap().getBoardByName(pile.getOwningBoardName()) == null) {
+            continue;
+          }
+        }
+
+        if (first == null) {
+          first = pile;
+          if (deck.getMap().equals(pile.getMap())) {
+            matching = pile;
+            break;
+          }
+        }
+      }
+    }
+    return matching != null ? matching : first;
+  }
+
   /**
    * Before refreshing, we need to go through every piece and commemorate all the Attachment trait relationships, since they contain direct references to other GamePieces, and all the references are
    * about to be jumbled/invalidated when new updated versions of pieces are created.

From 4b47553273f8b71048808a48cb683c91f84f3f97 Mon Sep 17 00:00:00 2001
From: Brent <90901032@westernsydney.edu.au>
Date: Sat, 27 Jul 2024 13:35:06 +1000
Subject: [PATCH 2/2] Fix bad fix

---
 .../java/VASSAL/build/module/GameRefresher.java | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/vassal-app/src/main/java/VASSAL/build/module/GameRefresher.java b/vassal-app/src/main/java/VASSAL/build/module/GameRefresher.java
index 5c89e3f796..acbac8d36b 100644
--- a/vassal-app/src/main/java/VASSAL/build/module/GameRefresher.java
+++ b/vassal-app/src/main/java/VASSAL/build/module/GameRefresher.java
@@ -609,10 +609,11 @@ private DrawPile findDrawPile(Deck deck) {
     DrawPile first = null;
     DrawPile matching = null;
     final String deckName = deck.getDeckName();
+
     for (final DrawPile pile : getModuleDrawPiles()) {
       if (deckName.equals(pile.getAttributeValueString(SetupStack.NAME))) {
 
-        // If drawPile is owned by a specific board, then we can only match it if that board is active in this game
+        // If the drawPile is owned by a specific board, then we can only match it if that board is active in this game
         final String pileBoardName = pile.getOwningBoardName();
         if (pileBoardName != null) {
           if (deck.getMap().getBoardByName(pileBoardName) == null) {
@@ -625,16 +626,22 @@ private DrawPile findDrawPile(Deck deck) {
             continue;
           }
         }
+        // At this point, we have found an Active DrawPile with the same name as our Deck.
+
+        // Finish searching immediately if they are both on the same Map
+        if (deck.getMap().equals(pile.getMap())) {
+          matching = pile;
+          break;
+        }
 
+        // Otherwise, record the first matching Deck
         if (first == null) {
           first = pile;
-          if (deck.getMap().equals(pile.getMap())) {
-            matching = pile;
-            break;
-          }
         }
       }
     }
+
+    // Report a DrawPile on the same Map as our Deck in preference to one that is not.
     return matching != null ? matching : first;
   }