Merge pull request #2716 from tool4ever/charmFix

Fix NPE when trying to build Charm description

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

@@ -26,7 +26,7 @@ public class CharmAi extends SpellAbilityAi {
     @Override
     protected boolean checkApiLogic(Player ai, SpellAbility sa) {
         final Card source = sa.getHostCard();
-        List<AbilitySub> choices = CharmEffect.makePossibleOptions(sa);
+        List<AbilitySub> choices = CharmEffect.makePossibleOptions(sa, false);
 
         final int num;
         final int min;

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

@@ -380,6 +380,8 @@ public final class GameActionUtil {
             return costs;
         }
 
+        sa.clearPipsToReduce();
+
         Card source = sa.getHostCard();
         final Game game = source.getGame();
         boolean lkicheck = false;

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

@@ -20,7 +20,7 @@ import forge.util.collect.FCollection;
 
 public class CharmEffect extends SpellAbilityEffect {
 
-    public static List<AbilitySub> makePossibleOptions(final SpellAbility sa) {
+    public static List<AbilitySub> makePossibleOptions(final SpellAbility sa, boolean forDesc) {
         final Card source = sa.getHostCard();
         List<String> restriction = null;
 
@@ -29,16 +29,19 @@ public class CharmEffect extends SpellAbilityEffect {
         }
 
         List<AbilitySub> choices = Lists.newArrayList(sa.getAdditionalAbilityList("Choices"));
-        List<AbilitySub> toRemove = Lists.newArrayList();
+
-        for (AbilitySub ch : choices) {
+        if (!forDesc) {
-            // 603.3c If one of the modes would be illegal, that mode can't be chosen.
+            List<AbilitySub> toRemove = Lists.newArrayList();
-            if ((ch.usesTargeting() && ch.isTrigger() && ch.getMinTargets() > 0 &&
+            for (AbilitySub ch : choices) {
-                    ch.getTargetRestrictions().getNumCandidates(ch, true) == 0) ||
+                // 603.3c If one of the modes would be illegal, that mode can't be chosen.
-                    (restriction != null && restriction.contains(ch.getDescription()))) {
+                if ((ch.usesTargeting() && ch.isTrigger() && ch.getMinTargets() > 0 &&
-                toRemove.add(ch);
+                        ch.getTargetRestrictions().getNumCandidates(ch, true) == 0) ||
+                        (restriction != null && restriction.contains(ch.getDescription()))) {
+                    toRemove.add(ch);
+                }
             }
+            choices.removeAll(toRemove);
         }
-        choices.removeAll(toRemove);
 
         int indx = 0;
         // set CharmOrder
@@ -52,7 +55,7 @@ public class CharmEffect extends SpellAbilityEffect {
     public static String makeFormatedDescription(SpellAbility sa) {
         Card source = sa.getHostCard();
 
-        List<AbilitySub> list = CharmEffect.makePossibleOptions(sa);
+        List<AbilitySub> list = CharmEffect.makePossibleOptions(sa, true);
         final int num;
         boolean additionalDesc = sa.hasParam("AdditionalDescription");
         boolean optional = sa.hasParam("Optional");
@@ -166,7 +169,7 @@ public class CharmEffect extends SpellAbilityEffect {
         //this resets all previous choices
         sa.setSubAbility(null);
 
-        List<AbilitySub> choices = makePossibleOptions(sa);
+        List<AbilitySub> choices = makePossibleOptions(sa, false);
 
         // Entwine does use all Choices
         if (sa.isEntwine()) {

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

@@ -258,7 +258,7 @@ public class CopyPermanentEffect extends TokenEffectBase {
             game.updateCombatForView();
             game.fireEvent(new GameEventCombatChanged());
         }
-    } // end resolve
+    }
 
     public static Card getProtoType(final SpellAbility sa, final Card original, final Player newOwner) {
         final Card host = sa.getHostCard();

forge-game/src/main/java/forge/game/cost/CostDiscard.java

@@ -22,7 +22,6 @@ import java.util.Set;
 
 import com.google.common.collect.Sets;
 
-import forge.card.CardType;
 import forge.game.ability.AbilityKey;
 import forge.game.card.Card;
 import forge.game.card.CardCollection;
@@ -106,11 +105,7 @@ public class CostDiscard extends CostPartWithList {
                 desc.append("card");
             }
             else {
-                if (this.getTypeDescription() == null) {
+                desc.append(this.getDescriptiveType());
-                    desc.append(CardType.CoreType.isValidEnum(this.getType()) ? this.getType().toLowerCase() : this.getType());
-                } else {
-                    desc.append(this.getTypeDescription());
-                }
                 desc.append(" card");
             }
 

forge-game/src/main/java/forge/game/cost/CostExile.java

@@ -17,7 +17,6 @@
  */
 package forge.game.cost;
 
-import forge.card.CardType;
 import forge.game.Game;
 import forge.game.ability.SpellAbilityEffect;
 import forge.game.card.Card;
@@ -85,10 +84,7 @@ public class CostExile extends CostPartWithList {
     @Override
     public final String toString() {
         final Integer i = this.convertAmount();
-        String desc = this.getTypeDescription() == null ? this.getType() : this.getTypeDescription();
+        String desc = this.getDescriptiveType();
-        if (CardType.CoreType.isValidEnum(desc)) {
-            desc = desc.toLowerCase();
-        }
         String origin = this.from.name().toLowerCase();
 
         if (this.payCostFromSource()) {

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

@@ -404,10 +404,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     }
 
     public boolean canPlayWithOptionalCost(OptionalCostValue opt) {
-        SpellAbility saOpt = GameActionUtil.addOptionalCosts(this, Lists.newArrayList(opt));
+        return GameActionUtil.addOptionalCosts(this, Lists.newArrayList(opt)).canPlay();
-        boolean result = saOpt.canPlay();
-        saOpt.clearPipsToReduce();
-        return result;
     }
 
     public boolean isPossible() {