Some cleanup (#6712)

forge-ai/src/main/java/forge/ai/AiController.java

@@ -18,7 +18,6 @@
 package forge.ai;
 
 import com.esotericsoftware.minlog.Log;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 
 import forge.ai.AiCardMemory.MemorySet;
@@ -64,6 +63,7 @@
 import io.sentry.Sentry;
 
 import java.util.*;
+import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.concurrent.CompletableFuture;
@@ -2208,8 +2208,6 @@ public List<SpellAbility> orderPlaySa(List<SpellAbility> activePlayerSAs) {
             return activePlayerSAs;
         }
 
-        List<SpellAbility> result = Lists.newArrayList();
-
         // filter list by ApiTypes
         List<SpellAbility> discard = filterListByApi(activePlayerSAs, ApiType.Discard);
         List<SpellAbility> mandatoryDiscard = filterList(discard, SpellAbilityPredicates.isMandatory());
@@ -2225,37 +2223,33 @@ public List<SpellAbility> orderPlaySa(List<SpellAbility> activePlayerSAs) {
         List<SpellAbility> pump = filterListByApi(activePlayerSAs, ApiType.Pump);
         List<SpellAbility> pumpAll = filterListByApi(activePlayerSAs, ApiType.PumpAll);
 
+        List<SpellAbility> result = Lists.newArrayList(activePlayerSAs);
+
         // do mandatory discard early if hand is empty or has DiscardMe card
-        boolean discardEarly = false;
         CardCollectionView playerHand = player.getCardsIn(ZoneType.Hand);
-        if (playerHand.isEmpty() || playerHand.anyMatch(CardPredicates.hasSVar("DiscardMe"))) {
-            discardEarly = true;
+        if (!playerHand.isEmpty() && !playerHand.anyMatch(CardPredicates.hasSVar("DiscardMe"))) {
             result.addAll(mandatoryDiscard);
+            mandatoryDiscard.clear();
         }
 
-        // token should be added first so they might get the pump bonus
-        result.addAll(token);
-        result.addAll(pump);
-        result.addAll(pumpAll);
+        // optional Discard, probably combined with Draw
+        result.addAll(discard);
+        // do Draw before Discard
+        result.addAll(draw);
 
-        // do Evolve Trigger before other PutCounter SpellAbilities
+        result.addAll(putCounterAll);
         // do putCounter before Draw/Discard because it can cause a Draw Trigger
-        result.addAll(evolve);
         result.addAll(putCounter);
-        result.addAll(putCounterAll);
-
-        // do Draw before Discard
-        result.addAll(draw);
-        result.addAll(discard); // optional Discard, probably combined with Draw
+        // do Evolve Trigger before other PutCounter SpellAbilities
+        result.addAll(evolve);
 
-        if (!discardEarly) {
-            result.addAll(mandatoryDiscard);
-        }
+        // token should be added first so they might get the pump bonus
+        result.addAll(pumpAll);
+        result.addAll(pump);
+        result.addAll(token);
 
-        result.addAll(activePlayerSAs);
+        result.addAll(mandatoryDiscard);
 
-        //need to reverse because of magic stack
-        Collections.reverse(result);
         return result;
     }
 
@@ -2267,6 +2261,10 @@ private static <T> List<T> filterList(List<T> input, Predicate<? super T> pred)
     }
 
     // TODO move to more common place
+    public static <T extends TriggerReplacementBase> List<T> filterList(List<T> input, Function<SpellAbility, Object> pred, Object value) {
+        return filterList(input, trb -> pred.apply(trb.ensureAbility()) == value);
+    }
+
     public static List<SpellAbility> filterListByApi(List<SpellAbility> input, ApiType type) {
         return filterList(input, SpellAbilityPredicates.isApi(type));
     }
@@ -2303,12 +2301,11 @@ private boolean checkAiSpecificRestrictions(final SpellAbility sa) {
     public ReplacementEffect chooseSingleReplacementEffect(List<ReplacementEffect> list) {
         // no need to choose anything
         if (list.size() <= 1) {
-            return Iterables.getFirst(list, null);
+            return list.get(0);
         }
 
-        ReplacementType mode = Iterables.getFirst(list, null).getMode();
+        ReplacementType mode = list.get(0).getMode();
 
-        // replace lifegain effects
         if (mode.equals(ReplacementType.GainLife)) {
             List<ReplacementEffect> noGain = filterListByAiLogic(list, "NoLife");
             List<ReplacementEffect> loseLife = filterListByAiLogic(list, "LoseLife");
@@ -2317,24 +2314,24 @@ public ReplacementEffect chooseSingleReplacementEffect(List<ReplacementEffect> l
 
             if (!noGain.isEmpty()) {
                 // no lifegain is better than lose life
-                return Iterables.getFirst(noGain, null);
+                return noGain.get(0);
             } else if (!loseLife.isEmpty()) {
                 // lose life before double life to prevent lose double
-                return Iterables.getFirst(loseLife, null);
+                return loseLife.get(0);
             } else if (!lichDraw.isEmpty()) {
                 // lich draw before double life to prevent to draw to much
-                return Iterables.getFirst(lichDraw, null);
+                return lichDraw.get(0);
             } else if (!doubleLife.isEmpty()) {
                 // other than that, do double life
-                return Iterables.getFirst(doubleLife, null);
+                return doubleLife.get(0);
             }
         } else if (mode.equals(ReplacementType.DamageDone)) {
             List<ReplacementEffect> prevention = filterList(list, CardTraitPredicates.hasParam("Prevent"));
 
             // TODO when Protection is done as ReplacementEffect do them
             // before normal prevention
             if (!prevention.isEmpty()) {
-                return Iterables.getFirst(prevention, null);
+                return prevention.get(0);
             }
         } else if (mode.equals(ReplacementType.Destroy)) {
             List<ReplacementEffect> shield = filterList(list, CardTraitPredicates.hasParam("ShieldCounter"));
@@ -2344,30 +2341,35 @@ public ReplacementEffect chooseSingleReplacementEffect(List<ReplacementEffect> l
 
             // Indestructible umbra armor is the best
             if (!umbraArmorIndestructible.isEmpty()) {
-                return Iterables.getFirst(umbraArmorIndestructible, null);
+                return umbraArmorIndestructible.get(0);
             }
 
             // then it might be better to remove shield counter if able?
             if (!shield.isEmpty()) {
-                return Iterables.getFirst(shield, null);
+                return shield.get(0);
             }
 
             // TODO get the RunParams for Affected to check if the creature already dealt combat damage for Regeneration effects
             // is using a Regeneration Effect better than using a Umbra Armor?
             if (!regeneration.isEmpty()) {
-                return Iterables.getFirst(regeneration, null);
+                return regeneration.get(0);
             }
 
             if (!umbraArmor.isEmpty()) {
                 // sort them by cmc
                 umbraArmor.sort(Comparator.comparing(CardTraitBase::getHostCard, Comparator.comparing(Card::getCMC)));
-                return Iterables.getFirst(umbraArmor, null);
+                return umbraArmor.get(0);
+            }
+        } else if (mode.equals(ReplacementType.Draw)) {
+            List<ReplacementEffect> winGame = filterList(list, SpellAbility::getApi, ApiType.WinsGame);
+            if (!winGame.isEmpty()) {
+                return winGame.get(0);
             }
         }
 
         // TODO always lower counters with Vorinclex first, might turn it from 1 to 0 as final
 
-        return Iterables.getFirst(list, null);
+        return list.get(0);
     }
 
 }

forge-ai/src/main/java/forge/ai/CreatureEvaluator.java

@@ -248,10 +248,10 @@ else if (c.hasKeyword(Keyword.WITHER)) {
                     value -= subValue(10, "echo-unpaid");
                 }
                 if (t.isKeyword(Keyword.FADING)) {
-                    value -= subValue(20 / (Math.max(1, c.getCounters(CounterEnumType.FADE))), "fading");
+                    value -= subValue(20 / (Math.max(1, c.isInPlay() ? c.getCounters(CounterEnumType.FADE) : c.getKeywordMagnitude(Keyword.FADING))), "fading");
                 }
                 if (t.isKeyword(Keyword.VANISHING)) {
-                    value -= subValue(20 / (Math.max(1, c.getCounters(CounterEnumType.TIME))), "vanishing");
+                    value -= subValue(20 / (Math.max(1, c.isInPlay() ? c.getCounters(CounterEnumType.TIME) : c.getKeywordMagnitude(Keyword.VANISHING))), "vanishing");
                 }
 
                 SpellAbility ab = t.ensureAbility();

forge-core/src/main/java/forge/card/CardRulesPredicates.java

@@ -253,7 +253,7 @@ public static Predicate<CardRules> hasColorIdentity(final int colormask) {
     }
 
     public static Predicate<CardRules> canBePartnerCommanderWith(final CardRules commander) {
-        return (rules) -> rules.canBePartnerCommanders(commander);
+        return rules -> rules.canBePartnerCommanders(commander);
     }
 
     private static class LeafString extends PredicateString<CardRules> {

forge-core/src/main/java/forge/util/collect/FCollection.java

@@ -549,17 +549,17 @@ public T get(final T obj) {
 
     @Override
     public Stream<T> stream() {
-            return list.stream();
+        return list.stream();
     }
 
     @Override
     public boolean anyMatch(Predicate<? super T> test) {
-            return set.stream().anyMatch(test);
+        return set.stream().anyMatch(test);
     }
 
     @Override
     public boolean allMatch(Predicate<? super T> test) {
-            return set.stream().allMatch(test);
+        return set.stream().allMatch(test);
     }
 
     /**

forge-game/src/main/java/forge/game/ability/effects/ActivateAbilityEffect.java

@@ -5,7 +5,6 @@
 import org.apache.commons.lang3.StringUtils;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
 
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
@@ -45,7 +44,7 @@ public void resolve(SpellAbility sa) {
 
             List<Card> list = CardLists.getType(p.getCardsIn(ZoneType.Battlefield), sa.getParamOrDefault("Type", "Card"));
             for (Card c : list) {
-                List<SpellAbility> possibleAb = Lists.newArrayList(c.getAllPossibleAbilities(p, true));
+                List<SpellAbility> possibleAb = c.getAllPossibleAbilities(p, true);
                 if (isManaAb) {
                     possibleAb.retainAll((FCollection<SpellAbility>)c.getManaAbilities());
                 }

forge-game/src/main/java/forge/game/ability/effects/ChangeZoneEffect.java

@@ -880,13 +880,7 @@ private void changeKnownOriginResolve(final SpellAbility sa) {
      *            a {@link forge.game.spellability.SpellAbility} object.
      */
     private void changeHiddenOriginResolve(final SpellAbility sa) {
-        List<Player> fetchers;
-
-        if (sa.hasParam("DefinedPlayer")) {
-            fetchers = AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam("DefinedPlayer"), sa);
-        } else {
-            fetchers = Lists.newArrayList(sa.getActivatingPlayer());
-        }
+        List<Player> fetchers = AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam("DefinedPlayer"), sa);
 
         Player chooser = null;
         if (sa.hasParam("Chooser")) {
@@ -1059,7 +1053,7 @@ else if (origin.contains(ZoneType.Hand) && player.isOpponentOf(decider)) {
                     handleCastWhileSearching(fetchList, decider);
                 }
                 final Map<AbilityKey, Object> runParams = AbilityKey.mapFromPlayer(decider);
-                runParams.put(AbilityKey.Target, Lists.newArrayList(player));
+                runParams.put(AbilityKey.Target, player);
                 game.getTriggerHandler().runTrigger(TriggerType.SearchedLibrary, runParams, false);
             }
             if (searchedLibrary && sa.hasParam("Searched")) {

forge-game/src/main/java/forge/game/ability/effects/ChooseCardNameEffect.java

@@ -22,7 +22,6 @@ public class ChooseCardNameEffect extends SpellAbilityEffect {
 
     @Override
     protected String getStackDescription(SpellAbility sa) {
-
         return Lang.joinHomogenous(getTargetPlayers(sa)) + " names a card.";
     }
 
@@ -58,21 +57,6 @@ public void resolve(SpellAbility sa) {
                 continue;
             }
             String chosen;
-            //This section was used for Momir Avatar, which no longer uses it - commented out 7/28/2021
-            //if (randomChoice) {
-            //String numericAmount = "X";
-            //final int validAmount = StringUtils.isNumeric(numericAmount) ? Integer.parseInt(numericAmount) :
-            //    AbilityUtils.calculateAmount(host, numericAmount, sa);
-            // Momir needs PaperCard
-            //Collection<PaperCard> cards = StaticData.instance().getCommonCards().getUniqueCards();
-            //Predicate<PaperCard> cpp = Predicates.and(
-            //    Predicates.compose(CardRulesPredicates.IS_CREATURE, PaperCard.FN_GET_RULES),
-            //    Predicates.compose(CardRulesPredicates.cmc(ComparableOp.EQUALS, validAmount), PaperCard.FN_GET_RULES));
-            //cards = Lists.newArrayList(Iterables.filter(cards, cpp));
-            //if (!cards.isEmpty()) { chosen = Aggregates.random(cards).getName();
-            //} else {
-            //    chosen = "";
-            //}
             if (chooseFromDefined) {
                 CardCollection choices = AbilityUtils.getDefinedCards(host, sa.getParam("ChooseFromDefinedCards"), sa);
                 choices = CardLists.getValidCards(choices, valid, host.getController(), host, sa);

forge-game/src/main/java/forge/game/ability/effects/ControlExchangeEffect.java

@@ -2,12 +2,11 @@
 
 import java.util.List;
 
-import com.google.common.collect.Lists;
-
 import forge.game.Game;
 import forge.game.ability.AbilityUtils;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
+import forge.game.card.CardCollectionView;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.util.CardTranslation;
@@ -23,9 +22,9 @@ public class ControlExchangeEffect extends SpellAbilityEffect {
     protected String getStackDescription(SpellAbility sa) {
         Card object1 = null;
         Card object2 = null;
-        List<Card> tgts = null;
+        CardCollectionView tgts = null;
         if (sa.usesTargeting()) {
-            tgts = Lists.newArrayList(sa.getTargets().getTargetCards());
+            tgts = sa.getTargets().getTargetCards();
             if (tgts.size() > 0) {
                 object1 = tgts.get(0);
             }
@@ -57,9 +56,9 @@ public void resolve(SpellAbility sa) {
         Card object1 = null;
         Card object2 = null;
 
-        List<Card> tgts = null;
+        CardCollectionView tgts = null;
         if (sa.usesTargeting()) {
-            tgts = Lists.newArrayList(sa.getTargets().getTargetCards());
+            tgts = sa.getTargets().getTargetCards();
             if (tgts.size() > 0) {
                 object1 = tgts.get(0);
             }

forge-game/src/main/java/forge/game/ability/effects/CopySpellAbilityEffect.java

@@ -72,10 +72,7 @@ public void resolve(SpellAbility sa) {
             return;
         }
 
-        List<Player> controllers = Lists.newArrayList(sa.getActivatingPlayer());
-        if (sa.hasParam("Controller")) {
-            controllers = AbilityUtils.getDefinedPlayers(card, sa.getParam("Controller"), sa);
-        }
+        List<Player> controllers = AbilityUtils.getDefinedPlayers(card, sa.getParam("Controller"), sa);
 
         boolean isOptional = sa.hasParam("Optional");
 

forge-game/src/main/java/forge/game/ability/effects/DrawEffect.java

@@ -65,7 +65,7 @@ public void resolve(SpellAbility sa) {
 
         final List<Player> tgts = getTargetPlayersWithDuplicates(true, "Defined", sa);
 
-        for (final Player p : Sets.newHashSet(tgts)) {
+        for (final Player p : Sets.newLinkedHashSet(tgts)) {
             if (!p.isInGame()) {
                 continue;
             }

forge-game/src/main/java/forge/game/ability/effects/FlipOntoBattlefieldEffect.java

@@ -98,7 +98,7 @@ private Card getNeighboringCard(Card c, int direction) {
         // as well as the current card attachments that are visually located next to the requested card or are assumed to be near it.
         Player controller = c.getController();
         ArrayList<Card> attachments = Lists.newArrayList();
-        ArrayList<Card> cardsOTB = Lists.newArrayList(CardLists.filter(
+        CardCollection cardsOTB = CardLists.filter(
                 controller.getCardsIn(ZoneType.Battlefield), card -> {
                     if (card.isAttachedToEntity(c)) {
                         attachments.add(card);
@@ -114,7 +114,7 @@ private Card getNeighboringCard(Card c, int direction) {
                     }
                     return card.sharesCardTypeWith(c);
                 }
-        ));
+        );
 
         // Chance to hit an attachment
         float hitAttachment = 0.50f;

forge-game/src/main/java/forge/game/ability/effects/LifeGainEffect.java

@@ -26,7 +26,7 @@ protected String getStackDescription(SpellAbility sa) {
 
         sb.append(Lang.joinHomogenous(getDefinedPlayersOrTargeted(sa)));
         if (sb.length() == 0 && spellDesc != null) {
-            return (spellDesc);
+            return spellDesc;
         } else {
             sb.append(getDefinedPlayersOrTargeted(sa).size() > 1 ? " gain " : " gains ");
             if (!StringUtils.isNumeric(amountStr) && spellDesc != null && spellDesc.contains("life equal to")) {

forge-game/src/main/java/forge/game/ability/effects/RestartGameEffect.java

@@ -29,7 +29,7 @@ public void resolve(SpellAbility sa) {
         List<ZoneType> restartZones = new ArrayList<>(Arrays.asList(ZoneType.Battlefield,
                 ZoneType.Library, ZoneType.Graveyard, ZoneType.Hand, ZoneType.Exile));
 
-        ZoneType leaveZone = ZoneType.smartValueOf(sa.hasParam("RestrictFromZone") ? sa.getParam("RestrictFromZone") : null);
+        ZoneType leaveZone = ZoneType.smartValueOf(sa.getParam("RestrictFromZone"));
         restartZones.remove(leaveZone);
         String leaveRestriction = sa.getParamOrDefault("RestrictFromValid", "Card");
 

forge-game/src/main/java/forge/game/spellability/AbilityManaPart.java

@@ -252,7 +252,7 @@ public void addNoCounterEffect(SpellAbility saBeingPaid) {
         eff.setColor(MagicColor.COLORLESS);
         eff.setGamePieceType(GamePieceType.EFFECT);
 
-        String cantcounterstr = "Event$ Counter | ValidCard$ Card.IsRemembered | Description$ That spell can't be countered.";
+        String cantcounterstr = "Event$ Counter | ValidSA$ Spell.IsRemembered | Description$ That spell can't be countered.";
         ReplacementEffect re = ReplacementHandler.parseReplacement(cantcounterstr, eff, true);
         re.setLayer(ReplacementLayer.CantHappen);
         eff.addReplacementEffect(re);