From c3c8fd7186f629ac5adb02d88ca7bfdcbbc2f27d Mon Sep 17 00:00:00 2001 From: Maxmtg Date: Mon, 23 Dec 2013 09:05:50 +0000 Subject: [PATCH] improve random choice of N cards remove gui calls from choosePile effect --- .../src/main/java/forge/util/Aggregates.java | 33 ++-- .../src/main/java/forge/ai/AiController.java | 42 +++++ .../ability/effects/ChooseCardEffect.java | 21 +-- .../game/ability/effects/PlayEffect.java | 2 +- .../game/ability/effects/TwoPilesEffect.java | 151 +++--------------- .../game/ability/effects/UntapEffect.java | 2 +- .../forge/game/player/PlayerController.java | 5 +- .../forge/game/player/PlayerControllerAi.java | 34 ++-- .../gui/player/PlayerControllerHuman.java | 47 +++++- .../util/PlayerControllerForTests.java | 10 +- 10 files changed, 160 insertions(+), 187 deletions(-) diff --git a/forge-core/src/main/java/forge/util/Aggregates.java b/forge-core/src/main/java/forge/util/Aggregates.java index da8e3fd1fea..a1a507d6069 100644 --- a/forge-core/src/main/java/forge/util/Aggregates.java +++ b/forge-core/src/main/java/forge/util/Aggregates.java @@ -79,8 +79,7 @@ public class Aggregates { if( null == source ) return null; Random rnd = MyRandom.getRandom(); - if ( source instanceof List ) - { + if ( source instanceof List ) { List src = (List)source; int len = src.size(); switch(len) { @@ -88,33 +87,43 @@ public class Aggregates { case 1: return src.get(0); default: return src.get(rnd.nextInt(len)); } - } - int n = 0; T candidate = null; + int lowest = Integer.MAX_VALUE; for (final T item : source) { - if ((rnd.nextDouble() * ++n) < 1) { + int next = rnd.nextInt(); + if(next < lowest) { + lowest = next; candidate = item; } } return candidate; } - // Get several random values - // should improve to make 1 pass over source and track N candidates at once public static final List random(final Iterable source, final int count) { final List result = new ArrayList(); - for (int i = 0; i < count; ++i) { - final T toAdd = Aggregates.random(source); - if (toAdd == null) { - break; + final int[] randoms = new int[count]; + for(int i = 0; i < count; i++) { + randoms[i] = Integer.MAX_VALUE; + result.add(null); + } + + Random rnd = MyRandom.getRandom(); + for(T item : source) { + int next = rnd.nextInt(); + for(int i = 0; i < count; i++) { + if(next < randoms[i]) { + randoms[i] = next; + result.set(i, item); + break; + } } - result.add(toAdd); } return result; } + public static final Iterable uniqueByLast(final Iterable source, final Function fnUniqueKey) { // this might be exotic final Map uniques = new Hashtable(); for (final U c : source) { diff --git a/forge-gui/src/main/java/forge/ai/AiController.java b/forge-gui/src/main/java/forge/ai/AiController.java index f2b444f809e..cfa8326e08b 100644 --- a/forge-gui/src/main/java/forge/ai/AiController.java +++ b/forge-gui/src/main/java/forge/ai/AiController.java @@ -956,5 +956,47 @@ public class AiController { return result; } + public List chooseCardsForEffect(List pool, SpellAbility sa, int min, int max, boolean isOptional) { + if( sa == null || sa.getApi() == null ) { + throw new UnsupportedOperationException(); + } + List result = new ArrayList<>(); + switch( sa.getApi()) { + case TwoPiles: + Card biggest = null; + Card smallest = null; + biggest = pool.get(0); + smallest = pool.get(0); + + for (Card c : pool) { + if (c.getCMC() >= biggest.getCMC()) { + biggest = c; + } else if (c.getCMC() <= smallest.getCMC()) { + smallest = c; + } + } + result.add(biggest); + + if (max >= 3 && !result.contains(smallest)) { + result.add(smallest); + } + + default: + for (int i = 0; i < max; i++) { + Card c = player.getController().chooseSingleCardForEffect(pool, sa, null, isOptional); + if (c != null) { + result.add(c); + pool.remove(c); + } else { + break; + } + } + + + } + return result; + + } + } diff --git a/forge-gui/src/main/java/forge/game/ability/effects/ChooseCardEffect.java b/forge-gui/src/main/java/forge/game/ability/effects/ChooseCardEffect.java index 03afbad6231..385b16272c4 100644 --- a/forge-gui/src/main/java/forge/game/ability/effects/ChooseCardEffect.java +++ b/forge-gui/src/main/java/forge/game/ability/effects/ChooseCardEffect.java @@ -36,7 +36,7 @@ public class ChooseCardEffect extends SpellAbilityEffect { final Card host = sa.getSourceCard(); final Player activator = sa.getActivatingPlayer(); final Game game = activator.getGame(); - final List chosen = new ArrayList(); + List chosen = new ArrayList(); final TargetRestrictions tgt = sa.getTargetRestrictions(); final List tgtPlayers = getTargetPlayers(sa); @@ -79,23 +79,10 @@ public class ChooseCardEffect extends SpellAbilityEffect { } } else if ((tgt == null) || p.canBeTargetedBy(sa)) { if (sa.hasParam("AtRandom")) { - for (int i = 0; i < validAmount; i++) { - Card c = Aggregates.random(choices); - if (c != null) { - chosen.add(c); - choices.remove(c); - } else { - break; - } - } + chosen = Aggregates.random(choices, validAmount); } else { - final List choice = p.getController().chooseCardsForEffect(choices, sa, sa.hasParam("ChoiceTitle") ? - sa.getParam("ChoiceTitle") : "Choose a card ", validAmount, !sa.hasParam("Mandatory")); - for (Card c : choice) { - if (c != null) { - chosen.add(c); - } - } + String title = sa.hasParam("ChoiceTitle") ? sa.getParam("ChoiceTitle") : "Choose a card "; + chosen = p.getController().chooseCardsForEffect(choices, sa, title, validAmount, validAmount, !sa.hasParam("Mandatory")); } } } diff --git a/forge-gui/src/main/java/forge/game/ability/effects/PlayEffect.java b/forge-gui/src/main/java/forge/game/ability/effects/PlayEffect.java index 5601c27aaaf..a43dc69510a 100644 --- a/forge-gui/src/main/java/forge/game/ability/effects/PlayEffect.java +++ b/forge-gui/src/main/java/forge/game/ability/effects/PlayEffect.java @@ -112,7 +112,7 @@ public class PlayEffect extends SpellAbilityEffect { } if (sa.hasParam("ChoiceNum")) { final int choicenum = AbilityUtils.calculateAmount(source, sa.getParam("ChoiceNum"), sa); - tgtCards = activator.getController().chooseCardsForEffect(choice, sa, source + " - Choose up to " + Lang.nounWithNumeral(choicenum, "card"), choicenum, true); + tgtCards = activator.getController().chooseCardsForEffect(choice, sa, source + " - Choose up to " + Lang.nounWithNumeral(choicenum, "card"), 0, choicenum, true); } else { tgtCards = choice; } diff --git a/forge-gui/src/main/java/forge/game/ability/effects/TwoPilesEffect.java b/forge-gui/src/main/java/forge/game/ability/effects/TwoPilesEffect.java index 7c6c50cd239..7d5be02e247 100644 --- a/forge-gui/src/main/java/forge/game/ability/effects/TwoPilesEffect.java +++ b/forge-gui/src/main/java/forge/game/ability/effects/TwoPilesEffect.java @@ -3,7 +3,9 @@ package forge.game.ability.effects; import java.util.ArrayList; import java.util.List; -import forge.ai.ComputerUtilCard; +import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; + import forge.game.ability.AbilityFactory; import forge.game.ability.AbilityUtils; import forge.game.ability.SpellAbilityEffect; @@ -14,8 +16,6 @@ import forge.game.spellability.AbilitySub; import forge.game.spellability.SpellAbility; import forge.game.spellability.TargetRestrictions; import forge.game.zone.ZoneType; -import forge.gui.GuiChoose; -import forge.gui.GuiDialog; public class TwoPilesEffect extends SpellAbilityEffect { @@ -79,8 +79,6 @@ public class TwoPilesEffect extends SpellAbilityEffect { for (final Player p : tgtPlayers) { if ((tgt == null) || p.canBeTargetedBy(sa)) { - final ArrayList pile1 = new ArrayList(); - final ArrayList pile2 = new ArrayList(); List pool = new ArrayList(); if (sa.hasParam("DefinedCards")) { pool = new ArrayList(AbilityUtils.getDefinedCards(sa.getSourceCard(), sa.getParam("DefinedCards"), sa)); @@ -91,157 +89,44 @@ public class TwoPilesEffect extends SpellAbilityEffect { int size = pool.size(); // first, separate the cards into piles - if (separator.isHuman()) { - final List firstPile = GuiChoose.order("Place into two piles", "Pile 1", -1, pool, null, card); - for (final Object o : firstPile) { - pile1.add((Card) o); - } - - for (final Card c : pool) { - if (!pile1.contains(c)) { - pile2.add(c); - } - } - } else if (size > 0) { - //computer separates - Card biggest = null; - Card smallest = null; - biggest = pool.get(0); - smallest = pool.get(0); - - for (Card c : pool) { - if (c.getCMC() >= biggest.getCMC()) { - biggest = c; - } else if (c.getCMC() <= smallest.getCMC()) { - smallest = c; - } - } - pile1.add(biggest); - - if (size > 3 && !pile1.contains(smallest)) { - pile1.add(smallest); - } - for (Card c : pool) { - if (!pile1.contains(c)) { - pile2.add(c); - } - } - } + final List pile1 = separator.getController().chooseCardsForEffect(pool, sa, "Divide cards into two piles", 1, size - 1, false); + final List pile2 = Lists.newArrayList(pool); + Iterables.removeAll(pile2, pile1); System.out.println("Pile 1:" + pile1); System.out.println("Pile 2:" + pile2); card.clearRemembered(); - pile1WasChosen = selectPiles(sa, pile1, pile2, chooser, card, pool); + pile1WasChosen = chooser.getController().chooseCardsPile(sa, pile1, pile2, !sa.hasParam("FaceDown")); + List chosenPile = pile1WasChosen ? pile1 : pile2; + List unchosenPile = !pile1WasChosen ? pile1 : pile2; + + p.getGame().getAction().nofityOfValue(sa, chooser, chooser + " chooses Pile " + (pile1WasChosen ? "1" : "2"), chooser); // take action on the chosen pile if (sa.hasParam("ChosenPile")) { + for (final Card z : chosenPile) { + card.addRemembered(z); + } final SpellAbility action = AbilityFactory.getAbility(card.getSVar(sa.getParam("ChosenPile")), card); action.setActivatingPlayer(sa.getActivatingPlayer()); ((AbilitySub) action).setParent(sa); - AbilityUtils.resolve(action); } - // take action on the chosen pile + // take action on the unchosen pile if (sa.hasParam("UnchosenPile")) { - //switch the remembered cards card.clearRemembered(); - if (pile1WasChosen) { - for (final Card c : pile2) { - card.addRemembered(c); - } - } else { - for (final Card c : pile1) { - card.addRemembered(c); - } + for (final Card z : unchosenPile) { + card.addRemembered(z); } + final SpellAbility action = AbilityFactory.getAbility(card.getSVar(sa.getParam("UnchosenPile")), card); action.setActivatingPlayer(sa.getActivatingPlayer()); ((AbilitySub) action).setParent(sa); - AbilityUtils.resolve(action); } } } } // end twoPiles resolve - - private boolean selectPiles(final SpellAbility sa, ArrayList pile1, ArrayList pile2, - Player chooser, Card card, List pool) { - boolean pile1WasChosen = true; - // then, the chooser picks a pile - - if (sa.hasParam("FaceDown")) { - // Used for Phyrexian Portal, FaceDown Pile choosing - if (chooser.isHuman()) { - final String p1Str = String.format("Pile 1 (%s cards)", pile1.size()); - final String p2Str = String.format("Pile 2 (%s cards)", pile2.size()); - final String[] possibleValues = { p1Str , p2Str }; - - pile1WasChosen = GuiDialog.confirm(card, "Choose a Pile", possibleValues); - } - else { - // AI will choose the first pile if it is larger or the same - // TODO Improve this to be slightly more random to not be so predictable - pile1WasChosen = pile1.size() >= pile2.size(); - } - } else { - if (chooser.isHuman()) { - final Card[] disp = new Card[pile1.size() + pile2.size() + 2]; - disp[0] = new Card(-1); - disp[0].setName("Pile 1"); - for (int i = 0; i < pile1.size(); i++) { - disp[1 + i] = pile1.get(i); - } - disp[pile1.size() + 1] = new Card(-2); - disp[pile1.size() + 1].setName("Pile 2"); - for (int i = 0; i < pile2.size(); i++) { - disp[pile1.size() + i + 2] = pile2.get(i); - } - - // make sure Pile 1 or Pile 2 is clicked on - while (true) { - final Object o = GuiChoose.one("Choose a pile", disp); - final Card c = (Card) o; - String name = c.getName(); - - if (!(name.equals("Pile 1") || name.equals("Pile 2"))) { - continue; - } - - pile1WasChosen = name.equals("Pile 1"); - break; - } - } else { - int cmc1 = ComputerUtilCard.evaluatePermanentList(new ArrayList(pile1)); - int cmc2 = ComputerUtilCard.evaluatePermanentList(new ArrayList(pile2)); - if (CardLists.getNotType(pool, "Creature").isEmpty()) { - cmc1 = ComputerUtilCard.evaluateCreatureList(new ArrayList(pile1)); - cmc2 = ComputerUtilCard.evaluateCreatureList(new ArrayList(pile2)); - System.out.println("value:" + cmc1 + " " + cmc2); - } - - // for now, this assumes that the outcome will be bad - // TODO: This should really have a ChooseLogic param to - // figure this out - pile1WasChosen = cmc1 >= cmc2; - if ("Worst".equals(sa.getParam("AILogic"))) { - pile1WasChosen = !pile1WasChosen; - } - GuiDialog.message("Computer chooses the Pile " + (pile1WasChosen ? "1" : "2")); - } - } - - if (pile1WasChosen) { - for (final Card z : pile1) { - card.addRemembered(z); - } - } else { - for (final Card z : pile2) { - card.addRemembered(z); - } - } - - return pile1WasChosen; - } } diff --git a/forge-gui/src/main/java/forge/game/ability/effects/UntapEffect.java b/forge-gui/src/main/java/forge/game/ability/effects/UntapEffect.java index 57ba62faa43..334401fcd79 100644 --- a/forge-gui/src/main/java/forge/game/ability/effects/UntapEffect.java +++ b/forge-gui/src/main/java/forge/game/ability/effects/UntapEffect.java @@ -78,7 +78,7 @@ public class UntapEffect extends SpellAbilityEffect { List list = CardLists.getType(p.getCardsIn(ZoneType.Battlefield), valid); list = CardLists.filter(list, Presets.TAPPED); - List selected = p.getController().chooseCardsForEffect(list, sa, "Select cards to untap", num, true); + List selected = p.getController().chooseCardsForEffect(list, sa, "Select cards to untap", 0, num, true); if( selected != null ) for( Card c : selected ) c.untap(); diff --git a/forge-gui/src/main/java/forge/game/player/PlayerController.java b/forge-gui/src/main/java/forge/game/player/PlayerController.java index 556bc1f2a90..7b5f0f17865 100644 --- a/forge-gui/src/main/java/forge/game/player/PlayerController.java +++ b/forge-gui/src/main/java/forge/game/player/PlayerController.java @@ -126,7 +126,8 @@ public abstract class PlayerController { // Specify a target of a spell (Spellskite) public abstract Pair chooseTarget(SpellAbility sa, List> allTargets); - public abstract List chooseCardsForEffect(List sourceList, SpellAbility sa, String title, int amount, boolean isOptional); + // 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 List chooseCardsForEffect(List sourceList, SpellAbility sa, String title, int min, int max, boolean isOptional); public final Card chooseSingleCardForEffect(Collection sourceList, SpellAbility sa, String title) { return chooseSingleCardForEffect(sourceList, sa, title, false, null); } public final Card chooseSingleCardForEffect(Collection sourceList, SpellAbility sa, String title, boolean isOptional) { return chooseSingleCardForEffect(sourceList, sa, title, isOptional, null); } public abstract Card chooseSingleCardForEffect(Collection sourceList, SpellAbility sa, String title, boolean isOptional, Player relatedPlayer); @@ -200,6 +201,6 @@ public abstract class PlayerController { public abstract void playTrigger(Card host, WrappedAbility wrapperAbility, boolean isMandatory); public abstract boolean playSaFromPlayEffect(SpellAbility tgtSA); - public abstract Map chooseProliferation(); + public abstract boolean chooseCardsPile(SpellAbility sa, List pile1,List pile2, boolean faceUp); } diff --git a/forge-gui/src/main/java/forge/game/player/PlayerControllerAi.java b/forge-gui/src/main/java/forge/game/player/PlayerControllerAi.java index 58e281e3dc8..e945b35d72f 100644 --- a/forge-gui/src/main/java/forge/game/player/PlayerControllerAi.java +++ b/forge-gui/src/main/java/forge/game/player/PlayerControllerAi.java @@ -127,19 +127,8 @@ public class PlayerControllerAi extends PlayerController { } @Override - public List chooseCardsForEffect(List sourceList, SpellAbility sa, String title, int amount, - boolean isOptional) { - List chosen = new ArrayList(); - for (int i = 0; i < amount; i++) { - Card c = this.chooseSingleCardForEffect(sourceList, sa, title, isOptional); - if (c != null) { - chosen.add(c); - sourceList.remove(c); - } else { - break; - } - } - return chosen; + public List chooseCardsForEffect(List sourceList, SpellAbility sa, String title, int min, int max, boolean isOptional) { + return brains.chooseCardsForEffect(sourceList, sa, min, max, isOptional); } @Override @@ -672,4 +661,23 @@ public class PlayerControllerAi extends PlayerController { return currentAbility.doTrigger(true, player); } + @Override + public boolean chooseCardsPile(SpellAbility sa, List pile1, List pile2, boolean faceUp) { + if (!faceUp) { + // AI will choose the first pile if it is larger or the same + // TODO Improve this to be slightly more random to not be so predictable + return pile1.size() >= pile2.size(); + } else { + boolean allCreatures = Iterables.all(Iterables.concat(pile1, pile2), CardPredicates.Presets.CREATURES); + int cmc1 = allCreatures ? ComputerUtilCard.evaluateCreatureList(pile1) : ComputerUtilCard.evaluatePermanentList(pile1); + int cmc2 = allCreatures ? ComputerUtilCard.evaluateCreatureList(pile2) : ComputerUtilCard.evaluatePermanentList(pile2); + System.out.println("value:" + cmc1 + " " + cmc2); + + // for now, this assumes that the outcome will be bad + // TODO: This should really have a ChooseLogic param to + // figure this out + return "Worst".equals(sa.getParam("AILogic")) ^ (cmc1 >= cmc2); + } + } + } diff --git a/forge-gui/src/main/java/forge/gui/player/PlayerControllerHuman.java b/forge-gui/src/main/java/forge/gui/player/PlayerControllerHuman.java index 42153f86816..7edc8e19d21 100644 --- a/forge-gui/src/main/java/forge/gui/player/PlayerControllerHuman.java +++ b/forge-gui/src/main/java/forge/gui/player/PlayerControllerHuman.java @@ -330,12 +330,12 @@ public class PlayerControllerHuman extends PlayerController { * @see forge.game.player.PlayerController#chooseCardsForEffect(java.util.Collection, forge.card.spellability.SpellAbility, java.lang.String, int, boolean) */ @Override - public List chooseCardsForEffect(List sourceList, SpellAbility sa, String title, int amount, - boolean isOptional) { + public List chooseCardsForEffect(List sourceList, SpellAbility sa, String title, int min, int max, boolean isOptional) { // If only one card to choose, use a dialog box. // Otherwise, use the order dialog to be able to grab multiple cards in one shot - if (amount == 1) { - return Lists.newArrayList(chooseSingleCardForEffect(sourceList, sa, title, isOptional)); + if (max == 1) { + Card singleChosen = chooseSingleCardForEffect(sourceList, sa, title, isOptional); + return singleChosen == null ? Lists.newArrayList() : Lists.newArrayList(singleChosen); } GuiUtils.setPanelSelection(sa.getSourceCard()); @@ -350,14 +350,14 @@ public class PlayerControllerHuman extends PlayerController { } if(cardsAreInMyHandOrBattlefield) { - InputSelectCards sc = new InputSelectCardsFromList(isOptional ? 0 : amount, amount, sourceList); + InputSelectCards sc = new InputSelectCardsFromList(min, max, sourceList); sc.setMessage(title); sc.setCancelAllowed(isOptional); sc.showAndWait(); return sc.hasCancelled() ? Lists.newArrayList() : sc.getSelected(); } - int remaining = isOptional ? -1 : Math.max(sourceList.size() - amount, 0); + int remaining = isOptional ? -1 : Math.max(sourceList.size() - max, 0); return GuiChoose.order(title, "Chosen Cards", remaining, sourceList, null, sa.getSourceCard()); } @@ -1025,4 +1025,39 @@ public class PlayerControllerHuman extends PlayerController { return select.chooseTargets(null); } + + @Override + public boolean chooseCardsPile(SpellAbility sa, List pile1, List pile2, boolean faceUp) { + if (!faceUp) { + final String p1Str = String.format("Pile 1 (%s cards)", pile1.size()); + final String p2Str = String.format("Pile 2 (%s cards)", pile2.size()); + final String[] possibleValues = { p1Str , p2Str }; + return GuiDialog.confirm(sa.getSourceCard(), "Choose a Pile", possibleValues); + } else { + final Card[] disp = new Card[pile1.size() + pile2.size() + 2]; + disp[0] = new Card(-1); + disp[0].setName("Pile 1"); + for (int i = 0; i < pile1.size(); i++) { + disp[1 + i] = pile1.get(i); + } + disp[pile1.size() + 1] = new Card(-2); + disp[pile1.size() + 1].setName("Pile 2"); + for (int i = 0; i < pile2.size(); i++) { + disp[pile1.size() + i + 2] = pile2.get(i); + } + + // make sure Pile 1 or Pile 2 is clicked on + while (true) { + final Object o = GuiChoose.one("Choose a pile", disp); + final Card c = (Card) o; + String name = c.getName(); + + if (!(name.equals("Pile 1") || name.equals("Pile 2"))) { + continue; + } + + return name.equals("Pile 1"); + } + } + } } diff --git a/forge-gui/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java b/forge-gui/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java index b0b5081832d..e5b9a040c31 100644 --- a/forge-gui/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java +++ b/forge-gui/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java @@ -57,6 +57,7 @@ import forge.gamesimulationtests.util.playeractions.DeclareAttackersAction; import forge.gamesimulationtests.util.playeractions.DeclareBlockersAction; import forge.gamesimulationtests.util.playeractions.PlayerActions; import forge.item.PaperCard; +import forge.util.MyRandom; /** * Default harmless implementation for tests. @@ -156,8 +157,8 @@ public class PlayerControllerForTests extends PlayerController { } @Override - public List chooseCardsForEffect(List sourceList, SpellAbility sa, String title, int amount, boolean isOptional) { - return chooseItems(sourceList, amount); + public List chooseCardsForEffect(List sourceList, SpellAbility sa, String title, int min, int max, boolean isOptional) { + return chooseItems(sourceList, max); } @Override @@ -537,4 +538,9 @@ public class PlayerControllerForTests extends PlayerController { public boolean chooseTargetsFor(SpellAbility currentAbility) { return currentAbility.doTrigger(true, player); } + + @Override + public boolean chooseCardsPile(SpellAbility sa, List pile1, List pile2, boolean faceUp) { + return MyRandom.getRandom().nextBoolean(); + } }