Omit attachment order data from saves by default. (#11771)

* Omit attachment order data from saves by default.

This data is only needed for the "export to XML" function which is only used by mapmakers. However, the data is a huge contributor to save file size (and saving time). With this change, Imperialism 1974 saves go from 1.2MB in size to 800K (1/3 reduction).

The functionality to export to XML is maintained by re-reading the attachment order data from the original XML file when the export function is used.

Tested exporting map XML and confirmed it still worked.

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<Path> 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<MapDescriptionYaml> findMapDescriptionYaml(final Path mapLocation) {

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<Tuple<IAttachment, List<Tuple<String, String>>>> 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();

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<GameData> parse(final Path xmlFile) {
+  public static Optional<GameData> parse(
+      final Path xmlFile, boolean collectAttachmentOrderAndValues) {
     log.debug("Parsing game XML: {}", xmlFile.toAbsolutePath());
     final Optional<GameData> 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<GameData> parse(final Path xmlFile) {
   public static Optional<GameData> 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<Tuple<String, String>> 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(

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);

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<Path, Optional<GameData>> 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<String> 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;
     }

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);

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));
   }
 }

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());
     }
   }

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<String, PlayerTypes.Type> playerTypes = new HashMap<>();
     for (var player : gameData.getPlayerList().getPlayers()) {

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<GameData> result =
         GameParser.parse(
-            xmlFile, new XmlGameElementMapper(), ProductVersionReader.getCurrentVersion());
+            xmlFile, new XmlGameElementMapper(), ProductVersionReader.getCurrentVersion(), false);
     assertThat(result, OptionalMatchers.isPresent());
   }