apply more review changes

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

@@ -37,9 +37,10 @@ public interface Filter<E> extends Serializable, Copyable<Filter<E>> {
      * <p>
      * (method should then call this.addExtra(predicate) after verify checks)
      */
-    void add(ObjectSourcePlayerPredicate predicate);
+    Filter<E> add(ObjectSourcePlayerPredicate predicate);
 
-    // TODO: if someone can find a way to not have to add this (overload of add made it necessary to introduce)
+    // TODO: if someone can find a way to not have to add this
+    //       Compiler was confused between overloads
     void addExtra(ObjectSourcePlayerPredicate predicate);
 
     boolean checkObjectClass(Object object);

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

@@ -41,15 +41,11 @@ public class FilterAbility extends FilterImpl<Ability> {
     }
 
     @Override
-    public Filter<Ability> add(Predicate<? super Ability> predicate) {
+    public FilterAbility add(ObjectSourcePlayerPredicate predicate) {
-        return super.add(predicate);
-    }
-
-    @Override
-    public void add(ObjectSourcePlayerPredicate predicate) {
         // Verify Checks
         Predicates.makeSurePredicateCompatibleWithFilter(predicate, Ability.class, StackObject.class);
         this.addExtra(predicate);
+        return this;
     }
 
     @Override

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

@@ -5,11 +5,9 @@ import mage.constants.TargetController;
 import mage.filter.predicate.ObjectSourcePlayerPredicate;
 import mage.filter.predicate.Predicate;
 import mage.filter.predicate.Predicates;
-import mage.game.Game;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.UUID;
 
 /**
  * Works with cards only. For objects like commanders you must override your canTarget method.
@@ -54,11 +52,12 @@ public class FilterCard extends FilterObject<Card> {
     }
 
     @Override
-    public void add(ObjectSourcePlayerPredicate predicate) {
+    public FilterCard add(ObjectSourcePlayerPredicate predicate) {
         // verify checks
         checkPredicateIsSuitableForCardFilter(predicate);
         Predicates.makeSurePredicateCompatibleWithFilter(predicate, Card.class);
         this.addExtra(predicate);
+        return this;
     }
 
     @Override

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

@@ -51,24 +51,20 @@ public abstract class FilterImpl<E> implements Filter<E> {
         if (!this.match(object, game)) {
             return false;
         }
-        ObjectSourcePlayer<E> osp = new ObjectSourcePlayer<>(object, sourceControllerId, source);
+        ObjectSourcePlayer osp = new ObjectSourcePlayer<>(object, sourceControllerId, source);
         return extraPredicates.stream().allMatch(p -> p.apply(osp, game));
     }
 
     @Override
     public Filter<E> add(Predicate<? super E> predicate) {
-        if (isLockedFilter()) {
+        checkUnlockedFilter();
-            throw new UnsupportedOperationException("You may not modify a locked filter");
-        }
         predicates.add(predicate);
         return this;
     }
 
     @Override
     public final void addExtra(ObjectSourcePlayerPredicate predicate) {
-        if (isLockedFilter()) {
+        checkUnlockedFilter();
-            throw new UnsupportedOperationException("You may not modify a locked filter");
-        }
         extraPredicates.add(predicate);
     }
 
@@ -79,10 +75,14 @@ public abstract class FilterImpl<E> implements Filter<E> {
 
     @Override
     public final void setMessage(String message) {
+        checkUnlockedFilter();
+        this.message = message;
+    }
+
+    protected void checkUnlockedFilter() {
         if (isLockedFilter()) {
             throw new UnsupportedOperationException("You may not modify a locked filter");
         }
-        this.message = message;
     }
 
     @Override

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

@@ -27,10 +27,11 @@ public class FilterObject<E extends MageObject> extends FilterImpl<E> {
     }
 
     @Override
-    public void add(ObjectSourcePlayerPredicate predicate) {
+    public FilterObject<E> add(ObjectSourcePlayerPredicate predicate) {
         // verify checks
         Predicates.makeSurePredicateCompatibleWithFilter(predicate, MageObject.class);
         this.addExtra(predicate);
+        return this;
     }
 
     @Override

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

@@ -1,11 +1,14 @@
 package mage.filter;
 
+import mage.abilities.Ability;
 import mage.constants.SubType;
 import mage.filter.predicate.ObjectSourcePlayerPredicate;
 import mage.filter.predicate.Predicates;
+import mage.game.Game;
 import mage.game.permanent.Permanent;
 
 import java.util.Set;
+import java.util.UUID;
 
 /**
  * @author North
@@ -42,10 +45,27 @@ public class FilterPermanent extends FilterObject<Permanent> {
     }
 
     @Override
-    public void add(ObjectSourcePlayerPredicate predicate) {
+    public FilterPermanent add(ObjectSourcePlayerPredicate predicate) {
         // verify checks
         Predicates.makeSurePredicateCompatibleWithFilter(predicate, Permanent.class);
         this.addExtra(predicate);
+        return this;
+    }
+
+    @Override
+    public boolean match(Permanent permanent, Game game) {
+        // TODO: if we can trust the target/checks using FilterPermanent to filter out Phased out permanent,
+        //       this overload would be no longer necessary.
+        return super.match(permanent, game)
+                && permanent.isPhasedIn();
+    }
+
+    @Override
+    public boolean match(Permanent permanent, UUID sourceControllerId, Ability source, Game game) {
+        // TODO: if we can trust the target/checks using FilterPermanent to filter out Phased out permanent,
+        //       this overload would be no longer necessary.
+        return super.match(permanent, sourceControllerId, source, game)
+                && permanent.isPhasedIn();
     }
 
     @Override

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

@@ -28,10 +28,11 @@ public class FilterPlayer extends FilterImpl<Player> {
     }
 
     @Override
-    public void add(ObjectSourcePlayerPredicate predicate) {
+    public FilterPlayer add(ObjectSourcePlayerPredicate predicate) {
         // verify checks
         Predicates.makeSurePredicateCompatibleWithFilter(predicate, Player.class);
         this.addExtra(predicate);
+        return this;
     }
 
     @Override

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

@@ -32,11 +32,12 @@ public class FilterSource extends FilterObject<MageObject> {
     }
 
     @Override
-    public void add(ObjectSourcePlayerPredicate predicate) {
+    public FilterSource add(ObjectSourcePlayerPredicate predicate) {
         // verify checks
         // A source can be a lot of different things, so a variety of predicates can be fed here
         Predicates.makeSurePredicateCompatibleWithFilter(predicate, Permanent.class, Card.class, StackObject.class, CommandObject.class);
         this.addExtra(predicate);
+        return this;
     }
 
     @Override

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

@@ -29,11 +29,13 @@ public class FilterStackObject extends FilterObject<StackObject> {
         return new FilterStackObject(this);
     }
 
-    public final void add(ObjectSourcePlayerPredicate predicate) {
+    @Override
+    public final FilterStackObject add(ObjectSourcePlayerPredicate predicate) {
         // verify checks
         // Spell implements Card interface, so it can use some default predicates like owner
         Predicates.makeSurePredicateCompatibleWithFilter(predicate, StackObject.class, Spell.class, Card.class);
         this.addExtra(predicate);
+        return this;
     }
 
     @Override

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

@@ -32,7 +32,10 @@ public abstract class MultiFilterImpl<E> implements Filter<E> {
         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()));
+        this.innerFilters.addAll(
+                Arrays.stream(filters)
+                        .map(f -> f.copy())
+                        .collect(Collectors.toList()));
     }
 
     protected MultiFilterImpl(final MultiFilterImpl<E> filter) {
@@ -46,25 +49,26 @@ public abstract class MultiFilterImpl<E> implements Filter<E> {
     public boolean match(E object, Game game) {
         return innerFilters
                 .stream()
-                .anyMatch((Filter filter) -> filter.match(object, game));
+                .anyMatch((Filter filter) -> filter.checkObjectClass(object) && filter.match(object, game));
     }
 
     @Override
     public boolean match(E object, UUID sourceControllerId, Ability source, Game game) {
         return innerFilters
                 .stream()
-                .anyMatch((Filter filter) -> filter.match(object, sourceControllerId, source, game));
+                .anyMatch((Filter filter) -> filter.checkObjectClass(object) && filter.match(object, sourceControllerId, source, game));
     }
 
     @Override
-    public Filter<E> add(Predicate<? super E> predicate) {
+    public MultiFilterImpl<E> add(Predicate<? super E> predicate) {
         innerFilters.forEach((Filter filter) -> filter.add(predicate));
         return this;
     }
 
     @Override
-    public void add(ObjectSourcePlayerPredicate predicate) {
+    public MultiFilterImpl<E> add(ObjectSourcePlayerPredicate predicate) {
         innerFilters.forEach((Filter filter) -> filter.add(predicate));
+        return this;
     }
 
     @Override

Mage/src/main/java/mage/players/ManaPool.java

@@ -101,7 +101,7 @@ public class ManaPool implements Serializable {
     /**
      * @param manaType      the mana type that should be paid
      * @param ability
-     * @param filter
+     * @param filter        filters the source of mana, only matching source are accepted.
      * @param game
      * @param costToPay     complete costs to pay (needed to check conditional
      *                      mana)
@@ -140,7 +140,8 @@ public class ManaPool implements Serializable {
         }
 
         for (ManaPoolItem mana : manaItems) {
-            if (filter != null && !filter.match(mana.getSourceObject(), game)) {
+            MageObject sourceObject = mana.getSourceObject();
+            if (filter != null && (!filter.checkObjectClass(sourceObject) || !filter.match(sourceObject, game))) {
                 // If here, then mana source does not match the filter
                 // However, alternate mana payment abilities such as convoke won't match the filter but are valid
                 // So we need to do some ugly checks to allow them