diff --git a/scripts/exporter/retrobat_exporter.py b/scripts/exporter/retrobat_exporter.py index c14d064f..bba1d449 100644 --- a/scripts/exporter/retrobat_exporter.py +++ b/scripts/exporter/retrobat_exporter.py @@ -81,6 +81,20 @@ class Exporter(BaseExporter): return bool(self.single_value(fe, "md5")) return super().states(fe, field_name) + def __init__(self) -> None: + super().__init__() + # Additions render did not write: another entry holds their path. + self._merged: set[int] = set() + + @staticmethod + def _path(fe: NativeFile) -> str: + declared = fe.native("native_path", "") + return str(declared) if declared else f"bios/{fe.destination}" + + def lands(self, fe: NativeFile) -> bool: + """An addition lands only where render wrote it.""" + return self.writable(fe) and id(fe) not in self._merged + def render( self, systems: dict[str, NativeSystem], @@ -103,6 +117,7 @@ class Exporter(BaseExporter): "fetch it again with --refresh-cache" ) from exc + self._merged = set() exportable = { system.native_id: (system, files) for system, files in self.exportable(systems, require="md5") @@ -117,12 +132,19 @@ class Exporter(BaseExporter): continue system, files = exportable[key] ours: OrderedDict[str, OrderedDict[str, str]] = OrderedDict() + # The checker reads one md5 per biosFiles item and reports the + # item by its path, so a path holds one item. An addition whose + # path the platform or an earlier addition holds is not written. + claimed = {self._path(fe) for fe in files if fe.platform is not None} for fe in files: + path = self._path(fe) + if fe.platform is None and (path in claimed or path in ours): + self._merged.add(id(fe)) + continue entry: OrderedDict[str, str] = OrderedDict() entry["md5"] = self.single_value(fe, "md5") - declared = fe.native("native_path", "") - entry["file"] = str(declared) if declared else f"bios/{fe.destination}" - ours.setdefault(entry["file"], entry) + entry["file"] = path + ours.setdefault(path, entry) bios_files = [] for theirs in (base.get(key) or {}).get("biosFiles", []): bios_files.append(ours.pop(theirs.get("file"), theirs)) @@ -165,7 +187,6 @@ class Exporter(BaseExporter): continue declared = {bios.get("file") for bios in entry.get("biosFiles", [])} for fe in files: - path = str(fe.native("native_path", "")) or f"bios/{fe.destination}" - if path not in declared: + if self._path(fe) not in declared: issues.append(f"absent: {system.native_id}/{fe.name}") return issues diff --git a/tests/test_export_counts.py b/tests/test_export_counts.py index a1a2f490..ac9edd54 100644 --- a/tests/test_export_counts.py +++ b/tests/test_export_counts.py @@ -204,6 +204,40 @@ class RetroBatKeepsWhatWeDoNotModel(unittest.TestCase): ]) +class RetroBatCountsOneAdditionPerPath(unittest.TestCase): + """Two cores each gave the truth a psx/scph5501.bin at one destination. + The render writes one biosFiles item per path, yet both were counted + added: 78 additions announced for 75 items written.""" + + def test_an_addition_at_a_held_path_does_not_land(self): + from export_native import landed_changes # noqa: PLC0415 + + theirs = NativeFile("scph1001.bin", "scph1001.bin", "psx", + platform={"md5": A}, truth={"md5": A}) + first = NativeFile("scph5501.bin", "scph5501.bin", "psx", truth={"md5": B}) + second = NativeFile("scph5501.bin", "scph5501.bin", "psx", truth={"md5": B}) + over = NativeFile("scph1001.bin", "scph1001.bin", "psx", truth={"md5": C}) + systems = {"psx": NativeSystem("psx", files=[first, theirs, second, over])} + original = {"psx": {"name": "Sony Playstation", "biosFiles": [ + {"md5": A, "file": "bios/scph1001.bin"}, + ]}} + exporter = RetroBat() + produced = json.loads(exporter.render( + systems, Report(), {"batocera-systems.json": json.dumps(original)}, + )["batocera-systems.json"]) + self.assertEqual(produced["psx"]["biosFiles"], [ + {"md5": A, "file": "bios/scph1001.bin"}, + {"md5": B, "file": "bios/scph5501.bin"}, + ]) + added = [r for r in landed_changes(exporter, systems).records + if r["kind"] == "added"] + self.assertEqual([r["name"] for r in added], ["scph5501.bin"]) + self.assertEqual( + [exporter.lands(fe) for fe in (first, theirs, second, over)], + [True, True, False, False], + ) + + class PureDataExportsKeepWhatWeDoNotModel(unittest.TestCase): """Recalbox's es_bios.xml and libretro's System.dat were rebuilt from our model too: an entry the platform added or renamed after our scrape left