From cbe769e74c133baeb70a3c028da3518636928e3d Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:12:56 +0200 Subject: [PATCH] fix: read all RetroPie helps, propose only its own Eight scriptmodules write "bios" in lower case (xm7, np2pi, quasi88, minivmac, px68k...) and none was examined. A module reported "left as is" for a name no profile documents was written all the same: the skip had no continue. The cap of six names came from that case-sensitive reading, where xm7 writes thirteen: a sentence now grows no longer than the longest RetroPie writes. lr- was stripped whatever the build, so the standalone minivmac package matched the libretro profile, which reads another file. A file of a system RetroPie lacks is proposed to no package: lr-geargrafx adds pcengine only (lr-geargrafx.sh:41-42) and was given the LaserActive's pac-n1.bin and pac-n10.bin. --- scripts/exporter/retropie_exporter.py | 52 ++++++++++----- tests/test_export_counts.py | 92 ++++++++++++++++++++++++++- tests/test_exporters.py | 6 +- 3 files changed, 131 insertions(+), 19 deletions(-) diff --git a/scripts/exporter/retropie_exporter.py b/scripts/exporter/retropie_exporter.py index a3806137..de622019 100644 --- a/scripts/exporter/retropie_exporter.py +++ b/scripts/exporter/retropie_exporter.py @@ -37,7 +37,6 @@ EMULATORS_DIR = Path(__file__).resolve().parents[2] / "emulators" # The longest list RetroPie writes is six names (lr-atari800). Past that the # sentence stops being something a reader uses, so the names are reported # instead of appended. -MAX_NAMES = 6 _MODULE_ID = re.compile(r'rp_module_id="([^"]+)"') _MODULE_HELP = re.compile(r'rp_module_help="((?:[^"\\]|\\.)*)"') @@ -48,8 +47,8 @@ _ABBREVIATION = re.compile(r"[a-z]\.[a-z]", re.I) # a.bin and b.bin to $biosdir", "The Sega CD requires the BIOS files a.bin, # b.bin copied to $biosdir"), so the clause is found by the word BIOS rather # than by a sentence template, and the names in it are the list to extend. -_BIOS_CLAUSE = re.compile(r"BIOS\b.*?(?=\\n|$)", re.DOTALL) -_BIOS_NOUN = re.compile(r"BIOS (files?)\b") +_BIOS_CLAUSE = re.compile(r"BIOS\b.*?(?=\\n|$)", re.DOTALL | re.I) +_BIOS_NOUN = re.compile(r"(BIOS) (files?)\b", re.I) _SEPARATOR = re.compile(r"\s*(?:,\s*(?:and\s+)?|and\s+)") _EXAMPLES = re.compile(r"\b(?:e\.?g\b|for example|for instance|such as|like)\W*$", re.I) @@ -99,17 +98,28 @@ class Exporter(BaseExporter): if handle is None: continue text = handle.read().decode("utf-8", errors="replace") - if "BIOS" in text: + # xm7, np2pi, quasi88 and five more write "bios". + if "bios" in text.lower(): found[relative] = text return found - def _core_index(self) -> dict[str, str]: - """Module id (without its lr- prefix) to the profile it stands for.""" - index: dict[str, str] = {} + def _core_index(self) -> dict[str, dict[str, str]]: + """Module id (without its lr- prefix) to the profile it stands for, + per build. + + An lr- package is a libretro core and a bare one a standalone + emulator: the standalone minivmac package matched the libretro + profile, which reads MacII.ROM where Mini vMac 37 reads vMac.ROM. + """ + index: dict[str, dict[str, str]] = {"libretro": {}, "standalone": {}} for key, profile in load_emulator_profiles(str(EMULATORS_DIR)).items(): - index[key.replace("-", "_").lower()] = key - for name in profile.get("cores", []) or []: - index[str(name).replace("-", "_").lower()] = key + kind = str(profile.get("type", "")) + for build in ("libretro", "standalone"): + if build not in kind: + continue + index[build][key.replace("-", "_").lower()] = key + for name in profile.get("cores", []) or []: + index[build][str(name).replace("-", "_").lower()] = key return index @staticmethod @@ -269,7 +279,7 @@ class Exporter(BaseExporter): continue module = _MODULE_ID.search(text) help_text = _MODULE_HELP.search(text) - if not module or not help_text or "BIOS" not in help_text.group(1): + if not module or not help_text or "bios" not in help_text.group(1).lower(): continue found[relative] = (module.group(1), help_text.group(1)) return found @@ -289,6 +299,9 @@ class Exporter(BaseExporter): ) index = self._core_index() + # A sentence grows no longer than the longest RetroPie writes itself: + # past that it stops reading as one, and the names are reported. + self._longest = max(len(self._listed(help_text)) for _, help_text in modules.values()) by_core = self._files_by_core(systems) # Aliases count as names we know: RetroPie writes dc_flash.bin where # flycast's profile files it as an alias of another primary. @@ -306,7 +319,8 @@ class Exporter(BaseExporter): self._skipped.setdefault(reason, []).append(module) for relative, (module_id, help_text) in sorted(modules.items()): - core = index.get(module_id.removeprefix("lr-").replace("-", "_").lower()) + build = "libretro" if module_id.startswith("lr-") else "standalone" + core = index[build].get(module_id.removeprefix("lr-").replace("-", "_").lower()) files = by_core.get(core or "", []) if not files: skip("which no profiled core matches", module_id) @@ -319,13 +333,17 @@ class Exporter(BaseExporter): f"whose help text names a file no profile documents yet ({', '.join(unknown)})", module_id, ) + continue # A file already listed under one of its other names is not # missing: proposing the primary would name the same bytes twice. + # A system RetroPie does not have is no package's: lr-geargrafx + # adds pcengine only, not the LaserActive its core also reads. candidates = [ fe for fe in files - if self._required_for(fe, core) + if not fe.foreign_system + and self._required_for(fe, core) and not ( {fe.name.lower()} | {str(a).lower() for a in (fe.native("aliases", []) or [])} @@ -350,13 +368,13 @@ class Exporter(BaseExporter): skip("whose help text lists no files to add to", module_id) continue - if len(listed) + len(missing) > MAX_NAMES: + if len(listed) + len(missing) > self._longest: skip("whose help text would grow longer than any it has today", module_id) continue new_help = self._extended(help_text, enumeration, missing) if len(listed) + len(missing) > 1: - new_help = _BIOS_NOUN.sub("BIOS files", new_help, count=1) + new_help = _BIOS_NOUN.sub(r"\1 files", new_help, count=1) produced[relative] = originals[relative].replace( f'rp_module_help="{help_text}"', f'rp_module_help="{new_help}"', @@ -394,10 +412,10 @@ class Exporter(BaseExporter): if not help_text: issues.append(f"{relative}: the help string is not closed") continue - if "BIOS" not in help_text.group(1): + if "bios" not in help_text.group(1).lower(): issues.append(f"{relative}: the BIOS sentence is gone") names = self._listed(help_text.group(1)) - if len(names) > MAX_NAMES: + if len(names) > getattr(self, "_longest", len(names)): issues.append( f"{relative}: {len(names)} names, past what the help enumerates" ) diff --git a/tests/test_export_counts.py b/tests/test_export_counts.py index a333788f..03f9880e 100644 --- a/tests/test_export_counts.py +++ b/tests/test_export_counts.py @@ -18,7 +18,7 @@ from pathlib import Path REPO_ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(REPO_ROOT / "scripts")) -from exporter.baseline import NativeFile, NativeSystem, build_native_model # noqa: E402 +from exporter.baseline import NativeFile, NativeSystem, Report, build_native_model # noqa: E402 from exporter.bizhawk_exporter import Exporter as BizHawk # noqa: E402 from exporter.misterfpga_exporter import Exporter as Mister # noqa: E402 from exporter.romm_exporter import Exporter as Romm # noqa: E402 @@ -610,6 +610,96 @@ class RetroPieProposals(unittest.TestCase): self.assertFalse(RetroPie._required_for(installed, "gngeopi")) self.assertTrue(RetroPie._required_for(brought, "gngeopi")) + @staticmethod + def _module(module_id: str, help_text: str) -> dict[str, str]: + script = f'rp_module_id="{module_id}"\nrp_module_help="{help_text}"\n' + return {f"scriptmodules/emulators/{module_id}.sh": script} + + @staticmethod + def _wanting(core: str, *names: str) -> dict: + files = [NativeFile(n, n, "sys", truth={ + "required": True, "_required_by": [core], "_cores": [core], + "_brought_by": [f"{core}:standalone"]}) for n in names] + return {"sys": NativeSystem("sys", files=files)} + + def test_a_lowercase_bios_sentence_is_read(self): + """Eight scriptmodules write "bios" in lower case (xm7, np2pi, + quasi88, minivmac, px68k...) and none of them was examined.""" + module = self._module("xm7", "Copy bios files a.rom and b.rom to $biosdir/xm7") + self.assertEqual(len(RetroPie().modules(module)), 1) + + def test_a_module_left_as_is_is_not_written(self): + """minivmac was reported "left as is" for a name no profile documents + and written all the same: the skip had no continue after it.""" + from unittest import mock + + exporter = RetroPie() + module = self._module("vmac", "Copy the required BIOS file vMac.ROM to $biosdir") + with mock.patch.object(RetroPie, "_core_index", + return_value={"standalone": {"vmac": "vmac"}, "libretro": {}}): + produced = exporter.render(self._wanting("vmac", "MacII.ROM"), Report(), module) + self.assertEqual(produced, {}) + + def test_a_standalone_package_does_not_borrow_a_libretro_profile(self): + """lr- was stripped whatever the build: RetroPie's standalone + minivmac package matched the libretro profile, which reads + MacII.ROM where Mini vMac 37 reads vMac.ROM.""" + from unittest import mock + + module = self._module("minivmac", "Copy the required BIOS file vMac.ROM to $biosdir") + index = {"libretro": {"minivmac": "minivmac"}, "standalone": {}} + with mock.patch.object(RetroPie, "_core_index", return_value=index): + exporter = RetroPie() + produced = exporter.render(self._wanting("minivmac", "MacII.ROM"), Report(), module) + self.assertEqual(produced, {}) + self.assertIn("minivmac", sum(exporter._skipped.values(), [])) + + def test_a_list_is_capped_by_retropies_longest_not_a_constant(self): + """The cap of six came from the longest list read, case-sensitively: + xm7 writes thirteen names.""" + from unittest import mock + + names = [f"n{i}.rom" for i in range(8)] + modules = { + **self._module("long", "Copy bios files " + ", ".join(names[:7]) + " and " + + names[7] + " to $biosdir"), + **self._module("short", "Copy the required BIOS files a.rom and b.rom to $biosdir"), + } + index = {"standalone": {"short": "short"}, "libretro": {}} + with mock.patch.object(RetroPie, "_core_index", return_value=index): + exporter = RetroPie() + produced = exporter.render( + self._wanting("short", "a.rom", "b.rom", "c.rom", "d.rom", "e.rom", + "f.rom", "g.rom"), Report(), modules) + self.assertEqual(len(produced), 1) + self.assertEqual(exporter.validate({}, produced), []) + + def test_a_file_of_a_system_retropie_lacks_is_not_proposed(self): + """lr-geargrafx adds pcengine only (addSystem, lr-geargrafx.sh:42), + and its sentence was extended with the LaserActive's pac-n1.bin and + pac-n10.bin, a system RetroPie does not have.""" + from unittest import mock + + files = [NativeFile(n, n, sys_id, truth={ + "required": True, "_required_by": ["geargrafx"], "_cores": ["geargrafx"], + "_brought_by": ["geargrafx:libretro"]}, foreign_system=foreign) + for n, sys_id, foreign in (("gexpress.pce", "pce", False), + ("pac-n1.bin", "laseractive", True))] + listed = NativeFile("syscard3.pce", "syscard3.pce", "pce", platform={"name": "syscard3.pce"}) + systems = {"pce": NativeSystem("pce", files=[listed, files[0]]), + "laseractive": NativeSystem("laseractive", files=[files[1]])} + module = { + **self._module("lr-geargrafx", "Copy the required BIOS file syscard3.pce to $biosdir"), + **self._module("xm7", "Copy bios files a.rom, b.rom and c.rom to $biosdir"), + } + with mock.patch.object(RetroPie, "_core_index", + return_value={"libretro": {"geargrafx": "geargrafx"}, + "standalone": {}}): + produced = RetroPie().render(systems, Report(), module) + (text,) = produced.values() + self.assertIn("gexpress.pce", text) + self.assertNotIn("pac-n1.bin", text) + def test_a_list_of_alternatives_is_not_extended(self): alternatives = "Copy the required BIOS file a.rom or b.rom to $biosdir" enumeration = "Copy the required BIOS files a.rom and b.rom to $biosdir" diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 2c3734d1..15bdb97d 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -656,9 +656,13 @@ class SearchOrder(unittest.TestCase): script = ('rp_module_id="lr-picodrive"\nrp_module_help="The Sega CD requires the ' 'BIOS files us_scd1_9210.bin, eu_mcd1_9210.bin copied to $biosdir"\n') exporter = discover_exporters()["retropie"]() + # The cap is the longest list RetroPie writes: xm7 names thirteen. + longest = ('rp_module_id="xm7"\nrp_module_help="Copy bios files ' + + ", ".join(f"n{i}.rom" for i in range(12)) + ' and n12.rom to $biosdir"\n') produced = exporter.render( {"segacd": NativeSystem("segacd", files=files)}, None, - {"scriptmodules/libretrocores/lr-picodrive.sh": script}, + {"scriptmodules/libretrocores/lr-picodrive.sh": script, + "scriptmodules/emulators/xm7.sh": longest}, )["scriptmodules/libretrocores/lr-picodrive.sh"] self.assertIn("us_scd1_9210.bin, eu_mcd1_9210.bin and jp_mcd2_921222.bin copied", produced)