diff --git a/game-app/game-core/src/main/java/games/strategy/engine/data/GameData.java b/game-app/game-core/src/main/java/games/strategy/engine/data/GameData.java index 67bde7143c7..c4c76191596 100644 --- a/game-app/game-core/src/main/java/games/strategy/engine/data/GameData.java +++ b/game-app/game-core/src/main/java/games/strategy/engine/data/GameData.java @@ -588,12 +588,14 @@ public void setSaveGameFileName(final String saveGameFileName) { * header. Returns empty if the 'map.yml' or game notes file cannot be found. */ public String loadGameNotes(final Path mapLocation) { - // Given a game name, the map.yml file can tell us the path to the game xml file. // From the game-xml file name, we can find the game-notes file. + return getGameXmlPath(mapLocation).map(GameNotes::loadGameNotes).orElse(""); + } + + public Optional getGameXmlPath(final Path mapLocation) { + // Given a game name, the map.yml file can tell us the path to the game xml file. return findMapDescriptionYaml(mapLocation) - .flatMap(yaml -> yaml.getGameXmlPathByGameName(getGameName())) - .map(GameNotes::loadGameNotes) - .orElse(""); + .flatMap(yaml -> yaml.getGameXmlPathByGameName(getGameName())); } private Optional findMapDescriptionYaml(final Path mapLocation) { diff --git a/game-app/game-core/src/main/java/games/strategy/engine/data/export/GameDataExporter.java b/game-app/game-core/src/main/java/games/strategy/engine/data/export/GameDataExporter.java index fb488ce39c4..1b3a16ff3fc 100644 --- a/game-app/game-core/src/main/java/games/strategy/engine/data/export/GameDataExporter.java +++ b/game-app/game-core/src/main/java/games/strategy/engine/data/export/GameDataExporter.java @@ -17,11 +17,13 @@ import games.strategy.engine.data.TerritoryEffect; import games.strategy.engine.data.UnitCollection; import games.strategy.engine.data.UnitType; +import games.strategy.engine.data.gameparser.GameParser; import games.strategy.engine.data.properties.IEditableProperty; import games.strategy.engine.data.properties.NumberProperty; import games.strategy.engine.framework.ServerGame; import games.strategy.triplea.Constants; import games.strategy.triplea.delegate.TechAdvance; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -60,7 +62,7 @@ public class GameDataExporter { * Converts a 'GameData' object into a 'Game' object, the latter is a POJO that models XML and is * suitable to then be written to an XML string. Use this method to export live game data to XML. */ - public static Game convertToXmlModel(final GameData data) { + public static Game convertToXmlModel(final GameData data, final Path existingMapXmlPath) { return Game.builder() .info(info(data)) .triplea( @@ -89,7 +91,7 @@ public static Game convertToXmlModel(final GameData data) { .technologies(technologies(data)) .playerTechs(playertechs(data)) .build()) - .attachmentList(attachments(data)) + .attachmentList(attachments(existingMapXmlPath)) .initialize( Initialize.builder() .ownerInitialize(ownerInitialize(data)) @@ -293,10 +295,17 @@ private static Initialize.OwnerInitialize ownerInitialize(final GameState data) .build(); } - private static AttachmentList attachments(final GameData data) { + private List>>> loadAttachmentOrderAndValues( + Path existingMapXmlPath) { + return GameParser.parse(existingMapXmlPath, true) + .map(GameData::getAttachmentOrderAndValues) + .orElse(List.of()); + } + + private static AttachmentList attachments(final Path existingMapXmlPath) { return AttachmentList.builder() .attachments( - data.getAttachmentOrderAndValues().stream() + loadAttachmentOrderAndValues(existingMapXmlPath).stream() .map(GameDataExporter::printAttachments) .collect(Collectors.toList())) .build(); diff --git a/game-app/game-core/src/main/java/games/strategy/engine/data/gameparser/GameParser.java b/game-app/game-core/src/main/java/games/strategy/engine/data/gameparser/GameParser.java index 59f0691177e..361a5ea091c 100644 --- a/game-app/game-core/src/main/java/games/strategy/engine/data/gameparser/GameParser.java +++ b/game-app/game-core/src/main/java/games/strategy/engine/data/gameparser/GameParser.java @@ -82,15 +82,18 @@ public final class GameParser { private final XmlGameElementMapper xmlGameElementMapper; private GameDataVariables variables; private final Version engineVersion; + private boolean collectAttachmentOrderAndValues; private GameParser( final Path xmlUri, final XmlGameElementMapper xmlGameElementMapper, - final Version engineVersion) { + final Version engineVersion, + final boolean collectAttachmentOrderAndValues) { data = new GameData(); this.xmlUri = xmlUri; this.xmlGameElementMapper = xmlGameElementMapper; this.engineVersion = engineVersion; + this.collectAttachmentOrderAndValues = collectAttachmentOrderAndValues; } /** @@ -100,11 +103,15 @@ private GameParser( * @return A complete {@link GameData} instance that can be used to play the game, otherwise * returns empty if the file could not parsed or is not valid. */ - public static Optional parse(final Path xmlFile) { + public static Optional parse( + final Path xmlFile, boolean collectAttachmentOrderAndValues) { log.debug("Parsing game XML: {}", xmlFile.toAbsolutePath()); final Optional gameData = GameParser.parse( - xmlFile, new XmlGameElementMapper(), ProductVersionReader.getCurrentVersion()); + xmlFile, + new XmlGameElementMapper(), + ProductVersionReader.getCurrentVersion(), + collectAttachmentOrderAndValues); // if parsed, find the 'map.yml' from a parent folder and set the 'mapName' property // using the 'map name' from 'map.yml' @@ -125,12 +132,14 @@ public static Optional parse(final Path xmlFile) { public static Optional parse( final Path xmlFile, final XmlGameElementMapper xmlGameElementMapper, - final Version engineVersion) { + final Version engineVersion, + final boolean collectAttachmentOrderAndValues) { return UrlStreams.openStream( xmlFile.toUri(), inputStream -> { try { - return new GameParser(xmlFile, xmlGameElementMapper, engineVersion) + return new GameParser( + xmlFile, xmlGameElementMapper, engineVersion, collectAttachmentOrderAndValues) .parse(xmlFile, inputStream); } catch (final EngineVersionException e) { log.warn("Game engine not compatible with: " + xmlFile, e); @@ -839,7 +848,9 @@ private void parseAttachment( final List> attachmentOptionValues = setOptions(attachment, current.getOptions(), foreach); // keep a list of attachment references in the order they were added - data.addToAttachmentOrderAndValues(Tuple.of(attachment, attachmentOptionValues)); + if (collectAttachmentOrderAndValues) { + data.addToAttachmentOrderAndValues(Tuple.of(attachment, attachmentOptionValues)); + } } private Attachable findAttachment( diff --git a/game-app/game-core/src/main/java/games/strategy/engine/framework/GameDataManager.java b/game-app/game-core/src/main/java/games/strategy/engine/framework/GameDataManager.java index dfcd2f919ca..0b8a0280d66 100644 --- a/game-app/game-core/src/main/java/games/strategy/engine/framework/GameDataManager.java +++ b/game-app/game-core/src/main/java/games/strategy/engine/framework/GameDataManager.java @@ -129,7 +129,7 @@ public static void saveGame(final OutputStream out, final GameData gameData) thr try (OutputStream os = Files.newOutputStream(tempFile); OutputStream bufferedOutStream = new BufferedOutputStream(os); OutputStream zippedOutStream = new GZIPOutputStream(bufferedOutStream)) { - saveGameUncompressed(zippedOutStream, gameData, Options.withEverything()); + saveGameUncompressed(zippedOutStream, gameData, Options.forSaveGame()); } // now write to sink (ensure sink is closed per method contract) @@ -152,6 +152,12 @@ public static Options withEverything() { return builder().withDelegates(true).withHistory(true).withAttachmentXmlData(true).build(); } + public static Options forSaveGame() { + // Omit attachment data as it uses a lot of memory and is only needed for XML exports, which + // now reads it from the original XML instead. + return builder().withDelegates(true).withHistory(true).withAttachmentXmlData(false).build(); + } + public static Options forBattleCalculator() { return builder().build(); } @@ -167,8 +173,6 @@ public static void saveGameUncompressed( if (!options.withHistory) { data.resetHistory(); } - // TODO: Attachment order data is only used for XML export and takes up lots of memory. - // Could we remove it and just get the info again from the XML when exporting? final var attachments = data.getAttachmentOrderAndValues(); if (!options.withAttachmentXmlData) { data.setAttachmentOrderAndValues(null); diff --git a/game-app/game-core/src/main/java/games/strategy/engine/framework/startup/ui/panels/main/game/selector/GameSelectorModel.java b/game-app/game-core/src/main/java/games/strategy/engine/framework/startup/ui/panels/main/game/selector/GameSelectorModel.java index 61977ceb7f7..59c1690d9ba 100644 --- a/game-app/game-core/src/main/java/games/strategy/engine/framework/startup/ui/panels/main/game/selector/GameSelectorModel.java +++ b/game-app/game-core/src/main/java/games/strategy/engine/framework/startup/ui/panels/main/game/selector/GameSelectorModel.java @@ -15,7 +15,6 @@ import java.util.List; import java.util.Observable; import java.util.Optional; -import java.util.function.Function; import java.util.function.Predicate; import javax.annotation.Nullable; import lombok.Getter; @@ -29,9 +28,6 @@ */ @Slf4j public class GameSelectorModel extends Observable implements GameSelector { - - private final Function> gameParser; - @Nullable @Getter(onMethod_ = {@Override}) private GameData gameData = null; @@ -46,9 +42,7 @@ public class GameSelectorModel extends Observable implements GameSelector { @Setter @Getter private ClientModel clientModelForHostBots = null; private Optional saveGameToLoad = Optional.empty(); - public GameSelectorModel() { - this.gameParser = GameParser::parse; - } + public GameSelectorModel() {} /** * Loads game data by parsing a given file. @@ -102,7 +96,7 @@ private void resetDefaultGame() { @Nullable private GameData parseAndValidate(final Path file) { - final GameData gameData = gameParser.apply(file).orElse(null); + final GameData gameData = GameParser.parse(file, false).orElse(null); if (gameData == null) { return null; } diff --git a/game-app/game-core/src/test/java/games/strategy/engine/data/gameparser/GameParserTest.java b/game-app/game-core/src/test/java/games/strategy/engine/data/gameparser/GameParserTest.java index 9edc05421dc..89b692b040f 100644 --- a/game-app/game-core/src/test/java/games/strategy/engine/data/gameparser/GameParserTest.java +++ b/game-app/game-core/src/test/java/games/strategy/engine/data/gameparser/GameParserTest.java @@ -26,7 +26,8 @@ final class GameParserTest { void backwardCompatibilityCheck() throws Exception { final Path mapFile = getTestMap("v1_8_map__270BC.xml"); final GameData gameData = - GameParser.parse(mapFile, new XmlGameElementMapper(), new Version("2.0.0")).orElseThrow(); + GameParser.parse(mapFile, new XmlGameElementMapper(), new Version("2.0.0"), false) + .orElseThrow(); assertNotNullGameData(gameData); verifyLegacyPropertiesAreUpdated(gameData); diff --git a/game-app/game-core/src/test/java/games/strategy/triplea/xml/TestMapGameData.java b/game-app/game-core/src/test/java/games/strategy/triplea/xml/TestMapGameData.java index deb158bc853..d8103b0ec3e 100644 --- a/game-app/game-core/src/test/java/games/strategy/triplea/xml/TestMapGameData.java +++ b/game-app/game-core/src/test/java/games/strategy/triplea/xml/TestMapGameData.java @@ -78,7 +78,8 @@ public GameData getGameData() { new XmlGameElementMapper( Map.of("TestDelegate", TestDelegate::new), Map.of("TestAttachment", TestAttachment::new)), - new Version("2.0.0")) + new Version("2.0.0"), + false) .orElseThrow(() -> new IllegalStateException("Error parsing: " + mapUri)); } } diff --git a/game-app/game-headed/src/main/java/games/strategy/triplea/ui/menubar/ExportMenu.java b/game-app/game-headed/src/main/java/games/strategy/triplea/ui/menubar/ExportMenu.java index b530acb12df..32c23039aab 100644 --- a/game-app/game-headed/src/main/java/games/strategy/triplea/ui/menubar/ExportMenu.java +++ b/game-app/game-headed/src/main/java/games/strategy/triplea/ui/menubar/ExportMenu.java @@ -91,6 +91,13 @@ private JMenuItem createExportXmlMenu() { } private void exportXmlFile() { + // The existing XML file is needed for attachment ordering data. + final Path gameXmlPath = gameData.getGameXmlPath(uiContext.getMapLocation()).orElse(null); + if (gameXmlPath == null) { + JOptionPane.showMessageDialog(frame, "Error: Existing XML file not found."); + return; + } + final JFileChooser chooser = new JFileChooser(); chooser.setFileSelectionMode(JFileChooser.FILES_ONLY); @@ -109,7 +116,7 @@ private void exportXmlFile() { return; } try (GameData.Unlocker ignored = gameData.acquireReadLock()) { - final Game xmlGameModel = GameDataExporter.convertToXmlModel(gameData); + final Game xmlGameModel = GameDataExporter.convertToXmlModel(gameData, gameXmlPath); GameXmlWriter.exportXml(xmlGameModel, chooser.getSelectedFile().toPath()); } } diff --git a/game-app/smoke-testing/src/test/java/games/strategy/engine/data/GameTestUtils.java b/game-app/smoke-testing/src/test/java/games/strategy/engine/data/GameTestUtils.java index 3fe5fd02ece..ebe617250dc 100644 --- a/game-app/smoke-testing/src/test/java/games/strategy/engine/data/GameTestUtils.java +++ b/game-app/smoke-testing/src/test/java/games/strategy/engine/data/GameTestUtils.java @@ -55,7 +55,7 @@ public static ServerGame setUpGameWithAis(String xmlName) { } GameData gameData = - GameParser.parse(xmlFilePath) + GameParser.parse(xmlFilePath, false) .orElseThrow(() -> new RuntimeException("Error parsing file: " + xmlFilePath)); Map playerTypes = new HashMap<>(); for (var player : gameData.getPlayerList().getPlayers()) { diff --git a/game-app/smoke-testing/src/test/java/games/strategy/engine/data/ParseGameXmlsTest.java b/game-app/smoke-testing/src/test/java/games/strategy/engine/data/ParseGameXmlsTest.java index 95be62b0f92..824040ff68e 100644 --- a/game-app/smoke-testing/src/test/java/games/strategy/engine/data/ParseGameXmlsTest.java +++ b/game-app/smoke-testing/src/test/java/games/strategy/engine/data/ParseGameXmlsTest.java @@ -22,7 +22,7 @@ class ParseGameXmlsTest { void parseGameFiles(final Path xmlFile) { final Optional result = GameParser.parse( - xmlFile, new XmlGameElementMapper(), ProductVersionReader.getCurrentVersion()); + xmlFile, new XmlGameElementMapper(), ProductVersionReader.getCurrentVersion(), false); assertThat(result, OptionalMatchers.isPresent()); }