From 29494440897a12a1289207a8c027588bd2544b71 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Thu, 5 Dec 2024 01:03:13 +0400 Subject: [PATCH] refactor: removed useless code on ChoiceCreatureType usage, removed possibly infinite code (game freezes) (related to #13106); --- Mage.Sets/src/mage/cards/f/ForTheAncestors.java | 4 +++- Mage.Sets/src/mage/cards/h/HarshMercy.java | 10 +++------- Mage.Sets/src/mage/cards/k/KaronaFalseGod.java | 11 ++++------- .../src/mage/cards/l/LongListOfTheEnts.java | 9 +++++---- .../src/mage/cards/m/MistformWakecaster.java | 6 ++---- .../src/mage/cards/p/PatriarchsBidding.java | 5 ++--- .../src/mage/cards/p/PolJamaarIllusionist.java | 6 ++---- Mage.Sets/src/mage/cards/s/Standardize.java | 17 ++++++----------- Mage.Sets/src/mage/cards/t/TribalUnity.java | 4 +--- .../BecomesChosenCreatureTypeTargetEffect.java | 7 +++---- .../java/mage/choices/ChoiceCreatureType.java | 4 +++- .../game/permanent/token/VolosJournalToken.java | 4 +++- 12 files changed, 37 insertions(+), 50 deletions(-) diff --git a/Mage.Sets/src/mage/cards/f/ForTheAncestors.java b/Mage.Sets/src/mage/cards/f/ForTheAncestors.java index ed859b5f147..afffd910b71 100644 --- a/Mage.Sets/src/mage/cards/f/ForTheAncestors.java +++ b/Mage.Sets/src/mage/cards/f/ForTheAncestors.java @@ -69,7 +69,9 @@ class ForTheAncestorsEffect extends OneShotEffect { return false; } ChoiceCreatureType choice = new ChoiceCreatureType(game, source); - player.choose(outcome, choice, game); + if (!player.choose(outcome, choice, game)) { + return false; + } SubType subType = SubType.byDescription(choice.getChoiceKey()); FilterCard filter; if (subType != null) { diff --git a/Mage.Sets/src/mage/cards/h/HarshMercy.java b/Mage.Sets/src/mage/cards/h/HarshMercy.java index 07c738a9d73..4188bcb9088 100644 --- a/Mage.Sets/src/mage/cards/h/HarshMercy.java +++ b/Mage.Sets/src/mage/cards/h/HarshMercy.java @@ -66,18 +66,14 @@ class HarshMercyEffect extends OneShotEffect { MageObject sourceObject = game.getObject(source); if (controller != null && sourceObject != null) { Set chosenTypes = new HashSet<>(); - PlayerIteration: for (UUID playerId : game.getState().getPlayersInRange(controller.getId(), game)) { Player player = game.getPlayer(playerId); Choice typeChoice = new ChoiceCreatureType(game, source); if (player != null && !player.choose(Outcome.DestroyPermanent, typeChoice, game)) { - continue PlayerIteration; - } - String chosenType = typeChoice.getChoiceKey(); - if (chosenType != null) { - game.informPlayers(sourceObject.getIdName() + ": " + player.getLogName() + " has chosen " + chosenType); - chosenTypes.add(chosenType); + continue; } + game.informPlayers(sourceObject.getIdName() + ": " + player.getLogName() + " has chosen " + typeChoice.getChoiceKey()); + chosenTypes.add(typeChoice.getChoiceKey()); } FilterPermanent filter = new FilterCreaturePermanent("creatures"); diff --git a/Mage.Sets/src/mage/cards/k/KaronaFalseGod.java b/Mage.Sets/src/mage/cards/k/KaronaFalseGod.java index 4f7836e5b17..a7a7f61227b 100644 --- a/Mage.Sets/src/mage/cards/k/KaronaFalseGod.java +++ b/Mage.Sets/src/mage/cards/k/KaronaFalseGod.java @@ -128,13 +128,10 @@ class KaronaFalseGodEffect extends OneShotEffect { if (!controller.choose(Outcome.BoostCreature, typeChoice, game)) { return false; } - String typeChosen = typeChoice.getChoiceKey(); - if (!typeChosen.isEmpty()) { - game.informPlayers(controller.getLogName() + " has chosen " + typeChosen); - FilterCreaturePermanent filter = new FilterCreaturePermanent(); - filter.add(SubType.byDescription(typeChosen).getPredicate()); - game.addEffect(new BoostAllEffect(3, 3, Duration.EndOfTurn, filter, false), source); - } + game.informPlayers(controller.getLogName() + " has chosen " + typeChoice.getChoiceKey()); + FilterCreaturePermanent filter = new FilterCreaturePermanent(); + filter.add(SubType.byDescription(typeChoice.getChoiceKey()).getPredicate()); + game.addEffect(new BoostAllEffect(3, 3, Duration.EndOfTurn, filter, false), source); return true; } return false; diff --git a/Mage.Sets/src/mage/cards/l/LongListOfTheEnts.java b/Mage.Sets/src/mage/cards/l/LongListOfTheEnts.java index 12dd936b4d8..aa9b9f02c9e 100644 --- a/Mage.Sets/src/mage/cards/l/LongListOfTheEnts.java +++ b/Mage.Sets/src/mage/cards/l/LongListOfTheEnts.java @@ -109,7 +109,7 @@ class LongListOfTheEntsEffect extends OneShotEffect { if (player == null) { return false; } - ChoiceCreatureType choice = new ChoiceCreatureType(game, source); + Object existingEntList = game.getState().getValue(LongListOfTheEnts.getKey(game, source, 0)); int offset; Set newEntList; @@ -124,12 +124,13 @@ class LongListOfTheEntsEffect extends OneShotEffect { .stream() .map(SubType::toString) .collect(Collectors.toSet()); + + ChoiceCreatureType choice = new ChoiceCreatureType(game, source); choice.getKeyChoices().keySet().removeIf(chosenTypes::contains); - player.choose(Outcome.BoostCreature, choice, game); - SubType subType = SubType.byDescription(choice.getChoiceKey()); - if (subType == null) { + if (!player.choose(Outcome.BoostCreature, choice, game)) { return false; } + SubType subType = SubType.byDescription(choice.getChoiceKey()); game.informPlayers(player.getLogName() + " notes the creature type " + subType); newEntList.add(subType); game.getState().setValue(LongListOfTheEnts.getKey(game, source, offset), newEntList); diff --git a/Mage.Sets/src/mage/cards/m/MistformWakecaster.java b/Mage.Sets/src/mage/cards/m/MistformWakecaster.java index 3ccc7f900bd..a8fc675fcf1 100644 --- a/Mage.Sets/src/mage/cards/m/MistformWakecaster.java +++ b/Mage.Sets/src/mage/cards/m/MistformWakecaster.java @@ -78,10 +78,8 @@ class BecomesChosenCreatureTypeControlledEffect extends OneShotEffect { String chosenType = ""; if (player != null && card != null) { Choice typeChoice = new ChoiceCreatureType(game, source); - while (!player.choose(Outcome.BoostCreature, typeChoice, game)) { - if (!player.canRespond()) { - return false; - } + if (!player.choose(Outcome.BoostCreature, typeChoice, game)) { + return false; } game.informPlayers(card.getName() + ": " + player.getLogName() + " has chosen " + typeChoice.getChoiceKey()); chosenType = typeChoice.getChoiceKey(); diff --git a/Mage.Sets/src/mage/cards/p/PatriarchsBidding.java b/Mage.Sets/src/mage/cards/p/PatriarchsBidding.java index 6e27d7fa8f0..d29629da719 100644 --- a/Mage.Sets/src/mage/cards/p/PatriarchsBidding.java +++ b/Mage.Sets/src/mage/cards/p/PatriarchsBidding.java @@ -69,9 +69,8 @@ class PatriarchsBiddingEffect extends OneShotEffect { if (!player.choose(Outcome.PutCreatureInPlay, typeChoice, game)) { continue; } - String chosenType = typeChoice.getChoiceKey(); - game.informPlayers(sourceObject.getLogName() + ": " + player.getLogName() + " has chosen " + chosenType); - chosenTypes.add(chosenType); + game.informPlayers(sourceObject.getLogName() + ": " + player.getLogName() + " has chosen " + typeChoice.getChoiceKey()); + chosenTypes.add(typeChoice.getChoiceKey()); } List predicates = new ArrayList<>(); diff --git a/Mage.Sets/src/mage/cards/p/PolJamaarIllusionist.java b/Mage.Sets/src/mage/cards/p/PolJamaarIllusionist.java index 5ea5e16108a..7ab00e4ada3 100644 --- a/Mage.Sets/src/mage/cards/p/PolJamaarIllusionist.java +++ b/Mage.Sets/src/mage/cards/p/PolJamaarIllusionist.java @@ -73,12 +73,10 @@ class PolJamaarIllusionistEffect extends OneShotEffect { return false; } ChoiceCreatureType choice = new ChoiceCreatureType(game, source); - player.choose(outcome, choice, game); - // must use choice.getChoiceKey() so that actual subtype is used - SubType subType = SubType.byDescription(choice.getChoiceKey()); - if (subType == null) { + if (!player.choose(outcome, choice, game)) { return false; } + SubType subType = SubType.byDescription(choice.getChoiceKey()); game.informPlayers(player.getLogName() + " chooses " + subType); int amount = game .getBattlefield() diff --git a/Mage.Sets/src/mage/cards/s/Standardize.java b/Mage.Sets/src/mage/cards/s/Standardize.java index fe413cb179f..53b7a86a21e 100644 --- a/Mage.Sets/src/mage/cards/s/Standardize.java +++ b/Mage.Sets/src/mage/cards/s/Standardize.java @@ -57,7 +57,6 @@ class StandardizeEffect extends OneShotEffect { public boolean apply(Game game, Ability source) { Player player = game.getPlayer(source.getControllerId()); MageObject sourceObject = game.getObject(source); - String chosenType = ""; if (player != null && sourceObject != null) { Choice typeChoice = new ChoiceCreatureType(game, source); typeChoice.setMessage("Choose a creature type other than Wall"); @@ -66,16 +65,12 @@ class StandardizeEffect extends OneShotEffect { return false; } game.informPlayers(sourceObject.getLogName() + ": " + player.getLogName() + " has chosen " + typeChoice.getChoiceKey()); - chosenType = typeChoice.getChoiceKey(); - if (chosenType != null && !chosenType.isEmpty()) { - // ADD TYPE TO TARGET - game.addEffect(new BecomesSubtypeAllEffect( - Duration.EndOfTurn, Arrays.asList(SubType.byDescription(chosenType)), - StaticFilters.FILTER_PERMANENT_CREATURE, true - ), source); - return true; - } - + // ADD TYPE TO TARGET + game.addEffect(new BecomesSubtypeAllEffect( + Duration.EndOfTurn, Arrays.asList(SubType.byDescription(typeChoice.getChoiceKey())), + StaticFilters.FILTER_PERMANENT_CREATURE, true + ), source); + return true; } return false; } diff --git a/Mage.Sets/src/mage/cards/t/TribalUnity.java b/Mage.Sets/src/mage/cards/t/TribalUnity.java index 9e0829c0672..93babcb756a 100644 --- a/Mage.Sets/src/mage/cards/t/TribalUnity.java +++ b/Mage.Sets/src/mage/cards/t/TribalUnity.java @@ -67,9 +67,7 @@ class TribalUnityEffect extends OneShotEffect { Choice typeChoice = new ChoiceCreatureType(game, source); if (player != null && player.choose(outcome, typeChoice, game)) { int boost = amount.calculate(game, source, this); - if (typeChoice.getChoiceKey() != null) { - game.informPlayers(sourceObject.getLogName() + " chosen type: " + typeChoice.getChoiceKey()); - } + game.informPlayers(sourceObject.getLogName() + " chosen type: " + typeChoice.getChoiceKey()); FilterCreaturePermanent filterCreaturePermanent = new FilterCreaturePermanent(); filterCreaturePermanent.add(SubType.byDescription(typeChoice.getChoiceKey()).getPredicate()); game.addEffect(new BoostAllEffect( diff --git a/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesChosenCreatureTypeTargetEffect.java b/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesChosenCreatureTypeTargetEffect.java index 259ecd0d490..474a0b93733 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesChosenCreatureTypeTargetEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesChosenCreatureTypeTargetEffect.java @@ -59,10 +59,9 @@ public class BecomesChosenCreatureTypeTargetEffect extends OneShotEffect { if (nonWall) { typeChoice.getKeyChoices().remove(SubType.WALL.getDescription()); } - while (!player.choose(Outcome.BoostCreature, typeChoice, game)) { - if (!player.canRespond()) { - return false; - } + + if (!player.choose(Outcome.BoostCreature, typeChoice, game)) { + return false; } game.informPlayers(card.getName() + ": " + player.getLogName() + " has chosen " + typeChoice.getChoiceKey()); chosenType = typeChoice.getChoiceKey(); diff --git a/Mage/src/main/java/mage/choices/ChoiceCreatureType.java b/Mage/src/main/java/mage/choices/ChoiceCreatureType.java index 2de289eab6f..733bb37ca78 100644 --- a/Mage/src/main/java/mage/choices/ChoiceCreatureType.java +++ b/Mage/src/main/java/mage/choices/ChoiceCreatureType.java @@ -10,7 +10,9 @@ import java.util.*; import java.util.stream.Collectors; /** - * Game's choose dialog to ask about creature type. Return getChoice + * Game's choose dialog to ask about creature type. + *

+ * Warning, must use getChoiceKey as result */ public class ChoiceCreatureType extends ChoiceImpl { diff --git a/Mage/src/main/java/mage/game/permanent/token/VolosJournalToken.java b/Mage/src/main/java/mage/game/permanent/token/VolosJournalToken.java index 44ea1af0e2b..e263f028106 100644 --- a/Mage/src/main/java/mage/game/permanent/token/VolosJournalToken.java +++ b/Mage/src/main/java/mage/game/permanent/token/VolosJournalToken.java @@ -135,7 +135,9 @@ class VolosJournalTokenEffect extends OneShotEffect { return true; } - player.choose(outcome, choice, game); + if (!player.choose(outcome, choice, game)) { + return false; + } notedTypes.add(choice.getChoiceKey()); return true; }