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 8f8af9a743a..f7c3ed93023 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 @@ -1319,7 +1319,7 @@ public class ComputerPlayer extends PlayerImpl implements Player { SpellAbility ability = card.getSpellAbility(); if (ability != null && ability.canActivate(playerId, game).canActivate() && !game.getContinuousEffects().preventedByRuleModification(GameEvent.getEvent(GameEvent.EventType.CAST_SPELL, ability.getSourceId(), ability, playerId), ability, game, true)) { - if (card.getCardType(game).contains(CardType.INSTANT) + if (card.isInstant(game) || card.hasAbility(FlashAbility.getInstance(), game)) { playableInstant.add(card); } else { diff --git a/Mage.Sets/src/mage/cards/d/DereviEmpyrialTactician.java b/Mage.Sets/src/mage/cards/d/DereviEmpyrialTactician.java index 4e139a41fa8..525bd894a5e 100644 --- a/Mage.Sets/src/mage/cards/d/DereviEmpyrialTactician.java +++ b/Mage.Sets/src/mage/cards/d/DereviEmpyrialTactician.java @@ -117,7 +117,7 @@ class DereviEmpyrialTacticianAbility extends ActivatedAbilityImpl { @Override public ActivationStatus canActivate(UUID playerId, Game game) { Zone currentZone = game.getState().getZone(this.getSourceId()); - if (currentZone == null || currentZone != Zone.COMMAND) { + if (currentZone != Zone.COMMAND) { return ActivationStatus.getFalse(); } return super.canActivate(playerId, game); diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/SuspendTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/SuspendTest.java index 69db45b6dc9..fcc2f0a191f 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/SuspendTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/SuspendTest.java @@ -17,7 +17,7 @@ public class SuspendTest extends CardTestPlayerBase { * dies, exile it with three time counters on it and it gains suspend. */ @Test - public void testEpochrasite() { + public void test_Single_Epochrasite() { addCard(Zone.BATTLEFIELD, playerA, "Plains", 2); // Epochrasite enters the battlefield with three +1/+1 counters on it if you didn't cast it from your hand. @@ -45,7 +45,7 @@ public class SuspendTest extends CardTestPlayerBase { * card. If it doesn't have suspend, it gains suspend. */ @Test - public void testJhoiraOfTheGhitu() { + public void test_Single_JhoiraOfTheGhitu() { addCard(Zone.BATTLEFIELD, playerA, "Mountain", 2); @@ -69,7 +69,7 @@ public class SuspendTest extends CardTestPlayerBase { * counters and can be cast after the 3 counters are removed */ @Test - public void testDelay() { + public void test_Single_Delay() { addCard(Zone.BATTLEFIELD, playerA, "Plains", 2); @@ -93,7 +93,7 @@ public class SuspendTest extends CardTestPlayerBase { } @Test - public void testDeepSeaKraken() { + public void test_Single_DeepSeaKraken() { addCard(Zone.BATTLEFIELD, playerA, "Island", 3); // Suspend 9-{2}{U} // Whenever an opponent casts a spell, if Deep-Sea Kraken is suspended, remove a time counter from it. @@ -118,7 +118,7 @@ public class SuspendTest extends CardTestPlayerBase { } @Test - public void testAncestralVisionCantBeCastDirectly() { + public void test_Single_AncestralVisionCantBeCastDirectly() { // Suspend 4-{U} // Target player draws three cards. addCard(Zone.HAND, playerA, "Ancestral Vision", 1); @@ -138,7 +138,7 @@ public class SuspendTest extends CardTestPlayerBase { * It made my Rift Bolt cost 2R to suspend instead of R */ @Test - public void testCostManipulation() { + public void test_CostManipulation() { // Rift Bolt deals 3 damage to any target. // Suspend 1-{R} addCard(Zone.HAND, playerA, "Rift Bolt", 1); @@ -162,7 +162,7 @@ public class SuspendTest extends CardTestPlayerBase { * Example: cards coming off suspend shouldn't trigger Knowledge Pool. */ @Test - public void testThatNotCastFromHand() { + public void test_ThatNotCastFromHand() { // Rift Bolt deals 3 damage to any target. // Suspend 1-{R} @@ -310,4 +310,40 @@ public class SuspendTest extends CardTestPlayerBase { execute(); assertAllCommandsUsed(); } + + @Test + public void test_OnlyOwnerCanActivateSuspend() { + // bug: you or AI can activate suspend from opponent's hand + + // Suspend 5-{G} + String suspendA = "Suspend 5"; // owner is player A + addCard(Zone.HAND, playerA, "Durkwood Baloth", 1); + addCard(Zone.BATTLEFIELD, playerA, "Forest", 1); + addCard(Zone.BATTLEFIELD, playerB, "Forest", 1); + // + // Suspend 3—{0} + String suspendB = "Suspend 3"; // owner is player B + addCard(Zone.HAND, playerB, "Mox Tantalite", 1); + + // turn 1 - A can own, B can't + checkPlayableAbility("T1 - Player A can own", 1, PhaseStep.PRECOMBAT_MAIN, playerA, suspendA, true); + checkPlayableAbility("T1 - Player A can't opponent", 1, PhaseStep.PRECOMBAT_MAIN, playerA, suspendB, false); + checkPlayableAbility("T1 - Player B can't own", 1, PhaseStep.PRECOMBAT_MAIN, playerB, suspendB, false); + checkPlayableAbility("T1 - Player B can't opponent", 1, PhaseStep.PRECOMBAT_MAIN, playerB, suspendA, false); + + // turn 2 - A can't, B can own + checkPlayableAbility("T2 - Player A can't own", 2, PhaseStep.PRECOMBAT_MAIN, playerA, suspendA, false); + checkPlayableAbility("T2 - Player A can't opponent", 2, PhaseStep.PRECOMBAT_MAIN, playerA, suspendB, false); + checkPlayableAbility("T2 - Player B can own", 2, PhaseStep.PRECOMBAT_MAIN, playerB, suspendB, true); + checkPlayableAbility("T2 - Player B can't opponent", 2, PhaseStep.PRECOMBAT_MAIN, playerB, suspendA, false); + + activateAbility(2, PhaseStep.POSTCOMBAT_MAIN, playerB, suspendB); + + setStrictChooseMode(true); + setStopAt(2, PhaseStep.END_TURN); + execute(); + assertAllCommandsUsed(); + + assertExileCount(playerB, "Mox Tantalite", 1); // suspended + } } diff --git a/Mage/src/main/java/mage/abilities/ActivatedAbility.java b/Mage/src/main/java/mage/abilities/ActivatedAbility.java index 2390b78c059..29e8d90bace 100644 --- a/Mage/src/main/java/mage/abilities/ActivatedAbility.java +++ b/Mage/src/main/java/mage/abilities/ActivatedAbility.java @@ -44,6 +44,13 @@ public interface ActivatedAbility extends Ability { } } + /** + * WARNING, don't forget to call super.canActivate on override in card's code + * + * @param playerId + * @param game + * @return + */ ActivationStatus canActivate(UUID playerId, Game game); // has to return a reference to the permitting ability/source void setMayActivate(TargetController mayActivate); diff --git a/Mage/src/main/java/mage/abilities/ActivatedAbilityImpl.java b/Mage/src/main/java/mage/abilities/ActivatedAbilityImpl.java index fac50900b30..418613b5b8a 100644 --- a/Mage/src/main/java/mage/abilities/ActivatedAbilityImpl.java +++ b/Mage/src/main/java/mage/abilities/ActivatedAbilityImpl.java @@ -9,6 +9,7 @@ import mage.abilities.costs.mana.ManaCosts; import mage.abilities.costs.mana.PhyrexianManaCost; import mage.abilities.effects.Effect; import mage.abilities.effects.Effects; +import mage.abilities.keyword.FlashAbility; import mage.abilities.mana.ManaOptions; import mage.cards.Card; import mage.constants.*; @@ -145,6 +146,7 @@ public abstract class ActivatedAbilityImpl extends AbilityImpl implements Activa protected boolean checkTargetController(UUID playerId, Game game) { switch (mayActivate) { case ANY: + case EACH_PLAYER: return true; case ACTIVE: return game.getActivePlayerId() == playerId; @@ -170,6 +172,15 @@ public abstract class ActivatedAbilityImpl extends AbilityImpl implements Activa return true; } + /** + * Basic activation check. It contains costs and targets legality too. + *

+ * WARNING, don't forget to call super.canActivate on override in card's code in most cases. + * + * @param playerId + * @param game + * @return + */ @Override public ActivationStatus canActivate(UUID playerId, Game game) { //20091005 - 602.2 @@ -178,26 +189,43 @@ public abstract class ActivatedAbilityImpl extends AbilityImpl implements Activa || condition.apply(game, this)))) { return ActivationStatus.getFalse(); } + if (!this.checkTargetController(playerId, game)) { return ActivationStatus.getFalse(); } + + // timing check //20091005 - 602.5d/602.5e + boolean asInstant; ApprovingObject approvingObject = game.getContinuousEffects() .asThough(sourceId, AsThoughEffectType.ACTIVATE_AS_INSTANT, this, controllerId, game); - if (timing == TimingRule.INSTANT - || game.canPlaySorcery(playerId) - || null != approvingObject) { - if (costs.canPay(this, this, playerId, game) - && canChooseTarget(game, playerId)) { - this.activatorId = playerId; - return new ActivationStatus(true, approvingObject); - } + asInstant = approvingObject != null; + asInstant |= (timing == TimingRule.INSTANT); + Card card = game.getCard(getSourceId()); + if (card != null) { + asInstant |= card.isInstant(game); + asInstant |= card.hasAbility(FlashAbility.getInstance(), game); } - return ActivationStatus.getFalse(); + if (!asInstant && !game.canPlaySorcery(playerId)) { + return ActivationStatus.getFalse(); + } + + // targets and costs check + if (!costs.canPay(this, this, playerId, game) + || canChooseTarget(game, playerId)) { + return ActivationStatus.getFalse(); + } + + // all fine, can be activated + // TODO: WTF, must be rework to remove data change in canActivate call + // (it can be called from any place by any player or card). + // So add game.inCheckPlayableState() here? + this.activatorId = playerId; + return new ActivationStatus(true, approvingObject); } @Override diff --git a/Mage/src/main/java/mage/abilities/SpellAbility.java b/Mage/src/main/java/mage/abilities/SpellAbility.java index 956d5fab00f..368af5b155f 100644 --- a/Mage/src/main/java/mage/abilities/SpellAbility.java +++ b/Mage/src/main/java/mage/abilities/SpellAbility.java @@ -80,6 +80,7 @@ public class SpellAbility extends ActivatedAbilityImpl { return null != game.getContinuousEffects().asThough(sourceId, AsThoughEffectType.CAST_AS_INSTANT, this, playerId, game) // check this first to allow Offering in main phase || timing == TimingRule.INSTANT + || object.isInstant(game) || object.hasAbility(FlashAbility.getInstance(), game) || game.canPlaySorcery(playerId); } diff --git a/Mage/src/main/java/mage/abilities/common/PassAbility.java b/Mage/src/main/java/mage/abilities/common/PassAbility.java index e49cc4ca4a8..5b8c124ef1f 100644 --- a/Mage/src/main/java/mage/abilities/common/PassAbility.java +++ b/Mage/src/main/java/mage/abilities/common/PassAbility.java @@ -8,6 +8,8 @@ import mage.game.Game; import java.util.UUID; /** + * AI only: fake ability for game simulations + * * @author BetaSteward_at_googlemail.com */ public class PassAbility extends ActivatedAbilityImpl { diff --git a/Mage/src/main/java/mage/abilities/keyword/ForetellAbility.java b/Mage/src/main/java/mage/abilities/keyword/ForetellAbility.java index efb4fa38886..60929049109 100644 --- a/Mage/src/main/java/mage/abilities/keyword/ForetellAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/ForetellAbility.java @@ -72,7 +72,6 @@ public class ForetellAbility extends SpecialAction { // activate only during the controller's turn if (game.getState().getContinuousEffects().getApplicableAsThoughEffects(AsThoughEffectType.ALLOW_FORETELL_ANYTIME, game).isEmpty() && !game.isActivePlayer(this.getControllerId())) { - // TODO: must be fixed to call super.canActivate here for additional checks someday return ActivationStatus.getFalse(); } return super.canActivate(playerId, game); diff --git a/Mage/src/main/java/mage/abilities/keyword/SuspendAbility.java b/Mage/src/main/java/mage/abilities/keyword/SuspendAbility.java index 12c6d39f261..ea752b29bb5 100644 --- a/Mage/src/main/java/mage/abilities/keyword/SuspendAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/SuspendAbility.java @@ -1,7 +1,6 @@ package mage.abilities.keyword; import mage.ApprovingObject; -import mage.MageObject; import mage.abilities.Ability; import mage.abilities.SpecialAction; import mage.abilities.TriggeredAbilityImpl; @@ -127,6 +126,8 @@ public class SuspendAbility extends SpecialAction { public SuspendAbility(int suspend, ManaCost cost, Card card, boolean shortRule) { super(Zone.HAND); this.addCost(cost); + // suspend uses both sorcery/instant timing depends on object, so it checks with object, see canActivate + this.setTiming(TimingRule.SORCERY); this.addEffect(new SuspendExileEffect(suspend)); this.usesStack = false; if (suspend == Integer.MAX_VALUE) { @@ -205,16 +206,12 @@ public class SuspendAbility extends SpecialAction { @Override public ActivationStatus canActivate(UUID playerId, Game game) { + // suspend can only be activated from a hand if (game.getState().getZone(getSourceId()) != Zone.HAND) { - // Supend can only be activated from hand return ActivationStatus.getFalse(); } - MageObject object = game.getObject(sourceId); - return new ActivationStatus(object.isInstant(game) - || object.hasAbility(FlashAbility.getInstance(), game) - || null != game.getContinuousEffects().asThough(sourceId, - AsThoughEffectType.CAST_AS_INSTANT, this, playerId, game) - || game.canPlaySorcery(playerId), null); + + return super.canActivate(playerId, game); } @Override