Some code review fixes

forge-game/src/main/java/forge/game/GameAction.java

@@ -2422,21 +2422,7 @@ public class GameAction {
 
                 if (e.getKey() instanceof Card && !lethalDamage.containsKey(e.getKey())) {
                     Card c = (Card) e.getKey();
-                    int lethal = 0;
-                    if (c.isCreature()) {
-                        lethal = Math.max(0, c.getLethalDamage());
-                    }
-                    if (c.isPlaneswalker()) {
-                        int lethalPW = c.getCurrentLoyalty();
-                        // CR 120.10
-                        lethal = c.isCreature() ? Math.min(lethal, lethalPW) : lethalPW;
-                    }
-                    if (c.isBattle()) {
-                        int lethalBattle = c.getCurrentDefense();
-
-                        lethal = c.isCreature() || c.isPlaneswalker() ? Math.min(lethal, lethalBattle) : lethalBattle;
-                    }
-                    lethalDamage.put(c, lethal);
+                    lethalDamage.put(c, c.getExcessDamageValue());
                 }
 
                 e.setValue(Integer.valueOf(e.getKey().addDamageAfterPrevention(e.getValue(), sourceLKI, isCombat, counterTable)));

forge-game/src/main/java/forge/game/ability/effects/DamageDealEffect.java

@@ -288,18 +288,7 @@ public class DamageDealEffect extends DamageBaseEffect {
         int excess = 0;
         int dmgToTarget = 0;
         if (sa.hasParam("ExcessDamage") || sa.hasParam("ExcessSVar")) {
-            int lethal = 0;
-            if (c.isCreature()) {
-                lethal = Math.max(0, c.getLethalDamage());
-                if (sourceLKI.hasKeyword(Keyword.DEATHTOUCH)) {
-                    lethal = Math.min(lethal, 1);
-                }
-            }
-            if (c.isPlaneswalker()) {
-                int lethalPW = c.getCurrentLoyalty();
-                // 120.4a
-                lethal = c.isCreature() ? Math.min(lethal, lethalPW) : lethalPW;
-            }
+            int lethal = c.getExcessDamageValue();
             dmgToTarget = Math.min(lethal, dmg);
             excess = dmg - dmgToTarget;
         }

forge-game/src/main/java/forge/game/ability/effects/PlayEffect.java

@@ -312,9 +312,12 @@ public class PlayEffect extends SpellAbilityEffect {
                 // For Illusionary Mask effect
                 tgtSA = CardFactoryUtil.abilityMorphDown(tgtCard.getCurrentState(), false);
             } else {
-                // only one mode can be used
                 if (sa.hasParam("CastTransformed")) {
-                    tgtCard.changeToState(CardStateName.Transformed);
+                    if (!tgtCard.changeToState(CardStateName.Transformed)) {
+                        // Failed to transform. In the future, we might need to just remove this option and continue
+                        amount--;
+                        continue;
+                    }
                 }
 
                 tgtSA = sa.getActivatingPlayer().getController().getAbilityToPlay(tgtCard, sas);

forge-game/src/main/java/forge/game/card/Card.java

@@ -1578,12 +1578,8 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars {
             getGame().getTriggerHandler().runTrigger(TriggerType.CounterRemoved, AbilityKey.newMap(runParams), false);
         }
         runParams.put(AbilityKey.CounterAmount, delta);
+        runParams.put(AbilityKey.NewCounterAmount, newValue);
         getGame().getTriggerHandler().runTrigger(TriggerType.CounterRemovedOnce, AbilityKey.newMap(runParams), false);
-
-        if (counterName.is(CounterEnumType.DEFENSE) && newValue == 0 && this.isBattle()) {
-            // Run defeated trigger
-            getGame().getTriggerHandler().runTrigger(TriggerType.Defeated, AbilityKey.newMap(runParams), false);
-        }
     }
 
     @Override
@@ -5569,6 +5565,27 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars {
         return getLethal() - getDamage() - getTotalAssignedDamage();
     }
 
+    public final int getExcessDamageValue() {
+        ArrayList<Integer> excessCharacteristics = new ArrayList<Integer>();
+
+        // CR 120.10
+        if (this.isCreature()) {
+            excessCharacteristics.add(Math.max(0, this.getLethalDamage()));
+        }
+        if (this.isPlaneswalker()) {
+            excessCharacteristics.add(this.getCurrentLoyalty());
+        }
+        if (this.isBattle()) {
+            excessCharacteristics.add(this.getCurrentDefense());
+        }
+
+        if (excessCharacteristics.size() == 0) {
+            return 0;
+        }
+
+        return Collections.min(excessCharacteristics);
+    }
+
     public final int getDamage() {
         int sum = 0;
         for (int i : damage.values()) {

forge-game/src/main/java/forge/game/card/CardFactoryUtil.java

@@ -3940,7 +3940,7 @@ public class CardFactoryUtil {
 
         // Defeated trigger
         StringBuilder triggerDefeated = new StringBuilder();
-        triggerDefeated.append("Mode$ Defeated | ValidCard$ Card.Self | Secondary$ True | ");
+        triggerDefeated.append("Mode$ CounterRemovedOnce | ValidCard$ Card.Self | Secondary$ True | CounterType$ DEFENSE | Remaining$ 0 | TriggerZones$ Battlefield  | ");
         triggerDefeated.append(" TriggerDescription$ When CARDNAME is defeated, exile it, then cast it transformed.");
 
         String castDefeatedBattle = "DB$ Play | Defined$ Self | WithoutManaCost$ True | CastTransformed$ True";

forge-game/src/main/java/forge/game/combat/Combat.java

@@ -304,7 +304,7 @@ public class Combat {
         // maybe attack on a controlled planeswalker?
         if (defender instanceof Card) {
             Card def = (Card)defender;
-            if (def.isBattle() && def.getType().hasSubtype("Siege")) {
+            if (def.isBattle()) {
                 return def.getProtectingPlayer();
             } else {
                 return def.getController();

forge-game/src/main/java/forge/game/trigger/TriggerCounterRemovedOnce.java

@@ -68,6 +68,14 @@ public class TriggerCounterRemovedOnce extends Trigger {
             }
         }
 
+        if (hasParam("Remaining")) {
+            final int remaining = Integer.parseInt(getParam("Remaining"));
+
+            if (remaining != (int) runParams.get(AbilityKey.NewCounterAmount)) {
+                return false;
+            }
+        }
+
         return true;
     }
 

forge-game/src/main/java/forge/game/trigger/TriggerDefeated.java

@@ -1,78 +0,0 @@
-/*
- * Forge: Play Magic: the Gathering.
- * Copyright (C) 2011  Forge Team
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-package forge.game.trigger;
-
-import java.util.Map;
-
-import forge.game.ability.AbilityKey;
-import forge.game.card.Card;
-import forge.game.spellability.SpellAbility;
-import forge.util.Localizer;
-
-/**
- * <p>
- * Trigger_Destroyed class.
- * </p>
- *
- * @author Forge
- * @version $Id: TriggerDestroyed.java 17802 2012-10-31 08:05:14Z Max mtg $
- */
-public class TriggerDefeated extends Trigger {
-
-    /**
-     * <p>
-     * Constructor for Trigger_Destroyed.
-     * </p>
-     *
-     * @param params
-     *            a {@link java.util.HashMap} object.
-     * @param host
-     *            a {@link forge.game.card.Card} object.
-     * @param intrinsic
-     *            the intrinsic
-     */
-    public TriggerDefeated(final Map<String, String> params, final Card host, final boolean intrinsic) {
-        super(params, host, intrinsic);
-    }
-
-    /** {@inheritDoc}
-     * @param runParams*/
-    @Override
-    public final boolean performTest(final Map<AbilityKey, Object> runParams) {
-        if (!matchesValidParam("ValidCard", runParams.get(AbilityKey.Card))) {
-            return false;
-        }
-
-        return true;
-    }
-
-    /** {@inheritDoc} */
-    @Override
-    public final void setTriggeringObjects(final SpellAbility sa, Map<AbilityKey, Object> runParams) {
-        sa.setTriggeringObjectsFrom(runParams, AbilityKey.Card);
-        sa.setTriggeringObject(AbilityKey.Amount, runParams.get(AbilityKey.CounterAmount));
-    }
-
-    @Override
-    public String getImportantStackObjects(SpellAbility sa) {
-        StringBuilder sb = new StringBuilder();
-        sb.append(Localizer.getInstance().getMessage("lblRemovedFrom")).append(": ").append(sa.getTriggeringObject(AbilityKey.Card));
-        sb.append(" ").append(Localizer.getInstance().getMessage("lblAmount")).append(": ").append(sa.getTriggeringObject(AbilityKey.Amount));
-        return sb.toString();
-    }
-}

forge-game/src/main/java/forge/game/trigger/TriggerType.java

@@ -59,7 +59,6 @@ public enum TriggerType {
     DamagePrevented(TriggerDamagePrevented.class),
     DamagePreventedOnce(TriggerDamagePreventedOnce.class),
     DayTimeChanges (TriggerDayTimeChanges.class),
-    Defeated(TriggerDestroyed.class),
     Destroyed(TriggerDestroyed.class),
     Devoured(TriggerDevoured.class),
     Discarded(TriggerDiscarded.class),

forge-gui/res/cardsfolder/r/rootwater_matriarch.txt

@@ -2,7 +2,7 @@ Name:Rootwater Matriarch
 ManaCost:2 U U
 Types:Creature Merfolk
 PT:2/3
-A:AB$ GainControl | Cost$ T | ValidTgts$ Creature | AITgts$ Creature.enchanted | LoseControl$ StaticCommandCheck | StaticCommandCheckSVar$ X | StaticCommandSVarCompare$ EQ0 | ConditionCheckSVar$ Y | ContionSVarCompare$ GE1 | SpellDescription$ Gain control of target creature for as long as that creature is enchanted.
+A:AB$ GainControl | Cost$ T | ValidTgts$ Creature | AITgts$ Creature.Enchanted | LoseControl$ StaticCommandCheck | StaticCommandCheckSVar$ X | StaticCommandSVarCompare$ EQ0 | ConditionCheckSVar$ Y | ContionSVarCompare$ GE1 | SpellDescription$ Gain control of target creature for as long as that creature is enchanted.
 # the hostcard of SVar X is the controlled card
 SVar:X:Count$Valid Card.Self+enchanted
 SVar:Y:Targeted$Valid Card.enchanted

forge-gui/res/cardsfolder/upcoming/invasion_of_zendikar__awakened_skyclave.txt

@@ -4,7 +4,7 @@ Types:Battle Siege
 Defense:3
 T:Mode$ ChangesZone | Origin$ Any | Destination$ Battlefield | ValidCard$ Card.Self | Execute$ TrigSearch | TriggerDescription$ When CARDNAME enters the battlefield, search your library for up to two basic land cards, put them onto the battlefield tapped, then shuffle.
 SVar:TrigSearch:DB$ ChangeZone | Origin$ Library | Destination$ Battlefield | Tapped$ True | ChangeType$ Land.Basic | ChangeNum$ 2 | Shuffle$ True
-Oracle:(As a Siege enters, choose an opponent to protect it. You and others can attack it. When it’s defeated, exile it, then cast it transformed.) When Invasion of Zendikar enters the battlefield, search your library for up to two basic land cards, put them onto the battlefield tapped, then shuffle.
+Oracle:(As a Siege enters, choose an opponent to protect it. You and others can attack it. When it's defeated, exile it, then cast it transformed.) When Invasion of Zendikar enters the battlefield, search your library for up to two basic land cards, put them onto the battlefield tapped, then shuffle.
 
 AlternateMode:DoubleFaced
 
@@ -19,4 +19,4 @@ K:Vigilance
 K:Haste
 S:Mode$ Continuous | Affected$ Card.Self | AddType$ Land | Description$ As long as Awakened Skyclave is on the battlefield, it’s a land in addition to its other types.
 A:AB$ Mana | Cost$ T | Produced$ Any | SpellDescription$ Add one mana of any color.
-Oracle:Vigilance, haste. \nAs long as Awakened Skyclave is on the battlefield, it’s a land in addition to its other types.\n{T}: Add one mana of any color
+Oracle:Vigilance, haste. \nAs long as Awakened Skyclave is on the battlefield, it's a land in addition to its other types.\n{T}: Add one mana of any color