Lots of code cleanup, including removing some useless null checks,

preventing some NPE's before they ever occur, and clean up some
of the inheritance hierarchy.

.gitattributes

@@ -319,7 +319,7 @@ forge-game/src/main/java/forge/game/ability/AbilityApiBased.java -text
 forge-game/src/main/java/forge/game/ability/AbilityFactory.java svneol=native#text/plain
 forge-game/src/main/java/forge/game/ability/AbilityUtils.java -text
 forge-game/src/main/java/forge/game/ability/ApiType.java -text
-forge-game/src/main/java/forge/game/ability/SaTargetRoutines.java -text
+forge-game/src/main/java/forge/game/ability/IllegalAbilityException.java -text
 forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java -text
 forge-game/src/main/java/forge/game/ability/SpellApiBased.java -text
 forge-game/src/main/java/forge/game/ability/StaticAbilityApiBased.java -text

forge-ai/src/main/java/forge/ai/ComputerUtilMana.java

@@ -1209,7 +1209,7 @@ public class ComputerUtilMana {
      * @return map between creatures and shards to convoke
      */
     public static Map<Card, ManaCostShard> getConvokeFromList(final ManaCost cost, List<Card> list) {
-        HashMap<Card, ManaCostShard> convoke = new HashMap<Card, ManaCostShard>();
+        final Map<Card, ManaCostShard> convoke = new HashMap<Card, ManaCostShard>();
         Card convoked = null;
         for (ManaCostShard toPay : cost) {
             for (Card c : list) {
@@ -1219,9 +1219,6 @@ public class ComputerUtilMana {
                     convoke.put(c, toPay);
                     break;
                 }
-                if (convoked != null){
-                    break;
-                }
             }
             if (convoked != null) {
                 list.remove(convoked);

forge-ai/src/main/java/forge/ai/SpellAbilityAi.java

@@ -1,10 +1,11 @@
 package forge.ai;
 
+import java.util.Collection;
+import java.util.List;
 
 import com.google.common.collect.Iterables;
 
 import forge.game.GameEntity;
-import forge.game.ability.SaTargetRoutines;
 import forge.game.card.Card;
 import forge.game.phase.PhaseHandler;
 import forge.game.phase.PhaseType;
@@ -14,10 +15,7 @@ import forge.game.spellability.AbilitySub;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.TargetRestrictions;
 
-import java.util.Collection;
+public abstract class SpellAbilityAi {
-import java.util.List;
-
-public abstract class SpellAbilityAi extends SaTargetRoutines {
 
     public final boolean canPlayAIWithSubs(final Player aiPlayer, final SpellAbility sa) {
         if (!canPlayAI(aiPlayer, sa)) {

forge-ai/src/main/java/forge/ai/SpellApiToAi.java

@@ -1,6 +1,10 @@
 package forge.ai;
 
 import java.util.EnumMap;
+import java.util.Map;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 
 import forge.ai.ability.*;
 import forge.game.ability.ApiType;
@@ -9,144 +13,143 @@ import forge.util.ReflectionUtil;
 public enum SpellApiToAi {
     Converter;
 
-    private final static EnumMap<ApiType, Class<? extends SpellAbilityAi>> apiToClass = new EnumMap<>(ApiType.class);
+    private final Map<ApiType, SpellAbilityAi> apiToInstance = new EnumMap<>(ApiType.class);
-    private final EnumMap<ApiType, SpellAbilityAi> apiToInstance = new EnumMap<>(ApiType.class);
 
-    static {
+    // Do the extra copy to make an actual EnumMap (faster)
-        apiToClass.put(ApiType.Abandon, AlwaysPlayAi.class);
+    private final Map<ApiType, Class<? extends SpellAbilityAi>> apiToClass = Maps.newEnumMap(ImmutableMap
-        apiToClass.put(ApiType.ActivateAbility, ActivateAbilityAi.class);
+            .<ApiType, Class<? extends SpellAbilityAi>>builder()
-        apiToClass.put(ApiType.AddOrRemoveCounter, CountersPutOrRemoveAi.class);
+            .put(ApiType.Abandon, AlwaysPlayAi.class)
-        apiToClass.put(ApiType.AddPhase, AddPhaseAi.class);
+            .put(ApiType.ActivateAbility, ActivateAbilityAi.class)
-        apiToClass.put(ApiType.AddTurn, AddTurnAi.class);
+            .put(ApiType.AddOrRemoveCounter, CountersPutOrRemoveAi.class)
-        apiToClass.put(ApiType.Animate, AnimateAi.class);
+            .put(ApiType.AddPhase, AddPhaseAi.class)
-        apiToClass.put(ApiType.AnimateAll, AnimateAllAi.class);
+            .put(ApiType.AddTurn, AddTurnAi.class)
-        apiToClass.put(ApiType.Attach, AttachAi.class);
+            .put(ApiType.Animate, AnimateAi.class)
-        apiToClass.put(ApiType.Balance, BalanceAi.class);
+            .put(ApiType.AnimateAll, AnimateAllAi.class)
-        apiToClass.put(ApiType.BecomesBlocked, BecomesBlockedAi.class);
+            .put(ApiType.Attach, AttachAi.class)
-        apiToClass.put(ApiType.BidLife, BidLifeAi.class);
+            .put(ApiType.Balance, BalanceAi.class)
-        apiToClass.put(ApiType.Bond, BondAi.class);
+            .put(ApiType.BecomesBlocked, BecomesBlockedAi.class)
-        apiToClass.put(ApiType.ChangeTargets, ChangeTargetsAi.class);
+            .put(ApiType.BidLife, BidLifeAi.class)
-        apiToClass.put(ApiType.ChangeZone, ChangeZoneAi.class);
+            .put(ApiType.Bond, BondAi.class)
-        apiToClass.put(ApiType.ChangeZoneAll, ChangeZoneAllAi.class);
+            .put(ApiType.ChangeTargets, ChangeTargetsAi.class)
+            .put(ApiType.ChangeZone, ChangeZoneAi.class)
+            .put(ApiType.ChangeZoneAll, ChangeZoneAllAi.class)
+            .put(ApiType.Charm, CharmAi.class)
+            .put(ApiType.ChooseCard, ChooseCardAi.class)
+            .put(ApiType.ChooseColor, ChooseColorAi.class)
+            .put(ApiType.ChooseDirection, ChooseDirectionAi.class)
+            .put(ApiType.ChooseNumber, ChooseNumberAi.class)
+            .put(ApiType.ChoosePlayer, ChoosePlayerAi.class)
+            .put(ApiType.ChooseSource, ChooseSourceAi.class)
+            .put(ApiType.ChooseType, ChooseTypeAi.class)
+            .put(ApiType.Clash, ClashAi.class)
+            .put(ApiType.Cleanup, AlwaysPlayAi.class)
+            .put(ApiType.Clone, CloneAi.class)
+            .put(ApiType.CopyPermanent, CopyPermanentAi.class)
+            .put(ApiType.CopySpellAbility, CanPlayAsDrawbackAi.class)
+            .put(ApiType.ControlPlayer, CannotPlayAi.class)
+            .put(ApiType.ControlSpell, CannotPlayAi.class)
+            .put(ApiType.Counter, CounterAi.class)
+            .put(ApiType.DamageAll, DamageAllAi.class)
+            .put(ApiType.DealDamage, DamageDealAi.class)
+            .put(ApiType.Debuff, DebuffAi.class)
+            .put(ApiType.DeclareCombatants, CannotPlayAi.class)
+            .put(ApiType.DelayedTrigger, DelayedTriggerAi.class)
+            .put(ApiType.Destroy, DestroyAi.class)
+            .put(ApiType.DestroyAll, DestroyAllAi.class)
+            .put(ApiType.Dig, DigAi.class)
+            .put(ApiType.DigUntil, DigUntilAi.class)
+            .put(ApiType.Discard, DiscardAi.class)
+            .put(ApiType.DrainMana, DrainManaAi.class)
+            .put(ApiType.Draw, DrawAi.class)
+            .put(ApiType.EachDamage, DamageEachAi.class)
+            .put(ApiType.Effect, EffectAi.class)
+            .put(ApiType.Encode, EncodeAi.class)
+            .put(ApiType.EndTurn, EndTurnAi.class)
+            .put(ApiType.ExchangeLife, LifeExchangeAi.class)
+            .put(ApiType.ExchangeControl, ControlExchangeAi.class)
+            .put(ApiType.ExchangeControlVariant, CannotPlayAi.class)
+            .put(ApiType.ExchangePower, PowerExchangeAi.class)
+            .put(ApiType.ExchangeZone, ZoneExchangeAi.class)
+            .put(ApiType.Fight, FightAi.class)
+            .put(ApiType.FlipACoin, FlipACoinAi.class)
+            .put(ApiType.Fog, FogAi.class)
+            .put(ApiType.GainControl, ControlGainAi.class)
+            .put(ApiType.GainLife, LifeGainAi.class)
+            .put(ApiType.GainOwnership, CannotPlayAi.class)
+            .put(ApiType.GenericChoice, ChooseGenericEffectAi.class)
+            .put(ApiType.LoseLife, LifeLoseAi.class)
+            .put(ApiType.LosesGame, GameLossAi.class)
+            .put(ApiType.Mana, ManaEffectAi.class)
+            .put(ApiType.ManaReflected, CannotPlayAi.class)
+            .put(ApiType.Manifest, ManifestAi.class)
+            .put(ApiType.Mill, MillAi.class)
+            .put(ApiType.MoveCounter, CountersMoveAi.class)
+            .put(ApiType.MultiplePiles, CannotPlayAi.class)
+            .put(ApiType.MustAttack, MustAttackAi.class)
+            .put(ApiType.MustBlock, MustBlockAi.class)
+            .put(ApiType.NameCard, ChooseCardNameAi.class)
+            .put(ApiType.PeekAndReveal, PeekAndRevealAi.class)
+            .put(ApiType.PermanentCreature, PermanentCreatureAi.class)
+            .put(ApiType.PermanentNoncreature, PermanentNoncreatureAi.class)
+            .put(ApiType.Phases, PhasesAi.class)
+            .put(ApiType.Planeswalk, AlwaysPlayAi.class)
+            .put(ApiType.Play, PlayAi.class)
+            .put(ApiType.PlayLandVariant, CannotPlayAi.class)
+            .put(ApiType.Poison, PoisonAi.class)
+            .put(ApiType.PreventDamage, DamagePreventAi.class)
+            .put(ApiType.PreventDamageAll, DamagePreventAllAi.class)
+            .put(ApiType.Proliferate, CountersProliferateAi.class)
+            .put(ApiType.Protection, ProtectAi.class)
+            .put(ApiType.ProtectionAll, ProtectAllAi.class)
+            .put(ApiType.Pump, PumpAi.class)
+            .put(ApiType.PumpAll, PumpAllAi.class)
+            .put(ApiType.PutCounter, CountersPutAi.class)
+            .put(ApiType.PutCounterAll, CountersPutAllAi.class)
+            .put(ApiType.RearrangeTopOfLibrary, RearrangeTopOfLibraryAi.class)
+            .put(ApiType.Regenerate, RegenerateAi.class)
+            .put(ApiType.RegenerateAll, RegenerateAllAi.class)
+            .put(ApiType.RemoveCounter, CountersRemoveAi.class)
+            .put(ApiType.RemoveCounterAll, CannotPlayAi.class)
+            .put(ApiType.RemoveFromCombat, RemoveFromCombatAi.class)
+            .put(ApiType.ReorderZone, AlwaysPlayAi.class)
+            .put(ApiType.Repeat, RepeatAi.class)
+            .put(ApiType.RepeatEach, RepeatEachAi.class)
+            .put(ApiType.RestartGame, RestartGameAi.class)
+            .put(ApiType.Reveal, RevealAi.class)
+            .put(ApiType.RevealHand, RevealHandAi.class)
+            .put(ApiType.ReverseTurnOrder, AlwaysPlayAi.class)
+            .put(ApiType.RollPlanarDice, RollPlanarDiceAi.class)
+            .put(ApiType.RunSVarAbility, AlwaysPlayAi.class)
+            .put(ApiType.Sacrifice, SacrificeAi.class)
+            .put(ApiType.SacrificeAll, SacrificeAllAi.class)
+            .put(ApiType.Scry, ScryAi.class)
+            .put(ApiType.SetInMotion, AlwaysPlayAi.class)
+            .put(ApiType.SetLife, LifeSetAi.class)
+            .put(ApiType.SetState, SetStateAi.class)
+            .put(ApiType.Shuffle, ShuffleAi.class)
+            .put(ApiType.SkipTurn, SkipTurnAi.class)
+            .put(ApiType.StoreMap, StoreMapAi.class)
+            .put(ApiType.StoreSVar, StoreSVarAi.class)
+            .put(ApiType.Tap, TapAi.class)
+            .put(ApiType.TapAll, TapAllAi.class)
+            .put(ApiType.TapOrUntap, TapOrUntapAi.class)
+            .put(ApiType.TapOrUntapAll, TapOrUntapAllAi.class)
+            .put(ApiType.Token, TokenAi.class)
+            .put(ApiType.TwoPiles, TwoPilesAi.class)
+            .put(ApiType.Unattach, CannotPlayAi.class)
+            .put(ApiType.UnattachAll, UnattachAllAi.class)
+            .put(ApiType.Untap, UntapAi.class)
+            .put(ApiType.UntapAll, UntapAllAi.class)
+            .put(ApiType.Vote, VoteAi.class)
+            .put(ApiType.WinsGame, GameWinAi.class)
 
-        apiToClass.put(ApiType.Charm, CharmAi.class);
+            .put(ApiType.InternalEtbReplacement, CanPlayAsDrawbackAi.class)
-        apiToClass.put(ApiType.ChooseCard, ChooseCardAi.class);
+            .put(ApiType.InternalLegendaryRule, LegendaryRuleAi.class)
-        apiToClass.put(ApiType.ChooseColor, ChooseColorAi.class);
+            .put(ApiType.InternalHaunt, HauntAi.class)
-        apiToClass.put(ApiType.ChooseDirection, ChooseDirectionAi.class);
+            .put(ApiType.InternalIgnoreEffect, CannotPlayAi.class)
-        apiToClass.put(ApiType.ChooseNumber, ChooseNumberAi.class);
+            .build());
-        apiToClass.put(ApiType.ChoosePlayer, ChoosePlayerAi.class);
-        apiToClass.put(ApiType.ChooseSource, ChooseSourceAi.class);
-        apiToClass.put(ApiType.ChooseType, ChooseTypeAi.class);
-        apiToClass.put(ApiType.Clash, ClashAi.class);
-        apiToClass.put(ApiType.Cleanup, AlwaysPlayAi.class);
-        apiToClass.put(ApiType.Clone, CloneAi.class);
-        apiToClass.put(ApiType.CopyPermanent, CopyPermanentAi.class);
-        apiToClass.put(ApiType.CopySpellAbility, CanPlayAsDrawbackAi.class);
-        apiToClass.put(ApiType.ControlPlayer, CannotPlayAi.class);
-        apiToClass.put(ApiType.ControlSpell, CannotPlayAi.class);
-        apiToClass.put(ApiType.Counter, CounterAi.class);
-        apiToClass.put(ApiType.DamageAll, DamageAllAi.class);
 
-        apiToClass.put(ApiType.DealDamage, DamageDealAi.class);
+    public SpellAbilityAi get(final ApiType api) {
-        apiToClass.put(ApiType.Debuff, DebuffAi.class);
-        apiToClass.put(ApiType.DeclareCombatants, CannotPlayAi.class);
-        apiToClass.put(ApiType.DelayedTrigger, DelayedTriggerAi.class);
-        apiToClass.put(ApiType.Destroy, DestroyAi.class);
-        apiToClass.put(ApiType.DestroyAll, DestroyAllAi.class);
-        apiToClass.put(ApiType.Dig, DigAi.class);
-        apiToClass.put(ApiType.DigUntil, DigUntilAi.class);
-        apiToClass.put(ApiType.Discard, DiscardAi.class);
-        apiToClass.put(ApiType.DrainMana, DrainManaAi.class);
-        apiToClass.put(ApiType.Draw, DrawAi.class);
-        apiToClass.put(ApiType.EachDamage, DamageEachAi.class);
-        apiToClass.put(ApiType.Effect, EffectAi.class);
-        apiToClass.put(ApiType.Encode, EncodeAi.class);
-        apiToClass.put(ApiType.EndTurn, EndTurnAi.class);
-        apiToClass.put(ApiType.ExchangeLife, LifeExchangeAi.class);
-        apiToClass.put(ApiType.ExchangeControl, ControlExchangeAi.class);
-        apiToClass.put(ApiType.ExchangeControlVariant, CannotPlayAi.class);
-        apiToClass.put(ApiType.ExchangePower, PowerExchangeAi.class);
-        apiToClass.put(ApiType.ExchangeZone, ZoneExchangeAi.class);
-        apiToClass.put(ApiType.Fight, FightAi.class);
-        apiToClass.put(ApiType.FlipACoin, FlipACoinAi.class);
-        apiToClass.put(ApiType.Fog, FogAi.class);
-        apiToClass.put(ApiType.GainControl, ControlGainAi.class);
-        apiToClass.put(ApiType.GainLife, LifeGainAi.class);
-        apiToClass.put(ApiType.GainOwnership, CannotPlayAi.class);
-        apiToClass.put(ApiType.GenericChoice, ChooseGenericEffectAi.class);
-        apiToClass.put(ApiType.LoseLife, LifeLoseAi.class);
-        apiToClass.put(ApiType.LosesGame, GameLossAi.class);
-        apiToClass.put(ApiType.Mana, ManaEffectAi.class);
-        apiToClass.put(ApiType.ManaReflected, CannotPlayAi.class);
-        apiToClass.put(ApiType.Manifest, ManifestAi.class);
-        apiToClass.put(ApiType.Mill, MillAi.class);
-        apiToClass.put(ApiType.MoveCounter, CountersMoveAi.class);
-        apiToClass.put(ApiType.MultiplePiles, CannotPlayAi.class);
-        apiToClass.put(ApiType.MustAttack, MustAttackAi.class);
-        apiToClass.put(ApiType.MustBlock, MustBlockAi.class);
-        apiToClass.put(ApiType.NameCard, ChooseCardNameAi.class);
-        apiToClass.put(ApiType.PeekAndReveal, PeekAndRevealAi.class);
-        apiToClass.put(ApiType.PermanentCreature, PermanentCreatureAi.class);
-        apiToClass.put(ApiType.PermanentNoncreature, PermanentNoncreatureAi.class);
-        apiToClass.put(ApiType.Phases, PhasesAi.class);
-        apiToClass.put(ApiType.Planeswalk, AlwaysPlayAi.class);
-        apiToClass.put(ApiType.Play, PlayAi.class);
-        apiToClass.put(ApiType.PlayLandVariant, CannotPlayAi.class);
-        apiToClass.put(ApiType.Poison, PoisonAi.class);
-        apiToClass.put(ApiType.PreventDamage, DamagePreventAi.class);
-        apiToClass.put(ApiType.PreventDamageAll, DamagePreventAllAi.class);
-        apiToClass.put(ApiType.Proliferate, CountersProliferateAi.class);
-        apiToClass.put(ApiType.Protection, ProtectAi.class);
-        apiToClass.put(ApiType.ProtectionAll, ProtectAllAi.class);
-        apiToClass.put(ApiType.Pump, PumpAi.class);
-        apiToClass.put(ApiType.PumpAll, PumpAllAi.class);
-        apiToClass.put(ApiType.PutCounter, CountersPutAi.class);
-        apiToClass.put(ApiType.PutCounterAll, CountersPutAllAi.class);
-        apiToClass.put(ApiType.RearrangeTopOfLibrary, RearrangeTopOfLibraryAi.class);
-        apiToClass.put(ApiType.Regenerate, RegenerateAi.class);
-        apiToClass.put(ApiType.RegenerateAll, RegenerateAllAi.class);
-        apiToClass.put(ApiType.RemoveCounter, CountersRemoveAi.class);
-        apiToClass.put(ApiType.RemoveCounterAll, CannotPlayAi.class);
-        apiToClass.put(ApiType.RemoveFromCombat, RemoveFromCombatAi.class);
-        apiToClass.put(ApiType.ReorderZone, AlwaysPlayAi.class);
-        apiToClass.put(ApiType.Repeat, RepeatAi.class);
-        apiToClass.put(ApiType.RepeatEach, RepeatEachAi.class);
-        apiToClass.put(ApiType.RestartGame, RestartGameAi.class);
-        apiToClass.put(ApiType.Reveal, RevealAi.class);
-        apiToClass.put(ApiType.RevealHand, RevealHandAi.class);
-        apiToClass.put(ApiType.ReverseTurnOrder, AlwaysPlayAi.class);
-        apiToClass.put(ApiType.RollPlanarDice, RollPlanarDiceAi.class);
-        apiToClass.put(ApiType.RunSVarAbility, AlwaysPlayAi.class);
-        apiToClass.put(ApiType.Sacrifice, SacrificeAi.class);
-        apiToClass.put(ApiType.SacrificeAll, SacrificeAllAi.class);
-        apiToClass.put(ApiType.Scry, ScryAi.class);
-        apiToClass.put(ApiType.SetInMotion, AlwaysPlayAi.class);
-        apiToClass.put(ApiType.SetLife, LifeSetAi.class);
-        apiToClass.put(ApiType.SetState, SetStateAi.class);
-        apiToClass.put(ApiType.Shuffle, ShuffleAi.class);
-        apiToClass.put(ApiType.SkipTurn, SkipTurnAi.class);
-        apiToClass.put(ApiType.StoreMap, StoreMapAi.class);
-        apiToClass.put(ApiType.StoreSVar, StoreSVarAi.class);
-        apiToClass.put(ApiType.Tap, TapAi.class);
-        apiToClass.put(ApiType.TapAll, TapAllAi.class);
-        apiToClass.put(ApiType.TapOrUntap, TapOrUntapAi.class);
-        apiToClass.put(ApiType.TapOrUntapAll, TapOrUntapAllAi.class);
-        apiToClass.put(ApiType.Token, TokenAi.class);
-        apiToClass.put(ApiType.TwoPiles, TwoPilesAi.class);
-        apiToClass.put(ApiType.Unattach, CannotPlayAi.class);
-        apiToClass.put(ApiType.UnattachAll, UnattachAllAi.class);
-        apiToClass.put(ApiType.Untap, UntapAi.class);
-        apiToClass.put(ApiType.UntapAll, UntapAllAi.class);
-        apiToClass.put(ApiType.Vote, VoteAi.class);
-        apiToClass.put(ApiType.WinsGame, GameWinAi.class);
-
-        apiToClass.put(ApiType.InternalEtbReplacement, CanPlayAsDrawbackAi.class);
-        apiToClass.put(ApiType.InternalLegendaryRule, LegendaryRuleAi.class);
-        apiToClass.put(ApiType.InternalHaunt, HauntAi.class);
-        apiToClass.put(ApiType.InternalIgnoreEffect, CannotPlayAi.class);
-    }
-    
-    public SpellAbilityAi get(ApiType api) {
         SpellAbilityAi result = apiToInstance.get(api);
         if (null == result) {
             Class<? extends SpellAbilityAi> clz = apiToClass.get(api);

forge-ai/src/main/java/forge/ai/ability/ChangeZoneAi.java

@@ -738,9 +738,9 @@ public class ChangeZoneAi extends SpellAbilityAi {
         if (source.isInZone(ZoneType.Hand)) {
             list = CardLists.filter(list, Predicates.not(CardPredicates.nameEquals(source.getName()))); // Don't get the same card back.
         }
-        System.out.println("isPreferredTarget " + list);
+        //System.out.println("isPreferredTarget " + list);
         if (sa.hasParam("AttachedTo")) {
-        	System.out.println("isPreferredTarget att " + list);
+            //System.out.println("isPreferredTarget att " + list);
             list = CardLists.filter(list, new Predicate<Card>() {
                 @Override
                 public boolean apply(final Card c) {
@@ -752,7 +752,7 @@ public class ChangeZoneAi extends SpellAbilityAi {
                     return false;
                 }
             });
-            System.out.println("isPreferredTarget ok " + list);
+            //System.out.println("isPreferredTarget ok " + list);
         }
 
         if (list.size() < tgt.getMinTargets(sa.getHostCard(), sa)) {

forge-ai/src/main/java/forge/ai/ability/ChooseSourceAi.java

@@ -1,6 +1,9 @@
 package forge.ai.ability;
 
+import java.util.List;
+
 import com.google.common.base.Predicate;
+import com.google.common.collect.Lists;
 
 import forge.ai.ComputerUtilCard;
 import forge.ai.ComputerUtilCombat;
@@ -22,8 +25,6 @@ import forge.game.spellability.SpellAbilityStackInstance;
 import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
 
-import java.util.List;
-
 public class ChooseSourceAi extends SpellAbilityAi {
 
     /* (non-Javadoc)
@@ -184,4 +185,10 @@ public class ChooseSourceAi extends SpellAbilityAi {
         }
         return null;
     }    
+
+    private static List<GameObject> getTargets(final SpellAbility sa) {
+        return sa.usesTargeting() && (!sa.hasParam("Defined"))
+                ? Lists.newArrayList(sa.getTargets().getTargets()) 
+                : AbilityUtils.getDefinedObjects(sa.getHostCard(), sa.getParam("Defined"), sa);
+    }
 }

forge-ai/src/main/java/forge/ai/ability/DamagePreventAi.java

@@ -146,7 +146,7 @@ public class DamagePreventAi extends SpellAbilityAi {
                 }
             }
         }
-        if (sa.hasParam("DividedAsYouChoose") && sa.getTargets() != null && !sa.getTargets().getTargets().isEmpty()) {
+        if (tgt != null && sa.hasParam("DividedAsYouChoose") && sa.getTargets() != null && !sa.getTargets().getTargets().isEmpty()) {
             tgt.addDividedAllocation(sa.getTargets().getTargets().get(0), AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("Amount"), sa));
         }
 

forge-ai/src/main/java/forge/ai/ability/LifeLoseAi.java

@@ -11,6 +11,7 @@ import forge.game.phase.PhaseType;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.TargetRestrictions;
+import forge.util.FCollection;
 
 import java.util.List;
 
@@ -19,7 +20,9 @@ public class LifeLoseAi extends SpellAbilityAi {
     @Override
     public boolean chkAIDrawback(SpellAbility sa, Player ai) {
 
-        List<Player> tgtPlayers = getTargetPlayers(sa);
+        final List<Player> tgtPlayers = sa.usesTargeting() && !sa.hasParam("Defined")
+                ? new FCollection<Player>(sa.getTargets().getTargetPlayers()) 
+                : AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam("Defined"), sa);
 
         final Card source = sa.getHostCard();
         final String amountStr = sa.getParam("LifeAmount");
@@ -158,7 +161,9 @@ public class LifeLoseAi extends SpellAbilityAi {
             amount = AbilityUtils.calculateAmount(source, amountStr, sa);
         }
 
-        List<Player> tgtPlayers = getTargetPlayers(sa);
+        final List<Player> tgtPlayers = sa.usesTargeting() && !sa.hasParam("Defined")
+                ? new FCollection<Player>(sa.getTargets().getTargetPlayers()) 
+                : AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam("Defined"), sa);
 
         if (!mandatory && tgtPlayers.contains(ai) && amount > 0 && amount + 3 > ai.getLife()) {
             // For cards like Foul Imp, ETB you lose life

forge-ai/src/main/java/forge/ai/ability/TwoPilesAi.java

@@ -1,5 +1,7 @@
 package forge.ai.ability;
 
+import java.util.List;
+
 import forge.ai.SpellAbilityAi;
 import forge.game.ability.AbilityUtils;
 import forge.game.card.Card;
@@ -9,8 +11,7 @@ import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.TargetRestrictions;
 import forge.game.zone.ZoneType;
-
+import forge.util.FCollection;
-import java.util.List;
 
 public class TwoPilesAi extends SpellAbilityAi {
     @Override
@@ -37,7 +38,9 @@ public class TwoPilesAi extends SpellAbilityAi {
             }
         }
 
-        List<Player> tgtPlayers = getTargetPlayers(sa);
+        final List<Player> tgtPlayers = sa.usesTargeting() && !sa.hasParam("Defined")
+                ? new FCollection<Player>(sa.getTargets().getTargetPlayers()) 
+                : AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam("Defined"), sa);
 
         final Player p = tgtPlayers.get(0);
         CardCollectionView pool;

forge-core/src/main/java/forge/util/ReflectionUtil.java

@@ -1,40 +1,61 @@
 package forge.util;
 
 import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
 
 /**
- * TODO: Write javadoc for this type.
+ * Static utilities related to reflection.
  * 
+ * @see java.lang.Class
  */
-public class ReflectionUtil {
+public final class ReflectionUtil {
 
-    public static <T> T makeDefaultInstanceOf(Class<? extends T> cls) {
+    /**
-        if ( null == cls )
+     * Private constructor to prevent instantiation.
+     */
+    private ReflectionUtil() {
+    }
+
+    /**
+     * Generates a default instance of the supplied class, using that class'
+     * default constructor.
+     * 
+     * @param cls
+     *            a {@link Class}.
+     * @return an instance of the supplied class.
+     * @throws RuntimeException
+     *             if the supplied class has no visible default constructor, or
+     *             if an exception is thrown by the constructor.
+     * @see Class#newInstance()
+     */
+    public static <T> T makeDefaultInstanceOf(final Class<? extends T> cls) {
+        if (null == cls) {
             throw new IllegalArgumentException("Class<? extends T> cls must not be null");
+        }
 
-        @SuppressWarnings("unchecked")
-        Constructor<? extends T>[] cc = (Constructor<? extends T>[]) cls.getConstructors();
-        for (Constructor<? extends T> c : cc) {
-            Class<?>[] pp = c.getParameterTypes();
-            if (pp.length == 0) {
         try {
-                    T res = c.newInstance();
+            final Constructor<? extends T> c = cls.getConstructor();
-                    return res;
+            return c.newInstance();
-                } catch (Exception e) {
+        } catch (final NoSuchMethodException e) {
-                    e.printStackTrace();
+            throw new RuntimeException(String.format("No default constructor found in class %s", cls.getName()));
+        } catch (final SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
+            throw new RuntimeException(String.format("Can't instantiate class %s using default constructor", cls.getName()));
         }
     }
-        }
-        throw new RuntimeException("No default constructor found in class " + cls.getName());
-    }
 
     /**
      * Cast object to a given type if possible, returning null if not possible
+     * 
      * @param obj
+     *            an object.
      * @param type
+     *            the {@link Class} to which to cast the object.
+     * @return a reference to the object if it's an instance of the given class,
+     *         or {@code null} if it isn't.
+     * @see Class#isInstance(Object)
      */
     @SuppressWarnings("unchecked")
-    public static <T> T safeCast(Object obj, Class<T> type) {
+    public static <T> T safeCast(final Object obj, final Class<T> type) {
         if (type.isInstance(obj)) {
             return (T) obj;
         }

forge-game/src/main/java/forge/game/ability/IllegalAbilityException.java

@@ -0,0 +1,20 @@
+package forge.game.ability;
+
+import forge.game.spellability.SpellAbility;
+
+public class IllegalAbilityException extends RuntimeException {
+    private static final long serialVersionUID = -8638474348184716635L;
+
+    public IllegalAbilityException(final SpellAbility sa) {
+        this(sa.toString());
+    }
+
+    public IllegalAbilityException(final SpellAbility sa, final SpellAbilityEffect effect) {
+        this(String.format("%s (effect %s)", sa, effect.getClass().getName()));
+    }
+
+    private IllegalAbilityException(final String message) {
+        super(message);
+    }
+
+}

forge-game/src/main/java/forge/game/ability/SaTargetRoutines.java

@@ -1,62 +0,0 @@
-package forge.game.ability;
-
-import com.google.common.collect.Lists;
-
-import forge.game.GameObject;
-import forge.game.card.CardCollection;
-import forge.game.player.Player;
-import forge.game.spellability.SpellAbility;
-import forge.util.FCollection;
-
-import java.util.List;
-
-// Class contains all that methods that are used by both effects and AI to fetch their targets.
-// {SA}Effect and {SA}Ai now inherit from this class to use these routines, though they should not.
-public class SaTargetRoutines {
-    // Cards
-    protected CardCollection getTargetCards(SpellAbility sa) {                                  return getCards(false, "Defined", sa); }
-    protected CardCollection getTargetCards(SpellAbility sa, String definedParam) {             return getCards(false, definedParam, sa); }
-    protected CardCollection getDefinedCardsOrTargeted(SpellAbility sa) {                       return getCards(true, "Defined", sa); }
-    protected CardCollection getDefinedCardsOrTargeted(SpellAbility sa, String definedParam) {  return getCards(true, definedParam, sa); }
-
-    private static CardCollection getCards(boolean definedFirst, String definedParam, SpellAbility sa) {
-        boolean useTargets = sa.usesTargeting() && (!definedFirst || !sa.hasParam(definedParam))
-                && sa.getTargets() != null && (sa.getTargets().isTargetingAnyCard() || sa.getTargets().getTargets().isEmpty());
-        return useTargets ? new CardCollection(sa.getTargets().getTargetCards()) 
-                : AbilityUtils.getDefinedCards(sa.getHostCard(), sa.getParam(definedParam), sa);
-    }
-
-    // Players
-    protected FCollection<Player> getTargetPlayers(SpellAbility sa) {                                  return getPlayers(false, "Defined", sa); }
-    protected FCollection<Player> getTargetPlayers(SpellAbility sa, String definedParam) {             return getPlayers(false, definedParam, sa); }
-    protected FCollection<Player> getDefinedPlayersOrTargeted(SpellAbility sa ) {                      return getPlayers(true, "Defined", sa); }
-    protected FCollection<Player> getDefinedPlayersOrTargeted(SpellAbility sa, String definedParam) {  return getPlayers(true, definedParam, sa); }
-
-    private static FCollection<Player> getPlayers(boolean definedFirst, String definedParam, SpellAbility sa) {
-        boolean useTargets = sa.usesTargeting() && (!definedFirst || !sa.hasParam(definedParam));
-        return useTargets ? new FCollection<Player>(sa.getTargets().getTargetPlayers()) 
-                : AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam(definedParam), sa);
-    }
-
-    // Spells
-    protected List<SpellAbility> getTargetSpells(SpellAbility sa) {                                  return getSpells(false, "Defined", sa); }
-    protected List<SpellAbility> getTargetSpells(SpellAbility sa, String definedParam) {             return getSpells(false, definedParam, sa); }
-    protected List<SpellAbility> getDefinedSpellsOrTargeted(SpellAbility sa, String definedParam) {  return getSpells(true, definedParam, sa); }
-    
-    private static List<SpellAbility> getSpells(boolean definedFirst, String definedParam, SpellAbility sa) {
-        boolean useTargets = sa.usesTargeting() && (!definedFirst || !sa.hasParam(definedParam));
-        return useTargets ? Lists.newArrayList(sa.getTargets().getTargetSpells()) 
-                : AbilityUtils.getDefinedSpellAbilities(sa.getHostCard(), sa.getParam(definedParam), sa);
-    }
-
-    // Targets of unspecified type
-    protected List<GameObject> getTargets(SpellAbility sa) {                                   return getTargetables(false, "Defined", sa); }
-    protected List<GameObject> getTargets(SpellAbility sa, String definedParam) {              return getTargetables(false, definedParam, sa); }
-    protected List<GameObject> getDefinedOrTargeted(SpellAbility sa, String definedParam) {  return getTargetables(true, definedParam, sa); }
-
-    private static List<GameObject> getTargetables(boolean definedFirst, String definedParam, SpellAbility sa) {
-        boolean useTargets = sa.usesTargeting() && (!definedFirst || !sa.hasParam(definedParam));
-        return useTargets ? Lists.newArrayList(sa.getTargets().getTargets()) 
-                : AbilityUtils.getDefinedObjects(sa.getHostCard(), sa.getParam(definedParam), sa);
-    }
-}

forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java

@@ -1,16 +1,22 @@
 package forge.game.ability;
 
-import forge.game.GameObject;
-import forge.game.card.CardFactoryUtil;
-import forge.game.spellability.AbilitySub;
-import forge.game.spellability.SpellAbility;
-
-import org.apache.commons.lang3.StringUtils;
-
 import java.util.List;
 import java.util.Map;
 import java.util.StringTokenizer;
 
+import org.apache.commons.lang3.StringUtils;
+
+import com.google.common.collect.Lists;
+
+import forge.game.GameObject;
+import forge.game.card.Card;
+import forge.game.card.CardCollection;
+import forge.game.card.CardFactoryUtil;
+import forge.game.player.Player;
+import forge.game.spellability.AbilitySub;
+import forge.game.spellability.SpellAbility;
+import forge.util.FCollection;
+
 /**
  * <p>
  * AbilityFactory_AlterLife class.
@@ -20,7 +26,7 @@ import java.util.StringTokenizer;
  * @version $Id: AbilityFactoryAlterLife.java 17656 2012-10-22 19:32:56Z Max mtg $
  */
 
-public abstract class SpellAbilityEffect extends SaTargetRoutines {
+public abstract class SpellAbilityEffect {
 
     public abstract void resolve(SpellAbility sa);
 
@@ -129,4 +135,52 @@ public abstract class SpellAbilityEffect extends SaTargetRoutines {
             }
         }
     }
+
+    // Target/defined methods
+    // Cards
+    protected final static CardCollection getTargetCards(final SpellAbility sa) {                                       return getCards(false, "Defined",    sa); }
+    protected final static CardCollection getTargetCards(final SpellAbility sa, final String definedParam) {            return getCards(false, definedParam, sa); }
+    protected final static CardCollection getDefinedCardsOrTargeted(final SpellAbility sa) {                            return getCards(true,  "Defined",    sa); }
+    protected final static CardCollection getDefinedCardsOrTargeted(final SpellAbility sa, final String definedParam) { return getCards(true,  definedParam, sa); }
+
+    private static CardCollection getCards(final boolean definedFirst, final String definedParam, final SpellAbility sa) {
+        final boolean useTargets = sa.usesTargeting() && (!definedFirst || !sa.hasParam(definedParam))
+                && sa.getTargets() != null && (sa.getTargets().isTargetingAnyCard() || sa.getTargets().getTargets().isEmpty());
+        return useTargets ? new CardCollection(sa.getTargets().getTargetCards()) 
+                : AbilityUtils.getDefinedCards(sa.getHostCard(), sa.getParam(definedParam), sa);
+    }
+
+    // Players
+    protected final static FCollection<Player> getTargetPlayers(final SpellAbility sa) {                                       return getPlayers(false, "Defined",    sa); }
+    protected final static FCollection<Player> getTargetPlayers(final SpellAbility sa, final String definedParam) {            return getPlayers(false, definedParam, sa); }
+    protected final static FCollection<Player> getDefinedPlayersOrTargeted(final SpellAbility sa ) {                           return getPlayers(true,  "Defined",    sa); }
+    protected final static FCollection<Player> getDefinedPlayersOrTargeted(final SpellAbility sa, final String definedParam) { return getPlayers(true,  definedParam, sa); }
+
+    private static FCollection<Player> getPlayers(final boolean definedFirst, final String definedParam, final SpellAbility sa) {
+        final boolean useTargets = sa.usesTargeting() && (!definedFirst || !sa.hasParam(definedParam));
+        return useTargets ? new FCollection<Player>(sa.getTargets().getTargetPlayers()) 
+                : AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam(definedParam), sa);
+    }
+
+    // Spells
+    protected final static List<SpellAbility> getTargetSpells(final SpellAbility sa) {                                       return getSpells(false, "Defined",    sa); }
+    protected final static List<SpellAbility> getTargetSpells(final SpellAbility sa, final String definedParam) {            return getSpells(false, definedParam, sa); }
+    protected final static List<SpellAbility> getDefinedSpellsOrTargeted(final SpellAbility sa, final String definedParam) { return getSpells(true,  definedParam, sa); }
+
+    private static List<SpellAbility> getSpells(final boolean definedFirst, final String definedParam, final SpellAbility sa) {
+        final boolean useTargets = sa.usesTargeting() && (!definedFirst || !sa.hasParam(definedParam));
+        return useTargets ? Lists.newArrayList(sa.getTargets().getTargetSpells()) 
+                : AbilityUtils.getDefinedSpellAbilities(sa.getHostCard(), sa.getParam(definedParam), sa);
+    }
+
+    // Targets of unspecified type
+    protected final static List<GameObject> getTargets(final SpellAbility sa) {                                return getTargetables(false, "Defined",    sa); }
+    protected final static List<GameObject> getTargets(final SpellAbility sa, final String definedParam) {     return getTargetables(false, definedParam, sa); }
+    protected final static List<GameObject> getDefinedOrTargeted(SpellAbility sa, final String definedParam) { return getTargetables(true,  definedParam, sa); }
+
+    private static List<GameObject> getTargetables(final boolean definedFirst, final String definedParam, final SpellAbility sa) {
+        final boolean useTargets = sa.usesTargeting() && (!definedFirst || !sa.hasParam(definedParam));
+        return useTargets ? Lists.newArrayList(sa.getTargets().getTargets()) 
+                : AbilityUtils.getDefinedObjects(sa.getHostCard(), sa.getParam(definedParam), sa);
+    }
 }

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

@@ -1,10 +1,19 @@
 package forge.game.ability.effects;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+
 import com.google.common.collect.Lists;
 
 import forge.game.Game;
 import forge.game.ability.AbilityFactory;
 import forge.game.ability.AbilityUtils;
+import forge.game.ability.IllegalAbilityException;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
 import forge.game.card.CardCollection;
@@ -17,8 +26,6 @@ import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
 import forge.util.Aggregates;
 import forge.util.FCollection;
-import java.util.*;
-import java.util.Map.Entry;
 
 public class RepeatEachEffect extends SpellAbilityEffect {
 
@@ -63,6 +70,9 @@ public class RepeatEachEffect extends SpellAbilityEffect {
                 loopOverCards = true;
             }
         }
+        else {
+            throw new IllegalAbilityException(sa, this);
+        }
 
         if (sa.hasParam("ClearRemembered")) {
             source.clearRemembered();

forge-game/src/main/java/forge/game/player/PlayerView.java

@@ -8,6 +8,7 @@ import java.util.Map.Entry;
 
 import org.apache.commons.lang3.StringUtils;
 
+import com.google.common.base.Objects;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
@@ -23,6 +24,7 @@ import forge.game.zone.ZoneType;
 import forge.trackable.TrackableCollection;
 import forge.trackable.TrackableProperty;
 import forge.trackable.Tracker;
+import forge.util.FCollection;
 import forge.util.FCollectionView;
 import forge.util.Lang;
 
@@ -82,7 +84,7 @@ public class PlayerView extends GameEntityView {
     }
 
     public FCollectionView<PlayerView> getOpponents() {
-        return get(TrackableProperty.Opponents);
+        return Objects.firstNonNull(this.<FCollectionView<PlayerView>>get(TrackableProperty.Opponents), new FCollection<PlayerView>());
     }
     void updateOpponents(Player p) {
         set(TrackableProperty.Opponents, PlayerView.getCollection(p.getOpponents()));
@@ -123,7 +125,7 @@ public class PlayerView extends GameEntityView {
         final FCollectionView<PlayerView> opponents = getOpponents();
         final List<String> info = Lists.newArrayListWithExpectedSize(opponents.size());
         info.add(String.format("Commander: %s", commander));
-        for (final PlayerView p : Iterables.concat(Collections.singleton(this), opponents == null ? Collections.<PlayerView>emptySet() : opponents)) {
+        for (final PlayerView p : Iterables.concat(Collections.singleton(this), opponents)) {
             final int damage = this.getCommanderDamage(p.getCommander());
             if (damage > 0) {
                 final String text;

forge-game/src/main/java/forge/game/spellability/SpellAbility.java

@@ -103,7 +103,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     private int totalManaSpent = 0;
 
     /** The pay costs. */
-    private Cost payCosts = null;
+    private Cost payCosts;
     private SpellAbilityRestriction restrictions = new SpellAbilityRestriction();
     private SpellAbilityCondition conditions = new SpellAbilityCondition();
     private AbilitySub subAbility = null;

forge-gui/src/main/java/forge/deck/DeckgenUtil.java

@@ -66,7 +66,7 @@ public class DeckgenUtil {
             }
             gen.setSingleton(FModel.getPreferences().getPrefBoolean(FPref.DECKGEN_SINGLETONS));
             gen.setUseArtifacts(!FModel.getPreferences().getPrefBoolean(FPref.DECKGEN_ARTIFACTS));
-            CardPool cards = gen == null ? null : gen.getDeck(60, forAi);
+            final CardPool cards = gen.getDeck(60, forAi);
     
             if (null == deckName) {
                 deckName = Lang.joinHomogenous(Arrays.asList(selection));
@@ -312,7 +312,7 @@ public class DeckgenUtil {
         gen = new DeckGenerator2Color(cardDb, format, comColors.get(0), comColors.get(1));
         gen.setSingleton(true);
         gen.setUseArtifacts(!FModel.getPreferences().getPrefBoolean(FPref.DECKGEN_ARTIFACTS));
-        CardPool cards = gen == null ? null : gen.getDeck(gameType.getDeckFormat().getMainRange().getMaximum(), forAi);
+        CardPool cards = gen.getDeck(gameType.getDeckFormat().getMainRange().getMaximum(), forAi);
 
         // After generating card lists, build deck.
         deck = new Deck("Generated " + gameType.toString() + " deck (" + commander.getName() + ")");

forge-gui/src/main/java/forge/match/GameLobby.java

@@ -385,13 +385,14 @@ public abstract class GameLobby {
                     deck = autoGenerateVariant.autoGenerateDeck(rp);
                 }
 
+                // Initialise variables for other variants
+                deck = deck == null ? rp.getDeck() : deck;
+
                 final CardPool avatarPool = deck.get(DeckSection.Avatar);
                 if (avatarPool != null) {
                     vanguardAvatar = avatarPool.get(0);
                 }
 
-                // Initialise variables for other variants
-                deck = deck == null ? rp.getDeck() : deck;
                 Iterable<PaperCard> schemes = null;
                 Iterable<PaperCard> planes = null;
 

forge-gui/src/main/java/forge/net/FGameClient.java

@@ -155,18 +155,19 @@ public class FGameClient implements IToServer {
                 replies.complete(event.getIndex(), event.getReply());
             } else if (msg instanceof GuiGameEvent) {
                 final GuiGameEvent event = (GuiGameEvent) msg;
+                final String method = event.getMethod();
                 final Object[] args = event.getObjects();
                 Serializable reply = null;
                 boolean doReply = false;
 
                 final IButton btn;
-                if (args.length >= 2 && args[0] instanceof PlayerView && args[1] instanceof MatchButtonType) {
+                if (method.startsWith("btn_") && args.length >= 2 && args[0] instanceof PlayerView && args[1] instanceof MatchButtonType) {
                     btn = args[1] == MatchButtonType.OK ? clientGui.getBtnOK((PlayerView) args[0]) : clientGui.getBtnCancel((PlayerView) args[0]);
                 } else {
                     btn = null;
                 }
 
-                switch (event.getMethod()) {
+                switch (method) {
                 case "setGameView":
                     clientGui.setGameView((GameView) args[0]);
                     break;