From 848c5b6052fb67b3403eede427418967b9ec0b02 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Sat, 25 Jul 2020 19:00:59 +0400 Subject: [PATCH] Fixed missing watchers from DelayedTriggeredAbility: * Planeswalkers Mischief - fixed rollback error on play; * Psychic Theft - fixed rollback error on play; --- Mage.Sets/src/mage/cards/c/CyclopeanTomb.java | 2 +- .../mage/cards/p/PlaneswalkersMischief.java | 5 +-- Mage.Sets/src/mage/cards/p/PsychicTheft.java | 4 +- ...hersFromDelayedTriggeredAbilitiesTest.java | 43 +++++++++++++++++++ .../base/impl/CardTestPlayerAPIImpl.java | 21 +++++---- Mage/src/main/java/mage/game/GameState.java | 9 ++++ 6 files changed, 67 insertions(+), 17 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/watchers/WatchersFromDelayedTriggeredAbilitiesTest.java diff --git a/Mage.Sets/src/mage/cards/c/CyclopeanTomb.java b/Mage.Sets/src/mage/cards/c/CyclopeanTomb.java index 95643caea0f..6b637fda4c2 100644 --- a/Mage.Sets/src/mage/cards/c/CyclopeanTomb.java +++ b/Mage.Sets/src/mage/cards/c/CyclopeanTomb.java @@ -120,7 +120,7 @@ class CyclopeanTombCreateTriggeredEffect extends OneShotEffect { DelayedTriggeredAbility ability = new AtTheBeginOfYourNextUpkeepDelayedTriggeredAbility(new CyclopeanTombEffect(), Duration.EndOfGame, false); ability.setControllerId(source.getControllerId()); ability.setSourceId(source.getSourceId()); - game.addDelayedTriggeredAbility(ability); + game.addDelayedTriggeredAbility(ability, source); return true; } return false; diff --git a/Mage.Sets/src/mage/cards/p/PlaneswalkersMischief.java b/Mage.Sets/src/mage/cards/p/PlaneswalkersMischief.java index c546bfda19e..585eac5656e 100644 --- a/Mage.Sets/src/mage/cards/p/PlaneswalkersMischief.java +++ b/Mage.Sets/src/mage/cards/p/PlaneswalkersMischief.java @@ -22,7 +22,6 @@ import mage.watchers.common.SpellsCastWatcher; import java.util.List; import java.util.UUID; -import mage.MageObject; /** * @author jeffwadsworth @@ -76,7 +75,7 @@ class PlaneswalkersMischiefEffect extends OneShotEffect { Cards cards = new CardsImpl(revealedCard); opponent.revealCards(source, cards, game); if (revealedCard.isInstant() - || revealedCard.isSorcery()) { + || revealedCard.isSorcery()) { opponent.moveCardToExileWithInfo(revealedCard, source.getSourceId(), "Planeswalker's Mischief", source.getSourceId(), game, Zone.HAND, true); AsThoughEffect effect = new PlaneswalkersMischiefCastFromExileEffect(); effect.setTargetPointer(new FixedTarget(revealedCard.getId())); @@ -145,7 +144,7 @@ class PlaneswalkersMischiefCondition implements Condition { if (!game.getExile().getExileZone(exileId).contains(cardId)) { return false; } - SpellsCastWatcher watcher = game.getState().getWatcher(SpellsCastWatcher.class, source.getSourceId()); + SpellsCastWatcher watcher = game.getState().getWatcher(SpellsCastWatcher.class); if (watcher != null) { List spells = watcher.getSpellsCastThisTurn(source.getControllerId()); if (spells != null) { diff --git a/Mage.Sets/src/mage/cards/p/PsychicTheft.java b/Mage.Sets/src/mage/cards/p/PsychicTheft.java index 94b14e02fb9..565ecf2dbb7 100644 --- a/Mage.Sets/src/mage/cards/p/PsychicTheft.java +++ b/Mage.Sets/src/mage/cards/p/PsychicTheft.java @@ -80,7 +80,7 @@ class PsychicTheftEffect extends OneShotEffect { Card chosenCard = null; if (cardsHand > 0) { TargetCard target = new TargetCard(Zone.HAND, filter); - if (controller.choose(Outcome.Benefit, opponent.getHand(), target, game)) { + if (controller.choose(Outcome.Exile, opponent.getHand(), target, game)) { chosenCard = opponent.getHand().get(target.getFirstTarget(), game); } } @@ -156,7 +156,7 @@ class PsychicTheftCondition implements Condition { if (!game.getExile().getExileZone(exileId).contains(cardId)) { return false; } - SpellsCastWatcher watcher = game.getState().getWatcher(SpellsCastWatcher.class, source.getSourceId()); + SpellsCastWatcher watcher = game.getState().getWatcher(SpellsCastWatcher.class); if (watcher != null) { List spells = watcher.getSpellsCastThisTurn(source.getControllerId()); if (spells != null) { diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/watchers/WatchersFromDelayedTriggeredAbilitiesTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/watchers/WatchersFromDelayedTriggeredAbilitiesTest.java new file mode 100644 index 00000000000..afb479968d9 --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/watchers/WatchersFromDelayedTriggeredAbilitiesTest.java @@ -0,0 +1,43 @@ +package org.mage.test.cards.watchers; + +import mage.constants.PhaseStep; +import mage.constants.Zone; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +/** + * @author JayDi85 + */ +public class WatchersFromDelayedTriggeredAbilitiesTest extends CardTestPlayerBase { + + @Test + public void test_PsychicTheft_WatcherFromDelayedAbility() { + + // Target player reveals their hand. You choose an instant or sorcery card from it and exile that card. + // You may cast that card for as long as it remains exiled. At the beginning of the next end step, + // if you haven't cast the card, return it to its owner's hand. + addCard(Zone.HAND, playerA, "Psychic Theft", 1); // {1}{U} + addCard(Zone.BATTLEFIELD, playerA, "Island", 2); + addCard(Zone.BATTLEFIELD, playerA, "Mountain", 1); + // + addCard(Zone.HAND, playerB, "Lightning Bolt"); + + // reveal + activateManaAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "{T}: Add {U}", 2); + checkPlayableAbility("can't play bolt", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Cast Lightning Bolt", false); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Psychic Theft", playerB); + setChoice(playerA, "Lightning Bolt"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN); + + // can play until end step + checkPlayableAbility("can play bolt on 1", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Cast Lightning Bolt", true); + checkPlayableAbility("can play bolt on 2", 2, PhaseStep.PRECOMBAT_MAIN, playerA, "Cast Lightning Bolt", false); + + setStrictChooseMode(true); + setStopAt(2, PhaseStep.END_TURN); + execute(); + assertAllCommandsUsed(); + + assertHandCount(playerB, "Lightning Bolt", 1); + } +} diff --git a/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java b/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java index 8133445f799..dd97f07cfb4 100644 --- a/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java +++ b/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java @@ -1,14 +1,5 @@ package org.mage.test.serverside.base.impl; -import java.io.FileNotFoundException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.UUID; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import java.util.stream.Collectors; import mage.MageObject; import mage.Mana; import mage.ObjectColor; @@ -44,6 +35,12 @@ import org.mage.test.player.TestPlayer; import org.mage.test.serverside.base.CardTestAPI; import org.mage.test.serverside.base.MageTestPlayerBase; +import java.io.FileNotFoundException; +import java.util.*; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + /** * API for test initialization and asserting the test results. * @@ -202,7 +199,6 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement } /** - * * @throws GameException * @throws FileNotFoundException */ @@ -291,7 +287,7 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement } } Assert.assertFalse("Wrong stop command on " + this.stopOnTurn + " / " + this.stopAtStep + " (" + this.stopAtStep.getIndex() + ")" - + " (found actions after stop on " + maxTurn + " / " + maxPhase + ")", + + " (found actions after stop on " + maxTurn + " / " + maxPhase + ")", (maxTurn > this.stopOnTurn) || (maxTurn == this.stopOnTurn && maxPhase > this.stopAtStep.getIndex())); for (Player player : currentGame.getPlayers().values()) { @@ -533,6 +529,9 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement * Removes all cards from player's library from the game. Usually this * should be used once before initialization to form the library in certain * order. + *

+ * Warning, if you doesn't add cards to hand then player will lose the game on draw and test + * return unused actions (game ended too early) * * @param player {@link Player} to remove all library cards from. */ diff --git a/Mage/src/main/java/mage/game/GameState.java b/Mage/src/main/java/mage/game/GameState.java index 5f2259b11db..7ab0ae20768 100644 --- a/Mage/src/main/java/mage/game/GameState.java +++ b/Mage/src/main/java/mage/game/GameState.java @@ -890,6 +890,7 @@ public class GameState implements Serializable, Copyable { // TODO: add sources for triggers - the same way as in addEffect: sources this.triggers.add((TriggeredAbility) ability, sourceId, attachedTo); } + List watcherList = new ArrayList<>(ability.getWatchers()); // Workaround to prevent ConcurrentModificationException, not clear to me why this is happening now for (Watcher watcher : watcherList) { // TODO: Check that watcher for commanderAbility (where attachedTo = null) also work correctly @@ -897,6 +898,7 @@ public class GameState implements Serializable, Copyable { watcher.setSourceId(attachedTo == null ? ability.getSourceId() : attachedTo.getId()); watchers.add(watcher); } + for (Ability sub : ability.getSubAbilities()) { addAbility(sub, sourceId, attachedTo); } @@ -949,6 +951,13 @@ public class GameState implements Serializable, Copyable { public void addDelayedTriggeredAbility(DelayedTriggeredAbility ability) { this.delayed.add(ability); + + List watcherList = new ArrayList<>(ability.getWatchers()); // Workaround to prevent ConcurrentModificationException, not clear to me why this is happening now + for (Watcher watcher : watcherList) { + watcher.setControllerId(ability.getControllerId()); + watcher.setSourceId(ability.getSourceId()); + this.watchers.add(watcher); + } } public void removeDelayedTriggeredAbility(UUID abilityId) {