refactor: extract companion extras and the kept filter

scripts/packextras.py

@@ -266,30 +266,44 @@ def _agnostic_scan_extras(
                         "agnostic_scan": True,
                     }
                 )
-                for companion_sha1, companion in _companions(
+                extras.extend(_companion_extras(
-                    path, scan_name, _companion_extensions(profile, fname), files_db
+                    _companions(path, scan_name, _companion_extensions(profile, fname), files_db),
-                ):
+                    folder, extras_prefix, seen_dests, profile, emu_name, f,
-                    companion_dest = f"{folder}/{companion['name']}" if folder else companion["name"]
+                ))
-                    companion_full = (
+    return extras
-                        f"{extras_prefix}/{companion_dest}" if extras_prefix else companion_dest
+
-                    )
+
-                    if companion_full in seen_dests:
+def _companion_extras(
-                        continue
+    companions: list[tuple[str, dict]],
-                    seen_dests.add(companion_full)
+    folder: str,
-                    extras.append({
+    extras_prefix: str,
-                        "name": companion["name"],
+    seen_dests: set[str],
-                        "destination": companion_dest,
+    profile: dict,
-                        "sha1": companion_sha1,
+    emu_name: str,
-                        "required": False,
+    seed: dict,
-                        "hle_fallback": False,
+) -> list[dict]:
-                        "source_emulator": profile.get("emulator", emu_name),
+    """Extras for the companions of one scanned image, placed beside it."""
-                        "source_profile": emu_name,
+    extras: list[dict] = []
-                        "source_system": f.get("system"),
+    for companion_sha1, companion in companions:
-                        "source_systems": list(profile.get("systems", [])),
+        companion_dest = f"{folder}/{companion['name']}" if folder else companion["name"]
-                        "region": f.get("region"),
+        companion_full = f"{extras_prefix}/{companion_dest}" if extras_prefix else companion_dest
-                        "variant_group": f.get("variant_group"),
+        if companion_full in seen_dests:
-                        "agnostic_scan": True,
+            continue
-                    })
+        seen_dests.add(companion_full)
+        extras.append({
+            "name": companion["name"],
+            "destination": companion_dest,
+            "sha1": companion_sha1,
+            "required": False,
+            "hle_fallback": False,
+            "source_emulator": profile.get("emulator", emu_name),
+            "source_profile": emu_name,
+            "source_system": seed.get("system"),
+            "source_systems": list(profile.get("systems", [])),
+            "region": seed.get("region"),
+            "variant_group": seed.get("variant_group"),
+            "agnostic_scan": True,
+        })
     return extras
 
 
@@ -615,6 +629,13 @@ def _extra_system_ids(extra: dict) -> list[str]:
         return [str(explicit)]
     return [str(value) for value in extra.get("source_systems", []) if value]
 
+def _kept(entries: list[dict], required_only: bool) -> list[dict]:
+    """The entries a pack keeps: optional ones go under --required-only."""
+    if not required_only:
+        return entries
+    return [e for e in entries if e.get("required") is not False]
+
+
 def platform_region_groups(
     config: dict,
     systems: dict,
@@ -639,11 +660,9 @@ def platform_region_groups(
     groups: dict[str, list[tuple[str, str]]] = {}
     for sys_id, system in systems.items():
         members = groups.setdefault(sys_id, [])
-        for file_entry in system.get("files", []):
+        # Decided over what the pack keeps: an optional winner removed
-            if required_only and file_entry.get("required") is False:
+        # afterwards left its slot empty.
-                # Decided over what the pack keeps: an optional winner
+        for file_entry in _kept(system.get("files", []), required_only):
-                # removed afterwards left its slot empty.
-                continue
             dest = sanitize_pack_path(
                 file_entry.get("destination", file_entry.get("name", ""))
             )
@@ -654,7 +673,7 @@ def platform_region_groups(
     if not include_extras or db is None:
         return groups, extra_dests
 
-    for extra in _collect_emulator_extras(
+    extras = _collect_emulator_extras(
         config,
         emulators_dir,
         db,
@@ -663,9 +682,8 @@ def platform_region_groups(
         emu_profiles,
         target_cores=target_cores,
         include_all=include_all,
-    ):
+    )
-        if required_only and extra.get("required") is False:
+    for extra in _kept(extras, required_only):
-            continue
         dest = sanitize_pack_path(extra.get("destination", extra.get("name", "")))
         if not dest:
             continue

tests/test_region_required_only.py

@@ -8,6 +8,7 @@ the optional winner: the slot shipped empty.
 
 from __future__ import annotations
 
+import ast
 import sys
 import unittest
 from pathlib import Path
@@ -30,8 +31,15 @@ class RequiredOnlyBeforeRegion(unittest.TestCase):
         self.assertEqual(groups["vic20"], [("Vic20/kernel-ntsc.bin", "kernel-ntsc.bin")])
 
     def test_every_caller_passes_it(self):
-        source = (REPO_ROOT / "scripts" / "generate_pack.py").read_text(encoding="utf-8")
+        tree = ast.parse((REPO_ROOT / "scripts" / "generate_pack.py").read_text(encoding="utf-8"))
-        self.assertEqual(source.count("required_only=required_only,\n        )"), 2)
+        calls = [
+            node for node in ast.walk(tree)
+            if isinstance(node, ast.Call)
+            and getattr(node.func, "id", None) == "platform_region_groups"
+        ]
+        self.assertGreaterEqual(len(calls), 2)
+        for call in calls:
+            self.assertIn("required_only", {kw.arg for kw in call.keywords})
 
 
 if __name__ == "__main__":