fix: read the bios preference order the code applies

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"

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
 

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()

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

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 |