From 795422ba6bb86eed54ef09768f779f0206e59d08 Mon Sep 17 00:00:00 2001 From: Evan Kranzler Date: Sun, 10 Oct 2021 11:09:24 -0400 Subject: [PATCH] fixed a few more instances of getPlayers (#8348) --- Mage.Sets/src/mage/cards/g/GamePreserve.java | 61 +++++++-------- Mage.Sets/src/mage/cards/g/GoblinGame.java | 77 ++++++++++--------- .../src/mage/cards/g/GrimoireOfTheDead.java | 66 ++++++++-------- 3 files changed, 102 insertions(+), 102 deletions(-) diff --git a/Mage.Sets/src/mage/cards/g/GamePreserve.java b/Mage.Sets/src/mage/cards/g/GamePreserve.java index cd4a840eaff..1ebc3cc2147 100644 --- a/Mage.Sets/src/mage/cards/g/GamePreserve.java +++ b/Mage.Sets/src/mage/cards/g/GamePreserve.java @@ -1,11 +1,8 @@ - package mage.cards.g; -import java.util.UUID; import mage.abilities.Ability; import mage.abilities.common.BeginningOfUpkeepTriggeredAbility; import mage.abilities.effects.OneShotEffect; -import mage.cards.Card; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.cards.Cards; @@ -14,11 +11,13 @@ import mage.constants.CardType; import mage.constants.Outcome; import mage.constants.TargetController; import mage.constants.Zone; +import mage.filter.StaticFilters; import mage.game.Game; import mage.players.Player; +import java.util.UUID; + /** - * * @author TheElk801 */ public final class GamePreserve extends CardImpl { @@ -27,7 +26,9 @@ public final class GamePreserve extends CardImpl { super(ownerId, setInfo, new CardType[]{CardType.ENCHANTMENT}, "{2}{G}"); // At the beginning of your upkeep, each player reveals the top card of their library. If all cards revealed this way are creature cards, put those cards onto the battlefield under their owners' control. - this.addAbility(new BeginningOfUpkeepTriggeredAbility(new DuskmarEffect(), TargetController.YOU, false)); + this.addAbility(new BeginningOfUpkeepTriggeredAbility( + new GamePreserveEffect(), TargetController.YOU, false + )); } private GamePreserve(final GamePreserve card) { @@ -40,47 +41,43 @@ public final class GamePreserve extends CardImpl { } } -class DuskmarEffect extends OneShotEffect { +class GamePreserveEffect extends OneShotEffect { - public DuskmarEffect() { + public GamePreserveEffect() { super(Outcome.Detriment); this.staticText = "each player reveals the top card of their library. If all cards revealed this way are creature cards, put those cards onto the battlefield under their owners' control"; } - public DuskmarEffect(final DuskmarEffect effect) { + public GamePreserveEffect(final GamePreserveEffect effect) { super(effect); } @Override - public DuskmarEffect copy() { - return new DuskmarEffect(this); + public GamePreserveEffect copy() { + return new GamePreserveEffect(this); } @Override public boolean apply(Game game, Ability source) { Player controller = game.getPlayer(source.getControllerId()); - if (controller != null) { - boolean putToPlay = true; - Cards cards = new CardsImpl(); - for (Player player : game.getPlayers().values()) { - if (player.getLibrary().hasCards()) { - Card card = player.getLibrary().removeFromTop(game); - if (card != null) { - cards.add(card); - if (!card.isCreature(game)) { - putToPlay = false; - } - player.revealCards(source, "- Revealed by " + player.getName(), cards, game); - } - } else { - putToPlay = false; - } - } - if (putToPlay) { - controller.moveCards(cards.getCards(game), Zone.BATTLEFIELD, source, game, false, false, true, null); - } - return true; + if (controller == null) { + return false; } - return false; + Cards cards = new CardsImpl(); + for (UUID playerId : game.getState().getPlayersInRange(source.getControllerId(), game)) { + Player player = game.getPlayer(playerId); + if (player != null) { + cards.add(player.getLibrary().getFromTop(game)); + } + } + controller.revealCards(source, cards, game); + if (cards.isEmpty() || cards.count(StaticFilters.FILTER_CARD_NON_CREATURE, game) > 0) { + return false; + } + controller.moveCards( + cards.getCards(game), Zone.BATTLEFIELD, source, game, + false, false, true, null + ); + return true; } } diff --git a/Mage.Sets/src/mage/cards/g/GoblinGame.java b/Mage.Sets/src/mage/cards/g/GoblinGame.java index 432a2d9c6ce..5956c1ccfbc 100644 --- a/Mage.Sets/src/mage/cards/g/GoblinGame.java +++ b/Mage.Sets/src/mage/cards/g/GoblinGame.java @@ -1,9 +1,5 @@ - package mage.cards.g; -import java.util.HashMap; -import java.util.Map; -import java.util.UUID; import mage.abilities.Ability; import mage.abilities.effects.OneShotEffect; import mage.cards.CardImpl; @@ -13,8 +9,10 @@ import mage.constants.Outcome; import mage.game.Game; import mage.players.Player; +import java.util.*; +import java.util.stream.Collectors; + /** - * * @author L_J */ public final class GoblinGame extends CardImpl { @@ -25,7 +23,6 @@ public final class GoblinGame extends CardImpl { // Each player hides at least one item, then all players reveal them simultaneously. Each player loses life equal to the number of items they revealed. The player who revealed the fewest items then loses half their life, rounded up. If two or more players are tied for fewest, each loses half their life, rounded up. // Reinterpreted as: Each player secretly chooses a number greater than 0. Then those numbers are revealed. Each player loses life equal to their chosen number. The player who revealed the lowest number then loses half their life, rounded up. If two or more players are tied for lowest, each loses half their life, rounded up. this.getSpellAbility().addEffect(new GoblinGameEffect()); - } private GoblinGame(final GoblinGame card) { @@ -42,7 +39,10 @@ class GoblinGameEffect extends OneShotEffect { public GoblinGameEffect() { super(Outcome.Detriment); - this.staticText = "Each player hides at least one item, then all players reveal them simultaneously. Each player loses life equal to the number of items they revealed. The player who revealed the fewest items then loses half their life, rounded up. If two or more players are tied for fewest, each loses half their life, rounded up."; + this.staticText = "Each player hides at least one item, then all players reveal them simultaneously. " + + "Each player loses life equal to the number of items they revealed. " + + "The player who revealed the fewest items then loses half their life, rounded up. " + + "If two or more players are tied for fewest, each loses half their life, rounded up."; } public GoblinGameEffect(final GoblinGameEffect effect) { @@ -56,41 +56,42 @@ class GoblinGameEffect extends OneShotEffect { @Override public boolean apply(Game game, Ability source) { - int lowestNumber = 0; - int number = 0; - String message = "Choose a number of objects to hide."; - Map numberChosen = new HashMap<>(); + Map numberChosen = new HashMap<>(); - //players choose numbers - for (Player player : game.getPlayers().values()) { - if (player != null) { - // TODO: consider changing 1000 to another cap, or even Integer.MAX_VALUE if the Volcano Hellion binary wraparound gets addressed (although hiding over two billions of items would be rather difficult IRL) - number = player.getAmount(1, 1000, message, game); - numberChosen.put(player, number); - } + // players choose numbers + List players = game + .getState() + .getPlayersInRange(source.getControllerId(), game) + .stream() + .map(game::getPlayer) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + for (Player player : players) { + // TODO: consider changing 1000 to another cap, or even Integer.MAX_VALUE if the Volcano Hellion binary wraparound gets addressed (although hiding over two billions of items would be rather difficult IRL) + numberChosen.put(player.getId(), player.getAmount(1, 1000, "Choose a number of objects to hide.", game)); } - //get lowest number - for (Player player : numberChosen.keySet()) { - if (lowestNumber == 0 || lowestNumber > numberChosen.get(player)) { - lowestNumber = numberChosen.get(player); - } + + // get lowest number + int lowestNumber = numberChosen + .values() + .stream() + .mapToInt(x -> x) + .min() + .orElse(0); + + // reveal numbers to players and follow through with effects + for (Player player : players) { + game.informPlayers(player.getLogName() + " chose " + numberChosen.get(player.getId())); + player.loseLife(numberChosen.get(player.getId()), game, source, false); } - //reveal numbers to players and follow through with effects - for (Player player : game.getPlayers().values()) { - if (player != null) { - game.informPlayers(player.getLogName() + " chose number " + numberChosen.get(player)); - player.loseLife(numberChosen.get(player), game, source, false); + for (Player player : players) { + if (numberChosen.get(player.getId()) > lowestNumber) { + continue; } - } - for (Player player : game.getPlayers().values()) { - if (player != null) { - if (numberChosen.get(player) <= lowestNumber) { - game.informPlayers(player.getLogName() + " chose the lowest number"); - Integer amount = (int) Math.ceil(player.getLife() / 2f); - if (amount > 0) { - player.loseLife(amount, game, source, false); - } - } + game.informPlayers(player.getLogName() + " chose the lowest number"); + Integer amount = (int) Math.ceil(player.getLife() / 2f); + if (amount > 0) { + player.loseLife(amount, game, source, false); } } return true; diff --git a/Mage.Sets/src/mage/cards/g/GrimoireOfTheDead.java b/Mage.Sets/src/mage/cards/g/GrimoireOfTheDead.java index b9a809e0e37..9ef267f950d 100644 --- a/Mage.Sets/src/mage/cards/g/GrimoireOfTheDead.java +++ b/Mage.Sets/src/mage/cards/g/GrimoireOfTheDead.java @@ -1,13 +1,13 @@ - package mage.cards.g; import mage.abilities.Ability; import mage.abilities.common.SimpleActivatedAbility; +import mage.abilities.costs.CompositeCost; import mage.abilities.costs.common.DiscardTargetCost; import mage.abilities.costs.common.RemoveCountersSourceCost; import mage.abilities.costs.common.SacrificeSourceCost; import mage.abilities.costs.common.TapSourceCost; -import mage.abilities.costs.mana.ManaCostsImpl; +import mage.abilities.costs.mana.GenericManaCost; import mage.abilities.effects.ContinuousEffectImpl; import mage.abilities.effects.OneShotEffect; import mage.abilities.effects.common.counter.AddCountersSourceEffect; @@ -16,10 +16,12 @@ import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.*; import mage.counters.CounterType; +import mage.filter.StaticFilters; import mage.game.Game; import mage.game.permanent.Permanent; import mage.players.Player; import mage.target.common.TargetCardInHand; +import mage.target.targetpointer.FixedTargets; import java.util.LinkedHashSet; import java.util.Set; @@ -35,17 +37,20 @@ public final class GrimoireOfTheDead extends CardImpl { addSuperType(SuperType.LEGENDARY); // {1}, {tap}, Discard a card: Put a study counter on Grimoire of the Dead. - Ability ability1 = new SimpleActivatedAbility(Zone.BATTLEFIELD, new AddCountersSourceEffect(CounterType.STUDY.createInstance()), new ManaCostsImpl("{1}")); + Ability ability1 = new SimpleActivatedAbility( + new AddCountersSourceEffect(CounterType.STUDY.createInstance()), new GenericManaCost(1) + ); ability1.addCost(new TapSourceCost()); ability1.addCost(new DiscardTargetCost(new TargetCardInHand())); this.addAbility(ability1); // {tap}, Remove three study counters from Grimoire of the Dead and sacrifice it: Put all creature cards from all graveyards onto the battlefield under your control. They're black Zombies in addition to their other colors and types. - Ability ability2 = new SimpleActivatedAbility(Zone.BATTLEFIELD, new GrimoireOfTheDeadEffect(), new TapSourceCost()); - ability2.addCost(new RemoveCountersSourceCost(CounterType.STUDY.createInstance(3))); - ability2.addCost(new SacrificeSourceCost()); + Ability ability2 = new SimpleActivatedAbility(new GrimoireOfTheDeadEffect(), new TapSourceCost()); + ability2.addCost(new CompositeCost( + new RemoveCountersSourceCost(CounterType.STUDY.createInstance(3)), + new SacrificeSourceCost(), "Remove three study counters from {this} and sacrifice it" + )); this.addAbility(ability2); - } private GrimoireOfTheDead(final GrimoireOfTheDead card) { @@ -62,7 +67,8 @@ class GrimoireOfTheDeadEffect extends OneShotEffect { public GrimoireOfTheDeadEffect() { super(Outcome.PutCreatureInPlay); - staticText = "Put all creature cards in all graveyards onto the battlefield under your control. They're black Zombies in addition to their other colors and types"; + staticText = "Put all creature cards in all graveyards onto the battlefield under your control. " + + "They're black Zombies in addition to their other colors and types"; } public GrimoireOfTheDeadEffect(final GrimoireOfTheDeadEffect effect) { @@ -72,20 +78,20 @@ class GrimoireOfTheDeadEffect extends OneShotEffect { @Override public boolean apply(Game game, Ability source) { Player controller = game.getPlayer(source.getControllerId()); - if (controller != null) { - Set creatureCards = new LinkedHashSet<>(); - for (Player player : game.getPlayers().values()) { - for (Card card : player.getGraveyard().getCards(game)) { - if (card.isCreature(game)) { - creatureCards.add(card); - game.addEffect(new GrimoireOfTheDeadEffect2(card.getId()), source); - } - } - } - controller.moveCards(creatureCards, Zone.BATTLEFIELD, source, game, false, false, false, null); - return true; + if (controller == null) { + return false; } - return false; + Set creatureCards = new LinkedHashSet<>(); + for (UUID playerId : game.getState().getPlayersInRange(source.getControllerId(), game)) { + Player player = game.getPlayer(playerId); + if (player == null) { + continue; + } + creatureCards.addAll(player.getGraveyard().getCards(StaticFilters.FILTER_CARD_CREATURE, game)); + } + controller.moveCards(creatureCards, Zone.BATTLEFIELD, source, game); + game.addEffect(new GrimoireOfTheDeadEffect2().setTargetPointer(new FixedTargets(creatureCards, game)), source); + return true; } @Override @@ -97,17 +103,12 @@ class GrimoireOfTheDeadEffect extends OneShotEffect { class GrimoireOfTheDeadEffect2 extends ContinuousEffectImpl { - private final UUID targetId; - - public GrimoireOfTheDeadEffect2(UUID targetId) { + public GrimoireOfTheDeadEffect2() { super(Duration.Custom, Outcome.Neutral); - this.targetId = targetId; - staticText = "Becomes a black Zombie in addition to its other colors and types"; } public GrimoireOfTheDeadEffect2(final GrimoireOfTheDeadEffect2 effect) { super(effect); - this.targetId = effect.targetId; } @Override @@ -117,8 +118,11 @@ class GrimoireOfTheDeadEffect2 extends ContinuousEffectImpl { @Override public boolean apply(Layer layer, SubLayer sublayer, Ability source, Game game) { - Permanent permanent = game.getPermanent(targetId); - if (permanent != null) { + for (UUID permanentId : getTargetPointer().getTargets(game, source)) { + Permanent permanent = game.getPermanent(permanentId); + if (permanent == null) { + continue; + } switch (layer) { case ColorChangingEffects_5: permanent.getColor(game).setBlack(true); @@ -127,9 +131,8 @@ class GrimoireOfTheDeadEffect2 extends ContinuousEffectImpl { permanent.addSubType(game, SubType.ZOMBIE); break; } - return true; } - return false; + return true; } @Override @@ -141,5 +144,4 @@ class GrimoireOfTheDeadEffect2 extends ContinuousEffectImpl { public boolean hasLayer(Layer layer) { return layer == Layer.ColorChangingEffects_5 || layer == Layer.TypeChangingEffects_4; } - }