Consolidate some PlayEffect logic (#8133)

* Fix potential GUI lock if game ends during RepeatEffect

* Fix casting Feast of Blood from PlayEffect without creatures in play

forge-game/src/main/java/forge/game/ability/AbilityUtils.java

@@ -3001,24 +3001,15 @@ public class AbilityUtils {
         for (SpellAbility s : list) {
             if (s.isLandAbility()) {
                 s.setActivatingPlayer(controller);
-                // CR 305.3
+                if (controller.canPlayLand(tgtCard, true, s)) {
-                if (controller.getGame().getPhaseHandler().isPlayerTurn(controller) && controller.canPlayLand(tgtCard, true, s)) {
                     sas.add(s);
                 }
             } else {
                 final Spell newSA = (Spell) s.copy(controller);
-                SpellAbilityRestriction res = new SpellAbilityRestriction();
+                newSA.getRestrictions().setZone(null);
-                // timing restrictions still apply
+                newSA.setCastFromPlayEffect(true);
-                res.setPlayerTurn(s.getRestrictions().getPlayerTurn());
+                // extra timing restrictions still apply
-                res.setOpponentTurn(s.getRestrictions().getOpponentTurn());
+                if (newSA.canPlay()) {
-                res.setPhases(s.getRestrictions().getPhases());
-                res.setZone(null);
-                newSA.setRestrictions(res);
-                // timing restrictions still apply
-                if (res.checkTimingRestrictions(tgtCard, newSA)
-                        // still need to check the other restrictions like Aftermath
-                        && res.checkOtherRestrictions(tgtCard, newSA, controller)) {
-                    newSA.setCastFromPlayEffect(true);
                     sas.add(newSA);
                 }
             }

forge-game/src/main/java/forge/game/ability/effects/ChangeZoneEffect.java

@@ -916,6 +916,10 @@ public class ChangeZoneEffect extends SpellAbilityEffect {
                 player = sa.getTargets().getFirstTargetedPlayer();
             }
 
+            if (!player.isInGame()) {
+                continue;
+            }
+
             List<ZoneType> origin = Lists.newArrayList();
             if (sa.hasParam("Origin")) {
                 origin = ZoneType.listValueOf(sa.getParam("Origin"));

forge-game/src/main/java/forge/game/ability/effects/RepeatEffect.java

@@ -66,6 +66,10 @@ public class RepeatEffect extends SpellAbilityEffect {
         final Player activator = sa.getActivatingPlayer();
         final Game game = activator.getGame();
 
+        if (game.isGameOver()) {
+            return false;
+        }
+
         if (sa.hasParam("RepeatPresent")) {
             final String repeatPresent = sa.getParam("RepeatPresent");
             String repeatCompare = sa.getParamOrDefault("RepeatCompare", "GE1");

forge-game/src/main/java/forge/game/player/Player.java

@@ -1705,22 +1705,16 @@ public class Player extends GameEntity implements Comparable<Player> {
     }
 
     public final boolean canPlayLand(final Card land, final boolean ignoreZoneAndTiming, SpellAbility landSa) {
-        if (!ignoreZoneAndTiming) {
+        // CR 305.3
-            // CR 305.3
+        if (!game.getPhaseHandler().isPlayerTurn(this)) {
-            if (!game.getPhaseHandler().isPlayerTurn(this)) {
-                return false;
-            }
-            if (!canCastSorcery() && (landSa == null || !landSa.withFlash(land, this))) {
-                return false;
-            }
-        }
-
-        // CantBeCast static abilities
-        if (StaticAbilityCantBeCast.cantPlayLandAbility(landSa, land, this)) {
             return false;
         }
 
-        if (land != null && !ignoreZoneAndTiming) {
+        if (!ignoreZoneAndTiming) {
+            if (!canCastSorcery() && (landSa == null || !landSa.withFlash(land, this))) {
+                return false;
+            }
+
             final boolean mayPlay = landSa == null ? !land.mayPlay(this).isEmpty() : landSa.getMayPlay() != null;
             if (land.getOwner() != this && !mayPlay) {
                 return false;
@@ -1732,6 +1726,10 @@ public class Player extends GameEntity implements Comparable<Player> {
             }
         }
 
+        if (StaticAbilityCantBeCast.cantPlayLandAbility(landSa, land, this)) {
+            return false;
+        }
+
         // **** Check for land play limit per turn ****
         // Dev Mode
         if (getMaxLandPlaysInfinite()) {

forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java

@@ -353,6 +353,10 @@ public class SpellAbilityRestriction extends SpellAbilityVariables {
     public final boolean checkActivatorRestrictions(final Card c, final SpellAbility sa) {
         Player activator = sa.getActivatingPlayer();
 
+        if (sa.isCastFromPlayEffect()) {
+            return true;
+        }
+
         if (sa.isSpell()) {
             // Spells should always default to "controller" but use mayPlay check.
             final CardPlayOption o = c.mayPlay(sa.getMayPlay());
@@ -615,14 +619,12 @@ public class SpellAbilityRestriction extends SpellAbilityVariables {
             return false;
         }
 
-        if (!sa.isCastFromPlayEffect()) {
+        if (!checkActivatorRestrictions(c, sa)) {
-            if (!checkTimingRestrictions(c, sa)) {
+            return false;
-                return false;
+        }
-            }
 
-            if (!checkActivatorRestrictions(c, sa)) {
+        if (!checkTimingRestrictions(c, sa)) {
-                return false;
+            return false;
-            }
         }
 
         if (!checkZoneRestrictions(c, sa)) {

forge-gui/res/cardsfolder/a/alphinaud_leveilleur.txt

@@ -1,7 +1,7 @@
 Name:Alphinaud Leveilleur
 ManaCost:3 U
 Types:Legendary Creature Elf Wizard
-PT:3/2
+PT:2/4
 K:Partner:Alisaie Leveilleur
 K:Vigilance
 T:Mode$ SpellCast | ValidCard$ Card.YouCtrl | TriggerZones$ Battlefield | Execute$ TrigDraw | ValidActivatingPlayer$ You | ActivatorThisTurnCast$ EQ2 | TriggerDescription$ Eukrasia — Whenever you cast your second spell each turn, draw a card.

forge-gui/res/cardsfolder/b/bonders_ornament.txt

@@ -2,7 +2,5 @@ Name:Bonder's Ornament
 ManaCost:3
 Types:Artifact
 A:AB$ Mana | Cost$ T | Produced$ Any | SpellDescription$ Add one mana of any color.
-A:AB$ RepeatEach | Cost$ 4 T | RepeatPlayers$ Player | RepeatSubAbility$ DBDraw | SpellDescription$ Each player who controls a permanent named Bonder's Ornament draws a card.
+A:AB$ Draw | Cost$ 4 T | Defined$ Player.controlsPermanent.namedBonder's Ornament | SpellDescription$ Each player who controls a permanent named Bonder's Ornament draws a card.
-SVar:DBDraw:DB$ Draw | NumCards$ 1 | Defined$ Player.IsRemembered | ConditionCheckSVar$ OrnCheck | ConditionSVarCompare$ GE1
-SVar:OrnCheck:PlayerCountRemembered$Valid Permanent.namedBonder's Ornament+RememberedPlayerCtrl
 Oracle:{T}: Add one mana of any color.\n{4}, {T}: Each player who controls a permanent named Bonder's Ornament draws a card.