From f50f985e996afd48d27c763ca07a0dd22b09bbe0 Mon Sep 17 00:00:00 2001 From: tool4ever Date: Sun, 29 Dec 2024 10:27:07 +0100 Subject: [PATCH] Some cleanup (#6728) --- .../src/main/java/forge/ai/AiController.java | 2 +- .../src/main/java/forge/ai/ComputerUtil.java | 12 -- .../java/forge/ai/PlayerControllerAi.java | 112 ++++++++---------- .../java/forge/game/combat/CombatUtil.java | 7 +- .../main/java/forge/game/player/Player.java | 1 - .../forge/game/player/PlayerController.java | 15 +-- .../home/settings/VSubmenuPreferences.java | 14 +-- .../util/PlayerControllerForTests.java | 7 +- .../forge/player/PlayerControllerHuman.java | 11 +- 9 files changed, 64 insertions(+), 117 deletions(-) diff --git a/forge-ai/src/main/java/forge/ai/AiController.java b/forge-ai/src/main/java/forge/ai/AiController.java index b7350535ae6..edd2e6ed4a7 100644 --- a/forge-ai/src/main/java/forge/ai/AiController.java +++ b/forge-ai/src/main/java/forge/ai/AiController.java @@ -549,7 +549,7 @@ public class AiController { repParams.put(AbilityKey.EffectOnly, true); repParams.put(AbilityKey.CounterTable, table); repParams.put(AbilityKey.CounterMap, table.column(land)); - + boolean foundTapped = false; for (ReplacementEffect re : player.getGame().getReplacementHandler().getReplacementList(ReplacementType.Moved, repParams, ReplacementLayer.Other)) { SpellAbility reSA = re.ensureAbility(); diff --git a/forge-ai/src/main/java/forge/ai/ComputerUtil.java b/forge-ai/src/main/java/forge/ai/ComputerUtil.java index b83fd81ae61..d663d2c0ba4 100644 --- a/forge-ai/src/main/java/forge/ai/ComputerUtil.java +++ b/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); diff --git a/forge-ai/src/main/java/forge/ai/PlayerControllerAi.java b/forge-ai/src/main/java/forge/ai/PlayerControllerAi.java index 998b830a4ba..99233b5b162 100644 --- a/forge-ai/src/main/java/forge/ai/PlayerControllerAi.java +++ b/forge-ai/src/main/java/forge/ai/PlayerControllerAi.java @@ -452,13 +452,13 @@ public class PlayerControllerAi extends PlayerController { } @Override - public Player chooseStartingPlayer(boolean isFirstgame) { - return this.player; // AI is brave :) + 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 CardCollection orderBlockers(Card attacker, CardCollection blockers) { - return AiBlockController.orderBlockers(attacker, blockers); + public boolean confirmReplacementEffect(ReplacementEffect replacementEffect, SpellAbility effectSA, GameEntity affected, String question) { + 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) { - if (ComputerUtil.playNoStack(c.getController(), sa, getGame(), true)) { - return true; - } - return false; - } - - @Override - public List chooseSaToActivateFromOpeningHand(List usableFromOpeningHand) { - return brains.chooseSaToActivateFromOpeningHand(usableFromOpeningHand); + public Player chooseStartingPlayer(boolean isFirstgame) { + return this.player; // AI is brave :) } @Override @@ -863,6 +846,11 @@ public class PlayerControllerAi extends PlayerController { return bestZone; } + @Override + public List chooseSaToActivateFromOpeningHand(List 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 cpp, String name) { - throw new UnsupportedOperationException("Should not be called for AI"); // or implement it if you know how - } - @Override public List chooseColors(String message, SpellAbility sa, int min, int max, List 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 - 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); + return Aggregates.random(options); // if worst comes to worst, at least do something } @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 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 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 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 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 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 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 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 states, String message, Map params) { ApiType api = sa.getApi(); @@ -1638,6 +1623,11 @@ public class PlayerControllerAi extends PlayerController { return max; } + @Override + public List orderCosts(List costs) { + return costs; + } + @Override public CardCollection chooseCardsForEffectMultiple(Map 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 orderCosts(List costs) { - return costs; - } } diff --git a/forge-game/src/main/java/forge/game/combat/CombatUtil.java b/forge-game/src/main/java/forge/game/combat/CombatUtil.java index 77cc98d5179..943411ac261 100644 --- a/forge-game/src/main/java/forge/game/combat/CombatUtil.java +++ b/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, - "Pay additional cost to declare " + attacker + " an attacker", ManaPaymentPurpose.DeclareAttacker); + return attacker.getController().getController().payCombatCost(attacker, attackCost, fakeSA, + "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) { diff --git a/forge-game/src/main/java/forge/game/player/Player.java b/forge-game/src/main/java/forge/game/player/Player.java index 5bc8147b807..79ca1eb04b9 100644 --- a/forge-game/src/main/java/forge/game/player/Player.java +++ b/forge-game/src/main/java/forge/game/player/Player.java @@ -1807,7 +1807,6 @@ public class Player extends GameEntity implements Comparable { public final void addMaingameCardMapping(Card subgameCard, Card maingameCard) { maingameCardsMap.put(subgameCard, maingameCard); } - public final Card getMappingMaingameCard(Card subgameCard) { return maingameCardsMap.get(subgameCard); } diff --git a/forge-game/src/main/java/forge/game/player/PlayerController.java b/forge-game/src/main/java/forge/game/player/PlayerController.java index 75bfd74954f..e11046eb3b8 100644 --- a/forge-game/src/main/java/forge/game/player/PlayerController.java +++ b/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 abilities) { return getAbilityToPlay(hostCard, abilities, null); } public abstract SpellAbility getAbilityToPlay(Card hostCard, List abilities, ITriggerEvent triggerEvent); - @Deprecated - public abstract void playSpellAbilityForFree(SpellAbility copySA, boolean mayChoseNewTargets); public abstract void playSpellAbilityNoStack(SpellAbility effectSA, boolean mayChoseNewTargets); public abstract List 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 params); - public abstract boolean helpPayForAssistSpell(ManaCostBeingPaid cost, SpellAbility sa, int max, int requested); public abstract Player choosePlayerToAssistPayment(FCollectionView optionList, SpellAbility sa, String title, int max); public final T chooseSingleEntityForEffect(FCollectionView optionList, SpellAbility sa, String title, Map 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 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 possibleReplacers); public abstract String chooseProtectionType(String string, SpellAbility sa, List choices); - // these 4 need some refining. public abstract boolean payCostToPreventEffect(Cost cost, SpellAbility sa, boolean alreadyPaid, FCollectionView allPayers); public abstract void orderAndPlaySimultaneousSa(List 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() { diff --git a/forge-gui-desktop/src/main/java/forge/screens/home/settings/VSubmenuPreferences.java b/forge-gui-desktop/src/main/java/forge/screens/home/settings/VSubmenuPreferences.java index fd4912bd138..d6526bcc470 100644 --- a/forge-gui-desktop/src/main/java/forge/screens/home/settings/VSubmenuPreferences.java +++ b/forge-gui-desktop/src/main/java/forge/screens/home/settings/VSubmenuPreferences.java @@ -160,7 +160,6 @@ public enum VSubmenuPreferences implements IVSubmenu { 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 { 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); @@ -232,7 +228,7 @@ public enum VSubmenuPreferences implements IVSubmenu { pnlPrefs.add(cbpLandPlayed, comboBoxConstraints); pnlPrefs.add(new NoteLabel(localizer.getMessage("nlpLandPlayed")), descriptionConstraints); - + pnlPrefs.add(cbEnforceDeckLegality, titleConstraints); pnlPrefs.add(new NoteLabel(localizer.getMessage("nlEnforceDeckLegality")), descriptionConstraints); @@ -797,11 +793,11 @@ public enum VSubmenuPreferences implements IVSubmenu { public FComboBoxPanel getCbpStackAdditionsComboBoxPanel() { return cbpStackAdditions; } - + public FComboBoxPanel getCbpLandPlayedComboBoxPanel() { return cbpLandPlayed; } - + public FComboBoxPanel getGameLogVerbosityComboBoxPanel() { return cbpGameLogEntryType; } @@ -903,7 +899,7 @@ public enum VSubmenuPreferences implements IVSubmenu { public JCheckBox getCbSROptimize() { return cbSROptimize; } - + /** @return {@link javax.swing.JCheckBox} */ public JCheckBox getCbTimedTargOverlay() { return cbTimedTargOverlay; @@ -948,7 +944,7 @@ public enum VSubmenuPreferences implements IVSubmenu { public final JCheckBox getCbManaLostPrompt() { return cbManaLostPrompt; } - + public final JCheckBox getCbDetailedPaymentDesc() { return cbDetailedPaymentDesc; } diff --git a/forge-gui-desktop/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java b/forge-gui-desktop/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java index 44686e6f731..c7a0bf54a78 100644 --- a/forge-gui-desktop/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java +++ b/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!"); } diff --git a/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java b/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java index 3f3de0e8719..86b4782f587 100644 --- a/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java +++ b/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) { - if (cost.isOnlyManaCost() && cost.getTotalMana().isZero() && isFullControl(FullControlFlag.NoFreeCombatCostHandling) - && (purpose == ManaPaymentPurpose.DeclareAttacker || purpose == ManaPaymentPurpose.DeclareBlocker)) { + public boolean payCombatCost(final Card c, final Cost cost, final SpellAbility sa, final String prompt) { + if (cost.isOnlyManaCost() && cost.getTotalMana().isZero() && isFullControl(FullControlFlag.NoFreeCombatCostHandling)) { return true; } return HumanPlay.payCostDuringAbilityResolve(this, player, c, cost, sa, prompt);