diff --git a/Mage.Tests/src/test/java/org/mage/test/AI/basic/GameStatePerformanceTest.java b/Mage.Tests/src/test/java/org/mage/test/AI/basic/GameStatePerformanceTest.java new file mode 100644 index 00000000000..f08b05eb85f --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/AI/basic/GameStatePerformanceTest.java @@ -0,0 +1,46 @@ +package org.mage.test.AI.basic; + +import mage.constants.PhaseStep; +import mage.constants.Zone; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBaseWithAIHelps; + +/** + * @author JayDi85 + */ +public class GameStatePerformanceTest extends CardTestPlayerBaseWithAIHelps { + + @Test + public void test_StackObjects_BookmarkMustCleanDataAfterTriggerResolve() { + // memory overflow problem on too much stack objects + // related bug: https://github.com/magefree/mage/issues/9302 + // go to "save(GameState gameState)" and enable size logs to test real usage + final int MAX_STACK_OBJECTS = 300; + + // Whenever an artifact creature you control deals combat damage to a player, + // you may create a 1/1 blue Thopter artifact creature token with flying. + addCard(Zone.BATTLEFIELD, playerA, "Sharding Sphinx", MAX_STACK_OBJECTS); + // + // You can’t lose the game and your opponents can’t win the game. + addCard(Zone.BATTLEFIELD, playerB, "Platinum Angel", 1); + // + // Creatures can’t block this turn. + addCard(Zone.HAND, playerA, "Order // Chaos", 1); // instant, {2}{R} + addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3); + + // prepare not loose + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Chaos"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN); + + // attack and use triggers + attack(1, playerA, "Sharding Sphinx"); + setChoice(playerA, "Whenever an artifact creature you control", MAX_STACK_OBJECTS - 1); // triggers order + setChoice(playerA, true, MAX_STACK_OBJECTS); // triggers use + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPermanentCount(playerA, "Thopter Token", MAX_STACK_OBJECTS); + } +} diff --git a/Mage/src/main/java/mage/game/Game.java b/Mage/src/main/java/mage/game/Game.java index ffe66950d40..414b2f2b7ae 100644 --- a/Mage/src/main/java/mage/game/Game.java +++ b/Mage/src/main/java/mage/game/Game.java @@ -488,12 +488,29 @@ public interface Game extends MageItem, Serializable, Copyable { //game transaction methods void saveState(boolean bookmark); + /** + * Save current game state and return bookmark to it + * + * @return + */ int bookmarkState(); GameState restoreState(int bookmark, String context); + /** + * Remove selected bookmark and all newer bookmarks and game states + * Part of restore/rollback lifecycle + * + * @param bookmark + */ void removeBookmark(int bookmark); + /** + * TODO: remove logic changed, must research each usage of removeBookmark and replace it with new code + * @param bookmark + */ + void removeBookmark_v2(int bookmark); + int getSavedStateSize(); boolean isSaveGame(); diff --git a/Mage/src/main/java/mage/game/GameImpl.java b/Mage/src/main/java/mage/game/GameImpl.java index 2bffe9a0850..b52e611a4ef 100644 --- a/Mage/src/main/java/mage/game/GameImpl.java +++ b/Mage/src/main/java/mage/game/GameImpl.java @@ -118,7 +118,7 @@ public abstract class GameImpl implements Game { protected Map enterWithCounters = new HashMap<>(); protected GameState state; - private transient Stack savedStates = new Stack<>(); + private transient Stack savedStates = new Stack<>(); // bookmarks - 0-base refs to gameStates protected transient GameStates gameStates = new GameStates(); // game states to allow player rollback @@ -934,8 +934,7 @@ public abstract class GameImpl implements Game { } @Override - public void removeBookmark(int bookmark - ) { + public void removeBookmark(int bookmark) { if (!simulation) { if (bookmark != 0) { while (savedStates.size() > bookmark) { @@ -946,6 +945,20 @@ public abstract class GameImpl implements Game { } } + @Override + public void removeBookmark_v2(int bookmark) { + if (!simulation) { + if (bookmark != 0) { + while (savedStates.size() >= bookmark) { + int outdatedIndex = savedStates.pop(); + while (gameStates.getSize() - 1 >= outdatedIndex) { + gameStates.remove(gameStates.getSize() - 1); + } + } + } + } + } + private void clearAllBookmarks() { if (!simulation) { while (!savedStates.isEmpty()) { diff --git a/Mage/src/main/java/mage/game/GameStates.java b/Mage/src/main/java/mage/game/GameStates.java index a803a10aea7..f8ab70c111a 100644 --- a/Mage/src/main/java/mage/game/GameStates.java +++ b/Mage/src/main/java/mage/game/GameStates.java @@ -2,7 +2,7 @@ package mage.game; import java.io.Serializable; -import java.util.LinkedList; +import java.util.ArrayList; import java.util.List; import org.apache.log4j.Logger; @@ -16,13 +16,12 @@ public class GameStates implements Serializable { private final List states; public GameStates() { - this.states = new LinkedList<>(); + this.states = new ArrayList<>(); } public void save(GameState gameState) { -// states.add(new Copier().copyCompressed(gameState)); states.add(gameState.copy()); - logger.trace("Saved game state: " + states.size()); + //logger.warn("states size: " + states.size()); } public int getSize() { @@ -35,7 +34,6 @@ public class GameStates implements Serializable { states.remove(states.size() - 1); } logger.trace("Rolling back state: " + index); -// return new Copier().uncompressCopy(states.get(index)); return states.get(index); } return null; @@ -52,7 +50,6 @@ public class GameStates implements Serializable { public GameState get(int index) { if (index < states.size()) { -// return new Copier().uncompressCopy(states.get(index)); return states.get(index); } return null; diff --git a/Mage/src/main/java/mage/players/Player.java b/Mage/src/main/java/mage/players/Player.java index b9d66036faa..0cf2a27fa2d 100644 --- a/Mage/src/main/java/mage/players/Player.java +++ b/Mage/src/main/java/mage/players/Player.java @@ -38,7 +38,10 @@ import mage.util.Copyable; import mage.util.MultiAmountMessage; import java.io.Serializable; -import java.util.*; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; import java.util.stream.Collectors; /** @@ -56,10 +59,7 @@ public interface Player extends MageItem, Copyable { * Default is PayLifeCostLevel.allAbilities. */ enum PayLifeCostLevel { - allAbilities, - nonSpellnonActivatedAbilities, - onlyManaAbilities, - none + allAbilities, nonSpellnonActivatedAbilities, onlyManaAbilities, none } /** @@ -209,6 +209,7 @@ public interface Player extends MageItem, Copyable { Cards getHand(); void incrementLandsPlayed(); + void resetLandsPlayed(); int getLandsPlayed(); @@ -319,7 +320,7 @@ public interface Player extends MageItem, Copyable { * * @param game * @param playerUnderControlId - * @param info additional info to show in game logs like source + * @param info additional info to show in game logs like source */ void controlPlayersTurn(Game game, UUID playerUnderControlId, String info); @@ -556,8 +557,18 @@ public interface Player extends MageItem, Copyable { int getStoredBookmark(); + /** + * Save player's bookmark for undo, e.g. enable undo button on mana payment + * + * @param bookmark + */ void setStoredBookmark(int bookmark); + /** + * Reset player's bookmark, e.g. disable undo button + * + * @param game + */ void resetStoredBookmark(Game game); default GameState restoreState(int bookmark, String text, Game game) { @@ -746,10 +757,8 @@ public interface Player extends MageItem, Copyable { * @param game Game * @return List of integers with size equal to messages.size(). The sum of the integers is equal to max. */ - default List getMultiAmount(Outcome outcome, List messages, int min, int max, MultiAmountType type, - Game game) { - List constraints = messages.stream().map(s -> new MultiAmountMessage(s, 0, max)) - .collect(Collectors.toList()); + default List getMultiAmount(Outcome outcome, List messages, int min, int max, MultiAmountType type, Game game) { + List constraints = messages.stream().map(s -> new MultiAmountMessage(s, 0, max)).collect(Collectors.toList()); return getMultiAmountWithIndividualConstraints(outcome, constraints, min, max, type, game); } @@ -765,8 +774,7 @@ public interface Player extends MageItem, Copyable { * @param game Game * @return List of integers with size equal to messages.size(). The sum of the integers is equal to max. */ - List getMultiAmountWithIndividualConstraints(Outcome outcome, List messages, int min, - int max, MultiAmountType type, Game game); + List getMultiAmountWithIndividualConstraints(Outcome outcome, List messages, int min, int max, MultiAmountType type, Game game); void sideboard(Match match, Deck deck); @@ -1060,10 +1068,10 @@ public interface Player extends MageItem, Copyable { * without mana (null) or the mana set to manaCosts instead of its normal * mana costs. * - * @param sourceId the source that can be cast without mana - * @param manaCosts alternate ManaCost, null if it can be cast without mana - * cost - * @param costs alternate other costs you need to pay + * @param sourceId the source that can be cast without mana + * @param manaCosts alternate ManaCost, null if it can be cast without mana + * cost + * @param costs alternate other costs you need to pay * @param identifier if not using the MageIdentifier.Default, only apply the alternate mana when ApprovingSource if of that kind. */ void setCastSourceIdWithAlternateMana(UUID sourceId, ManaCosts manaCosts, Costs costs, MageIdentifier identifier); diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index 7c478be0a4a..fec60c350de 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -1613,7 +1613,7 @@ public abstract class PlayerImpl implements Player, Serializable { ability.getId(), ability, ability.getControllerId() )); } - game.removeBookmark(bookmark); + game.removeBookmark_v2(bookmark); return true; } } diff --git a/Mage/src/main/java/mage/util/Copier.java b/Mage/src/main/java/mage/util/Copier.java index b92e0b7c7fc..a757180727a 100644 --- a/Mage/src/main/java/mage/util/Copier.java +++ b/Mage/src/main/java/mage/util/Copier.java @@ -51,39 +51,4 @@ public class Copier { return copy; } - - public byte[] copyCompressed(T obj) { - FastByteArrayOutputStream fbos = null; - ObjectOutputStream out = null; - try { - fbos = new FastByteArrayOutputStream(); - out = new ObjectOutputStream(new GZIPOutputStream(fbos)); - - // Write the object out to a byte array - out.writeObject(obj); - out.flush(); - - byte[] copy = new byte[fbos.getSize()]; - System.arraycopy(fbos.getByteArray(), 0, copy, 0, fbos.getSize()); - return copy; - } - catch(IOException e) { - e.printStackTrace(); - } finally { - StreamUtils.closeQuietly(fbos); - StreamUtils.closeQuietly(out); - } - return null; - } - - public T uncompressCopy(byte[] buffer) { - T copy = null; - try (ObjectInputStream in = new CopierObjectInputStream(loader, new GZIPInputStream(new ByteArrayInputStream(buffer)))) { - copy = (T) in.readObject(); - } - catch(IOException | ClassNotFoundException e) { - e.printStackTrace(); - } - return copy; - } }