apply review

Author: Susucre

Mage/src/main/java/mage/filter/FilterCard.java

@@ -73,6 +73,9 @@ public void add(ObjectSourcePlayerPredicate predicate) {
 
     @Override
     public boolean checkObjectClass(Object object) {
+        // TODO: investigate if we can/should exclude Permanent here.
+        //       as it does extend Card (if so do cleanup the
+        //       MultiFilterImpl that match Permanent and Card/Spell)
         return object instanceof Card;
     }
 }

Mage/src/main/java/mage/filter/FilterImpl.java

@@ -34,7 +34,7 @@ public FilterImpl(String name) {
     protected FilterImpl(final FilterImpl<E> filter) {
         this.message = filter.message;
         this.predicates = new ArrayList<>(filter.predicates);
-        this.extraPredicates.addAll(filter.extraPredicates);
+        this.extraPredicates = new ArrayList<>(filter.extraPredicates);
         this.lockedFilter = false;// After copying a filter it's allowed to modify
     }
 
@@ -46,6 +46,7 @@ public boolean match(E e, Game game) {
         return false;
     }
 
+    @Override
     public boolean match(E object, UUID sourceControllerId, Ability source, Game game) {
         if (!this.match(object, game)) {
             return false;

Mage/src/main/java/mage/filter/MultiFilterImpl.java

@@ -13,6 +13,9 @@
 
 /**
  * Make a Filter out of multiple inner filters.
+ * If making a multi filter out of filterA and filterB,
+ * any object match the multi filter if it either match
+ * filterA or match filterB.
  *
  * @author Susucr
  */
@@ -26,6 +29,9 @@ public abstract class MultiFilterImpl<E> implements Filter<E> {
 
     protected MultiFilterImpl(String name, Filter<? extends E>... filters) {
         this.message = name;
+        if (filters.length < 2) {
+            throw new IllegalArgumentException("Wrong code usage: MultiFilterImpl should have at least 2 inner filters");
+        }
         this.innerFilters.addAll(Arrays.stream(filters).collect(Collectors.toList()));
     }
 
@@ -43,6 +49,7 @@ public boolean match(E object, Game game) {
                 .anyMatch((Filter filter) -> filter.match(object, game));
     }
 
+    @Override
     public boolean match(E object, UUID sourceControllerId, Ability source, Game game) {
         return innerFilters
                 .stream()

Mage/src/main/java/mage/filter/common/FilterPermanentOrSuspendedCard.java

@@ -1,12 +1,18 @@
 package mage.filter.common;
 
 import mage.MageObject;
+import mage.abilities.Ability;
 import mage.abilities.keyword.SuspendAbility;
+import mage.cards.Card;
 import mage.counters.CounterType;
 import mage.filter.FilterCard;
 import mage.filter.FilterPermanent;
 import mage.filter.MultiFilterImpl;
 import mage.filter.predicate.mageobject.AbilityPredicate;
+import mage.game.Game;
+import mage.game.permanent.Permanent;
+
+import java.util.UUID;
 
 /**
  * @author emerald000
@@ -27,6 +33,32 @@ protected FilterPermanentOrSuspendedCard(final FilterPermanentOrSuspendedCard fi
         super(filter);
     }
 
+    @Override
+    public boolean match(MageObject object, Game game) {
+        // We can not rely on super.match, since Permanent extend Card
+        // so a Permanent could get filtered by the FilterCard
+        if (object instanceof Permanent) {
+            return getPermanentFilter().match((Permanent) object, game);
+        } else if (object instanceof Card) {
+            return getCardFilter().match((Card) object, game);
+        } else {
+            return false;
+        }
+    }
+
+    @Override
+    public boolean match(MageObject object, UUID sourceControllerId, Ability source, Game game) {
+        // We can not rely on super.match, since Permanent extend Card
+        // so a Permanent could get filtered by the FilterCard
+        if (object instanceof Permanent) {
+            return getPermanentFilter().match((Permanent) object, sourceControllerId, source, game);
+        } else if (object instanceof Card) {
+            return getCardFilter().match((Card) object, sourceControllerId, source, game);
+        } else {
+            return false;
+        }
+    }
+
     @Override
     public FilterPermanentOrSuspendedCard copy() {
         return new FilterPermanentOrSuspendedCard(this);

Mage/src/main/java/mage/filter/common/FilterSpellOrPermanent.java

@@ -1,9 +1,15 @@
 package mage.filter.common;
 
 import mage.MageObject;
+import mage.abilities.Ability;
 import mage.filter.FilterPermanent;
 import mage.filter.FilterSpell;
 import mage.filter.MultiFilterImpl;
+import mage.game.Game;
+import mage.game.permanent.Permanent;
+import mage.game.stack.Spell;
+
+import java.util.UUID;
 
 /**
  * @author LevelX
@@ -22,6 +28,33 @@ protected FilterSpellOrPermanent(final FilterSpellOrPermanent filter) {
         super(filter);
     }
 
+
+    @Override
+    public boolean match(MageObject object, Game game) {
+        // We can not rely on super.match, since Permanent extend Card
+        // and currently FilterSpell accepts to filter Cards
+        if (object instanceof Permanent) {
+            return getPermanentFilter().match((Permanent) object, game);
+        } else if (object instanceof Spell) {
+            return getSpellFilter().match((Spell) object, game);
+        } else {
+            return false;
+        }
+    }
+
+    @Override
+    public boolean match(MageObject object, UUID sourceControllerId, Ability source, Game game) {
+        // We can not rely on super.match, since Permanent extend Card
+        // and currently FilterSpell accepts to filter Cards
+        if (object instanceof Permanent) {
+            return getPermanentFilter().match((Permanent) object, sourceControllerId, source, game);
+        } else if (object instanceof Spell) {
+            return getSpellFilter().match((Spell) object, sourceControllerId, source, game);
+        } else {
+            return false;
+        }
+    }
+
     @Override
     public FilterSpellOrPermanent copy() {
         return new FilterSpellOrPermanent(this);