From 91f4d7899269e7811da668da123e0d37dc5e6769 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Tue, 23 Feb 2021 02:00:38 +0400 Subject: [PATCH] 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; --- .../java/mage/player/ai/ComputerPlayer.java | 6 ++ .../src/mage/cards/g/GodEternalKefnet.java | 3 +- .../src/mage/cards/j/JestersScepter.java | 3 +- .../src/mage/cards/k/KahoMinamoHistorian.java | 3 +- Mage.Sets/src/mage/cards/m/MesmericFiend.java | 2 +- .../src/mage/cards/v/ValkiGodOfLies.java | 4 +- Mage.Sets/src/mage/cards/v/VoidMaw.java | 3 +- .../ModalDoubleFacesCardsTest.java | 66 +++++++++++++++ .../java/org/mage/test/player/TestPlayer.java | 15 ++++ .../abilities/keyword/CascadeAbility.java | 80 +++++++++++-------- Mage/src/main/java/mage/players/Player.java | 14 ++++ .../main/java/mage/players/PlayerImpl.java | 22 ++++- 12 files changed, 178 insertions(+), 43 deletions(-) diff --git a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java index 239711dea2..7101c48e3d 100644 --- a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java +++ b/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 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) { diff --git a/Mage.Sets/src/mage/cards/g/GodEternalKefnet.java b/Mage.Sets/src/mage/cards/g/GodEternalKefnet.java index a4fc79e72e..70e76107be 100644 --- a/Mage.Sets/src/mage/cards/g/GodEternalKefnet.java +++ b/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) diff --git a/Mage.Sets/src/mage/cards/j/JestersScepter.java b/Mage.Sets/src/mage/cards/j/JestersScepter.java index 75eff4eff9..32d8e74781 100644 --- a/Mage.Sets/src/mage/cards/j/JestersScepter.java +++ b/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) { diff --git a/Mage.Sets/src/mage/cards/k/KahoMinamoHistorian.java b/Mage.Sets/src/mage/cards/k/KahoMinamoHistorian.java index 4f81540362..a98ccd6d89 100644 --- a/Mage.Sets/src/mage/cards/k/KahoMinamoHistorian.java +++ b/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) { diff --git a/Mage.Sets/src/mage/cards/m/MesmericFiend.java b/Mage.Sets/src/mage/cards/m/MesmericFiend.java index 3f4a266f13..1373a9378d 100644 --- a/Mage.Sets/src/mage/cards/m/MesmericFiend.java +++ b/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); } } diff --git a/Mage.Sets/src/mage/cards/v/ValkiGodOfLies.java b/Mage.Sets/src/mage/cards/v/ValkiGodOfLies.java index f9f6c11ba0..aba7fb5329 100644 --- a/Mage.Sets/src/mage/cards/v/ValkiGodOfLies.java +++ b/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) { diff --git a/Mage.Sets/src/mage/cards/v/VoidMaw.java b/Mage.Sets/src/mage/cards/v/VoidMaw.java index 8091fd93f3..7b75a7e9d3 100644 --- a/Mage.Sets/src/mage/cards/v/VoidMaw.java +++ b/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) { diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsTest.java index 9aaa2e77fd..989d2d2ed8 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsTest.java +++ b/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); + } } \ No newline at end of file diff --git a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java index 02e48baf35..0e5da784a0 100644 --- a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java +++ b/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("\\^"); diff --git a/Mage/src/main/java/mage/abilities/keyword/CascadeAbility.java b/Mage/src/main/java/mage/abilities/keyword/CascadeAbility.java index 07c913a45d..98cd411a3a 100644 --- a/Mage/src/main/java/mage/abilities/keyword/CascadeAbility.java +++ b/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( - 0, event.getAmount(), StaticFilters.FILTER_CARD_LAND, null, true - ); + TargetCardInExile target = new TargetCardInExile(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 - )) { - // Check to see if player is allowed to cast the back half - // Front half is already checked by exile effect - if (cardToCast instanceof ModalDoubleFacesCard) { - ModalDoubleFacesCardHalf leftHalf = ((ModalDoubleFacesCard) cardToCast).getLeftHalfCard(); - ModalDoubleFacesCardHalf rightHalf = ((ModalDoubleFacesCard) cardToCast).getRightHalfCard(); - if (rightHalf.getConvertedManaCost() < sourceCost) { - castForFree(cardToCast, source, game, controller); - } else { - castForFree(leftHalf, source, game, controller); - } + + // 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 partsToCast = new ArrayList<>(); + if (cardToCast != null) { + if (cardToCast instanceof SplitCard) { + partsToCast.add(((SplitCard) cardToCast).getLeftHalfCard()); + partsToCast.add(((SplitCard) cardToCast).getRightHalfCard()); + partsToCast.add(cardToCast); } else if (cardToCast instanceof AdventureCard) { - Card adventureSpell = ((AdventureCard) cardToCast).getSpellCard(); - if (adventureSpell.getConvertedManaCost() < sourceCost) { - castForFree(cardToCast, source, game, controller); - } else { - game.getState().setValue("PlayFromNotOwnHandZone" + cardToCast.getId(), Boolean.TRUE); - controller.cast(cardToCast.getSpellAbility(), game, true, new ApprovingObject(source, game)); - game.getState().setValue("PlayFromNotOwnHandZone" + cardToCast.getId(), null); - } + partsToCast.add(((AdventureCard) cardToCast).getSpellCard()); + partsToCast.add(cardToCast); + } else if (cardToCast instanceof ModalDoubleFacesCard) { + partsToCast.add(((ModalDoubleFacesCard) cardToCast).getLeftHalfCard()); + partsToCast.add(((ModalDoubleFacesCard) cardToCast).getRightHalfCard()); } 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); diff --git a/Mage/src/main/java/mage/players/Player.java b/Mage/src/main/java/mage/players/Player.java index f9723f918d..09c491337c 100644 --- a/Mage/src/main/java/mage/players/Player.java +++ b/Mage/src/main/java/mage/players/Player.java @@ -365,6 +365,20 @@ public interface Player extends MageItem, Copyable { 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); diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index 771e1b8bad..6c6270ebc5 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/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 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 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 (as.canActivate()) { + if (((SpellAbility) ability).spellCanBeActivatedRegularlyNow(playerId, game)) { 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; }