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 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; }