Add Support for (most) 1.8 maps (#7631)

* Add: Support XML tag aliases, support "attatchment" mispelling

This initial round of updates allows for basic support of 1.8 maps,
they will load after this update.

* Add support to load (most) 1.8 maps

- Add conversion code to match attachments spelled either as 'attatchment' or as 'attachment

- Add annotation @RenameOnNextMajorRelease to help us know which items we can rename
 and to what when save game compatibility can be broken.

- Add ignore for properties: "takeUnitControl" and "giveUnitControl"

Add conversion logic for:
- isParatroop -> isAirTransportable
- isInfantry & isMechanized -> isLandTransportable
- Battleship|Units repair at beginning|end of round -> Units Repair at beginning|end of round
- 'occupiedTerrOf' -> 'originalOwner'
- 'victoryCity=false' -> 'victoryCity=0'
- 'isImpassible' -> 'isImpassable'
- 'turns' -> 'rounds'

game-core/src/main/java/games/strategy/engine/data/ChangeAttachmentChange.java

@@ -1,6 +1,6 @@
 package games.strategy.engine.data;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import java.util.Optional;
 
 /** A game data change that captures a change to an attachment property value. */
 public class ChangeAttachmentChange extends Change {
@@ -10,7 +10,7 @@ public class ChangeAttachmentChange extends Change {
   private final Object newValue;
   private final Object oldValue;
   private final String property;
-  private boolean clearFirst = false;
+  private final boolean clearFirst;
 
   /**
    * Initializes a new instance of the ChangeAttachmentChange class.
@@ -21,13 +21,13 @@ public class ChangeAttachmentChange extends Change {
    */
   public ChangeAttachmentChange(
       final IAttachment attachment, final Object newValue, final String property) {
-    checkNotNull(attachment, "null attachment; newValue: " + newValue + ", property: " + property);
-
-    attachedTo = attachment.getAttachedTo();
-    attachmentName = attachment.getName();
-    oldValue = attachment.getPropertyOrThrow(property).getValue();
-    this.newValue = newValue;
-    this.property = property;
+    this(
+        attachment.getAttachedTo(),
+        attachment.getName(),
+        newValue,
+        attachment.getPropertyOrThrow(property).getValue(),
+        property,
+        false);
   }
 
   /**
@@ -38,15 +38,14 @@ public ChangeAttachmentChange(
       final IAttachment attachment,
       final Object newValue,
       final String property,
-      final boolean resetFirst) {
-    checkNotNull(attachment, "null attachment; newValue: " + newValue + ", property: " + property);
-
-    attachedTo = attachment.getAttachedTo();
-    clearFirst = resetFirst;
-    attachmentName = attachment.getName();
-    oldValue = attachment.getPropertyOrThrow(property).getValue();
-    this.newValue = newValue;
-    this.property = property;
+      final boolean clearFirst) {
+    this(
+        attachment.getAttachedTo(),
+        attachment.getName(),
+        newValue,
+        attachment.getPropertyOrThrow(property).getValue(),
+        property,
+        clearFirst);
   }
 
   /**
@@ -59,13 +58,16 @@ public ChangeAttachmentChange(
       final Object newValue,
       final Object oldValue,
       final String property,
-      final boolean resetFirst) {
-    this.attachmentName = attachmentName;
+      final boolean clearFirst) {
+    this.attachmentName =
+        Optional.ofNullable(attachmentName)
+            .map(name -> name.replaceAll("ttatch", "ttach"))
+            .orElse(null);
     attachedTo = attachTo;
     this.newValue = newValue;
     this.oldValue = oldValue;
     this.property = property;
-    clearFirst = resetFirst;
+    this.clearFirst = clearFirst;
   }
 
   public Attachable getAttachedTo() {

game-core/src/main/java/games/strategy/engine/data/DefaultAttachment.java

@@ -115,7 +115,9 @@ public void setAttachedTo(final Attachable attachable) {
 
   @Override
   public String getName() {
-    return name;
+    return Optional.ofNullable(name)
+        .map(attachmentName -> attachmentName.replaceAll("ttatch", "ttach"))
+        .orElse(null);
   }
 
   @Override

game-core/src/main/java/games/strategy/engine/data/NamedAttachable.java

@@ -26,7 +26,7 @@ public Map<String, IAttachment> getAttachments() {
 
   @Override
   public void addAttachment(final String key, final IAttachment value) {
-    attachments.put(key, value);
+    attachments.put(key.replaceAll("ttatchment", "ttachment"), value);
   }
 
   @Override

game-core/src/main/java/games/strategy/engine/data/gameparser/GameParser.java

@@ -467,21 +467,21 @@ private void parseProperties(final PropertyList propertyList) {
               .orElseGet(() -> Strings.emptyToNull(current.getValue()));
 
       final boolean editable = current.isEditable();
-      final String property = current.getName();
+      final String propertyName = LegacyPropertyMapper.mapPropertyName(current.getName());
       if (editable) {
-        parseEditableProperty(current, property, value);
+        parseEditableProperty(current, propertyName, value);
       } else {
         if (current.getBooleanProperty() != null) {
-          properties.set(property, Boolean.parseBoolean(value));
+          properties.set(propertyName, Boolean.parseBoolean(value));
         } else if (current.getStringProperty() != null) {
-          properties.set(property, value);
+          properties.set(propertyName, value);
         } else {
           try {
             final int integer = Integer.parseInt(value);
-            properties.set(property, integer);
+            properties.set(propertyName, integer);
           } catch (final NumberFormatException e) {
             // then it must be a string
-            properties.set(property, value);
+            properties.set(propertyName, value);
           }
         }
       }
@@ -900,7 +900,11 @@ private List<Tuple<String, String>> setOptions(
     final List<Tuple<String, String>> results = new ArrayList<>();
     for (final AttachmentList.Attachment.Option option : options) {
       // decapitalize the property name for backwards compatibility
-      final String name = decapitalize(option.getName());
+      final String name = LegacyPropertyMapper.mapLegacyOptionName(decapitalize(option.getName()));
+      if (LegacyPropertyMapper.ignoreOptionName(name)) {
+        continue;
+      }
+
       if (name.isEmpty()) {
         throw new GameParseException(
             "Option name with zero length for attachment: " + attachment.getName());
@@ -912,7 +916,8 @@ private List<Tuple<String, String>> setOptions(
         continue; // Skip adding option if contains empty foreach variable
       }
       final String valueWithForeach = replaceForeachVariables(countAndValue, foreach);
-      final String finalValue = replaceVariables(valueWithForeach, variables);
+      final String interpolatedValue = replaceVariables(valueWithForeach, variables);
+      final String finalValue = LegacyPropertyMapper.mapLegacyOptionValue(name, interpolatedValue);
       try {
         attachment
             .getProperty(name)
@@ -928,8 +933,8 @@ private List<Tuple<String, String>> setOptions(
       } catch (final Exception e) {
         throw new GameParseException(
             String.format(
-                "map name: '%s', Unexpected Exception while setting values for attachment: %s",
-                mapName, attachment),
+                "map name: '%s', Unexpected Exception while setting values for attachment: %s, %s",
+                mapName, attachment, e.getMessage()),
             e);
       }
       results.add(Tuple.of(name, finalValue));

game-core/src/main/java/games/strategy/engine/data/gameparser/LegacyPropertyMapper.java

@@ -0,0 +1,58 @@
+package games.strategy.engine.data.gameparser;
+
+import games.strategy.triplea.Constants;
+import lombok.experimental.UtilityClass;
+
+/**
+ * This converts property names and values that were used in older map XMLs and updates those values
+ * to their newer versions. This is useful to allow the game code to only use the newer names and
+ * values while still being able to load older maps.
+ */
+@UtilityClass
+class LegacyPropertyMapper {
+
+  static String mapLegacyOptionName(final String optionName) {
+    if (optionName.equalsIgnoreCase("isParatroop")) {
+      return "isAirTransportable";
+    } else if (optionName.equalsIgnoreCase("isInfantry")
+        || optionName.equalsIgnoreCase("isMechanized")) {
+      return "isLandTransportable";
+    } else if (optionName.equalsIgnoreCase("occupiedTerrOf")) {
+      return Constants.ORIGINAL_OWNER;
+    } else if (optionName.equalsIgnoreCase("isImpassible")) {
+      return "isImpassable";
+    } else if (optionName.equalsIgnoreCase("turns")) {
+      return "rounds";
+    }
+    return optionName;
+  }
+
+  public String mapLegacyOptionValue(final String optionName, final String optionValue) {
+    if (optionName.equalsIgnoreCase("victoryCity")) {
+      if (optionValue.equalsIgnoreCase("true")) {
+        return "1";
+      } else if (optionValue.equalsIgnoreCase("false")) {
+        return "0";
+      } else {
+        return optionValue;
+      }
+    }
+    return optionValue;
+  }
+
+  public String mapPropertyName(final String propertyName) {
+    if (propertyName.equalsIgnoreCase("Battleships repair at end of round")
+        || propertyName.equalsIgnoreCase("Units repair at end of round")) {
+      return Constants.TWO_HIT_BATTLESHIPS_REPAIR_END_OF_TURN;
+    } else if (propertyName.equalsIgnoreCase("Battleships repair at beginning of round")
+        || propertyName.equalsIgnoreCase("Units repair at beginning of round")) {
+      return Constants.TWO_HIT_BATTLESHIPS_REPAIR_BEGINNING_OF_TURN;
+    }
+
+    return propertyName;
+  }
+
+  static boolean ignoreOptionName(final String name) {
+    return name.equalsIgnoreCase("takeUnitControl") || name.equalsIgnoreCase("giveUnitControl");
+  }
+}

game-core/src/main/java/games/strategy/engine/data/gameparser/XmlGameElementMapper.java

@@ -175,8 +175,7 @@ public Optional<IAttachment> newAttachment(
       final GameData gameData) {
     checkNotNull(typeName);
 
-    final String normalizedTypeName =
-        typeName.replaceAll("^games\\.strategy\\.triplea\\.attachments\\.", "");
+    final String normalizedTypeName = typeName.replaceAll("^.*\\.", "");
     final @Nullable AttachmentFactory attachmentFactory =
         attachmentFactoriesByTypeName.get(normalizedTypeName);
     if (attachmentFactory != null) {

game-core/src/main/java/games/strategy/triplea/attachments/AbstractRulesAttachment.java

@@ -1,5 +1,6 @@
 package games.strategy.triplea.attachments;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
 import games.strategy.engine.data.Attachable;
 import games.strategy.engine.data.GameData;
@@ -18,6 +19,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import org.triplea.java.RenameOnNextMajorRelease;
 import org.triplea.java.collections.CollectionUtils;
 
 /** The Purpose of this class is to hold shared and simple methods used by RulesAttachment. */
@@ -34,6 +36,7 @@ public abstract class AbstractRulesAttachment extends AbstractConditionsAttachme
   // only matters for objectiveValue, does not affect the condition
   protected int uses = -1;
   // condition for what turn it is
+  @RenameOnNextMajorRelease(newName = "rounds")
   protected Map<Integer, Integer> turns = null;
   // for on/off conditions
   protected boolean switched = true;
@@ -206,19 +209,16 @@ private void setRounds(final String rounds) throws GameParseException {
     }
   }
 
-  private void setTurns(final String turns) throws GameParseException {
-    setRounds(turns);
-  }
-
-  private void setTurns(final Map<Integer, Integer> value) {
+  private void setRounds(final Map<Integer, Integer> value) {
     turns = value;
   }
 
-  private Map<Integer, Integer> getTurns() {
+  @VisibleForTesting
+  public Map<Integer, Integer> getRounds() {
     return turns;
   }
 
-  private void resetTurns() {
+  private void resetRounds() {
     turns = null;
   }
 
@@ -433,8 +433,9 @@ public Map<String, MutableProperty<?>> getPropertyMap() {
             "uses",
             MutableProperty.of(this::setUses, this::setUses, this::getUses, this::resetUses))
         .put(
-            "turns",
-            MutableProperty.of(this::setTurns, this::setTurns, this::getTurns, this::resetTurns))
+            "rounds",
+            MutableProperty.of(
+                this::setRounds, this::setRounds, this::getRounds, this::resetRounds))
         .put(
             "switch",
             MutableProperty.of(
@@ -443,7 +444,6 @@ public Map<String, MutableProperty<?>> getPropertyMap() {
             "gameProperty",
             MutableProperty.ofString(
                 this::setGameProperty, this::getGameProperty, this::resetGameProperty))
-        .put("rounds", MutableProperty.ofWriteOnlyString(this::setRounds))
         .build();
   }
 }

game-core/src/main/java/games/strategy/triplea/attachments/TriggerAttachment.java

@@ -908,11 +908,13 @@ private void resetPlayers() {
     players = new ArrayList<>();
   }
 
-  private void setPlayerAttachmentName(final String name) throws GameParseException {
-    if (name == null) {
-      playerAttachmentName = null;
+  private void setPlayerAttachmentName(final String playerAttachmentName)
+      throws GameParseException {
+    if (playerAttachmentName == null) {
+      this.playerAttachmentName = null;
       return;
     }
+    final String name = playerAttachmentName.replaceAll("ttatch", "ttach");
     final String[] s = splitOnColon(name);
     if (s.length != 2) {
       throw new GameParseException(
@@ -961,7 +963,7 @@ private void setPlayerAttachmentName(final String name) throws GameParseExceptio
         && !s[0].startsWith(Constants.USERACTION_ATTACHMENT_PREFIX)) {
       throw new GameParseException("attachment incorrectly named:" + s[0] + thisErrorMsg());
     }
-    playerAttachmentName = Tuple.of(s[1], s[0]);
+    this.playerAttachmentName = Tuple.of(s[1], s[0]);
   }
 
   private void setPlayerAttachmentName(final Tuple<String, String> value) {

game-core/src/main/java/games/strategy/triplea/attachments/UnitAttachment.java

@@ -3690,12 +3690,6 @@ public Map<String, MutableProperty<?>> getPropertyMap() {
         .put(
             "isAir",
             MutableProperty.of(this::setIsAir, this::setIsAir, this::getIsAir, this::resetIsAir))
-        .put(
-            "isMechanized", // kept for map compatibility; remove upon next map-incompatible release
-            MutableProperty.ofWriteOnlyString(s -> {}))
-        .put(
-            "isParatroop", // kept for map compatibility; remove upon next map-incompatible release
-            MutableProperty.ofWriteOnlyString(s -> {}))
         .put(
             "isSea",
             MutableProperty.of(this::setIsSea, this::setIsSea, this::getIsSea, this::resetIsSea))
@@ -3943,13 +3937,6 @@ public Map<String, MutableProperty<?>> getPropertyMap() {
                 this::setIsAirTransportable,
                 this::getIsAirTransportable,
                 this::resetIsAirTransportable))
-        .put(
-            "isInfantry", // kept for map compatibility; remove upon next map-incompatible release
-            MutableProperty.of(
-                this::setIsLandTransportable,
-                this::setIsLandTransportable,
-                this::getIsLandTransportable,
-                this::resetIsLandTransportable))
         .put(
             "isLandTransport",
             MutableProperty.of(

game-core/src/main/java/games/strategy/triplea/ui/mapdata/MapData.java

@@ -500,8 +500,10 @@ public Color getPlayerColor(final String playerName) {
 
   /** returns the color for impassable territories. */
   public Color impassableColor() {
-    // just use getPlayerColor, since it parses the properties
-    return getPlayerColor(Constants.PLAYER_NAME_IMPASSABLE);
+    return Optional.ofNullable(
+            getColorProperty(PROPERTY_COLOR_PREFIX + Constants.PLAYER_NAME_IMPASSABLE))
+        .or(() -> Optional.ofNullable(getColorProperty(PROPERTY_COLOR_PREFIX + "Impassible")))
+        .orElseGet(defaultColors::nextColor);
   }
 
   /**