PlayerController: chooseColor returns MagicColor.Color

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

@@ -241,7 +241,8 @@ public class PlayerControllerAi extends PlayerController {
     public Map<Byte, Integer> specifyManaCombo(SpellAbility sa, ColorSet colorSet, int manaAmount, boolean different) {
         Map<Byte, Integer> result = new HashMap<>();
         for (int i = 0; i < manaAmount; ++i) {
-            Byte chosen = chooseColor("", sa, colorSet);
+            MagicColor.Color chosenColor = chooseColor("", sa, colorSet);
+            Byte chosen = chosenColor == null ? (byte)0 : chosenColor.getColormask();
             if (result.containsKey(chosen)) {
                 result.put(chosen, result.get(chosen) + 1);
             } else {
@@ -1018,19 +1019,22 @@ public class PlayerControllerAi extends PlayerController {
     }
 
     @Override
-    public byte chooseColorAllowColorless(String message, Card card, ColorSet colors) {
+    public MagicColor.Color chooseColorAllowColorless(String message, Card card, ColorSet colors) {
         final String c = ComputerUtilCard.getMostProminentColor(player.getCardsIn(ZoneType.Hand));
         byte chosenColorMask = MagicColor.fromName(c);
         if ((colors.getColor() & chosenColorMask) != 0) {
-            return chosenColorMask;
+            return MagicColor.Color.fromByte(chosenColorMask);
         }
-        return Iterables.getFirst(colors, (byte)0);
+        return Iterables.getFirst(colors.toEnumSet(), MagicColor.Color.COLORLESS);
     }
 
     @Override
-    public byte chooseColor(String message, SpellAbility sa, ColorSet colors) {
+    public MagicColor.Color chooseColor(String message, SpellAbility sa, ColorSet colors) {
+        if (colors.countColors() == 0) {
+            return null;
+        }
         if (colors.countColors() < 2) {
-            return Iterables.getFirst(colors, MagicColor.WHITE);
+            return Iterables.getFirst(colors.toEnumSet(), MagicColor.Color.WHITE);
         }
         // You may switch on sa.getApi() here and use sa.getParam("AILogic")
         CardCollectionView hand = player.getCardsIn(ZoneType.Hand);
@@ -1041,9 +1045,9 @@ public class PlayerControllerAi extends PlayerController {
         byte chosenColorMask = MagicColor.fromName(c);
 
         if ((colors.getColor() & chosenColorMask) != 0) {
-            return chosenColorMask;
+            return MagicColor.Color.fromByte(chosenColorMask);
         }
-        return Iterables.getFirst(colors, MagicColor.WHITE);
+        return Iterables.getFirst(colors.toEnumSet(), MagicColor.Color.WHITE);
     }
 
     @Override

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

@@ -30,26 +30,27 @@ public class ChangeTextEffect extends SpellAbilityEffect {
 
         final String changedColorWordOriginal, changedColorWordNew;
         if (sa.hasParam("ChangeColorWord")) {
-            byte originalColor = 0;
+            // all instances are Choose Choose
+            MagicColor.Color originalColor = null;
             final String[] changedColorWordsArray = sa.getParam("ChangeColorWord").split(" ");
             if (changedColorWordsArray[0].equals("Choose")) {
                 originalColor = sa.getActivatingPlayer().getController().chooseColor(
                         Localizer.getInstance().getMessage("lblChooseColorReplace"), sa, ColorSet.ALL_COLORS);
-                changedColorWordOriginal = TextUtil.capitalize(MagicColor.toLongString(originalColor));
+                changedColorWordOriginal = TextUtil.capitalize(originalColor.getName());
             } else {
                 changedColorWordOriginal = changedColorWordsArray[0];
-                originalColor = MagicColor.fromName(changedColorWordOriginal);
+                originalColor = MagicColor.Color.fromByte(MagicColor.fromName(changedColorWordOriginal));
             }
 
             if (changedColorWordsArray[1].equals("Choose")) {
                 final ColorSet possibleNewColors;
-                if (originalColor == 0) { // no original color (ie. any or absent)
+                if (originalColor == null) { // no original color (ie. any or absent)
                     possibleNewColors = ColorSet.ALL_COLORS;
                 } else { // may choose any except original color
-                    possibleNewColors = ColorSet.fromMask(originalColor).inverse();
+                    possibleNewColors = ColorSet.fromMask(originalColor.getColormask()).inverse();
                 }
                 final byte newColor = sa.getActivatingPlayer().getController().chooseColor(
-                        Localizer.getInstance().getMessage("lblChooseNewColor"), sa, possibleNewColors);
+                        Localizer.getInstance().getMessage("lblChooseNewColor"), sa, possibleNewColors).getColormask();
                 changedColorWordNew = TextUtil.capitalize(MagicColor.toLongString(newColor));
             } else {
                 changedColorWordNew = changedColorWordsArray[1];

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

@@ -114,15 +114,15 @@ public class ManaEffect extends SpellAbilityEffect {
                             // just use the first possible color.
                             choice = colorsProduced[differentChoice ? nMana : 0];
                         } else {
-                            byte chosenColor = chooser.getController().chooseColor(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa,
+                            MagicColor.Color chosenColor = chooser.getController().chooseColor(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa,
                                     differentChoice && (colorsNeeded == null || colorsNeeded.length <= nMana) ? fullOptions : colorOptions);
-                            if (chosenColor == 0)
+                            if (chosenColor == null)
                                 throw new RuntimeException("ManaEffect::resolve() /*combo mana*/ - " + p + " color mana choice is empty for " + card.getName());
 
                             if (differentChoice) {
-                                fullOptions = ColorSet.fromMask(fullOptions.getColor() - chosenColor);
+                                fullOptions = ColorSet.fromMask(fullOptions.getColor() - chosenColor.getColormask());
                             }
-                            choice = MagicColor.toShortString(chosenColor);
+                            choice = chosenColor.getShortName();
                         }
 
                         if (nMana > 0) {
@@ -157,13 +157,13 @@ public class ManaEffect extends SpellAbilityEffect {
                     mask |= MagicColor.fromName(colorsNeeded.charAt(nChar));
                 }
                 colorMenu = mask == 0 ? ColorSet.ALL_COLORS : ColorSet.fromMask(mask);
-                byte val = chooser.getController().chooseColor(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa, colorMenu);
+                MagicColor.Color val = chooser.getController().chooseColor(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa, colorMenu);
-                if (0 == val) {
+                if (val == null) {
                     throw new RuntimeException("ManaEffect::resolve() /*any mana*/ - " + p + " color mana choice is empty for " + card.getName());
                 }
 
-                game.getAction().notifyOfValue(sa, card, MagicColor.toSymbol(val), p);
+                game.getAction().notifyOfValue(sa, card, val.getSymbol(), p);
-                abMana.setExpressChoice(MagicColor.toShortString(val));
+                abMana.setExpressChoice(val.getShortName());
             }
             else if (abMana.isSpecialMana()) {
                 String type = abMana.getOrigProduced().split("Special ")[1];
@@ -178,22 +178,22 @@ public class ManaEffect extends SpellAbilityEffect {
 
                     for (ManaCostShard s : enchanted.getManaCost()) {
                         ColorSet cs = ColorSet.fromMask(s.getColorMask());
-                        byte chosenColor;
+                        MagicColor.Color chosenColor;
                         if (cs.isColorless())
                             continue;
                         if (s.isOr2Generic()) { // CR 106.8
                             chosenColor = chooser.getController().chooseColorAllowColorless(Localizer.getInstance().getMessage("lblChooseSingleColorFromTarget", s.toString()), card, cs);
-                            if (chosenColor == MagicColor.COLORLESS) {
+                            if (chosenColor == MagicColor.Color.COLORLESS) {
                                 generic += 2;
                                 continue;
                             }
                         }
                         else if (cs.isMonoColor())
-                            chosenColor = s.getColorMask();
+                            chosenColor = MagicColor.Color.fromByte(s.getColorMask());
                         else /* (cs.isMulticolor()) */ {
                             chosenColor = chooser.getController().chooseColor(Localizer.getInstance().getMessage("lblChooseSingleColorFromTarget", s.toString()), sa, cs);
                         }
-                        sb.append(MagicColor.toShortString(chosenColor));
+                        sb.append(chosenColor.getShortName());
                         sb.append(' ');
                     }
                     if (generic > 0) {
@@ -239,8 +239,8 @@ public class ManaEffect extends SpellAbilityEffect {
                             if (cs.isMonoColor())
                                 sb.append(MagicColor.toShortString(s.getColorMask()));
                             else /* (cs.isMulticolor()) */ {
-                                byte chosenColor = chooser.getController().chooseColor(Localizer.getInstance().getMessage("lblChooseSingleColorFromTarget", s.toString()), sa, cs);
+                                MagicColor.Color chosenColor = chooser.getController().chooseColor(Localizer.getInstance().getMessage("lblChooseSingleColorFromTarget", s.toString()), sa, cs);
-                                sb.append(MagicColor.toShortString(chosenColor));
+                                sb.append(chosenColor.getShortName());
                             }
                         }
                     }

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

@@ -95,7 +95,7 @@ public class ManaReflectedEffect extends SpellAbilityEffect {
         }
 
         if (mask == 0 && !expressChoiceColors.isEmpty() && colors.contains("colorless")) {
-            baseMana = MagicColor.toShortString(player.getController().chooseColorAllowColorless(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa.getHostCard(), ColorSet.fromMask(mask)));
+            baseMana = player.getController().chooseColorAllowColorless(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa.getHostCard(), ColorSet.fromMask(mask)).getShortName();
         } else {
             // Nothing set previously so ask player if needed
             if (mask == 0) {
@@ -105,18 +105,18 @@ public class ManaReflectedEffect extends SpellAbilityEffect {
                     baseMana = MagicColor.toShortString(colors.iterator().next());
                 } else {
                     if (colors.contains("colorless")) {
-                        baseMana = MagicColor.toShortString(player.getController().chooseColorAllowColorless(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa.getHostCard(), ColorSet.fromNames(colors)));
+                        baseMana = player.getController().chooseColorAllowColorless(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa.getHostCard(), ColorSet.fromNames(colors)).getShortName();
                     } else {
-                        baseMana = MagicColor.toShortString(player.getController().chooseColor(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa, ColorSet.fromNames(colors)));
+                        baseMana = player.getController().chooseColor(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa, ColorSet.fromNames(colors)).getShortName();
                     }
                 }
             } else {
                 colorMenu = ColorSet.fromMask(mask);
-                byte color = sa.getActivatingPlayer().getController().chooseColor(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa, colorMenu);
+                MagicColor.Color color = sa.getActivatingPlayer().getController().chooseColor(Localizer.getInstance().getMessage("lblSelectManaProduce"), sa, colorMenu);
-                if (color == 0) {
+                if (color == null) {
                     System.err.println("Unexpected behavior in ManaReflectedEffect: " + sa.getActivatingPlayer() + " - color mana choice is empty for " + sa.getHostCard().getName());
                 }
-                baseMana = MagicColor.toShortString(color);
+                baseMana = color.getShortName();
             }
         }
 

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

@@ -34,15 +34,15 @@ public class ReplaceManaEffect extends SpellAbilityEffect {
             // replace type and amount
             replaced = sa.getParam("ReplaceMana");
             if ("Any".equals(replaced)) {
-                byte rs = player.getController().chooseColor("Choose a color", sa, ColorSet.ALL_COLORS);
+                MagicColor.Color rs = player.getController().chooseColor("Choose a color", sa, ColorSet.ALL_COLORS);
-                replaced = MagicColor.toShortString(rs);
+                replaced = rs.getShortName();
             }
         } else if (sa.hasParam("ReplaceType")) {
             // replace color and colorless
             String color = sa.getParam("ReplaceType");
             if ("Any".equals(color)) {
-                byte rs = player.getController().chooseColor("Choose a color", sa, ColorSet.ALL_COLORS);
+                MagicColor.Color rs = player.getController().chooseColor("Choose a color", sa, ColorSet.ALL_COLORS);
-                color = MagicColor.toShortString(rs);
+                color = rs.getShortName();
             } else {
                 // convert in case Color Word used
                 color = MagicColor.toShortString(color);

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

@@ -6,6 +6,7 @@ import com.google.common.collect.Multimap;
 import forge.LobbyPlayer;
 import forge.card.ColorSet;
 import forge.card.ICardFace;
+import forge.card.MagicColor;
 import forge.card.mana.ManaCost;
 import forge.card.mana.ManaCostShard;
 import forge.deck.Deck;
@@ -265,8 +266,8 @@ public abstract class PlayerController {
 
     public abstract boolean chooseFlipResult(SpellAbility sa, Player flipper, boolean[] results, boolean call);
 
-    public abstract byte chooseColor(String message, SpellAbility sa, ColorSet colors);
+    public abstract MagicColor.Color chooseColor(String message, SpellAbility sa, ColorSet colors);
-    public abstract byte chooseColorAllowColorless(String message, Card c, ColorSet colors);
+    public abstract MagicColor.Color chooseColorAllowColorless(String message, Card c, ColorSet colors);
     public abstract List<String> chooseColors(String message, SpellAbility sa, int min, int max, List<String> options);
 
     public abstract ICardFace chooseSingleCardFace(SpellAbility sa, String message, Predicate<ICardFace> cpp, String name);

forge-gui-desktop/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java

@@ -470,13 +470,16 @@ public class PlayerControllerForTests extends PlayerController {
     }
 
     @Override
-    public byte chooseColor(String message, SpellAbility sa, ColorSet colors) {
+    public MagicColor.Color chooseColor(String message, SpellAbility sa, ColorSet colors) {
-        return Iterables.getFirst(colors, MagicColor.WHITE);
+        if (colors.countColors()==0) {
+            return null;
+        }
+        return Iterables.getFirst(colors.toEnumSet(), MagicColor.Color.WHITE);
     }
 
     @Override
-    public byte chooseColorAllowColorless(String message, Card card, ColorSet colors) {
+    public MagicColor.Color chooseColorAllowColorless(String message, Card card, ColorSet colors) {
-        return Iterables.getFirst(colors, (byte)0);
+        return Iterables.getFirst(colors.toEnumSet(), MagicColor.Color.COLORLESS);
     }
 
     private CardCollection chooseItems(CardCollectionView items, int amount) {

forge-gui/src/main/java/forge/gamemodes/match/input/InputSelectCardsForConvokeOrImprovise.java

@@ -77,7 +77,7 @@ public final class InputSelectCardsForConvokeOrImprovise extends InputSelectMany
                 }
                 if (colors.isMulticolor()) {
                     //prompt user if more than one option for which color to pay towards convoke
-                    chosenColor = player.getController().chooseColorAllowColorless("Convoke " + card.toString() + "  for which color?", card, colors);
+                    chosenColor = player.getController().chooseColorAllowColorless("Convoke " + card.toString() + "  for which color?", card, colors).getColormask();
                 } else {
                     // Since the convoke mana logic can use colored mana as generic if needed,
                     // there is no need to prompt the user when convoking with a mono-color creature.

forge-gui/src/main/java/forge/player/PlayerControllerHuman.java

@@ -1838,30 +1838,30 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
     }
 
     @Override
-    public byte chooseColor(final String message, final SpellAbility sa, final ColorSet colors) {
+    public MagicColor.Color chooseColor(final String message, final SpellAbility sa, final ColorSet colors) {
         final int cntColors = colors.countColors();
         switch (cntColors) {
             case 0:
-                return 0;
+                return null;
             case 1:
-                return colors.getColor();
+                return MagicColor.Color.fromByte(colors.getColor());
             default:
                 return chooseColorCommon(message, sa == null ? null : sa.getHostCard(), colors, false);
         }
     }
 
     @Override
-    public byte chooseColorAllowColorless(final String message, final Card c, final ColorSet colors) {
+    public MagicColor.Color chooseColorAllowColorless(final String message, final Card c, final ColorSet colors) {
         final int cntColors = 1 + colors.countColors();
         switch (cntColors) {
             case 1:
-                return 0;
+                return MagicColor.Color.COLORLESS;
             default:
                 return chooseColorCommon(message, c, colors, true);
         }
     }
 
-    private byte chooseColorCommon(final String message, final Card c, final ColorSet colors,
+    private MagicColor.Color chooseColorCommon(final String message, final Card c, final ColorSet colors,
                                    final boolean withColorless) {
         List<MagicColor.Color> options = Lists.newArrayList(colors.toEnumSet());
         if (withColorless && colors.countColors() > 0) {
@@ -1869,14 +1869,14 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
         }
 
         if (options.size() > 2) {
-            return getGui().one(message, options).getColormask();
+            return getGui().one(message, options);
         }
 
         boolean confirmed = false;
         confirmed = InputConfirm.confirm(this, CardView.get(c), message, true,
                 options.stream().map(MagicColor.Color::toString).collect(Collectors.toList()));
         final int idxChosen = confirmed ? 0 : 1;
-        return options.get(idxChosen).getColormask();
+        return options.get(idxChosen);
     }
 
     @Override