Some cleanup (#6728)

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

@@ -249,18 +249,6 @@ public class ComputerUtil {
         return false;
     }
 
-    public static final void playSpellAbilityForFree(final Player ai, final SpellAbility sa) {
-        final Game game = ai.getGame();
-        sa.setActivatingPlayer(ai);
-
-        final Card source = sa.getHostCard();
-        if (sa.isSpell() && !source.isCopiedSpell()) {
-            sa.setHostCard(game.getAction().moveToStack(source, sa));
-        }
-
-        game.getStack().add(sa);
-    }
-
     public static final boolean playSpellAbilityWithoutPayingManaCost(final Player ai, final SpellAbility sa, final Game game) {
         SpellAbility newSA = sa.copyWithNoManaCost();
         newSA.setActivatingPlayer(ai);

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

@@ -452,13 +452,13 @@ public class PlayerControllerAi extends PlayerController {
     }
 
     @Override
-    public Player chooseStartingPlayer(boolean isFirstgame) {
+    public boolean confirmPayment(CostPart costPart, String prompt, SpellAbility sa) {
-        return this.player; // AI is brave :)
+        return brains.confirmPayment(costPart); // AI is expected to know what it is paying for at the moment (otherwise add another parameter to this method)
     }
 
     @Override
-    public CardCollection orderBlockers(Card attacker, CardCollection blockers) {
+    public boolean confirmReplacementEffect(ReplacementEffect replacementEffect, SpellAbility effectSA, GameEntity affected, String question) {
-        return AiBlockController.orderBlockers(attacker, blockers);
+        return brains.aiShouldRun(replacementEffect, effectSA, affected);
     }
 
     @Override
@@ -481,6 +481,11 @@ public class PlayerControllerAi extends PlayerController {
         return chosenAttackers;
     }
 
+    @Override
+    public CardCollection orderBlockers(Card attacker, CardCollection blockers) {
+        return AiBlockController.orderBlockers(attacker, blockers);
+    }
+
     @Override
     public CardCollection orderBlocker(Card attacker, Card blocker, CardCollection oldBlockers) {
     	return AiBlockController.orderBlocker(attacker, blocker, oldBlockers);
@@ -672,20 +677,6 @@ public class PlayerControllerAi extends PlayerController {
                : ComputerUtil.getCardsToDiscardFromOpponent(player, p, sa, validCards, min, max);
     }
 
-    @Override
-    public void playSpellAbilityForFree(SpellAbility copySA, boolean mayChooseNewTargets) {
-        // Ai is known to set targets in doTrigger, so if it cannot choose new targets, we won't call canPlays
-        if (mayChooseNewTargets) {
-            if (copySA instanceof Spell) {
-                Spell spell = (Spell) copySA;
-                ((PlayerControllerAi) player.getController()).getAi().canPlayFromEffectAI(spell, true, true);
-            } else {
-                getAi().canPlaySa(copySA);
-            }
-        }
-        ComputerUtil.playSpellAbilityForFree(player, copySA);
-    }
-
     @Override
     public void playSpellAbilityNoStack(SpellAbility effectSA, boolean canSetupTargets) {
         if (canSetupTargets)
@@ -835,16 +826,8 @@ public class PlayerControllerAi extends PlayerController {
     }
 
     @Override
-    public boolean payManaOptional(Card c, Cost cost, SpellAbility sa, String prompt, ManaPaymentPurpose purpose) {
+    public Player chooseStartingPlayer(boolean isFirstgame) {
-        if (ComputerUtil.playNoStack(c.getController(), sa, getGame(), true)) {
+        return this.player; // AI is brave :)
-            return true;
-        }
-        return false;
-    }
-
-    @Override
-    public List<SpellAbility> chooseSaToActivateFromOpeningHand(List<SpellAbility> usableFromOpeningHand) {
-        return brains.chooseSaToActivateFromOpeningHand(usableFromOpeningHand);
     }
 
     @Override
@@ -863,6 +846,11 @@ public class PlayerControllerAi extends PlayerController {
         return bestZone;
     }
 
+    @Override
+    public List<SpellAbility> chooseSaToActivateFromOpeningHand(List<SpellAbility> usableFromOpeningHand) {
+        return brains.chooseSaToActivateFromOpeningHand(usableFromOpeningHand);
+    }
+
     @Override
     public int chooseNumber(SpellAbility sa, String title, int min, int max) {
         return brains.chooseNumber(sa, title, min, max);
@@ -1032,12 +1020,6 @@ public class PlayerControllerAi extends PlayerController {
         return Iterables.getFirst(colors, MagicColor.WHITE);
     }
 
-    @Override
-    public ICardFace chooseSingleCardFace(SpellAbility sa, String message,
-            Predicate<ICardFace> cpp, String name) {
-        throw new UnsupportedOperationException("Should not be called for AI"); // or implement it if you know how
-    }
-
     @Override
     public List<String> chooseColors(String message, SpellAbility sa, int min, int max, List<String> options) {
         return ComputerUtilCard.chooseColor(sa, min, max, options);
@@ -1123,19 +1105,9 @@ public class PlayerControllerAi extends PlayerController {
         }
         if (!possible.isEmpty()) {
             return Aggregates.random(possible);
-        } else {
-            return Aggregates.random(options); // if worst comes to worst, at least do something
         }
-    }
 
-    @Override
+        return Aggregates.random(options); // if worst comes to worst, at least do something
-    public boolean confirmPayment(CostPart costPart, String prompt, SpellAbility sa) {
-        return brains.confirmPayment(costPart); // AI is expected to know what it is paying for at the moment (otherwise add another parameter to this method)
-    }
-
-    @Override
-    public boolean confirmReplacementEffect(ReplacementEffect replacementEffect, SpellAbility effectSA, GameEntity affected, String question) {
-        return brains.aiShouldRun(replacementEffect, effectSA, affected);
     }
 
     @Override
@@ -1212,6 +1184,19 @@ public class PlayerControllerAi extends PlayerController {
         return choice;
     }
 
+    @Override
+    public boolean payManaCost(ManaCost toPay, CostPartMana costPartMana, SpellAbility sa, String prompt /* ai needs hints as well */, ManaConversionMatrix matrix, boolean effect) {
+        return ComputerUtilMana.payManaCost(new Cost(toPay, effect), player, sa, effect);
+    }
+
+    @Override
+    public boolean payCombatCost(Card c, Cost cost, SpellAbility sa, String prompt) {
+        if (ComputerUtil.playNoStack(c.getController(), sa, getGame(), true)) {
+            return true;
+        }
+        return false;
+    }
+
     @Override
     public boolean payCostToPreventEffect(Cost cost, SpellAbility sa, boolean alreadyPaid, FCollectionView<Player> allPayers) {
         if (SpellApiToAi.Converter.get(sa.getApi()).willPayUnlessCost(sa, player, cost, alreadyPaid, allPayers)) {
@@ -1358,11 +1343,6 @@ public class PlayerControllerAi extends PlayerController {
         return losses;
     }
 
-    @Override
-    public boolean payManaCost(ManaCost toPay, CostPartMana costPartMana, SpellAbility sa, String prompt /* ai needs hints as well */, ManaConversionMatrix matrix, boolean effect) {
-        return ComputerUtilMana.payManaCost(new Cost(toPay, effect), player, sa, effect);
-    }
-
     @Override
     public Map<Card, ManaCostShard> chooseCardsForConvokeOrImprovise(SpellAbility sa, ManaCost manaCost, CardCollectionView untappedCards, boolean improvise) {
         final Player ai = sa.getActivatingPlayer();
@@ -1397,6 +1377,15 @@ public class PlayerControllerAi extends PlayerController {
         return ComputerUtilMana.getConvokeOrImproviseFromList(manaCost, untapped, improvise);
     }
 
+    @Override
+    public String chooseCardName(SpellAbility sa, List<ICardFace> faces, String message) {
+        ApiType api = sa.getApi();
+        if (null == api) {
+            throw new InvalidParameterException("SA is not api-based, this is not supported yet");
+        }
+        return SpellApiToAi.Converter.get(api).chooseCardName(player, sa, faces);
+    }
+
     @Override
     public String chooseCardName(SpellAbility sa, Predicate<ICardFace> cpp, String valid, String message) {
         if (sa.hasParam("AILogic")) {
@@ -1497,15 +1486,6 @@ public class PlayerControllerAi extends PlayerController {
         // Do nothing
     }
 
-    @Override
-    public String chooseCardName(SpellAbility sa, List<ICardFace> faces, String message) {
-        ApiType api = sa.getApi();
-        if (null == api) {
-            throw new InvalidParameterException("SA is not api-based, this is not supported yet");
-        }
-        return SpellApiToAi.Converter.get(api).chooseCardName(player, sa, faces);
-    }
-
     @Override
     public ICardFace chooseSingleCardFace(SpellAbility sa, List<ICardFace> faces, String message) {
         ApiType api = sa.getApi();
@@ -1515,6 +1495,11 @@ public class PlayerControllerAi extends PlayerController {
         return SpellApiToAi.Converter.get(api).chooseCardFace(player, sa, faces);
     }
 
+    @Override
+    public ICardFace chooseSingleCardFace(SpellAbility sa, String message, Predicate<ICardFace> cpp, String name) {
+        throw new UnsupportedOperationException("Should not be called for AI"); // or implement it if you know how
+    }
+
     @Override
     public CardState chooseSingleCardState(SpellAbility sa, List<CardState> states, String message, Map<String, Object> params) {
         ApiType api = sa.getApi();
@@ -1638,6 +1623,11 @@ public class PlayerControllerAi extends PlayerController {
         return max;
     }
 
+    @Override
+    public List<CostPart> orderCosts(List<CostPart> costs) {
+        return costs;
+    }
+
     @Override
     public CardCollection chooseCardsForEffectMultiple(Map<String, CardCollection> validMap, SpellAbility sa, String title, boolean isOptional) {
         CardCollection choices = new CardCollection();
@@ -1654,8 +1644,4 @@ public class PlayerControllerAi extends PlayerController {
         return choices;
     }
 
-    @Override
-    public List<CostPart> orderCosts(List<CostPart> costs) {
-        return costs;
-    }
 }

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

@@ -30,7 +30,6 @@ import forge.game.keyword.Keyword;
 import forge.game.keyword.KeywordInterface;
 import forge.game.phase.PhaseType;
 import forge.game.player.Player;
-import forge.game.player.PlayerController.ManaPaymentPurpose;
 import forge.game.spellability.SpellAbility;
 import forge.game.staticability.StaticAbility;
 import forge.game.staticability.StaticAbilityBlockRestrict;
@@ -275,8 +274,8 @@ public class CombatUtil {
         fakeSA.setPayCosts(attackCost);
         // prevent recalculating X
         fakeSA.setSVar("X", "0");
-        return attacker.getController().getController().payManaOptional(attacker, attackCost, fakeSA,
+        return attacker.getController().getController().payCombatCost(attacker, attackCost, fakeSA,
-                "Pay additional cost to declare " + attacker + " an attacker", ManaPaymentPurpose.DeclareAttacker);
+                "Pay additional cost to declare " + attacker + " an attacker");
     }
 
     public static Cost getAttackCost(final Game game, final Card attacker, final GameEntity defender) {
@@ -338,7 +337,7 @@ public class CombatUtil {
         fakeSA.setCardState(blocker.getCurrentState());
         fakeSA.setPayCosts(blockCost);
         fakeSA.setSVar("X", "0");
-        return blocker.getController().getController().payManaOptional(blocker, blockCost, fakeSA, "Pay cost to declare " + blocker + " a blocker. ", ManaPaymentPurpose.DeclareBlocker);
+        return blocker.getController().getController().payCombatCost(blocker, blockCost, fakeSA, "Pay cost to declare " + blocker + " a blocker. ");
     }
 
     public static Cost getBlockCost(Game game, Card blocker, Card attacker) {

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

@@ -1807,7 +1807,6 @@ public class Player extends GameEntity implements Comparable<Player> {
     public final void addMaingameCardMapping(Card subgameCard, Card maingameCard) {
         maingameCardsMap.put(subgameCard, maingameCard);
     }
-
     public final Card getMappingMaingameCard(Card subgameCard) {
         return maingameCardsMap.get(subgameCard);
     }

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

@@ -48,11 +48,6 @@ import java.util.function.Predicate;
  */
 public abstract class PlayerController {
 
-    public enum ManaPaymentPurpose {
-        DeclareAttacker,
-        DeclareBlocker
-    }
-
     public enum BinaryChoiceType {
         HeadsOrTails, // coin
         TapOrUntap,
@@ -101,8 +96,6 @@ public abstract class PlayerController {
     public final SpellAbility getAbilityToPlay(final Card hostCard, final List<SpellAbility> abilities) { return getAbilityToPlay(hostCard, abilities, null); }
     public abstract SpellAbility getAbilityToPlay(Card hostCard, List<SpellAbility> abilities, ITriggerEvent triggerEvent);
 
-    @Deprecated
-    public abstract void playSpellAbilityForFree(SpellAbility copySA, boolean mayChoseNewTargets);
     public abstract void playSpellAbilityNoStack(SpellAbility effectSA, boolean mayChoseNewTargets);
 
     public abstract List<PaperCard> sideboard(final Deck deck, GameType gameType, String message);
@@ -124,7 +117,6 @@ public abstract class PlayerController {
     // Q: why is there min/max and optional at once? A: This is to handle cases like 'choose 3 to 5 cards or none at all'
     public abstract CardCollectionView chooseCardsForEffect(CardCollectionView sourceList, SpellAbility sa, String title, int min, int max, boolean isOptional, Map<String, Object> params);
 
-
     public abstract boolean helpPayForAssistSpell(ManaCostBeingPaid cost, SpellAbility sa, int max, int requested);
     public abstract Player choosePlayerToAssistPayment(FCollectionView<Player> optionList, SpellAbility sa, String title, int max);
     public final <T extends GameEntity> T chooseSingleEntityForEffect(FCollectionView<T> optionList, SpellAbility sa, String title, Map<String, Object> params) { return chooseSingleEntityForEffect(optionList, null, sa, title, false, null, params); }
@@ -224,11 +216,10 @@ public abstract class PlayerController {
 
     public abstract void declareAttackers(Player attacker, Combat combat);
     public abstract void declareBlockers(Player defender, Combat combat);
+
     public abstract List<SpellAbility> chooseSpellAbilityToPlay();
     public abstract boolean playChosenSpellAbility(SpellAbility sa);
 
-    public abstract boolean payManaOptional(Card card, Cost cost, SpellAbility sa, String prompt, ManaPaymentPurpose purpose);
-
     public abstract int chooseNumberForCostReduction(final SpellAbility sa, final int min, final int max);
     public abstract int chooseNumberForKeywordCost(SpellAbility sa, Cost cost, KeywordInterface keyword, String prompt, int max);
     public boolean addKeywordCost(SpellAbility sa, Cost cost, KeywordInterface keyword, String prompt) {
@@ -266,7 +257,6 @@ public abstract class PlayerController {
     public abstract StaticAbility chooseSingleStaticAbility(String prompt, List<StaticAbility> possibleReplacers);
     public abstract String chooseProtectionType(String string, SpellAbility sa, List<String> choices);
 
-    // these 4 need some refining.
     public abstract boolean payCostToPreventEffect(Cost cost, SpellAbility sa, boolean alreadyPaid, FCollectionView<Player> allPayers);
     public abstract void orderAndPlaySimultaneousSa(List<SpellAbility> activePlayerSAs);
     public abstract boolean playTrigger(Card host, WrappedAbility wrapperAbility, boolean isMandatory);
@@ -283,6 +273,8 @@ public abstract class PlayerController {
 
     public abstract void resetAtEndOfTurn(); // currently used by the AI to perform card memory cleanup
 
+    public abstract boolean payCombatCost(Card card, Cost cost, SpellAbility sa, String prompt);
+
     public final boolean payManaCost(CostPartMana costPartMana, SpellAbility sa, String prompt, ManaConversionMatrix matrix, boolean effect) {
         return payManaCost(costPartMana.getManaCostFor(sa), costPartMana, sa, prompt, matrix, effect);
     }
@@ -311,7 +303,6 @@ public abstract class PlayerController {
     public abstract void autoPassCancel();
 
     public abstract void awaitNextInput();
-
     public abstract void cancelAwaitNextInput();
 
     public void resetInputs() {

forge-gui-desktop/src/main/java/forge/screens/home/settings/VSubmenuPreferences.java

@@ -160,7 +160,6 @@ public enum VSubmenuPreferences implements IVSubmenu<CSubmenuPreferences> {
         final String comboBoxConstraints = "w 80%!, h 25px!, gap 25px 0 0 0px, span 2 1";
         final String descriptionConstraints = "w 80%!, h 22px!, gap 28px 0 0 20px, span 2 1";
 
-        // Troubleshooting
         pnlPrefs.add(new SectionLabel(localizer.getMessage("Troubleshooting")), sectionConstraints);
 
         // Reset buttons
@@ -175,11 +174,8 @@ public enum VSubmenuPreferences implements IVSubmenu<CSubmenuPreferences> {
         pnlPrefs.add(btnClearImageCache, twoButtonConstraints1);
         pnlPrefs.add(btnTokenPreviewer, twoButtonConstraints2);
 
-        // General Configuration
         pnlPrefs.add(new SectionLabel(localizer.getMessage("GeneralConfiguration")), sectionConstraints);
 
-        // language
-
         pnlPrefs.add(cbpAutoUpdater, comboBoxConstraints);
         pnlPrefs.add(new NoteLabel(localizer.getMessage("nlAutoUpdater")), descriptionConstraints);
 

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

@@ -86,11 +86,6 @@ public class PlayerControllerForTests extends PlayerController {
         return player;
     }
 
-    @Override
-    public void playSpellAbilityForFree(SpellAbility copySA, boolean mayChoseNewTargets) {
-        throw new IllegalStateException("Callers of this method currently assume that it performs extra functionality!");
-    }
-
     @Override
     public void playSpellAbilityNoStack(SpellAbility effectSA, boolean mayChoseNewTargets) {
         //TODO: eventually (when the real code is refactored) this should be handled normally...
@@ -447,7 +442,7 @@ public class PlayerControllerForTests extends PlayerController {
     }
 
     @Override
-    public boolean payManaOptional(Card card, Cost cost, SpellAbility sa, String prompt, ManaPaymentPurpose purpose) {
+    public boolean payCombatCost(Card card, Cost cost, SpellAbility sa, String prompt) {
         throw new IllegalStateException("Callers of this method currently assume that it performs extra functionality!");
     }
 

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

@@ -31,7 +31,6 @@ import forge.game.mana.Mana;
 import forge.game.mana.ManaConversionMatrix;
 import forge.game.mana.ManaCostBeingPaid;
 import forge.game.player.*;
-import forge.game.player.PlayerController.FullControlFlag;
 import forge.game.player.actions.SelectCardAction;
 import forge.game.player.actions.SelectPlayerAction;
 import forge.game.replacement.ReplacementEffect;
@@ -238,11 +237,6 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
         return resultView == null ? null : spellViewCache.get(resultView);
     }
 
-    @Override
-    public void playSpellAbilityForFree(final SpellAbility copySA, final boolean mayChoseNewTargets) {
-        HumanPlay.playSaWithoutPayingManaCost(this, copySA, mayChoseNewTargets);
-    }
-
     @Override
     public void playSpellAbilityNoStack(final SpellAbility effectSA, final boolean canSetupTargets) {
         HumanPlay.playSpellAbilityNoStack(this, player, effectSA, !canSetupTargets);
@@ -1556,9 +1550,8 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
     }
 
     @Override
-    public boolean payManaOptional(final Card c, final Cost cost, final SpellAbility sa, final String prompt, final ManaPaymentPurpose purpose) {
+    public boolean payCombatCost(final Card c, final Cost cost, final SpellAbility sa, final String prompt) {
-        if (cost.isOnlyManaCost() && cost.getTotalMana().isZero() && isFullControl(FullControlFlag.NoFreeCombatCostHandling)
+        if (cost.isOnlyManaCost() && cost.getTotalMana().isZero() && isFullControl(FullControlFlag.NoFreeCombatCostHandling)) {
-                && (purpose == ManaPaymentPurpose.DeclareAttacker || purpose == ManaPaymentPurpose.DeclareBlocker)) {
             return true;
         }
         return HumanPlay.payCostDuringAbilityResolve(this, player, c, cost, sa, prompt);