From 95b782d519de71dfb4039018b98f53722dad3539 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Sat, 25 Sep 2021 18:09:21 +0400 Subject: [PATCH] Tests for #5630 --- Mage.Sets/src/mage/cards/p/PeaceTalks.java | 5 +- .../abilities/keywords/HexproofTest.java | 116 +++++++++++++++++- .../java/mage/game/events/TargetEvent.java | 6 + .../mage/game/permanent/PermanentImpl.java | 14 ++- .../main/java/mage/players/PlayerImpl.java | 91 +++++++------- 5 files changed, 180 insertions(+), 52 deletions(-) diff --git a/Mage.Sets/src/mage/cards/p/PeaceTalks.java b/Mage.Sets/src/mage/cards/p/PeaceTalks.java index 2e4785aaf3..b0b9785f8b 100644 --- a/Mage.Sets/src/mage/cards/p/PeaceTalks.java +++ b/Mage.Sets/src/mage/cards/p/PeaceTalks.java @@ -24,11 +24,8 @@ public final class PeaceTalks extends CardImpl { public PeaceTalks(UUID ownerId, CardSetInfo setInfo) { super(ownerId, setInfo, new CardType[]{CardType.SORCERY}, "{1}{W}"); - // This turn and next turn, creatures can't attack, - // and players and permanents can't be the targets - // of spells or activated abilities. + // This turn and next turn, creatures can't attack, and players and permanents can't be the targets of spells or activated abilities. this.getSpellAbility().addEffect(new PeaceTalksEffect()); - } private PeaceTalks(final PeaceTalks card) { diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/HexproofTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/HexproofTest.java index fafc7c99fd..2f78ce079f 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/HexproofTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/HexproofTest.java @@ -3,13 +3,14 @@ package org.mage.test.cards.abilities.keywords; import mage.abilities.keyword.HexproofAbility; import mage.constants.PhaseStep; import mage.constants.Zone; +import org.junit.Assert; import org.junit.Test; -import org.mage.test.serverside.base.CardTestPlayerBase; +import org.mage.test.serverside.base.CardTestPlayerBaseWithAIHelps; /** - * @author LevelX2 + * @author LevelX2, JayDi85 */ -public class HexproofTest extends CardTestPlayerBase { +public class HexproofTest extends CardTestPlayerBaseWithAIHelps { /** * Tests one target gets hexproof @@ -103,4 +104,113 @@ public class HexproofTest extends CardTestPlayerBase { assertPermanentCount(playerA, "Knight of Grace", 0); } + + @Test + public void test_Human_CanTargetValid() { + // +1: Target player discards a card. + addCard(Zone.BATTLEFIELD, playerA, "Liliana Vess", 1); + addCard(Zone.HAND, playerA, "Balduvian Bears", 1); + addCard(Zone.HAND, playerA, "Swamp", 1); + addCard(Zone.HAND, playerB, "Matter Reshaper", 1); + addCard(Zone.HAND, playerB, "Mountain", 1); + // + // You have hexproof. (You can't be the target of spells or abilities your opponents control.) + addCard(Zone.BATTLEFIELD, playerB, "Leyline of Sanctity", 1); + + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "+1:"); + addTarget(playerA, playerA); + setChoice(playerA, "Swamp"); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + assertAllCommandsUsed(); + + assertGraveyardCount(playerA, "Swamp", 1); + } + + @Test + public void test_Human_CantTargetInvalid() { + // +1: Target player discards a card. + addCard(Zone.BATTLEFIELD, playerA, "Liliana Vess", 1); + addCard(Zone.HAND, playerA, "Balduvian Bears", 1); + addCard(Zone.HAND, playerA, "Swamp", 1); + addCard(Zone.HAND, playerB, "Matter Reshaper", 1); + addCard(Zone.HAND, playerB, "Mountain", 1); + // + // You have hexproof. (You can't be the target of spells or abilities your opponents control.) + addCard(Zone.BATTLEFIELD, playerB, "Leyline of Sanctity", 1); + + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "+1:"); + addTarget(playerA, playerB); + + try { + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + assertAllCommandsUsed(); + Assert.fail("must throw exception on execute"); + } catch (Throwable e) { + if (!e.getMessage().contains("setup good targets")) { + Assert.fail("must thow error about bad targets, but got:\n" + e.getMessage()); + } + } + } + + @Test + public void test_AI_MustTargetOnlyValid() { + // +1: Target player discards a card. + addCard(Zone.BATTLEFIELD, playerA, "Liliana Vess", 1); + addCard(Zone.HAND, playerA, "Balduvian Bears", 1); + addCard(Zone.HAND, playerA, "Swamp", 1); + addCard(Zone.HAND, playerB, "Matter Reshaper", 1); + addCard(Zone.HAND, playerB, "Mountain", 1); + // + // You have hexproof. (You can't be the target of spells or abilities your opponents control.) + addCard(Zone.BATTLEFIELD, playerB, "Leyline of Sanctity", 1); + + // ai must not use +1 on itself (due bad score) and must not use on opponent (due hexproof) + aiPlayStep(1, PhaseStep.PRECOMBAT_MAIN, playerA); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + assertAllCommandsUsed(); + + // no discarded cards + assertGraveyardCount(playerA, 0); + assertGraveyardCount(playerB, 0); + } + + @Test + public void test_RulesModificationForPlayers() { + // This turn and next turn, creatures can't attack, and players and permanents can't be the targets + // of spells or activated abilities. + addCard(Zone.HAND, playerA, "Peace Talks", 1); // {1}{W} + addCard(Zone.BATTLEFIELD, playerA, "Plains", 2); + // + addCard(Zone.HAND, playerA, "Lightning Bolt"); + addCard(Zone.BATTLEFIELD, playerA, "Mountain", 1); + + checkPlayableAbility("before", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Cast Lightning Bolt", true); + + // activate restriction + activateManaAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "{T}: Add {W}", 2); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Peace Talks"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN); + + // playable doesn't check illegal targets, so it will be active + // ai can cast on turn 3 only + aiPlayStep(1, PhaseStep.PRECOMBAT_MAIN, playerA); + checkLife("after 1", 1, PhaseStep.POSTCOMBAT_MAIN, playerB, 20); + aiPlayStep(2, PhaseStep.PRECOMBAT_MAIN, playerA); + checkLife("after 2", 2, PhaseStep.POSTCOMBAT_MAIN, playerB, 20); + aiPlayStep(3, PhaseStep.PRECOMBAT_MAIN, playerA); + checkLife("after 3", 3, PhaseStep.POSTCOMBAT_MAIN, playerB, 20 - 3); + + setStrictChooseMode(true); + setStopAt(3, PhaseStep.END_TURN); + execute(); + assertAllCommandsUsed(); + } } diff --git a/Mage/src/main/java/mage/game/events/TargetEvent.java b/Mage/src/main/java/mage/game/events/TargetEvent.java index 063d16ac2a..f2c90ebc87 100644 --- a/Mage/src/main/java/mage/game/events/TargetEvent.java +++ b/Mage/src/main/java/mage/game/events/TargetEvent.java @@ -2,6 +2,7 @@ package mage.game.events; import mage.abilities.Ability; import mage.cards.Card; +import mage.players.Player; import java.util.UUID; @@ -20,6 +21,11 @@ public class TargetEvent extends GameEvent { this.setSourceId(sourceId); } + public TargetEvent(Player target, UUID sourceId, UUID sourceControllerId) { + super(GameEvent.EventType.TARGET, target.getId(), null, sourceControllerId); + this.setSourceId(sourceId); + } + /** * @param targetId * @param source diff --git a/Mage/src/main/java/mage/game/permanent/PermanentImpl.java b/Mage/src/main/java/mage/game/permanent/PermanentImpl.java index d2a210af1e..78dcabfa8b 100644 --- a/Mage/src/main/java/mage/game/permanent/PermanentImpl.java +++ b/Mage/src/main/java/mage/game/permanent/PermanentImpl.java @@ -1117,8 +1117,9 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { return false; } } + if (game.getPlayer(this.getControllerId()).hasOpponent(sourceControllerId, game) - && game.getContinuousEffects().asThough(this.getId(), AsThoughEffectType.HEXPROOF, null, sourceControllerId, game) == null + && null == game.getContinuousEffects().asThough(this.getId(), AsThoughEffectType.HEXPROOF, null, sourceControllerId, game) && abilities.stream() .filter(HexproofBaseAbility.class::isInstance) .map(HexproofBaseAbility.class::cast) @@ -1129,9 +1130,14 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { if (hasProtectionFrom(source, game)) { return false; } - // needed to get the correct possible targets if target rule modification effects are active - // e.g. Fiendslayer Paladin tried to target with Ultimate Price - return !game.getContinuousEffects().preventedByRuleModification(new TargetEvent(this, source.getId(), sourceControllerId), null, game, true); + + // example: Fiendslayer Paladin tried to target with Ultimate Price + return !game.getContinuousEffects().preventedByRuleModification( + new TargetEvent(this, source.getId(), sourceControllerId), + null, + game, + true + ); } return true; diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index 94c55f11f4..2feceac5e7 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -634,24 +634,32 @@ public abstract class PlayerImpl implements Player, Serializable { return false; } if (source != null) { - // there is only variant of shroud, so check the instance and any asthougheffects that would ignore it if (abilities.containsKey(ShroudAbility.getInstance().getId()) - && game.getContinuousEffects().asThough(this.getId(), AsThoughEffectType.SHROUD, null, sourceControllerId, game) == null) { + && null == game.getContinuousEffects().asThough(this.getId(), AsThoughEffectType.SHROUD, null, sourceControllerId, game)) { return false; } - // check for all variants of hexproof and any asthougheffects that would ignore it - // TODO there may be "prevented by rule-modification" effects, so add them if known + if (sourceControllerId != null - && this.hasOpponent(sourceControllerId, game)) { - for (Ability a : abilities) { - if (a instanceof HexproofBaseAbility - && ((HexproofBaseAbility) a).checkObject(source, game) - && game.getContinuousEffects().asThough(this.getId(), AsThoughEffectType.HEXPROOF, null, sourceControllerId, game) == null) { - return false; - } - } + && this.hasOpponent(sourceControllerId, game) + && null == game.getContinuousEffects().asThough(this.getId(), AsThoughEffectType.HEXPROOF, null, sourceControllerId, game) + && abilities.stream() + .filter(HexproofBaseAbility.class::isInstance) + .map(HexproofBaseAbility.class::cast) + .anyMatch(ability -> ability.checkObject(source, game))) { + return false; } - return !hasProtectionFrom(source, game); + + if (hasProtectionFrom(source, game)) { + return false; + } + + // example: Peace Talks + return !game.getContinuousEffects().preventedByRuleModification( + new TargetEvent(this, source.getId(), sourceControllerId), + null, + game, + true + ); } return true; } @@ -686,7 +694,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, false, null, game); } @@ -1159,7 +1167,7 @@ public abstract class PlayerImpl implements Player, Serializable { /** * @param originalAbility * @param game - * @param noMana cast it without paying mana costs + * @param noMana cast it without paying mana costs * @param approvingObject which object approved the cast * @return */ @@ -2921,7 +2929,7 @@ public abstract class PlayerImpl implements Player, Serializable { * @return */ private Object rollDieInner(Outcome outcome, Game game, Ability source, RollDieType rollDieType, - int sidesAmount, int chaosSidesAmount, int planarSidesAmount, int rollsAmount) { + int sidesAmount, int chaosSidesAmount, int planarSidesAmount, int rollsAmount) { if (rollsAmount == 1) { return rollDieInnerWithReplacement(game, source, rollDieType, sidesAmount, chaosSidesAmount, planarSidesAmount); } @@ -3017,8 +3025,8 @@ public abstract class PlayerImpl implements Player, Serializable { * @param outcome * @param source * @param game - * @param sidesAmount number of sides the dice has - * @param rollsAmount number of tries to roll the dice + * @param sidesAmount number of sides the dice has + * @param rollsAmount number of tries to roll the dice * @param ignoreLowestAmount remove the lowest rolls from the results * @return the number that the player rolled */ @@ -3036,18 +3044,18 @@ public abstract class PlayerImpl implements Player, Serializable { * @param outcome * @param source * @param game - * @param rollDieType die type to roll, e.g. planar or numerical - * @param sidesAmount sides per die - * @param chaosSidesAmount for planar die: chaos sides - * @param planarSidesAmount for planar die: planar sides - * @param rollsAmount rolls + * @param rollDieType die type to roll, e.g. planar or numerical + * @param sidesAmount sides per die + * @param chaosSidesAmount for planar die: chaos sides + * @param planarSidesAmount for planar die: planar sides + * @param rollsAmount rolls * @param ignoreLowestAmount for numerical die: ignore multiple rolls with - * the lowest values + * the lowest values * @return */ private List rollDiceInner(Outcome outcome, Ability source, Game game, RollDieType rollDieType, - int sidesAmount, int chaosSidesAmount, int planarSidesAmount, - int rollsAmount, int ignoreLowestAmount) { + int sidesAmount, int chaosSidesAmount, int planarSidesAmount, + int rollsAmount, int ignoreLowestAmount) { RollDiceEvent rollDiceEvent = new RollDiceEvent(source, rollDieType, sidesAmount, rollsAmount); if (ignoreLowestAmount > 0) { rollDiceEvent.incIgnoreLowestAmount(ignoreLowestAmount); @@ -3207,10 +3215,10 @@ public abstract class PlayerImpl implements Player, Serializable { /** * @param source * @param game - * @param chaosSidesAmount The number of chaos sides the planar die - * currently has (normally 1 but can be 5) + * @param chaosSidesAmount The number of chaos sides the planar die + * currently has (normally 1 but can be 5) * @param planarSidesAmount The number of chaos sides the planar die - * currently has (normally 1) + * currently has (normally 1) * @return the outcome that the player rolled. Either ChaosRoll, PlanarRoll * or BlankRoll */ @@ -3268,7 +3276,7 @@ public abstract class PlayerImpl implements Player, Serializable { for (Card card : getHand().getCards(game)) { Abilities manaAbilities = card.getAbilities(game).getAvailableActivatedManaAbilities(Zone.HAND, playerId, game); - for (Iterator it = manaAbilities.iterator(); it.hasNext();) { + for (Iterator it = manaAbilities.iterator(); it.hasNext(); ) { ActivatedManaAbilityImpl ability = it.next(); Abilities noTapAbilities = new AbilitiesImpl<>(ability); if (ability.getManaCosts().isEmpty() && !ability.isPoolDependant()) { @@ -3285,7 +3293,7 @@ public abstract class PlayerImpl implements Player, Serializable { boolean useLater = false; // sources with mana costs or mana pool dependency Abilities manaAbilities = permanent.getAbilities(game).getAvailableActivatedManaAbilities(Zone.BATTLEFIELD, playerId, game); // returns ability only if canActivate is true - 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); @@ -3327,7 +3335,7 @@ public abstract class PlayerImpl implements Player, Serializable { boolean usePoolDependantAbilities = false; // use such abilities later than other if possible because it can maximize mana production while (anAbilityWasUsed && !sourceWithCosts.isEmpty()) { anAbilityWasUsed = false; - for (Iterator> iterator = sourceWithCosts.iterator(); iterator.hasNext();) { + for (Iterator> iterator = sourceWithCosts.iterator(); iterator.hasNext(); ) { Abilities manaAbilities = iterator.next(); if (usePoolDependantAbilities || !manaAbilities.hasPoolDependantAbilities()) { boolean used; @@ -3363,7 +3371,7 @@ public abstract class PlayerImpl implements Player, Serializable { * and cleared thereafter * * @param netManaAvailable the net mana produced by the triggered mana - * abaility + * abaility */ @Override public void addAvailableTriggeredMana(List netManaAvailable @@ -3445,7 +3453,7 @@ public abstract class PlayerImpl implements Player, Serializable { /** * @param ability * @param availableMana if null, it won't be checked if enough mana is - * available + * available * @param sourceObject * @param game * @return @@ -3877,13 +3885,14 @@ public abstract class PlayerImpl implements Player, Serializable { /** * Returns a list of all available spells and abilities the player can - * currently cast/activate with his available resources + * currently cast/activate with his available resources. + * Without target validation. * * @param game - * @param hidden also from hidden objects (e.g. turned face down cards ?) - * @param fromZone of objects from which zone (ALL = from all zones) + * @param hidden also from hidden objects (e.g. turned face down cards ?) + * @param fromZone of objects from which zone (ALL = from all zones) * @param hideDuplicatedAbilities if equal abilities exist return only the - * first instance + * first instance * @return */ public List getPlayable(Game game, boolean hidden, Zone fromZone, boolean hideDuplicatedAbilities) { @@ -4479,7 +4488,7 @@ public abstract class PlayerImpl implements Player, Serializable { @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); } @@ -4638,7 +4647,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(); @@ -4816,7 +4825,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" + CardUtil.getSourceLogName(game, source, card.getId())); + + ' ' : "") + "to the exile zone" + CardUtil.getSourceLogName(game, source, card.getId())); } }