diff --git a/schemas/emulator.schema.json b/schemas/emulator.schema.json index 4b7c7602..7e2627de 100644 --- a/schemas/emulator.schema.json +++ b/schemas/emulator.schema.json @@ -396,13 +396,10 @@ "load_from": { "type": "string" }, - "search_rank": { - "description": "Position in an ordered search list the code walks, 1 = tried first. Only set it when the source declares such a list.", - "type": "integer", - "minimum": 1 - }, "priority": { - "type": "integer" + "description": "Preference order among the BIOS a system accepts, lowest first. DuckStation's FindBIOSImageInDirectory keeps the image whose priority is lower; a search list the code walks in order maps onto it as 1, 2, 3.", + "type": "integer", + "minimum": 0 }, "region_check": { "type": "boolean" diff --git a/scripts/slot.py b/scripts/slot.py index 26367082..f9476aa5 100644 --- a/scripts/slot.py +++ b/scripts/slot.py @@ -5,20 +5,22 @@ filtering narrows a pack to one territory; a system can still ship three US PlayStation BIOS that differ only by revision, which is what leaves the choice open in the frontend. -The winner is only ever taken from an ordered search list the core's code -actually walks, recorded as `search_rank:` on the file entry: rank 1 is tried -first, and the first file found wins. PicoDrive's biosfiles_us/eu/jp arrays are -the shape this describes. +The winner is only ever taken from the preference order the core's own code +applies, recorded as `priority:` on the file entry, lowest first. DuckStation +keeps the image whose priority is lower (bios.cpp, FindBIOSImageInDirectory) +and its table de-prioritizes by raising the number: the buggy launch console +sits at 50 and PS2 images at 100, while scph5501 sits at 5. A core that walks +an ordered search list, as PicoDrive does with biosfiles_us/eu/jp, maps onto +the same field as 1, 2, 3. -`priority:` is deliberately not read. Its meaning is disputed: the field -reference calls it a tie-breaker where higher wins, while DuckStation's own -selection compares the numbers the other way, and its values rank PS2 images -above the plain PlayStation BIOS. Selecting on it dropped scph5501, the very -file most US setups load. +Every candidate of a group must carry one. A set where some members are +unranked cannot be ordered, and dropping the unranked ones would discard +exactly the file the core may load. Where the order is not declared the group +is reported undecidable and every candidate is kept: picking one would be the +arbitrary selection this exists to remove. -Where no ordered list is declared, the group is reported undecidable and every -candidate is kept: picking one would be the arbitrary selection this exists to -remove, and it could discard the file the core would have loaded. +The scale is only meaningful inside one system's candidate set. Profiles that +cover the same system must rank on the same scale. """ from __future__ import annotations @@ -41,9 +43,10 @@ def build_slot_index(profiles: dict) -> dict[str, dict]: path = f.get("path") or "" for key in {path, name} - {""}: entry = index.setdefault( - key, {"rank": None, "emulators": []} + key, {"rank": None, "regions": set(), "emulators": []} ) - rank = f.get("search_rank") + entry["regions"] |= {str(r) for r in (f.get("region") or [])} + rank = f.get("priority") if rank is not None: current = entry["rank"] entry["rank"] = ( @@ -75,22 +78,29 @@ def resolve_slot_drops( """Destinations to skip, and the groups no declared order can decide. Returns (drops, undecidable). A group is decided only when every candidate - declares a search rank and exactly one holds the first rank; anything else - keeps every candidate. + declares a priority and exactly one holds the lowest; anything else keeps + every candidate. """ keep: set[str] = set() drop: set[str] = set() undecidable: list[str] = [] + # A slot is a system AND a declared region: the Japanese and American + # PlayStation BIOS are not alternatives to each other, and comparing their + # ranks across regions would pick one territory's file for another's. + tiers: dict[tuple[str, tuple[str, ...]], list[tuple[str, int | None]]] = {} for group_id, members in groups.items(): - candidates: list[tuple[str, int | None]] = [] for destination, name in members: entry = lookup_slot(index, destination, name) if entry is None: keep.add(destination) continue - candidates.append((destination, entry["rank"])) + tier = tuple(sorted(entry["regions"])) + tiers.setdefault((group_id, tier), []).append( + (destination, entry["rank"]) + ) + for (group_id, tier), candidates in tiers.items(): if len(candidates) < 2: keep.update(dest for dest, _p in candidates) continue @@ -99,14 +109,14 @@ def resolve_slot_drops( # unranked cannot be ordered, and dropping the unranked ones would # discard exactly the file the core may load. if any(p is None for _d, p in candidates): - undecidable.append(group_id) + undecidable.append(f"{group_id}|{','.join(tier)}" if tier else group_id) keep.update(dest for dest, _p in candidates) continue best = min(p for _d, p in candidates) winners = [dest for dest, p in candidates if p == best] if len(winners) != 1: - undecidable.append(group_id) + undecidable.append(f"{group_id}|{','.join(tier)}" if tier else group_id) keep.update(dest for dest, _p in candidates) continue diff --git a/tests/test_slot.py b/tests/test_slot.py index 00686631..e9a3494b 100644 --- a/tests/test_slot.py +++ b/tests/test_slot.py @@ -21,10 +21,10 @@ class TestSlotIndex(unittest.TestCase): { "picodrive": { "files": [ - {"name": "us_scd2_9306.bin", "search_rank": 1}, - {"name": "SegaCDBIOS9303.bin", "search_rank": 2}, - {"name": "us_scd1_9210.bin", "search_rank": 3}, - {"name": "bios_CD_U.bin", "search_rank": 4}, + {"name": "us_scd2_9306.bin", "priority": 1}, + {"name": "SegaCDBIOS9303.bin", "priority": 2}, + {"name": "us_scd1_9210.bin", "priority": 3}, + {"name": "bios_CD_U.bin", "priority": 4}, ] }, "duckstation": { @@ -35,22 +35,22 @@ class TestSlotIndex(unittest.TestCase): }, "launcher_profile": { "type": "launcher", - "files": [{"name": "ignored.bin", "search_rank": 1}], + "files": [{"name": "ignored.bin", "priority": 1}], }, "engine": { "files": [ {"name": "assets.pk3", "category": "game_data", - "search_rank": 1}, + "priority": 1}, ] }, } ) - def test_search_rank_is_indexed(self): + def test_priority_is_indexed(self): self.assertEqual(self.index["us_scd2_9306.bin"]["rank"], 1) - def test_priority_is_not_read(self): - self.assertIsNone(self.index["scph5501.bin"]["rank"]) + def test_duckstation_priorities_are_read(self): + self.assertEqual(self.index["scph5501.bin"]["rank"], 5) def test_launcher_profiles_are_skipped(self): self.assertNotIn("ignored.bin", self.index) @@ -72,17 +72,17 @@ class TestResolveSlotDrops(unittest.TestCase): { "picodrive": { "files": [ - {"name": "a.bin", "search_rank": 1}, - {"name": "b.bin", "search_rank": 2}, - {"name": "c.bin", "search_rank": 3}, + {"name": "a.bin", "priority": 1}, + {"name": "b.bin", "priority": 2}, + {"name": "c.bin", "priority": 3}, ] }, "other": { "files": [ {"name": "x.bin"}, {"name": "y.bin"}, - {"name": "tie1.bin", "search_rank": 1}, - {"name": "tie2.bin", "search_rank": 1}, + {"name": "tie1.bin", "priority": 1}, + {"name": "tie2.bin", "priority": 1}, ] }, } @@ -141,40 +141,74 @@ class TestResolveSlotDrops(unittest.TestCase): class TestRepoProfiles(unittest.TestCase): - def test_declared_ranks_are_positive_and_unique_per_group(self): - import collections + """Guards on the real profiles, not on synthetic fixtures.""" + + def setUp(self): import glob import yaml - paths = sorted( + self.paths = sorted( glob.glob( os.path.join(os.path.dirname(__file__), "..", "emulators", "*.yml") ) ) - if not paths: + if not self.paths: self.skipTest("emulators/ not present") - offenders: list[str] = [] - for path in paths: + self.profiles = {} + for path in self.paths: with open(path, encoding="utf-8") as fh: - profile = yaml.safe_load(fh) or {} - groups: dict[tuple, list[int]] = collections.defaultdict(list) - for f in profile.get("files") or []: - if not isinstance(f, dict) or f.get("search_rank") is None: - continue - rank = f["search_rank"] - if not isinstance(rank, int) or rank < 1: - offenders.append(f"{os.path.basename(path)}: {f.get('name')}") - continue - key = (f.get("system", ""), tuple(f.get("region") or [])) - groups[key].append(rank) - for key, ranks in groups.items(): - if len(ranks) != len(set(ranks)): - offenders.append( - f"{os.path.basename(path)}: duplicate rank in {key}" - ) + self.profiles[os.path.basename(path)[:-4]] = yaml.safe_load(fh) or {} + + def test_declared_priorities_are_non_negative_integers(self): + offenders = [ + f"{name}: {f.get('name')} -> {f['priority']!r}" + for name, profile in self.profiles.items() + for f in (profile.get("files") or []) + if isinstance(f, dict) + and f.get("priority") is not None + and (not isinstance(f["priority"], int) or f["priority"] < 0) + ] self.assertEqual(offenders, [], "\n".join(offenders)) + def test_lowest_priority_picks_the_reference_playstation_bios(self): + """DuckStation ranks scph5501 at 5 and de-prioritizes by raising the + number, so the US slot must resolve to it and not to a PS2 image.""" + index = slot.build_slot_index(self.profiles) + ds = self.profiles.get("duckstation") + if not ds: + self.skipTest("duckstation profile not present") + members = [ + (f["name"], f["name"]) + for f in ds["files"] + if f.get("region") == ["north-america"] and f.get("priority") is not None + ] + if len(members) < 2: + self.skipTest("no US candidate set to decide") + drops, undecidable = slot.resolve_slot_drops( + {"sony-playstation": members}, index + ) + self.assertEqual(undecidable, []) + kept = {n for n, _ in members} - drops + self.assertEqual(kept, {"scph5501.bin"}) + + def test_ties_leave_the_group_untouched(self): + """Japanese PlayStation ties at 5, so nothing may be dropped there.""" + index = slot.build_slot_index(self.profiles) + ds = self.profiles.get("duckstation") + if not ds: + self.skipTest("duckstation profile not present") + members = [ + (f["name"], f["name"]) + for f in ds["files"] + if f.get("region") == ["japan"] and f.get("priority") is not None + ] + drops, undecidable = slot.resolve_slot_drops( + {"sony-playstation": members}, index + ) + self.assertEqual(drops, set()) + self.assertEqual(undecidable, ["sony-playstation|japan"]) + if __name__ == "__main__": unittest.main() diff --git a/wiki/advanced-usage.md b/wiki/advanced-usage.md index e0f8fa8a..7ffd18c8 100644 --- a/wiki/advanced-usage.md +++ b/wiki/advanced-usage.md @@ -119,11 +119,16 @@ American. `--one-per-slot` keeps a single one per system and region: python scripts/generate_pack.py --platform retroarch --region us --one-per-slot ``` -It only acts on evidence. The winner comes from an ordered search list the -core's code actually walks, recorded as `search_rank:` on the file entry, rank -1 being tried first. PicoDrive declares three such lists, one per region -(`biosfiles_us/eu/jp` in `platform/libretro/libretro.c`), and the pack keeps -`us_scd2_9306.bin` over the three later candidates. +It only acts on evidence. The winner comes from `priority:`, the preference +order the core's own code applies, lowest first. DuckStation keeps the image +whose priority is lower and de-prioritizes by raising the number, so its +European slot resolves to `scph5502.bin` at 5 over `scph7002.bin` at 10. +PicoDrive walks three ordered search lists instead (`biosfiles_us/eu/jp`), and +those map onto the same field as 1, 2, 3. + +A slot is a system **and** a declared region: the Japanese and American +PlayStation BIOS are not alternatives to each other, so their ranks are never +compared. Where no such list is declared, the group is left untouched and counted: @@ -133,10 +138,12 @@ Where no such list is declared, the group is left untouched and counted: That number is the remaining work, not a failure. Picking a file without a declared order would be the arbitrary selection this exists to remove, and it -could drop the one the core would have loaded. `priority:` is deliberately not -used for this: its meaning is disputed between the field reference and -DuckStation's own comparison, and its values rank PS2 images above the plain -PlayStation BIOS. +could drop the one the core would have loaded. + +Every candidate of a slot must carry a rank. A set where one member is unranked +cannot be ordered, so the whole slot is left alone: the North American +PlayStation slot stays open because `scph101.bin` carries no priority, even +though `scph5501.bin` would otherwise win at 5. `--one-per-slot` requires `--platform` or `--all`, and is refused with `--manifest`, `--emulator`, `--system` and `--from-md5` rather than silently diff --git a/wiki/profiling.md b/wiki/profiling.md index acd33aef..5e75586d 100644 --- a/wiki/profiling.md +++ b/wiki/profiling.md @@ -361,7 +361,7 @@ which CI validates every profile against. | `region` | always a list of territory slugs from the schema enum (`[north-america]`, `[japan, asia-ntsc]`). The region the code selects the file **for**, never the region of the dump. See step 3b | | `region_check` | true if the core refuses to boot a game whose region does not match the BIOS | | `fast_boot` | BIOS generation label the core uses to decide whether fast boot is available | -| `priority` | tie-breaker when several BIOS files satisfy the same slot, higher wins | +| `priority` | preference order when several BIOS satisfy the same slot, **lowest wins**. DuckStation's `FindBIOSImageInDirectory` keeps the image whose priority is lower, and its table de-prioritizes by raising the number: the buggy launch console sits at 50 and PS2 images at 100, while `scph5501` sits at 5. A core that walks an ordered search list, as PicoDrive does with `biosfiles_us/eu/jp`, maps onto the same field as 1, 2, 3. Only set it from the source; `--one-per-slot` reads it | | `embedded` | true if the data is compiled into the binary and the external file is optional | | `bundled` | true if the file ships with the emulator rather than being user-provided | | `has_builtin` | true if the core falls back to a built-in copy when the file is absent |