From 4f1368f3de90d177bd11f463e83f156aa70a5f3a Mon Sep 17 00:00:00 2001 From: LevelX2 Date: Thu, 25 Dec 2014 02:07:40 +0100 Subject: [PATCH] * Made a lot of changes to handling of continuous and triggered abilities. This should fix the problems with Mage Singletons like Flyinging / Intimidate / Reach not working. Fixed also #533 and some other problems with copy effects of cards like Clone that did not end if e.g. Clone left the battlefield. --- .../sets/dragonsmaze/ProgenitorMimic.java | 1 + .../mage/sets/gatecrash/MasterBiomancer.java | 3 +- .../org/mage/test/cards/copy/CloneTest.java | 55 +++++++ .../cards/copy/LazavDimirMastermindTest.java | 4 + .../ZoneChangeReplacementTest.java | 2 + .../test/cards/single/roe/WorldAtWarTest.java | 9 +- .../cards/triggers/dies/ReefWormTest.java | 4 +- .../src/test/resources/log4j.properties | 4 +- Mage/src/mage/abilities/AbilityImpl.java | 8 +- .../mage/abilities/TriggeredAbilities.java | 32 ++-- .../effects/ContinuousEffectImpl.java | 13 +- .../abilities/effects/ContinuousEffects.java | 148 +++++++++++++----- .../effects/ContinuousEffectsList.java | 47 +++--- .../effects/EntersBattlefieldEffect.java | 8 +- .../abilities/effects/common/CopyEffect.java | 29 ++-- .../AddCardSubTypeTargetEffect.java | 7 +- .../abilities/keyword/SuspendAbility.java | 4 - Mage/src/mage/cards/CardImpl.java | 3 +- Mage/src/mage/game/Game.java | 1 - Mage/src/mage/game/GameImpl.java | 18 --- Mage/src/mage/game/GameState.java | 40 ++++- Mage/src/mage/game/permanent/Permanent.java | 3 +- .../mage/game/permanent/PermanentImpl.java | 14 +- .../mage/game/permanent/PermanentToken.java | 20 ++- Mage/src/mage/game/stack/Spell.java | 8 +- 25 files changed, 316 insertions(+), 169 deletions(-) diff --git a/Mage.Sets/src/mage/sets/dragonsmaze/ProgenitorMimic.java b/Mage.Sets/src/mage/sets/dragonsmaze/ProgenitorMimic.java index 0a5b592bd9e..adbf77737ee 100644 --- a/Mage.Sets/src/mage/sets/dragonsmaze/ProgenitorMimic.java +++ b/Mage.Sets/src/mage/sets/dragonsmaze/ProgenitorMimic.java @@ -54,6 +54,7 @@ import mage.game.permanent.Permanent; import mage.game.permanent.token.EmptyToken; import mage.util.CardUtil; import mage.util.functions.ApplyToPermanent; +import org.apache.log4j.Logger; /** * diff --git a/Mage.Sets/src/mage/sets/gatecrash/MasterBiomancer.java b/Mage.Sets/src/mage/sets/gatecrash/MasterBiomancer.java index e9494a01f1e..898b2369dce 100644 --- a/Mage.Sets/src/mage/sets/gatecrash/MasterBiomancer.java +++ b/Mage.Sets/src/mage/sets/gatecrash/MasterBiomancer.java @@ -46,6 +46,7 @@ import mage.game.events.GameEvent; import mage.game.events.GameEvent.EventType; import mage.game.permanent.Permanent; import mage.target.targetpointer.FixedTarget; +import org.apache.log4j.Logger; /** * @@ -116,7 +117,7 @@ class MasterBiomancerEntersBattlefieldEffect extends ReplacementEffectImpl { if (power > 0) { creature.addCounters(CounterType.P1P1.createInstance(power), game); } - ContinuousEffect effect = new AddCardSubTypeTargetEffect("Mutant", Duration.WhileOnBattlefield); + ContinuousEffect effect = new AddCardSubTypeTargetEffect("Mutant", Duration.Custom); effect.setTargetPointer(new FixedTarget(creature.getId())); game.addEffect(effect, source); } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/copy/CloneTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/copy/CloneTest.java index 6b10053fe5a..47bf48d5091 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/copy/CloneTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/copy/CloneTest.java @@ -1,7 +1,15 @@ package org.mage.test.cards.copy; +import java.util.Iterator; +import mage.abilities.effects.ContinuousEffect; +import mage.abilities.effects.ContinuousEffectsList; +import mage.cards.Card; import mage.constants.PhaseStep; import mage.constants.Zone; +import mage.filter.FilterCard; +import mage.filter.predicate.mageobject.NamePredicate; +import org.apache.log4j.Logger; +import org.junit.Assert; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; @@ -90,4 +98,51 @@ public class CloneTest extends CardTestPlayerBase { assertPowerToughness(playerB, "Craw Wurm", 4, 4); assertPowerToughness(playerA, "Craw Wurm", 6, 4); } + + // copy Nightmare test, check that the P/T setting effect ends + // if the clone leaves battlefield + + @Test + public void testCopyNightmare() { + addCard(Zone.BATTLEFIELD, playerA, "Island", 5); + addCard(Zone.BATTLEFIELD, playerA, "Swamp", 1); + addCard(Zone.BATTLEFIELD, playerB, "Swamp", 6); + addCard(Zone.BATTLEFIELD, playerB, "Forest", 1); + addCard(Zone.HAND, playerB, "Ranger's Guile"); + + addCard(Zone.HAND, playerA, "Clone"); + addCard(Zone.HAND, playerA, "Disperse"); + + addCard(Zone.BATTLEFIELD, playerB, "Nightmare", 1); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Clone"); + setChoice(playerA, "Nightmare"); + castSpell(1, PhaseStep.BEGIN_COMBAT, playerB, "Ranger's Guile", "Nightmare"); + castSpell(1, PhaseStep.POSTCOMBAT_MAIN, playerA, "Disperse", "Nightmare"); + + setStopAt(2, PhaseStep.UPKEEP); + execute(); + + assertPermanentCount(playerB, "Nightmare", 1); + assertPowerToughness(playerB, "Nightmare", 6, 6); + assertPermanentCount(playerA, "Nightmare", 0); + assertHandCount(playerA, "Disperse", 0); + + FilterCard filter = new FilterCard(); + filter.add(new NamePredicate("Clone")); + Card card = playerA.getHand().getCards(filter, currentGame).iterator().next(); + if (card != null) { + Assert.assertEquals("Power has to be 0 because copy from nightmare P/T ability may no longer be applied", 0, card.getPower().getValue()); + } + + Logger.getLogger(CloneTest.class).debug("EXISTING CONTINUOUS EFFECTS:"); + for (ContinuousEffectsList effectsList : currentGame.getContinuousEffects().allEffectsLists) { + Iterator it = effectsList.iterator(); + while (it.hasNext()) { + ContinuousEffect effect = (ContinuousEffect) it.next(); + Logger.getLogger(CloneTest.class).debug("- " + effect.toString()); + } + } + } + } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/copy/LazavDimirMastermindTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/copy/LazavDimirMastermindTest.java index 89b6274d4c2..0a12f1f9129 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/copy/LazavDimirMastermindTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/copy/LazavDimirMastermindTest.java @@ -30,8 +30,12 @@ public class LazavDimirMastermindTest extends CardTestPlayerBase { @Test public void testCopySimpleCreature() { addCard(Zone.BATTLEFIELD, playerA, "Lazav, Dimir Mastermind", 1); + // Codex Shredder - Artifact + // {T}: Target player puts the top card of his or her library into his or her graveyard. + // {5}, {T}, Sacrifice Codex Shredder: Return target card from your graveyard to your hand. addCard(Zone.BATTLEFIELD, playerA, "Codex Shredder", 1); + // Flying 3/2 addCard(Zone.LIBRARY, playerB, "Assault Griffin",5); skipInitShuffling(); diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/replacement/ZoneChangeReplacementTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/replacement/ZoneChangeReplacementTest.java index e34a00e85c7..bd0cab05be8 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/replacement/ZoneChangeReplacementTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/replacement/ZoneChangeReplacementTest.java @@ -136,6 +136,8 @@ public class ZoneChangeReplacementTest extends CardTestPlayerBase { @Test public void testHumilityDeactivatesReplacementEffectAbilities() { + // Protection from everything + // If Progenitus would be put into a graveyard from anywhere, reveal Progenitus and shuffle it into its owner's library instead. addCard(Zone.BATTLEFIELD, playerA, "Progenitus"); // Enchantment {2}{W}{W} // All creatures lose all abilities and are 1/1. diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/single/roe/WorldAtWarTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/single/roe/WorldAtWarTest.java index 2a1675f3d57..f1ec0349cac 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/single/roe/WorldAtWarTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/single/roe/WorldAtWarTest.java @@ -45,9 +45,16 @@ public class WorldAtWarTest extends CardTestPlayerBase { @Test public void testCardWithRebound() { addCard(Zone.BATTLEFIELD, playerA, "Mountain", 5); + // Enchantment - Creatures you control have haste. (They can attack and as soon as they come under your control.) addCard(Zone.BATTLEFIELD, playerA, "Fervor"); + // After the first postcombat main phase this turn, there's an additional combat phase followed by + // an additional main phase. At the beginning of that combat, untap all creatures that attacked this turn. + // Rebound (If you cast this spell from your hand, exile it as it resolves. At the beginning of your + // next upkeep, you may cast this card from exile without paying its mana cost.) addCard(Zone.HAND, playerA, "World at War"); + // Creature - Human Soldier 2/1 addCard(Zone.BATTLEFIELD, playerA, "Elite Vanguard"); + // Creature - Hound 1/1 addCard(Zone.BATTLEFIELD, playerA, "Warclamp Mastiff"); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "World at War"); @@ -71,7 +78,7 @@ public class WorldAtWarTest extends CardTestPlayerBase { } /** - * Tests creatures attack thrice + * Tests creatures attack three times */ @Test public void testDoubleCast() { diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/triggers/dies/ReefWormTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/triggers/dies/ReefWormTest.java index 2a8b315b352..7c57a3d80fe 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/triggers/dies/ReefWormTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/triggers/dies/ReefWormTest.java @@ -60,12 +60,12 @@ public class ReefWormTest extends CardTestPlayerBase { assertLife(playerA, 20); assertLife(playerB, 20); - assertGraveyardCount(playerA, "Lightning Bolt", 4); - assertPermanentCount(playerB, "Fish", 0); assertPermanentCount(playerB, "Whale", 0); assertPermanentCount(playerB, "Kraken", 1); assertGraveyardCount(playerB, "Reef Worm", 1); + + assertGraveyardCount(playerA, "Lightning Bolt", 4); } } \ No newline at end of file diff --git a/Mage.Tests/src/test/resources/log4j.properties b/Mage.Tests/src/test/resources/log4j.properties index b732a3205b4..fe23f19fc59 100644 --- a/Mage.Tests/src/test/resources/log4j.properties +++ b/Mage.Tests/src/test/resources/log4j.properties @@ -6,7 +6,9 @@ log4j.logger.com.j256.ormlite=warn log4j.appender.console=org.apache.log4j.ConsoleAppender log4j.appender.console.layout=org.apache.log4j.PatternLayout log4j.appender.console.layout.ConversionPattern=%-5p [%d{yyyy-MM-dd HH:mm [ss:SSS]}] %C{1}[%t]: %m%n -log4j.appender.console.Threshold=INFO +log4j.appender.console.Threshold=info +#log4j.appender.console.Threshold=debug + #file log log4j.appender.file=org.apache.log4j.FileAppender diff --git a/Mage/src/mage/abilities/AbilityImpl.java b/Mage/src/mage/abilities/AbilityImpl.java index 4e834e33e13..d24400e162d 100644 --- a/Mage/src/mage/abilities/AbilityImpl.java +++ b/Mage/src/mage/abilities/AbilityImpl.java @@ -536,7 +536,13 @@ public abstract class AbilityImpl implements Ability { @Override public void setSourceId(UUID sourceId) { - this.sourceId = sourceId; + if (this.sourceId == null) { + this.sourceId = sourceId; + } else { + if (!(this instanceof MageSingleton)) { + this.sourceId = sourceId; + } + } } @Override diff --git a/Mage/src/mage/abilities/TriggeredAbilities.java b/Mage/src/mage/abilities/TriggeredAbilities.java index 731f82686bf..c9d00fcbccf 100644 --- a/Mage/src/mage/abilities/TriggeredAbilities.java +++ b/Mage/src/mage/abilities/TriggeredAbilities.java @@ -44,6 +44,7 @@ import mage.game.events.GameEvent; import mage.game.events.GameEvent.EventType; import mage.game.permanent.Permanent; import mage.game.permanent.PermanentCard; +import org.apache.log4j.Logger; /** * @@ -129,6 +130,7 @@ public class TriggeredAbilities extends ConcurrentHashMap uuidList = new LinkedList<>(); uuidList.add(sourceId); @@ -138,6 +140,7 @@ public class TriggeredAbilities extends ConcurrentHashMap removeGainedAbilitiesForSource(UUID sourceId) { + public void removeAbilitiesOfSource(UUID sourceId) { List keysToRemove = new ArrayList<>(); - - for (Map.Entry> entry : sources.entrySet()) { - if (entry.getValue().contains(sourceId)) { - keysToRemove.add(entry.getKey()); + for (String key: this.keySet()) { + if(key.endsWith(sourceId.toString())) { + keysToRemove.add(key); } } - for (String key: keysToRemove) { - sources.remove(key); + for(String key: keysToRemove) { + remove(key); } - return keysToRemove; + } + + public void removeAllGainedAbilities() { + Logger.getLogger(TriggeredAbilities.class).debug("REMOVE all gained Triggered Abilities"); + + for(String key: sources.keySet()) { + this.remove(key); + } + sources.clear(); } public TriggeredAbilities copy() { diff --git a/Mage/src/mage/abilities/effects/ContinuousEffectImpl.java b/Mage/src/mage/abilities/effects/ContinuousEffectImpl.java index 9310d100d43..ce4342d5b6a 100644 --- a/Mage/src/mage/abilities/effects/ContinuousEffectImpl.java +++ b/Mage/src/mage/abilities/effects/ContinuousEffectImpl.java @@ -29,9 +29,9 @@ package mage.abilities.effects; import java.util.ArrayList; -import java.util.Date; import java.util.List; import java.util.UUID; +import mage.MageObjectReference; import mage.abilities.Ability; import mage.abilities.MageSingleton; import mage.abilities.dynamicvalue.DynamicValue; @@ -67,8 +67,8 @@ public abstract class ContinuousEffectImpl extends EffectImpl implements Continu protected boolean used = false; protected boolean discarded = false; // for manual effect discard protected boolean affectedObjectsSet = false; - protected List objects = new ArrayList<>(); - // protected Map metadata = new HashMap<>(); + protected List affectedObjectList = new ArrayList<>(); + // until your next turn protected int startingTurn; protected UUID startingControllerId; @@ -95,7 +95,7 @@ public abstract class ContinuousEffectImpl extends EffectImpl implements Continu this.used = effect.used; this.discarded = effect.discarded; this.affectedObjectsSet = effect.affectedObjectsSet; - this.objects.addAll(effect.objects); + this.affectedObjectList.addAll(effect.affectedObjectList); this.startingTurn = effect.startingTurn; this.startingControllerId = effect.startingControllerId; } @@ -230,7 +230,8 @@ public abstract class ContinuousEffectImpl extends EffectImpl implements Continu } @Override - public List getAffectedObjects() { - return this.objects; + public List getAffectedObjects() { + return affectedObjectList; } + } diff --git a/Mage/src/mage/abilities/effects/ContinuousEffects.java b/Mage/src/mage/abilities/effects/ContinuousEffects.java index 6e7273cd3b3..3019fb6ac67 100644 --- a/Mage/src/mage/abilities/effects/ContinuousEffects.java +++ b/Mage/src/mage/abilities/effects/ContinuousEffects.java @@ -63,7 +63,10 @@ import mage.filter.predicate.Predicates; import mage.filter.predicate.mageobject.CardIdPredicate; import mage.game.Game; import mage.game.events.GameEvent; +import mage.game.events.GameEvent.EventType; +import mage.game.events.ZoneChangeEvent; import mage.game.permanent.Permanent; +import mage.game.permanent.PermanentCard; import mage.players.Player; import mage.target.common.TargetCardInHand; import org.apache.log4j.Logger; @@ -90,15 +93,16 @@ public class ContinuousEffects implements Serializable { private ContinuousEffectsList spliceCardEffects = new ContinuousEffectsList<>(); private final Map> asThoughEffectsMap = new EnumMap<>(AsThoughEffectType.class); - private final List> allEffectsLists = new ArrayList<>(); + public final List> allEffectsLists = new ArrayList<>(); private final ApplyCountersEffect applyCounters; private final PlaneswalkerRedirectionEffect planeswalkerRedirectionEffect; private final AuraReplacementEffect auraReplacementEffect; private final List previous = new ArrayList<>(); - // effect.id -> sourceId - which effect was added by which sourceId - private final Map sources = new HashMap<>(); + // note all effect/abilities that were only added temporary + private final Map> temporaryEffects = new HashMap<>(); + private final ContinuousEffectSorter sorter = new ContinuousEffectSorter(); public ContinuousEffects() { @@ -125,7 +129,7 @@ public class ContinuousEffects implements Serializable { costModificationEffects = effect.costModificationEffects.copy(); spliceCardEffects = effect.spliceCardEffects.copy(); - sources.putAll(effect.sources); + temporaryEffects.putAll(effect.temporaryEffects); collectAllEffects(); order = effect.order; } @@ -339,8 +343,10 @@ public class ContinuousEffects implements Serializable { if (!(ability instanceof StaticAbility) || ability.isInUseableZone(game, null, false)) { if (effect.getDuration() != Duration.OneUse || !effect.isUsed()) { if (!game.getScopeRelevant() || effect.hasSelfScope() || !event.getTargetId().equals(ability.getSourceId())) { - if (effect.applies(event, ability, game)) { - applicableAbilities.add(ability); + if (checkAbilityStillExists(ability, effect, event, game)) { + if (effect.applies(event, ability, game)) { + applicableAbilities.add(ability); + } } } } @@ -374,6 +380,34 @@ public class ContinuousEffects implements Serializable { return replaceEffects; } + private boolean checkAbilityStillExists(Ability ability, ContinuousEffect effect, GameEvent event, Game game) { + if (effect.getDuration().equals(Duration.OneUse)) { // needed for some special replacment effects (e.g. Undying) + return true; + } + MageObject object; + if (event.getType().equals(EventType.ZONE_CHANGE) && + ((ZoneChangeEvent)event).getFromZone().equals(Zone.BATTLEFIELD)&& + event.getTargetId().equals(ability.getSourceId())) { + object = ((ZoneChangeEvent)event).getTarget(); + } else { + object = game.getObject(ability.getSourceId()); + } + if (object == null) { + return false; + } + boolean exists = true; + if (!object.getAbilities().contains(ability)) { + exists = false; + if (object instanceof PermanentCard) { + PermanentCard permanent = (PermanentCard)object; + if (permanent.canTransform() && event.getType() == GameEvent.EventType.TRANSFORMED) { + exists = permanent.getCard().getAbilities().contains(ability); + } + } + } + return exists; + } + /** * Filters out cost modification effects that are not active. * @@ -711,7 +745,7 @@ public class ContinuousEffects implements Serializable { break; } - // add used effect to consumed effects + // add the applied effect to the consumed effects if (rEffect != null) { if (consumed.containsKey(rEffect.getId())) { HashSet set = consumed.get(rEffect.getId()); @@ -728,7 +762,7 @@ public class ContinuousEffects implements Serializable { consumed.put(rEffect.getId(), set); } } - + // Must be called here so some game.applyEffects(); } while (true); return caught; @@ -854,9 +888,31 @@ public class ContinuousEffects implements Serializable { } } + /** + * Adds a continuous ability with a reference to a sourceId. + * It's used for effects that cease to exist again + * So this effects were removed again before each applyEffecs + * @param effect + * @param sourceId + * @param source + */ public void addEffect(ContinuousEffect effect, UUID sourceId, Ability source) { + if (!(source instanceof MageSingleton)) { // because MageSingletons may never be removed by removing the temporary effecs they are not added to the temporaryEffects to prevent this + Set abilities = temporaryEffects.get(effect); + if (abilities == null) { + abilities = new HashSet<>(); + temporaryEffects.put(effect, abilities); + } else { + if (abilities.contains(source)) { + // this ability (for the continuous effect) is already added + return; + } + } + abilities.add(source); + + // add the effect itself + } addEffect(effect, source); - sources.put(effect.getId(), sourceId); } public void addEffect(ContinuousEffect effect, Ability source) { @@ -865,7 +921,7 @@ public class ContinuousEffects implements Serializable { return; } else if (source == null) { logger.warn("Adding effect without ability : " +effect.toString()); - } + } switch (effect.getEffectType()) { case REPLACEMENT: case REDIRECTION: @@ -909,7 +965,7 @@ public class ContinuousEffects implements Serializable { ContinuousRuleModifiyingEffect newContinuousRuleModifiyingEffect = (ContinuousRuleModifiyingEffect)effect; continuousRuleModifyingEffects.addEffect(newContinuousRuleModifiyingEffect, source); break; - default: + default: layeredEffects.addEffect(effect, source); break; } @@ -942,40 +998,52 @@ public class ContinuousEffects implements Serializable { for (ContinuousEffectsList effectsList : allEffectsLists) { effectsList.clear(); } - sources.clear(); + temporaryEffects.clear(); } - - /** - * Removes effects granted by sourceId - * - * @param sourceId - * @return - */ - public List removeGainedEffectsForSource(UUID sourceId) { - List effects = new ArrayList<>(); - Set effectsToRemove = new HashSet<>(); - for (Map.Entry source : sources.entrySet()) { - if (sourceId.equals(source.getValue())) { - for (ContinuousEffectsList effectsList : allEffectsLists) { - Iterator it = effectsList.iterator(); - while (it.hasNext()) { - ContinuousEffect effect = (ContinuousEffect) it.next(); - if (source.getKey().equals(effect.getId())) { - effectsToRemove.add(effect.getId()); - effectsList.removeEffectAbilityMap(effect.getId()); - it.remove(); - } + public void removeAllTemporaryEffects() { + for (Map.Entry> entry : temporaryEffects.entrySet()) { + switch (entry.getKey().getEffectType()) { + case REPLACEMENT: + case REDIRECTION: + replacementEffects.removeEffects(entry.getKey().getId(), entry.getValue()); + break; + case PREVENTION: + preventionEffects.removeEffects(entry.getKey().getId(), entry.getValue()); + break; + case RESTRICTION: + restrictionEffects.removeEffects(entry.getKey().getId(), entry.getValue()); + break; + case RESTRICTION_UNTAP_NOT_MORE_THAN: + restrictionUntapNotMoreThanEffects.removeEffects(entry.getKey().getId(), entry.getValue()); + break; + case REQUIREMENT: + requirementEffects.removeEffects(entry.getKey().getId(), entry.getValue()); + break; + case ASTHOUGH: + AsThoughEffect newAsThoughEffect = (AsThoughEffect)entry.getKey(); + if (!asThoughEffectsMap.containsKey(newAsThoughEffect.getAsThoughEffectType())) { + break; } - } + asThoughEffectsMap.get(newAsThoughEffect.getAsThoughEffectType()).removeEffects(entry.getKey().getId(), entry.getValue()); + break; + case COSTMODIFICATION: + costModificationEffects.removeEffects(entry.getKey().getId(), entry.getValue()); + break; + case SPLICE: + spliceCardEffects.removeEffects(entry.getKey().getId(), entry.getValue()); + break; + case CONTINUOUS_RULE_MODIFICATION: + continuousRuleModifyingEffects.removeEffects(entry.getKey().getId(), entry.getValue()); + break; + default: + layeredEffects.removeEffects(entry.getKey().getId(), entry.getValue()); + break; } - } - for (UUID effectId: effectsToRemove) { - sources.remove(effectId); - } - return effects; - } + } + temporaryEffects.clear(); + } public Map getReplacementEffectsTexts(HashMap> rEffects, Game game) { Map texts = new LinkedHashMap<>(); diff --git a/Mage/src/mage/abilities/effects/ContinuousEffectsList.java b/Mage/src/mage/abilities/effects/ContinuousEffectsList.java index 19b16f73ef1..5c801474d63 100644 --- a/Mage/src/mage/abilities/effects/ContinuousEffectsList.java +++ b/Mage/src/mage/abilities/effects/ContinuousEffectsList.java @@ -32,8 +32,10 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.Map; +import java.util.Set; import java.util.UUID; import mage.abilities.Ability; +import mage.abilities.MageSingleton; import mage.constants.Duration; import mage.game.Game; import org.apache.log4j.Logger; @@ -111,23 +113,25 @@ public class ContinuousEffectsList extends ArrayList Ability ability = it.next(); if (ability == null) { it.remove(); + } else if (ability instanceof MageSingleton) { + return false; } else if (effect.isDiscarded()) { it.remove(); + } else if (game.getObject(ability.getSourceId()) == null) { + logger.debug("Ability without source object removed: " + ability.getSourceId() + " " + ability.getRule()); + it.remove(); // if the related source object does no longer exist the effect has to be removed } else { switch(effect.getDuration()) { - case WhileOnBattlefield: - if (game.getObject(ability.getSourceId()) == null) {//TODO: does this really works?? object is returned across the game - it.remove(); - } - break; case OneUse: if (effect.isUsed()) { + logger.debug("Ability one use removed: " + ability.getSourceId() + " " + ability.getRule()); it.remove(); } break; case Custom: case UntilYourNextTurn: if (effect.isInactive(ability , game)) { + logger.debug("Ability custom removed: " + ability.getSourceId() + " " + ability.getRule()); it.remove(); } } @@ -165,34 +169,21 @@ public class ContinuousEffectsList extends ArrayList return effectAbilityMap.get(effectId); } - /** - * Removes an effect and / or a connected ability. - * If no ability for this effect is left in the effectAbilityMap, the effect will be removed. - * Otherwise the effect won't be removed. - * - * @param effect - effect to remove if all abilities are removed - * @param ability - ability to remove - */ - - public void removeEffect(T effect, Ability ability) { - for (Iterator i = this.iterator(); i.hasNext();) { - T entry = i.next(); - if (entry.equals(effect)) { - HashSet abilities = effectAbilityMap.get(effect.getId()); - if (!abilities.isEmpty()) { - abilities.remove(ability); - } - if (abilities.isEmpty()) { - i.remove(); + public void removeEffects(UUID effectIdToRemove, Set abilitiesToRemove) { + HashSet abilities = effectAbilityMap.get(effectIdToRemove); + abilities.removeAll(abilitiesToRemove); + if (abilities.isEmpty()) { + for (Iterator iterator = this.iterator(); iterator.hasNext();) { + ContinuousEffect effect = iterator.next(); + if (effect.getId().equals(effectIdToRemove)) { + iterator.remove(); + break; } } + effectAbilityMap.remove(effectIdToRemove); } } - public void removeEffectAbilityMap(UUID effectId) { - effectAbilityMap.remove(effectId); - } - @Override public void clear() { super.clear(); diff --git a/Mage/src/mage/abilities/effects/EntersBattlefieldEffect.java b/Mage/src/mage/abilities/effects/EntersBattlefieldEffect.java index 894f084ea10..c80ce8d095f 100644 --- a/Mage/src/mage/abilities/effects/EntersBattlefieldEffect.java +++ b/Mage/src/mage/abilities/effects/EntersBattlefieldEffect.java @@ -69,7 +69,7 @@ public class EntersBattlefieldEffect extends ReplacementEffectImpl { } public EntersBattlefieldEffect(Effect baseEffect, Condition condition, String text, boolean selfScope, boolean optional) { - super(Duration.OneUse, baseEffect.getOutcome(), selfScope); + super(Duration.WhileOnBattlefield, baseEffect.getOutcome(), selfScope); this.baseEffects.add(baseEffect); this.text = text; this.condition = condition; @@ -124,12 +124,6 @@ public class EntersBattlefieldEffect extends ReplacementEffectImpl { game.addEffect((ContinuousEffect) effect, source); } else { - // noxx: commented it out because of resulting in a bug - // with CopyEffect (PhantasmalImageTest.java) - /*if (spell != null) - effect.apply(game, spell.getSpellAbility()); - else - effect.apply(game, source);*/ if (spell != null) { effect.setValue(SOURCE_CAST_SPELL_ABILITY, spell.getSpellAbility()); } diff --git a/Mage/src/mage/abilities/effects/common/CopyEffect.java b/Mage/src/mage/abilities/effects/common/CopyEffect.java index e696baa5619..3098fb03fbc 100644 --- a/Mage/src/mage/abilities/effects/common/CopyEffect.java +++ b/Mage/src/mage/abilities/effects/common/CopyEffect.java @@ -28,8 +28,10 @@ package mage.abilities.effects.common; + import java.util.UUID; import mage.MageObject; +import mage.MageObjectReference; import mage.abilities.Ability; import mage.abilities.effects.ContinuousEffectImpl; import mage.cards.Card; @@ -55,7 +57,6 @@ public class CopyEffect extends ContinuousEffectImpl { */ private MageObject target; private UUID sourceId; - private int zoneChangeCounter; private ApplyToPermanent applier; public CopyEffect(MageObject target, UUID sourceId) { @@ -72,23 +73,27 @@ public class CopyEffect extends ContinuousEffectImpl { super(effect); this.target = effect.target.copy(); this.sourceId = effect.sourceId; - this.zoneChangeCounter = effect.zoneChangeCounter; this.applier = effect.applier; } @Override public void init(Ability source, Game game) { super.init(source, game); - Permanent permanent = game.getPermanent(sourceId); - if (permanent != null) { - this.zoneChangeCounter = permanent.getZoneChangeCounter(); + if (affectedObjectsSet) { + affectedObjectList.add(new MageObjectReference(source.getSourceId(), game)); } } @Override public boolean apply(Game game, Ability source) { Permanent permanent = game.getPermanent(this.sourceId); + if (affectedObjectsSet) { + permanent = affectedObjectList.get(0).getPermanent(game); + } else { + permanent = game.getPermanent(this.sourceId); + } if (permanent == null) { + discard(); return false; } permanent.setName(target.getName()); @@ -107,9 +112,10 @@ public class CopyEffect extends ContinuousEffectImpl { for (String type: target.getSupertype()) { permanent.getSupertype().add(type); } + permanent.removeAllAbilities(source.getSourceId(), game); for (Ability ability: target.getAbilities()) { - permanent.addAbility(ability, game); + permanent.addAbility(ability, getSourceId(), game, false); // no new Id so consumed replacement effects are known while new continuousEffects.apply happen. } permanent.getPower().setValue(target.getPower().getValue()); permanent.getToughness().setValue(target.getToughness().getValue()); @@ -131,17 +137,9 @@ public class CopyEffect extends ContinuousEffectImpl { } permanent.setCopy(true); - return true; } - @Override - public boolean isInactive(Ability source, Game game) { - // The copy effect is added, if the copy takes place. If source left battlefield, the copy effect has cease to exist - Permanent permanent = game.getPermanent(this.sourceId); - return permanent == null || permanent.getZoneChangeCounter() != this.zoneChangeCounter; - } - @Override public CopyEffect copy() { return new CopyEffect(this); @@ -166,6 +164,5 @@ public class CopyEffect extends ContinuousEffectImpl { public void setApplier(ApplyToPermanent applier) { this.applier = applier; } - - + } diff --git a/Mage/src/mage/abilities/effects/common/continious/AddCardSubTypeTargetEffect.java b/Mage/src/mage/abilities/effects/common/continious/AddCardSubTypeTargetEffect.java index d8b4707c7de..27e92de2adf 100644 --- a/Mage/src/mage/abilities/effects/common/continious/AddCardSubTypeTargetEffect.java +++ b/Mage/src/mage/abilities/effects/common/continious/AddCardSubTypeTargetEffect.java @@ -42,7 +42,8 @@ import mage.game.permanent.Permanent; * @author nantuko */ public class AddCardSubTypeTargetEffect extends ContinuousEffectImpl { - private String addedSubType; + + private final String addedSubType; public AddCardSubTypeTargetEffect(String addedSubType, Duration duration) { super(duration, Layer.TypeChangingEffects_4, SubLayer.NA, Outcome.Benefit); @@ -61,6 +62,10 @@ public class AddCardSubTypeTargetEffect extends ContinuousEffectImpl { if (!target.hasSubtype(addedSubType)) { target.getSubtype().add(addedSubType); } + } else { + if (Duration.Custom.equals(duration)) { + discard(); + } } return false; } diff --git a/Mage/src/mage/abilities/keyword/SuspendAbility.java b/Mage/src/mage/abilities/keyword/SuspendAbility.java index ebb2fae4a85..4f1e82e9a7b 100644 --- a/Mage/src/mage/abilities/keyword/SuspendAbility.java +++ b/Mage/src/mage/abilities/keyword/SuspendAbility.java @@ -339,10 +339,6 @@ class SuspendPlayCardEffect extends OneShotEffect { abilitiesToRemove.add(ability); } } - // remove the triggered abilities from the game - game.getState().resetTriggersForSourceId(card.getId()); - // remove the continious effects from the game - game.getState().getContinuousEffects().removeGainedEffectsForSource(card.getId()); // remove the abilities from the card card.getAbilities().removeAll(abilitiesToRemove); } diff --git a/Mage/src/mage/cards/CardImpl.java b/Mage/src/mage/cards/CardImpl.java index 042f8c06585..a69b43d715a 100644 --- a/Mage/src/mage/cards/CardImpl.java +++ b/Mage/src/mage/cards/CardImpl.java @@ -388,7 +388,6 @@ public abstract class CardImpl extends MageObjectImpl implements Card { break; case BATTLEFIELD: PermanentCard permanent = new PermanentCard(this, event.getPlayerId()); // controller can be replaced (e.g. Gather Specimens) - game.resetForSourceId(permanent.getId()); game.addPermanent(permanent); game.setZone(objectId, Zone.BATTLEFIELD); game.setScopeRelevant(true); @@ -555,7 +554,7 @@ public abstract class CardImpl extends MageObjectImpl implements Card { updateZoneChangeCounter(); PermanentCard permanent = new PermanentCard(this, event.getPlayerId()); // reset is done to end continuous effects from previous instances of that permanent (e.g undying) - game.resetForSourceId(permanent.getId()); + // game.resetForSourceId(permanent.getId()); // make sure the controller of all continuous effects of this card are switched to the current controller game.getContinuousEffects().setController(objectId, event.getPlayerId()); game.addPermanent(permanent); diff --git a/Mage/src/mage/game/Game.java b/Mage/src/mage/game/Game.java index 3876457898d..0b6609dd59b 100644 --- a/Mage/src/mage/game/Game.java +++ b/Mage/src/mage/game/Game.java @@ -148,7 +148,6 @@ public interface Game extends MageItem, Serializable { Player getLosingPlayer(); void setStateCheckRequired(); boolean getStateCheckRequired(); - void resetForSourceId(UUID sourceId); //client event methods void addTableEventListener(Listener listener); diff --git a/Mage/src/mage/game/GameImpl.java b/Mage/src/mage/game/GameImpl.java index f83eb6fc59a..4467a77f343 100644 --- a/Mage/src/mage/game/GameImpl.java +++ b/Mage/src/mage/game/GameImpl.java @@ -57,7 +57,6 @@ import mage.abilities.effects.ContinuousEffects; import mage.abilities.effects.Effect; import mage.abilities.effects.PreventionEffectData; import mage.abilities.effects.common.CopyEffect; -import mage.abilities.effects.common.continious.SourceEffect; import mage.abilities.keyword.LeylineAbility; import mage.abilities.keyword.MorphAbility; import mage.abilities.keyword.TransformAbility; @@ -2263,23 +2262,6 @@ public abstract class GameImpl implements Game, Serializable { shortLivingLKI.clear(); } - @Override - public void resetForSourceId(UUID sourceId) { - // make sure that all effects don't touch this card once it returns back to battlefield - // e.g. this prevents that effects affect creature with undying return from graveyard - for (ContinuousEffect effect : getContinuousEffects().getLayeredEffects(this)) { - if (effect.getAffectedObjects().contains(sourceId)) { - effect.getAffectedObjects().remove(sourceId); - if (effect instanceof SourceEffect) { - effect.discard(); - } - } - } - getContinuousEffects().removeGainedEffectsForSource(sourceId); - // remove gained triggered abilities - getState().resetTriggersForSourceId(sourceId); - } - @Override public void cheat(UUID ownerId, Map commands) { if (commands != null) { diff --git a/Mage/src/mage/game/GameState.java b/Mage/src/mage/game/GameState.java index d763658636c..fc4575202de 100644 --- a/Mage/src/mage/game/GameState.java +++ b/Mage/src/mage/game/GameState.java @@ -58,6 +58,7 @@ import mage.watchers.Watchers; import java.io.Serializable; import java.util.*; +import org.apache.log4j.Logger; /** * @@ -397,6 +398,7 @@ public class GameState implements Serializable, Copyable { } public void applyEffects(Game game) { + Logger.getLogger(GameState.class).debug("Apply Effects turn: " + game.getTurnNum() + " - Step: " + (game.getStep() == null ? "null":game.getStep().getType().toString())); for (Player player: players.values()) { player.reset(); } @@ -508,6 +510,7 @@ public class GameState implements Serializable, Copyable { eventsToHandle.addAll(simultaneousEvents); simultaneousEvents.clear(); for (GameEvent event:eventsToHandle) { + Logger.getLogger(GameState.class).debug("Handle Simultanous events - type: " + event.getType().toString() + " sourceId " + event.getSourceId() + " targetId " + event.getTargetId()); this.handleEvent(event, game); } } @@ -551,7 +554,12 @@ public class GameState implements Serializable, Copyable { } } - @Deprecated + /** + * Used for adding abilities that exist permanent on cards/permanents and are not + * only gained for a certain time (e.g. until end of turn). + * @param ability + * @param attachedTo + */ public void addAbility(Ability ability, MageObject attachedTo) { if (ability instanceof StaticAbility) { for (Mode mode: ability.getModes().values()) { @@ -567,6 +575,12 @@ public class GameState implements Serializable, Copyable { } } + /** + * Abilities that are applied to other objects or applie for a certain time span + * @param ability + * @param sourceId + * @param attachedTo + */ public void addAbility(Ability ability, UUID sourceId, MageObject attachedTo) { if (ability instanceof StaticAbility) { for (Mode mode: ability.getModes().values()) { @@ -678,18 +692,23 @@ public class GameState implements Serializable, Copyable { } /** - * Removes Triggered abilities that were gained from sourceId + * Removes Triggered abilities that belong to sourceId + * This is used if a token leaves the battlefield * * @param sourceId */ - public void resetTriggersForSourceId(UUID sourceId) { - List keysToRemove = triggers.removeGainedAbilitiesForSource(sourceId); - for (String key : keysToRemove) { - triggers.remove(key); - } + public void removeTriggersOfSourceId(UUID sourceId) { + triggers.removeAbilitiesOfSource(sourceId); } + + /** + * Called before applyEffects + */ private void reset() { + // All gained abilities have to be removed to prevent adding it multiple times + triggers.removeAllGainedAbilities(); + getContinuousEffects().removeAllTemporaryEffects(); this.setLegendaryRuleActive(true); this.resetOtherAbilities(); } @@ -740,4 +759,11 @@ public class GameState implements Serializable, Copyable { this.legendaryRuleActive = legendaryRuleActive; } + /** + * Used for diagbostic purposes + * @return + */ + public TriggeredAbilities getTriggers() { + return triggers; + } } diff --git a/Mage/src/mage/game/permanent/Permanent.java b/Mage/src/mage/game/permanent/Permanent.java index fcf59f6ef0a..562f89ec536 100644 --- a/Mage/src/mage/game/permanent/Permanent.java +++ b/Mage/src/mage/game/permanent/Permanent.java @@ -113,7 +113,8 @@ public interface Permanent extends Card, Controllable { @Deprecated void addAbility(Ability ability, Game game); void addAbility(Ability ability, UUID sourceId, Game game); - + void addAbility(Ability ability, UUID sourceId, Game game, boolean createNewId); + void removeAllAbilities(UUID sourceId, Game game); void addLoyaltyUsed(); diff --git a/Mage/src/mage/game/permanent/PermanentImpl.java b/Mage/src/mage/game/permanent/PermanentImpl.java index 2a013f66625..2b6ab80f4c4 100644 --- a/Mage/src/mage/game/permanent/PermanentImpl.java +++ b/Mage/src/mage/game/permanent/PermanentImpl.java @@ -204,12 +204,18 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { abilities.add(copyAbility); } } - @Override public void addAbility(Ability ability, UUID sourceId, Game game) { + addAbility(ability, sourceId, game, true); + } + + @Override + public void addAbility(Ability ability, UUID sourceId, Game game, boolean createNewId) { if (!abilities.containsKey(ability.getId())) { Ability copyAbility = ability.copy(); - copyAbility.newId(); // needed so that source can get an ability multiple times (e.g. Raging Ravine) + if (createNewId) { + copyAbility.newId(); // needed so that source can get an ability multiple times (e.g. Raging Ravine) + } copyAbility.setControllerId(controllerId); copyAbility.setSourceId(objectId); game.getState().addAbility(copyAbility, sourceId, this); @@ -220,10 +226,6 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { @Override public void removeAllAbilities(UUID sourceId, Game game) { getAbilities().clear(); - // removes abilities that were gained from abilities of this permanent - game.getContinuousEffects().removeGainedEffectsForSource(this.getId()); - // remove gained triggered abilities - game.getState().resetTriggersForSourceId(this.getId()); } @Override diff --git a/Mage/src/mage/game/permanent/PermanentToken.java b/Mage/src/mage/game/permanent/PermanentToken.java index 19c8e5216b7..8351ce0c0d8 100644 --- a/Mage/src/mage/game/permanent/PermanentToken.java +++ b/Mage/src/mage/game/permanent/PermanentToken.java @@ -49,7 +49,7 @@ public class PermanentToken extends PermanentImpl { super(controllerId, controllerId, token.getName()); this.expansionSetCode = expansionSetCode; this.token = token; - this.copyFromToken(this.token, game); // needed to have e.g. subtypes for entersTheBattlefield replacement effects + this.copyFromToken(this.token, game, false); // needed to have e.g. subtypes for entersTheBattlefield replacement effects } public PermanentToken(final PermanentToken permanent) { @@ -61,16 +61,21 @@ public class PermanentToken extends PermanentImpl { @Override public void reset(Game game) { Token tokenCopy = token.copy(); - copyFromToken(tokenCopy, game); + copyFromToken(tokenCopy, game, true); super.reset(game); } - private void copyFromToken(Token token, Game game) { + private void copyFromToken(Token token, Game game, boolean reset) { this.name = token.getName(); this.abilities.clear(); - for (Ability ability : token.getAbilities()) { - this.addAbility(ability, game); + if (reset) { + this.abilities.addAll(token.getAbilities()); + } else { + for (Ability ability : token.getAbilities()) { + this.addAbility(ability, game); + } } + this.abilities.setControllerId(this.controllerId); this.manaCost.clear(); for (ManaCost cost: token.getManaCost()) { this.getManaCost().add(cost.copy()); @@ -90,8 +95,7 @@ public class PermanentToken extends PermanentImpl { if (game.getPlayer(controllerId).removeFromBattlefield(this, game)) { game.setZone(objectId, zone); // needed for triggered dies abilities game.fireEvent(new ZoneChangeEvent(this, this.getControllerId(), Zone.BATTLEFIELD, zone)); - game.getState().resetTriggersForSourceId(this.getId());// if token is gone triggered abilities no longer needed - game.getState().getContinuousEffects().removeGainedEffectsForSource(this.getId()); + game.getState().removeTriggersOfSourceId(this.getId());// if token is gone endless triggered abilities have to be removed return true; } } @@ -125,7 +129,7 @@ public class PermanentToken extends PermanentImpl { Ability copyAbility = ability.copy(); copyAbility.setControllerId(controllerId); copyAbility.setSourceId(objectId); - game.getState().addAbility(copyAbility, this.getId(), this); + game.getState().addAbility(copyAbility, this); abilities.add(copyAbility); } } diff --git a/Mage/src/mage/game/stack/Spell.java b/Mage/src/mage/game/stack/Spell.java index 7ce5a47cf54..acf9dddbcea 100644 --- a/Mage/src/mage/game/stack/Spell.java +++ b/Mage/src/mage/game/stack/Spell.java @@ -211,7 +211,7 @@ public class Spell implements StackObject, Card { } } if (game.getState().getZone(card.getId()) == Zone.STACK) { - card.moveToZone(Zone.GRAVEYARD, ability.getId(), game, false); + card.moveToZone(Zone.GRAVEYARD, ability.getSourceId(), game, false); } } return result; @@ -229,7 +229,7 @@ public class Spell implements StackObject, Card { // Otherwise effects like evolve trigger from creature comes into play event card.getCardType().remove(CardType.CREATURE); } - if (card.putOntoBattlefield(game, fromZone, ability.getId(), controllerId)) { + if (card.putOntoBattlefield(game, fromZone, ability.getSourceId(), controllerId)) { if (bestow) { // card will be copied during putOntoBattlefield, so the card of CardPermanent has to be changed // TODO: Find a better way to prevent bestow creatures from being effected by creature affecting abilities @@ -250,7 +250,7 @@ public class Spell implements StackObject, Card { // Aura has no legal target and its a bestow enchantment -> Add it to battlefield as creature if (this.getSpellAbility() instanceof BestowAbility) { updateOptionalCosts(0); - result = card.putOntoBattlefield(game, fromZone, ability.getId(), controllerId); + result = card.putOntoBattlefield(game, fromZone, ability.getSourceId(), controllerId); return result; } else { //20091005 - 608.2b @@ -263,7 +263,7 @@ public class Spell implements StackObject, Card { if (isFaceDown()) { card.setFaceDown(true); } - result = card.putOntoBattlefield(game, fromZone, ability.getId(), controllerId); + result = card.putOntoBattlefield(game, fromZone, ability.getSourceId(), controllerId); return result; } }