cleanup: CardsImpl and related (#11585)

* minor cleanup of unused params in ExileZone

* cleanup CardsImpl

standardize logic for different methods

remove unused ownerId param

Mage/src/main/java/mage/cards/CardImpl.java

@@ -462,7 +462,7 @@ public abstract class CardImpl extends MageObjectImpl implements Card {
                     break;
                 case EXILED:
                     if (game.getExile().getCard(getId(), game) != null) {
-                        removed = game.getExile().removeCard(this, game);
+                        removed = game.getExile().removeCard(this);
                     }
                     break;
                 case STACK:

Mage/src/main/java/mage/cards/Cards.java

@@ -40,6 +40,9 @@ public interface Cards extends Set<UUID>, Serializable, Copyable<Cards> {
 
     Set<Card> getCards(Game game);
 
+    /**
+     * Warning: this method ignores ObjectSourcePlayer predicates in the filter
+     */
     Set<Card> getCards(FilterCard filter, Game game);
 
     Set<Card> getCards(FilterCard filter, UUID playerId, Ability source, Game game);
@@ -56,6 +59,9 @@ public interface Cards extends Set<UUID>, Serializable, Copyable<Cards> {
 
     Card getRandom(Game game);
 
+    /**
+     * Warning: this method ignores ObjectSourcePlayer predicates in the filter
+     */
     int count(FilterCard filter, Game game);
 
     int count(FilterCard filter, UUID playerId, Game game);

Mage/src/main/java/mage/cards/CardsImpl.java

@@ -20,8 +20,6 @@ public class CardsImpl extends LinkedHashSet<UUID> implements Cards, Serializabl
 
     private static final ThreadLocalStringBuilder threadLocalBuilder = new ThreadLocalStringBuilder(200);
 
-    private UUID ownerId;
-
     public CardsImpl() {
     }
 
@@ -47,7 +45,6 @@ public class CardsImpl extends LinkedHashSet<UUID> implements Cards, Serializabl
 
     protected CardsImpl(final CardsImpl cards) {
         this.addAll(cards);
-        this.ownerId = cards.ownerId;
     }
 
     @Override
@@ -85,11 +82,11 @@ public class CardsImpl extends LinkedHashSet<UUID> implements Cards, Serializabl
             return null;
         }
 
-        // neccessary if permanent tokens are in the collection
+        // necessary if permanent tokens are in the collection
         Set<MageObject> cardsForRandomPick = this
-                .stream().map(uuid -> game.getObject(uuid))
+                .stream().map(game::getObject)
                 .filter(Objects::nonNull)
-                .filter(mageObject -> mageObject instanceof Card)
+                .filter(Card.class::isInstance)
                 .collect(Collectors.toSet());
 
         return (Card) RandomUtil.randomFromCollection(cardsForRandomPick);
@@ -117,46 +114,37 @@ public class CardsImpl extends LinkedHashSet<UUID> implements Cards, Serializabl
 
     @Override
     public Set<Card> getCards(FilterCard filter, UUID playerId, Ability source, Game game) {
-        Set<Card> cards = new LinkedHashSet<>();
+        return stream()
-        for (UUID cardId : this) {
+                .map(cardId -> getPermanentOrCard(cardId, game))
-            Card card = game.getCard(cardId);
+                .filter(Objects::nonNull)
-            if (card != null) {
+                .filter(card -> filter.match(card, playerId, source, game))
-                boolean match = filter.match(card, playerId, source, game);
+                .collect(Collectors.toCollection(LinkedHashSet::new));
-                if (match) {
-                    cards.add(game.getCard(cardId));
-                }
-            }
-        }
-        return cards;
     }
 
-    // TODO: Why is this used a completely different implementation than the version without the filter?
     @Override
     public Set<Card> getCards(FilterCard filter, Game game) {
         return stream()
-                .map(game::getCard)
+                .map(cardId -> getPermanentOrCard(cardId, game))
                 .filter(Objects::nonNull)
                 .filter(card -> filter.match(card, game))
-                .collect(Collectors.toSet());
+                .collect(Collectors.toCollection(LinkedHashSet::new));
     }
 
     @Override
     public Set<Card> getCards(Game game) {
-        Set<Card> cards = new LinkedHashSet<>();
+        return stream()
-        for (Iterator<UUID> it = this.iterator(); it.hasNext(); ) { // Changed to iterator because of ConcurrentModificationException
+                .map(cardId -> getPermanentOrCard(cardId, game))
-            UUID cardId = it.next();
+                .filter(Objects::nonNull)
+                .collect(Collectors.toCollection(LinkedHashSet::new));
+    }
 
     // cards from battlefield must be as permanent, not card (moveCards uses instanceOf Permanent)
+    private static Card getPermanentOrCard(UUID cardId, Game game) {
         Card card = game.getPermanent(cardId);
         if (card == null) {
             card = game.getCard(cardId);
         }
-
+        return card;
-            if (card != null) { // this can happen during the cancelation (player concedes) of a game
-                cards.add(card);
-            }
-        }
-        return cards;
     }
 
     @Override

Mage/src/main/java/mage/game/Exile.java

@@ -45,11 +45,7 @@ public class Exile implements Serializable, Copyable<Exile> {
     }
 
     public ExileZone createZone(UUID id, String name) {
-        return createZone(id, name + " - Exile", false);
+        return exileZones.computeIfAbsent(id, x -> new ExileZone(id, name + " - Exile"));
-    }
-
-    private ExileZone createZone(UUID id, String name, boolean hidden) {
-        return exileZones.computeIfAbsent(id, x -> new ExileZone(id, name, hidden));
     }
 
     public ExileZone getExileZone(UUID id) {
@@ -77,10 +73,6 @@ public class Exile implements Serializable, Copyable<Exile> {
 
     /**
      * Return exiled cards owned by a specific player. Use it in effects to find all cards in range.
-     *
-     * @param game
-     * @param fromPlayerId
-     * @return
      */
     public List<Card> getAllCards(Game game, UUID fromPlayerId) {
         List<Card> res = new ArrayList<>();
@@ -102,7 +94,7 @@ public class Exile implements Serializable, Copyable<Exile> {
         return res;
     }
 
-    public boolean removeCard(Card card, Game game) {
+    public boolean removeCard(Card card) {
         for (ExileZone exile : exileZones.values()) {
             if (exile.contains(card.getId())) {
                 return exile.remove(card.getId());
@@ -113,10 +105,6 @@ public class Exile implements Serializable, Copyable<Exile> {
 
     /**
      * Move card from one exile zone to another. Use case example: create special zone for exiled and castable card.
-     *
-     * @param card
-     * @param game
-     * @param toZoneId
      */
     public void moveToAnotherZone(Card card, Game game, ExileZone exileZone) {
         if (getCard(card.getId(), game) == null) {
@@ -125,8 +113,7 @@ public class Exile implements Serializable, Copyable<Exile> {
         if (exileZone == null) {
             throw new IllegalArgumentException("Exile zone must exists: " + card.getIdName());
         }
-
+        removeCard(card);
-        removeCard(card, game);
         exileZone.add(card);
     }
 

Mage/src/main/java/mage/game/ExileZone.java

@@ -9,32 +9,20 @@ import java.util.UUID;
  */
 public class ExileZone extends CardsImpl {
 
-    private UUID id;
+    private final UUID id;
-    private String name;
+    private final String name;
-    private boolean hidden;
     private boolean cleanupOnEndTurn = false; // moved cards from that zone to default on end of turn (to cleanup exile windows)
 
     public ExileZone(UUID id, String name) {
-        this(id, name, false);
-    }
-
-    public ExileZone(UUID id, String name, boolean hidden) {
-        this(id, name, false, false);
-    }
-
-    public ExileZone(UUID id, String name, boolean hidden, boolean cleanupOnEndTurn) {
         super();
         this.id = id;
         this.name = name;
-        this.hidden = hidden;
-        this.cleanupOnEndTurn = cleanupOnEndTurn;
     }
 
     protected ExileZone(final ExileZone zone) {
         super(zone);
         this.id = zone.id;
         this.name = zone.name;
-        this.hidden = zone.hidden;
         this.cleanupOnEndTurn = zone.cleanupOnEndTurn;
     }
 
@@ -46,10 +34,6 @@ public class ExileZone extends CardsImpl {
         return name;
     }
 
-    public boolean isHidden() {
-        return hidden;
-    }
-
     public boolean isCleanupOnEndTurn() {
         return cleanupOnEndTurn;
     }

Mage/src/main/java/mage/game/GameImpl.java

@@ -2338,7 +2338,7 @@ public abstract class GameImpl implements Game {
                 }
 
                 case EXILED: {
-                    getExile().removeCard(copiedCard, this);
+                    getExile().removeCard(copiedCard);
                     break;
                 }