From 56e8cbe046cf32f6572d230a57433b70dbb60734 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 11:33:39 +0200 Subject: [PATCH] feat: file mame device sets by their machines The -listxml names, for every machine, its parent (romof), the devices of its default configuration (device_ref) and the cards its slots offer (slotoption). A machine takes the system of the first set on its romof chain a written entry lists, and passes it to each device it can instantiate. A device set reached from one system gets system:, from several the new per-entry systems:, from none neither, and keeps serving every system of the profile. The gap report, the core extras and slots read the entry's own systems, so generate_pack --system and the installer pick by them. --- schemas/emulator.schema.json | 17 ++ scripts/common.py | 23 ++- scripts/packextras.py | 14 +- scripts/scraper/_hash_merge.py | 87 ++++++++- scripts/scraper/mame_devices.py | 139 +++++++++++++- scripts/scraper/mame_hash_scraper.py | 24 ++- scripts/slots.py | 8 +- scripts/validate_schemas.py | 23 +++ scripts/verify.py | 5 +- tests/test_install_selection.py | 5 +- tests/test_mame_devices.py | 268 ++++++++++++++++++++++++++- wiki/profiling.md | 1 + wiki/testing-guide.md | 7 + wiki/tools.md | 10 + 14 files changed, 604 insertions(+), 27 deletions(-) diff --git a/schemas/emulator.schema.json b/schemas/emulator.schema.json index c53b6bdf..2bd48c88 100644 --- a/schemas/emulator.schema.json +++ b/schemas/emulator.schema.json @@ -411,6 +411,15 @@ "system": { "type": "string" }, + "systems": { + "type": "array", + "minItems": 2, + "uniqueItems": true, + "items": { + "type": "string" + }, + "description": "The systems of the profile this entry serves, when it serves several and not all: a MAME device set the machines of two systems load. One system is written as system:." + }, "path": { "type": [ "string", @@ -800,6 +809,14 @@ } }, "allOf": [ + { + "not": { + "required": [ + "system", + "systems" + ] + } + }, { "if": { "properties": { diff --git a/scripts/common.py b/scripts/common.py index bd1b6e7b..da919984 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -1785,16 +1785,29 @@ def profiles_for_systems( return matching +def entry_system_ids(entry: dict, profile: dict) -> list[str]: + """Systems a profile entry serves. + + The entry's own `system:`, else its own `systems:` (a MAME device set + the machines of several systems load), else every system of its + profile. + """ + own = entry.get("system") + if own: + return [str(own)] + listed = entry.get("systems") or profile.get("systems") or [] + return [str(sid) for sid in listed] + + def entry_serves_systems(entry: dict, profile: dict, system_ids: list[str]) -> bool: """Whether a profile entry belongs to one of the systems, spelled either way. - The entry's own `system:` decides. One without it serves every system - of its profile, the rule a platform pack applies to core extras. + The entry's own `system:` or `systems:` decides. One without either + serves every system of its profile, the rule a platform pack applies to + core extras. """ - own = entry.get("system") - systems = [own] if own else profile.get("systems", []) wanted = {_norm_system_id(sid) for sid in system_ids} - return bool(wanted & {_norm_system_id(str(sid)) for sid in systems}) + return bool(wanted & {_norm_system_id(sid) for sid in entry_system_ids(entry, profile)}) def select_emulator_profiles( diff --git a/scripts/packextras.py b/scripts/packextras.py index 66f15b63..d2a3457a 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -8,7 +8,7 @@ from __future__ import annotations from collections.abc import Iterable -from common import PROFILE_IDENTITY_FIELDS, _norm_system_id +from common import PROFILE_IDENTITY_FIELDS, _norm_system_id, entry_system_ids from common import archive_members, holds_required_members from common import declared_root, expand_platform_declared_names, platform_declarations from common import load_emulator_profiles @@ -128,7 +128,9 @@ def _slug_for( {"systems": [own]}, platform_systems, norm_map, sys_to_slug ) emu_name = u.get("profile") or u.get("emulator", "") - profile = profiles.get(emu_name, {}) + # The report carries the entry's own systems: (a MAME device set) or + # its profile's, the same list either way when the entry names none. + profile = {"systems": u["systems"]} if u.get("systems") else profiles.get(emu_name, {}) if not profile: # The report names the display name where the key failed. profile = next( @@ -477,7 +479,7 @@ def _agnostic_scan_extras( "source_path": f.get("path") or "", "category": f.get("category", "bios"), "source_system": f.get("system"), - "source_systems": list(profile.get("systems", [])), + "source_systems": entry_system_ids(f, profile), "region": f.get("region"), "variant_group": f.get("variant_group"), "agnostic_scan": True, @@ -633,7 +635,7 @@ def _companion_extras( "source_path": seed.get("path") or "", "category": seed.get("category", "bios"), "source_system": seed.get("system"), - "source_systems": list(profile.get("systems", [])), + "source_systems": entry_system_ids(seed, profile), "region": seed.get("region"), "variant_group": seed.get("variant_group"), "agnostic_scan": True, @@ -749,7 +751,7 @@ def _archive_prefix_extras( "source_path": archive_name, "category": archive_entry.get("category", "bios"), "source_system": archive_entry.get("system"), - "source_systems": list(profile.get("systems", [])), + "source_systems": entry_system_ids(archive_entry, profile), "region": archive_entry.get("region"), "variant_group": archive_entry.get("variant_group"), } @@ -1051,7 +1053,7 @@ def _collect_emulator_extras( "source_path": raw, "category": f.get("category", "bios"), "source_system": f.get("system"), - "source_systems": list(profile.get("systems", [])), + "source_systems": entry_system_ids(f, profile), "region": f.get("region"), "variant_group": f.get("variant_group"), } diff --git a/scripts/scraper/_hash_merge.py b/scripts/scraper/_hash_merge.py index cb76848b..9ecb1a96 100644 --- a/scripts/scraper/_hash_merge.py +++ b/scripts/scraper/_hash_merge.py @@ -595,12 +595,82 @@ def _zip_scalar(name: str) -> str: return name if re.fullmatch(r"[a-z0-9_]+\.zip", name) else _yaml_scalar(name) +_PLAIN_SYSTEM = re.compile(r"^[a-z][a-z0-9-]*$") + + +def _system_lines(systems: list[str]) -> list[str]: + """`system:` for one system, `systems:` for several, nothing for none.""" + def scalar(value: str) -> str: + return value if _PLAIN_SYSTEM.match(value) else _yaml_scalar(value) + + if not systems: + return [] + if len(systems) == 1: + return [f" system: {scalar(systems[0])}"] + return [" systems:", *(f" - {scalar(s)}" for s in systems)] + + +def _entry_systems(entry: dict) -> list[str]: + """The systems an entry states, in the shape _system_lines writes.""" + if entry.get("system"): + return [str(entry["system"])] + return [str(s) for s in entry.get("systems") or []] + + +def _section_names(text: str) -> set[str]: + """Entry names inside the generated device section.""" + start = _device_section_start(text) + if start is None: + return set() + end = _FILES_END.search(text, text.index("\n", start) + 1) + section = text[start : end.start() if end else len(text)] + return { + m.group(1).strip('"').strip("'") + for m in re.finditer(r"^ - name:\s*(.+?)\s*$", section, re.M) + } + + +def _set_section_systems(text: str, systems: dict[str, list[str]]) -> str: + """Rewrite the system fields of the named entries of the device section. + + Any `system:` or `systems:` the entry holds is dropped and the new one + written right after its category, where a written entry keeps it. + """ + if not systems: + return text + start = _device_section_start(text) + if start is None: + return text + head, section = text[:start], text[start:] + out: list[str] = [] + current = "" + in_list = False + for line in section.split("\n"): + entry_head = re.match(r"^ - name:\s*(.+?)\s*$", line) + if entry_head: + current = entry_head.group(1).strip('"').strip("'") + if current in systems: + if in_list and line.startswith(" - "): + continue + in_list = False + if re.match(r"^ system:", line): + continue + if re.match(r"^ systems:\s*$", line): + in_list = True + continue + out.append(line) + if current in systems and re.match(r"^ category:", line): + out.extend(_system_lines(systems[current])) + return head + "\n".join(out) + + def _format_device_entry(name: str, device: dict, role: str) -> str: """One generated device entry, in the field order of the written ones.""" lines = [ f" - name: {_zip_scalar(name)}", " required: false", " category: bios_zip", + *_system_lines(device.get("systems") or []), ] if device.get("source_line"): lines.append(f' source_ref: "{_build_source_ref(device, role)}"') @@ -632,6 +702,11 @@ def merge_mame_devices( has none. A set it lacks is written under the generated section, in name order, with required false. An entry of another category under a device name is reported, never touched. + + A device carrying `systems` (attribute_systems) has them written on its + entry when the entry is in the generated section: `system:` for one, + `systems:` for several, neither for none. An entry above the section + keeps the system a reader gave it. """ path = Path(profile_path) text = path.read_text(encoding="utf-8") @@ -646,7 +721,9 @@ def merge_mame_devices( } contents_patches: list[dict] = [] ref_patches: dict[str, str] = {} + system_patches: dict[str, list[str]] = {} new_blocks: dict[str, str] = {} + generated = _section_names(text) for set_name, device in sorted(devices.items()): name = f"{set_name}.zip" existing = entries.get(name) @@ -666,10 +743,18 @@ def merge_mame_devices( if not old.get("source_ref") and device.get("source_line"): ref_patches[name] = _build_source_ref(device, role) changed = True + if ( + "systems" in device + and name in generated + and _entry_systems(old) != list(device["systems"]) + ): + system_patches[name] = list(device["systems"]) + changed = True report["updated" if changed else "unchanged"].append(set_name) - if write and (contents_patches or ref_patches or new_blocks): + if write and (contents_patches or ref_patches or system_patches or new_blocks): patched = _add_source_refs(text, ref_patches) + patched = _set_section_systems(patched, system_patches) patched = _patch_bios_entries(patched, contents_patches) patched = _insert_device_entries(patched, new_blocks) shutil.copy2(path, path.with_suffix(PROFILE_BACKUP_SUFFIX)) diff --git a/scripts/scraper/mame_devices.py b/scripts/scraper/mame_devices.py index 79edc439..618e7004 100644 --- a/scripts/scraper/mame_devices.py +++ b/scripts/scraper/mame_devices.py @@ -8,6 +8,9 @@ isdevice="yes", its ROMs and the source file defining the device. The lines cited for a set are the ROM_START block of that file declaring those ROMs, read at the release tag. A set whose block cannot be told apart is left without lines and reported. + +The same document says which machines load each set, so a set is filed under +the systems of those machines (attribute_systems). """ from __future__ import annotations @@ -20,7 +23,11 @@ from collections.abc import Callable, Iterator from concurrent.futures import ThreadPoolExecutor from contextlib import contextmanager from pathlib import Path +from typing import NamedTuple +from common import yaml_load + +from ._hash_merge import _device_section_start from .mame_parser import _collect_rom_macros, _expand_macros, _parse_rom_entries LISTXML_CHUNK = 8 * 1024 * 1024 @@ -31,6 +38,8 @@ _ATTR = re.compile(rb'(\w+)="([^"]*)"') _DESCRIPTION = re.compile(rb"(.*?)", re.S) _BIOSSET = re.compile(rb"]*)>") _ROM = re.compile(rb"]*)>") +_DEVICE_REF = re.compile(rb']*\bname="([^"]+)"') +_SLOT_OPTION = re.compile(rb']*\bdevname="([^"]+)"') _ROM_START = re.compile(r"ROM_START\s*\(\s*(\w+)\s*\)") _ROM_END = re.compile(r"\bROM_END\b") @@ -97,14 +106,40 @@ def open_listxml(path: Path) -> Iterator[io.BufferedIOBase]: yield stream -def scan_device_sets(stream: io.BufferedIOBase) -> tuple[str, dict[str, dict]]: - """Release tag of a -listxml document and its device sets with dumped ROMs. +class MachineLinks(NamedTuple): + """What one listxml element says about the sets it loads. + + *romof* is the parent the element names. *uses* are the devices it can + instantiate: every subdevice of its default configuration, each device a + selectable slot option adds, and for a device its parent ROM device, + whose set the device's ROMs are also searched in. + """ + + romof: str + isdevice: bool + uses: tuple[str, ...] + + +class Release(NamedTuple): + tag: str + devices: dict[str, dict] + machines: dict[str, MachineLinks] + + +def scan_release(stream: io.BufferedIOBase) -> Release: + """Release tag, device sets with dumped ROMs and every element's links. The document is read through a sliding window, never whole: the 0.289 - listxml is 311 MB. + listxml is 311 MB. The links are what infoxml.cpp writes: romof is the + driver's parent (src/frontend/mame/infoxml.cpp:761-765) or a device's + parent ROM device (856-858), device_ref every subdevice of the default + configuration (786, 1010-1015), slotoption each selectable card of a + slot (2129-2172). device_t::searchpath puts the parent ROM device after + the device itself (src/emu/device.cpp:140-152). """ tag = "" devices: dict[str, dict] = {} + machines: dict[str, MachineLinks] = {} buffer = b"" while chunk := stream.read(LISTXML_CHUNK): buffer += chunk @@ -115,13 +150,24 @@ def scan_device_sets(stream: io.BufferedIOBase) -> tuple[str, dict[str, dict]]: end = buffer.find(b"", match.end()) if end == -1: break + name = match.group(1).decode() attrs = _attrs(match.group(2)) - if attrs.get("isdevice") == "yes": - body = buffer[match.end() : end] + body = buffer[match.end() : end] + isdevice = attrs.get("isdevice") == "yes" + romof = attrs.get("romof", "") + uses = { + ref.decode() + for ref in (*_DEVICE_REF.findall(body), *_SLOT_OPTION.findall(body)) + } + if isdevice and romof: + uses.add(romof) + uses.discard(name) + machines[name] = MachineLinks(romof, isdevice, tuple(sorted(uses))) + if isdevice: roms = _device_roms(body) if roms: description = _DESCRIPTION.search(body) - devices[match.group(1).decode()] = { + devices[name] = { "description": html.unescape(description.group(1).decode()) if description else "", "sourcefile": attrs.get("sourcefile", ""), @@ -131,7 +177,86 @@ def scan_device_sets(stream: io.BufferedIOBase) -> tuple[str, dict[str, dict]]: buffer = buffer[position:] if position else buffer[-(2 * LISTXML_CHUNK) :] if not tag: raise ValueError("no header: not a MAME -listxml document") - return tag, devices + return Release(tag, devices, machines) + + +def scan_device_sets(stream: io.BufferedIOBase) -> tuple[str, dict[str, dict]]: + """Release tag of a -listxml document and its device sets with dumped ROMs.""" + release = scan_release(stream) + return release.tag, release.devices + + +def profile_seeds(text: str) -> dict[str, str]: + """Set name to system, for the bios_zip entries above the device section. + + These are the entries a reader wrote, the seeds attribute_systems starts + from. The generated section is left out: its systems derive from these, + and reading them back would carry a previous release's answer forward. + """ + start = _device_section_start(text) + written = yaml_load(text if start is None else text[:start]) or {} + seeds: dict[str, str] = {} + for entry in written.get("files") or []: + name = str(entry.get("name") or "") + if entry.get("category") == "bios_zip" and name.endswith(".zip") and entry.get("system"): + seeds[name[:-4]] = str(entry["system"]) + return seeds + + +def _seed_system( + name: str, machines: dict[str, MachineLinks], seeds: dict[str, str] +) -> str: + """System a profile entry gives this element, through its romof chain. + + A clone names its parent and a parent its BIOS root set, so a Neo Geo + game reaches neogeo and takes its system. A device is never walked: its + romof is a parent ROM device, not the machine it belongs to. The driver + file decides nothing: one file holds machines of several systems + (neogeo.cpp declares the MVS and the AES, atari400.cpp the 800 and the + 5200), and filing a machine under the wrong one would take its devices + away from its own system. + """ + seen: set[str] = set() + while name and name not in seen: + seen.add(name) + if name in seeds: + return seeds[name] + links = machines.get(name) + if links is None or links.isdevice: + return "" + name = links.romof + return "" + + +def attribute_systems( + machines: dict[str, MachineLinks], seeds: dict[str, str] +) -> dict[str, list[str]]: + """Systems whose machines can load each set. + + *seeds* maps a set name to the system a profile entry states for it. + Each element _seed_system places passes its system to every device it + can instantiate, followed through those devices' own links. One walk per + system, so the cost is the size of the graph times the number of + systems. + """ + starts: dict[str, list[str]] = {} + for name in machines: + system = _seed_system(name, machines, seeds) + if system: + starts.setdefault(system, []).append(name) + systems: dict[str, set[str]] = {} + for system, nodes in starts.items(): + seen = set(nodes) + stack = list(nodes) + while stack: + links = machines.get(stack.pop()) + for used in links.uses if links else (): + if used not in seen: + seen.add(used) + stack.append(used) + for name in seen: + systems.setdefault(name, set()).add(system) + return {name: sorted(found) for name, found in systems.items()} def source_path(sourcefile: str) -> str: diff --git a/scripts/scraper/mame_hash_scraper.py b/scripts/scraper/mame_hash_scraper.py index e3726fc5..d3e7dec6 100644 --- a/scripts/scraper/mame_hash_scraper.py +++ b/scripts/scraper/mame_hash_scraper.py @@ -16,6 +16,7 @@ import subprocess import sys import urllib.error import urllib.request +from collections import Counter from datetime import datetime, timezone from pathlib import Path from typing import Any @@ -33,7 +34,13 @@ from ._hash_merge import ( merge_mame_devices, merge_mame_profile, ) -from .mame_devices import locate_sources, open_listxml, scan_device_sets +from .mame_devices import ( + attribute_systems, + locate_sources, + open_listxml, + profile_seeds, + scan_release, +) from .mame_parser import parse_mame_source_tree log = logging.getLogger(__name__) @@ -474,9 +481,10 @@ def _merge_devices(args: argparse.Namespace) -> None: have to be the lines of the profile's upstream_commit. """ profile_path = _EMULATORS_DIR / "mame.yml" - profile = yaml_load(profile_path.read_text(encoding="utf-8")) or {} + text = profile_path.read_text(encoding="utf-8") + profile = yaml_load(text) or {} with open_listxml(args.listxml) as stream: - tag, devices = scan_device_sets(stream) + tag, devices, machines = scan_release(stream) version = _parse_version_tag(tag) if str(profile.get("core_version")) != version: raise RuntimeError( @@ -494,6 +502,9 @@ def _merge_devices(args: argparse.Namespace) -> None: devices, lambda path: upstream.fetch_file(repo, commit, path, upstream.CACHE_DIR), ) + systems = attribute_systems(machines, profile_seeds(text)) + for set_name, device in devices.items(): + device["systems"] = systems.get(set_name, []) report = merge_mame_devices(str(profile_path), devices, write=not args.dry_run) print(f"mame-devices: {len(devices)} device ROM sets in the {tag} listxml") @@ -503,6 +514,13 @@ def _merge_devices(args: argparse.Namespace) -> None: print(f" = {len(report['unchanged'])} unchanged") for set_name in report["conflict"]: print(f" ! {set_name}.zip is listed under another category, left as written") + by_count = Counter(len(device["systems"]) for device in devices.values()) + print( + f" systems: {by_count[1]} sets under one system, " + f"{sum(n for k, n in by_count.items() if k > 1)} under several, " + f"{by_count[0]} loaded by no machine a profile entry places " + "(they serve every system of the profile)" + ) if unplaced: print(f" {len(unplaced)} without source lines:") for set_name, reason in sorted(unplaced.items()): diff --git a/scripts/slots.py b/scripts/slots.py index 97058121..f43aa125 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -26,6 +26,7 @@ from common import ( archive_entry, build_zip_contents_index, check_inside_zip, + entry_system_ids, holds_required_members, load_data_dir_registry, parse_md5_list, @@ -234,7 +235,12 @@ def profile_claims( full = f"{base_dest}/{dest}" if base_dest else dest else: placed = placement.place( - {"profile": emu_name, "system": entry.get("system")}, dest + { + "profile": emu_name, + "system": entry.get("system"), + "systems": entry_system_ids(entry, profile), + }, + dest, ) if placed is None: continue diff --git a/scripts/validate_schemas.py b/scripts/validate_schemas.py index 2f9c3d52..bf376955 100644 --- a/scripts/validate_schemas.py +++ b/scripts/validate_schemas.py @@ -59,6 +59,7 @@ def _validate_yaml_directory( out.extend(_unreachable_citations(path, data)) if schema_name == "emulator.schema.json": out.extend(_checks_without_values(path, data)) + out.extend(_systems_outside_profile(path, data)) out.extend(_doubled_paths(path)) return out @@ -72,6 +73,28 @@ _CHECK_FIELDS = { } +def _systems_outside_profile(path: Path, document: object) -> list[str]: + """An entry's systems: naming a system its profile does not list. + + A --system pick selects the profile by its systems: and then the entry + by its own, so a name the profile lacks is one no pick ever reaches. + """ + if not isinstance(document, dict): + return [] + listed = {str(s) for s in document.get("systems") or []} + out: list[str] = [] + for index, entry in enumerate(document.get("files") or []): + if not isinstance(entry, dict): + continue + unknown = [str(s) for s in entry.get("systems") or [] if str(s) not in listed] + if unknown: + out.append( + f"{path.relative_to(ROOT)}: files[{index}] ({entry.get('name')}) " + f"systems {', '.join(unknown)} not in the profile's systems" + ) + return out + + def _checks_without_values(path: Path, document: object) -> list[str]: """A check an entry declares with nothing to compare against. diff --git a/scripts/verify.py b/scripts/verify.py index cf5ae83c..753985ec 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -71,6 +71,7 @@ from common import ( runs_standalone, standalone_set as platform_standalone_set, entry_serves_systems, + entry_system_ids, sanitize_pack_path, select_emulator_profiles, ) @@ -618,7 +619,7 @@ def find_undeclared_files( archive_entries[archive_key] = { "profile": emu_name, "emulator": profile.get("emulator", emu_name), - "systems": list(profile.get("systems", [])), + "systems": entry_system_ids(f, profile), "system": f.get("system"), "region": f.get("region"), "variant_group": f.get("variant_group"), @@ -697,7 +698,7 @@ def find_undeclared_files( entry = { "profile": emu_name, "emulator": profile.get("emulator", emu_name), - "systems": list(profile.get("systems", [])), + "systems": entry_system_ids(f, profile), "system": f.get("system"), "region": f.get("region"), "variant_group": f.get("variant_group"), diff --git a/tests/test_install_selection.py b/tests/test_install_selection.py index 2dcfb9d5..037a94fb 100644 --- a/tests/test_install_selection.py +++ b/tests/test_install_selection.py @@ -578,13 +578,16 @@ class InstallMatchesASystemPack(unittest.TestCase): """`install.py --system X --region R` against a real `generate_pack.py --system X --region R` build. The cases are the ones a review reproduced: ColecoVision's extras filed at destinations another system declares, and - PicoDrive's Mega CD variant group spread over two systems.""" + PicoDrive's Mega CD variant group spread over two systems. The last pick + holds MAME device sets filed under several systems (systems:), under one, + and under none.""" PLATFORM = "retroarch" CASES = ( (["coleco-colecovision"], []), (["sega-mega-cd"], ["north-america"]), (["sony-playstation"], ["japan"]), + (["elektronika-bk"], []), ) @classmethod diff --git a/tests/test_mame_devices.py b/tests/test_mame_devices.py index b1542922..60898895 100644 --- a/tests/test_mame_devices.py +++ b/tests/test_mame_devices.py @@ -3,7 +3,8 @@ Synthetic listxml and driver source, no network: the scan keeps only device sets with dumped ROMs, the source line is the ROM_START block holding them, and the merge adds sets, refreshes the contents of written ones without -touching what a reader wrote, and leaves BIOS root sets alone. +touching what a reader wrote, and leaves BIOS root sets alone. Each set is +filed under the systems of the machines that can load it, never guessed. """ from __future__ import annotations @@ -25,10 +26,14 @@ from scripts.scraper.mame_devices import ( ABSENT, AMBIGUOUS, NO_FILE, + MachineLinks, + attribute_systems, find_rom_block, locate_sources, open_listxml, + profile_seeds, scan_device_sets, + scan_release, source_path, ) @@ -359,6 +364,267 @@ class RefRole(unittest.TestCase): self.assertEqual(mame_ref_role({"emulator": "MAME"}), "") +GRAPH_LISTXML = b""" + +\t +\t\t +\t +\t +\t\t +\t\t +\t +\t +\t\t +\t +\t +\t\t +\t +\t +\t\t +\t\t\t +\t\t\t +\t\t +\t +\t +\t\t +\t\t +\t +\t +\t\t +\t +\t +\t\t +\t +\t +\t\t +\t +\t +\t\t +\t +\t +\t\t +\t +\t +\t\t +\t +\t +\t\t +\t +\t +\t\t +\t + +""" + +SEEDS = {"neogeo": "snk-neogeo-mvs", "apple2gs": "apple2gs"} + + +def _release(): + return scan_release(io.BytesIO(GRAPH_LISTXML)) + + +class TheListxmlSaysWhoLoadsASet(unittest.TestCase): + """infoxml.cpp writes romof, device_ref and slotoption: the scan keeps + them for every element, device or machine.""" + + def test_links(self): + machines = _release().machines + self.assertEqual(machines["mslugj"], MachineLinks("mslug", False, ("cart_dev",))) + self.assertEqual(machines["apple2gs"].uses, ("a2diskii", "a2ssc")) + # A device's ROMs are searched in its parent ROM device's set too. + self.assertEqual(machines["cart_dev"].uses, ("cart_base",)) + self.assertTrue(machines["cart_dev"].isdevice) + + def test_scan_device_sets_is_the_same_scan(self): + release = _release() + self.assertEqual( + scan_device_sets(io.BytesIO(GRAPH_LISTXML)), (release.tag, release.devices) + ) + + +class DeviceSetsFollowTheMachinesThatLoadThem(unittest.TestCase): + + def setUp(self): + self.systems = attribute_systems(_release().machines, SEEDS) + + def test_a_clone_reaches_the_bios_through_its_parent(self): + self.assertEqual(self.systems["cart_dev"], ["snk-neogeo-mvs"]) + self.assertEqual(self.systems["ng_memcard"], ["snk-neogeo-mvs"]) + + def test_a_parent_rom_device_goes_with_its_child(self): + self.assertEqual(self.systems["cart_base"], ["snk-neogeo-mvs"]) + + def test_slot_cards_and_their_own_devices(self): + self.assertEqual(self.systems["a2ssc"], ["apple2gs"]) + self.assertEqual(self.systems["a2diskii"], ["apple2gs"]) + self.assertEqual(self.systems["diskii_fdc"], ["apple2gs"]) + + def test_a_machine_no_entry_places_files_nothing(self): + # aes shares neogeo.cpp with the MVS but no entry lists it: the + # driver file is not a system, so ym2610 gets the MVS only through + # the MVS machines, and pc_kbd nothing at all. + self.assertEqual(self.systems["ym2610"], ["snk-neogeo-mvs"]) + self.assertNotIn("pc_kbd", self.systems) + + def test_several_systems(self): + systems = attribute_systems(_release().machines, {**SEEDS, "aes": "snk-neogeo-aes"}) + self.assertEqual(systems["ym2610"], ["snk-neogeo-aes", "snk-neogeo-mvs"]) + self.assertEqual(systems["ng_memcard"], ["snk-neogeo-mvs"]) + + def test_a_device_seed_passes_to_what_it_uses_never_up_its_romof(self): + systems = attribute_systems( + _release().machines, {"a2diskii": "apple2gs", "cart_base": "x-system"} + ) + self.assertEqual(systems["diskii_fdc"], ["apple2gs"]) + self.assertNotIn("cart_dev", systems) + + def test_seeds_are_the_written_entries_only(self): + text = ( + PROFILE + "\n" + DEVICE_SECTION + "\n - name: ym2610.zip\n" + " required: false\n category: bios_zip\n system: snk-neogeo-mvs\n" + ) + self.assertEqual( + profile_seeds(text), {"neogeo": "snk-neogeo-mvs", "qsound": "capcom-cps2"} + ) + + +class TheMergeWritesTheSystems(unittest.TestCase): + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.path = Path(self._tmp.name) / "mame.yml" + self.path.write_text(PROFILE, encoding="utf-8") + self.devices = _release().devices + self.devices["qsound"] = _devices()["qsound"] + systems = attribute_systems(_release().machines, SEEDS) + for name, device in self.devices.items(): + device["systems"] = systems.get(name, []) + + def tearDown(self): + self._tmp.cleanup() + + def _merge(self) -> dict[str, list[str]]: + return merge_mame_devices(str(self.path), self.devices, write=True) + + def _entries(self) -> dict[str, dict]: + data = yaml_load(self.path.read_text(encoding="utf-8")) + return {f["name"]: f for f in data["files"]} + + def test_one_system_several_or_none(self): + self.devices["ym2610"]["systems"] = ["snk-neogeo-aes", "snk-neogeo-mvs"] + self._merge() + entries = self._entries() + self.assertEqual(entries["a2diskii.zip"]["system"], "apple2gs") + self.assertNotIn("systems", entries["a2diskii.zip"]) + self.assertEqual(entries["ym2610.zip"]["systems"], ["snk-neogeo-aes", "snk-neogeo-mvs"]) + self.assertNotIn("system", entries["ym2610.zip"]) + self.assertNotIn("system", entries["pc_kbd.zip"]) + self.assertNotIn("systems", entries["pc_kbd.zip"]) + text = self.path.read_text(encoding="utf-8") + block = text[text.index("- name: a2diskii.zip"):] + self.assertLess(block.index("category: bios_zip"), block.index("system: apple2gs")) + + def test_a_written_entry_keeps_its_system(self): + self.devices["qsound"]["systems"] = ["apple2gs"] + self._merge() + self.assertEqual(self._entries()["qsound.zip"]["system"], "capcom-cps2") + + def test_a_refresh_rewrites_the_generated_systems(self): + self._merge() + self.devices["a2diskii"]["systems"] = ["apple2gs", "coco"] + self.devices["cart_dev"]["systems"] = [] + report = self._merge() + self.assertIn("a2diskii", report["updated"]) + entries = self._entries() + self.assertEqual(entries["a2diskii.zip"]["systems"], ["apple2gs", "coco"]) + self.assertNotIn("system", entries["cart_dev.zip"]) + self.devices["a2diskii"]["systems"] = ["coco"] + self._merge() + entries = self._entries() + self.assertEqual(entries["a2diskii.zip"]["system"], "coco") + self.assertNotIn("systems", entries["a2diskii.zip"]) + # The list items of systems: went with it, the member list did not. + self.assertEqual(entries["a2diskii.zip"]["contents"][0]["name"], "341-0027.bin") + + def test_a_second_run_changes_nothing(self): + self._merge() + first = self.path.read_text(encoding="utf-8") + report = self._merge() + self.assertEqual(report["updated"], []) + self.assertEqual(self.path.read_text(encoding="utf-8"), first) + + +class APickReadsTheEntrySystems(unittest.TestCase): + """--system on a pack and on the installer selects by the entry's own + systems: before the profile's.""" + + SYSTEMS = ["apple2gs", "coco", "snk-neogeo-mvs"] + + def test_entry_system_ids(self): + from scripts.common import entry_system_ids + + profile = {"systems": self.SYSTEMS} + self.assertEqual(entry_system_ids({"system": "coco"}, profile), ["coco"]) + self.assertEqual( + entry_system_ids({"systems": ["apple2gs", "coco"]}, profile), ["apple2gs", "coco"] + ) + self.assertEqual(entry_system_ids({}, profile), self.SYSTEMS) + + def test_entry_serves_systems(self): + from scripts.common import entry_serves_systems + + profile = {"systems": self.SYSTEMS} + entry = {"systems": ["apple2gs", "coco"]} + self.assertTrue(entry_serves_systems(entry, profile, ["coco"])) + self.assertFalse(entry_serves_systems(entry, profile, ["snk-neogeo-mvs"])) + self.assertTrue(entry_serves_systems({}, profile, ["snk-neogeo-mvs"])) + + def test_the_gap_report_and_the_extras_carry_them(self): + import sys + + sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "scripts")) + from packextras import _collect_emulator_extras, _extra_system_ids + from verify import find_undeclared_files + + profile = { + "emulator": "MAME", "type": "libretro", "systems": self.SYSTEMS, + "files": [{"name": "a2diskii.zip", "category": "bios_zip", + "systems": ["apple2gs", "coco"]}], + } + config = {"cores": ["mame"], "systems": {}} + db = { + "files": {"s1": {"name": "a2diskii.zip", "path": "bios/Apple/a2diskii.zip", + "size": 10, "sha1": "s1", "md5": "m1", "crc32": "12345678"}}, + "indexes": {"by_name": {"a2diskii.zip": ["s1"]}, "by_md5": {"m1": "s1"}, + "by_crc32": {"12345678": "s1"}}, + } + (gap,) = find_undeclared_files(config, "emulators", db, emu_profiles={"mame": profile}) + self.assertEqual(gap["systems"], ["apple2gs", "coco"]) + (extra,) = _collect_emulator_extras( + config, "emulators", db, set(), "", {"mame": profile}, include_all=True + ) + self.assertEqual(_extra_system_ids(extra), ["apple2gs", "coco"]) + + def test_the_schema_and_the_profile_list(self): + try: + from scripts import validate_schemas + except ImportError as exc: + self.skipTest(f"validate_schemas needs jsonschema: {exc}") + validator = validate_schemas._validator("emulator.schema.json") + root = Path(__file__).resolve().parents[1] + base = {"emulator": "x", "type": "libretro", "systems": ["a", "b"]} + both = {**base, "files": [{"name": "x.zip", "system": "a", "systems": ["a", "b"]}]} + self.assertTrue(list(validator.iter_errors(both))) + one = {**base, "files": [{"name": "x.zip", "systems": ["a"]}]} + self.assertTrue(list(validator.iter_errors(one))) + fine = {**base, "files": [{"name": "x.zip", "systems": ["a", "b"]}]} + self.assertEqual(list(validator.iter_errors(fine)), []) + outside = {**base, "files": [{"name": "x.zip", "systems": ["a", "c"]}]} + found = validate_schemas._systems_outside_profile(root / "emulators" / "x.yml", outside) + self.assertEqual(len(found), 1) + self.assertIn("systems c", found[0]) + + class Cli(unittest.TestCase): def test_devices_needs_a_listxml(self): diff --git a/wiki/profiling.md b/wiki/profiling.md index 6684eb45..84edd7fd 100644 --- a/wiki/profiling.md +++ b/wiki/profiling.md @@ -525,6 +525,7 @@ partitions), and `game_count`. | `name` | filename as the core expects it | | `required` | true if the core needs this file to function | | `system` | system ID this file belongs to (for multi-system profiles) | +| `systems` | the systems of the profile this file serves, when it serves several but not all (a MAME device set the machines of two systems load). Never written with `system` | | `archive` | ROM set the file lives inside, for arcade entries (e.g. `neogeo.zip`) | | `size` | expected size in bytes, or a list when the code accepts several exact sizes | | `min_size`, `max_size` | size range when the code accepts a range | diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index 30abcf46..9a3dfd9c 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -110,6 +110,13 @@ and merged into a profile, on an inline listxml and driver source. Covers: entry's contents refreshed with its manual fields kept, BIOS root sets left as written, a second run changing nothing, a new BIOS root placed before the section +- Systems: the links the scan keeps (`romof`, `device_ref`, `slotoption`, a + device's parent ROM device), a set reached through a clone, its parent and + a slot card, one set under two systems, a machine no entry lists placing + nothing, seeds read above the generated section only, `system:` or + `systems:` written and rewritten by a refresh, a written entry's system + kept, and a `--system` pick reading the entry's own systems through the gap + report and the core extras **test_fbneo_parser.** Tests the FBNeo C source parser that identifies `BDF_BOARDROM` sets. Same inline fixture approach. diff --git a/wiki/tools.md b/wiki/tools.md index e3a61246..7d2a9786 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -857,6 +857,16 @@ go under the `Device ROM sets (generated from MAME -listxml)` comment at the end of `files:` with `required: false`. A set the profile already lists keeps every field a reader wrote and only gets its contents refreshed. +Each generated set is filed under the systems of the machines that can load +it. A machine takes the `system:` of the first set on its `romof` chain that a +written entry lists (a clone reaches its parent, a parent its BIOS root set), +and passes it to every device in its `device_ref` list and every card its +slots offer (`slotoption`), followed through those devices' own links and +their parent ROM devices. A set loaded by machines of one system gets +`system:`, of several `systems:`, of none neither field, and then serves every +system of the profile. The driver file is never used: `neogeo.cpp` declares +the MVS and the AES. The output counts the sets in each case. + A plain run (pipeline step 2a) that moves `mame.yml` to a new release downloads that release's `-listxml` into `.cache/dats/` and runs the device merge itself, so the BIOS root sets and the device sets name one release.