Merge pull request #2098 from HullSeals/enhancement/2051/remove-the-p…

…ickle

[2051] Convert Pickle to Json

build.py

@@ -73,8 +73,10 @@ def generate_data_files(
                 "ChangeLog.md",
                 "snd_good.wav",
                 "snd_bad.wav",
-                "modules.p",
-                "ships.p",
+                "modules.p",  # TODO: Remove in 6.0
+                "resources/modules.json",
+                "resources/ships.json",
+                "ships.p",  # TODO: Remove in 6.0
                 f"{app_name}.VisualElementsManifest.xml",
                 f"{app_name}.ico",
                 "EDMarketConnector - TRACE.bat",

coriolis-update-files.py

@@ -1,16 +1,18 @@
-#!/usr/bin/env python3
 """
-Build ship and module databases from https://github.com/EDCD/coriolis-data/ .
+coriolis-update-files.py - Build ship and module databases from https://github.com/EDCD/coriolis-data/.
 
-This script also utilise the file outfitting.csv.   Due to how collate.py
-both reads and writes to this file a local copy is used, in the root of the
-project structure, is used for this purpose.  If you want to utilise the
+Copyright (c) EDCD, All Rights Reserved
+Licensed under the GNU General Public License.
+See LICENSE file.
+
+This script also utilizes the file outfitting.csv. Due to how collate.py
+both reads and writes to this file, a local copy in the root of the
+project structure is used for this purpose. If you want to utilize the
 FDevIDs/ version of the file, copy it over the local one.
 """
 
 
 import json
-import pickle
 import subprocess
 import sys
 from collections import OrderedDict
@@ -29,7 +31,9 @@ def add(modules, name, attributes) -> None:
     # Regenerate coriolis-data distribution
     subprocess.check_call('npm install', cwd='coriolis-data', shell=True, stdout=sys.stdout, stderr=sys.stderr)
 
-    data = json.load(open('coriolis-data/dist/index.json'))
+    file_path = 'coriolis-data/dist/index.json'
+    with open(file_path) as file:
+        data = json.load(file)
 
     # Symbolic name from in-game name
     reverse_ship_map = {v: k for k, v in list(ship_name_map.items())}
@@ -44,11 +48,12 @@ def add(modules, name, attributes) -> None:
         name = coriolis_ship_map.get(m['properties']['name'], str(m['properties']['name']))
         assert name in reverse_ship_map, name
         ships[name] = {'hullMass': m['properties']['hullMass']}
-        for i in range(len(bulkheads)):
-            modules['_'.join([reverse_ship_map[name], 'armour', bulkheads[i]])] = {'mass': m['bulkheads'][i]['mass']}
+        for i, bulkhead in enumerate(bulkheads):
+            modules['_'.join([reverse_ship_map[name], 'armour', bulkhead])] = {'mass': m['bulkheads'][i]['mass']}
 
     ships = OrderedDict([(k, ships[k]) for k in sorted(ships)])  # sort for easier diffing
-    pickle.dump(ships, open('ships.p', 'wb'))
+    with open("resources/ships.json", "w") as ships_file:
+        json.dump(ships, ships_file, indent=4)
 
     # Module masses
     for cat in list(data['Modules'].values()):
@@ -82,4 +87,5 @@ def add(modules, name, attributes) -> None:
     add(modules, 'hpt_multicannon_fixed_medium_advanced',         {'mass': 4})
 
     modules = OrderedDict([(k, modules[k]) for k in sorted(modules)])  # sort for easier diffing
-    pickle.dump(modules, open('modules.p', 'wb'))
+    with open("resources/modules.json", "w") as modules_file:
+        json.dump(modules, modules_file, indent=4)

docs/Releasing.md

@@ -156,7 +156,7 @@ Before you create a new install each time you should:
    1. `cd coriolis-data`
    2. `git pull`
    3. `npm install` - to check it's worked.
-3. Run `coriolis-update-files.py` to update `modules.p` and `ships.p`. **NB:
+3. Run `coriolis-update-files.py` to update `modules.json` and `ships.json`. **NB:
     The submodule might have been updated by a GitHub workflow/PR/merge, so
     be sure to perform this step for every build.**
 4. XXX: Test ?

edshipyard.py

@@ -1,12 +1,13 @@
 """Export ship loadout in ED Shipyard plain text format."""
+from __future__ import annotations
 
+import json
 import os
 import pathlib
-import pickle
 import re
 import time
 from collections import defaultdict
-from typing import Dict, List, Union
+from typing import Union
 
 import outfitting
 import util_ships
@@ -17,14 +18,15 @@
 
 logger = get_main_logger()
 
-__Module = Dict[str, Union[str, List[str]]]
+__Module = dict[str, Union[str, list[str]]]  # Have to keep old-style here for compatibility
 
 # Map API ship names to ED Shipyard names
 ship_map = ship_name_map.copy()
 
 # Ship masses
-# TODO: prefer something other than pickle for this storage (dev readability, security)
-ships = pickle.load(open(pathlib.Path(config.respath_path) / 'ships.p', 'rb'))
+ships_file = config.respath_path / "resources" / "ships.json"
+with open(ships_file, encoding="utf-8") as ships_file_handle:
+    ships = json.load(ships_file_handle)
 
 
 def export(data, filename=None) -> None:  # noqa: C901, CCR001
@@ -75,7 +77,6 @@ def class_rating(module: __Module) -> str:
     jumpboost = 0
 
     for slot in sorted(data['ship']['modules']):
-
         v = data['ship']['modules'][slot]
         try:
             if not v:
@@ -116,15 +117,14 @@ def class_rating(module: __Module) -> str:
 
             jumpboost += module.get('jumpboost', 0)  # type: ignore
 
-            for s in slot_map:
-                if slot.lower().startswith(s):
-                    loadout[slot_map[s]].append(cr + name)
+            for slot_prefix, index in slot_map.items():
+                if slot.lower().startswith(slot_prefix):
+                    loadout[index].append(cr + name)
                     break
 
             else:
                 if slot.lower().startswith('slot'):
                     loadout[slot[-1]].append(cr + name)
-
                 elif not slot.lower().startswith('planetaryapproachsuite'):
                     logger.debug(f'EDShipyard: Unknown slot {slot}')
 
@@ -138,7 +138,6 @@ def class_rating(module: __Module) -> str:
 
     # Construct description
     ship = ship_map.get(data['ship']['name'].lower(), data['ship']['name'])
-
     if data['ship'].get('shipName') is not None:
         _ships = f'{ship}, {data["ship"]["shipName"]}'
 
@@ -185,15 +184,14 @@ def class_rating(module: __Module) -> str:
     if filename:
         with open(filename, 'wt') as h:
             h.write(string)
-
         return
 
     # Look for last ship of this type
     ship = util_ships.ship_file_name(data['ship'].get('shipName'), data['ship']['name'])
     regexp = re.compile(re.escape(ship) + r'\.\d{4}-\d\d-\d\dT\d\d\.\d\d\.\d\d\.txt')
     oldfiles = sorted([x for x in os.listdir(config.get_str('outdir')) if regexp.match(x)])
     if oldfiles:
-        with (pathlib.Path(config.get_str('outdir')) / oldfiles[-1]).open('r') as h:
+        with (pathlib.Path(config.get_str('outdir')) / oldfiles[-1]).open() as h:
             if h.read() == string:
                 return  # same as last time - don't write
 

outfitting.py

@@ -1,30 +1,37 @@
-"""Code dealing with ship outfitting."""
+"""
+outfitting.py - Code dealing with ship outfitting.
 
-import pickle
+Copyright (c) EDCD, All Rights Reserved
+Licensed under the GNU General Public License.
+See LICENSE file.
+"""
+from __future__ import annotations
+
+import json
 from collections import OrderedDict
-from os.path import join
-from typing import Optional
 from typing import OrderedDict as OrderedDictT
 
 from config import config
-from edmc_data import outfitting_armour_map as armour_map
-from edmc_data import outfitting_cabin_map as cabin_map
-from edmc_data import outfitting_corrosion_rating_map as corrosion_rating_map
-from edmc_data import outfitting_countermeasure_map as countermeasure_map
-from edmc_data import outfitting_fighter_rating_map as fighter_rating_map
-from edmc_data import outfitting_internal_map as internal_map
-from edmc_data import outfitting_misc_internal_map as misc_internal_map
-from edmc_data import outfitting_missiletype_map as missiletype_map
-from edmc_data import outfitting_planet_rating_map as planet_rating_map
-from edmc_data import outfitting_rating_map as rating_map
-from edmc_data import outfitting_standard_map as standard_map
-from edmc_data import outfitting_utility_map as utility_map
-from edmc_data import outfitting_weapon_map as weapon_map
-from edmc_data import outfitting_weaponclass_map as weaponclass_map
-from edmc_data import outfitting_weaponmount_map as weaponmount_map
-from edmc_data import outfitting_weaponoldvariant_map as weaponoldvariant_map
-from edmc_data import outfitting_weaponrating_map as weaponrating_map
-from edmc_data import ship_name_map
+from edmc_data import (
+    outfitting_armour_map as armour_map,
+    outfitting_cabin_map as cabin_map,
+    outfitting_corrosion_rating_map as corrosion_rating_map,
+    outfitting_countermeasure_map as countermeasure_map,
+    outfitting_fighter_rating_map as fighter_rating_map,
+    outfitting_internal_map as internal_map,
+    outfitting_misc_internal_map as misc_internal_map,
+    outfitting_missiletype_map as missiletype_map,
+    outfitting_planet_rating_map as planet_rating_map,
+    outfitting_rating_map as rating_map,
+    outfitting_standard_map as standard_map,
+    outfitting_utility_map as utility_map,
+    outfitting_weapon_map as weapon_map,
+    outfitting_weaponclass_map as weaponclass_map,
+    outfitting_weaponmount_map as weaponmount_map,
+    outfitting_weaponoldvariant_map as weaponoldvariant_map,
+    outfitting_weaponrating_map as weaponrating_map,
+    ship_name_map,
+)
 from EDMCLogging import get_main_logger
 
 logger = get_main_logger()
@@ -33,7 +40,7 @@
 moduledata: OrderedDictT = OrderedDict()
 
 
-def lookup(module, ship_map, entitled=False) -> Optional[dict]:  # noqa: C901, CCR001
+def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR001
     """
     Produce a standard dict description of the given module.
 
@@ -51,7 +58,8 @@ def lookup(module, ship_map, entitled=False) -> Optional[dict]:  # noqa: C901, C
     """
     # Lazily populate
     if not moduledata:
-        moduledata.update(pickle.load(open(join(config.respath_path, 'modules.p'),  'rb')))
+        modules_path = config.respath_path / "resources" / "modules.json"
+        moduledata.update(json.loads(modules_path.read_text()))
 
     if not module.get('name'):
         raise AssertionError(f'{module["id"]}')
@@ -61,10 +69,13 @@ def lookup(module, ship_map, entitled=False) -> Optional[dict]:  # noqa: C901, C
 
     # Armour - e.g. Federation_Dropship_Armour_Grade2
     if name[-2] == 'armour':
-        name = module['name'].lower().rsplit('_', 2)  # Armour is ship-specific, and ship names can have underscores
+        # Armour is ship-specific, and ship names can have underscores
+        ship_name, armour_grade = module["name"].lower().rsplit("_", 2)[0:2]
+        if ship_name not in ship_map:
+            raise AssertionError(f"Unknown ship: {ship_name}")
         new['category'] = 'standard'
-        new['name'] = armour_map[name[2]]
-        new['ship'] = ship_map[name[0]]  # Generate error on unknown ship
+        new["name"] = armour_map[armour_grade]
+        new["ship"] = ship_map[ship_name]
         new['class'] = '1'
         new['rating'] = 'I'
 
@@ -219,10 +230,7 @@ def lookup(module, ship_map, entitled=False) -> Optional[dict]:  # noqa: C901, C
         new['enabled'], new['priority'] = module['on'], module['priority']  # priority is zero-based
 
     # Entitlements
-    if not module.get('sku'):
-        pass
-
-    else:
+    if module.get('sku'):
         new['entitlement'] = module['sku']
 
     # Extra module data
@@ -245,10 +253,11 @@ def lookup(module, ship_map, entitled=False) -> Optional[dict]:  # noqa: C901, C
 
     new.update(moduledata.get(module['name'].lower(), {}))
 
-    # check we've filled out mandatory fields
-    for thing in ['id', 'symbol', 'category', 'name', 'class', 'rating']:  # Don't consider mass etc as mandatory
-        if not new.get(thing):
-            raise AssertionError(f'{module["id"]}: failed to set {thing}')
+    # Check we've filled out mandatory fields
+    mandatory_fields = ["id", "symbol", "category", "name", "class", "rating"]
+    for field in mandatory_fields:
+        if not new.get(field):
+            raise AssertionError(f'{module["id"]}: failed to set {field}')
 
     if new['category'] == 'hardpoint' and not new.get('mount'):
         raise AssertionError(f'{module["id"]}: failed to set mount')
@@ -263,15 +272,15 @@ def export(data, filename) -> None:
     :param data: CAPI data to export.
     :param filename: Filename to export into.
     """
-    assert data['lastSystem'].get('name')
-    assert data['lastStarport'].get('name')
+    assert "name" in data["lastSystem"]
+    assert "name" in data["lastStarport"]
 
     header = 'System,Station,Category,Name,Mount,Guidance,Ship,Class,Rating,FDevID,Date\n'
     rowheader = f'{data["lastSystem"]["name"]},{data["lastStarport"]["name"]}'
 
     with open(filename, 'wt') as h:
         h.write(header)
-        for v in list(data['lastStarport'].get('modules', {}).values()):
+        for v in data["lastStarport"].get("modules", {}).values():
             try:
                 m = lookup(v, ship_name_map)
                 if m: