From a395adc2b6366ca9026c2a6a427a2bb105f69b46 Mon Sep 17 00:00:00 2001 From: Maxmtg Date: Fri, 21 Jun 2013 11:28:16 +0000 Subject: [PATCH] CopySpellAbilityEffect refactored: fixed Precursor Golem (and hopefully all similiar spells), removed calls to isHuman/isComputer PlayerController: added chooseSingleSpellForEffect --- .../forge/card/ability/SpellAbilityAi.java | 5 ++ .../effects/CopySpellAbilityEffect.java | 82 ++++++++----------- .../forge/card/cardfactory/CardFactory.java | 8 +- src/main/java/forge/game/ai/ComputerUtil.java | 2 +- .../java/forge/game/player/HumanPlay.java | 6 +- .../forge/game/player/PlayerController.java | 4 +- .../forge/game/player/PlayerControllerAi.java | 44 ++++++---- .../game/player/PlayerControllerHuman.java | 20 +++-- src/main/java/forge/game/zone/MagicStack.java | 3 +- src/main/java/forge/gui/GuiDisplayUtil.java | 2 +- 10 files changed, 89 insertions(+), 87 deletions(-) diff --git a/src/main/java/forge/card/ability/SpellAbilityAi.java b/src/main/java/forge/card/ability/SpellAbilityAi.java index 7f667e1deca..5db6b4fad30 100644 --- a/src/main/java/forge/card/ability/SpellAbilityAi.java +++ b/src/main/java/forge/card/ability/SpellAbilityAi.java @@ -122,4 +122,9 @@ public abstract class SpellAbilityAi extends SaTargetRountines { System.err.println("Warning: default (ie. inherited from base class) implementation of chooseSinglePlayer is used for " + this.getClass().getName() + ". Consider declaring an overloaded method"); return options.get(0); } + + public SpellAbility chooseSingleSpellAbility(Player player, SpellAbility sa, List spells) { + System.err.println("Warning: default (ie. inherited from base class) implementation of chooseSingleSpellAbility is used for " + this.getClass().getName() + ". Consider declaring an overloaded method"); + return spells.get(0); + } } diff --git a/src/main/java/forge/card/ability/effects/CopySpellAbilityEffect.java b/src/main/java/forge/card/ability/effects/CopySpellAbilityEffect.java index 48d3277f213..e30c24f8f58 100644 --- a/src/main/java/forge/card/ability/effects/CopySpellAbilityEffect.java +++ b/src/main/java/forge/card/ability/effects/CopySpellAbilityEffect.java @@ -4,6 +4,8 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import com.google.common.collect.Iterables; + import forge.Card; import forge.CardLists; import forge.ITargetable; @@ -13,6 +15,7 @@ import forge.card.cardfactory.CardFactory; import forge.card.spellability.SpellAbility; import forge.game.player.Player; import forge.gui.GuiChoose; +import forge.util.Lang; public class CopySpellAbilityEffect extends SpellAbilityEffect { @@ -66,48 +69,23 @@ public class CopySpellAbilityEffect extends SpellAbilityEffect { return; } + boolean mayChoseNewTargets = true; + List copies = new ArrayList(); + if (sa.hasParam("CopyMultipleSpells")) { final int spellCount = Integer.parseInt(sa.getParam("CopyMultipleSpells")); ArrayList chosenSAs = new ArrayList(); - SpellAbility chosenSAtmp = null; - for (int multi = 0; multi < spellCount; multi++) { - if (tgtSpells.size() == 1) { - chosenSAs.addAll(tgtSpells); - } else if (sa.getActivatingPlayer().isHuman()) { - String num = ""; - if (multi == 1 - 1) { - num = "first"; - } - else if (multi == 2 - 1) { - num = "second"; - } - else if (multi == 3 - 1) { - num = "third"; - } else { - num = Integer.toString(multi - 1) + "th"; - } - chosenSAtmp = GuiChoose.one("Select " + num + " spell to copy to stack", tgtSpells); - chosenSAs.add(chosenSAtmp); - tgtSpells.remove(chosenSAtmp); - } else { - chosenSAs.add(tgtSpells.get(multi)); - } - } - for (final SpellAbility chosenSAcopy : chosenSAs) { - chosenSAcopy.setActivatingPlayer(controller); - for (int i = 0; i < amount; i++) { - CardFactory.copySpellontoStack(card, chosenSAcopy.getSourceCard(), chosenSAcopy, true, null); - } + for (int multi = 0; multi < spellCount && !tgtSpells.isEmpty(); multi++) { + String prompt = "Select " + Lang.getOrdinal(multi) + " spell to copy to stack"; + SpellAbility chosen = controller.getController().chooseSingleSpellForEffect(tgtSpells, sa, prompt); + copies.add(CardFactory.copySpellAbilityAndSrcCard(card, chosen.getSourceCard(), chosen, true)); + chosenSAs.add(chosen); + tgtSpells.remove(chosen); } } else if (sa.hasParam("CopyForEachCanTarget")) { - SpellAbility chosenSA = null; - if (tgtSpells.size() == 1 || sa.getActivatingPlayer().isComputer()) { - chosenSA = tgtSpells.get(0); - } else { - chosenSA = GuiChoose.one("Select a spell to copy", tgtSpells); - } + SpellAbility chosenSA = controller.getController().chooseSingleSpellForEffect(tgtSpells, sa, "Select a spell to copy"); chosenSA.setActivatingPlayer(controller); List candidates = chosenSA.getTargetRestrictions().getAllCandidates(chosenSA, true); if (sa.hasParam("CanTargetPlayer")) { @@ -115,8 +93,13 @@ public class CopySpellAbilityEffect extends SpellAbilityEffect { // Remove targeted players because getAllCandidates include all the valid players for(Player p : chosenSA.getTargets().getTargetPlayers()) candidates.remove(p); + + mayChoseNewTargets = false; for (ITargetable o : candidates) { - CardFactory.copySpellontoStack(card, chosenSA.getSourceCard(), chosenSA, true, o); + SpellAbility copy = CardFactory.copySpellAbilityAndSrcCard(card, chosenSA.getSourceCard(), chosenSA, true); + copy.resetTargets(); + copy.getTargets().add(o); + copies.add(copy); } } else {// Precursor Golem, Ink-Treader Nephilim final String type = sa.getParam("CopyForEachCanTarget"); @@ -126,28 +109,29 @@ public class CopySpellAbilityEffect extends SpellAbilityEffect { valid.add((Card) o); } } - valid = CardLists.getValidCards(valid, type, chosenSA.getActivatingPlayer(), - chosenSA.getSourceCard()); + valid = CardLists.getValidCards(valid, type, chosenSA.getActivatingPlayer(), chosenSA.getSourceCard()); + Card originalTarget = Iterables.getFirst(getTargetCards(chosenSA), null); + valid.remove(originalTarget); + mayChoseNewTargets = false; for (Card c : valid) { - CardFactory.copySpellontoStack(card, chosenSA.getSourceCard(), chosenSA, true, c); + SpellAbility copy = CardFactory.copySpellAbilityAndSrcCard(card, chosenSA.getSourceCard(), chosenSA, true); + copy.resetTargets(); + copy.getTargets().add(c); + copies.add(copy); } } } else { - SpellAbility chosenSA = null; - if (tgtSpells.size() == 1) { - chosenSA = tgtSpells.get(0); - } else if (sa.getActivatingPlayer().isHuman()) { - chosenSA = GuiChoose.one("Select a spell to copy", tgtSpells); - } else { - chosenSA = tgtSpells.get(0); - } - + SpellAbility chosenSA = controller.getController().chooseSingleSpellForEffect(tgtSpells, sa, "Select a spell to copy"); chosenSA.setActivatingPlayer(controller); for (int i = 0; i < amount; i++) { - CardFactory.copySpellontoStack(card, chosenSA.getSourceCard(), chosenSA, true, null); + copies.add(CardFactory.copySpellAbilityAndSrcCard(card, chosenSA.getSourceCard(), chosenSA, true)); } } + + for(SpellAbility copySA : copies) { + controller.getController().playSpellAbilityForFree(copySA, mayChoseNewTargets); + } } // end resolve } diff --git a/src/main/java/forge/card/cardfactory/CardFactory.java b/src/main/java/forge/card/cardfactory/CardFactory.java index 7ab7fd6fb3e..d0d7d0f4367 100644 --- a/src/main/java/forge/card/cardfactory/CardFactory.java +++ b/src/main/java/forge/card/cardfactory/CardFactory.java @@ -138,8 +138,7 @@ public class CardFactory { * @param bCopyDetails * a boolean. */ - public final static void copySpellontoStack(final Card source, final Card original, final SpellAbility sa, - final boolean bCopyDetails, final ITargetable definedTarget) { + public final static SpellAbility copySpellAbilityAndSrcCard(final Card source, final Card original, final SpellAbility sa, final boolean bCopyDetails) { //Player originalController = original.getController(); Player controller = sa.getActivatingPlayer(); final Card c = copyCard(original); @@ -204,10 +203,7 @@ public class CardFactory { } copySA.setPaidHash(sa.getPaidHash()); } - - controller.getController().playSpellAbilityForFree(copySA); - - //c.addController(originalController); + return copySA; } /** diff --git a/src/main/java/forge/game/ai/ComputerUtil.java b/src/main/java/forge/game/ai/ComputerUtil.java index a260b619000..b349cef7ebb 100644 --- a/src/main/java/forge/game/ai/ComputerUtil.java +++ b/src/main/java/forge/game/ai/ComputerUtil.java @@ -264,7 +264,7 @@ public class ComputerUtil { * @param sa * a {@link forge.card.spellability.SpellAbility} object. */ - public static final void playStackFree(final Player ai, final SpellAbility sa) { + public static final void playSpellAbilityForFree(final Player ai, final SpellAbility sa) { sa.setActivatingPlayer(ai); final Card source = sa.getSourceCard(); diff --git a/src/main/java/forge/game/player/HumanPlay.java b/src/main/java/forge/game/player/HumanPlay.java index 20897ac1441..7809c442f2b 100644 --- a/src/main/java/forge/game/player/HumanPlay.java +++ b/src/main/java/forge/game/player/HumanPlay.java @@ -184,7 +184,7 @@ public class HumanPlay { if (sa != null) { sa.setActivatingPlayer(player); - playSaWithoutPayingManaCost(player.getGame(), sa); + playSaWithoutPayingManaCost(player.getGame(), sa, true); } } @@ -196,7 +196,7 @@ public class HumanPlay { * @param sa * a {@link forge.card.spellability.SpellAbility} object. */ - public static final void playSaWithoutPayingManaCost(final Game game, final SpellAbility sa) { + public static final void playSaWithoutPayingManaCost(final Game game, final SpellAbility sa, boolean mayChooseNewTargets) { FThreads.assertExecutedByEdt(false); final Card source = sa.getSourceCard(); @@ -209,7 +209,7 @@ public class HumanPlay { final CostPayment payment = new CostPayment(sa.getPayCosts(), sa); final HumanPlaySpellAbility req = new HumanPlaySpellAbility(sa, payment); - req.fillRequirements(false, true, false); + req.fillRequirements(!mayChooseNewTargets, true, false); } else { if (sa.isSpell()) { final Card c = sa.getSourceCard(); diff --git a/src/main/java/forge/game/player/PlayerController.java b/src/main/java/forge/game/player/PlayerController.java index b5487c25dc9..1514b2883fa 100644 --- a/src/main/java/forge/game/player/PlayerController.java +++ b/src/main/java/forge/game/player/PlayerController.java @@ -94,7 +94,7 @@ public abstract class PlayerController { */ //public abstract void playFromSuspend(Card c); public abstract boolean playCascade(Card cascadedCard, Card sourceCard); - public abstract void playSpellAbilityForFree(SpellAbility copySA); + public abstract void playSpellAbilityForFree(SpellAbility copySA, boolean mayChoseNewTargets); public abstract Deck sideboard(final Deck deck, GameType gameType); @@ -112,6 +112,8 @@ public abstract class PlayerController { public Card chooseSingleCardForEffect(List sourceList, SpellAbility sa, String title) { return chooseSingleCardForEffect(sourceList, sa, title, false); } public abstract Card chooseSingleCardForEffect(List sourceList, SpellAbility sa, String title, boolean isOptional); public abstract Player chooseSinglePlayerForEffect(List options, SpellAbility sa, String title); + public abstract SpellAbility chooseSingleSpellForEffect(List spells, SpellAbility sa, String title); + public abstract boolean confirmAction(SpellAbility sa, PlayerActionConfirmMode mode, String message); public abstract boolean getWillPlayOnFirstTurn(boolean isFirstGame); public abstract boolean confirmStaticApplication(Card hostCard, GameEntity affected, String logic, String message); diff --git a/src/main/java/forge/game/player/PlayerControllerAi.java b/src/main/java/forge/game/player/PlayerControllerAi.java index 0e0d561df19..5b341e67682 100644 --- a/src/main/java/forge/game/player/PlayerControllerAi.java +++ b/src/main/java/forge/game/player/PlayerControllerAi.java @@ -94,22 +94,6 @@ public class PlayerControllerAi extends PlayerController { return brains; } - /* (non-Javadoc) - * @see forge.game.player.PlayerController#mayPlaySpellAbilityForFree(forge.card.spellability.SpellAbility) - */ - @Override - public void playSpellAbilityForFree(SpellAbility copySA) { - if (copySA instanceof Spell) { - Spell spell = (Spell) copySA; - if (spell.canPlayFromEffectAI(true, true)) { - ComputerUtil.playStackFree(player, copySA); - } - } else { - copySA.canPlayAI(); - ComputerUtil.playStackFree(player, copySA); - } - } - @Override public Deck sideboard(Deck deck, GameType gameType) { // AI does not know how to sideboard @@ -156,6 +140,15 @@ public class PlayerControllerAi extends PlayerController { return api.getAi().chooseSinglePlayer(player, sa, options); } + @Override + public SpellAbility chooseSingleSpellForEffect(java.util.List spells, SpellAbility sa, String title) { + ApiType api = sa.getApi(); + if ( null == api ) { + throw new InvalidParameterException("SA is not api-based, this is not supported yet"); + } + return api.getAi().chooseSingleSpellAbility(player, sa, spells); + } + @Override public boolean confirmAction(SpellAbility sa, PlayerActionConfirmMode mode, String message) { @@ -235,6 +228,25 @@ public class PlayerControllerAi extends PlayerController { return getAi().chooseCardToDredge(dredgers); } + /* (non-Javadoc) + * @see forge.game.player.PlayerController#mayPlaySpellAbilityForFree(forge.card.spellability.SpellAbility) + */ + @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; + if (!spell.canPlayFromEffectAI(true, true)) { + return; // is this legal at all? + } + } else { + copySA.canPlayAI(); + } + } + ComputerUtil.playSpellAbilityForFree(player, copySA); + } + @Override public void playMiracle(SpellAbility miracle, Card card) { getAi().chooseAndPlaySa(false, false, miracle); diff --git a/src/main/java/forge/game/player/PlayerControllerHuman.java b/src/main/java/forge/game/player/PlayerControllerHuman.java index 1550e1c8341..f19ecb6f380 100644 --- a/src/main/java/forge/game/player/PlayerControllerHuman.java +++ b/src/main/java/forge/game/player/PlayerControllerHuman.java @@ -116,8 +116,8 @@ public class PlayerControllerHuman extends PlayerController { * @see forge.game.player.PlayerController#mayPlaySpellAbilityForFree(forge.card.spellability.SpellAbility) */ @Override - public void playSpellAbilityForFree(SpellAbility copySA) { - HumanPlay.playSaWithoutPayingManaCost(player.getGame(), copySA); + public void playSpellAbilityForFree(SpellAbility copySA, boolean mayChoseNewTargets) { + HumanPlay.playSaWithoutPayingManaCost(player.getGame(), copySA, mayChoseNewTargets); } /* (non-Javadoc) @@ -278,22 +278,24 @@ public class PlayerControllerHuman extends PlayerController { @Override public int chooseNumber(SpellAbility sa, String title, int min, int max) { - final String[] choices = new String[max + 1]; + final Integer[] choices = new Integer[max + 1]; for (int i = min; i <= max; i++) { - choices[i] = Integer.toString(i); + choices[i] = Integer.valueOf(i); } - return Integer.parseInt(GuiChoose.one(title, choices)); + return GuiChoose.one(title, choices).intValue(); } @Override public Player chooseSinglePlayerForEffect(List options, SpellAbility sa, String title) { // Human is supposed to read the message and understand from it what to choose - if (options.size() > 1) - return GuiChoose.one(title, options); - else - return options.get(0); + return options.size() < 2 ? options.get(0) : GuiChoose.one(title, options); } + @Override + public SpellAbility chooseSingleSpellForEffect(java.util.List spells, SpellAbility sa, String title) { + // Human is supposed to read the message and understand from it what to choose + return spells.size() < 2 ? spells.get(0) : GuiChoose.one(title, spells); + } /* (non-Javadoc) * @see forge.game.player.PlayerController#confirmAction(forge.card.spellability.SpellAbility, java.lang.String, java.lang.String) diff --git a/src/main/java/forge/game/zone/MagicStack.java b/src/main/java/forge/game/zone/MagicStack.java index 87c026c706e..9db36e4e4f0 100644 --- a/src/main/java/forge/game/zone/MagicStack.java +++ b/src/main/java/forge/game/zone/MagicStack.java @@ -360,7 +360,8 @@ public class MagicStack /* extends MyObservable */ implements Iterable