Move custom logic for playerColours and privateGame with property def…

…initions. Also update documentation for gameBuild and privateGame
bmaupin · Jan 5, 2018 · 9681f0e · 9681f0e
1 parent 433e40a
commit 9681f0e
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 22 deletions.
diff --git a/src/Civ5Save.js b/src/Civ5Save.js
@@ -114,15 +114,6 @@ class Civ5Save {
         saveGameVersion = properties.saveGameVersion.getValue(this._saveData);
       }
 
-      // Before build 310700, playerColours is a list of bytes. There isn't much value in implementing that. And
-      // privateGame comes after playerColours.
-      // TODO: move this logic into property definitions file
-      if (propertyName === 'playerColours' || propertyName === 'privateGame') {
-        if (Number(this.gameBuild) < 310700) {
-          continue;
-        }
-      }
-
       // Make propertyDefinition a copy; otherwise it will modify the property for every instance of the Civ5Save class
       let propertyDefinition = Object.assign({}, Civ5SavePropertyDefinitions[propertyName]);
 
@@ -132,8 +123,9 @@ class Civ5Save {
         continue;
       }
 
+      let propertyLength = propertyDefinition.length;
       if (propertyDefinition.hasOwnProperty('getLength')) {
-        propertyDefinition.length = propertyDefinition.getLength(properties.enabledDLC.getArray());
+        propertyLength = propertyDefinition.getLength(properties.enabledDLC.getArray());
       }
 
       let propertyByteOffset = 0;
@@ -149,7 +141,7 @@ class Civ5Save {
         properties[propertyName] = Civ5SavePropertyFactory.fromType(
           propertyDefinition.type,
           propertyByteOffset,
-          propertyDefinition.length,
+          propertyLength,
           this._saveData);
       } catch (e) {
         throw new ParseError(`Failure parsing save at property ${propertyName}`);
@@ -249,6 +241,10 @@ class Civ5Save {
 
   /**
    * Game build number.
+   *
+   * Note that for games created or saved before build 230620, this will return the game build that was used to create
+   * the save file. Starting with build 230620, this will return the game build that was last used to save the save
+   * file.
    * @type {string}
    */
   get gameBuild() {
@@ -475,6 +471,9 @@ class Civ5Save {
 
   /**
    * Private setting for multiplayer games.
+   *
+   * Note that this will be `undefined` if [gameBuild](#instance-get-gameBuild) is less than 310700 because it isn't
+   * implemented. `undefined` is used instead of `null` because `null` might incorrectly imply the value is empty.
    * @type {boolean}
    */
   get privateGame() {

diff --git a/src/Civ5SavePropertyDefinitions.js b/src/Civ5SavePropertyDefinitions.js
@@ -40,7 +40,8 @@ export default {
       }
     }
   },
-  // This property is updated if the game is saved with a newer version of Civ 5. The build of Civ 5 that was originally used when the save file was created is stored later, after gameOptionsMap.
+  // This property is updated if the game is saved with a newer version of Civ 5. The build of Civ 5 that was originally
+  // used when the save file was created is stored later, after gameOptionsMap.
   'gameBuild': {
     'byteOffsetInSection': null,
     'length': null,
@@ -161,7 +162,8 @@ export default {
       'None'
     ]
   },
-  // Starting with build 310700 this is a list of strings. Before that I'm not sure if it's a list of bytes or not there at all
+  // Starting with build 310700 this is a list of strings. Before that I'm not sure if it's a list of bytes or not there
+  // at all
   'playerCivilizations': {
     'byteOffsetInSection': 4,
     // Length is number of items, not bytes
@@ -171,7 +173,8 @@ export default {
     },
     'type': 'stringArray'
   },
-  // This is rare but seems to contain the full path to the save file, e.g. C:\Users\Username\Documents\My Games\Sid Meier's Civilization 5\Saves\multi\auto\AutoSave_0310 AD-2030.Civ5Save
+  // This is rare but seems to contain the full path to the save file, e.g.
+  // C:\Users\Username\Documents\My Games\Sid Meier's Civilization 5\Saves\multi\auto\AutoSave_0310 AD-2030.Civ5Save
   'section19SkipSavePath': {
     'byteOffsetInSection': 260,
     'length': null,
@@ -267,14 +270,18 @@ export default {
     },
     'type': 'int'
   },
-  // Starting with build 310700 this is a list of strings. Before that it's a list of bytes.
   'playerColours': {
     'byteOffsetInSection': 4,
     // Length is number of items, not bytes
     'length': 64,
-    'sectionByBuild': {
-      '98650': 23,
-      '262623': 24,
+    // This is technically incorrect; before build 310700 this property exists, but it's a list of bytes instead of a
+    // list of strings, and there isn't much value in adding the extra complexity for old save games. For reference, the
+    // correct values are:
+    //  '98650': 23,
+    //  '262623': 24,
+    //  '395070': 25
+    'sectionByBuild': {
+      '310700': 24,
       '395070': 25
     },
     'type': 'stringArray'
@@ -283,9 +290,11 @@ export default {
   'privateGame': {
     'byteOffsetInSection': null,
     'length': 1,
+    // As with playerColours, this is technically incorrect, but there isn't much value in implementing this for older
+    // games because 1. it would require implementing playerColours and 2. it's only relevant for multiplayer games,
+    // however logic for identifying multiplayer games before build 230620 hasn't been implemented (see gameMode)
     'sectionByBuild': {
-      '98650': 23,
-      '262623': 24,
+      '310700': 24,
       '395070': 25
     },
     'type': 'bool'
@@ -483,7 +492,8 @@ export default {
     },
     'type': 'string'
   },
-  // This section is 80 bytes long if Brave New World is installed. It's 76 bytes if only Gods and Kings is installed. Otherwise it's 72 bytes long.
+  // This section is 80 bytes long if Brave New World is installed. It's 76 bytes if only Gods and Kings is installed.
+  // Otherwise it's 72 bytes long.
   'section30Skip3': {
     'byteOffsetInSection': null,
     'length': null,
@@ -503,7 +513,8 @@ export default {
       }
     }
   },
-  // This is where a large chunk of game options are stored (http://civilization.wikia.com/wiki/Module:Data/Civ5/BNW/GameOptions)
+  // This is where a large chunk of game options are stored
+  // (http://civilization.wikia.com/wiki/Module:Data/Civ5/BNW/GameOptions)
   'gameOptionsMap': {
     'byteOffsetInSection': null,
     'length': null,