Merge pull request #903 from martin2844/martin2844/fix-blue-rom-setup

Fix Blue ROM source setup

.github/workflows/ci.yml

@@ -383,6 +383,9 @@ jobs:
       - name: install luajit
         run: sudo apt-get update && sudo apt-get install -y luajit
 
+      - name: install Pillow
+        run: python3 -m pip install --upgrade pillow
+
       - name: interpreter version
         run: luajit -v
 

scripts/run.sh

@@ -1,8 +1,8 @@
 #!/usr/bin/env bash
 # Run the LÖVE2D Pokémon Red port (macOS-friendly).
 #
-# Assumes scripts/setup.sh has been run once (generated data present and
-# LÖVE installed).  Extra arguments are passed through to LÖVE.
+# Assumes scripts/setup.sh has been run once for at least one game (generated
+# data present and LÖVE installed). Extra arguments are passed through to LÖVE.
 #
 # Link play is peer-to-peer (lua-enet, bundled with LÖVE): one player
 # picks HOST A GAME in START > LINK and reads out the address shown;
@@ -15,8 +15,11 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd)"
 
 fail() { printf '\033[1;31merror:\033[0m %s\n' "$*" >&2; exit 1; }
 
-[ -f "$ROOT/data/generated/maps.lua" ] \
-  || fail "generated data missing,  run scripts/setup.sh first"
+if [ ! -f "$ROOT/data/generated/maps.lua" ] \
+    && [ ! -f "$ROOT/blue/data/generated/maps.lua" ] \
+    && [ ! -f "$ROOT/yellow/data/generated/maps.lua" ]; then
+  fail "generated data missing,  run scripts/setup.sh first"
+fi
 
 find_love() {
   command -v love >/dev/null 2>&1 && { echo "love"; return; }

scripts/test.sh

@@ -66,6 +66,7 @@ run_tier() {
 
 # ------- ROM-free tiers: these are what CI runs
 
+run_tier "T0 ROM builder version routing" python3 tests/build_rom_data_cli_test.py
 run_tier "T0 switch CI workflow content gate" "$LUA" tests/switch_ci_workflows_test.lua
 run_tier "T0 switch transfer docs gate" "$LUA" tests/switch_transfer_docs_test.lua
 # NX Blue/Yellow asset overlay: ROM-free, must run on every checkout so a

src/import/RomImporter.lua

@@ -156,18 +156,22 @@ local function allRequiredFilesExist(version)
   return ok
 end
 
--- A developer checkout / Python build leaves Red's generated data in the
--- physfs SOURCE at the un-prefixed root (the checked-out data/generated and
--- assets/generated); it is always current and never moves into red/.  Only
--- Red ships this way (Blue/Yellow are import-only).  The check goes through
--- love.filesystem directly so the red/ cache prefix cannot hide the source
--- tree, and the realDirectory test keeps a save-dir cache from counting.
-local function sourceTreeHasData()
+-- A developer checkout / Python build leaves generated data in the physfs
+-- source: Red at the historical root, Blue/Yellow in their versioned trees.
+-- Imported Red caches still live under red/.  Check source paths directly so
+-- that cache prefix cannot hide Red's source tree, and keep save-dir caches
+-- from counting as current source data.
+local function sourceTreeHasData(version)
   if not love.filesystem.getRealDirectory then return false end
+  local prefix = version == "red" and "" or GameVersion.cachePrefix(version)
   for _, path in ipairs(REQUIRED_FILES) do
-    if love.filesystem.getInfo(path, "file") == nil then return false end
+    if love.filesystem.getInfo(prefix .. path, "file") == nil then return false end
   end
-  local real = love.filesystem.getRealDirectory(REQUIRED_FILES[1])
+  for _, path in ipairs(VERSION_REQUIRED_FILES[version] or {}) do
+    if love.filesystem.getInfo(prefix .. path, "file") == nil then return false end
+  end
+  local path = prefix .. REQUIRED_FILES[1]
+  local real = love.filesystem.getRealDirectory(path)
   return real == love.filesystem.getSource()
 end
 
@@ -244,10 +248,8 @@ function RomImporter.isReady(version)
     -- save-directory copy that would otherwise shadow it at runtime.
     purgeSaveDirCache()
   end
-  -- Red generated data in the physfs source (developer checkout / Python
-  -- build) is always current; Blue is import-only and falls through to the
-  -- version-marker gate.
-  if version == "red" and sourceTreeHasData() then return true end
+  -- Generated data in a developer checkout / Python build is always current.
+  if sourceTreeHasData(version) then return true end
   local saved = CacheFs.prefix
   CacheFs.prefix = GameVersion.cachePrefix(version)
   local marker = CacheFs.read(MARKER_PATH)

tests/build_rom_data_cli_test.py

@@ -0,0 +1,65 @@
+#!/usr/bin/env python3
+"""ROM-free regression tests for source-build version detection and routing."""
+
+from contextlib import redirect_stderr, redirect_stdout
+from io import StringIO
+from pathlib import Path
+from types import SimpleNamespace
+from unittest import TestCase, main, mock
+
+import sys
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "tools"))
+import build_rom_data  # noqa: E402
+
+
+class BuildRomDataCliTest(TestCase):
+    def run_builder(self, sha1, *extra):
+        manifest = {"romSha1": sha1, "symbols": {}}
+        rom = SimpleNamespace(sha1=sha1)
+        with mock.patch.object(build_rom_data, "RomImage", return_value=rom), \
+                mock.patch.object(build_rom_data, "load_manifest", return_value=manifest), \
+                mock.patch.object(build_rom_data, "build") as build, \
+                mock.patch.object(build_rom_data.os, "makedirs"), \
+                redirect_stdout(StringIO()), redirect_stderr(StringIO()):
+            result = build_rom_data.main([
+                "--rom", "fixture.gb", "--only", "constants", *extra])
+        return result, build
+
+    def test_blue_rom_selects_blue_manifest_and_cache_paths(self):
+        result, build = self.run_builder(
+            build_rom_data.CANONICAL_BLUE_SHA1)
+
+        self.assertEqual(result, 0)
+        args = build.call_args.args
+        self.assertEqual(args[3], "blue/data/generated")
+        self.assertEqual(args[4], "blue/assets/generated")
+
+    def test_red_rom_keeps_historical_root_paths(self):
+        result, build = self.run_builder(build_rom_data.CANONICAL_RED_SHA1)
+
+        self.assertEqual(result, 0)
+        args = build.call_args.args
+        self.assertEqual(args[3], "data/generated")
+        self.assertEqual(args[4], "assets/generated")
+
+    def test_explicit_output_paths_are_preserved(self):
+        result, build = self.run_builder(
+            build_rom_data.CANONICAL_YELLOW_SHA1,
+            "--out", "/tmp/custom-data", "--assets", "/tmp/custom-assets")
+
+        self.assertEqual(result, 0)
+        args = build.call_args.args
+        self.assertEqual(args[3], "/tmp/custom-data")
+        self.assertEqual(args[4], "/tmp/custom-assets")
+
+    def test_unknown_rom_is_rejected_before_build(self):
+        unknown = "0" * 40
+        result, build = self.run_builder(unknown)
+
+        self.assertEqual(result, 1)
+        build.assert_not_called()
+
+
+if __name__ == "__main__":
+    main()

