From 1cd6c9ee4d0121a3ba16c31d6cd56ec68417786c Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Thu, 22 Oct 2026 19:58:26 +0200 Subject: [PATCH] feat: place save-dir files in packs and lists load_from: save_dir entries of a libretro build were left out of every pack. They now go below the platform's save folder (registry save_directory: RetroArch saves/, Lakka savefiles/, by core), the manifest marks them root: save, and verify replays the same placement. Platforms keeping saves per system or per game list them as omitted. --- platforms/_registry.yml | 35 +++++++++ schemas/emulator.schema.json | 11 ++- schemas/install-manifest.schema.json | 42 ++++++++++- scripts/cross_reference.py | 11 ++- scripts/generate_pack.py | 103 ++++++++++++++++++++------- scripts/packextras.py | 101 +++++++++++++++++++++++--- scripts/packpaths.py | 88 ++++++++++++++++++++++- scripts/packreadme.py | 24 +++++++ scripts/slots.py | 15 ++-- scripts/validate_schemas.py | 102 ++++++++++++++++++++++++++ scripts/validation.py | 45 +++++++++--- scripts/verify.py | 27 +++++-- tests/test_audit_regressions.py | 15 +++- tests/test_libretro_layout.py | 24 ++++++- tests/test_native_mode.py | 7 +- 15 files changed, 584 insertions(+), 66 deletions(-) diff --git a/platforms/_registry.yml b/platforms/_registry.yml index 81e15bf5..e3ffbad6 100644 --- a/platforms/_registry.yml +++ b/platforms/_registry.yml @@ -190,6 +190,15 @@ platforms: hash_type: sha1 verification_mode: existence base_destination: system + # The directory RETRO_ENVIRONMENT_GET_SAVE_DIRECTORY returns + # (runloop.c:2699-2700, libretro/RetroArch a81e450c): savefile_directory, + # saves/ beside system/ by default (platform_unix.c:3364-3365, + # platform_win32.c:1343, platform_darwin.m:456), with the core's + # library_name appended since sort_savefiles_enable defaults to true + # (config.def.h:1099, runloop.c:11285-11293). + save_directory: + folder: saves + by_core: true case_insensitive_fs: true schedule: weekly cores: all_libretro @@ -484,6 +493,12 @@ platforms: source_url: https://raw.githubusercontent.com/batocera-linux/batocera.linux/master/package/batocera/core/batocera-scripts/scripts/batocera-systems source_format: python_dict hash_type: md5 + # The libretro launcher hands each core /userdata/saves/ with + # sort_savefiles_enable off (python-src/batocera-launch-libretro/ + # batocera_launch_libretro/emulator.py:754-757 at 8bbc0d6e, saves_dir in + # python-src/batocera-launch/batocera_launch/emulator.py:251-252 at + # d30617e5): one folder per launched system, no single place. + save_directory: null schedule: weekly target_scraper: batocera_targets target_source: https://github.com/batocera-linux/batocera.linux @@ -537,6 +552,11 @@ platforms: hash_type: md5 # Windows: names differing only by case land on one file. case_insensitive_fs: true + # emulatorLauncher hands a libretro core saves\ (saves\\ + # LR-amiberry for amiberry, saves\ for mame), sorted by core for dolphin + # only (emulatorLauncher/Generators/LibRetro.Generator.cs:1128-1131 and + # 1305-1315 at 7f8dc28a): one folder per launched system, no single place. + save_directory: null schedule: weekly target_scraper: null target_source: null @@ -617,6 +637,13 @@ platforms: scraper: libretro inherits_from: retroarch cores: all_libretro + # Lakka's retroarch.cfg sets savefile_directory to /storage/savefiles, + # beside /storage/system, and sorts saves by core + # (packages/lakka/retroarch_base/retroarch/config/retroarch.cfg:3164-3165 + # and 3206-3207, libretro/Lakka-LibreELEC cc3bf85d). + save_directory: + folder: savefiles + by_core: true # Lakka is a Linux distribution: two names that differ by case are two # files there, not the one RetroArch's Windows and macOS builds see. case_insensitive_fs: false @@ -863,6 +890,14 @@ platforms: # scriptmodules are what say which packages it ships. # RetroPie runs on Linux, where case tells two files apart. case_insensitive_fs: false + # Not RetroArch's: RetroPie sets no savefile_directory and turns + # sort_savefiles_enable off (scriptmodules/emulators/retroarch.sh:289-290, + # RetroPie-Setup 46128cd2), and its RetroArch (retropie-v1.19.0) has no + # default save directory on Linux (frontend/drivers/platform_unix.c sets + # DEFAULT_DIR_SRAM only at 1624-1626, for Android). The save directory is + # then the folder of the loaded content (runloop.c runloop_path_set_redirect), + # one per game: a file read below it has no single place. + save_directory: null schedule: null target_scraper: retropie_targets target_source: https://retropie.org.uk/stats/pkgflags/ diff --git a/schemas/emulator.schema.json b/schemas/emulator.schema.json index dd54993d..e4cca6ee 100644 --- a/schemas/emulator.schema.json +++ b/schemas/emulator.schema.json @@ -335,6 +335,15 @@ "type": "integer", "minimum": 0 }, + "library_name": { + "description": "The library_name the libretro core reports in retro_get_system_info. A frontend sorting saves by core (RetroArch's sort_savefiles_enable) hands the core /, so a profile with a file the libretro build reads below the save directory (load_from: save_dir) declares it. Empty when the core reports an empty name, which RetroArch appends nothing for.", + "type": "string", + "pattern": "^[^/\\\\]*$" + }, + "save_folder_once": { + "description": "true when the core adds its own folder to the save directory only where the directory it is handed is not already named so (Cemu's in_named_dir), so it reads the same place whether the frontend sorts saves by core or not. Every load_from: save_dir path then starts with that folder, the library_name.", + "const": true + }, "logo": { "type": "string" }, @@ -536,7 +545,7 @@ ] }, "load_from": { - "description": "Where the emulator reads the file when not from the system directory, keyed by build when the libretro and standalone builds read it from different places.", + "description": "Where the emulator reads the file when not from the system directory, keyed by build when the libretro and standalone builds read it from different places. save_dir: path is relative to the directory the frontend returns for RETRO_ENVIRONMENT_GET_SAVE_DIRECTORY, and packs and the installer place the libretro build's file there.", "oneOf": [ { "type": "string" diff --git a/schemas/install-manifest.schema.json b/schemas/install-manifest.schema.json index 1dd9e05f..41882b9b 100644 --- a/schemas/install-manifest.schema.json +++ b/schemas/install-manifest.schema.json @@ -175,7 +175,9 @@ "external", "user_provided", "no_platform_slug", - "below_platform_folder" + "below_platform_folder", + "no_save_directory", + "no_core_folder" ] }, "category": { @@ -316,6 +318,16 @@ } } } + }, + "root": { + "description": "save: dest is relative to the save directory the frontend hands the core (RETRO_ENVIRONMENT_GET_SAVE_DIRECTORY), not to the BIOS directory. Absent for a file of the BIOS tree.", + "const": "save" + }, + "core_folder": { + "description": "The core's library_name, the folder a frontend sorting saves by core (RetroArch's sort_savefiles_enable) inserts between its save directory and dest. Only with root: save. Absent on a save entry when the core reads the same place sorted or not.", + "type": "string", + "minLength": 1, + "pattern": "^[^/\\\\]+$" } } } @@ -407,6 +419,24 @@ } } } + }, + "save_directory": { + "description": "Where the platform keeps the save directory its frontend hands a core, present when an entry carries root: save. folder is the pack folder holding those files and the save directory's default place beside the BIOS directory, and by_core says the frontend appends each core's library_name (core_folder).", + "type": "object", + "additionalProperties": false, + "required": [ + "folder", + "by_core" + ], + "properties": { + "folder": { + "type": "string", + "pattern": "^[^/\\\\.][^/\\\\]*$" + }, + "by_core": { + "type": "boolean" + } + } } }, "$defs": { @@ -861,6 +891,16 @@ } } } + }, + "root": { + "description": "save: dest is relative to the save directory the frontend hands the core (RETRO_ENVIRONMENT_GET_SAVE_DIRECTORY), not to the BIOS directory. Absent for a file of the BIOS tree.", + "const": "save" + }, + "core_folder": { + "description": "The core's library_name, the folder a frontend sorting saves by core (RetroArch's sort_savefiles_enable) inserts between its save directory and dest. Only with root: save. Absent on a save entry when the core reads the same place sorted or not.", + "type": "string", + "minLength": 1, + "pattern": "^[^/\\\\]+$" } }, "oneOf": [ diff --git a/scripts/cross_reference.py b/scripts/cross_reference.py index 090204c3..d48d9199 100644 --- a/scripts/cross_reference.py +++ b/scripts/cross_reference.py @@ -51,6 +51,7 @@ from validation import ( declaration_settles, installed_with_emulator, outside_gap_scope, + read_from_save_dir, unread_on, ) @@ -657,6 +658,7 @@ def slot_key(f: dict, standalone: bool = False) -> tuple: f.get("system"), f.get("variant_group"), f.get("mode", "both"), + read_from_save_dir(f, standalone), ) @@ -725,8 +727,11 @@ def _cross_reference_profile( # takes it: Batocera's standalone Cemu reads keys.txt at # standalone_path, and the libretro Cemu/keys.txt reported here # settled the declaration on a slot the platform never fills. + save_root = read_from_save_dir(f, is_standalone) if is_standalone: effective_path = f.get("standalone_path") or f.get("path") or fname + elif save_root: + effective_path = f.get("path") or fname else: effective_path = system_dir_path(f, f.get("path") or fname) slot = slot_key(f, is_standalone) @@ -757,7 +762,9 @@ def _cross_reference_profile( # Check platform declaration: by name, by an alias the core answers # to (RetroArch lists amiberry's cd32.rom as kick40060.CD32), or by # archive. - in_platform = declaration_settles( + # A platform declares files of its BIOS tree: none answers for one + # the core reads below the save directory. + in_platform = not save_root and declaration_settles( f, fname, platform_names, effective_path, index.get("folded_slots", frozenset()) ) @@ -836,6 +843,8 @@ def _cross_reference_profile( # What tells this file from a homonym another emulator wants. "identity": _first_hash(f), } + if save_root: + entry["root"] = "save" gaps.append(entry) for ag in sorted(archive_gaps.values(), key=lambda e: e["name"]): diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index dc3b3cc6..f10c97b2 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -12,7 +12,7 @@ and 3-tier storage (embedded/external/user_provided). from __future__ import annotations -from collections.abc import Callable, Iterator +from collections.abc import Callable, Iterable, Iterator from artifacts import _build_timestamp, set_plain_mode import argparse @@ -1074,6 +1074,7 @@ def generate_pack( ) core_count = 0 + save_count = 0 extra_resolver = ExtraResolver( db, bios_dir, zip_contents, data_registry, validation_index, validation_owners, offline, profiles=emu_profiles, @@ -1089,15 +1090,13 @@ def generate_pack( # Core extras: _collect_emulator_extras already adjusted # destinations for slug-based platforms. Apply the effective # prefix (base_dest, or inferred from YAML when base_dest is - # empty, as RetroDECK infers "bios"). - extras_pfx = _detect_extras_prefix(config, base_dest) - if extras_pfx: - if not dest.startswith(f"{extras_pfx}/"): - full_dest = f"{extras_pfx}/{dest}" - else: - full_dest = dest - else: - full_dest = dest + # empty, as RetroDECK infers "bios"). A file read below the save + # directory goes to the pack's save folder instead. + full_dest = extra_full_destination( + fe, _detect_extras_prefix(config, base_dest) + ) + if not full_dest: + continue # Taken, a case twin on a case-insensitive filesystem, or a # file/directory conflict (SGB1.sfc vs SGB1.sfc/): verify replays # the same rule (packextras.hidden_extras). @@ -1158,6 +1157,8 @@ def generate_pack( if case_insensitive: seen_lower.add(full_dest.lower()) core_count += 1 + if fe.get("root") == "save": + save_count += 1 total_files += 1 total_files += _pack_data_directories( @@ -1190,6 +1191,7 @@ def generate_pack( _pack_registry.get(platform_name, {}).get("install", {}) or {} ).get("standalone_copies", []), launch_folders=len(config.get("core_system_dirs") or {}), + save_folder=(save_layout(config).folder if save_count else ""), ) _write_generated_member(zf, "README.txt", readme_text) @@ -3856,13 +3858,10 @@ def _manifest_core_entries( continue if region_drops and dest in region_drops: continue - if extras_pfx: - if not dest.startswith(f"{extras_pfx}/"): - full_dest = f"{extras_pfx}/{dest}" - else: - full_dest = dest - else: - full_dest = dest + full_dest = extra_full_destination(fe, extras_pfx) + if not full_dest: + continue + save_root = fe.get("root") == "save" if ( case_insensitive @@ -3874,9 +3873,14 @@ def _manifest_core_entries( # entry kept, or a pack built without the first core loses the # file the installer would still have to place. Collisions are # rare, so the entry is looked up only when one happens. + folded = ( + save_manifest_dest(fe) if save_root + else _manifest_dest_of(full_dest, base_dest) + ).lower() held = next( (e for e in manifest_files - if e["dest"].lower() == _manifest_dest_of(full_dest, base_dest).lower()), + if e["dest"].lower() == folded + and (e.get("root") == "save") == save_root), None, ) if held is not None and held.get("cores") is not None: @@ -3917,13 +3921,19 @@ def _manifest_core_entries( # Manifest dests are relative to base_destination. Keep the inferred # extras prefix when it is an internal layout dir (RetroDECK bios/). - manifest_dest = _manifest_dest_of(full_dest, base_dest) + # A file read below the save directory is written relative to the + # directory the core is handed, and says so (save_entry_fields). + manifest_dest = ( + save_manifest_dest(fe) if save_root else _manifest_dest_of(full_dest, base_dest) + ) entry, file_size = _manifest_entry( local_path, manifest_dest, [source_emu] if source_emu else [], db, repo_root, selection.fields(dest, fe.get("name", "")), ) + if entry is not None and save_root: + entry.update(save_entry_fields(fe)) if entry is None: if not _claim_platform_omission( omitted_by_destination.get(full_dest), source_emu @@ -3965,6 +3975,24 @@ def _manifest_core_entries( return total_size +def _check_save_destinations(files: list[dict], omitted: Iterable[dict]) -> None: + """Refuse a manifest naming one destination below both directories. + + The installer keys every entry by its ``dest``: a file read below the + save directory and one of the BIOS tree written alike would be taken + for one file. + """ + roots: dict[str, set[bool]] = {} + for entry in [*files, *omitted]: + roots.setdefault(entry["dest"], set()).add(entry.get("root") == "save") + clashes = sorted(dest for dest, kinds in roots.items() if len(kinds) > 1) + if clashes: + raise ValueError( + "destinations below both the system and save directories: " + + ", ".join(clashes[:10]) + ) + + def _manifest_dest_of(full_dest: str, base_dest: str) -> str: """A pack destination as the manifest writes it, below base_destination.""" if base_dest and full_dest.startswith(f"{base_dest}/"): @@ -4061,10 +4089,9 @@ def _record_absent( dest = sanitize_pack_path(fe.get("destination", fe.get("name", ""))) if not dest or (region_drops and dest in region_drops): continue - full_dest = ( - dest if not extras_pfx or dest.startswith(f"{extras_pfx}/") - else f"{extras_pfx}/{dest}" - ) + full_dest = extra_full_destination(fe, extras_pfx) + if not full_dest: + continue if full_dest in seen_destinations or full_dest in omitted_by_destination: continue source_emu = fe.get("source_profile") or fe.get("source_emulator", "") @@ -4081,9 +4108,16 @@ def _record_unplaceable(unplaceable: list[dict], record_omission: Callable[..., Said in the manifest, where RomM's DOSBox Pure ROMs were simply absent. """ for u in unplaceable: + where = u.get("path") or u.get("name", "") record_omission( - u.get("path") or u.get("name", ""), u, u.get("system") or "", - u.get("unplaceable", "no_platform_slug"), [u.get("emulator", "")], + # Keyed apart from the BIOS tree's paths, which a file read below + # the save directory can repeat. + f"save:{save_manifest_dest(u)}" if u.get("root") == "save" else where, + u, u.get("system") or "", + # The profile key, the name a target's core list and the + # installer's narrowing match: the display name matched none. + u.get("unplaceable", "no_platform_slug"), + [u.get("profile") or u.get("emulator", "")], ) @@ -4222,8 +4256,13 @@ def generate_manifest( bare = sanitize_pack_path( file_entry.get("destination", file_entry.get("name", "")) ) + save_root = file_entry.get("root") == "save" omitted_by_destination[full_destination] = { - "dest": manifest_destination(full_destination), + "dest": ( + save_manifest_dest(file_entry) if save_root + else manifest_destination(full_destination) + ), + **(save_entry_fields(file_entry) if save_root else {}), "name": str(file_entry.get("name") or ""), "system": system_id, "required": bool(file_entry.get("required", True)), @@ -4534,7 +4573,8 @@ def generate_manifest( "base_destination": base_dest, "detect": detect, "standalone_copies": _placed_copies( - standalone_copies, [entry["dest"] for entry in manifest_files] + standalone_copies, + [entry["dest"] for entry in manifest_files if entry.get("root") != "save"], ), "total_files": len(manifest_files), "total_size": total_size, @@ -4556,6 +4596,11 @@ def generate_manifest( } if data_directories: result["data_directories"] = data_directories + saved = [entry for entry in manifest_files if entry.get("root") == "save"] + if saved: + _check_save_destinations(manifest_files, omitted_by_destination.values()) + layout = save_layout(config) + result["save_directory"] = {"folder": layout.folder, "by_core": layout.by_core} if case_insensitive and fold_case is None and any( d.get("os") not in CASE_FOLDING_OS for d in detect ): @@ -4992,9 +5037,13 @@ from packpaths import ( # noqa: E402,F401 _flat, _resolve_destination, placement_block, + save_layout, ) from packextras import ( # noqa: E402,F401 ExtraResolver, + extra_full_destination, + save_entry_fields, + save_manifest_dest, _detect_extras_prefix, _detect_slug_structure, _map_emulator_to_slug, diff --git a/scripts/packextras.py b/scripts/packextras.py index 847475ab..1dcd773d 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -19,7 +19,8 @@ from common import core_system_dirs, platform_layouts, system_dir_path from common import standalone_set as platform_standalone_set from common import sanitize_pack_path from common import size_fits -from validation import deliverable, read_from_system_dir +from validation import deliverable, read_from_save_dir, read_from_system_dir +from packpaths import save_destination, save_layout from region import FILE_GROUP @@ -31,6 +32,55 @@ def _detect_extras_prefix(config: dict, base_dest: str) -> str: """ return base_dest or declared_root(config) + +def extra_full_destination(fe: dict, prefix: str) -> str: + """Where a pack writes a core extra, from the pack root. + + A file the core reads below the frontend's save directory goes to the + pack path its placement recorded (ExtraPlacement.place). Any other one + goes below the platform's prefix, unless its destination carries it. + The builder, the manifest and verify's replay all read this. + """ + if fe.get("root") == "save": + return str(fe.get("pack_path") or "") + dest = sanitize_pack_path(fe.get("destination", fe.get("name", ""))) + if not dest or not prefix or dest.startswith(f"{prefix}/"): + return dest + return f"{prefix}/{dest}" + + +def save_entry_fields(fe: dict) -> dict: + """What a manifest entry adds for a file read below the save directory. + + ``root: save`` puts its ``dest`` below the frontend's save directory, + laid out as the pack lays it out. ``core_folder`` is the core's + library_name when the platform's frontend inserts it as the first folder + of ``dest`` (RetroArch's sort_savefiles_enable): the installer drops it + where the frontend's own config turns sorting off. + """ + fields = {"root": "save"} + if fe.get("core_folder"): + fields["core_folder"] = str(fe["core_folder"]) + return fields + + +def save_manifest_dest(fe: dict) -> str: + """A manifest dest for a file read below the save directory. + + The pack path below the platform's save folder when the file is placed, + the core's own folder first where the frontend sorts saves by core. + Without a save folder (an omission), the same shape: the core's folder, + then the path the core reads, so two cores reading one name below their + own folders stay two entries. + """ + pack_path = str(fe.get("pack_path") or "") + if pack_path: + return pack_path.split("/", 1)[1] + rel = sanitize_pack_path(fe.get("destination") or fe.get("path") or fe.get("name", "")) + folder = fe.get("core_folder") + return f"{folder}/{rel}" if folder else rel + + def _detect_slug_structure(config: dict) -> tuple[bool, dict[str, str]]: """Detect whether a platform uses per-system slug destinations. @@ -287,7 +337,9 @@ def hidden_extras( dest = sanitize_pack_path(fe.get("destination", fe.get("name", ""))) if not dest or (region_drops and dest in region_drops): continue - full = dest if not prefix or dest.startswith(f"{prefix}/") else f"{prefix}/{dest}" + full = extra_full_destination(fe, prefix) + if not full: + continue reason = placement_block(full, seen, lower, parents, case_insensitive) if reason == "taken": # Another core's claim on the same path, settled by the claims @@ -348,18 +400,16 @@ def unplaceable_extras( the pack. The answer comes from the builder's own placement. """ placement = ExtraPlacement(config, "", profiles) - if not placement.slug_based: - return {} unplaced: dict[tuple[str, str, str], str] = {} for u in undeclared: if not u.get("in_repo"): continue + if not placement.slug_based and u.get("root") != "save": + continue dest = _extra_dest(u) if placement.place(u, dest) is None: key = (u.get("emulator", ""), u.get("name", ""), u.get("path") or "") - unplaced[key] = ( - "below_platform_folder" if placement.nested(dest) else "no_platform_slug" - ) + unplaced[key] = placement.unplaced_reason(u, dest) return unplaced @@ -438,6 +488,10 @@ def _agnostic_scan_extras( mode == "libretro" and standalone ): continue + # The scan walks the BIOS tree: a file read below the save + # directory is placed by the gap report's pass. + if read_from_save_dir(f, standalone): + continue # An agnostic BIOS mode says the BIOS filename is free, not that # every file the emulator loads is. A free filename still has a # known shape, and that shape is what identifies a candidate: an @@ -838,6 +892,7 @@ class ExtraPlacement: self.platform_systems = set(config.get("systems", {}).keys()) self.native_systems = native_slug_systems(config) self.flat = bool(config.get("firmware_folder_flat")) + self.save = save_layout(config) self.norm_map = ( {_norm_system_id(sid): sid for sid in self.platform_systems} if self.slug_based @@ -850,8 +905,16 @@ class ExtraPlacement: ``entry`` names its profile and may name its own system. A slug-based platform cannot place a file of a system it has no - folder for. + folder for. A file the core reads below the frontend's save + directory (``root: save``) keeps its path relative to that + directory and goes below the platform's save folder, which a + platform keeping saves beside the content does not have. """ + if entry.get("root") == "save": + if self.save is None: + return None + full = save_destination(dest, entry.get("core_folder"), self.save) + return (dest, full) if full else None if self.slug_based: slug = _slug_for( entry, self.profiles, self.platform_systems, self.norm_map, @@ -866,6 +929,12 @@ class ExtraPlacement: """True when the file would sit below a folder the frontend lists flat.""" return self.slug_based and self.flat and "/" in dest + def unplaced_reason(self, entry: dict, dest: str) -> str: + """Why ``place`` returned None for this entry.""" + if entry.get("root") == "save": + return "no_save_directory" if self.save is None else "no_core_folder" + return "below_platform_folder" if self.nested(dest) else "no_platform_slug" + def undeclared_report( config: dict, @@ -988,13 +1057,16 @@ def _collect_emulator_extras( placed = placement.place(u, dest) if not u["in_repo"]: if placed is not None: - absent.append(_report_extra(u, name, placed[0])) + missing = _report_extra(u, name, placed[0]) + if u.get("root") == "save": + missing["pack_path"] = placed[1] + absent.append(missing) continue if placed is None: if unplaceable is not None: + reason = placement.unplaced_reason(u, dest) unplaceable.append( - {**u, "unplaceable": "below_platform_folder"} - if placement.nested(dest) else u + u if reason == "no_platform_slug" else {**u, "unplaceable": reason} ) continue dest, full_dest = placed @@ -1005,6 +1077,9 @@ def _collect_emulator_extras( candidates = [ _report_extra(c, name, dest) for c in (u, *u.get("other_claims", [])) ] + if u.get("root") == "save": + for item in candidates: + item["pack_path"] = full_dest # Where the builder meets each claim: the strongest proof first, the # report's profile order between equals, archives after loose files. for item in candidates: @@ -1248,6 +1323,10 @@ def _report_extra(u: dict, name: str, dest: str) -> dict: } if u.get("members"): extra["members"] = u["members"] + if u.get("root") == "save": + extra["root"] = "save" + if u.get("core_folder") is not None: + extra["core_folder"] = u["core_folder"] if u.get("system_dir"): extra["system_dir"] = u["system_dir"] # The archive layout the core opens ranks same-named copies by what diff --git a/scripts/packpaths.py b/scripts/packpaths.py index 8c6c0644..54fb0eb5 100644 --- a/scripts/packpaths.py +++ b/scripts/packpaths.py @@ -6,7 +6,88 @@ on Windows is not a pack.""" from __future__ import annotations +from dataclasses import dataclass + from common import sanitize_pack_path +from validation import read_from_save_dir + + +@dataclass(frozen=True) +class SaveLayout: + """Where a platform's frontend keeps the save directory it hands a core. + + ``folder`` names it twice: the folder of the pack that holds the files + read below it, and its default place beside the system directory + (RetroArch's own folder holds system/ and saves/). A pack is extracted + into the system directory, so that folder is moved by hand, and the + installer writes to the save directory the frontend's config names. + ``by_core`` is set when the frontend hands each core /, RetroArch's sort_savefiles_enable. + """ + + folder: str + by_core: bool + + +def save_layout(config: dict) -> SaveLayout | None: + """The platform's save directory, or None when it is no fixed place. + + Read from the registry's ``save_directory``. A platform that keeps + saves beside the content, or in one folder per launched system, has + none: a file read below it has no single destination. + """ + block = config.get("save_directory") + if not isinstance(block, dict): + return None + folder = sanitize_pack_path(str(block.get("folder") or "")) + if not folder or "/" in folder: + return None + return SaveLayout(folder, bool(block.get("by_core"))) + + +def save_destination( + path: str, core_folder: str | None, layout: SaveLayout +) -> str | None: + """The pack path of a file a core reads below the save directory. + + ``path`` is relative to the directory the core is handed. ``core_folder`` + is the folder a frontend sorting saves by core inserts: the core's + library_name, empty when it inserts none (an empty library_name, or a + core that reads the same place sorted or not), None when the profile + does not say. None then, where the frontend sorts by core. + """ + rel = sanitize_pack_path(path) + if not rel: + return None + if layout.by_core and core_folder: + return f"{layout.folder}/{sanitize_pack_path(core_folder)}/{rel}" + if layout.by_core and core_folder is None: + return None + return f"{layout.folder}/{rel}" + + +def core_save_folder(profile: dict) -> str | None: + """The folder a frontend sorting saves by core inserts for this core. + + RetroArch appends a non-empty library_name only (runloop.c + runloop_path_set_redirect). A core declaring ``save_folder_once`` adds + its own folder to the save directory unless it is already named so + (Cemu's in_named_dir): its paths start with that folder and land in the + same place sorted or not. None when the profile names no library_name. + """ + if "library_name" not in profile: + return None + if profile.get("save_folder_once"): + return "" + return str(profile.get("library_name") or "") + + +# Top folder of an emulator pack holding the files a libretro core reads +# below the frontend's save directory, apart from those of its system +# directory at the root. +EMULATOR_PACK_SAVE_FOLDER = "saves" + + def _path_parents(dest: str) -> list[str]: """Return all parent directory segments of a path.""" parts = dest.split("/") @@ -79,7 +160,12 @@ def _resolve_destination(file_entry: dict, standalone: bool) -> str: else: rel = file_entry.get("name", "") - return sanitize_pack_path(rel) + dest = sanitize_pack_path(rel) + # Read below the frontend's save directory: kept apart from the system + # directory's files at the root, in the folder the pack names for it. + if dest and read_from_save_dir(file_entry, standalone): + return f"{EMULATOR_PACK_SAVE_FOLDER}/{dest}" + return dest def _archive_destination( diff --git a/scripts/packreadme.py b/scripts/packreadme.py index bcb8c05d..11fffe12 100644 --- a/scripts/packreadme.py +++ b/scripts/packreadme.py @@ -116,6 +116,24 @@ def _generic_manual(bios_paths: list[str]) -> str: ) +def _save_section(display: str, folder: str) -> str: + """Where the files of the pack's save folder go. + + Some cores read their system data below the save directory the frontend + hands them, not below the system directory the archive is extracted + into: Azahar keeps its user folder there. + """ + return ( + " Files read from the save directory\n" + " ----------------------------------\n" + f" Some cores read files from {display}'s save directory, not its\n" + f" system directory. The {folder}/ folder of this archive holds them.\n" + f" Move its contents into the save directory ({folder}/ beside\n" + " system/ by default, see Settings > Directory > Save Files), keeping\n" + " the folders it holds. The automatic install writes them there.\n\n" + ) + + def _build_readme( platform_name: str, platform_display: str, @@ -133,11 +151,14 @@ def _build_readme( detect: list[dict] | None = None, standalone_copies: list[dict] | None = None, launch_folders: int = 0, + save_folder: str = "", ) -> str: """Build a personalized step-by-step README for each platform pack. ``launch_folders`` counts the folders a platform that hands each core its own system directory gives them, where it names no system. + ``save_folder`` is the pack folder holding files cores read below the + frontend's save directory, empty when the pack holds none. """ narrowings = narrowings or [] sep = "=" * 50 @@ -167,6 +188,9 @@ def _build_readme( + (pack_manual(platform_name) or _generic_manual(bios_paths)) ) + if save_folder: + guide += _save_section(platform_display, save_folder) + if regions: region_help = ( " - Wrong region? This pack was filtered. Only the\n" diff --git a/scripts/slots.py b/scripts/slots.py index a26a0d14..f2478ac4 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -39,7 +39,7 @@ from common import ( variant_base_name, ) from packresolve import preferred_entries -from validation import outside_gap_scope, with_archive_members +from validation import outside_gap_scope, read_from_save_dir, with_archive_members if TYPE_CHECKING: from packextras import ExtraPlacement @@ -216,9 +216,12 @@ def profile_claims( if not isinstance(entry, dict): continue # The same rule as the gap report and the builder: an entry the - # pack never carries (read from the save directory, a - # placeholder, a filename-agnostic scan) arbitrates nothing. - if outside_gap_scope(entry, is_standalone): + # pack never carries (a placeholder, a filename-agnostic scan) + # arbitrates nothing. One read below the save directory sits + # outside the BIOS tree, where no platform declares a file. + if outside_gap_scope(entry, is_standalone) or read_from_save_dir( + entry, is_standalone + ): continue # A member of a romset is not a file at the BIOS root: the archive # holding it occupies the destination (_archive_claims). Claiming @@ -291,7 +294,9 @@ def _archive_claims( """ files = [ entry for entry in profile.get("files") or [] - if isinstance(entry, dict) and not outside_gap_scope(entry, is_standalone) + if isinstance(entry, dict) + and not outside_gap_scope(entry, is_standalone) + and not read_from_save_dir(entry, is_standalone) ] prefix = profile.get("archive_prefix", "") claims: list[Claim] = [] diff --git a/scripts/validate_schemas.py b/scripts/validate_schemas.py index d7964f81..038b05ee 100644 --- a/scripts/validate_schemas.py +++ b/scripts/validate_schemas.py @@ -60,6 +60,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(_save_dir_without_library_name(path, data)) out.extend(_systems_outside_profile(path, data)) out.extend(_doubled_paths(path)) return out @@ -343,6 +344,88 @@ def _semantic_database_checks(database: dict) -> list[str]: return out +def _save_dir_without_library_name(path: Path, document: object) -> list[str]: + """A file the libretro build reads below the save directory, in a profile + naming no library_name. + + RetroArch hands each core / by default + (sort_savefiles_enable): without the name, packs and the installer + cannot say where the file goes, and leave it out. + """ + from validation import read_from_save_dir + + if not isinstance(document, dict): + return [] + if "library_name" in document: + return _save_folder_once_paths(path, document) + return [ + f"{path.relative_to(ROOT)}: files[{index}] ({entry.get('name')}) is read " + "below the save directory and the profile declares no library_name" + for index, entry in enumerate(document.get("files") or []) + if isinstance(entry, dict) + and read_from_save_dir(entry) + and not ("path" in entry and entry["path"] is None) + ] + + +def _save_folder_once_paths(path: Path, document: dict) -> list[str]: + """A core adding its own folder once: each save path starts with it. + + The pack places such a file at / whether the + frontend sorts saves by core or not, which holds only when the path + already names the folder the core reads below. + """ + from validation import read_from_save_dir + + if not document.get("save_folder_once"): + return [] + folder = f"{document.get('library_name') or ''}/" + return [ + f"{path.relative_to(ROOT)}: files[{index}] ({entry.get('name')}) does not " + f"start with {folder}, the folder save_folder_once says the core adds" + for index, entry in enumerate(document.get("files") or []) + if isinstance(entry, dict) + and read_from_save_dir(entry) + and entry.get("path") + and not str(entry["path"]).startswith(folder) + ] + + +def _save_directory_blocks() -> list[str]: + """Each registry save_directory is null or a folder and a by_core flag. + + The folder names the pack folder and the save directory's default place + beside the BIOS directory: one path segment. + """ + registry_path = ROOT / "platforms" / "_registry.yml" + if not registry_path.is_file(): + return [] + with registry_path.open(encoding="utf-8") as handle: + platforms = (yaml_load(handle) or {}).get("platforms") or {} + out: list[str] = [] + for name, entry in sorted(platforms.items()): + if not isinstance(entry, dict) or "save_directory" not in entry: + continue + block = entry["save_directory"] + if block is None: + continue + folder = block.get("folder") if isinstance(block, dict) else None + if ( + not isinstance(block, dict) + or set(block) != {"folder", "by_core"} + or not isinstance(folder, str) + or not folder + or re.search(r"[/\\]", folder) + or folder in (".", "..") + or not isinstance(block.get("by_core"), bool) + ): + out.append( + f"platforms/_registry.yml: {name}.save_directory must be null or " + "{folder: , by_core: }" + ) + return out + + def _semantic_install_checks(path: Path, manifest: dict) -> list[str]: out: list[str] = [] files = manifest.get("files", []) @@ -367,6 +450,24 @@ def _semantic_install_checks(path: Path, manifest: dict) -> list[str]: out.append( f"{path.relative_to(ROOT)}: destinations are both downloadable and omitted" ) + saved = [ + entry for entry in [*files, *manifest.get("case_variants", [])] + if entry.get("root") == "save" + ] + layout = manifest.get("save_directory") + if saved and not layout: + out.append( + f"{path.relative_to(ROOT)}: entries read below the save directory " + "and no save_directory" + ) + stray = sorted( + entry["dest"] for entry in [*files, *omitted] + if entry.get("core_folder") and entry.get("root") != "save" + ) + if stray: + out.append( + f"{path.relative_to(ROOT)}: core_folder outside root: save ({stray[0]})" + ) return out @@ -412,6 +513,7 @@ def main() -> int: ) errors.extend(_unknown_data_dir_refs()) errors.extend(_repository_layout_in_profiles()) + errors.extend(_save_directory_blocks()) if not args.source_only: database_path = ROOT / "database.json" diff --git a/scripts/validation.py b/scripts/validation.py index 6f216e67..7a8e9dad 100644 --- a/scripts/validation.py +++ b/scripts/validation.py @@ -429,16 +429,40 @@ def check_file_validation( def read_from_system_dir(file_entry: dict, standalone: bool = False) -> bool: """Whether the emulator reads this entry from the system directory. - A file loaded from the save, content or working directory is not a BIOS: - no platform lists it, no pack carries it, and the truth model must not - ask a platform to declare it. Two builds can read one file from two + A file loaded from the save, content or working directory is not in the + BIOS tree: no platform lists it, and the truth model must not ask a + platform to declare it. Packs place one read below the save directory + there (read_from_save_dir), never in the BIOS tree. Two builds can read one file from two places: libretro Dolphin keeps its User tree in the save directory, the standalone app in its own folder, so ``load_from`` can be keyed by build. """ + load_from = _load_from(file_entry, standalone) + return not load_from or load_from == "system_dir" + + +def _load_from(file_entry: dict, standalone: bool) -> str: + """The ``load_from`` value that applies to one build.""" load_from = file_entry.get("load_from", "") if isinstance(load_from, dict): load_from = load_from.get("standalone" if standalone else "libretro", "") - return not load_from or load_from == "system_dir" + return load_from or "" + + +def read_from_save_dir(file_entry: dict, standalone: bool = False) -> bool: + """Whether the libretro build reads this entry below the frontend's save + directory, the one RETRO_ENVIRONMENT_GET_SAVE_DIRECTORY returns. + + ``path`` is then relative to that directory. A standalone app has no + frontend to hand it one: its files are placed by ``standalone_path``. + """ + return not standalone and _load_from(file_entry, False) == "save_dir" + + +def entry_root(file_entry: dict, standalone: bool = False) -> str: + """The frontend directory an entry's ``path`` is relative to: ``save`` + for a file the libretro build reads below the save directory, else + ``system``.""" + return "save" if read_from_save_dir(file_entry, standalone) else "system" def outside_gap_scope(file_entry: dict, is_standalone: bool) -> bool: @@ -447,8 +471,10 @@ def outside_gap_scope(file_entry: dict, is_standalone: bool) -> bool: Read by the gap report and by verify's undeclared-file pass, so both set aside the same entries: a placeholder standing for a family of files, a file the user imports through the UI (``path: null``), an entry of the - build the platform does not run, a file read outside the system - directory, and a filename-agnostic entry the builder scans for itself. + build the platform does not run, a file read outside the system and + save directories, and a filename-agnostic entry the builder scans for + itself. A file the libretro build reads below the save directory is in + scope: the pack places it there (``entry_root``). """ name = file_entry.get("name", "") if "<" in name or ">" in name or "*" in name: @@ -460,9 +486,10 @@ def outside_gap_scope(file_entry: dict, is_standalone: bool) -> bool: mode == "libretro" and is_standalone ): return True - return not read_from_system_dir(file_entry, is_standalone) or bool( - file_entry.get("agnostic") - ) + return not ( + read_from_system_dir(file_entry, is_standalone) + or read_from_save_dir(file_entry, is_standalone) + ) or bool(file_entry.get("agnostic")) def case_folded_slots( diff --git a/scripts/verify.py b/scripts/verify.py index 2b0430d0..1800f56c 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -106,10 +106,12 @@ from validation import ( installed_with_emulator, unread_on, outside_gap_scope, + read_from_save_dir, with_archive_members, core_size_refusal, same_slot_entries, ) +from packpaths import core_save_folder DEFAULT_DB = "database.json" DEFAULT_PLATFORMS_DIR = "platforms" @@ -574,8 +576,14 @@ def find_undeclared_files( # The key used to skip that fallback, so clk's Acorn/basic.rom and # Electron/basic.rom both keyed as "basic.rom" under Batocera's # standalone clk and the second was dropped. + # A file the libretro build reads below the save directory is + # placed there, apart from the system directory the platform + # lays out: no declaration of the BIOS tree answers for it. + save_root = read_from_save_dir(f, is_standalone) if is_standalone: dest = f.get("standalone_path") or f.get("path") or fname + elif save_root: + dest = f.get("path") or fname else: dest = system_dir_path(f, f.get("path") or fname) raw_regions = f.get("region") or [] @@ -593,6 +601,10 @@ def find_undeclared_files( f.get("system"), f.get("variant_group"), region_key, + # Below the save directory each core is handed a folder + # of its own: two cores reading hiscore.dat there read + # two files. + ("save", core_save_folder(profile)) if save_root else None, ) if not fname or unread_on(f, config.get("_key", "")): continue @@ -601,8 +613,8 @@ def find_undeclared_files( if duplicate and (first is None or first["profile"] == emu_name): continue verdict = _candidate_verdict( - f, fname, is_standalone, include_all, declared_names, dest, - folded_slots, + f, fname, is_standalone, include_all, + {} if save_root else declared_names, dest, folded_slots, ) if verdict == "settled": seen_files.add(seen_key) @@ -742,12 +754,17 @@ def find_undeclared_files( } if f.get("contents"): entry["contents"] = f["contents"] - if f.get("system_dir"): + if save_root: + entry["root"] = "save" + folder = core_save_folder(profile) + if folder is not None: + entry["core_folder"] = folder + elif f.get("system_dir"): entry["system_dir"] = f["system_dir"] members = with_archive_members(f, profile.get("files", [])).get("members") if members: entry["members"] = members - if launcher_layer: + if launcher_layer and not save_root: launcher_candidates.append((emu_name, is_standalone, f, entry)) if duplicate: first.setdefault("other_claims", []).append(entry) @@ -857,6 +874,8 @@ def find_exclusion_notes( _UNPLACED_REASONS = { "no_platform_slug": "no platform slug for its system", "below_platform_folder": "below the folder the platform lists", + "no_save_directory": "read below a save directory the platform keeps in no fixed place", + "no_core_folder": "read below the save directory and the profile names no library_name", } diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 78388d20..5849ba14 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -3253,11 +3253,16 @@ class KidVidTapesStayWithTheContent(unittest.TestCase): working directory) or from its base directory (stella2023, the content folder, and stella, a folder under the save directory): never the system directory. Without load_from every Linux pack put about 68 MB of WAVs at - the BIOS root, in two cases.""" + the BIOS root, in two cases. The save directory is a place a pack + reaches: stella's tapes go there, never to the BIOS tree.""" def test_no_pack_places_them_in_the_system_directory(self): from scripts.common import load_emulator_profiles - from scripts.validation import outside_gap_scope + from scripts.validation import ( + outside_gap_scope, + read_from_save_dir, + read_from_system_dir, + ) profiles = load_emulator_profiles(str(ROOT / "emulators")) for name in ("stella", "stella2014", "stella2023"): @@ -3265,7 +3270,11 @@ class KidVidTapesStayWithTheContent(unittest.TestCase): if f["name"].lower().endswith(".wav")] with self.subTest(profile=name): self.assertEqual(len(tapes), 7) - self.assertTrue(all(outside_gap_scope(f, False) for f in tapes)) + self.assertFalse(any(read_from_system_dir(f, False) for f in tapes)) + for f in tapes: + self.assertTrue( + outside_gap_scope(f, False) or read_from_save_dir(f, False) + ) class ModuleConstantsDeclaredOnce(unittest.TestCase): diff --git a/tests/test_libretro_layout.py b/tests/test_libretro_layout.py index ff982984..76b08980 100644 --- a/tests/test_libretro_layout.py +++ b/tests/test_libretro_layout.py @@ -18,7 +18,11 @@ REPO_ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(REPO_ROOT / "scripts")) from packpaths import _resolve_destination # noqa: E402 -from validation import outside_gap_scope, read_from_system_dir # noqa: E402 +from validation import ( # noqa: E402 + outside_gap_scope, + read_from_save_dir, + read_from_system_dir, +) class PathIsTheDestination(unittest.TestCase): @@ -38,7 +42,11 @@ class LoadFromPerBuild(unittest.TestCase): def test_each_build_reads_its_own_place(self): self.assertFalse(read_from_system_dir(self.ENTRY, False)) self.assertTrue(read_from_system_dir(self.ENTRY, True)) - self.assertTrue(outside_gap_scope(self.ENTRY, False)) + # The libretro build's copy goes below the save directory, the + # standalone one below the folder the platform hands it. + self.assertTrue(read_from_save_dir(self.ENTRY, False)) + self.assertFalse(read_from_save_dir(self.ENTRY, True)) + self.assertFalse(outside_gap_scope(self.ENTRY, False)) self.assertFalse(outside_gap_scope(self.ENTRY, True)) def test_a_plain_value_holds_for_both(self): @@ -67,11 +75,23 @@ class RetroArchCarriesDolphinWhereItReads(unittest.TestCase): dolphin = [ e["destination"] for e in extras if e.get("source_emulator") in ("Dolphin", "Ishiiruka") + and e.get("root") != "save" ] self.assertTrue(dolphin) self.assertEqual( [d for d in dolphin if not d.startswith("dolphin-emu/Sys/")], [] ) + # The User tree goes below the save directory, each core's folder + # first (RetroArch sorts saves by core). + user = { + e["pack_path"] for e in extras + if e.get("source_emulator") in ("Dolphin", "Ishiiruka") + and e.get("root") == "save" + } + self.assertIn("saves/dolphin-emu/User/GBA/gba_bios.bin", user) + self.assertTrue(all( + p.startswith(("saves/dolphin-emu/User/", "saves/Ishiiruka/User/")) for p in user + )) self.assertIn("dolphin-emu/Sys/Triforce/segaboot.gcm", dolphin) diff --git a/tests/test_native_mode.py b/tests/test_native_mode.py index fa7aac4e..e913bb93 100644 --- a/tests/test_native_mode.py +++ b/tests/test_native_mode.py @@ -330,8 +330,13 @@ class CandidateVerdict(unittest.TestCase): ) def test_a_file_read_from_elsewhere_is_not_a_bios_gap(self): + for where in ("content_dir", "working_directory", "internal_storage"): + self.assertEqual( + self.verdict({"name": "save.bin", "load_from": where}), "skip" + ) + # Below the save directory: the pack places it there. self.assertEqual( - self.verdict({"name": "save.bin", "load_from": "save_dir"}), "skip" + self.verdict({"name": "save.bin", "load_from": "save_dir"}), "keep" ) self.assertEqual( self.verdict({"name": "b.bin", "load_from": "system_dir"}), "keep"