Fix Angel of Jubilation.

The Angel now properly only restricts the sacrifice of creatures.

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() {

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) {
+    public AngelOfJubilationEffect() {
-        super(duration, Layer.PlayerEffects, SubLayer.NA, Outcome.Detriment);
+        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);
+	}
+	
 }

Mage.Sets/src/mage/sets/saviorsofkamigawa/TombOfUrami.java

@@ -102,9 +102,11 @@ class SacrificeAllLandCost extends CostImpl {
 
     @Override
     public boolean canPay(Ability ability, UUID sourceId, UUID controllerId, Game game) {
-        if (!game.getPlayer(controllerId).canPaySacrificeCost()) {
+    	for(Permanent permanent : game.getBattlefield().getActivePermanents(new FilterControlledLandPermanent(), ability.getControllerId(), game)){
+            if (!game.getPlayer(controllerId).canPaySacrificeCost(permanent, sourceId, controllerId, game)) {
                 return false;
             }
+        }
         return true;
     }
 

Mage.Tests/src/test/java/org/mage/test/cards/continuous/AngelOfJubilationTest.java

@@ -59,4 +59,121 @@ public class AngelOfJubilationTest extends CardTestPlayerBase {
         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);
+    }
+
 }

Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java

@@ -1731,13 +1731,18 @@ public class TestPlayer implements Player {
     }
 
     @Override
-    public boolean canPaySacrificeCost() {
+    public boolean canPaySacrificeCost(Permanent permanent, UUID sourceId, UUID controllerId, Game game) {
-        return computerPlayer.canPaySacrificeCost();
+        return computerPlayer.canPaySacrificeCost(permanent, sourceId, controllerId, game);
     }
     
     @Override
-    public void setCanPaySacrificeCost(boolean canPaySacrificeCost) {
+    public FilterPermanent getSacrificeCostFilter() {
-        computerPlayer.setCanPaySacrificeCost(canPaySacrificeCost);
+        return computerPlayer.getSacrificeCostFilter();
+    }
+
+    @Override
+    public void setCanPaySacrificeCostFilter(FilterPermanent permanent) {
+        computerPlayer.setCanPaySacrificeCostFilter(permanent);
     }
 
     @Override

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;
@@ -209,15 +211,18 @@ public class PlayerStub implements Player {
     }
 
 	@Override
-    public void setCanPaySacrificeCost(boolean canPaySacrificeCost) {
+	public void setCanPaySacrificeCostFilter(FilterPermanent filter) {
-
 	}
 
 	@Override
-    public boolean canPaySacrificeCost() {
+	public FilterPermanent getSacrificeCostFilter() {
-        return false;
+		return null;
 	}
 
+	@Override
+	public boolean canPaySacrificeCost(Permanent permanent, UUID sourceId, UUID controllerId, Game game) {
+		return false;
+	}
     @Override
     public void setLifeTotalCanChange(boolean lifeTotalCanChange) {
 

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;
     }
 

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

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

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<Player> {
 
     boolean canPayLifeCost();
 
-    void setCanPaySacrificeCost(boolean canPaySacrificeCost);
+    void setCanPaySacrificeCostFilter(FilterPermanent filter);
     
-    boolean canPaySacrificeCost();
+    FilterPermanent getSacrificeCostFilter();
+
+    boolean canPaySacrificeCost(Permanent permanent, UUID sourceId, UUID controllerId, Game game);
 
     void setLifeTotalCanChange(boolean lifeTotalCanChange);
 

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,10 +201,11 @@ 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> alternativeSourceCosts = new ArrayList<>();
 
     protected boolean isGameUnderControl = true;
@@ -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() {
+    public boolean canPaySacrificeCost(Permanent permanent, UUID sourceId, UUID controllerId, Game game) {
-        return canPaySacrificeCost;
+        return sacrificeCostFilter == null || !sacrificeCostFilter.match(permanent, sourceId, controllerId, game);
     }
 
     @Override
-    public void setCanPaySacrificeCost(boolean canPaySacrificeCost) {
+    public void setCanPaySacrificeCostFilter(FilterPermanent filter) {
-        this.canPaySacrificeCost = canPaySacrificeCost;
+        this.sacrificeCostFilter = filter;
+    }
+    
+    @Override
+    public FilterPermanent getSacrificeCostFilter() {
+    	return sacrificeCostFilter;
     }
 
     @Override