From bb148c9cb5ad6c716de4e958b7f07b591d987405 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Tue, 4 Aug 2020 22:09:57 +0400 Subject: [PATCH] * Special mana abilities: fixed that it can be used to skip card's mana payments (#6937); --- .../cards/abilities/keywords/DelveTest.java | 23 ++--- .../java/org/mage/test/player/TestPlayer.java | 62 +++++++------- .../main/java/mage/players/PlayerImpl.java | 83 ++++++++++--------- 3 files changed, 85 insertions(+), 83 deletions(-) diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/DelveTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/DelveTest.java index 5c2c4cb7ca4..854f9c9ada6 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/DelveTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/DelveTest.java @@ -2,7 +2,6 @@ package org.mage.test.cards.abilities.keywords; import mage.constants.PhaseStep; import mage.constants.Zone; -import org.junit.Ignore; import org.junit.Test; import org.mage.test.player.TestPlayer; import org.mage.test.serverside.base.CardTestPlayerBaseWithAIHelps; @@ -85,7 +84,6 @@ public class DelveTest extends CardTestPlayerBaseWithAIHelps { } @Test - @Ignore public void test_CheatWithCancel() { // possible bug: users can start to pay delve special action with nothing (done button) and gets no mana cast // https://github.com/magefree/mage/issues/6937 @@ -99,7 +97,7 @@ public class DelveTest extends CardTestPlayerBaseWithAIHelps { addCard(Zone.BATTLEFIELD, playerA, "Island", 1); addCard(Zone.GRAVEYARD, playerA, "Balduvian Bears", 7); // delve pay - // user case: + // use case: // 1. Use mana from land (fill mana pool) // 2. Use delve as special action // 3. Press done without real delve pay @@ -107,20 +105,17 @@ public class DelveTest extends CardTestPlayerBaseWithAIHelps { activateManaAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "{T}: Add {U}"); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Treasure Cruise"); - //setChoice(playerA, "Blue"); - setChoice(playerA, "Exile cards"); // delve activate - setChoice(playerA, TestPlayer.CHOICE_SKIP); - setChoice(playerA, TestPlayer.MANA_CANCEL); + setChoice(playerA, "Exile cards"); // delve activate (special button in UI) + setChoice(playerA, TestPlayer.CHOICE_SKIP); // devle cost with nothing (done button in UI) + setChoice(playerA, TestPlayer.MANA_CANCEL); // mana payment cancel (cancel button in UI) + setChoice(playerA, TestPlayer.SKIP_FAILED_COMMAND); // delete cast/activate command from queue - waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN); - showGraveyard("hmm", 1, PhaseStep.PRECOMBAT_MAIN, playerA); - - // it uses bug with rollback, so test commands restores with rollback too (no strict or commands usage check) - //setStrictChooseMode(true); + // it uses bug with rollback, so test player will execute it multiple times + setStrictChooseMode(true); setStopAt(1, PhaseStep.END_TURN); execute(); - //assertAllCommandsUsed(); + assertAllCommandsUsed(); - assertHandCount(playerA, 0); // no resolve, so no draw cards (if rollback bug active then it shows 3 cards) + assertHandCount(playerA, 1); // no resolve, so no draw cards (if rollback bug active then it shows 3 cards) } } \ 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 3fecad2d7f8..5bf77cb2500 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 @@ -1,15 +1,6 @@ package org.mage.test.player; -import java.io.Serializable; -import java.util.*; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import java.util.stream.Collectors; -import mage.MageItem; -import mage.MageObject; -import mage.MageObjectReference; -import mage.Mana; -import mage.ObjectColor; +import mage.*; import mage.abilities.*; import mage.abilities.costs.AlternativeSourceCosts; import mage.abilities.costs.Cost; @@ -63,6 +54,13 @@ import mage.util.CardUtil; import org.apache.log4j.Logger; import org.junit.Assert; import org.junit.Ignore; + +import java.io.Serializable; +import java.util.*; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + import static org.mage.test.serverside.base.impl.CardTestPlayerAPIImpl.*; /** @@ -78,6 +76,7 @@ public class TestPlayer implements Player { public static final String TARGET_SKIP = "[target_skip]"; // stop/skip targeting public static final String CHOICE_SKIP = "[choice_skip]"; // stop/skip choice public static final String MANA_CANCEL = "[mana_cancel]"; // cancel payment + public static final String SKIP_FAILED_COMMAND = "[skip_failed_command]"; // skip next command in player's queue (can remove cast commands after try to activate) public static final String BLOCK_SKIP = "[block_skip]"; public static final String ATTACK_SKIP = "[attack_skip]"; public static final String NO_TARGET = "NO_TARGET"; // cast spell or activate ability without target defines @@ -592,7 +591,14 @@ public class TestPlayer implements Player { foundNoAction = 0; // Reset enless loop check because of no action return true; } else { - game.restoreState(bookmark, ability.getRule()); + computerPlayer.restoreState(bookmark, ability.getRule(), game); + + // skip failed command + if (!choices.isEmpty() && choices.get(0).equals(SKIP_FAILED_COMMAND)) { + actions.remove(action); + choices.remove(0); + return true; + } } groupsForTargetHandling = null; } @@ -1112,13 +1118,13 @@ public class TestPlayer implements Player { List data = cards.stream() .map(c -> (((c instanceof PermanentToken) ? "[T] " : "[C] ") - + c.getIdName() - + (c.isCopy() ? " [copy of " + c.getCopyFrom().getId().toString().substring(0, 3) + "]" : "") - + " - " + c.getPower().getValue() + "/" + c.getToughness().getValue() - + (c.isPlaneswalker() ? " - L" + c.getCounters(game).getCount(CounterType.LOYALTY) : "") - + ", " + (c.isTapped() ? "Tapped" : "Untapped") - + getPrintableAliases(", [", c.getId(), "]") - + (c.getAttachedTo() == null ? "" : ", attached to " + game.getPermanent(c.getAttachedTo()).getIdName()))) + + c.getIdName() + + (c.isCopy() ? " [copy of " + c.getCopyFrom().getId().toString().substring(0, 3) + "]" : "") + + " - " + c.getPower().getValue() + "/" + c.getToughness().getValue() + + (c.isPlaneswalker() ? " - L" + c.getCounters(game).getCount(CounterType.LOYALTY) : "") + + ", " + (c.isTapped() ? "Tapped" : "Untapped") + + getPrintableAliases(", [", c.getId(), "]") + + (c.getAttachedTo() == null ? "" : ", attached to " + game.getPermanent(c.getAttachedTo()).getIdName()))) .sorted() .collect(Collectors.toList()); @@ -1142,12 +1148,12 @@ public class TestPlayer implements Player { List data = abilities.stream() .map(a -> (a.getZone() + " -> " - + a.getSourceObject(game).getIdName() + " -> " - + (a.toString().startsWith("Cast ") ? "[" + a.getManaCostsToPay().getText() + "] -> " : "") // printed cost, not modified - + (a.toString().length() > 0 - ? a.toString().substring(0, Math.min(20, a.toString().length())) - : a.getClass().getSimpleName()) - + "...")) + + a.getSourceObject(game).getIdName() + " -> " + + (a.toString().startsWith("Cast ") ? "[" + a.getManaCostsToPay().getText() + "] -> " : "") // printed cost, not modified + + (a.toString().length() > 0 + ? a.toString().substring(0, Math.min(20, a.toString().length())) + : a.getClass().getSimpleName()) + + "...")) .sorted() .collect(Collectors.toList()); @@ -1562,7 +1568,7 @@ public class TestPlayer implements Player { UUID defenderId = null; boolean mustAttackByAction = false; boolean madeAttackByAction = false; - for (Iterator it = actions.iterator(); it.hasNext();) { + for (Iterator it = actions.iterator(); it.hasNext(); ) { PlayerAction action = it.next(); // aiXXX commands @@ -2153,7 +2159,7 @@ public class TestPlayer implements Player { // skip targets if (targets.get(0).equals(TARGET_SKIP)) { Assert.assertTrue("found skip target, but it require more targets, needs " - + (target.getMinNumberOfTargets() - target.getTargets().size()) + " more", + + (target.getMinNumberOfTargets() - target.getTargets().size()) + " more", target.getTargets().size() >= target.getMinNumberOfTargets()); targets.remove(0); return true; @@ -2464,7 +2470,7 @@ public class TestPlayer implements Player { this.chooseStrictModeFailed("choice", game, "Triggered list (total " + abilities.size() + "):\n" - + abilities.stream().map(a -> getInfo(a, game)).collect(Collectors.joining("\n"))); + + abilities.stream().map(a -> getInfo(a, game)).collect(Collectors.joining("\n"))); return computerPlayer.chooseTriggeredAbility(abilities, game); } @@ -3713,7 +3719,7 @@ public class TestPlayer implements Player { // skip targets if (targets.get(0).equals(TARGET_SKIP)) { Assert.assertTrue("found skip target, but it require more targets, needs " - + (target.getMinNumberOfTargets() - target.getTargets().size()) + " more", + + (target.getMinNumberOfTargets() - target.getTargets().size()) + " more", target.getTargets().size() >= target.getMinNumberOfTargets()); targets.remove(0); return false; // false in chooseTargetAmount = stop to choose diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index fe2d86a7eca..4e3643af6ff 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -1,10 +1,6 @@ package mage.players; import com.google.common.collect.ImmutableMap; -import java.io.Serializable; -import java.util.*; -import java.util.Map.Entry; -import java.util.stream.Collectors; import mage.ConditionalMana; import mage.MageObject; import mage.MageObjectReference; @@ -70,6 +66,11 @@ import mage.util.GameLog; import mage.util.RandomUtil; import org.apache.log4j.Logger; +import java.io.Serializable; +import java.util.*; +import java.util.Map.Entry; +import java.util.stream.Collectors; + public abstract class PlayerImpl implements Player, Serializable { private static final Logger logger = Logger.getLogger(PlayerImpl.class); @@ -613,9 +614,9 @@ public abstract class PlayerImpl implements Player, Serializable { && this.hasOpponent(sourceControllerId, game) && game.getContinuousEffects().asThough(this.getId(), AsThoughEffectType.HEXPROOF, null, sourceControllerId, game) == null && abilities.stream() - .filter(HexproofBaseAbility.class::isInstance) - .map(HexproofBaseAbility.class::cast) - .anyMatch(ability -> ability.checkObject(source, game))) { + .filter(HexproofBaseAbility.class::isInstance) + .map(HexproofBaseAbility.class::cast) + .anyMatch(ability -> ability.checkObject(source, game))) { return false; } @@ -655,7 +656,7 @@ public abstract class PlayerImpl implements Player, Serializable { game.informPlayers(getLogName() + " discards down to " + this.maxHandSize + (this.maxHandSize == 1 - ? " hand card" : " hand cards")); + ? " hand card" : " hand cards")); } discard(hand.size() - this.maxHandSize, false, null, game); } @@ -804,7 +805,7 @@ public abstract class PlayerImpl implements Player, Serializable { } GameEvent gameEvent = GameEvent.getEvent(GameEvent.EventType.DISCARD_CARD, card.getId(), source == null - ? null : source.getSourceId(), playerId); + ? null : source.getSourceId(), playerId); gameEvent.setFlag(source != null); // event from effect or from cost (source == null) if (game.replaceEvent(gameEvent, source)) { return false; @@ -1149,7 +1150,6 @@ public abstract class PlayerImpl implements Player, Serializable { } /** - * * @param originalAbility * @param game * @param noMana cast it without paying mana costs @@ -1177,6 +1177,7 @@ public abstract class PlayerImpl implements Player, Serializable { if (!game.replaceEvent(GameEvent.getEvent(GameEvent.EventType.CAST_SPELL, ability.getId(), ability.getSourceId(), playerId, permittingObject), ability)) { int bookmark = game.bookmarkState(); + setStoredBookmark(bookmark); // move global bookmark to current state (if you activated mana before then you can't rollback it) Zone fromZone = game.getState().getZone(card.getMainCard().getId()); card.cast(game, fromZone, ability, playerId); Spell spell = game.getStack().getSpell(ability.getId()); @@ -1383,7 +1384,7 @@ public abstract class PlayerImpl implements Player, Serializable { return false; } - protected void restoreState(int bookmark, String text, Game game) { + public void restoreState(int bookmark, String text, Game game) { game.restoreState(bookmark, text); if (storedBookmark >= bookmark) { resetStoredBookmark(game); @@ -1859,9 +1860,9 @@ public abstract class PlayerImpl implements Player, Serializable { } private List getPermanentsThatCanBeUntapped(Game game, - List canBeUntapped, - RestrictionUntapNotMoreThanEffect handledEffect, - Map>, Integer> notMoreThanEffectsUsage) { + List canBeUntapped, + RestrictionUntapNotMoreThanEffect handledEffect, + Map>, Integer> notMoreThanEffectsUsage) { List leftForUntap = new ArrayList<>(); // select permanents that can still be untapped for (Permanent permanent : canBeUntapped) { @@ -2574,7 +2575,7 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean searchLibrary(TargetCardInLibrary target, Ability source, Game game, UUID targetPlayerId, - boolean triggerEvents) { + boolean triggerEvents) { //20091005 - 701.14c Library searchedLibrary = null; String searchInfo = null; @@ -2724,7 +2725,7 @@ public abstract class PlayerImpl implements Player, Serializable { * @param winnable * @param appliedEffects * @return if winnable, true if player won the toss, if not winnable, true - * for heads and false for tails + * for heads and false for tails */ @Override public boolean flipCoin(Ability source, Game game, boolean winnable, List appliedEffects) { @@ -2818,11 +2819,11 @@ public abstract class PlayerImpl implements Player, Serializable { * @param numberPlanarSides The number of chaos sides the planar die * currently has (normally 1) * @return the outcome that the player rolled. Either ChaosRoll, PlanarRoll - * or NilRoll + * or NilRoll */ @Override public PlanarDieRoll rollPlanarDie(Game game, List appliedEffects, int numberChaosSides, - int numberPlanarSides) { + int numberPlanarSides) { int result = RandomUtil.nextInt(9) + 1; PlanarDieRoll roll = PlanarDieRoll.NIL_ROLL; if (numberChaosSides + numberPlanarSides > 9) { @@ -2899,7 +2900,7 @@ public abstract class PlayerImpl implements Player, Serializable { boolean withCost = false; Abilities manaAbilities = permanent.getAbilities().getAvailableActivatedManaAbilities(Zone.BATTLEFIELD, game); - for (Iterator it = manaAbilities.iterator(); it.hasNext();) { + for (Iterator it = manaAbilities.iterator(); it.hasNext(); ) { ActivatedManaAbilityImpl ability = it.next(); if (canUse == null) { canUse = permanent.canUseActivatedAbilities(game); @@ -2940,7 +2941,7 @@ public abstract class PlayerImpl implements Player, Serializable { boolean anAbilityWasUsed = true; while (anAbilityWasUsed && !sourceWithCosts.isEmpty()) { anAbilityWasUsed = false; - for (Iterator> iterator = sourceWithCosts.iterator(); iterator.hasNext();) { + for (Iterator> iterator = sourceWithCosts.iterator(); iterator.hasNext(); ) { Abilities manaAbilities = iterator.next(); boolean used = availableMana.addManaWithCost(manaAbilities, game); if (used) { @@ -3624,7 +3625,7 @@ public abstract class PlayerImpl implements Player, Serializable { * * @param game * @return A Set of cardIds that are playable and amount of playable - * abilities + * abilities */ @Override public Map getPlayableObjects(Game game, Zone zone) { @@ -3832,7 +3833,7 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean canPaySacrificeCost(Permanent permanent, UUID sourceId, - UUID controllerId, Game game + UUID controllerId, Game game ) { return sacrificeCostFilter == null || !sacrificeCostFilter.match(permanent, sourceId, controllerId, game); } @@ -3985,8 +3986,8 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean moveCards(Card card, Zone toZone, - Ability source, Game game, - boolean tapped, boolean faceDown, boolean byOwner, List appliedEffects + Ability source, Game game, + boolean tapped, boolean faceDown, boolean byOwner, List appliedEffects ) { Set cardList = new HashSet<>(); if (card != null) { @@ -3997,22 +3998,22 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean moveCards(Cards cards, Zone toZone, - Ability source, Game game + Ability source, Game game ) { return moveCards(cards.getCards(game), toZone, source, game); } @Override public boolean moveCards(Set cards, Zone toZone, - Ability source, Game game + Ability source, Game game ) { return moveCards(cards, toZone, source, game, false, false, false, null); } @Override public boolean moveCards(Set cards, Zone toZone, - Ability source, Game game, - boolean tapped, boolean faceDown, boolean byOwner, List appliedEffects + Ability source, Game game, + boolean tapped, boolean faceDown, boolean byOwner, List appliedEffects ) { if (cards.isEmpty()) { return true; @@ -4114,8 +4115,8 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean moveCardsToExile(Card card, Ability source, - Game game, boolean withName, UUID exileId, - String exileZoneName + Game game, boolean withName, UUID exileId, + String exileZoneName ) { Set cards = new HashSet<>(); cards.add(card); @@ -4124,8 +4125,8 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean moveCardsToExile(Set cards, Ability source, - Game game, boolean withName, UUID exileId, - String exileZoneName + Game game, boolean withName, UUID exileId, + String exileZoneName ) { if (cards.isEmpty()) { return true; @@ -4141,14 +4142,14 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean moveCardToHandWithInfo(Card card, UUID sourceId, - Game game + Game game ) { return this.moveCardToHandWithInfo(card, sourceId, game, true); } @Override public boolean moveCardToHandWithInfo(Card card, UUID sourceId, - Game game, boolean withName + Game game, boolean withName ) { boolean result = false; Zone fromZone = game.getState().getZone(card.getId()); @@ -4173,7 +4174,7 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public Set moveCardsToGraveyardWithInfo(Set allCards, Ability source, - Game game, Zone fromZone + Game game, Zone fromZone ) { UUID sourceId = source == null ? null : source.getSourceId(); Set movedCards = new LinkedHashSet<>(); @@ -4181,7 +4182,7 @@ public abstract class PlayerImpl implements Player, Serializable { // identify cards from one owner Cards cards = new CardsImpl(); UUID ownerId = null; - for (Iterator it = allCards.iterator(); it.hasNext();) { + for (Iterator it = allCards.iterator(); it.hasNext(); ) { Card card = it.next(); if (cards.isEmpty()) { ownerId = card.getOwnerId(); @@ -4244,7 +4245,7 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean moveCardToGraveyardWithInfo(Card card, UUID sourceId, - Game game, Zone fromZone + Game game, Zone fromZone ) { if (card == null) { return false; @@ -4273,8 +4274,8 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean moveCardToLibraryWithInfo(Card card, UUID sourceId, - Game game, Zone fromZone, - boolean toTop, boolean withName + Game game, Zone fromZone, + boolean toTop, boolean withName ) { if (card == null) { return false; @@ -4339,7 +4340,7 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean moveCardToExileWithInfo(Card card, UUID exileId, String exileName, UUID sourceId, - Game game, Zone fromZone, boolean withName) { + Game game, Zone fromZone, boolean withName) { if (card == null) { return false; } @@ -4363,7 +4364,7 @@ public abstract class PlayerImpl implements Player, Serializable { game.informPlayers(this.getLogName() + " moves " + (withName ? card.getLogName() + (card.isCopy() ? " (Copy)" : "") : "a card face down") + ' ' + (fromZone != null ? "from " + fromZone.toString().toLowerCase(Locale.ENGLISH) - + ' ' : "") + "to the exile zone"); + + ' ' : "") + "to the exile zone"); } }