Added some test and some minor fixes to effect ability handling.

Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java

@@ -972,9 +972,11 @@ public class ComputerPlayer extends PlayerImpl implements Player {
 
     @Override
     public boolean activateAbility(ActivatedAbility ability, Game game) {
-        for (Target target: ability.getModes().getMode().getTargets()) {
+        if (!isTestMode()) { // Test player already sends target event as he selects the target
-            for (UUID targetId: target.getTargets()) {
+            for (Target target: ability.getModes().getMode().getTargets()) {
-                game.fireEvent(GameEvent.getEvent(EventType.TARGETED, targetId, ability.getId(), ability.getControllerId()));
+                for (UUID targetId: target.getTargets()) {
+                    game.fireEvent(GameEvent.getEvent(EventType.TARGETED, targetId, ability.getId(), ability.getControllerId()));
+                }
             }
         }
         return super.activateAbility(ability, game);

Mage.Sets/src/mage/sets/magic2013/PublicExecution.java

@@ -61,7 +61,6 @@ public class PublicExecution extends CardImpl {
         super(ownerId, 105, "Public Execution", Rarity.UNCOMMON, new CardType[]{CardType.INSTANT}, "{5}{B}");
         this.expansionSetCode = "M13";
 
-
         // Destroy target creature an opponent controls. Each other creature that player controls gets -2/-0 until end of turn.
         this.getSpellAbility().addEffect(new DestroyTargetEffect());
         this.getSpellAbility().addTarget(new TargetCreaturePermanent(filter));

Mage.Sets/src/mage/sets/mirrodin/Shatter.java

@@ -52,6 +52,7 @@ public class Shatter extends CardImpl {
         super(ownerId, 105, "Shatter", Rarity.COMMON, new CardType[]{CardType.INSTANT}, "{1}{R}");
         this.expansionSetCode = "MRD";
 
+        // Destroy target artifact.
         this.getSpellAbility().addEffect(new DestroyTargetEffect());
         this.getSpellAbility().addTarget(new TargetPermanent(filter));
     }

Mage.Sets/src/mage/sets/tenthedition/DemonsHorn.java

@@ -29,16 +29,15 @@
 package mage.sets.tenthedition;
 
 import java.util.UUID;
+import mage.ObjectColor;
 import mage.constants.CardType;
 import mage.constants.Rarity;
-import mage.constants.Zone;
+import mage.abilities.common.SpellCastAllTriggeredAbility;
-import mage.abilities.TriggeredAbilityImpl;
+import mage.abilities.dynamicvalue.common.StaticValue;
 import mage.abilities.effects.common.GainLifeEffect;
 import mage.cards.CardImpl;
-import mage.game.Game;
+import mage.filter.FilterSpell;
-import mage.game.events.GameEvent;
+import mage.filter.predicate.mageobject.ColorPredicate;
-import mage.game.events.GameEvent.EventType;
-import mage.game.stack.Spell;
 
 /**
  *
@@ -46,10 +45,18 @@ import mage.game.stack.Spell;
  */
 public class DemonsHorn extends CardImpl {
 
+    private final static FilterSpell filter = new FilterSpell("a black spell");
+    
+    static {
+        filter.add(new ColorPredicate(ObjectColor.BLACK));
+    }
+    
     public DemonsHorn(UUID ownerId) {
         super(ownerId, 320, "Demon's Horn", Rarity.UNCOMMON, new CardType[]{CardType.ARTIFACT}, "{2}");
         this.expansionSetCode = "10E";
-        this.addAbility(new DemonsHornAbility());
+        
+        // Whenever a player casts a black spell, you may gain 1 life.
+        this.addAbility(new SpellCastAllTriggeredAbility(new GainLifeEffect(new StaticValue(1), "you may gain 1 life"), filter, true));
     }
 
     public DemonsHorn(final DemonsHorn card) {
@@ -62,36 +69,3 @@ public class DemonsHorn extends CardImpl {
     }
 
 }
-
-class DemonsHornAbility extends TriggeredAbilityImpl {
-
-    public DemonsHornAbility() {
-        super(Zone.BATTLEFIELD, new GainLifeEffect(1), true);
-    }
-
-    public DemonsHornAbility(final DemonsHornAbility ability) {
-        super(ability);
-    }
-
-    @Override
-    public DemonsHornAbility copy() {
-        return new DemonsHornAbility(this);
-    }
-
-    @Override
-    public boolean checkTrigger(GameEvent event, Game game) {
-        if (event.getType() == EventType.SPELL_CAST) {
-            Spell spell = game.getStack().getSpell(event.getTargetId());
-            if (spell != null && spell.getColor().isBlack()) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    @Override
-    public String getRule() {
-        return "Whenever a player casts a black spell, you may gain 1 life.";
-    }
-
-}

Mage.Tests/src/test/java/org/mage/test/cards/continuous/MightOfOldKrosaTest.java

@@ -0,0 +1,83 @@
+/*
+ *  Copyright 2010 BetaSteward_at_googlemail.com. All rights reserved.
+ *
+ *  Redistribution and use in source and binary forms, with or without modification, are
+ *  permitted provided that the following conditions are met:
+ *
+ *     1. Redistributions of source code must retain the above copyright notice, this list of
+ *        conditions and the following disclaimer.
+ *
+ *     2. Redistributions in binary form must reproduce the above copyright notice, this list
+ *        of conditions and the following disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY BetaSteward_at_googlemail.com ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ *  WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ *  FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL BetaSteward_at_googlemail.com OR
+ *  CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ *  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ *  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ *  ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ *  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ *  ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ *  The views and conclusions contained in the software and documentation are those of the
+ *  authors and should not be interpreted as representing official policies, either expressed
+ *  or implied, of BetaSteward_at_googlemail.com.
+ */
+package org.mage.test.cards.continuous;
+
+import mage.constants.PhaseStep;
+import mage.constants.Zone;
+import org.junit.Test;
+import org.mage.test.serverside.base.CardTestPlayerBase;
+
+/**
+ *
+ * @author LevelX2
+ */
+
+public class MightOfOldKrosaTest extends CardTestPlayerBase {
+
+    @Test
+    public void testTwiceMightOfOldKrosaBeginCombat() {
+        addCard(Zone.BATTLEFIELD, playerA, "Forest", 2);
+        addCard(Zone.HAND, playerA, "Might of Old Krosa", 2);
+        
+        addCard(Zone.BATTLEFIELD, playerA, "Silvercoat Lion", 1);
+
+        castSpell(1, PhaseStep.BEGIN_COMBAT, playerA, "Might of Old Krosa", "Silvercoat Lion");
+        castSpell(1, PhaseStep.BEGIN_COMBAT, playerA, "Might of Old Krosa", "Silvercoat Lion");
+
+        setStopAt(1, PhaseStep.END_COMBAT); 
+        execute();
+
+        assertGraveyardCount(playerA, "Might of Old Krosa", 2);
+        
+        assertPermanentCount(playerA, "Silvercoat Lion", 1);
+        assertPowerToughness(playerA,  "Silvercoat Lion", 6, 6);
+    }
+    /**
+     * Threw two Might of old Krosa's onto a creature, but only one had any effect.
+     */
+    
+    @Test
+    public void testTwiceMightOfOldKrosa() {
+        addCard(Zone.BATTLEFIELD, playerA, "Forest", 2);
+        addCard(Zone.HAND, playerA, "Might of Old Krosa", 2);
+        
+        addCard(Zone.BATTLEFIELD, playerA, "Silvercoat Lion", 1);
+
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Might of Old Krosa", "Silvercoat Lion");
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Might of Old Krosa", "Silvercoat Lion");
+
+        setStopAt(1, PhaseStep.BEGIN_COMBAT); 
+        execute();
+
+        assertGraveyardCount(playerA, "Might of Old Krosa", 2);
+        
+        assertPermanentCount(playerA, "Silvercoat Lion", 1);
+        assertPowerToughness(playerA,  "Silvercoat Lion", 10, 10);
+    }
+
+}

Mage.Tests/src/test/java/org/mage/test/cards/copy/PhantasmalImageTest.java

@@ -328,7 +328,7 @@ public class PhantasmalImageTest extends CardTestPlayerBase {
         setChoice(playerB, "Frost Titan");
 
         castSpell(2, PhaseStep.POSTCOMBAT_MAIN, playerA, "Terror", "Frost Titan"); // of player Bs Phantasmal Image copying Frost Titan
-                                                                                   // should be countered if not paying {2}
+        // should be countered if not paying {2}
 
         setStopAt(2, PhaseStep.END_TURN);
         execute();
@@ -349,11 +349,10 @@ public class PhantasmalImageTest extends CardTestPlayerBase {
     // When my Phantasmal Image died, it didn't triggered the Wurmcoil Engine's last ability 
     // (When Wurmcoil Engine dies, put a 3/3 colorless Wurm artifact creature token with deathtouch and
     // a 3/3 colorless Wurm artifact creature token with lifelink onto the battlefield.)
-    
     @Test
     public void testDiesTriggeredAbilities() {
         addCard(Zone.BATTLEFIELD, playerA, "Wurmcoil Engine");
-        // Destroy target artifact or enchantment.
+        // Destroy target creature an opponent controls. Each other creature that player controls gets -2/-0 until end of turn.
         addCard(Zone.HAND, playerA, "Public Execution");
         addCard(Zone.BATTLEFIELD, playerA, "Swamp", 6);
 
@@ -364,7 +363,7 @@ public class PhantasmalImageTest extends CardTestPlayerBase {
         setChoice(playerB, "Wurmcoil Engine");
 
         castSpell(2, PhaseStep.POSTCOMBAT_MAIN, playerA, "Public Execution", "Wurmcoil Engine"); // of player Bs Phantasmal Image copying Frost Titan
-                                                                                   // should be countered if not paying {2}
+        // should be countered if not paying {2}
 
         setStopAt(2, PhaseStep.END_TURN);
         execute();
@@ -374,7 +373,6 @@ public class PhantasmalImageTest extends CardTestPlayerBase {
         assertLife(playerB, 20);
         assertLife(playerA, 20);
 
-        
         assertPermanentCount(playerA, "Wurmcoil Engine", 1);
 
         assertGraveyardCount(playerB, "Phantasmal Image", 1);
@@ -382,4 +380,107 @@ public class PhantasmalImageTest extends CardTestPlayerBase {
 
     }
 
+    /**
+     * Phantasmal Image is not regestering Leave the battlefield triggers,
+     * persist and undying triggers
+     */
+    @Test
+    public void testLeavesTheBattlefieldTriggeredAbilities() {
+        // Shadow (This creature can block or be blocked by only creatures with shadow.)
+        // When Thalakos Seer leaves the battlefield, draw a card.
+        addCard(Zone.BATTLEFIELD, playerA, "Thalakos Seer");
+
+        // Destroy target creature an opponent controls. Each other creature that player controls gets -2/-0 until end of turn.
+        addCard(Zone.HAND, playerA, "Public Execution");
+        addCard(Zone.BATTLEFIELD, playerA, "Swamp", 6);
+
+        addCard(Zone.BATTLEFIELD, playerB, "Island", 2);
+        addCard(Zone.HAND, playerB, "Phantasmal Image");
+
+        castSpell(2, PhaseStep.PRECOMBAT_MAIN, playerB, "Phantasmal Image"); // not targeted
+        setChoice(playerB, "Thalakos Seer");
+
+        castSpell(2, PhaseStep.POSTCOMBAT_MAIN, playerA, "Public Execution", "Thalakos Seer");
+
+        setStopAt(2, PhaseStep.END_TURN);
+        execute();
+
+        assertGraveyardCount(playerA, "Public Execution", 1);
+
+        assertLife(playerB, 20);
+        assertLife(playerA, 20);
+
+        assertPermanentCount(playerA, "Thalakos Seer", 1);
+
+        assertGraveyardCount(playerB, "Phantasmal Image", 1);
+
+        assertHandCount(playerB, 2); // 1 from draw turn 2 and 1 from Thalakos Seer leaves the battlefield trigger
+    }
+
+    /**
+     *                    Action
+     *      Game State 1 -----------------> Game State 2
+     *      (On 'field)     (Move to GY)    (In graveyard)
+     *      
+     *  LTB abilities such as Persist are expceptional in that they trigger based on their existence and 
+     *  state of objects before the event (Game State 1, when the card is on the battlefield) rather than 
+     *  after (Game State 2, when the card is in the graveyard). It doesn't matter that the LTB ability
+     *  doesn't exist in Game State 2. [CR 603.6d]
+     * 
+     *  603.6d Normally, objects that exist immediately after an event are checked to see if the event matched any trigger conditions. 
+     *  Continuous effects that exist at that time are used to determine what the trigger conditions are and what the objects involved 
+     *  in the event look like. However, some triggered abilities must be treated specially. Leaves-the-battlefield abilities, abilities
+     *  that trigger when a permanent phases out, abilities that trigger when an object that all players can see is put into a hand or 
+     *  library, abilities that trigger specifically when an object becomes unattached, abilities that trigger when a player loses control 
+     *  of an object, and abilities that trigger when a player planeswalks away from a plane will trigger based on their existence, and 
+     *  the appearance of objects, prior to the event rather than afterward. The game has to “look back in time” to determine if these abilities trigger.
+     * 
+     *  Example: Two creatures are on the battlefield along with an artifact that has the ability “Whenever a creature dies, you gain 1 life.” 
+     *  Someone plays a spell that destroys all artifacts, creatures, and enchantments. The artifact’s ability triggers twice, even though 
+     *  the artifact goes to its owner’s graveyard at the same time as the creatures.
+     * 
+     */
+    @Test
+    public void testPersist() {
+        addCard(Zone.BATTLEFIELD, playerA, "Forest", 3);        
+        // When Kitchen Finks enters the battlefield, you gain 2 life.
+        // Persist (When this creature dies, if it had no -1/-1 counters on it, return it to the battlefield under its owner's control with a -1/-1 counter on it.)
+        addCard(Zone.HAND, playerA, "Kitchen Finks");
+
+        // Destroy target creature an opponent controls. Each other creature that player controls gets -2/-0 until end of turn.
+        addCard(Zone.HAND, playerA, "Public Execution");
+        addCard(Zone.BATTLEFIELD, playerA, "Swamp", 6);
+
+        addCard(Zone.BATTLEFIELD, playerB, "Island", 2);
+        
+        // You may have Phantasmal Image enter the battlefield as a copy of any creature
+        // on the battlefield, except it's an Illusion in addition to its other types and
+        // it gains "When this creature becomes the target of a spell or ability, sacrifice it."        
+        addCard(Zone.HAND, playerB, "Phantasmal Image");
+
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Kitchen Finks"); 
+        
+        castSpell(2, PhaseStep.PRECOMBAT_MAIN, playerB, "Phantasmal Image"); // not targeted
+        setChoice(playerB, "Kitchen Finks");
+        
+
+        castSpell(2, PhaseStep.POSTCOMBAT_MAIN, playerA, "Public Execution", "Kitchen Finks");
+        setChoice(playerB, "Kitchen Finks");
+        
+        setStopAt(2, PhaseStep.END_TURN);
+        execute();
+
+        assertGraveyardCount(playerA, "Public Execution", 1);
+
+        assertLife(playerA, 22);
+        assertLife(playerB, 24);
+
+        assertPermanentCount(playerA, "Kitchen Finks", 1);
+
+        assertHandCount(playerB, "Phantasmal Image", 0);
+        assertGraveyardCount(playerB, "Phantasmal Image", 0);
+        assertPermanentCount(playerB, "Kitchen Finks", 1);
+        assertPowerToughness(playerB,  "Kitchen Finks", 2, 1);
+
+    }
 }

Mage.Tests/src/test/java/org/mage/test/cards/triggers/state/SynodCenturionTest.java

@@ -25,11 +25,15 @@ public class SynodCenturionTest extends CardTestPlayerBase {
     @Test
     public void testAlone() {
         addCard(Zone.BATTLEFIELD, playerA, "Mountain", 6);
+        // Whenever a player casts a black spell, you may gain 1 life.
         addCard(Zone.BATTLEFIELD, playerA, "Demon's Horn");
+        // Destroy target artifact.
         addCard(Zone.HAND, playerA, "Shatter");
+        // When you control no other artifacts, sacrifice Synod Centurion.
         addCard(Zone.HAND, playerA, "Synod Centurion");
 
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Synod Centurion");
+        
         castSpell(1, PhaseStep.POSTCOMBAT_MAIN, playerA, "Shatter", "Demon's Horn");
 
         setStopAt(1, PhaseStep.END_TURN);

Mage/src/mage/abilities/StateTriggeredAbility.java

@@ -32,6 +32,7 @@ import java.util.UUID;
 import mage.abilities.effects.Effect;
 import mage.constants.Zone;
 import mage.game.Game;
+import mage.game.events.GameEvent;
 
 /**
  *
@@ -48,23 +49,29 @@ public abstract class StateTriggeredAbility extends TriggeredAbilityImpl {
     }
 
     @Override
-    public void trigger(Game game, UUID controllerId) {
+    public final boolean checkEventType(GameEvent event, Game game) {
         //20100716 - 603.8
-        Boolean triggered = (Boolean) game.getState().getValue(this.getSourceId().toString() + "triggered");
+        Boolean triggered = (Boolean) game.getState().getValue(getSourceId().toString() + "triggered");
         if (triggered == null) {
             triggered = Boolean.FALSE;
         }
-        if (!triggered) {
+        return !triggered;
-            game.getState().setValue(this.getSourceId().toString() + "triggered", Boolean.TRUE);
+    }
-            super.trigger(game, controllerId);
+
-        }
+    
+    @Override
+    public void trigger(Game game, UUID controllerId) {
+        //20100716 - 603.8
+        game.getState().setValue(this.getSourceId().toString() + "triggered", Boolean.TRUE);
+        super.trigger(game, controllerId);
     }
 
     @Override
     public boolean resolve(Game game) {
         //20100716 - 603.8
+        boolean result = super.resolve(game);
         game.getState().setValue(this.getSourceId().toString() + "triggered", Boolean.FALSE);
-        return super.resolve(game);
+        return result;        
     }
 
     public void counter(Game game) {

Mage/src/mage/abilities/common/BecomesTargetTriggeredAbility.java

@@ -32,7 +32,6 @@ import mage.abilities.TriggeredAbilityImpl;
 import mage.abilities.effects.Effect;
 import mage.game.Game;
 import mage.game.events.GameEvent;
-import mage.game.events.GameEvent.EventType;
 
 /**
  *
@@ -60,7 +59,7 @@ public class BecomesTargetTriggeredAbility extends TriggeredAbilityImpl {
 
     @Override
     public boolean checkTrigger(GameEvent event, Game game) {
-        return event.getTargetId().equals(sourceId);
+        return event.getTargetId().equals(getSourceId());
     }
 
     @Override

Mage/src/mage/abilities/effects/common/SacrificeSourceEffect.java

@@ -28,6 +28,7 @@
 
 package mage.abilities.effects.common;
 
+import mage.MageObject;
 import mage.abilities.Ability;
 import mage.abilities.effects.OneShotEffect;
 import mage.constants.Outcome;
@@ -56,13 +57,17 @@ public class SacrificeSourceEffect extends OneShotEffect {
 
     @Override
     public boolean apply(Game game, Ability source) {
-        Permanent permanent = game.getPermanent(source.getSourceId());
+        MageObject sourceObject = source.getSourceObjectIfItStillExists(game);
-        if (permanent != null) {
+        if (sourceObject instanceof Permanent) {
+            Permanent permanent = (Permanent) sourceObject;
             // you can only sacrifice a permanent you control
             if (source.getControllerId().equals(permanent.getControllerId())) {
                 return permanent.sacrifice(source.getSourceId(), game);
             }
             return true;
+        } else {
+            // no permanent?
+            sourceObject.getName();
         }
         return false;
     }

Mage/src/mage/abilities/keyword/PersistAbility.java

@@ -35,10 +35,11 @@ import mage.abilities.effects.ReplacementEffectImpl;
 import mage.abilities.effects.common.ReturnSourceFromGraveyardToBattlefieldEffect;
 import mage.constants.Duration;
 import mage.constants.Outcome;
-import mage.constants.Zone;
 import mage.counters.CounterType;
 import mage.game.Game;
+import mage.game.events.EntersTheBattlefieldEvent;
 import mage.game.events.GameEvent;
+import mage.game.events.ZoneChangeEvent;
 import mage.game.permanent.Permanent;
 import mage.target.targetpointer.FixedTarget;
 
@@ -68,9 +69,11 @@ public class PersistAbility extends DiesTriggeredAbility {
     @Override
     public boolean checkTrigger(GameEvent event, Game game) {
         if (super.checkTrigger(event, game)) {
-            Permanent p = (Permanent) game.getLastKnownInformation(event.getTargetId(), Zone.BATTLEFIELD);
+            Permanent permanent = ((ZoneChangeEvent) event).getTarget();
-            if (p.getCounters().getCount(CounterType.M1M1) == 0) {
+            if (permanent.getCounters().getCount(CounterType.M1M1) == 0) {
-                game.getState().setValue("persist" + getSourceId().toString(), new FixedTarget(p.getId()));
+                FixedTarget fixedTarget = new FixedTarget(permanent.getId());
+                fixedTarget.init(game, this);
+                game.getState().setValue("persist" + getSourceId().toString(), fixedTarget);
                 return true;
             }
         }
@@ -109,7 +112,7 @@ class PersistEffect extends OneShotEffect {
 class PersistReplacementEffect extends ReplacementEffectImpl {
 
     PersistReplacementEffect() {
-        super(Duration.OneUse, Outcome.UnboostCreature, false);
+        super(Duration.Custom, Outcome.UnboostCreature, false);
         selfScope = true;
         staticText = "return it to the battlefield under its owner's control with a -1/-1 counter on it";
     }
@@ -129,7 +132,7 @@ class PersistReplacementEffect extends ReplacementEffectImpl {
         if (permanent != null) {
             permanent.addCounters(CounterType.M1M1.createInstance(), game);
         }
-        used = true;
+        discard();
         return false;
     }
 
@@ -142,7 +145,9 @@ class PersistReplacementEffect extends ReplacementEffectImpl {
     public boolean applies(GameEvent event, Ability source, Game game) {
         if (event.getTargetId().equals(source.getSourceId())) {
             Object fixedTarget = game.getState().getValue("persist" + source.getSourceId().toString());
-            if (fixedTarget instanceof FixedTarget && ((FixedTarget) fixedTarget).getFirst(game, source).equals(source.getSourceId())) {
+            if (fixedTarget instanceof FixedTarget && ((FixedTarget) fixedTarget).getTarget().equals(source.getSourceId()) &&
+                    ((FixedTarget) fixedTarget).getZoneChangeCounter() + 1 == game.getState().getZoneChangeCounter(source.getSourceId())) {
+                
                 return true;
             }
         }