From 2e60801df6c6909ce2bc9a4852ec6b312f67abb1 Mon Sep 17 00:00:00 2001 From: LevelX2 Date: Wed, 12 Dec 2012 15:51:54 +0100 Subject: [PATCH] Fixed some problems concerning applying layeres effects in timestamp order. Tests now always successful. 1.Timestamps not distinct. 2. Timestamps not updated when attachments are attached (mainly equipments). --- .../lose/LoseAbilityByEquipmentTest.java | 47 +++++++ .../abilities/effects/ContinuousEffects.java | 64 ++++++--- .../mage/game/permanent/PermanentImpl.java | 125 +++++++++++++----- 3 files changed, 183 insertions(+), 53 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/abilities/lose/LoseAbilityByEquipmentTest.java diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/lose/LoseAbilityByEquipmentTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/lose/LoseAbilityByEquipmentTest.java new file mode 100644 index 0000000000..b42c23b09e --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/lose/LoseAbilityByEquipmentTest.java @@ -0,0 +1,47 @@ +package org.mage.test.cards.abilities.lose; + +import mage.Constants; +import mage.abilities.keyword.FlyingAbility; +import mage.game.permanent.Permanent; +import org.junit.Assert; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +/** + * + * @author LevelX2 + */ +public class LoseAbilityByEquipmentTest extends CardTestPlayerBase { + + /** + * Tests that gaining flying by and after that losing flying by eqipments results in not have flying + */ + @Test + public void testGainVsLoseByEquipmentAbility() { + addCard(Constants.Zone.BATTLEFIELD, playerA, "Silvercoat Lion"); + addCard(Constants.Zone.BATTLEFIELD, playerA, "Plains", 3); + addCard(Constants.Zone.BATTLEFIELD, playerA, "Island", 4); + + addCard(Constants.Zone.HAND, playerA, "Magebane Armor"); // loses Flying + addCard(Constants.Zone.HAND, playerA, "Cobbled Wings"); // gives Flying + + castSpell(1, Constants.PhaseStep.PRECOMBAT_MAIN, playerA, "Magebane Armor"); + castSpell(3, Constants.PhaseStep.PRECOMBAT_MAIN, playerA, "Cobbled Wings"); + activateAbility(3, Constants.PhaseStep.PRECOMBAT_MAIN, playerA, "Equip {1}", "Silvercoat Lion"); // give Flying + activateAbility(3, Constants.PhaseStep.POSTCOMBAT_MAIN, playerA, "Equip {2}", "Silvercoat Lion"); // lose Flying + + setStopAt(3, Constants.PhaseStep.END_TURN); + execute(); + + assertLife(playerA, 20); + Permanent silvercoatLion = getPermanent("Silvercoat Lion", playerA.getId()); + Assert.assertNotNull(silvercoatLion); + Assert.assertEquals("Silvercoat Lion equipments", 2, silvercoatLion.getAttachments().size()); + Assert.assertEquals("Silvercoat Lion power",4, silvercoatLion.getPower().getValue()); + Assert.assertEquals("Silvercoat Lion toughness",6, silvercoatLion.getToughness().getValue()); + + // should NOT have flying + Assert.assertFalse("Silvercoat Lion has flying but shouldn't have",silvercoatLion.getAbilities().contains(FlyingAbility.getInstance())); + } + +} diff --git a/Mage/src/mage/abilities/effects/ContinuousEffects.java b/Mage/src/mage/abilities/effects/ContinuousEffects.java index b9efba3e99..69a7c3bd0a 100644 --- a/Mage/src/mage/abilities/effects/ContinuousEffects.java +++ b/Mage/src/mage/abilities/effects/ContinuousEffects.java @@ -28,6 +28,8 @@ package mage.abilities.effects; +import java.io.Serializable; +import java.util.*; import mage.Constants.AsThoughEffectType; import mage.Constants.Duration; import mage.Constants.Layer; @@ -39,16 +41,14 @@ import mage.game.events.GameEvent; import mage.game.permanent.Permanent; import mage.players.Player; -import java.io.Serializable; -import java.util.*; - - /** * * @author BetaSteward_at_googlemail.com */ public class ContinuousEffects implements Serializable { + private Date lastSetTimestamp; + //transient Continuous effects private ContinuousEffectsList layeredEffects = new ContinuousEffectsList(); private ContinuousEffectsList replacementEffects = new ContinuousEffectsList(); @@ -90,6 +90,7 @@ public class ContinuousEffects implements Serializable { sources.put(entry.getKey(), entry.getValue()); } collectAllEffects(); + lastSetTimestamp = effect.lastSetTimestamp; } private void collectAllEffects() { @@ -116,18 +117,24 @@ public class ContinuousEffects implements Serializable { public Ability getAbility(UUID effectId) { Ability ability = layeredEffects.getAbility(effectId); - if (ability == null) + if (ability == null) { ability = replacementEffects.getAbility(effectId); - if (ability == null) + } + if (ability == null) { ability = preventionEffects.getAbility(effectId); - if (ability == null) + } + if (ability == null) { ability = requirementEffects.getAbility(effectId); - if (ability == null) + } + if (ability == null) { ability = restrictionEffects.getAbility(effectId); - if (ability == null) + } + if (ability == null) { ability = asThoughEffects.getAbility(effectId); - if (ability == null) + } + if (ability == null) { ability = costModificationEffects.getAbility(effectId); + } return ability; } @@ -159,8 +166,9 @@ public class ContinuousEffects implements Serializable { case WhileOnStack: case WhileInGraveyard: Ability ability = layeredEffects.getAbility(effect.getId()); - if (ability.isInUseableZone(game, null, false)) + if (ability.isInUseableZone(game, null, false)) { layerEffects.add(effect); + } break; default: layerEffects.add(effect); @@ -183,18 +191,27 @@ public class ContinuousEffects implements Serializable { for (ContinuousEffect continuousEffect : layerEffects) { // check if it's new, then set timestamp if (!previous.contains(continuousEffect)) { - continuousEffect.setTimestamp(); + setUniqueTimesstamp(continuousEffect); } } previous.clear(); previous.addAll(layerEffects); } + public void setUniqueTimesstamp(ContinuousEffect effect) { + do { + effect.setTimestamp(); + } + while (effect.getTimestamp().equals(lastSetTimestamp)); // prevent to set the same timestamp so logical order is saved + lastSetTimestamp = effect.getTimestamp(); + } + private List filterLayeredEffects(List effects, Layer layer) { List layerEffects = new ArrayList(); for (ContinuousEffect effect: effects) { - if (effect.hasLayer(layer)) + if (effect.hasLayer(layer)) { layerEffects.add(effect); + } } return layerEffects; } @@ -204,8 +221,9 @@ public class ContinuousEffects implements Serializable { for (RequirementEffect effect: requirementEffects) { Ability ability = requirementEffects.getAbility(effect.getId()); if (!(ability instanceof StaticAbility) || ability.isInUseableZone(game, null, false)) { - if (effect.applies(permanent, ability, game)) + if (effect.applies(permanent, ability, game)) { effects.add(effect); + } } } return effects; @@ -216,8 +234,9 @@ public class ContinuousEffects implements Serializable { for (RestrictionEffect effect: restrictionEffects) { Ability ability = restrictionEffects.getAbility(effect.getId()); if (!(ability instanceof StaticAbility) || ability.isInUseableZone(game, permanent, false)) { - if (effect.applies(permanent, ability, game)) + if (effect.applies(permanent, ability, game)) { effects.add(effect); + } } } return effects; @@ -231,10 +250,12 @@ public class ContinuousEffects implements Serializable { */ private List getApplicableReplacementEffects(GameEvent event, Game game) { List replaceEffects = new ArrayList(); - if (planeswalkerRedirectionEffect.applies(event, null, game)) + if (planeswalkerRedirectionEffect.applies(event, null, game)) { replaceEffects.add(planeswalkerRedirectionEffect); - if(auraReplacementEffect.applies(event, null, game)) + } + if(auraReplacementEffect.applies(event, null, game)){ replaceEffects.add(auraReplacementEffect); + } //get all applicable transient Replacement effects for (ReplacementEffect effect: replacementEffects) { Ability ability = replacementEffects.getAbility(effect.getId()); @@ -339,11 +360,13 @@ public class ContinuousEffects implements Serializable { List rEffects = getApplicableReplacementEffects(event, game); for (Iterator i = rEffects.iterator(); i.hasNext();) { ReplacementEffect entry = i.next(); - if (consumed.contains(entry.getId())) + if (consumed.contains(entry.getId())) { i.remove(); + } } - if (rEffects.isEmpty()) + if (rEffects.isEmpty()) { break; + } int index; if (rEffects.size() == 1) { index = 0; @@ -355,8 +378,9 @@ public class ContinuousEffects implements Serializable { } ReplacementEffect rEffect = rEffects.get(index); caught = rEffect.replaceEvent(event, this.getAbility(rEffect.getId()), game); - if (caught) + if (caught) { break; + } consumed.add(rEffect.getId()); game.applyEffects(); } while (true); diff --git a/Mage/src/mage/game/permanent/PermanentImpl.java b/Mage/src/mage/game/permanent/PermanentImpl.java index f133cbb9eb..c2f244e9b0 100644 --- a/Mage/src/mage/game/permanent/PermanentImpl.java +++ b/Mage/src/mage/game/permanent/PermanentImpl.java @@ -28,23 +28,44 @@ package mage.game.permanent; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import mage.Constants; import mage.Constants.AsThoughEffectType; import mage.Constants.CardType; import mage.Constants.Zone; import mage.MageObject; import mage.abilities.Ability; +import mage.abilities.effects.ContinuousEffect; +import mage.abilities.effects.Effect; import mage.abilities.effects.RestrictionEffect; -import mage.abilities.keyword.*; +import mage.abilities.keyword.DeathtouchAbility; +import mage.abilities.keyword.DefenderAbility; +import mage.abilities.keyword.HasteAbility; +import mage.abilities.keyword.HexproofAbility; +import mage.abilities.keyword.InfectAbility; +import mage.abilities.keyword.LifelinkAbility; +import mage.abilities.keyword.ProtectionAbility; +import mage.abilities.keyword.ShroudAbility; +import mage.abilities.keyword.WitherAbility; import mage.cards.CardImpl; import mage.counters.Counter; import mage.counters.CounterType; import mage.counters.Counters; import mage.game.Game; -import mage.game.events.*; +import mage.game.events.DamageCreatureEvent; +import mage.game.events.DamagePlaneswalkerEvent; +import mage.game.events.DamagedCreatureEvent; +import mage.game.events.DamagedPlaneswalkerEvent; +import mage.game.events.GameEvent; import mage.game.events.GameEvent.EventType; import mage.players.Player; -import java.util.*; /** * @author BetaSteward_at_googlemail.com @@ -144,10 +165,12 @@ public abstract class PermanentImpl> extends CardImpl public void reset(Game game) { this.beforeResetControllerId = this.controllerId; this.controllerId = originalControllerId; - if (!controllerId.equals(beforeResetControllerId)) + if (!controllerId.equals(beforeResetControllerId)) { controllerChanged = true; - else + } + else { controllerChanged = false; + } this.maxBlocks = 1; this.minBlockedBy = 1; this.copy = false; @@ -174,22 +197,22 @@ public abstract class PermanentImpl> extends CardImpl @Override public void addAbility(Ability ability, Game game) { if (!abilities.containsKey(ability.getId())) { - Ability copy = ability.copy(); - copy.setControllerId(controllerId); - copy.setSourceId(objectId); - game.getState().addAbility(copy, this); - abilities.add(copy); + Ability copyAbility = ability.copy(); + copyAbility.setControllerId(controllerId); + copyAbility.setSourceId(objectId); + game.getState().addAbility(copyAbility, this); + abilities.add(copyAbility); } } @Override public void addAbility(Ability ability, UUID sourceId, Game game) { if (!abilities.containsKey(ability.getId())) { - Ability copy = ability.copy(); - copy.setControllerId(controllerId); - copy.setSourceId(objectId); - game.getState().addAbility(copy, sourceId, this); - abilities.add(copy); + Ability copyAbility = ability.copy(); + copyAbility.setControllerId(controllerId); + copyAbility.setSourceId(objectId); + game.getState().addAbility(copyAbility, sourceId, this); + abilities.add(copyAbility); } } @@ -240,8 +263,9 @@ public abstract class PermanentImpl> extends CardImpl counters.removeCounter(name, amount); GameEvent event = GameEvent.getEvent(EventType.COUNTER_REMOVED, objectId, controllerId); event.setData(name); - for (int i = 0; i < amount; i++) + for (int i = 0; i < amount; i++) { game.fireEvent(event); + } } @Override @@ -560,6 +584,26 @@ public abstract class PermanentImpl> extends CardImpl } } this.attachedTo = permanentId; + /* + * 20121001 613.6. Within a layer or sublayer, determining which order effects are applied in is + * usually done using a timestamp system. An effect with an earlier timestamp is + * applied before an effect with a later timestamp + * 20121001 613.6d If an Aura, Equipment, or Fortification becomes attached to an object or player, + * the Aura, Equipment, or Fortification receives a new timestamp at that time. + */ + for (Iterator it = this.getAbilities().iterator(); it.hasNext();) { + Ability ability = it.next(); + for (Iterator ite = ability.getEffects(game, Constants.EffectType.CONTINUOUS).iterator(); ite.hasNext();) { + ContinuousEffect effect = (ContinuousEffect) ite.next(); + game.getContinuousEffects().setUniqueTimesstamp(effect); + // It's important is to update timestamp of the copied effect in ContinuousEffects because it does the action + for (ContinuousEffect conEffect: game.getContinuousEffects().getLayeredEffects(game)) { + if (conEffect.getId().equals(effect.getId())) { + game.getContinuousEffects().setUniqueTimesstamp(conEffect); + } + } + } + } } @Override @@ -690,9 +734,9 @@ public abstract class PermanentImpl> extends CardImpl if (source != null && hasProtectionFrom(source, game)) { GameEvent preventEvent = new GameEvent(GameEvent.EventType.PREVENT_DAMAGE, this.objectId, sourceId, this.controllerId, event.getAmount(), false); if (!game.replaceEvent(preventEvent)) { - int damage = event.getAmount(); + int preventedDamage = event.getAmount(); event.setAmount(0); - game.fireEvent(GameEvent.getEvent(GameEvent.EventType.PREVENTED_DAMAGE, this.objectId, sourceId, this.controllerId, damage)); + game.fireEvent(GameEvent.getEvent(GameEvent.EventType.PREVENTED_DAMAGE, this.objectId, sourceId, this.controllerId, preventedDamage)); return 0; } } @@ -715,13 +759,17 @@ public abstract class PermanentImpl> extends CardImpl @Override public boolean canBeTargetedBy(MageObject source, UUID sourceControllerId, Game game) { if (source != null) { - if (abilities.containsKey(ShroudAbility.getInstance().getId())) + if (abilities.containsKey(ShroudAbility.getInstance().getId())) { return false; - if (abilities.containsKey(HexproofAbility.getInstance().getId())) - if (game.getOpponents(controllerId).contains(sourceControllerId)) + } + if (abilities.containsKey(HexproofAbility.getInstance().getId())) { + if (game.getOpponents(controllerId).contains(sourceControllerId)) { return false; - if (hasProtectionFrom(source, game)) + } + } + if (hasProtectionFrom(source, game)) { return false; + } } return true; @@ -730,8 +778,9 @@ public abstract class PermanentImpl> extends CardImpl @Override public boolean hasProtectionFrom(MageObject source, Game game) { for (ProtectionAbility ability : abilities.getProtectionAbilities()) { - if (!ability.canTarget(source, game)) + if (!ability.canTarget(source, game)) { return true; + } } return false; } @@ -801,51 +850,61 @@ public abstract class PermanentImpl> extends CardImpl @Override public boolean canAttack(Game game) { - if (tapped) + if (tapped) { return false; - if (hasSummoningSickness()) + } + if (hasSummoningSickness()) { return false; + } //20101001 - 508.1c for (RestrictionEffect effect : game.getContinuousEffects().getApplicableRestrictionEffects(this, game)) { - if (!effect.canAttack(game)) + if (!effect.canAttack(game)) { return false; + } } - if (abilities.containsKey(DefenderAbility.getInstance().getId()) && !game.getContinuousEffects().asThough(this.objectId, AsThoughEffectType.ATTACK, game)) + if (abilities.containsKey(DefenderAbility.getInstance().getId()) && !game.getContinuousEffects().asThough(this.objectId, AsThoughEffectType.ATTACK, game)) { return false; + } return true; } @Override public boolean canBlock(UUID attackerId, Game game) { - if (tapped) + if (tapped) { return false; + } Permanent attacker = game.getPermanent(attackerId); //20101001 - 509.1b for (RestrictionEffect effect : game.getContinuousEffects().getApplicableRestrictionEffects(this, game)) { - if (!effect.canBlock(attacker, this, game.getContinuousEffects().getAbility(effect.getId()), game)) + if (!effect.canBlock(attacker, this, game.getContinuousEffects().getAbility(effect.getId()), game)) { return false; + } } // check also attacker's restriction effects for (RestrictionEffect effect : game.getContinuousEffects().getApplicableRestrictionEffects(attacker, game)) { /*if (!effect.canBlock(attacker, this, game)) return false;*/ - if (!effect.canBeBlocked(attacker, this, game.getContinuousEffects().getAbility(effect.getId()), game)) + if (!effect.canBeBlocked(attacker, this, game.getContinuousEffects().getAbility(effect.getId()), game)) { return false; + } } - if (attacker != null && attacker.hasProtectionFrom(this, game)) + if (attacker != null && attacker.hasProtectionFrom(this, game)) { return false; + } return true; } @Override public boolean canBlockAny(Game game) { - if (tapped) + if (tapped) { return false; + } //20101001 - 509.1b for (RestrictionEffect effect : game.getContinuousEffects().getApplicableRestrictionEffects(this, game)) { - if (!effect.canBlock(null, this, game.getContinuousEffects().getAbility(effect.getId()), game)) + if (!effect.canBlock(null, this, game.getContinuousEffects().getAbility(effect.getId()), game)) { return false; + } } return true;