From ad2039d5a13fb26fd298e0fd2039a7af3d02d4fe Mon Sep 17 00:00:00 2001 From: tool4ever Date: Mon, 29 Jan 2024 12:50:43 +0100 Subject: [PATCH] Fix crash (#4610) * Fix crash * Move check earlier so Full Control doesn't ask for non-matches * Restore fix * Clean up duration checks --- .../main/java/forge/ai/ability/PumpAi.java | 1 + .../java/forge/game/ability/AbilityUtils.java | 2 +- .../game/ability/SpellAbilityEffect.java | 21 +++++++++++++++++++ .../ability/effects/AlterAttributeEffect.java | 10 ++++----- .../game/ability/effects/AnimateEffect.java | 7 +------ .../ability/effects/ChangeZoneAllEffect.java | 3 +-- .../ability/effects/ChangeZoneEffect.java | 3 +-- .../game/ability/effects/CloneEffect.java | 7 +++---- .../game/ability/effects/EffectEffect.java | 9 +------- .../game/ability/effects/PumpAllEffect.java | 4 ++++ .../game/ability/effects/PumpEffect.java | 21 ++++--------------- .../java/forge/game/cost/CostAdjustment.java | 6 +----- .../java/forge/game/cost/CostPartMana.java | 2 +- .../res/cardsfolder/b/breath_of_fury.txt | 14 ++++++------- .../t/the_blackstaff_of_waterdeep.txt | 5 +---- .../res/cardsfolder/t/thran_weaponry.txt | 5 +---- 16 files changed, 52 insertions(+), 68 deletions(-) diff --git a/forge-ai/src/main/java/forge/ai/ability/PumpAi.java b/forge-ai/src/main/java/forge/ai/ability/PumpAi.java index af616ad677a..e499cf49e74 100644 --- a/forge-ai/src/main/java/forge/ai/ability/PumpAi.java +++ b/forge-ai/src/main/java/forge/ai/ability/PumpAi.java @@ -313,6 +313,7 @@ public class PumpAi extends PumpAiBase { attack = root.getXManaCostPaid(); } } else { + // TODO add Double attack = AbilityUtils.calculateAmount(sa.getHostCard(), numAttack, sa); if (numAttack.contains("X") && sa.getSVar("X").equals("Count$CardsInYourHand") && source.isInZone(ZoneType.Hand)) { attack--; // the card will be spent casting the spell, so actual power is 1 less 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 a23263bb582..c91b9340faf 100644 --- a/forge-game/src/main/java/forge/game/ability/AbilityUtils.java +++ b/forge-game/src/main/java/forge/game/ability/AbilityUtils.java @@ -2733,7 +2733,7 @@ public class AbilityUtils { // Count$ThisTurnEntered [from ] if (sq[0].startsWith("ThisTurnEntered") || sq[0].startsWith("LastTurnEntered")) { - final String[] workingCopy = paidparts[0].split("_"); + final String[] workingCopy = paidparts[0].split("_", 5); ZoneType destination = ZoneType.smartValueOf(workingCopy[1]); final boolean hasFrom = workingCopy[2].equals("from"); ZoneType origin = hasFrom ? ZoneType.smartValueOf(workingCopy[3]) : null; diff --git a/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java b/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java index 941ed5d1cac..5f50b37c7f1 100644 --- a/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java +++ b/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java @@ -930,6 +930,27 @@ public abstract class SpellAbilityEffect { } } + protected static boolean checkValidDuration(String duration, SpellAbility sa) { + if (duration == null) { + return true; + } + Card hostCard = sa.getHostCard(); + + //if host is not on the battlefield don't apply + // Suspend should does Affect the Stack + if ((duration.startsWith("UntilHostLeavesPlay") || "UntilLoseControlOfHost".equals(duration) || "UntilUntaps".equals(duration)) + && !(hostCard.isInPlay() || hostCard.isInZone(ZoneType.Stack))) { + return false; + } + if ("UntilLoseControlOfHost".equals(duration) && hostCard.getController() != sa.getActivatingPlayer()) { + return false; + } + if ("UntilUntaps".equals(duration) && !hostCard.isTapped()) { + return false; + } + return true; + } + public static Player getNewChooser(final SpellAbility sa, final Player activator, final Player loser) { // CR 800.4g final PlayerCollection options; diff --git a/forge-game/src/main/java/forge/game/ability/effects/AlterAttributeEffect.java b/forge-game/src/main/java/forge/game/ability/effects/AlterAttributeEffect.java index fd196e14ca8..e1eba2e289b 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/AlterAttributeEffect.java +++ b/forge-game/src/main/java/forge/game/ability/effects/AlterAttributeEffect.java @@ -1,6 +1,5 @@ package forge.game.ability.effects; -import com.google.common.collect.Lists; import forge.game.ability.SpellAbilityEffect; import forge.game.card.Card; import forge.game.card.CardCollection; @@ -8,13 +7,12 @@ import forge.game.spellability.SpellAbility; import forge.util.Lang; import forge.util.TextUtil; -import java.util.ArrayList; public class AlterAttributeEffect extends SpellAbilityEffect { @Override public void resolve(SpellAbility sa) { boolean activate = Boolean.valueOf(sa.getParamOrDefault("Activate", "true")); - ArrayList attributes = Lists.newArrayList(sa.getParam("Attributes").split(",")); + String[] attributes = sa.getParam("Attributes").split(","); CardCollection defined = getDefinedCardsOrTargeted(sa, "Defined"); if (sa.hasParam("Optional")) { @@ -28,9 +26,9 @@ public class AlterAttributeEffect extends SpellAbilityEffect { } } - for(Card c : defined) { - for(String attr : attributes) { - switch(attr.trim()) { + for (Card c : defined) { + for (String attr : attributes) { + switch (attr.trim()) { case "Solve": case "Solved": c.setSolved(activate); diff --git a/forge-game/src/main/java/forge/game/ability/effects/AnimateEffect.java b/forge-game/src/main/java/forge/game/ability/effects/AnimateEffect.java index 89f53fa0e55..cd101951483 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/AnimateEffect.java +++ b/forge-game/src/main/java/forge/game/ability/effects/AnimateEffect.java @@ -31,12 +31,7 @@ public class AnimateEffect extends AnimateEffectBase { String animateRemembered = null; String animateImprinted = null; - //if host is not on the battlefield don't apply - if (("UntilHostLeavesPlay".equals(duration) || "UntilLoseControlOfHost".equals(duration)) - && !source.isInPlay()) { - return; - } - if ("UntilLoseControlOfHost".equals(duration) && source.getController() != sa.getActivatingPlayer()) { + if (!checkValidDuration(duration, sa)) { return; } diff --git a/forge-game/src/main/java/forge/game/ability/effects/ChangeZoneAllEffect.java b/forge-game/src/main/java/forge/game/ability/effects/ChangeZoneAllEffect.java index cf64c876b2a..81cf557bce8 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/ChangeZoneAllEffect.java +++ b/forge-game/src/main/java/forge/game/ability/effects/ChangeZoneAllEffect.java @@ -45,8 +45,7 @@ public class ChangeZoneAllEffect extends SpellAbilityEffect { public void resolve(SpellAbility sa) { final Card source = sa.getHostCard(); - //if host is not on the battlefield don't apply - if ("UntilHostLeavesPlay".equals(sa.getParam("Duration")) && !source.isInPlay()) { + if (!checkValidDuration(sa.getParam("Duration"), sa)) { return; } 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 7a769769343..f4a1996c13b 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 @@ -428,8 +428,7 @@ public class ChangeZoneEffect extends SpellAbilityEffect { @Override public void resolve(SpellAbility sa) { - //if host is not on the battlefield don't apply - if ("UntilHostLeavesPlay".equals(sa.getParam("Duration")) && !sa.getHostCard().isInPlay()) { + if (!checkValidDuration(sa.getParam("Duration"), sa)) { return; } diff --git a/forge-game/src/main/java/forge/game/ability/effects/CloneEffect.java b/forge-game/src/main/java/forge/game/ability/effects/CloneEffect.java index 44ab6858356..58c5711dff3 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/CloneEffect.java +++ b/forge-game/src/main/java/forge/game/ability/effects/CloneEffect.java @@ -128,10 +128,9 @@ public class CloneEffect extends SpellAbilityEffect { for (Card tgtCard : cloneTargets) { game.getTriggerHandler().clearActiveTriggers(tgtCard, null); - if (sa.hasParam("CloneZone")) { - if (!tgtCard.isInZone(ZoneType.smartValueOf(sa.getParam("CloneZone")))) { - continue; - } + if (sa.hasParam("CloneZone") && + !tgtCard.isInZone(ZoneType.smartValueOf(sa.getParam("CloneZone")))) { + continue; } if (tgtCard.isPhasedOut()) { diff --git a/forge-game/src/main/java/forge/game/ability/effects/EffectEffect.java b/forge-game/src/main/java/forge/game/ability/effects/EffectEffect.java index f7a945ee86c..e32fe03e4dc 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/EffectEffect.java +++ b/forge-game/src/main/java/forge/game/ability/effects/EffectEffect.java @@ -55,14 +55,7 @@ public class EffectEffect extends SpellAbilityEffect { String noteCounterDefined = null; final String duration = sa.getParam("Duration"); - if (((duration != null && duration.startsWith("UntilHostLeavesPlay")) || "UntilLoseControlOfHost".equals(duration) || "UntilUntaps".equals(duration)) - && !(hostCard.isInPlay() || hostCard.isInZone(ZoneType.Stack))) { - return; - } - if ("UntilLoseControlOfHost".equals(duration) && hostCard.getController() != sa.getActivatingPlayer()) { - return; - } - if ("UntilUntaps".equals(duration) && !hostCard.isTapped()) { + if (!checkValidDuration(duration, sa)) { return; } diff --git a/forge-game/src/main/java/forge/game/ability/effects/PumpAllEffect.java b/forge-game/src/main/java/forge/game/ability/effects/PumpAllEffect.java index e90f3ae22c5..46a029edeaf 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/PumpAllEffect.java +++ b/forge-game/src/main/java/forge/game/ability/effects/PumpAllEffect.java @@ -128,6 +128,10 @@ public class PumpAllEffect extends SpellAbilityEffect { final List affectedZones = Lists.newArrayList(); final Game game = sa.getActivatingPlayer().getGame(); + if (!checkValidDuration(sa.getParam("Duration"), sa)) { + return; + } + if (sa.hasParam("PumpZone")) { affectedZones.addAll(ZoneType.listValueOf(sa.getParam("PumpZone"))); } else { diff --git a/forge-game/src/main/java/forge/game/ability/effects/PumpEffect.java b/forge-game/src/main/java/forge/game/ability/effects/PumpEffect.java index 37e39e5ac22..3b8f804bbdf 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/PumpEffect.java +++ b/forge-game/src/main/java/forge/game/ability/effects/PumpEffect.java @@ -44,16 +44,6 @@ public class PumpEffect extends SpellAbilityEffect { final String duration = sa.getParam("Duration"); final boolean perpetual = ("Perpetual").equals(duration); - //if host is not on the battlefield don't apply - // Suspend should does Affect the Stack - if (((duration != null && duration.startsWith("UntilHostLeavesPlay")) || "UntilLoseControlOfHost".equals(duration)) - && !(host.isInPlay() || host.isInZone(ZoneType.Stack))) { - return; - } - if ("UntilLoseControlOfHost".equals(duration) && host.getController() != sa.getActivatingPlayer()) { - return; - } - // do Game Check there in case of LKI final Card gameCard = game.getCardState(applyTo, null); if (gameCard == null || !applyTo.equalsWithTimestamp(gameCard)) { @@ -152,13 +142,6 @@ public class PumpEffect extends SpellAbilityEffect { final Card host = sa.getHostCard(); final String duration = sa.getParam("Duration"); - //if host is not on the battlefield don't apply - // Suspend should does Affect the Stack - if (((duration != null && duration.startsWith("UntilHostLeavesPlay")) || "UntilLoseControlOfHost".equals(duration)) - && !(host.isInPlay() || host.isInZone(ZoneType.Stack))) { - return; - } - if (!keywords.isEmpty()) { p.addChangedKeywords(keywords, ImmutableList.of(), timestamp, 0); } @@ -294,6 +277,10 @@ public class PumpEffect extends SpellAbilityEffect { @Override public void resolve(final SpellAbility sa) { + if (!checkValidDuration(sa.getParam("Duration"), sa)) { + return; + } + final Player activator = sa.getActivatingPlayer(); final Game game = activator.getGame(); final Card host = sa.getHostCard(); diff --git a/forge-game/src/main/java/forge/game/cost/CostAdjustment.java b/forge-game/src/main/java/forge/game/cost/CostAdjustment.java index a3bed20e557..762ab422908 100644 --- a/forge-game/src/main/java/forge/game/cost/CostAdjustment.java +++ b/forge-game/src/main/java/forge/game/cost/CostAdjustment.java @@ -200,7 +200,7 @@ public class CostAdjustment { // Sort abilities to apply them in proper order for (Card c : cardsOnBattlefield) { for (final StaticAbility stAb : c.getStaticAbilities()) { - if (stAb.checkMode("ReduceCost")) { + if (stAb.checkMode("ReduceCost") && checkRequirement(sa, stAb)) { reduceAbilities.add(stAb); } else if (stAb.checkMode("SetCost")) { @@ -401,10 +401,6 @@ public class CostAdjustment { final Card card = sa.getHostCard(); final String amount = staticAbility.getParam("Amount"); - if (!checkRequirement(sa, staticAbility)) { - return 0; - } - int value; if ("AffectedX".equals(amount)) { value = AbilityUtils.calculateAmount(card, amount, staticAbility); diff --git a/forge-game/src/main/java/forge/game/cost/CostPartMana.java b/forge-game/src/main/java/forge/game/cost/CostPartMana.java index 49a4275db96..8aa399dc86d 100644 --- a/forge-game/src/main/java/forge/game/cost/CostPartMana.java +++ b/forge-game/src/main/java/forge/game/cost/CostPartMana.java @@ -138,7 +138,7 @@ public class CostPartMana extends CostPart { if (isCostPayAnyNumberOfTimes) { int timesToPay = AbilityUtils.calculateAmount(sa.getHostCard(), sa.getSVar("NumTimes"), sa); if (timesToPay == 0) { - return null; + return ManaCost.NO_COST; } ManaCostBeingPaid totalMana = new ManaCostBeingPaid(getMana()); for (int i = 1; i < timesToPay; i++) { diff --git a/forge-gui/res/cardsfolder/b/breath_of_fury.txt b/forge-gui/res/cardsfolder/b/breath_of_fury.txt index f0002b0e021..7bf1f8a78c6 100644 --- a/forge-gui/res/cardsfolder/b/breath_of_fury.txt +++ b/forge-gui/res/cardsfolder/b/breath_of_fury.txt @@ -3,13 +3,11 @@ ManaCost:2 R R Types:Enchantment Aura K:Enchant creature you control A:SP$ Attach | Cost$ 2 R R | ValidTgts$ Creature.YouCtrl | TgtPrompt$ Select target creature you control to attach Breath of Fury to | AILogic$ Pump -T:Mode$ DamageDone | ValidSource$ Card.AttachedBy | ValidTarget$ Player | CombatDamage$ True | Execute$ NewFuriousLeader | TriggerZones$ Battlefield | TriggerDescription$ When enchanted creature deals combat damage to a player, sacrifice it and attach CARDNAME to a creature you control. If you do, untap all creatures you control and after this phase, there is an additional combat phase. -SVar:NewFuriousLeader:DB$ ChooseCard | Defined$ You | Amount$ 1 | Choices$ Creature.NotEnchantedBy+YouCtrl+CanBeEnchantedBy | ChoiceTitle$ Choose another creature you control to attach Breath of Fury to | SubAbility$ TrigSacrifice -SVar:TrigSacrifice:DB$ SacrificeAll | ValidCards$ Card.EnchantedBy | SubAbility$ StillFurious | RememberSacrificed$ True -SVar:StillFurious:DB$ Attach | Defined$ ChosenCard | ConditionCheckSVar$ WasSacced | ConditionSVarCompare$ EQ1 | SubAbility$ Cleanup -SVar:Cleanup:DB$ Cleanup | ClearRemembered$ True | SubAbility$ CatchBreath -SVar:CatchBreath:DB$ UntapAll | ValidCards$ Creature.YouCtrl | SubAbility$ TheFuryContinues -SVar:TheFuryContinues:DB$ AddPhase | ExtraPhase$ Combat | AfterPhase$ EndCombat -SVar:WasSacced:Remembered$Amount +T:Mode$ DamageDone | ValidSource$ Card.AttachedBy | ValidTarget$ Player | CombatDamage$ True | Execute$ TrigSacrifice | TriggerZones$ Battlefield | TriggerDescription$ When enchanted creature deals combat damage to a player, sacrifice it and attach CARDNAME to a creature you control. If you do, untap all creatures you control and after this phase, there is an additional combat phase. +SVar:TrigSacrifice:DB$ SacrificeAll | ValidCards$ Card.EnchantedBy | SubAbility$ StillFurious +SVar:StillFurious:DB$ Attach | Object$ Self | Choices$ Creature.YouCtrl | ChoiceTitle$ Choose a creature you control to attach Breath of Fury to | RememberAttached$ True | SubAbility$ CatchBreath +SVar:CatchBreath:DB$ UntapAll | ValidCards$ Creature.YouCtrl | SubAbility$ TheFuryContinues | ConditionDefined$ Rememembered | ConditionPresent$ Card +SVar:TheFuryContinues:DB$ AddPhase | ExtraPhase$ Combat | AfterPhase$ EndCombat | ConditionDefined$ Remembered | ConditionPresent$ Card | SubAbility$ Cleanup +SVar:Cleanup:DB$ Cleanup | ClearRemembered$ True AI:RemoveDeck:All Oracle:Enchant creature you control\nWhen enchanted creature deals combat damage to a player, sacrifice it and attach Breath of Fury to a creature you control. If you do, untap all creatures you control and after this phase, there is an additional combat phase. diff --git a/forge-gui/res/cardsfolder/t/the_blackstaff_of_waterdeep.txt b/forge-gui/res/cardsfolder/t/the_blackstaff_of_waterdeep.txt index 5206270cb1c..0fd361f9573 100644 --- a/forge-gui/res/cardsfolder/t/the_blackstaff_of_waterdeep.txt +++ b/forge-gui/res/cardsfolder/t/the_blackstaff_of_waterdeep.txt @@ -2,8 +2,5 @@ Name:The Blackstaff of Waterdeep ManaCost:U Types:Legendary Artifact K:You may choose not to untap CARDNAME during your untap step. -A:AB$ Pump | Cost$ 1 U T | ValidTgts$ Artifact.Other+nonToken+YouCtrl | TgtPrompt$ Select another target nontoken artifact you control | RememberTargets$ True | AILogic$ ContinuousBonus | PrecostDesc$ Animate Walking Statue — | SpellDescription$ Another target nontoken artifact you control becomes a 4/4 artifact creature for as long as CARDNAME remains tapped. Activate only as a sorcery. | SorcerySpeed$ True | StackDescription$ SpellDescription -S:Mode$ Continuous | Affected$ Artifact.IsRemembered | SetPower$ 4 | SetToughness$ 4 | AddType$ Creature -T:Mode$ Untaps | ValidCard$ Card.Self | TriggerZones$ Battlefield | Execute$ ClearRemembered | Static$ True -SVar:ClearRemembered:DB$ Cleanup | ClearRemembered$ True +A:AB$ Animate | Cost$ 1 U T | ValidTgts$ Artifact.Other+nonToken+YouCtrl | TgtPrompt$ Select another target nontoken artifact you control | Duration$ UntilUntaps | Power$ 4 | Toughness$ 4 | Types$ Creature | PrecostDesc$ Animate Walking Statue — | SpellDescription$ Another target nontoken artifact you control becomes a 4/4 artifact creature for as long as CARDNAME remains tapped. Activate only as a sorcery. | SorcerySpeed$ True | StackDescription$ SpellDescription Oracle:You may choose not to untap The Blackstaff of Waterdeep during your untap step.\nAnimate Walking Statue — {1}{U}, {T}: Another target nontoken artifact you control becomes a 4/4 artifact creature for as long as The Blackstaff of Waterdeep remains tapped. Activate only as a sorcery. diff --git a/forge-gui/res/cardsfolder/t/thran_weaponry.txt b/forge-gui/res/cardsfolder/t/thran_weaponry.txt index f1f3ec1f883..1e964d3e8ad 100644 --- a/forge-gui/res/cardsfolder/t/thran_weaponry.txt +++ b/forge-gui/res/cardsfolder/t/thran_weaponry.txt @@ -3,9 +3,6 @@ ManaCost:4 Types:Artifact K:Echo:4 K:You may choose not to untap CARDNAME during your untap step. -A:AB$ PumpAll | Cost$ 2 T | ValidCards$ Creature | RememberPumped$ True | SpellDescription$ All creatures get +2/+2 for as long as CARDNAME remains tapped. -S:Mode$ Continuous | Affected$ Creature.IsRemembered | AddPower$ 2 | AddToughness$ 2 -T:Mode$ Untaps | ValidCard$ Card.Self | TriggerZones$ Battlefield | Execute$ ClearRemembered | Static$ True -SVar:ClearRemembered:DB$ Cleanup | ClearRemembered$ True +A:AB$ PumpAll | Cost$ 2 T | ValidCards$ Creature | Duration$ UntilUntaps | NumAtt$ 2 | NumDef$ 2 | SpellDescription$ All creatures get +2/+2 for as long as CARDNAME remains tapped. AI:RemoveDeck:All Oracle:Echo {4} (At the beginning of your upkeep, if this came under your control since the beginning of your last upkeep, sacrifice it unless you pay its echo cost.)\nYou may choose not to untap Thran Weaponry during your untap step.\n{2}, {T}: All creatures get +2/+2 for as long as Thran Weaponry remains tapped.