Tests for #5630

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, 
+        // This turn and next turn, creatures can't attack, and players and permanents can't be the targets of spells or activated abilities.
-        // and players and permanents can't be the targets 
-        // of spells or activated abilities.
         this.getSpellAbility().addEffect(new PeaceTalksEffect());
-
     }
 
     private PeaceTalks(final PeaceTalks card) {

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();
+    }
 }

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

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
+            // example: Fiendslayer Paladin tried to target with Ultimate Price
-            return !game.getContinuousEffects().preventedByRuleModification(new TargetEvent(this, source.getId(), sourceControllerId), null, game, true);
+            return !game.getContinuousEffects().preventedByRuleModification(
+                    new TargetEvent(this, source.getId(), sourceControllerId),
+                    null,
+                    game,
+                    true
+            );
         }
 
         return true;

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)) {
+                    && this.hasOpponent(sourceControllerId, game)
-                for (Ability a : abilities) {
+                    && null == game.getContinuousEffects().asThough(this.getId(), AsThoughEffectType.HEXPROOF, null, sourceControllerId, game)
-                    if (a instanceof HexproofBaseAbility
+                    && abilities.stream()
-                            && ((HexproofBaseAbility) a).checkObject(source, game)
+                    .filter(HexproofBaseAbility.class::isInstance)
-                            && game.getContinuousEffects().asThough(this.getId(), AsThoughEffectType.HEXPROOF, null, sourceControllerId, game) == null) {
+                    .map(HexproofBaseAbility.class::cast)
-                        return false;
+                    .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 sidesAmount        number of sides the dice has
-     * @param rollsAmount number of tries to roll the dice
+     * @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 rollDieType        die type to roll, e.g. planar or numerical
-     * @param sidesAmount sides per die
+     * @param sidesAmount        sides per die
-     * @param chaosSidesAmount for planar die: chaos sides
+     * @param chaosSidesAmount   for planar die: chaos sides
-     * @param planarSidesAmount for planar die: planar sides
+     * @param planarSidesAmount  for planar die: planar sides
-     * @param rollsAmount rolls
+     * @param rollsAmount        rolls
      * @param ignoreLowestAmount for numerical die: ignore multiple rolls with
-     * the lowest values
+     *                           the lowest values
      * @return
      */
     private List<Object> rollDiceInner(Outcome outcome, Ability source, Game game, RollDieType rollDieType,
-            int sidesAmount, int chaosSidesAmount, int planarSidesAmount,
+                                       int sidesAmount, int chaosSidesAmount, int planarSidesAmount,
-            int rollsAmount, int ignoreLowestAmount) {
+                                       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
+     * @param chaosSidesAmount  The number of chaos sides the planar die
-     * currently has (normally 1 but can be 5)
+     *                          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<ActivatedManaAbilityImpl> manaAbilities
                     = card.getAbilities(game).getAvailableActivatedManaAbilities(Zone.HAND, playerId, game);
-            for (Iterator<ActivatedManaAbilityImpl> it = manaAbilities.iterator(); it.hasNext();) {
+            for (Iterator<ActivatedManaAbilityImpl> it = manaAbilities.iterator(); it.hasNext(); ) {
                 ActivatedManaAbilityImpl ability = it.next();
                 Abilities<ActivatedManaAbilityImpl> 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<ActivatedManaAbilityImpl> manaAbilities
                     = permanent.getAbilities(game).getAvailableActivatedManaAbilities(Zone.BATTLEFIELD, playerId, game); // returns ability only if canActivate is true
-            for (Iterator<ActivatedManaAbilityImpl> it = manaAbilities.iterator(); it.hasNext();) {
+            for (Iterator<ActivatedManaAbilityImpl> 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<Abilities<ActivatedManaAbilityImpl>> iterator = sourceWithCosts.iterator(); iterator.hasNext();) {
+            for (Iterator<Abilities<ActivatedManaAbilityImpl>> iterator = sourceWithCosts.iterator(); iterator.hasNext(); ) {
                 Abilities<ActivatedManaAbilityImpl> 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<Mana> 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 hidden                  also from hidden objects (e.g. turned face down cards ?)
-     * @param fromZone of objects from which zone (ALL = from all zones)
+     * @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<ActivatedAbility> 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<? extends Card> 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<? extends Card> it = allCards.iterator(); it.hasNext();) {
+            for (Iterator<? extends Card> 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()));
                 }
 
             }