tools/build_rom_data.py

@@ -92,6 +92,17 @@ def version_for_manifest(manifest, requested_version=None, manifest_explicit=Fal
     return detected or requested_version or "red"
 
 
+def detect_rom_version(path):
+    """Read a canonical ROM once and return its supported game version."""
+    rom = RomImage(path, None)
+    version = SHA1_TO_VERSION.get(rom.sha1)
+    if version is None:
+        expected = ", ".join(VERSION_SHA1[name] for name in VERSION_MANIFESTS)
+        raise ValueError(
+            f"unsupported ROM SHA-1 {rom.sha1}; expected one of {expected}")
+    return version, rom
+
+
 def extract_constants(manifest, out_dir):
     data = manifest["constants"]
     util.write_lua(
@@ -2086,48 +2097,73 @@ def build(rom, symbols, manifest, out_dir, assets_dir, datasets):
     return results
 
 
-def main():
+def main(argv=None):
     parser = argparse.ArgumentParser(description=__doc__)
     parser.add_argument(
         "--rom", required=True,
         help="canonical US Pokemon Red, Blue, or Yellow ROM")
     parser.add_argument(
-        "--version", choices=sorted(VERSION_MANIFESTS), default="red",
-        help="select the shipped manifest for this version (default: red)")
+        "--version", choices=["auto", *sorted(VERSION_MANIFESTS)], default="auto",
+        help="select the shipped manifest for this version (default: detect from ROM)")
     parser.add_argument(
         "--manifest", default=None,
         help="explicit manifest path (overrides --version default path; "
              "RomImage hash still comes from the file's romSha1)")
-    parser.add_argument("--out", default="data/generated")
-    parser.add_argument("--assets", default="assets/generated")
+    parser.add_argument(
+        "--out", default=None,
+        help="generated data directory (default: version-specific cache path)")
+    parser.add_argument(
+        "--assets", default=None,
+        help="generated assets directory (default: version-specific cache path)")
     parser.add_argument("--clean", action="store_true")
     parser.add_argument(
         "--only", action="append", choices=DATASETS,
         help="build one dataset (repeatable); default builds all implemented")
-    args = parser.parse_args()
+    args = parser.parse_args(argv)
 
     try:
         manifest_explicit = args.manifest is not None
-        manifest_path = resolve_manifest_path(args.version, args.manifest)
-        manifest = load_manifest(manifest_path)
-        version = version_for_manifest(
-            manifest, args.version, manifest_explicit=manifest_explicit)
-        expected_sha1 = manifest.get("romSha1") or VERSION_SHA1[version]
-        rom = RomImage(args.rom, expected_sha1)
+        requested_version = None if args.version == "auto" else args.version
+        if manifest_explicit:
+            manifest_path = resolve_manifest_path(
+                requested_version or "red", args.manifest)
+            manifest = load_manifest(manifest_path)
+            version = version_for_manifest(
+                manifest, requested_version, manifest_explicit=True)
+            expected_sha1 = manifest.get("romSha1") or VERSION_SHA1[version]
+            rom = RomImage(args.rom, expected_sha1)
+        elif requested_version is None:
+            version, rom = detect_rom_version(args.rom)
+            manifest_path = resolve_manifest_path(version, None)
+            manifest = load_manifest(manifest_path)
+            expected_sha1 = manifest.get("romSha1") or VERSION_SHA1[version]
+            if rom.sha1 != expected_sha1:
+                raise ValueError(
+                    f"unsupported ROM SHA-1 {rom.sha1}; expected {expected_sha1}")
+        else:
+            version = requested_version
+            manifest_path = resolve_manifest_path(version, None)
+            manifest = load_manifest(manifest_path)
+            version = version_for_manifest(manifest, version)
+            expected_sha1 = manifest.get("romSha1") or VERSION_SHA1[version]
+            rom = RomImage(args.rom, expected_sha1)
         symbols = SymbolTable(manifest["symbols"])
     except (OSError, ValueError, KeyError, json.JSONDecodeError) as exc:
         print(f"error: {exc}", file=sys.stderr)
         return 1
 
+    prefix = "" if version == "red" else version + os.sep
+    out_dir = args.out or prefix + os.path.join("data", "generated")
+    assets_dir = args.assets or prefix + os.path.join("assets", "generated")
     if args.clean:
-        for path in (args.out, args.assets):
+        for path in (out_dir, assets_dir):
             if os.path.isdir(path):
                 shutil.rmtree(path)
-    os.makedirs(args.out, exist_ok=True)
-    os.makedirs(args.assets, exist_ok=True)
+    os.makedirs(out_dir, exist_ok=True)
+    os.makedirs(assets_dir, exist_ok=True)
     datasets = tuple(args.only) if args.only else DATASETS
     try:
-        build(rom, symbols, manifest, args.out, args.assets, datasets)
+        build(rom, symbols, manifest, out_dir, assets_dir, datasets)
     except (ValueError, KeyError, IndexError) as exc:
         print(f"error: {exc}", file=sys.stderr)
         return 1