From 8f4eb8596dc8d7f28e2f5f4e96453ffd12b138b2 Mon Sep 17 00:00:00 2001 From: kms70847 Date: Fri, 13 Jul 2018 18:20:58 -0400 Subject: [PATCH] Don't treat a copied ability's host like it's a copy of a card Bugfix follow up to commit that solved https://git.cardforge.org/core-developers/forge/issues/615 --- .../effects/CopySpellAbilityEffect.java | 10 +- .../java/forge/game/card/CardFactory.java | 101 ++++++++++++------ 2 files changed, 73 insertions(+), 38 deletions(-) diff --git a/forge-game/src/main/java/forge/game/ability/effects/CopySpellAbilityEffect.java b/forge-game/src/main/java/forge/game/ability/effects/CopySpellAbilityEffect.java index 4d74f5e274e..116b5aae57b 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/CopySpellAbilityEffect.java +++ b/forge-game/src/main/java/forge/game/ability/effects/CopySpellAbilityEffect.java @@ -86,7 +86,7 @@ public class CopySpellAbilityEffect extends SpellAbilityEffect { String prompt = "Select " + Lang.getOrdinal(multi + 1) + " spell to copy to stack"; SpellAbility chosen = controller.getController().chooseSingleSpellForEffect(tgtSpells, sa, prompt, ImmutableMap.of()); - SpellAbility copiedSpell = CardFactory.copySpellAbilityAndSrcCard(card, chosen.getHostCard(), chosen, true); + SpellAbility copiedSpell = CardFactory.copySpellAbilityAndPossiblyHost(card, chosen.getHostCard(), chosen, true); copiedSpell.getHostCard().setController(card.getController(), card.getGame().getNextTimestamp()); copiedSpell.setActivatingPlayer(controller); copies.add(copiedSpell); @@ -117,7 +117,7 @@ public class CopySpellAbilityEffect extends SpellAbilityEffect { mayChooseNewTargets = false; for (GameEntity o : candidates) { - SpellAbility copy = CardFactory.copySpellAbilityAndSrcCard(card, chosenSA.getHostCard(), chosenSA, true); + SpellAbility copy = CardFactory.copySpellAbilityAndPossiblyHost(card, chosenSA.getHostCard(), chosenSA, true); resetFirstTargetOnCopy(copy, o, targetedSA); copies.add(copy); } @@ -143,12 +143,12 @@ public class CopySpellAbilityEffect extends SpellAbilityEffect { valid.remove(originalTarget); mayChooseNewTargets = false; for (final Card c : valid) { - SpellAbility copy = CardFactory.copySpellAbilityAndSrcCard(card, chosenSA.getHostCard(), chosenSA, true); + SpellAbility copy = CardFactory.copySpellAbilityAndPossiblyHost(card, chosenSA.getHostCard(), chosenSA, true); resetFirstTargetOnCopy(copy, c, targetedSA); copies.add(copy); } for (final Player p : players) { - SpellAbility copy = CardFactory.copySpellAbilityAndSrcCard(card, chosenSA.getHostCard(), chosenSA, true); + SpellAbility copy = CardFactory.copySpellAbilityAndPossiblyHost(card, chosenSA.getHostCard(), chosenSA, true); resetFirstTargetOnCopy(copy, p, targetedSA); copies.add(copy); } @@ -159,7 +159,7 @@ public class CopySpellAbilityEffect extends SpellAbilityEffect { "Select a spell to copy", ImmutableMap.of()); chosenSA.setActivatingPlayer(controller); for (int i = 0; i < amount; i++) { - copies.add(CardFactory.copySpellAbilityAndSrcCard(card, chosenSA.getHostCard(), chosenSA, true)); + copies.add(CardFactory.copySpellAbilityAndPossiblyHost(card, chosenSA.getHostCard(), chosenSA, true)); } } diff --git a/forge-game/src/main/java/forge/game/card/CardFactory.java b/forge-game/src/main/java/forge/game/card/CardFactory.java index d039b1c1d8a..38f66097150 100644 --- a/forge-game/src/main/java/forge/game/card/CardFactory.java +++ b/forge-game/src/main/java/forge/game/card/CardFactory.java @@ -147,22 +147,16 @@ public class CardFactory { /** *

