Unpack ROMs to bins before building (#520)

.github/workflows/ci.yml

@@ -113,8 +113,8 @@ jobs:
         # https://github.com/microsoft/vcpkg/issues/20121
         doNotCache: true
 
-    - name: Download tar.gz.b64
-      run: ./curl_tar_gz_b64.sh
+    - name: Download and unpack ROMs
+      run: ./scripts/download_unpack_roms.sh
 
     - name: Build
       run: python -m pip install --verbose .[test]

.github/workflows/release.yml

@@ -18,8 +18,8 @@ jobs:
     - name: Unzip wheels asset
       run: |
         unzip wheels.zip -d dist
-    - name: Download tar.gz.b64
-      run: ./curl_tar_gz_b64.sh
+    - name: Download and unpack ROMs
+      run: ./scripts/download_unpack_roms.sh
     - name: Publish to PyPi
       uses: pypa/gh-action-pypi-publish@release/v1
       with:

pyproject.toml

@@ -56,7 +56,7 @@ packages = [
   "ale_py.roms",
 ]
 package-dir = {ale_py = "src/python"}
-package-data = {"ale_py" = ["py.typed", "*.pyi", "**/*.pyi"], "ale_py.roms" = ["md5.json", "Roms.tar.gz.b64"]}
+package-data = {"ale_py" = ["py.typed", "*.pyi", "**/*.pyi"], "ale_py.roms" = ["md5.json", "*.bin"]}
 
 [tool.pytest.ini_options]
 minversion = "7.0"

scripts/download_unpack_roms.sh

@@ -0,0 +1,20 @@
+#!/bin/bash
+
+# define some directories
+base_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &> /dev/null && pwd)/.."
+unpack_dir="${base_dir}/unpack_dir"
+target_dir="${base_dir}/src/python/roms"
+
+# make the directory where we will do the unpacking
+mkdir $unpack_dir
+
+# download the ROMs from the git gist, decode it, then unpack it
+curl "https://gist.githubusercontent.com/jjshoots/61b22aefce4456920ba99f2c36906eda/raw/00046ac3403768bfe45857610a3d333b8e35e026/Roms.tar.gz.b64" \
+  | base64 --decode \
+  | tar -xzf - -C $unpack_dir
+
+# move the ROMs out into a roms folder
+mv ${unpack_dir}/ROM/*/*.bin $target_dir
+
+# clean up
+rm -r $unpack_dir

src/python/roms/__init__.py

@@ -1,72 +1,23 @@
 from __future__ import annotations
 
-import base64
 import functools
 import hashlib
 import json
-import tarfile
 import warnings
 from pathlib import Path
 
 
 @functools.lru_cache(maxsize=1)
-def _get_all_rom_hashes() -> dict[str, str]:
+def _get_expected_bin_hashes() -> dict[str, str]:
     # this is a map of {rom.bin : md5 checksum}
     with open(Path(__file__).parent / "md5.json") as f:
         return json.load(f)
 
 
-def _unpack_roms() -> None:
-    """Unpacks all roms from the tar.gz file, then matches it to the expected md5 checksum."""
-    all_roms = _get_all_rom_hashes()
-
-    # load the b64 file, this assumes that the b64 already exists
-    # if you are a dev seeing this, please run `./curl_tar_gz_b64.sh` in the root of this project.
-    with open(Path(__file__).parent / "Roms.tar.gz.b64") as f:
-        tar_gz_b64 = f.read()
-
-    # decode the b64 into the tar.gz and save it
-    tar_gz = base64.b64decode(tar_gz_b64)
-    save_path = Path(__file__).parent / "Roms.tar.gz"
-    with open(save_path, "wb") as f:
-        f.write(tar_gz)
-
-    # iterate through each file in the tar
-    with tarfile.open(name=save_path) as tar_fp:
-        for member in tar_fp.getmembers():
-            # ignore if this is not a valid bin file
-            if not (member.isfile() and member.name.endswith(".bin")):
-                continue
-
-            # grab the rom name from the member name, ie: `pong.bin`
-            # member.name is ROM/rom_name/rom_name.bin, so we just want the last thing
-            rom_name = member.name.split("/")[-1]
-
-            # assert that this member.name should be supported
-            assert (
-                rom_name in all_roms
-            ), f"File {rom_name} not supported. Please report this to a dev."
-
-            # extract the rom file from the archive
-            rom_bytes = tar_fp.extractfile(
-                member
-            ).read()  # pyright: ignore[reportOptionalMemberAccess]
-
-            # get hash
-            md5 = hashlib.md5()
-            md5.update(rom_bytes)
-            md5_hash = md5.hexdigest()
-
-            # assert that the hash matches
-            assert md5_hash == all_roms[rom_name], (
-                f"Rom {rom_name}'s hash ({md5_hash}) does not match what was expected ({all_roms[rom_name]}). "
-                "Please report this to a dev."
-            )
-
-            # save this rom
-            rom_path = Path(__file__).parent / rom_name
-            with open(rom_path, "wb") as rom_fp:
-                rom_fp.write(rom_bytes)
+@functools.lru_cache(maxsize=1)
+def get_all_rom_ids() -> list[str]:
+    """Returns a list of all available rom_ids, ie: ['tetris', 'pong', 'zaxxon', ...]."""
+    return [key.split(".")[0] for key in _get_expected_bin_hashes().keys()]
 
 
 def get_rom_path(name: str) -> Path | None:
@@ -76,20 +27,23 @@ def get_rom_path(name: str) -> Path | None:
     bin_path = Path(__file__).parent / bin_file
 
     # check if it exists within the the hash dictionary
-    all_roms = _get_all_rom_hashes()
-    if bin_file not in all_roms:
+    bin_hashes = _get_expected_bin_hashes()
+    if bin_file not in bin_hashes.keys():
         warnings.warn(f"Rom {name} not supported.")
         return None
 
-    # if the bin_path doesn't exist, TELL SOMEONE PANIC THE WORLD IS ENDING
-    if not bin_path.exists():
-        _unpack_roms()
+    # check the rom hash
+    with open(bin_path, "rb") as bin_fp:
+        md5 = hashlib.md5()
+        md5.update(bin_fp.read())
+        md5_hash = md5.hexdigest()
+
+        if md5_hash != bin_hashes[bin_file]:
+            raise OSError(
+                f"The hash of {bin_file} does not match what was expected!\n"
+                f"Expected ({bin_hashes[bin_file]}),\n"
+                f"Obtained ({md5_hash})."
+            )
 
     # return the path
     return bin_path
-
-
-def get_all_rom_ids() -> list[str]:
-    """Returns a list of all available rom_ids, ie: ['tetris', 'pong', 'zaxxon', ...]."""
-    all_roms = _get_all_rom_hashes()
-    return [key.split(".")[0] for key in all_roms.keys()]