From ec4c79e0e0d9de0216c882b4e073ce1ed5b36a6d Mon Sep 17 00:00:00 2001 From: ssk97 Date: Sun, 12 Nov 2023 16:57:39 -0800 Subject: [PATCH] Fix copying subabilities to no longer duplicate them (#11399) * Fix Subability copy bug (fix #10526 ) * Cards which copy abilities of other cards should not copy subabilities. * Enable previously-failing tests * Find more addAbility that should be done without subabilities * Add documentation to addAbility function * Add warning about not using basic addAbility when copying from a source * Invert withSubabilities to fromExistingObject --- .../src/mage/cards/a/AgathasSoulCauldron.java | 2 +- Mage.Sets/src/mage/cards/d/DarkImpostor.java | 2 +- .../src/mage/cards/d/DranaAndLinvala.java | 2 +- .../src/mage/cards/e/ExperimentKraj.java | 2 +- Mage.Sets/src/mage/cards/h/HavengulLich.java | 3 +-- .../src/mage/cards/k/KasminaEnigmaSage.java | 2 +- .../src/mage/cards/m/MairsilThePretender.java | 2 +- .../src/mage/cards/m/ManascapeRefractor.java | 2 +- .../mage/cards/m/MetamorphicAlteration.java | 2 +- .../src/mage/cards/m/MirranSafehouse.java | 2 +- Mage.Sets/src/mage/cards/m/MyrWelder.java | 2 +- Mage.Sets/src/mage/cards/n/NecroticOoze.java | 2 +- .../src/mage/cards/n/NicolBolasDragonGod.java | 2 +- .../src/mage/cards/p/PatchworkCrawler.java | 2 +- .../mage/cards/q/QuicksilverElemental.java | 1 + .../src/mage/cards/r/RobaranMercenaries.java | 2 +- Mage.Sets/src/mage/cards/s/SchemingFence.java | 2 +- .../mage/cards/s/SharkeyTyrantOfTheShire.java | 2 +- .../mage/cards/t/TheBookOfVileDarkness.java | 2 +- .../src/mage/cards/t/TrazynTheInfinite.java | 2 +- .../mage/cards/v/VolrathsShapeshifter.java | 2 +- .../cards/abilities/keywords/SquadTest.java | 2 -- .../cards/copy/CopyPermanentSpellTest.java | 2 -- .../src/main/java/mage/abilities/Ability.java | 2 ++ .../abilities/effects/common/CopyEffect.java | 4 +-- .../effects/common/CopyTokenEffect.java | 2 +- ...GainActivatedAbilitiesOfTopCardEffect.java | 2 +- .../continuous/BecomesCreatureAllEffect.java | 2 +- .../BecomesCreatureAttachedEffect.java | 2 +- .../BecomesCreatureSourceEffect.java | 2 +- .../BecomesCreatureTargetEffect.java | 2 +- .../mage/abilities/keyword/CrewAbility.java | 2 ++ .../abilities/keyword/TransformAbility.java | 2 +- .../java/mage/game/permanent/Permanent.java | 1 + .../mage/game/permanent/PermanentImpl.java | 25 ++++++++++++++++++- .../mage/game/permanent/PermanentToken.java | 3 ++- .../java/mage/game/permanent/token/Token.java | 1 + .../mage/game/permanent/token/TokenImpl.java | 20 ++++++++++++++- .../util/functions/CopyTokenFunction.java | 5 ++-- 39 files changed, 83 insertions(+), 40 deletions(-) diff --git a/Mage.Sets/src/mage/cards/a/AgathasSoulCauldron.java b/Mage.Sets/src/mage/cards/a/AgathasSoulCauldron.java index c7d6367a597..42de3ee1e46 100644 --- a/Mage.Sets/src/mage/cards/a/AgathasSoulCauldron.java +++ b/Mage.Sets/src/mage/cards/a/AgathasSoulCauldron.java @@ -139,7 +139,7 @@ class AgathasSoulCauldronAbilityEffect extends ContinuousEffectImpl { } for (Permanent permanent : game.getBattlefield().getActivePermanents(filter, source.getControllerId(), source, game)) { for (Ability ability : abilities) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } } return true; diff --git a/Mage.Sets/src/mage/cards/d/DarkImpostor.java b/Mage.Sets/src/mage/cards/d/DarkImpostor.java index d4bcdcb049a..43b586090eb 100644 --- a/Mage.Sets/src/mage/cards/d/DarkImpostor.java +++ b/Mage.Sets/src/mage/cards/d/DarkImpostor.java @@ -110,7 +110,7 @@ class DarkImpostorContinuousEffect extends ContinuousEffectImpl { for (Card card : exileZone.getCards(StaticFilters.FILTER_CARD_CREATURE, game)) { for (Ability ability : card.getAbilities(game)) { if (ability instanceof ActivatedAbility) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } } } diff --git a/Mage.Sets/src/mage/cards/d/DranaAndLinvala.java b/Mage.Sets/src/mage/cards/d/DranaAndLinvala.java index c9757496c10..f18f8686b84 100644 --- a/Mage.Sets/src/mage/cards/d/DranaAndLinvala.java +++ b/Mage.Sets/src/mage/cards/d/DranaAndLinvala.java @@ -123,7 +123,7 @@ class DranaAndLinvalaGainAbilitiesEffect extends ContinuousEffectImpl { .filter(ability -> ability.getAbilityType() == AbilityType.ACTIVATED || ability.getAbilityType() == AbilityType.MANA) .collect(Collectors.toList())) { - Ability addedAbility = perm.addAbility(ability, source.getSourceId(), game); + Ability addedAbility = perm.addAbility(ability, source.getSourceId(), game, true); if (addedAbility != null) { addedAbility.getEffects().setValue("dranaLinvalaFlag", true); } diff --git a/Mage.Sets/src/mage/cards/e/ExperimentKraj.java b/Mage.Sets/src/mage/cards/e/ExperimentKraj.java index 394ff4d0322..081acc136bf 100644 --- a/Mage.Sets/src/mage/cards/e/ExperimentKraj.java +++ b/Mage.Sets/src/mage/cards/e/ExperimentKraj.java @@ -79,7 +79,7 @@ class ExperimentKrajEffect extends ContinuousEffectImpl { for (Permanent creature :game.getState().getBattlefield().getActivePermanents(filter, source.getControllerId(), source, game)){ for (Ability ability: creature.getAbilities()) { if (ability instanceof ActivatedAbility) { - perm.addAbility(ability, source.getSourceId(), game); + perm.addAbility(ability, source.getSourceId(), game, true); } } } diff --git a/Mage.Sets/src/mage/cards/h/HavengulLich.java b/Mage.Sets/src/mage/cards/h/HavengulLich.java index 3c588d26cfb..b3dd771454b 100644 --- a/Mage.Sets/src/mage/cards/h/HavengulLich.java +++ b/Mage.Sets/src/mage/cards/h/HavengulLich.java @@ -25,7 +25,6 @@ import mage.filter.FilterCard; import mage.filter.common.FilterCreatureCard; import mage.game.Game; import mage.game.events.GameEvent; -import mage.game.events.GameEvent.EventType; import mage.game.permanent.Permanent; import mage.target.common.TargetCardInGraveyard; @@ -188,7 +187,7 @@ class HavengulLichEffect extends ContinuousEffectImpl { Card card = game.getCard(cardId); if (permanent != null && card != null) { for (ActivatedAbility ability : card.getAbilities(game).getActivatedAbilities(Zone.BATTLEFIELD)) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } } return false; diff --git a/Mage.Sets/src/mage/cards/k/KasminaEnigmaSage.java b/Mage.Sets/src/mage/cards/k/KasminaEnigmaSage.java index fce705fcb43..0b359926d35 100644 --- a/Mage.Sets/src/mage/cards/k/KasminaEnigmaSage.java +++ b/Mage.Sets/src/mage/cards/k/KasminaEnigmaSage.java @@ -93,7 +93,7 @@ class KasminaEnigmaSageGainAbilitiesEffect extends ContinuousEffectImpl { continue; } for (Ability ability : loyaltyAbilities) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } } return true; diff --git a/Mage.Sets/src/mage/cards/m/MairsilThePretender.java b/Mage.Sets/src/mage/cards/m/MairsilThePretender.java index 73b9f7d3e76..def29ce6501 100644 --- a/Mage.Sets/src/mage/cards/m/MairsilThePretender.java +++ b/Mage.Sets/src/mage/cards/m/MairsilThePretender.java @@ -141,7 +141,7 @@ class MairsilThePretenderGainAbilitiesEffect extends ContinuousEffectImpl { if (ability instanceof ActivatedAbility) { ActivatedAbility copyAbility = (ActivatedAbility) ability.copy(); copyAbility.setMaxActivationsPerTurn(1); - perm.addAbility(copyAbility, source.getSourceId(), game); + perm.addAbility(copyAbility, source.getSourceId(), game, true); } } } diff --git a/Mage.Sets/src/mage/cards/m/ManascapeRefractor.java b/Mage.Sets/src/mage/cards/m/ManascapeRefractor.java index 0f7995d9b36..8676129be31 100644 --- a/Mage.Sets/src/mage/cards/m/ManascapeRefractor.java +++ b/Mage.Sets/src/mage/cards/m/ManascapeRefractor.java @@ -90,7 +90,7 @@ class ManascapeRefractorGainAbilitiesEffect extends ContinuousEffectImpl { || perm.getAbilities(game) .stream() .noneMatch(ability.getClass()::isInstance)) { - perm.addAbility(ability, source.getSourceId(), game); + perm.addAbility(ability, source.getSourceId(), game, true); } } return true; diff --git a/Mage.Sets/src/mage/cards/m/MetamorphicAlteration.java b/Mage.Sets/src/mage/cards/m/MetamorphicAlteration.java index 090511d2ed9..08df1993fd5 100644 --- a/Mage.Sets/src/mage/cards/m/MetamorphicAlteration.java +++ b/Mage.Sets/src/mage/cards/m/MetamorphicAlteration.java @@ -140,7 +140,7 @@ class MetamorphicAlterationEffect extends ContinuousEffectImpl { permanent.getColor(game).setColor(copied.getColor(game)); permanent.removeAllAbilities(source.getSourceId(), game); for (Ability ability : copied.getAbilities()) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } permanent.getPower().setModifiedBaseValue(copied.getPower().getBaseValue()); permanent.getToughness().setModifiedBaseValue(copied.getToughness().getBaseValue()); diff --git a/Mage.Sets/src/mage/cards/m/MirranSafehouse.java b/Mage.Sets/src/mage/cards/m/MirranSafehouse.java index 5623a89ea10..e6c6530e226 100644 --- a/Mage.Sets/src/mage/cards/m/MirranSafehouse.java +++ b/Mage.Sets/src/mage/cards/m/MirranSafehouse.java @@ -73,7 +73,7 @@ class MirranSafehouseEffect extends ContinuousEffectImpl { .filter(ActivatedAbility.class::isInstance) .collect(Collectors.toSet()); for (Ability ability : abilities) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } return true; } diff --git a/Mage.Sets/src/mage/cards/m/MyrWelder.java b/Mage.Sets/src/mage/cards/m/MyrWelder.java index 825523be912..8079d56eea1 100644 --- a/Mage.Sets/src/mage/cards/m/MyrWelder.java +++ b/Mage.Sets/src/mage/cards/m/MyrWelder.java @@ -101,7 +101,7 @@ class MyrWelderContinuousEffect extends ContinuousEffectImpl { if (card != null) { for (Ability ability : card.getAbilities(game)) { if (ability instanceof ActivatedAbility) { - perm.addAbility(ability, source.getId(), game); + perm.addAbility(ability, source.getId(), game, true); } } } diff --git a/Mage.Sets/src/mage/cards/n/NecroticOoze.java b/Mage.Sets/src/mage/cards/n/NecroticOoze.java index b99f4b829a7..586eae9a4d6 100644 --- a/Mage.Sets/src/mage/cards/n/NecroticOoze.java +++ b/Mage.Sets/src/mage/cards/n/NecroticOoze.java @@ -79,7 +79,7 @@ class NecroticOozeEffect extends ContinuousEffectImpl { .filter(ActivatedAbility.class::isInstance) .collect(Collectors.toSet()); for (Ability ability : abilities) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } return true; } diff --git a/Mage.Sets/src/mage/cards/n/NicolBolasDragonGod.java b/Mage.Sets/src/mage/cards/n/NicolBolasDragonGod.java index ae152e3d98f..342a3d916cb 100644 --- a/Mage.Sets/src/mage/cards/n/NicolBolasDragonGod.java +++ b/Mage.Sets/src/mage/cards/n/NicolBolasDragonGod.java @@ -91,7 +91,7 @@ class NicolBolasDragonGodGainAbilitiesEffect extends ContinuousEffectImpl { )) { for (Ability ability : permanent.getAbilities()) { if (ability instanceof LoyaltyAbility) { - perm.addAbility(ability, source.getSourceId(), game); + perm.addAbility(ability, source.getSourceId(), game, true); } } } diff --git a/Mage.Sets/src/mage/cards/p/PatchworkCrawler.java b/Mage.Sets/src/mage/cards/p/PatchworkCrawler.java index 680fa682921..8a3ab27355f 100644 --- a/Mage.Sets/src/mage/cards/p/PatchworkCrawler.java +++ b/Mage.Sets/src/mage/cards/p/PatchworkCrawler.java @@ -77,7 +77,7 @@ class PatchworkCrawlerEffect extends ContinuousEffectImpl { for (Card card : exileZone.getCards(StaticFilters.FILTER_CARD_CREATURE, game)) { for (Ability ability : card.getAbilities(game)) { if (ability instanceof ActivatedAbility) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } } } diff --git a/Mage.Sets/src/mage/cards/q/QuicksilverElemental.java b/Mage.Sets/src/mage/cards/q/QuicksilverElemental.java index 86e75908849..e545522c800 100644 --- a/Mage.Sets/src/mage/cards/q/QuicksilverElemental.java +++ b/Mage.Sets/src/mage/cards/q/QuicksilverElemental.java @@ -77,6 +77,7 @@ class QuicksilverElementalEffect extends OneShotEffect { for (ActivatedAbility ability : creature.getAbilities().getActivatedAbilities(Zone.BATTLEFIELD)) { Ability newAbility = ability.copy(); newAbility.newOriginalId(); + newAbility.getSubAbilities().clear(); //Should not copy subabilities game.addEffect(new GainAbilitySourceEffect(newAbility, Duration.EndOfTurn), source); } return true; diff --git a/Mage.Sets/src/mage/cards/r/RobaranMercenaries.java b/Mage.Sets/src/mage/cards/r/RobaranMercenaries.java index 95060b484b9..f4f1553614d 100644 --- a/Mage.Sets/src/mage/cards/r/RobaranMercenaries.java +++ b/Mage.Sets/src/mage/cards/r/RobaranMercenaries.java @@ -87,7 +87,7 @@ class RobaranMercenariesEffect extends ContinuousEffectImpl { || perm.getAbilities(game) .stream() .noneMatch(ability.getClass()::isInstance)) { - perm.addAbility(ability, source.getSourceId(), game); + perm.addAbility(ability, source.getSourceId(), game, true); } } return true; diff --git a/Mage.Sets/src/mage/cards/s/SchemingFence.java b/Mage.Sets/src/mage/cards/s/SchemingFence.java index 4e911fe9701..c1258ca5a2f 100644 --- a/Mage.Sets/src/mage/cards/s/SchemingFence.java +++ b/Mage.Sets/src/mage/cards/s/SchemingFence.java @@ -184,7 +184,7 @@ class SchemingFenceGainEffect extends ContinuousEffectImpl { if (!(ability instanceof LoyaltyAbility)) { Ability copied = ability.copy(); ability.getEffects().setValue("schemingFence", source.getSourceId()); - permanent.addAbility(copied, source.getSourceId(), game); + permanent.addAbility(copied, source.getSourceId(), game, true); } } return true; diff --git a/Mage.Sets/src/mage/cards/s/SharkeyTyrantOfTheShire.java b/Mage.Sets/src/mage/cards/s/SharkeyTyrantOfTheShire.java index b7039e786c4..529e5854a4e 100644 --- a/Mage.Sets/src/mage/cards/s/SharkeyTyrantOfTheShire.java +++ b/Mage.Sets/src/mage/cards/s/SharkeyTyrantOfTheShire.java @@ -145,7 +145,7 @@ class SharkeyTyrantOfTheShireContinousEffect extends ContinuousEffectImpl { .filter(Objects::nonNull) .filter(ability -> ability.getAbilityType() == AbilityType.ACTIVATED) // Mana abilities are separated in their own AbilityType.Mana .collect(Collectors.toList())) { - perm.addAbility(ability, source.getSourceId(), game); + perm.addAbility(ability, source.getSourceId(), game, true); } return true; } diff --git a/Mage.Sets/src/mage/cards/t/TheBookOfVileDarkness.java b/Mage.Sets/src/mage/cards/t/TheBookOfVileDarkness.java index 9283eed30da..16646ce8684 100644 --- a/Mage.Sets/src/mage/cards/t/TheBookOfVileDarkness.java +++ b/Mage.Sets/src/mage/cards/t/TheBookOfVileDarkness.java @@ -191,7 +191,7 @@ class TheBookOfVileDarknessEffect extends OneShotEffect { Ability copyAbility = ability.copy(); copyAbility.newId(); copyAbility.setControllerId(source.getControllerId()); - token.addAbility(copyAbility); + token.addAbility(copyAbility, true); } } } diff --git a/Mage.Sets/src/mage/cards/t/TrazynTheInfinite.java b/Mage.Sets/src/mage/cards/t/TrazynTheInfinite.java index fba3ab2ce54..443da994d89 100644 --- a/Mage.Sets/src/mage/cards/t/TrazynTheInfinite.java +++ b/Mage.Sets/src/mage/cards/t/TrazynTheInfinite.java @@ -77,7 +77,7 @@ class TrazynTheInfiniteEffect extends ContinuousEffectImpl { .filter(ActivatedAbility.class::isInstance) .collect(Collectors.toSet()); for (Ability ability : abilities) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } return true; } diff --git a/Mage.Sets/src/mage/cards/v/VolrathsShapeshifter.java b/Mage.Sets/src/mage/cards/v/VolrathsShapeshifter.java index 14794acb568..e1cfb03fda4 100644 --- a/Mage.Sets/src/mage/cards/v/VolrathsShapeshifter.java +++ b/Mage.Sets/src/mage/cards/v/VolrathsShapeshifter.java @@ -97,7 +97,7 @@ class VolrathsShapeshifterEffect extends ContinuousEffectImpl { for (Ability ability : card.getAbilities(game)) { if (!permanent.getAbilities().contains(ability)) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/SquadTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/SquadTest.java index 416edca71cc..ab461dbc6f1 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/SquadTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/SquadTest.java @@ -198,9 +198,7 @@ public class SquadTest extends CardTestPlayerBase { } // The squad status is a copiable value of the spell, and should be carried over on copy. - @Ignore @Test - //TODO: Enable after fixing subability copying twice bug public void test_CopyingSpellMustKeepSquadStatus() { addCard(Zone.HAND, playerA, flagellant, 1); diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/copy/CopyPermanentSpellTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/copy/CopyPermanentSpellTest.java index 3e4390bb526..78e94cca589 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/copy/CopyPermanentSpellTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/copy/CopyPermanentSpellTest.java @@ -80,7 +80,6 @@ public class CopyPermanentSpellTest extends CardTestPlayerBase { assertPermanentCount(playerA, "Dead Weight", 2); } - @Ignore // currently fails @Test public void testKickerTrigger() { makeTester(); @@ -98,7 +97,6 @@ public class CopyPermanentSpellTest extends CardTestPlayerBase { assertPowerToughness(playerA, "Grizzly Bears", 4, 2); } - @Ignore // currently fails @Test public void testKickerReplacement() { makeTester(); diff --git a/Mage/src/main/java/mage/abilities/Ability.java b/Mage/src/main/java/mage/abilities/Ability.java index b017418652c..6bbf5e83b97 100644 --- a/Mage/src/main/java/mage/abilities/Ability.java +++ b/Mage/src/main/java/mage/abilities/Ability.java @@ -332,6 +332,8 @@ public interface Ability extends Controllable, Serializable { /** * Gets the list of sub-abilities associated with this ability. + * When copying, subabilities are copied separately and thus the list is desynced. + * Do not interact with the subabilities list during a game! * * @return */ diff --git a/Mage/src/main/java/mage/abilities/effects/common/CopyEffect.java b/Mage/src/main/java/mage/abilities/effects/common/CopyEffect.java index 53a5514476a..d7b721d877c 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/CopyEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/CopyEffect.java @@ -119,11 +119,11 @@ public class CopyEffect extends ContinuousEffectImpl { permanent.removeAllAbilities(source.getSourceId(), game); if (copyFromObject instanceof Permanent) { for (Ability ability : ((Permanent) copyFromObject).getAbilities(game)) { - permanent.addAbility(ability, getSourceId(), game); + permanent.addAbility(ability, getSourceId(), game, true); } } else { for (Ability ability : copyFromObject.getAbilities()) { - permanent.addAbility(ability, getSourceId(), game); + permanent.addAbility(ability, getSourceId(), game, true); } } diff --git a/Mage/src/main/java/mage/abilities/effects/common/CopyTokenEffect.java b/Mage/src/main/java/mage/abilities/effects/common/CopyTokenEffect.java index c1f8414dd52..4a9f5082214 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/CopyTokenEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/CopyTokenEffect.java @@ -39,7 +39,7 @@ public class CopyTokenEffect extends ContinuousEffectImpl { } permanent.getAbilities().clear(); for (Ability ability : token.getAbilities()) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } permanent.getPower().setModifiedBaseValue(token.getPower().getModifiedBaseValue()); permanent.getToughness().setModifiedBaseValue(token.getToughness().getModifiedBaseValue()); diff --git a/Mage/src/main/java/mage/abilities/effects/common/GainActivatedAbilitiesOfTopCardEffect.java b/Mage/src/main/java/mage/abilities/effects/common/GainActivatedAbilitiesOfTopCardEffect.java index d368ffc5e07..e65fdc71dd5 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/GainActivatedAbilitiesOfTopCardEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/GainActivatedAbilitiesOfTopCardEffect.java @@ -43,7 +43,7 @@ public class GainActivatedAbilitiesOfTopCardEffect extends ContinuousEffectImpl if (permanent != null) { for (Ability ability : card.getAbilities(game)) { if (ability instanceof ActivatedAbility) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } } return true; diff --git a/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureAllEffect.java b/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureAllEffect.java index d0d73ce34c9..f4cb447b8f1 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureAllEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureAllEffect.java @@ -128,7 +128,7 @@ public class BecomesCreatureAllEffect extends ContinuousEffectImpl { case AbilityAddingRemovingEffects_6: if (!token.getAbilities().isEmpty()) { for (Ability ability : token.getAbilities()) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } } break; diff --git a/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureAttachedEffect.java b/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureAttachedEffect.java index 837977fcabc..543245abb19 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureAttachedEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureAttachedEffect.java @@ -112,7 +112,7 @@ public class BecomesCreatureAttachedEffect extends ContinuousEffectImpl { break; } for (Ability ability : token.getAbilities()) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } break; diff --git a/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureSourceEffect.java b/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureSourceEffect.java index e2289ffab40..9504f680a05 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureSourceEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureSourceEffect.java @@ -141,7 +141,7 @@ public class BecomesCreatureSourceEffect extends ContinuousEffectImpl { permanent.removeAllAbilities(source.getSourceId(), game); } for (Ability ability : token.getAbilities()) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } break; diff --git a/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureTargetEffect.java b/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureTargetEffect.java index 1131dd6e917..c9266a7f3ae 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureTargetEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureTargetEffect.java @@ -133,7 +133,7 @@ public class BecomesCreatureTargetEffect extends ContinuousEffectImpl { if (sublayer == SubLayer.NA) { if (!token.getAbilities().isEmpty()) { for (Ability ability : token.getAbilities()) { - permanent.addAbility(ability, source.getSourceId(), game); + permanent.addAbility(ability, source.getSourceId(), game, true); } } } diff --git a/Mage/src/main/java/mage/abilities/keyword/CrewAbility.java b/Mage/src/main/java/mage/abilities/keyword/CrewAbility.java index 2c35e7f17df..205ceaeed30 100644 --- a/Mage/src/main/java/mage/abilities/keyword/CrewAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/CrewAbility.java @@ -51,6 +51,8 @@ public class CrewAbility extends SimpleActivatedAbility { this.addIcon(new CardIconImpl(CardIconType.ABILITY_CREW, "Crew " + value)); this.value = value; if (altCost != null) { + //TODO: the entire alternative cost should be included in the subability, not just the hint text + // Heart of Kiran's alternative crew cost is a static ability, not part of the activated ability directly this.addSubAbility(new SimpleStaticAbility(Zone.ALL, new InfoEffect( "you may " + CardUtil.addCostVerb(altCost.getText()) + " rather than pay {this}'s crew cost" diff --git a/Mage/src/main/java/mage/abilities/keyword/TransformAbility.java b/Mage/src/main/java/mage/abilities/keyword/TransformAbility.java index ff3de87c56b..7ef9808df92 100644 --- a/Mage/src/main/java/mage/abilities/keyword/TransformAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/TransformAbility.java @@ -67,7 +67,7 @@ public class TransformAbility extends SimpleStaticAbility { for (Ability ability : sourceCard.getAbilities()) { // source == null -- call from init card (e.g. own abilities) // source != null -- from apply effect - permanent.addAbility(ability, source == null ? permanent.getId() : source.getSourceId(), game); + permanent.addAbility(ability, source == null ? permanent.getId() : source.getSourceId(), game, true); } permanent.getPower().setModifiedBaseValue(sourceCard.getPower().getValue()); permanent.getToughness().setModifiedBaseValue(sourceCard.getToughness().getValue()); diff --git a/Mage/src/main/java/mage/game/permanent/Permanent.java b/Mage/src/main/java/mage/game/permanent/Permanent.java index c9e1494ca6f..3bc89050c19 100644 --- a/Mage/src/main/java/mage/game/permanent/Permanent.java +++ b/Mage/src/main/java/mage/game/permanent/Permanent.java @@ -218,6 +218,7 @@ public interface Permanent extends Card, Controllable { * @return can be null for exists abilities */ Ability addAbility(Ability ability, UUID sourceId, Game game); + Ability addAbility(Ability ability, UUID sourceId, Game game, boolean fromExistingObject); void removeAllAbilities(UUID sourceId, Game game); diff --git a/Mage/src/main/java/mage/game/permanent/PermanentImpl.java b/Mage/src/main/java/mage/game/permanent/PermanentImpl.java index 04e40c63fd1..821096c34ff 100644 --- a/Mage/src/main/java/mage/game/permanent/PermanentImpl.java +++ b/Mage/src/main/java/mage/game/permanent/PermanentImpl.java @@ -388,8 +388,29 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { return super.getAbilities(game); } + /** + * Add an ability to the permanent. When copying from an existing source + * you should use the fromExistingObject variant of this function to prevent double-copying subabilities + * @param ability The ability to be added + * @param sourceId id of the source doing the added (for the effect created to add it) + * @param game + * @return The newly added ability copy + */ @Override public Ability addAbility(Ability ability, UUID sourceId, Game game) { + return addAbility(ability, sourceId, game, false); + } + + /** + * @param ability The ability to be added + * @param sourceId id of the source doing the added (for the effect created to add it) + * @param game + * @param fromExistingObject if copying abilities from an existing source then must ignore sub-abilities because they're already on the source object + * Otherwise sub-abilities will be added twice to the resulting object + * @return The newly added ability copy + */ + @Override + public Ability addAbility(Ability ability, UUID sourceId, Game game, boolean fromExistingObject) { // singleton abilities -- only one instance // other abilities -- any amount of instances if (!abilities.containsKey(ability.getId())) { @@ -404,7 +425,9 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { game.getState().addAbility(copyAbility, sourceId, this); } abilities.add(copyAbility); - abilities.addAll(ability.getSubAbilities()); + if (!fromExistingObject) { + abilities.addAll(copyAbility.getSubAbilities()); + } return copyAbility; } return null; diff --git a/Mage/src/main/java/mage/game/permanent/PermanentToken.java b/Mage/src/main/java/mage/game/permanent/PermanentToken.java index b494746aeca..5cab268b6e1 100644 --- a/Mage/src/main/java/mage/game/permanent/PermanentToken.java +++ b/Mage/src/main/java/mage/game/permanent/PermanentToken.java @@ -89,7 +89,8 @@ public class PermanentToken extends PermanentImpl { // first time -> create ContinuousEffects only once // so sourceId must be null (keep triggered abilities forever?) for (Ability ability : token.getAbilities()) { - this.addAbility(ability, null, game); + //Don't add subabilities since the original token already has them in its abilities list + this.addAbility(ability, null, game, true); } } this.abilities.setControllerId(this.controllerId); diff --git a/Mage/src/main/java/mage/game/permanent/token/Token.java b/Mage/src/main/java/mage/game/permanent/token/Token.java index 686f8ef4e5a..8c10813529e 100644 --- a/Mage/src/main/java/mage/game/permanent/token/Token.java +++ b/Mage/src/main/java/mage/game/permanent/token/Token.java @@ -22,6 +22,7 @@ public interface Token extends MageObject { List getLastAddedTokenIds(); void addAbility(Ability ability); + void addAbility(Ability ability, boolean fromExistingObject); void removeAbility(Ability abilityToRemove); diff --git a/Mage/src/main/java/mage/game/permanent/token/TokenImpl.java b/Mage/src/main/java/mage/game/permanent/token/TokenImpl.java index 55c2862689d..990e303b9b0 100644 --- a/Mage/src/main/java/mage/game/permanent/token/TokenImpl.java +++ b/Mage/src/main/java/mage/game/permanent/token/TokenImpl.java @@ -77,15 +77,33 @@ public abstract class TokenImpl extends MageObjectImpl implements Token { return new ArrayList<>(lastAddedTokenIds); } + /** + * Add an ability to the token. When copying from an existing source + * you should use the fromExistingObject variant of this function to prevent double-copying subabilities + * @param ability The ability to be added + */ @Override public void addAbility(Ability ability) { + addAbility(ability, false); + } + + /** + * @param ability The ability to be added + * @param fromExistingObject if copying abilities from an existing source then must ignore sub-abilities because they're already on the source object + * Otherwise sub-abilities will be added twice to the resulting object + */ + @Override + public void addAbility(Ability ability, boolean fromExistingObject) { ability.setSourceId(this.getId()); abilities.add(ability); - abilities.addAll(ability.getSubAbilities()); + if (!fromExistingObject) { + abilities.addAll(ability.getSubAbilities()); + } // TODO: remove all override and backFace changes (bug example: active transform ability in back face) if (backFace != null) { backFace.addAbility(ability); + // Maybe supposed to add subabilities here too? } } diff --git a/Mage/src/main/java/mage/util/functions/CopyTokenFunction.java b/Mage/src/main/java/mage/util/functions/CopyTokenFunction.java index 851f76f38f5..74eeabd96b6 100644 --- a/Mage/src/main/java/mage/util/functions/CopyTokenFunction.java +++ b/Mage/src/main/java/mage/util/functions/CopyTokenFunction.java @@ -140,8 +140,8 @@ public class CopyTokenFunction { // otherwise there are problems to check for created continuous effects to check if // the source (the Token) has still this ability ability.newOriginalId(); - - target.addAbility(ability); + //Don't re-add subabilities since they've already in sourceObj's abilities list + target.addAbility(ability, true); } target.setPower(sourceObj.getPower().getBaseValue()); @@ -152,7 +152,6 @@ public class CopyTokenFunction { private Token from(Card source, Game game, Spell spell) { apply(source, game); - // token's ZCC must be synced with original card to keep abilities settings // Example: kicker ability and kicked status if (spell != null) {