Changes related to Cascade ability (#7583):

* Cascade: added correct spell ability choose for forced cast of mdf and adventure cards (can contains one or both sides);
* Cascade: added tests from latest oracle changes;
* AI: improved spell ability choose for forced cast (example: cast target card without mana cost);
* GUI: improved spell ability choose for forced cast (now you can see only castable spells to choose);
* Other: fixed wrong PlayFromNotOwnHandZone in some cards, fixed NPE;

Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java

@@ -2781,6 +2781,12 @@ public class ComputerPlayer extends PlayerImpl implements Player {
         return randomOpponentId;
     }
 
+    @Override
+    public SpellAbility chooseAbilityForCast(Card card, Game game, boolean noMana) {
+        Map<UUID, ActivatedAbility> useable = PlayerImpl.getSpellAbilities(this.getId(), card, game.getState().getZone(card.getId()), game);
+        return (SpellAbility) useable.values().stream().findFirst().orElse(null);
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) {

Mage.Sets/src/mage/cards/g/GodEternalKefnet.java

@@ -90,7 +90,6 @@ class GodEternalKefnetDrawCardReplacementEffect extends ReplacementEffectImpl {
         you.setTopCardRevealed(true);
 
         // cast copy
-
         if (topCard.isInstantOrSorcery()
                 && you.chooseUse(outcome, "Would you like to copy " + topCard.getName() + " and cast it for {2} less?", source, game)) {
             Card blueprint = topCard.copy();
@@ -105,7 +104,9 @@ class GodEternalKefnetDrawCardReplacementEffect extends ReplacementEffectImpl {
             }
             Card copiedCard = game.copyCard(blueprint, source, source.getControllerId());
             you.moveCardToHandWithInfo(copiedCard, source, game, true); // The copy is created in and cast from your hand. (2019-05-03)
+            game.getState().setValue("PlayFromNotOwnHandZone" + copiedCard.getId(), Boolean.TRUE);
             you.cast(you.chooseAbilityForCast(copiedCard, game, false), game, false, new ApprovingObject(source, game));
+            game.getState().setValue("PlayFromNotOwnHandZone" + copiedCard.getId(), null);
         }
 
         // draw (return false for default draw)

Mage.Sets/src/mage/cards/j/JestersScepter.java

@@ -156,7 +156,8 @@ class JestersScepterCost extends CostImpl {
             TargetCardInExile target = new TargetCardInExile(new FilterCard(), CardUtil.getCardExileZoneId(game, ability));
             target.setNotTarget(true);
             Cards cards = game.getExile().getExileZone(CardUtil.getCardExileZoneId(game, ability));
-            if (!cards.isEmpty()
+            if (cards != null
+                    && !cards.isEmpty()
                     && controller.choose(Outcome.Benefit, cards, target, game)) {
                 Card card = game.getCard(target.getFirstTarget());
                 if (card != null) {

Mage.Sets/src/mage/cards/k/KahoMinamoHistorian.java

@@ -130,7 +130,8 @@ class KahoMinamoHistorianCastEffect extends OneShotEffect {
             filter.add(new ConvertedManaCostPredicate(ComparisonType.EQUAL_TO, source.getManaCostsToPay().getX()));
             TargetCardInExile target = new TargetCardInExile(filter, CardUtil.getCardExileZoneId(game, source));
             Cards cards = game.getExile().getExileZone(CardUtil.getCardExileZoneId(game, source));
-            if (!cards.isEmpty()
+            if (cards != null
+                    && !cards.isEmpty()
                     && controller.choose(Outcome.PlayForFree, cards, target, game)) {
                 Card card = game.getCard(target.getFirstTarget());
                 if (card != null) {

Mage.Sets/src/mage/cards/m/MesmericFiend.java

@@ -122,7 +122,7 @@ class MesmericFiendLeaveEffect extends OneShotEffect {
             UUID exileId = (UUID) game.getState().getValue(source.getSourceId().toString() + zoneChangeMinusOne);
             if (exileId != null) {
                 Cards cards = game.getExile().getExileZone(exileId);
-                if (!cards.isEmpty()) {
+                if (cards != null && !cards.isEmpty()) {
                     return controller.moveCards(cards, Zone.HAND, source, game);
                 }
             }

Mage.Sets/src/mage/cards/v/ValkiGodOfLies.java

@@ -124,6 +124,7 @@ class ValkiGodOfLiesRevealExileEffect extends OneShotEffect {
                 if (opponent != null) {
                     opponent.revealCards(source, opponent.getHand(), game);
                     TargetCard targetToExile = new TargetCard(Zone.HAND, StaticFilters.FILTER_CARD_CREATURE);
+                    targetToExile.withChooseHint("card to exile");
                     targetToExile.setNotTarget(true);
                     if (controller.choose(Outcome.Exile, opponent.getHand(), targetToExile, game)) {
                         Card targetedCardToExile = game.getCard(targetToExile.getFirstTarget());
@@ -240,7 +241,8 @@ class ValkiGodOfLiesCopyExiledEffect extends OneShotEffect {
             filter.add(new ConvertedManaCostPredicate(ComparisonType.EQUAL_TO, source.getManaCostsToPay().getX()));
             TargetCardInExile target = new TargetCardInExile(filter, exileId);
             Cards cards = game.getExile().getExileZone(exileId);
-            if (!cards.isEmpty()
+            if (cards != null
+                    && !cards.isEmpty()
                     && controller.choose(Outcome.Benefit, cards, target, game)) {
                 Card chosenExiledCard = game.getCard(target.getFirstTarget());
                 if (chosenExiledCard != null) {

Mage.Sets/src/mage/cards/v/VoidMaw.java

@@ -138,7 +138,8 @@ class VoidMawCost extends CostImpl {
             TargetCardInExile target = new TargetCardInExile(new FilterCard(), CardUtil.getCardExileZoneId(game, ability));
             target.setNotTarget(true);
             Cards cards = game.getExile().getExileZone(CardUtil.getCardExileZoneId(game, ability));
-            if (!cards.isEmpty()
+            if (cards != null
+                    && !cards.isEmpty()
                     && controller.choose(Outcome.Benefit, cards, target, game)) {
                 Card card = game.getCard(target.getFirstTarget());
                 if (card != null) {

Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsTest.java

@@ -10,6 +10,7 @@ import mage.util.CardUtil;
 import mage.util.ManaUtil;
 import org.junit.Assert;
 import org.junit.Test;
+import org.mage.test.player.TestPlayer;
 import org.mage.test.serverside.base.CardTestPlayerBase;
 
 /**
@@ -797,4 +798,69 @@ public class ModalDoubleFacesCardsTest extends CardTestPlayerBase {
         execute();
         assertAllCommandsUsed();
     }
+
+    @Test
+    public void test_Cascade_ValkiGodOfLies() {
+        // https://magic.wizards.com/en/articles/archive/news/february-15-2021-banned-and-restricted-announcement
+        // For example, if you cast Bloodbraid Elf and exile Valki, God of Lies from your library,
+        // you'll be able to cast Valki but not Tibalt, Cosmic Impostor. On the other hand, if you
+        // exile Cosima, God of the Voyage, you may cast either Cosima or The Omenkeel, as each face
+        // has a lesser converted mana cost than Bloodbraid Elf.
+        removeAllCardsFromLibrary(playerA);
+        skipInitShuffling();
+
+        // Cascade
+        addCard(Zone.HAND, playerA, "Bloodbraid Elf"); // {2}{R}{G}
+        addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3);
+        addCard(Zone.BATTLEFIELD, playerA, "Forest", 1);
+        //
+        addCard(Zone.LIBRARY, playerA, "Swamp", 2);
+        addCard(Zone.LIBRARY, playerA, "Valki, God of Lies", 1);
+        addCard(Zone.LIBRARY, playerA, "Island", 2);
+
+        // play elf with cascade
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Bloodbraid Elf");
+        setChoice(playerA, "Yes"); // use free cast
+        //setChoice(playerA, "Cast Valki, God of Lies"); possible bug: you can see two spell abilities to choose, but only one allows here
+        setChoice(playerA, TestPlayer.CHOICE_SKIP); // no choices for valki's etb exile
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertPermanentCount(playerA, "Valki, God of Lies", 1);
+    }
+
+    @Test
+    public void test_Cascade_CosimaGodOfTheVoyage() {
+        // https://magic.wizards.com/en/articles/archive/news/february-15-2021-banned-and-restricted-announcement
+        // For example, if you cast Bloodbraid Elf and exile Valki, God of Lies from your library,
+        // you'll be able to cast Valki but not Tibalt, Cosmic Impostor. On the other hand, if you
+        // exile Cosima, God of the Voyage, you may cast either Cosima or The Omenkeel, as each face
+        // has a lesser converted mana cost than Bloodbraid Elf.
+        removeAllCardsFromLibrary(playerA);
+        skipInitShuffling();
+
+        // Cascade
+        addCard(Zone.HAND, playerA, "Bloodbraid Elf"); // {2}{R}{G}
+        addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3);
+        addCard(Zone.BATTLEFIELD, playerA, "Forest", 1);
+        //
+        addCard(Zone.LIBRARY, playerA, "Swamp", 2);
+        addCard(Zone.LIBRARY, playerA, "Cosima, God of the Voyage", 1);
+        addCard(Zone.LIBRARY, playerA, "Island", 2);
+
+        // play elf with cascade
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Bloodbraid Elf");
+        setChoice(playerA, "Yes"); // use free cast
+        setChoice(playerA, "Cast The Omenkeel"); // can cast any side here
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertPermanentCount(playerA, "The Omenkeel", 1);
+    }
 }

Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java

@@ -3777,6 +3777,21 @@ public class TestPlayer implements Player {
     ) {
         assertAliasSupportInChoices(false);
         if (!choices.isEmpty()) {
+
+            // skip choices
+            if (choices.get(0).equals(CHOICE_SKIP)) {
+                choices.remove(0);
+                if (cards.isEmpty()) {
+                    // cancel button forced in GUI on no possible choices
+                    return false;
+                } else {
+                    Assert.assertTrue("found skip choice, but it require more choices, needs "
+                                    + (target.getMinNumberOfTargets() - target.getTargets().size()) + " more",
+                            target.getTargets().size() >= target.getMinNumberOfTargets());
+                    return true;
+                }
+            }
+
             for (String choose2 : choices) {
                 // TODO: More targetting to fix
                 String[] targetList = choose2.split("\\^");

Mage/src/main/java/mage/abilities/keyword/CascadeAbility.java

@@ -1,6 +1,7 @@
 package mage.abilities.keyword;
 
 import mage.ApprovingObject;
+import mage.MageObject;
 import mage.abilities.Ability;
 import mage.abilities.TriggeredAbilityImpl;
 import mage.abilities.effects.OneShotEffect;
@@ -14,6 +15,10 @@ import mage.game.stack.Spell;
 import mage.players.Player;
 import mage.target.common.TargetCardInExile;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
 /**
  * @author BetaSteward_at_googlemail.com
  */
@@ -91,6 +96,8 @@ class CascadeEffect extends OneShotEffect {
         if (sourceCard == null) {
             return false;
         }
+
+        // exile cards from the top of your library until you exile a nonland card whose converted mana cost is less than this spell's converted mana cost
         Cards cardsToExile = new CardsImpl();
         int sourceCost = sourceCard.getConvertedManaCost();
         Card cardToCast = null;
@@ -101,60 +108,65 @@ class CascadeEffect extends OneShotEffect {
                 break;
             }
         }
-
         controller.moveCards(cardsToExile, Zone.EXILED, source, game);
         controller.getLibrary().reset(); // set back empty draw state if that caused an empty draw
 
+        // additional replacement effect: As you cascade, you may put a land card from among the exiled cards onto the battlefield tapped
         GameEvent event = GameEvent.getEvent(GameEvent.EventType.CASCADE_LAND, source.getSourceId(), source, source.getControllerId(), 0);
         game.replaceEvent(event);
         if (event.getAmount() > 0) {
-            TargetCardInExile target = new TargetCardInExile(
+            TargetCardInExile target = new TargetCardInExile(0, event.getAmount(), StaticFilters.FILTER_CARD_LAND, null, true);
-                    0, event.getAmount(), StaticFilters.FILTER_CARD_LAND, null, true
+            target.withChooseHint("land to put onto battlefield tapped");
-            );
             controller.choose(Outcome.PutCardInPlay, cardsToExile, target, game);
             controller.moveCards(
                     new CardsImpl(target.getTargets()).getCards(game), Zone.BATTLEFIELD,
                     source, game, true, false, false, null
             );
         }
-        if (cardToCast != null && controller.chooseUse(
+
-                outcome, "Use cascade effect on " + cardToCast.getLogName() + '?', source, game
+        // You may cast that spell without paying its mana cost if its converted mana cost is less than this spell's converted mana cost.
-        )) {
+        List<Card> partsToCast = new ArrayList<>();
-            // Check to see if player is allowed to cast the back half
+        if (cardToCast != null) {
-            // Front half is already checked by exile effect
+            if (cardToCast instanceof SplitCard) {
-            if (cardToCast instanceof ModalDoubleFacesCard) {
+                partsToCast.add(((SplitCard) cardToCast).getLeftHalfCard());
-                ModalDoubleFacesCardHalf leftHalf = ((ModalDoubleFacesCard) cardToCast).getLeftHalfCard();
+                partsToCast.add(((SplitCard) cardToCast).getRightHalfCard());
-                ModalDoubleFacesCardHalf rightHalf = ((ModalDoubleFacesCard) cardToCast).getRightHalfCard();
+                partsToCast.add(cardToCast);
-                if (rightHalf.getConvertedManaCost() < sourceCost) {
-                    castForFree(cardToCast, source, game, controller);
-                } else {
-                    castForFree(leftHalf, source, game, controller);
-                }
             } else if (cardToCast instanceof AdventureCard) {
-                Card adventureSpell = ((AdventureCard) cardToCast).getSpellCard();
+                partsToCast.add(((AdventureCard) cardToCast).getSpellCard());
-                if (adventureSpell.getConvertedManaCost() < sourceCost) {
+                partsToCast.add(cardToCast);
-                    castForFree(cardToCast, source, game, controller);
+            } else if (cardToCast instanceof ModalDoubleFacesCard) {
-                } else {
+                partsToCast.add(((ModalDoubleFacesCard) cardToCast).getLeftHalfCard());
-                    game.getState().setValue("PlayFromNotOwnHandZone" + cardToCast.getId(), Boolean.TRUE);
+                partsToCast.add(((ModalDoubleFacesCard) cardToCast).getRightHalfCard());
-                    controller.cast(cardToCast.getSpellAbility(), game, true, new ApprovingObject(source, game));
-                    game.getState().setValue("PlayFromNotOwnHandZone" + cardToCast.getId(), null);
-                }
             } else {
-                castForFree(cardToCast, source, game, controller);
+                partsToCast.add(cardToCast);
+            }
+            // remove too big cmc
+            partsToCast.removeIf(card -> card.getConvertedManaCost() >= sourceCost);
+            // remove non spells
+            partsToCast.removeIf(card -> card.getSpellAbility() == null);
+        }
+
+        String partsInfo = partsToCast.stream()
+                .map(MageObject::getIdName)
+                .collect(Collectors.joining(" or "));
+        if (cardToCast != null
+                && partsToCast.size() > 0
+                && controller.chooseUse(outcome, "Cast spell without paying its mana cost (" + partsInfo + ")?", source, game)) {
+            try {
+                // enable free cast for all compatible parts
+                partsToCast.forEach(card -> game.getState().setValue("PlayFromNotOwnHandZone" + card.getId(), Boolean.TRUE));
+                controller.cast(controller.chooseAbilityForCast(cardToCast, game, true),
+                        game, true, new ApprovingObject(source, game));
+            } finally {
+                partsToCast.forEach(card -> game.getState().setValue("PlayFromNotOwnHandZone" + card.getId(), null));
             }
         }
-        // Move the remaining cards to the buttom of the library in a random order
+
+        // Then put all cards exiled this way that weren't cast on the bottom of your library in a random order.
         cardsToExile.removeIf(uuid -> game.getState().getZone(uuid) != Zone.EXILED);
         return controller.putCardsOnBottomOfLibrary(cardsToExile, game, source, false);
     }
 
-    private void castForFree(Card cardToCast, Ability source, Game game, Player controller) {
-        game.getState().setValue("PlayFromNotOwnHandZone" + cardToCast.getId(), Boolean.TRUE);
-        controller.cast(controller.chooseAbilityForCast(cardToCast, game, true),
-                game, true, new ApprovingObject(source, game));
-        game.getState().setValue("PlayFromNotOwnHandZone" + cardToCast.getId(), null);
-    }
-
     @Override
     public CascadeEffect copy() {
         return new CascadeEffect(this);

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

@@ -365,6 +365,20 @@ public interface Player extends MageItem, Copyable<Player> {
 
     boolean cast(SpellAbility ability, Game game, boolean noMana, ApprovingObject approvingObject);
 
+    /**
+     * Force player to choose spell ability to cast. Use it in effects while casting cards.
+     *
+     * Commands order in all use cases:
+     * - PlayFromNotOwnHandZone - true
+     * - chooseAbilityForCast
+     * - cast
+     * - PlayFromNotOwnHandZone - false
+     *
+     * @param card
+     * @param game
+     * @param noMana
+     * @return
+     */
     SpellAbility chooseAbilityForCast(Card card, Game game, boolean noMana);
 
     boolean putInHand(Card card, Game game);

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

@@ -1519,14 +1519,26 @@ public abstract class PlayerImpl implements Player, Serializable {
         return false;
     }
 
+    /**
+     * Return spells for possible cast
+     * Uses in GUI to show only playable spells for choosing from the card
+     * (example: effect allow to cast card and player must choose the spell ability)
+     *
+     * @param playerId
+     * @param object
+     * @param zone
+     * @param game
+     * @return
+     */
     public static LinkedHashMap<UUID, ActivatedAbility> getSpellAbilities(UUID playerId, MageObject object, Zone zone, Game game) {
+        // it uses simple check from spellCanBeActivatedRegularlyNow
+        // reason: no approved info here (e.g. forced to choose spell ability from cast card)
         LinkedHashMap<UUID, ActivatedAbility> useable = new LinkedHashMap<>();
         for (Ability ability : object.getAbilities()) {
             if (ability instanceof SpellAbility) {
                 switch (((SpellAbility) ability).getSpellAbilityType()) {
                     case BASE_ALTERNATE:
-                        ActivationStatus as = ((SpellAbility) ability).canActivate(playerId, game);
+                        if (((SpellAbility) ability).spellCanBeActivatedRegularlyNow(playerId, game)) {
-                        if (as.canActivate()) {
                             useable.put(ability.getId(), (SpellAbility) ability);  // example: Chandra, Torch of Defiance +1 loyal ability
                         }
                         return useable;
@@ -1560,7 +1572,9 @@ public abstract class PlayerImpl implements Player, Serializable {
                         }
                         return useable;
                     default:
-                        useable.put(ability.getId(), (SpellAbility) ability);
+                        if (((SpellAbility) ability).spellCanBeActivatedRegularlyNow(playerId, game)) {
+                            useable.put(ability.getId(), (SpellAbility) ability);
+                        }
                 }
             }
         }
@@ -2713,7 +2727,9 @@ public abstract class PlayerImpl implements Player, Serializable {
 
             // casting selected card
             // TODO: fix costs (why is Panglacial Wurm automatically accepting payment?)
+            game.getState().setValue("PlayFromNotOwnHandZone" + card.getId(), Boolean.TRUE);
             targetPlayer.cast(targetPlayer.chooseAbilityForCast(card, game, false), game, false, null);
+            game.getState().setValue("PlayFromNotOwnHandZone" + card.getId(), null);
             castableCards.remove(card.getId());
             casted = true;
         }