From 43205b6f46e2031797ea6088e00d56bd02e2608c Mon Sep 17 00:00:00 2001 From: Nathaniel Brandes Date: Mon, 9 May 2016 00:25:36 -0700 Subject: [PATCH] Fix Angel of Jubilation. The Angel now properly only restricts the sacrifice of creatures. --- .../server/record/UserStatsRepository.java | 2 +- .../avacynrestored/AngelOfJubilation.java | 56 ++++++++- .../sets/saviorsofkamigawa/TombOfUrami.java | 6 +- .../continuous/AngelOfJubilationTest.java | 117 ++++++++++++++++++ .../java/org/mage/test/player/TestPlayer.java | 13 +- .../java/org/mage/test/stub/PlayerStub.java | 21 ++-- .../costs/common/SacrificeAllCost.java | 19 +++ .../costs/common/SacrificeSourceCost.java | 6 +- .../costs/common/SacrificeTargetCost.java | 14 ++- Mage/src/main/java/mage/players/Player.java | 7 +- .../main/java/mage/players/PlayerImpl.java | 23 ++-- 11 files changed, 246 insertions(+), 38 deletions(-) diff --git a/Mage.Server/src/main/java/mage/server/record/UserStatsRepository.java b/Mage.Server/src/main/java/mage/server/record/UserStatsRepository.java index 26348de66a..80bfd0123d 100644 --- a/Mage.Server/src/main/java/mage/server/record/UserStatsRepository.java +++ b/Mage.Server/src/main/java/mage/server/record/UserStatsRepository.java @@ -171,7 +171,7 @@ public enum UserStatsRepository { } } } - return new ArrayList(updatedUsers); + return new ArrayList<>(updatedUsers); } public void closeDB() { diff --git a/Mage.Sets/src/mage/sets/avacynrestored/AngelOfJubilation.java b/Mage.Sets/src/mage/sets/avacynrestored/AngelOfJubilation.java index 654b525bbb..77e92985c5 100644 --- a/Mage.Sets/src/mage/sets/avacynrestored/AngelOfJubilation.java +++ b/Mage.Sets/src/mage/sets/avacynrestored/AngelOfJubilation.java @@ -28,23 +28,33 @@ package mage.sets.avacynrestored; import java.util.UUID; + import mage.MageInt; import mage.ObjectColor; import mage.abilities.Ability; +import mage.abilities.SpellAbility; import mage.abilities.common.SimpleStaticAbility; +import mage.abilities.costs.Cost; +import mage.abilities.costs.common.SacrificeTargetCost; import mage.abilities.effects.ContinuousEffectImpl; import mage.abilities.effects.common.continuous.BoostControlledEffect; +import mage.abilities.effects.common.cost.CostModificationEffectImpl; +import mage.abilities.keyword.FlashbackAbility; import mage.abilities.keyword.FlyingAbility; import mage.cards.CardImpl; +import mage.constants.AbilityType; import mage.constants.CardType; +import mage.constants.CostModificationType; import mage.constants.Duration; import mage.constants.Layer; import mage.constants.Outcome; import mage.constants.Rarity; import mage.constants.SubLayer; import mage.constants.Zone; +import mage.filter.Filter; import mage.filter.common.FilterCreaturePermanent; import mage.filter.predicate.Predicates; +import mage.filter.predicate.mageobject.CardTypePredicate; import mage.filter.predicate.mageobject.ColorPredicate; import mage.game.Game; import mage.players.Player; @@ -74,7 +84,8 @@ public class AngelOfJubilation extends CardImpl { this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new BoostControlledEffect(1, 1, Duration.WhileOnBattlefield, filterNonBlack, true))); // Players can't pay life or sacrifice creatures to cast spells or activate abilities. - this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new AngelOfJubilationEffect(Duration.WhileOnBattlefield))); + this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new AngelOfJubilationEffect())); + this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new AngelOfJubilationSacrificeFilterEffect())); } public AngelOfJubilation(final AngelOfJubilation card) { @@ -89,8 +100,8 @@ public class AngelOfJubilation extends CardImpl { class AngelOfJubilationEffect extends ContinuousEffectImpl { - public AngelOfJubilationEffect(Duration duration) { - super(duration, Layer.PlayerEffects, SubLayer.NA, Outcome.Detriment); + public AngelOfJubilationEffect() { + super(Duration.WhileOnBattlefield, Layer.PlayerEffects, SubLayer.NA, Outcome.Detriment); staticText = "Players can't pay life or sacrifice creatures to cast spells or activate abilities"; } @@ -107,9 +118,44 @@ class AngelOfJubilationEffect extends ContinuousEffectImpl { public boolean apply(Game game, Ability source) { for (Player player : game.getPlayers().values()) { player.setCanPayLifeCost(false); - player.setCanPaySacrificeCost(false); + player.setCanPaySacrificeCostFilter(new FilterCreaturePermanent()); } return true; } - +} + +class AngelOfJubilationSacrificeFilterEffect extends CostModificationEffectImpl { + + public AngelOfJubilationSacrificeFilterEffect() { + super(Duration.WhileOnBattlefield, Outcome.Detriment, CostModificationType.SET_COST); + staticText = "Players can't pay life or sacrifice creatures to cast spells or activate abilities"; + } + + protected AngelOfJubilationSacrificeFilterEffect(AngelOfJubilationSacrificeFilterEffect effect) { + super(effect); + } + + @Override + public boolean apply(Game game, Ability source, Ability abilityToModify) { + for (Cost cost : abilityToModify.getCosts()) { + if(cost instanceof SacrificeTargetCost) { + SacrificeTargetCost sacrificeCost = (SacrificeTargetCost) cost; + Filter filter = sacrificeCost.getTargets().get(0).getFilter(); + filter.add(Predicates.not(new CardTypePredicate(CardType.CREATURE))); + } + } + return true; + } + + @Override + public boolean applies(Ability abilityToModify, Ability source, Game game) { + return abilityToModify.getAbilityType().equals(AbilityType.ACTIVATED) || + abilityToModify instanceof SpellAbility || abilityToModify instanceof FlashbackAbility; + } + + @Override + public AngelOfJubilationSacrificeFilterEffect copy() { + return new AngelOfJubilationSacrificeFilterEffect(this); + } + } diff --git a/Mage.Sets/src/mage/sets/saviorsofkamigawa/TombOfUrami.java b/Mage.Sets/src/mage/sets/saviorsofkamigawa/TombOfUrami.java index 5a1b780edd..d7d1c13ca2 100644 --- a/Mage.Sets/src/mage/sets/saviorsofkamigawa/TombOfUrami.java +++ b/Mage.Sets/src/mage/sets/saviorsofkamigawa/TombOfUrami.java @@ -102,8 +102,10 @@ class SacrificeAllLandCost extends CostImpl { @Override public boolean canPay(Ability ability, UUID sourceId, UUID controllerId, Game game) { - if (!game.getPlayer(controllerId).canPaySacrificeCost()) { - return false; + for(Permanent permanent : game.getBattlefield().getActivePermanents(new FilterControlledLandPermanent(), ability.getControllerId(), game)){ + if (!game.getPlayer(controllerId).canPaySacrificeCost(permanent, sourceId, controllerId, game)) { + return false; + } } return true; } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/continuous/AngelOfJubilationTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/continuous/AngelOfJubilationTest.java index 9becbc62b3..e8093a640e 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/continuous/AngelOfJubilationTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/continuous/AngelOfJubilationTest.java @@ -58,5 +58,122 @@ public class AngelOfJubilationTest extends CardTestPlayerBase { assertPowerToughness(playerA, "Devout Chaplain", 2, 2); assertPowerToughness(playerA, "Corpse Traders", 3, 3); } + + @Test + public void testOpponentCantSacrificeCreatures() { + addCard(Zone.BATTLEFIELD, playerA, "Angel of Jubilation"); + addCard(Zone.BATTLEFIELD, playerB, "Nantuko Husk"); + addCard(Zone.BATTLEFIELD, playerB, "Corpse Traders"); + + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Sacrifice a creature: {this} gets +2/+2 until end of turn."); + playerB.addChoice("Corpse Traders"); + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPowerToughness(playerB, "Nantuko Husk", 2, 2); + assertPermanentCount(playerB, "Corpse Traders", 1); + } + + @Test + public void testOpponentCanSacrificeNonCreaturePermanents() { + addCard(Zone.BATTLEFIELD, playerA, "Angel of Jubilation"); + addCard(Zone.BATTLEFIELD, playerA, "Savannah Lions"); + addCard(Zone.BATTLEFIELD, playerB, "Barrin, Master Wizard"); + addCard(Zone.BATTLEFIELD, playerB, "Nantuko Husk"); + addCard(Zone.BATTLEFIELD, playerB, "Island", 4); + addCard(Zone.BATTLEFIELD, playerB, "Food Chain"); + + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerB, "{2},Sacrifice a permanent you control: Return target creature to its owner's hand."); + playerB.addChoice("Food Chain"); + playerA.addTarget("Angel of Jubilation"); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPermanentCount(playerA, "Angel of Jubilation", 0); + assertPermanentCount(playerB, "Food Chain", 0); + } + + @Test + public void testOpponentCantSacrificeCreaturesAsPartOfPermanentsOptions() { + addCard(Zone.BATTLEFIELD, playerA, "Angel of Jubilation"); + addCard(Zone.BATTLEFIELD, playerB, "Barrin, Master Wizard"); + addCard(Zone.BATTLEFIELD, playerB, "Nantuko Husk"); + addCard(Zone.BATTLEFIELD, playerB, "Llanowar Elves", 2); + + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerB, "{2},Sacrifice a permanent you control: Return target creature to its owner's hand."); + playerB.addChoice("Nantuko Husk"); + playerA.addTarget("Angel of Jubilation"); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPermanentCount(playerA, "Angel of Jubilation", 1); + assertPermanentCount(playerB, "Nantuko Husk", 1); + } + + @Test + public void testOpponentCantSacrificeAll() { + addCard(Zone.BATTLEFIELD, playerA, "Angel of Jubilation"); + addCard(Zone.BATTLEFIELD, playerB, "Nantuko Husk"); + addCard(Zone.BATTLEFIELD, playerB, "Corpse Traders"); + addCard(Zone.HAND, playerB, "Soulblast"); + addCard(Zone.BATTLEFIELD, playerB, "Mountain", 6); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Soulblast", playerA); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertLife(playerA, 20); + assertLife(playerB, 20); + + assertPermanentCount(playerB, "Nantuko Husk", 1); + assertPermanentCount(playerB, "Corpse Traders", 1); + } + + @Test + public void testOpponentCantSacrificeCreatureSource() { + addCard(Zone.BATTLEFIELD, playerA, "Angel of Jubilation"); + addCard(Zone.BATTLEFIELD, playerB, "Children of Korlis"); + + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Sacrifice {this}: You gain life equal to the life you've lost this turn."); + playerB.addChoice("Skirk Prospector"); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPermanentCount(playerB, "Children of Korlis", 1); + } + + @Test + public void testOpponentCanSacrificeAllLands() { + addCard(Zone.BATTLEFIELD, playerA, "Angel of Jubilation"); + addCard(Zone.BATTLEFIELD, playerB, "Tomb of Urami"); + addCard(Zone.BATTLEFIELD, playerB, "Swamp", 4); + + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerB, "{2}{B}{B},{T}, Sacrifice all lands you control: Put a legendary 5/5 black Demon Spirit creature token with flying named Urami onto the battlefield."); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPermanentCount(playerB, "Swamp", 0); + } + + @Test + public void testOpponentCanSacrificeNonCreatureSource() { + addCard(Zone.BATTLEFIELD, playerA, "Angel of Jubilation"); + addCard(Zone.BATTLEFIELD, playerA, "Tundra"); + addCard(Zone.BATTLEFIELD, playerB, "Wasteland"); + + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerB, "{T}, Sacrifice {this}: Destroy target nonbasic land."); + playerB.addTarget("Tundra"); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPermanentCount(playerA, "Tundra", 0); + assertPermanentCount(playerB, "Wasteland", 0); + } } 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 ff8f1b33c9..0a48edc89c 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 @@ -1731,13 +1731,18 @@ public class TestPlayer implements Player { } @Override - public boolean canPaySacrificeCost() { - return computerPlayer.canPaySacrificeCost(); + public boolean canPaySacrificeCost(Permanent permanent, UUID sourceId, UUID controllerId, Game game) { + return computerPlayer.canPaySacrificeCost(permanent, sourceId, controllerId, game); + } + + @Override + public FilterPermanent getSacrificeCostFilter() { + return computerPlayer.getSacrificeCostFilter(); } @Override - public void setCanPaySacrificeCost(boolean canPaySacrificeCost) { - computerPlayer.setCanPaySacrificeCost(canPaySacrificeCost); + public void setCanPaySacrificeCostFilter(FilterPermanent permanent) { + computerPlayer.setCanPaySacrificeCostFilter(permanent); } @Override diff --git a/Mage.Tests/src/test/java/org/mage/test/stub/PlayerStub.java b/Mage.Tests/src/test/java/org/mage/test/stub/PlayerStub.java index 730e9a652e..415722e9c5 100644 --- a/Mage.Tests/src/test/java/org/mage/test/stub/PlayerStub.java +++ b/Mage.Tests/src/test/java/org/mage/test/stub/PlayerStub.java @@ -34,6 +34,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; + import mage.MageObject; import mage.abilities.Abilities; import mage.abilities.Ability; @@ -61,6 +62,7 @@ import mage.constants.RangeOfInfluence; import mage.constants.Zone; import mage.counters.Counter; import mage.counters.Counters; +import mage.filter.FilterPermanent; import mage.game.Game; import mage.game.Graveyard; import mage.game.Table; @@ -208,16 +210,19 @@ public class PlayerStub implements Player { return false; } - @Override - public void setCanPaySacrificeCost(boolean canPaySacrificeCost) { + @Override + public void setCanPaySacrificeCostFilter(FilterPermanent filter) { + } - } - - @Override - public boolean canPaySacrificeCost() { - return false; - } + @Override + public FilterPermanent getSacrificeCostFilter() { + return null; + } + @Override + public boolean canPaySacrificeCost(Permanent permanent, UUID sourceId, UUID controllerId, Game game) { + return false; + } @Override public void setLifeTotalCanChange(boolean lifeTotalCanChange) { diff --git a/Mage/src/main/java/mage/abilities/costs/common/SacrificeAllCost.java b/Mage/src/main/java/mage/abilities/costs/common/SacrificeAllCost.java index 3a64d1abe5..58f767345e 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/SacrificeAllCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/SacrificeAllCost.java @@ -31,11 +31,14 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; import mage.abilities.Ability; +import mage.abilities.ActivatedAbilityImpl; import mage.abilities.costs.Cost; import mage.abilities.costs.CostImpl; +import mage.constants.AbilityType; import mage.filter.FilterPermanent; import mage.game.Game; import mage.game.permanent.Permanent; +import mage.target.common.TargetControlledPermanent; /** * @@ -71,6 +74,22 @@ public class SacrificeAllCost extends CostImpl { @Override public boolean canPay(Ability ability, UUID sourceId, UUID controllerId, Game game) { + UUID activator = controllerId; + if (ability.getAbilityType().equals(AbilityType.ACTIVATED) || ability.getAbilityType().equals(AbilityType.SPECIAL_ACTION)) { + if (((ActivatedAbilityImpl) ability).getActivatorId() != null) { + activator = ((ActivatedAbilityImpl) ability).getActivatorId(); + } else { + // Aktivator not filled? + activator = controllerId; + } + } + + for (Permanent permanent :game.getBattlefield().getAllActivePermanents(filter, controllerId, game)) { + if(!game.getPlayer(activator).canPaySacrificeCost(permanent, sourceId, controllerId, game)) { + return false; + } + } + return true; } diff --git a/Mage/src/main/java/mage/abilities/costs/common/SacrificeSourceCost.java b/Mage/src/main/java/mage/abilities/costs/common/SacrificeSourceCost.java index 00af162be3..7b84a1f89a 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/SacrificeSourceCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/SacrificeSourceCost.java @@ -59,11 +59,9 @@ public class SacrificeSourceCost extends CostImpl { @Override public boolean canPay(Ability ability, UUID sourceId, UUID controllerId, Game game) { - if (!game.getPlayer(controllerId).canPaySacrificeCost()) { - return false; - } Permanent permanent = game.getPermanent(sourceId); - return permanent != null; + + return permanent != null && game.getPlayer(controllerId).canPaySacrificeCost(permanent, sourceId, controllerId, game); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/common/SacrificeTargetCost.java b/Mage/src/main/java/mage/abilities/costs/common/SacrificeTargetCost.java index 19f83fdb68..22a0ab34de 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/SacrificeTargetCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/SacrificeTargetCost.java @@ -30,6 +30,7 @@ package mage.abilities.costs.common; import java.util.ArrayList; import java.util.List; import java.util.UUID; + import mage.abilities.Ability; import mage.abilities.ActivatedAbilityImpl; import mage.abilities.costs.Cost; @@ -51,7 +52,7 @@ public class SacrificeTargetCost extends CostImpl { public SacrificeTargetCost(TargetControlledPermanent target) { this.addTarget(target); target.setNotTarget(true); // sacrifice is never targeted - this.text = "sacrifice " + target.getTargetName(); + this.text = "sacrifice a " + target.getTargetName(); target.setTargetName(target.getTargetName() + " (to sacrifice)"); } @@ -99,10 +100,15 @@ public class SacrificeTargetCost extends CostImpl { activator = controllerId; } } - if (!game.getPlayer(activator).canPaySacrificeCost()) { - return false; + + int validTargets = 0; + for (Permanent permanent :game.getBattlefield().getAllActivePermanents(((TargetControlledPermanent)targets.get(0)).getFilter(), controllerId, game)) { + if(game.getPlayer(activator).canPaySacrificeCost(permanent, sourceId, controllerId, game)) { + validTargets++; + } } - return targets.canChoose(activator, game); + + return validTargets > 0; } @Override diff --git a/Mage/src/main/java/mage/players/Player.java b/Mage/src/main/java/mage/players/Player.java index 9041719b65..a17340faa7 100644 --- a/Mage/src/main/java/mage/players/Player.java +++ b/Mage/src/main/java/mage/players/Player.java @@ -62,6 +62,7 @@ import mage.constants.RangeOfInfluence; import mage.constants.Zone; import mage.counters.Counter; import mage.counters.Counters; +import mage.filter.FilterPermanent; import mage.game.Game; import mage.game.Graveyard; import mage.game.Table; @@ -132,9 +133,11 @@ public interface Player extends MageItem, Copyable { boolean canPayLifeCost(); - void setCanPaySacrificeCost(boolean canPaySacrificeCost); + void setCanPaySacrificeCostFilter(FilterPermanent filter); + + FilterPermanent getSacrificeCostFilter(); - boolean canPaySacrificeCost(); + boolean canPaySacrificeCost(Permanent permanent, UUID sourceId, UUID controllerId, Game game); void setLifeTotalCanChange(boolean lifeTotalCanChange); diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index fba36c7260..71bce77c5c 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -99,6 +99,7 @@ import mage.counters.Counter; import mage.counters.CounterType; import mage.counters.Counters; import mage.filter.FilterCard; +import mage.filter.FilterPermanent; import mage.filter.common.FilterControlledPermanent; import mage.filter.common.FilterCreatureForCombat; import mage.filter.common.FilterCreatureForCombatBlock; @@ -200,9 +201,10 @@ public abstract class PlayerImpl implements Player, Serializable { protected boolean canGainLife = true; protected boolean canLoseLife = true; protected boolean canPayLifeCost = true; - protected boolean canPaySacrificeCost = true; protected boolean loseByZeroOrLessLife = true; protected boolean canPlayCardsFromGraveyard = true; + + protected FilterPermanent sacrificeCostFilter; protected final List alternativeSourceCosts = new ArrayList<>(); @@ -299,7 +301,7 @@ public abstract class PlayerImpl implements Player, Serializable { this.inRange.addAll(player.inRange); this.userData = player.userData; this.canPayLifeCost = player.canPayLifeCost; - this.canPaySacrificeCost = player.canPaySacrificeCost; + this.sacrificeCostFilter = player.sacrificeCostFilter; this.alternativeSourceCosts.addAll(player.alternativeSourceCosts); this.storedBookmark = player.storedBookmark; @@ -378,7 +380,7 @@ public abstract class PlayerImpl implements Player, Serializable { this.inRange.clear(); this.inRange.addAll(player.getInRange()); this.canPayLifeCost = player.canPayLifeCost(); - this.canPaySacrificeCost = player.canPaySacrificeCost(); + this.sacrificeCostFilter = player.getSacrificeCostFilter() != null ? player.getSacrificeCostFilter().copy() : null; this.loseByZeroOrLessLife = player.canLoseByZeroOrLessLife(); this.canPlayCardsFromGraveyard = player.canPlayCardsFromGraveyard(); this.alternativeSourceCosts.addAll(player.getAlternativeSourceCosts()); @@ -476,7 +478,7 @@ public abstract class PlayerImpl implements Player, Serializable { this.canGainLife = true; this.canLoseLife = true; this.canPayLifeCost = true; - this.canPaySacrificeCost = true; + this.sacrificeCostFilter = null; this.loseByZeroOrLessLife = true; this.canPlayCardsFromGraveyard = false; this.topCardRevealed = false; @@ -2937,13 +2939,18 @@ public abstract class PlayerImpl implements Player, Serializable { } @Override - public boolean canPaySacrificeCost() { - return canPaySacrificeCost; + public boolean canPaySacrificeCost(Permanent permanent, UUID sourceId, UUID controllerId, Game game) { + return sacrificeCostFilter == null || !sacrificeCostFilter.match(permanent, sourceId, controllerId, game); } @Override - public void setCanPaySacrificeCost(boolean canPaySacrificeCost) { - this.canPaySacrificeCost = canPaySacrificeCost; + public void setCanPaySacrificeCostFilter(FilterPermanent filter) { + this.sacrificeCostFilter = filter; + } + + @Override + public FilterPermanent getSacrificeCostFilter() { + return sacrificeCostFilter; } @Override