From 514d40aefdaca62d47b32b5a7fa1024203d06047 Mon Sep 17 00:00:00 2001 From: tool4ever Date: Sun, 20 Jul 2025 10:35:03 +0000 Subject: [PATCH] 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 --- .../java/forge/game/ability/AbilityUtils.java | 19 ++++----------- .../ability/effects/ChangeZoneEffect.java | 4 ++++ .../game/ability/effects/RepeatEffect.java | 4 ++++ .../main/java/forge/game/player/Player.java | 24 +++++++++---------- .../spellability/SpellAbilityRestriction.java | 16 +++++++------ .../cardsfolder/a/alphinaud_leveilleur.txt | 2 +- .../res/cardsfolder/b/bonders_ornament.txt | 4 +--- 7 files changed, 35 insertions(+), 38 deletions(-) diff --git a/forge-game/src/main/java/forge/game/ability/AbilityUtils.java b/forge-game/src/main/java/forge/game/ability/AbilityUtils.java index 28f26ae520a..eea56d276ea 100644 --- a/forge-game/src/main/java/forge/game/ability/AbilityUtils.java +++ b/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.getGame().getPhaseHandler().isPlayerTurn(controller) && controller.canPlayLand(tgtCard, true, s)) { + if (controller.canPlayLand(tgtCard, true, s)) { sas.add(s); } } else { final Spell newSA = (Spell) s.copy(controller); - SpellAbilityRestriction res = new SpellAbilityRestriction(); - // timing restrictions still apply - res.setPlayerTurn(s.getRestrictions().getPlayerTurn()); - res.setOpponentTurn(s.getRestrictions().getOpponentTurn()); - 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); + newSA.getRestrictions().setZone(null); + newSA.setCastFromPlayEffect(true); + // extra timing restrictions still apply + if (newSA.canPlay()) { sas.add(newSA); } } diff --git a/forge-game/src/main/java/forge/game/ability/effects/ChangeZoneEffect.java b/forge-game/src/main/java/forge/game/ability/effects/ChangeZoneEffect.java index 6944c6fdaf0..2af1066b637 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/ChangeZoneEffect.java +++ b/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 origin = Lists.newArrayList(); if (sa.hasParam("Origin")) { origin = ZoneType.listValueOf(sa.getParam("Origin")); diff --git a/forge-game/src/main/java/forge/game/ability/effects/RepeatEffect.java b/forge-game/src/main/java/forge/game/ability/effects/RepeatEffect.java index 4ccb9ca01d9..c6b4f79b956 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/RepeatEffect.java +++ b/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"); diff --git a/forge-game/src/main/java/forge/game/player/Player.java b/forge-game/src/main/java/forge/game/player/Player.java index 16cc3e9c99f..cc07564aa34 100644 --- a/forge-game/src/main/java/forge/game/player/Player.java +++ b/forge-game/src/main/java/forge/game/player/Player.java @@ -1705,22 +1705,16 @@ public class Player extends GameEntity implements Comparable { } public final boolean canPlayLand(final Card land, final boolean ignoreZoneAndTiming, SpellAbility landSa) { - if (!ignoreZoneAndTiming) { - // CR 305.3 - 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)) { + // CR 305.3 + if (!game.getPhaseHandler().isPlayerTurn(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 { } } + if (StaticAbilityCantBeCast.cantPlayLandAbility(landSa, land, this)) { + return false; + } + // **** Check for land play limit per turn **** // Dev Mode if (getMaxLandPlaysInfinite()) { diff --git a/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java b/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java index 3a425af9465..615d27406c6 100644 --- a/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java +++ b/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 (!checkTimingRestrictions(c, sa)) { - return false; - } + if (!checkActivatorRestrictions(c, sa)) { + return false; + } - if (!checkActivatorRestrictions(c, sa)) { - return false; - } + if (!checkTimingRestrictions(c, sa)) { + return false; } if (!checkZoneRestrictions(c, sa)) { diff --git a/forge-gui/res/cardsfolder/a/alphinaud_leveilleur.txt b/forge-gui/res/cardsfolder/a/alphinaud_leveilleur.txt index bd923bbb42b..36f71d5faf1 100644 --- a/forge-gui/res/cardsfolder/a/alphinaud_leveilleur.txt +++ b/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. diff --git a/forge-gui/res/cardsfolder/b/bonders_ornament.txt b/forge-gui/res/cardsfolder/b/bonders_ornament.txt index 2c41a392166..3fa9ece22e5 100644 --- a/forge-gui/res/cardsfolder/b/bonders_ornament.txt +++ b/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. -SVar:DBDraw:DB$ Draw | NumCards$ 1 | Defined$ Player.IsRemembered | ConditionCheckSVar$ OrnCheck | ConditionSVarCompare$ GE1 -SVar:OrnCheck:PlayerCountRemembered$Valid Permanent.namedBonder's Ornament+RememberedPlayerCtrl +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. Oracle:{T}: Add one mana of any color.\n{4}, {T}: Each player who controls a permanent named Bonder's Ornament draws a card.