- * copySpellontoStack. + * copySpellHost. + * Helper function for copySpellAbilityAndPossiblyHost. + * creates a copy of the card hosting the ability we want to copy. + * Updates various attributes of the card that the copy needs, + * which wouldn't ordinarily get set during a simple Card.copy() call. *

- * - * @param source - * a {@link forge.game.card.Card} object. - * @param original - * a {@link forge.game.card.Card} object. - * @param sa - * a {@link forge.game.spellability.SpellAbility} object. - * @param bCopyDetails - * a boolean. - */ - public final static SpellAbility copySpellAbilityAndSrcCard(final Card source, final Card original, final SpellAbility sa, final boolean bCopyDetails) { - //Player originalController = original.getController(); + * */ + private final static Card copySpellHost(final Card source, final Card original, final SpellAbility sa, final boolean bCopyDetails){ Player controller = sa.getActivatingPlayer(); - final Card c = (sa.isSpell() ? copyCard(original, true) : original); //copy spells, but not abilities + final Card c = copyCard(original, true); // change the color of the copy (eg: Fork) final SpellAbility sourceSA = source.getFirstSpellAbility(); @@ -178,29 +172,11 @@ public class CardFactory { c.addColor(finalColors, !sourceSA.hasParam("OverwriteColors"), c.getTimestamp()); } - + c.clearControllers(); c.setOwner(controller); c.setCopiedSpell(true); - final SpellAbility copySA; - if (sa.isTrigger()) { - copySA = getCopiedTriggeredAbility(sa); - } else { - copySA = sa.copy(c, false); - } - c.getCurrentState().setNonManaAbilities(copySA); - copySA.setCopied(true); - //remove all costs - if (!copySA.isTrigger()) { - copySA.setPayCosts(new Cost("", sa.isAbility())); - } - if (sa.getTargetRestrictions() != null) { - TargetRestrictions target = new TargetRestrictions(sa.getTargetRestrictions()); - copySA.setTargetRestrictions(target); - } - copySA.setActivatingPlayer(controller); - if (bCopyDetails) { c.setXManaCostPaid(original.getXManaCostPaid()); c.setXManaCostPaidByColor(original.getXManaCostPaidByColor()); @@ -219,6 +195,65 @@ public class CardFactory { for (OptionalCost cost : original.getOptionalCostsPaid()) { c.addOptionalCostPaid(cost); } + } + return c; + } + /** + *

+ * copySpellAbilityAndPossiblyHost. + * creates a copy of the Spell/ability `sa`, and puts it on the stack. + * if sa is a spell, that spell's host is also copied. + *

+ * + * @param source + * a {@link forge.game.card.Card} object. The card doing the copying. + * @param original + * a {@link forge.game.card.Card} object. The host of the spell or ability being copied. + * @param sa + * a {@link forge.game.spellability.SpellAbility} object. The spell or ability being copied. + * @param bCopyDetails + * a boolean. + */ + public final static SpellAbility copySpellAbilityAndPossiblyHost(final Card source, final Card original, final SpellAbility sa, final boolean bCopyDetails) { + Player controller = sa.getActivatingPlayer(); + + //it is only necessary to copy the host card if the SpellAbility is a spell, not an ability + final Card c; + if (sa.isSpell()){ + c = copySpellHost(source, original, sa, bCopyDetails); + } + else { + c = original; + } + + final SpellAbility copySA; + if (sa.isTrigger()) { + copySA = getCopiedTriggeredAbility(sa); + } else { + copySA = sa.copy(c, false); + } + + if (sa.isSpell()){ + //only update c's abilities if c is a copy. + //(it would be nice to move this into `copySpellHost`, + // so all the c-mutating code is together in one place. + // but copySA doesn't exist until after `copySpellHost` finishes executing, + // so it's hard to resolve that dependency.) + c.getCurrentState().setNonManaAbilities(copySA); + } + + copySA.setCopied(true); + //remove all costs + if (!copySA.isTrigger()) { + copySA.setPayCosts(new Cost("", sa.isAbility())); + } + if (sa.getTargetRestrictions() != null) { + TargetRestrictions target = new TargetRestrictions(sa.getTargetRestrictions()); + copySA.setTargetRestrictions(target); + } + copySA.setActivatingPlayer(controller); + + if (bCopyDetails) { copySA.setPaidHash(sa.getPaidHash()); } return copySA;