From b51fdae550210f0566e354c89bc57d0a9fa5ade6 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Thu, 24 Oct 2024 13:19:05 +0400 Subject: [PATCH] refactor: fixed potential game freeze in some infinite cycles, disconnects and top library's card usage (related to #11285); --- .../mage/server/util/ServerMessagesUtil.java | 30 ++++--------------- .../mage/cards/g/GoldberryRiverDaughter.java | 2 +- Mage.Sets/src/mage/cards/m/MagesContest.java | 2 +- .../mage/cards/n/NicolBolasGodPharaoh.java | 9 +++--- .../src/mage/cards/p/PossibilityStorm.java | 10 +++++-- Mage.Sets/src/mage/cards/p/PrimalSurge.java | 22 +++++++------- .../src/mage/cards/r/ResourcefulDefense.java | 2 +- Mage.Sets/src/mage/cards/r/Ricochet.java | 3 +- Mage.Sets/src/mage/cards/s/StolenGoods.java | 8 +++-- .../src/mage/cards/s/SuntailSquadron.java | 4 ++- Mage.Sets/src/mage/cards/t/Timesifter.java | 3 +- 11 files changed, 45 insertions(+), 50 deletions(-) diff --git a/Mage.Server/src/main/java/mage/server/util/ServerMessagesUtil.java b/Mage.Server/src/main/java/mage/server/util/ServerMessagesUtil.java index a13df85c70d..4e04579d7b1 100644 --- a/Mage.Server/src/main/java/mage/server/util/ServerMessagesUtil.java +++ b/Mage.Server/src/main/java/mage/server/util/ServerMessagesUtil.java @@ -149,45 +149,27 @@ public enum ServerMessagesUtil { } public void incGamesStarted() { - int value; - do { - value = gamesStarted.get(); - } while (!gamesStarted.compareAndSet(value, value + 1)); + gamesStarted.incrementAndGet(); } public void incGamesEnded() { - int value; - do { - value = gamesEnded.get(); - } while (!gamesEnded.compareAndSet(value, value + 1)); + gamesEnded.incrementAndGet(); } public void incTournamentsStarted() { - int value; - do { - value = tournamentsStarted.get(); - } while (!tournamentsStarted.compareAndSet(value, value + 1)); + tournamentsStarted.incrementAndGet(); } public void incTournamentsEnded() { - int value; - do { - value = tournamentsEnded.get(); - } while (!tournamentsEnded.compareAndSet(value, value + 1)); + tournamentsEnded.incrementAndGet(); } public void incReconnects() { - int value; - do { - value = reconnects.get(); - } while (!reconnects.compareAndSet(value, value + 1)); + reconnects.incrementAndGet(); } public void incLostConnection() { - int value; - do { - value = lostConnection.get(); - } while (!lostConnection.compareAndSet(value, value + 1)); + lostConnection.incrementAndGet(); } } diff --git a/Mage.Sets/src/mage/cards/g/GoldberryRiverDaughter.java b/Mage.Sets/src/mage/cards/g/GoldberryRiverDaughter.java index bf82f6bc668..bcc6d2b75df 100644 --- a/Mage.Sets/src/mage/cards/g/GoldberryRiverDaughter.java +++ b/Mage.Sets/src/mage/cards/g/GoldberryRiverDaughter.java @@ -151,7 +151,7 @@ class GoldberryRiverDaughterToEffect extends OneShotEffect { max, MultiAmountType.COUNTERS, game); total = choices.stream().reduce(0, Integer::sum); - } while (total < 1); + } while (total < 1 && controller.canRespond()); // Move the counters. Make sure some counters were actually moved. boolean movedCounters = false; diff --git a/Mage.Sets/src/mage/cards/m/MagesContest.java b/Mage.Sets/src/mage/cards/m/MagesContest.java index 3c7f2b71950..51968f24497 100644 --- a/Mage.Sets/src/mage/cards/m/MagesContest.java +++ b/Mage.Sets/src/mage/cards/m/MagesContest.java @@ -90,7 +90,7 @@ class MagesContestEffect extends OneShotEffect { break; } } - } while (!Objects.equals(currentPlayer, winner)); + } while (you.canRespond() && spellController.canRespond() && !Objects.equals(currentPlayer, winner)); game.informPlayers(winner.getLogName() + " has won the contest with a high bid of " + highBid + " life"); winner.loseLife(highBid, game, source, false); if (winner == you) { diff --git a/Mage.Sets/src/mage/cards/n/NicolBolasGodPharaoh.java b/Mage.Sets/src/mage/cards/n/NicolBolasGodPharaoh.java index 5700f909cc5..08ea929dc27 100644 --- a/Mage.Sets/src/mage/cards/n/NicolBolasGodPharaoh.java +++ b/Mage.Sets/src/mage/cards/n/NicolBolasGodPharaoh.java @@ -162,9 +162,11 @@ class NicolBolasGodPharaohPlusTwoEffect extends OneShotEffect { do { card = library.getFromTop(game); if (card == null) { - continue; + break; + } + if (!opponent.moveCards(card, Zone.EXILED, source, game)) { + break; } - opponent.moveCards(card, Zone.EXILED, source, game); if (card.isLand(game)) { continue; } @@ -172,8 +174,7 @@ class NicolBolasGodPharaohPlusTwoEffect extends OneShotEffect { effect.setTargetPointer(new FixedTarget(card, game)); game.addEffect(effect, source); break; - } while (library.hasCards() - && card != null); + } while (library.hasCards()); return true; } } diff --git a/Mage.Sets/src/mage/cards/p/PossibilityStorm.java b/Mage.Sets/src/mage/cards/p/PossibilityStorm.java index 7a17a85ea6e..78a4ed9e0b4 100644 --- a/Mage.Sets/src/mage/cards/p/PossibilityStorm.java +++ b/Mage.Sets/src/mage/cards/p/PossibilityStorm.java @@ -129,10 +129,14 @@ class PossibilityStormEffect extends OneShotEffect { Card card; do { card = library.getFromTop(game); - if (card != null) { - spellController.moveCardsToExile(card, source, game, true, source.getSourceId(), sourceObject.getIdName()); + if (card == null) { + break; } - } while (library.hasCards() && card != null && !sharesType(card, spell.getCardType(game), game)); + + if (!spellController.moveCardsToExile(card, source, game, true, source.getSourceId(), sourceObject.getIdName())) { + break; + }; + } while (library.hasCards() && !sharesType(card, spell.getCardType(game), game)); if (card != null && sharesType(card, spell.getCardType(game), game) && !card.isLand(game) diff --git a/Mage.Sets/src/mage/cards/p/PrimalSurge.java b/Mage.Sets/src/mage/cards/p/PrimalSurge.java index 4494d061687..ef40b445aeb 100644 --- a/Mage.Sets/src/mage/cards/p/PrimalSurge.java +++ b/Mage.Sets/src/mage/cards/p/PrimalSurge.java @@ -59,19 +59,21 @@ class PrimalSurgeEffect extends OneShotEffect { return false; } - boolean repeat; do { - repeat = false; Card card = controller.getLibrary().getFromTop(game); - if (card != null) { - controller.moveCards(card, Zone.EXILED, source, game); - if (card.isPermanent(game) - && controller.chooseUse(Outcome.PutCardInPlay, "Put " + card.getName() + " onto the battlefield?", source, game)) { - controller.moveCards(card, Zone.BATTLEFIELD, source, game); - repeat = true; - } + if (card == null) { + break; } - } while (controller.canRespond() && repeat); + if (!controller.moveCards(card, Zone.EXILED, source, game)) { + break; + } + if (card.isPermanent(game) + && controller.chooseUse(Outcome.PutCardInPlay, "Put " + card.getName() + " onto the battlefield?", source, game)) { + controller.moveCards(card, Zone.BATTLEFIELD, source, game); + continue; + } + break; + } while (controller.canRespond()); return true; } diff --git a/Mage.Sets/src/mage/cards/r/ResourcefulDefense.java b/Mage.Sets/src/mage/cards/r/ResourcefulDefense.java index b38ff2e65dc..1308d0e693f 100644 --- a/Mage.Sets/src/mage/cards/r/ResourcefulDefense.java +++ b/Mage.Sets/src/mage/cards/r/ResourcefulDefense.java @@ -109,7 +109,7 @@ class ResourcefulDefenseMoveCounterEffect extends OneShotEffect { max, MultiAmountType.COUNTERS, game); total = choices.stream().reduce(0, Integer::sum); - } while (total < 0); + } while (total < 0 && controller.canRespond()); // Move the counters. Make sure some counters were actually moved. for (int i = 0; i < choices.size(); i++) { diff --git a/Mage.Sets/src/mage/cards/r/Ricochet.java b/Mage.Sets/src/mage/cards/r/Ricochet.java index e83cf6498ae..04fb7f77c8e 100644 --- a/Mage.Sets/src/mage/cards/r/Ricochet.java +++ b/Mage.Sets/src/mage/cards/r/Ricochet.java @@ -1,4 +1,3 @@ - package mage.cards.r; import java.util.Collections; @@ -114,6 +113,8 @@ class RicochetEffect extends OneShotEffect { playerRolls.put(player, 7); } } + + // roll until only 1 min result do { for (Player player : playerRolls.keySet()) { playerRolls.put(player, player.rollDice(Outcome.Detriment, source, game, 6)); // bad outcome - ai must choose lowest value diff --git a/Mage.Sets/src/mage/cards/s/StolenGoods.java b/Mage.Sets/src/mage/cards/s/StolenGoods.java index 404f858e5db..bfdd86470ed 100644 --- a/Mage.Sets/src/mage/cards/s/StolenGoods.java +++ b/Mage.Sets/src/mage/cards/s/StolenGoods.java @@ -66,15 +66,17 @@ class StolenGoodsEffect extends OneShotEffect { do { card = opponent.getLibrary().getFromTop(game); if (card == null) { - continue; + break; } if (card.isLand(game)) { - opponent.moveCardsToExile(card, source, game, true, source.getSourceId(), CardUtil.createObjectRealtedWindowTitle(source, game, null)); + if (!opponent.moveCardsToExile(card, source, game, true, source.getSourceId(), CardUtil.createObjectRealtedWindowTitle(source, game, null))) { + break; + } } else { PlayFromNotOwnHandZoneTargetEffect.exileAndPlayFromExile(game, source, card, TargetController.YOU, Duration.EndOfTurn, true, false, true); break; } - } while (card != null && card.isLand(game)); + } while (card.isLand(game)); return true; } diff --git a/Mage.Sets/src/mage/cards/s/SuntailSquadron.java b/Mage.Sets/src/mage/cards/s/SuntailSquadron.java index c40183e4ab5..9b724920309 100644 --- a/Mage.Sets/src/mage/cards/s/SuntailSquadron.java +++ b/Mage.Sets/src/mage/cards/s/SuntailSquadron.java @@ -60,7 +60,9 @@ class SuntailSquadronEffect extends OneShotEffect { } Effect effect = new ConjureCardEffect("Suntail Hawk"); do { - effect.apply(game, source); + if (!effect.apply(game, source)) { + break; + } } while (player.getHand().size() < 7); return true; } diff --git a/Mage.Sets/src/mage/cards/t/Timesifter.java b/Mage.Sets/src/mage/cards/t/Timesifter.java index f1224ddbba8..68c4f799b3d 100644 --- a/Mage.Sets/src/mage/cards/t/Timesifter.java +++ b/Mage.Sets/src/mage/cards/t/Timesifter.java @@ -63,7 +63,7 @@ class TimesifterEffect extends OneShotEffect { List playersExiling = game.getState().getPlayersInRange(source.getControllerId(), game); do { int highestCMC = Integer.MIN_VALUE; - List playersWithHighestCMC = new ArrayList<>(1); + List playersWithHighestCMC = new ArrayList<>(); for (UUID playerId : playersExiling) { Player player = game.getPlayer(playerId); if (player != null) { @@ -84,6 +84,7 @@ class TimesifterEffect extends OneShotEffect { } playersExiling = new ArrayList<>(playersWithHighestCMC); } while (playersExiling.size() > 1); + for (UUID playerId : playersExiling) { Effect effect = new AddExtraTurnTargetEffect(); effect.setTargetPointer(new FixedTarget(playerId));