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 = {