From bb5b9587e05064cc04c6a24b75ff8fbce6ca26d1 Mon Sep 17 00:00:00 2001 From: LevelX2 Date: Wed, 5 Dec 2012 01:16:28 +0100 Subject: [PATCH] Fixed the causes that triggered abilities were applied more often than they should. Fixed the lose ability bug (test with Master of the Pearl Trident giving island walk). Tests now build without errors. Only rarely the Grounded/Drake Umbra lose ability test fails. --- .../cards/abilities/lose/LoseAbilityTest.java | 21 +++++++ .../mage/abilities/TriggeredAbilities.java | 62 ++++++++++++++++--- .../continious/GainAbilityAllEffect.java | 2 +- .../continious/GainAbilityAttachedEffect.java | 2 +- .../continious/GainAbilityPairedEffect.java | 2 +- .../continious/GainAbilityTargetEffect.java | 2 +- .../continious/LoseAbilityAttachedEffect.java | 8 ++- Mage/src/mage/game/GameImpl.java | 6 +- Mage/src/mage/game/GameState.java | 32 +++++++--- .../mage/game/permanent/PermanentCard.java | 9 ++- .../mage/game/permanent/PermanentImpl.java | 2 + 11 files changed, 119 insertions(+), 29 deletions(-) diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/lose/LoseAbilityTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/lose/LoseAbilityTest.java index c4c15a11ec1..e1ec48eebba 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/lose/LoseAbilityTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/lose/LoseAbilityTest.java @@ -96,4 +96,25 @@ public class LoseAbilityTest extends CardTestPlayerBase { // should NOT have flying Assert.assertFalse(airElemental.getAbilities().contains(FlyingAbility.getInstance())); } + /** + * Tests that gaining two times a triggered ability and losing one will result in only one triggering + */ + @Test + public void testMultiGainTriggeredVsLoseAbility() { + addCard(Constants.Zone.BATTLEFIELD, playerA, "Sublime Archangel",2); + addCard(Constants.Zone.BATTLEFIELD, playerA, "Silvercoat Lion"); + addCard(Constants.Zone.BATTLEFIELD, playerA, "Plains", 3); + addCard(Constants.Zone.BATTLEFIELD, playerA, "Island", 3); + addCard(Constants.Zone.HAND, playerA, "Turn to Frog"); + addCard(Constants.Zone.BATTLEFIELD, playerB, "Island", 5); + + castSpell(3, Constants.PhaseStep.PRECOMBAT_MAIN, playerA, "Turn to Frog", "Sublime Archangel"); + attack(3, playerA, "Silvercoat Lion"); + + setStopAt(3, Constants.PhaseStep.END_COMBAT); + execute(); + + assertLife(playerA, 20); + assertLife(playerB, 16); + } } diff --git a/Mage/src/mage/abilities/TriggeredAbilities.java b/Mage/src/mage/abilities/TriggeredAbilities.java index ba3fc39f1cc..594f4ff91be 100644 --- a/Mage/src/mage/abilities/TriggeredAbilities.java +++ b/Mage/src/mage/abilities/TriggeredAbilities.java @@ -1,3 +1,4 @@ + /* * Copyright 2010 BetaSteward_at_googlemail.com. All rights reserved. * @@ -28,28 +29,36 @@ package mage.abilities; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.UUID; import mage.MageObject; import mage.game.Game; import mage.game.events.GameEvent; import mage.game.permanent.Permanent; import mage.game.permanent.PermanentCard; -import java.util.HashMap; -import java.util.Map; -import java.util.UUID; /** - * - * @author BetaSteward_at_googlemail.com - */ +* +* @author BetaSteward_at_googlemail.com +*/ public class TriggeredAbilities extends HashMap { + private Map> sources = new HashMap>(); + public TriggeredAbilities() {} public TriggeredAbilities(final TriggeredAbilities abilities) { for (Map.Entry entry: abilities.entrySet()) { this.put(entry.getKey(), entry.getValue().copy()); } + for (Map.Entry> entry : abilities.sources.entrySet()) { + sources.put(entry.getKey(), entry.getValue()); + } } public void checkTriggers(GameEvent event, Game game) { @@ -78,7 +87,7 @@ public class TriggeredAbilities extends HashMap { if (object instanceof PermanentCard) { PermanentCard permanent = (PermanentCard)object; if (permanent.canTransform() && event.getType() == GameEvent.EventType.TRANSFORMED) { - exists = permanent.getCard().getAbilities().contains(ability); + exists = permanent.getCard().getAbilities().contains(ability); } } } @@ -96,10 +105,26 @@ public class TriggeredAbilities extends HashMap { return object; } + /** + * Adds a by sourceId gained triggered ability + * + * @param ability - the gained ability + * @param sourceId - the source that assigned the ability + * @param attachedTo - the object that gained the ability + */ + public void add(TriggeredAbility ability, UUID sourceId, MageObject attachedTo) { + this.add(ability, attachedTo); + List uuidList = new LinkedList(); + uuidList.add(sourceId); + // if the object that gained the ability moves zone, also then the triggered ability must be removed + uuidList.add(attachedTo.getId()); + sources.put(getKey(ability, attachedTo), uuidList); + } + public void add(TriggeredAbility ability, MageObject attachedTo) { this.put(getKey(ability, attachedTo), ability); } - + private String getKey(TriggeredAbility ability, MageObject target) { String key = ability.getId() + "_"; if (target != null) { @@ -108,8 +133,27 @@ public class TriggeredAbilities extends HashMap { return key; } + /** + * Removes gained abilities by sourceId + * + * @param sourceId + */ + public List removeGainedAbilitiesForSource(UUID sourceId) { + List keysToRemove = new ArrayList(); + + for (Map.Entry> entry : sources.entrySet()) { + if (entry.getValue().contains(sourceId)) { + keysToRemove.add(entry.getKey()); + } + } + for (String key: keysToRemove) { + sources.remove(key); + } + return keysToRemove; + } + public TriggeredAbilities copy() { return new TriggeredAbilities(this); } -} +} \ No newline at end of file diff --git a/Mage/src/mage/abilities/effects/common/continious/GainAbilityAllEffect.java b/Mage/src/mage/abilities/effects/common/continious/GainAbilityAllEffect.java index ec1c51cc028..66eecc8a005 100644 --- a/Mage/src/mage/abilities/effects/common/continious/GainAbilityAllEffect.java +++ b/Mage/src/mage/abilities/effects/common/continious/GainAbilityAllEffect.java @@ -98,7 +98,7 @@ public class GainAbilityAllEffect extends ContinuousEffectImpl { + private final static Logger logger = Logger.getLogger(LoseAbilityAttachedEffect.class); + protected Ability ability; protected AttachmentType attachmentType; @@ -68,7 +71,10 @@ public class LoseAbilityAttachedEffect extends ContinuousEffectImpl> implements Game, Serializa return null; return gameCards.get(cardId); } - + @Override public Ability getAbility(UUID abilityId, UUID sourceId) { MageObject object = getObject(sourceId); @@ -1456,6 +1456,8 @@ public abstract class GameImpl> implements Game, Serializa } } getContinuousEffects().removeGainedEffectsForSource(sourceId); + // remove gained triggered abilities + getState().resetForSourceId(sourceId); } @Override @@ -1533,7 +1535,7 @@ public abstract class GameImpl> implements Game, Serializa card.setOwnerId(ownerId); PermanentCard permanent = new PermanentCard(card.getCard(), ownerId); getBattlefield().addPermanent(permanent); - permanent.entersBattlefield(permanent.getId(), this); + permanent.entersBattlefield(permanent.getId(), this); ((PermanentImpl)permanent).removeSummoningSickness(); if (card.isTapped()) permanent.setTapped(true); diff --git a/Mage/src/mage/game/GameState.java b/Mage/src/mage/game/GameState.java index ac978502e9c..7067617d40d 100644 --- a/Mage/src/mage/game/GameState.java +++ b/Mage/src/mage/game/GameState.java @@ -60,15 +60,15 @@ import java.io.Serializable; import java.util.*; /** - * - * @author BetaSteward_at_googlemail.com - * - * since at any time the game state may be copied and restored you cannot - * rely on any object maintaining it's instance - * it then becomes necessary to only refer to objects by their ids since - * these will always remain constant throughout its lifetime - * - */ +* +* @author BetaSteward_at_googlemail.com +* +* since at any time the game state may be copied and restored you cannot +* rely on any object maintaining it's instance +* it then becomes necessary to only refer to objects by their ids since +* these will always remain constant throughout its lifetime +* +*/ public class GameState implements Serializable, Copyable { private Players players; @@ -478,7 +478,7 @@ public class GameState implements Serializable, Copyable { } else if (ability instanceof TriggeredAbility) { // TODO: add sources for triggers - the same way as in addEffect: sources - this.triggers.add((TriggeredAbility)ability, attachedTo); + this.triggers.add((TriggeredAbility)ability, sourceId, attachedTo); } } @@ -555,6 +555,18 @@ public class GameState implements Serializable, Copyable { } } + /** + * Removes Triggered abilities that were gained from sourceId + * + * @param sourceId + */ + public void resetForSourceId(UUID sourceId) { + List keysToRemove = triggers.removeGainedAbilitiesForSource(sourceId); + for (String key : keysToRemove) { + triggers.remove(key); + } + } + public void clear() { battlefield.clear(); effects.clear(); diff --git a/Mage/src/mage/game/permanent/PermanentCard.java b/Mage/src/mage/game/permanent/PermanentCard.java index 0fb63b87838..3c47351a703 100644 --- a/Mage/src/mage/game/permanent/PermanentCard.java +++ b/Mage/src/mage/game/permanent/PermanentCard.java @@ -173,9 +173,6 @@ public class PermanentCard extends PermanentImpl { if (controller != null && controller.removeFromBattlefield(this, game)) { ZoneChangeEvent event = new ZoneChangeEvent(this, sourceId, controllerId, fromZone, toZone); if (!game.replaceEvent(event)) { - if (event.getFromZone().equals(Zone.BATTLEFIELD)) { - game.resetForSourceId(getId()); - } Player owner = game.getPlayer(ownerId); game.rememberLKI(objectId, Zone.BATTLEFIELD, this); if (owner != null) { @@ -201,6 +198,9 @@ public class PermanentCard extends PermanentImpl { } game.setZone(objectId, event.getToZone()); game.fireEvent(event); + if (event.getFromZone().equals(Zone.BATTLEFIELD)) { + game.resetForSourceId(getId()); + } return game.getState().getZone(objectId) == toZone; } } @@ -224,6 +224,9 @@ public class PermanentCard extends PermanentImpl { } game.setZone(objectId, event.getToZone()); game.fireEvent(event); + if (event.getFromZone().equals(Zone.BATTLEFIELD)) { + game.resetForSourceId(getId()); + } return true; } } diff --git a/Mage/src/mage/game/permanent/PermanentImpl.java b/Mage/src/mage/game/permanent/PermanentImpl.java index 857d451efff..f133cbb9ebf 100644 --- a/Mage/src/mage/game/permanent/PermanentImpl.java +++ b/Mage/src/mage/game/permanent/PermanentImpl.java @@ -196,6 +196,8 @@ public abstract class PermanentImpl> extends CardImpl @Override public void removeAllAbilities(UUID sourceId, Game game) { getAbilities().clear(); + // removes abilities that were gained from abilities of this permanent + game.resetForSourceId(this.getId()); } @Override