diff --git a/scripts/common.py b/scripts/common.py index dfab57ce..ecfe8328 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -481,23 +481,30 @@ def _affinity_tokens(text: str) -> set[str]: def _by_affinity(paths: list[str], file_entry: dict, dest_hint: str) -> list[str]: - """Same-named candidates, the ones stored with the file's owner first. + """Same-named candidates, the likeliest first. A name step without a tie-break took whichever path sorted first: the - TheXTech coin.ogg for Syobon Action, a Hurrican sound for ARMSX2. The - destination's directories and the profile that asks name where its own - copy lives; the order is otherwise kept. + TheXTech coin.ogg for Syobon Action, a Hurrican sound for ARMSX2, the + 1 MB CM-32L PCM ROM for ScummVM's 512 KB MT32_PCM.ROM. A size the entry + declares orders the candidates first (ordering rejects nothing, so an + unvalidated size keeps its informative status); then the destination's + directories and the profile that asks name where its own copy lives. The + order is otherwise kept. """ + if len(paths) < 2: + return paths hint = dest_hint or file_entry.get("path") or file_entry.get("destination") or "" wanted = _affinity_tokens(hint.rsplit("/", 1)[0] if "/" in hint else "") for owner in (file_entry.get("source_profile"), file_entry.get("source_emulator")): if owner: wanted |= _affinity_tokens(str(owner).replace(" ", "")) - if not wanted or len(paths) < 2: - return paths + sized = any(file_entry.get(k) for k in ("size", "min_size", "max_size")) - def score(path: str) -> int: - return len(wanted & _affinity_tokens(path.rsplit("/", 1)[0])) + def score(path: str) -> tuple[int, int]: + fits = 0 + if sized and os.path.exists(path): + fits = int(size_fits(file_entry, os.path.getsize(path))) + return fits, len(wanted & _affinity_tokens(path.rsplit("/", 1)[0])) return sorted(paths, key=score, reverse=True) diff --git a/scripts/generate_db.py b/scripts/generate_db.py index f5ddba5b..4b3f3689 100644 --- a/scripts/generate_db.py +++ b/scripts/generate_db.py @@ -488,9 +488,23 @@ def _collect_all_aliases(files: dict) -> dict: emu_config = yaml_load(f) or {} except (yaml.YAMLError, OSError): continue + # A profile whose entries name each other (dosbox-x opens + # MT32_CONTROL.ROM and CM32L_CONTROL.ROM alike, so each entry + # aliases the other) states that ITS slot takes either name. + # Indexed globally that became evidence for every emulator: + # ScummVM's MT-32 slot received the CM-32L ROMs. + primary_names = { + str(fe.get("name", "")).lower() + for fe in emu_config.get("files", []) + if isinstance(fe, dict) + } for file_entry in emu_config.get("files", []): - entry_aliases = list(file_entry.get("aliases") or []) entry_name = file_entry.get("name", "") + entry_aliases = [ + alias for alias in file_entry.get("aliases") or [] + if str(alias).lower() == str(entry_name).lower() + or str(alias).lower() not in primary_names + ] # A profile may accept several revisions: each held one # is designated by the entry, none of them by guess. matched: set[str] = { diff --git a/tests/test_db_aliases.py b/tests/test_db_aliases.py index 25263709..2881bcfb 100644 --- a/tests/test_db_aliases.py +++ b/tests/test_db_aliases.py @@ -111,5 +111,29 @@ class AcceptedRevisionLists(unittest.TestCase): for sha in ("s1", "s2"): self.assertEqual([a["name"] for a in aliases.get(sha, [])], ["MT32_CONTROL.ROM"]) +class CrossNamingStaysInItsProfile(unittest.TestCase): + def test_an_alias_naming_a_sibling_entry_is_not_indexed(self): + """dosbox-x aliases MT32_CONTROL.ROM and CM32L_CONTROL.ROM to each other.""" + import generate_db + + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as tmp: + previous = os.getcwd() + os.chdir(tmp) + try: + Path("emulators").mkdir() + Path("emulators/d.yml").write_text( + 'files:\n' + ' - name: "MT32_CONTROL.ROM"\n sha1: "mt"\n aliases: ["CM32L_CONTROL.ROM"]\n' + ' - name: "CM32L_CONTROL.ROM"\n sha1: "cm"\n aliases: ["MT32_CONTROL.ROM"]\n' + ) + aliases = generate_db._collect_all_aliases({ + "mt": {"name": "mt32_control.rom", "path": "a", "md5": "m1"}, + "cm": {"name": "cm32l_control.rom", "path": "b", "md5": "m2"}, + }) + finally: + os.chdir(previous) + self.assertEqual([a["name"] for a in aliases["mt"]], ["MT32_CONTROL.ROM"]) + self.assertEqual([a["name"] for a in aliases["cm"]], ["CM32L_CONTROL.ROM"]) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_name_tiebreak.py b/tests/test_name_tiebreak.py new file mode 100644 index 00000000..0a5439f4 --- /dev/null +++ b/tests/test_name_tiebreak.py @@ -0,0 +1,36 @@ +"""Same-named candidates are ordered by the size the entry declares. + +ScummVM declares MT32_PCM.ROM at 524288 bytes; the name step took the 1 MB +CM-32L PCM ROM that sorted first. Ordering rejects nothing, so a size +without `validation: [size]` keeps its informative status. +""" + +from __future__ import annotations + +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +from common import _by_affinity # noqa: E402 + + +class DeclaredSizeOrders(unittest.TestCase): + def test_the_fitting_size_comes_first(self): + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as tmp: + big = Path(tmp) / "a" / "pcm.rom" + small = Path(tmp) / "b" / "pcm.rom" + for path, size in ((big, 1024), (small, 512)): + path.parent.mkdir() + path.write_bytes(b"x" * size) + ordered = _by_affinity([str(big), str(small)], {"size": 512}, "") + self.assertEqual(ordered[0], str(small)) + kept = _by_affinity([str(big), str(small)], {}, "") + self.assertEqual(kept, [str(big), str(small)]) + + +if __name__ == "__main__": + unittest.main()