From 2e29b3be03816f8308309fb07b8748dcd3b97cfb Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:10:54 +0200 Subject: [PATCH] fix: read recalbox mandatory per named core An element lists its cores. Its mandatory flag now follows whether those cores need the file, and stays as written where none of them reads it. cdi/cdibios.zip keeps mandatory=false (same_cdi does not read it), the two amiberry kickstarts keep theirs, saturn_bios.bin becomes optional for the three HLE cores it names. --- exports/recalbox.json | 40 +++++++------------------ exports/recalbox/es_bios.xml | 8 ++--- scripts/export_native.py | 30 ++++++++++--------- scripts/exporter/base_exporter.py | 8 +++++ scripts/exporter/recalbox_exporter.py | 42 +++++++++++++++++++++++++-- tests/test_exporters.py | 28 ++++++++++++++++++ 6 files changed, 108 insertions(+), 48 deletions(-) diff --git a/exports/recalbox.json b/exports/recalbox.json index b5f738f4..b8d10259 100644 --- a/exports/recalbox.json +++ b/exports/recalbox.json @@ -7,7 +7,7 @@ "files": [ "es_bios.xml" ], - "summary": "357 kept, 87 added, 2 hashes corrected, 129 requirements corrected, 528 additions left out, 102 entries found by content not listed", + "summary": "357 kept, 87 added, 2 hashes corrected, 127 requirements corrected, 528 additions left out, 102 entries found by content not listed", "counts": { "kept": 357, "added": 87, @@ -15,15 +15,6 @@ "not_kept": 0 }, "changes": [ - { - "kind": "requirement_corrected", - "system": "amiga600", - "name": "kick33180.A500", - "destination": "amiga/bios/kick33180.A500", - "field": "required", - "old": true, - "new": false - }, { "kind": "requirement_corrected", "system": "amiga600", @@ -318,15 +309,6 @@ "required": false } }, - { - "kind": "requirement_corrected", - "system": "amiga1200", - "name": "kick39106.A1200", - "destination": "amiga/bios/kick39106.A1200", - "field": "required", - "old": true, - "new": false - }, { "kind": "requirement_corrected", "system": "amiga1200", @@ -1923,6 +1905,15 @@ "old": true, "new": false }, + { + "kind": "requirement_corrected", + "system": "saturn", + "name": "saturn_bios.bin", + "destination": "saturn/saturn_bios.bin", + "field": "required", + "old": true, + "new": false + }, { "kind": "requirement_corrected", "system": "saturn", @@ -2205,15 +2196,6 @@ "required": false } }, - { - "kind": "requirement_corrected", - "system": "cdi", - "name": "cdibios.zip", - "destination": "cdi/cdibios.zip", - "field": "required", - "old": false, - "new": true - }, { "kind": "added", "system": "scummvm", @@ -2247,7 +2229,7 @@ "sizes_corrected": [], "sizes_removed": [], "hashes_cleared": [], - "requirements_corrected": 129, + "requirements_corrected": 127, "withheld": [], "found_by_content": 102, "removal_candidates": [ diff --git a/exports/recalbox/es_bios.xml b/exports/recalbox/es_bios.xml index ee5b9e89..d3418b98 100644 --- a/exports/recalbox/es_bios.xml +++ b/exports/recalbox/es_bios.xml @@ -1,7 +1,7 @@ - + @@ -32,7 +32,7 @@ - + @@ -579,7 +579,7 @@ - + @@ -688,7 +688,7 @@ - + diff --git a/scripts/export_native.py b/scripts/export_native.py index 629eb03f..e5f5f40a 100644 --- a/scripts/export_native.py +++ b/scripts/export_native.py @@ -509,7 +509,7 @@ def _added_fields(exporter: object, entry: object) -> dict: if "size" in carried and entry.size() is not None: written["size"] = entry.size() if "required" in carried: - written["required"] = entry.required + written["required"] = exporter.required_value(entry) return written @@ -535,20 +535,24 @@ def landed_changes(exporter: object, systems: dict) -> LandedChanges: "added", entry, None, None, _added_fields(exporter, entry) )) continue + if ( + (entry.truth or {}).get("required") is not None + and entry.platform.get("required") is not None + and exporter.states(entry, "required") + and exporter.required_value(entry) != bool(entry.platform["required"]) + ): + records.append(_change( + "requirement_corrected", entry, "required", + bool(entry.platform["required"]), exporter.required_value(entry), + )) for field_name in entry.corrections: - if not exporter.states(entry, field_name): + if field_name == "required" or not exporter.states(entry, field_name): continue - if field_name == "required": - records.append(_change( - "requirement_corrected", entry, "required", - bool(entry.platform.get("required")), entry.required, - )) - else: - records.append(_change( - "hash_corrected", entry, field_name, - _platform_value(entry, field_name), - exporter.written(entry, field_name), - )) + records.append(_change( + "hash_corrected", entry, field_name, + _platform_value(entry, field_name), + exporter.written(entry, field_name), + )) records.extend( _change("hash_filled", entry, field_name, None, exporter.written(entry, field_name)) diff --git a/scripts/exporter/base_exporter.py b/scripts/exporter/base_exporter.py index b91289a0..020f6a2e 100644 --- a/scripts/exporter/base_exporter.py +++ b/scripts/exporter/base_exporter.py @@ -200,6 +200,14 @@ class BaseExporter(ABC): return self.single_value(fe, field_name) return ",".join(fe.hashes(field_name)) + def required_value(self, fe: NativeFile) -> bool: + """The requirement the render writes for an entry. + + The model's flag is true when any core needs the file. A format + that names the cores of each entry narrows it to those cores. + """ + return fe.required + def states(self, fe: NativeFile, field_name: str) -> bool: """Whether the written file carries this entry's corrected field. diff --git a/scripts/exporter/recalbox_exporter.py b/scripts/exporter/recalbox_exporter.py index da43fc82..0647bcb8 100644 --- a/scripts/exporter/recalbox_exporter.py +++ b/scripts/exporter/recalbox_exporter.py @@ -20,7 +20,7 @@ from xml.sax.saxutils import quoteattr sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) -from common import parse_untrusted_xml +from common import load_emulator_profiles, parse_untrusted_xml from .base_exporter import BaseExporter from .baseline import NativeFile, NativeSystem, Report @@ -29,6 +29,8 @@ SOURCE_URL = ( "https://gitlab.com/recalbox/recalbox/-/raw/master/board/recalbox/fsoverlay" "/recalbox/share_init/system/.emulationstation/es_bios.xml" ) +# Resolved from the file: the profiles say which names each core answers to. +EMULATORS_DIR = Path(__file__).resolve().parents[2] / "emulators" SCHEMA_URL = ( "https://gitlab.com/recalbox/recalbox/-/raw/master/board/recalbox/fsoverlay" "/recalbox/share_init/system/.emulationstation/es_bios.xsd" @@ -149,12 +151,48 @@ class Exporter(BaseExporter): return path return fe.destination or fe.name + def _profile_keys(self) -> dict[str, str]: + """Each name a core answers to, as Recalbox writes it, to its profile.""" + index = getattr(self, "_keys", None) + if index is None: + index = {} + for key, profile in load_emulator_profiles(str(EMULATORS_DIR)).items(): + for name in (key, *(str(c) for c in profile.get("cores") or [])): + index.setdefault(name.replace("-", "_").lower(), key) + self._keys = index + return index + + def required_value(self, fe: NativeFile) -> bool: + """Whether the cores the element names need the file. + + An element lists its cores, and the merged flag is true when any + core needs the file: cdi/cdibios.zip names libretro/same_cdi, whose + driver declares no such set, and became mandatory because MAME + requires it. Where none of the element's cores reads the file, the + truth says nothing about it and the platform's value stands. + """ + truth = fe.truth or {} + required_by = truth.get("_required_by") + readers = truth.get("_cores") + if required_by is None or not readers: + return fe.required + index = self._profile_keys() + named = { + index.get(core.rsplit("/", 1)[-1].replace("-", "_").lower(), "") + for core in fe.cores() + } & set(readers) + if not named: + if fe.platform is not None and fe.platform.get("required") is not None: + return bool(fe.platform["required"]) + return fe.required + return bool(named & set(required_by)) + def _bios_element(self, fe: NativeFile, native_id: str) -> str: attrs = [f"path={quoteattr(self._path(fe))}"] attrs.append(f'md5={quoteattr(",".join(fe.hashes("md5")))}') attrs.append(f'core={quoteattr(",".join(fe.cores()))}') - if not fe.required: + if not self.required_value(fe): attrs.append('mandatory="false"') elif fe.native("mandatory_declared", None) is True: attrs.append('mandatory="true"') diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 904735b2..c2f4d91c 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -1141,6 +1141,34 @@ class RecalboxExport(unittest.TestCase): self.assertEqual({k: v for k, v in after.items() if k in before}, before) self.assertEqual(set(after) - set(before), set()) + def test_mandatory_follows_the_cores_the_element_names(self): + """cdi/cdibios.zip names libretro/same_cdi, which does not read the + set: MAME requiring it says nothing about that element.""" + scraped = {"systems": {"philips-cdi": {"native_id": "cdi", "files": [ + {"name": "cdibios.zip", "destination": "cdi/cdibios.zip", "md5": "a" * 32, + "native_system": "cdi", "core": "libretro/same_cdi", "required": False}, + {"name": "cdimono1.zip", "destination": "cdi/cdimono1.zip", "md5": "b" * 32, + "native_system": "cdi", "core": "libretro/same_cdi", "required": False}, + ]}}} + truth = {"systems": {"philips-cdi": {"files": [ + {"name": "cdibios.zip", "required": True, "_cores": ["mame0278"], + "_required_by": ["mame0278"]}, + {"name": "cdimono1.zip", "required": True, "_cores": ["same_cdi", "mame0278"], + "_required_by": ["same_cdi"]}, + ]}}} + systems, report = build_native_model(truth, scraped) + exporter = discover_exporters()["recalbox"]() + produced = exporter.render(systems, report, {}, None)["es_bios.xml"] + mandatory = { + b.get("path"): b.get("mandatory") for b in ET.fromstring(produced).iter("bios") + } + self.assertEqual(mandatory, {"cdi/cdibios.zip": "false", "cdi/cdimono1.zip": None}) + changes = landed_changes(exporter, systems).records + self.assertEqual( + [(c["name"], c["old"], c["new"]) for c in changes], + [("cdimono1.zip", False, True)], + ) + def test_a_truncated_export_is_reported(self): exporter, systems, _ = self._render() empty = {