diff --git a/docs/config_schema.md b/docs/config_schema.md index 34eb4d24..493c324d 100644 --- a/docs/config_schema.md +++ b/docs/config_schema.md @@ -127,6 +127,12 @@ respective files. ### Multi-disc selection +Relative paths in executable-side `settings.toml` (`[disc] path`, `[bios] path`, +`[memcard] dir`, `card1`, `card2`) and in `disc.cfg` / `bios.cfg` resolve from the +executable directory, including when launched from another working directory. +Absolute paths and UNC paths keep their original location. A relative `--disc` +command-line argument still resolves from the caller's working directory. + A build whose `discs` array has more than one entry grows a **Disc Selection** dropdown in the launcher, above the Serial/Region/ISO-header checklist. The choice is persisted in `settings.toml`: diff --git a/recompiler/CMakeLists.txt b/recompiler/CMakeLists.txt index 4ec44ad8..58a2218f 100644 --- a/recompiler/CMakeLists.txt +++ b/recompiler/CMakeLists.txt @@ -698,6 +698,21 @@ if(BUILD_TESTING) target_link_libraries(rewind_settings_test PRIVATE fmt) add_test(NAME rewind_settings_test COMMAND rewind_settings_test) + # Launcher-saved paths stay relative inside a portable game folder. + add_executable(portable_settings_paths_test + tests/portable_settings_paths_test.cpp + src/config_loader.cpp + src/ps1_exe_parser.cpp + ) + target_include_directories(portable_settings_paths_test PRIVATE + include + src + lib/fmt/include + lib/toml11 + ) + target_link_libraries(portable_settings_paths_test PRIVATE fmt) + add_test(NAME portable_settings_paths_test COMMAND portable_settings_paths_test) + # Needs bios/SCPH1001.toml — run from the framework root. add_test(NAME bios_address_model_test COMMAND bios_address_model_test diff --git a/recompiler/src/config_loader.cpp b/recompiler/src/config_loader.cpp index 60fb00c5..b137e00f 100644 --- a/recompiler/src/config_loader.cpp +++ b/recompiler/src/config_loader.cpp @@ -2783,6 +2783,19 @@ UserSettings load_user_settings(const fs::path& path) { return s; } +fs::path relative_to_folder(const fs::path& p, const fs::path& folder) { + if (p.empty() || folder.empty() || !p.is_absolute()) return p; + std::error_code ec; + const fs::path base = fs::weakly_canonical(folder, ec); + if (ec) return p; + const fs::path full = fs::weakly_canonical(p, ec); + if (ec) return p; + const fs::path r = full.lexically_relative(base); + // Outside the folder (other drive, or a "../" climb) stays absolute. + if (r.empty() || r.is_absolute() || *r.begin() == "..") return p; + return r; +} + bool save_user_settings(const fs::path& path, const UserSettings& s) { std::error_code ec; if (!path.parent_path().empty()) @@ -2796,6 +2809,11 @@ bool save_user_settings(const fs::path& path, const UserSettings& s) { std::string str = p.generic_string(); return str; }; + // Paths inside the game folder are stored relative to it, so a portable + // folder still finds its disc, BIOS and memory cards after it is moved or + // copied to another PC. Readers anchor relative paths on the exe + // directory, which is where settings.toml lives. + auto rel = [&](const fs::path& p) { return fwd(relative_to_folder(p, path.parent_path())); }; f << "# psxrecomp user settings - written by the launcher. Safe to hand-edit.\n"; f << "# Overrides the bundled game.toml; the command line overrides this file.\n\n"; @@ -2898,11 +2916,11 @@ bool save_user_settings(const fs::path& path, const UserSettings& s) { f << "lobby_url = \"" << s.netplay_lobby_url << "\"\n"; } if (s.has_bios_path) - f << "\n[bios]\npath = \"" << fwd(s.bios_path) << "\"\n"; + f << "\n[bios]\npath = \"" << rel(s.bios_path) << "\"\n"; if (s.has_disc_path || s.has_disc_index) { f << "\n[disc]\n"; if (s.has_disc_path) - f << "path = \"" << fwd(s.disc_path) << "\"\n"; + f << "path = \"" << rel(s.disc_path) << "\"\n"; /* Only meaningful for a multi-disc title; harmless (and informative) * for a single-disc one, where it is always 1. */ if (s.has_disc_index) @@ -2912,11 +2930,11 @@ bool save_user_settings(const fs::path& path, const UserSettings& s) { s.has_memcard1_enabled || s.has_memcard2_enabled) { f << "\n[memcard]\n"; if (s.has_memcard_dir) - f << "dir = \"" << fwd(s.memcard_dir) << "\"\n"; + f << "dir = \"" << rel(s.memcard_dir) << "\"\n"; if (s.has_memcard1_path) - f << "card1 = \"" << fwd(s.memcard1_path) << "\"\n"; + f << "card1 = \"" << rel(s.memcard1_path) << "\"\n"; if (s.has_memcard2_path) - f << "card2 = \"" << fwd(s.memcard2_path) << "\"\n"; + f << "card2 = \"" << rel(s.memcard2_path) << "\"\n"; if (s.has_memcard1_enabled) f << "enable1 = " << (s.memcard1_enabled ? "true" : "false") << "\n"; if (s.has_memcard2_enabled) diff --git a/recompiler/src/config_loader.h b/recompiler/src/config_loader.h index 8e53322b..23ed4217 100644 --- a/recompiler/src/config_loader.h +++ b/recompiler/src/config_loader.h @@ -1450,6 +1450,11 @@ UserSettings load_user_settings(const std::filesystem::path& path); // Write settings.toml deterministically. Returns false on I/O failure. bool save_user_settings(const std::filesystem::path& path, const UserSettings& s); +// `p` relative to `folder` when it lies inside it; otherwise `p` unchanged. +// Keeps portable game folders portable once the launcher saves a path. +std::filesystem::path relative_to_folder(const std::filesystem::path& p, + const std::filesystem::path& folder); + // Surgical upsert of `key = true|false` under [controller] in game.toml. // Preserves comments and unrelated keys. Creates [controller] if missing. // Used to persist launcher multitap_analog into the title game config. diff --git a/recompiler/tests/portable_settings_paths_test.cpp b/recompiler/tests/portable_settings_paths_test.cpp new file mode 100644 index 00000000..ddfba54f --- /dev/null +++ b/recompiler/tests/portable_settings_paths_test.cpp @@ -0,0 +1,78 @@ +/* portable_settings_paths_test + * + * A portable game folder must keep working after it is moved or copied to + * another PC. The launcher saves the disc, BIOS and memory-card paths the + * player used; saved as absolute paths they pinned the folder to its first + * location. What this pins: + * + * 1. a path inside the settings folder is written relative to it; + * 2. a path outside it (a NAS disc, another drive) stays absolute; + * 3. the relative form reads back unchanged, so the reader's exe-directory + * anchoring resolves it on the new PC. + */ +#include "config_loader.h" + +#include +#include +#include +#include +#include + +namespace fs = std::filesystem; +using PSXRecompV4::relative_to_folder; + +static int failures = 0; + +static void check(bool cond, const char* what) { + if (!cond) { + std::fprintf(stderr, "FAIL: %s\n", what); + ++failures; + } +} + +int main() { + const fs::path game = fs::temp_directory_path() / "psx portable paths test" / "game"; + std::error_code ec; + fs::remove_all(game.parent_path(), ec); + fs::create_directories(game / "inputs" / "disc-1"); + fs::create_directories(game.parent_path() / "elsewhere"); + + const fs::path disc = game / "inputs" / "disc-1" / "Game (Europe).chd"; + const fs::path outside = game.parent_path() / "elsewhere" / "bios.bin"; + + check(relative_to_folder(disc, game).generic_string() == "inputs/disc-1/Game (Europe).chd", + "a path inside the folder becomes relative"); + check(relative_to_folder(outside, game) == outside, "a path outside the folder stays absolute"); + check(relative_to_folder(fs::path("inputs/x.bin"), game) == fs::path("inputs/x.bin"), + "an already relative path is unchanged"); + check(relative_to_folder(fs::path(), game).empty(), "an empty path stays empty"); + + PSXRecompV4::UserSettings s{}; + s.bios_path = game / "inputs" / "[EU] SCPH5552.BIN"; s.has_bios_path = true; + s.disc_path = disc; s.has_disc_path = true; + s.memcard_dir = game / "saves"; s.has_memcard_dir = true; + s.memcard1_path = game / "saves" / "card1.mcd"; s.has_memcard1_path = true; + s.memcard2_path = outside; s.has_memcard2_path = true; + const fs::path settings = game / "settings.toml"; + check(PSXRecompV4::save_user_settings(settings, s), "settings save"); + + std::stringstream text; + text << std::ifstream(settings).rdbuf(); + const std::string body = text.str(); + check(body.find("path = \"inputs/[EU] SCPH5552.BIN\"") != std::string::npos, "BIOS saved relative"); + check(body.find("path = \"inputs/disc-1/Game (Europe).chd\"") != std::string::npos, "disc saved relative"); + check(body.find("dir = \"saves\"") != std::string::npos, "memory card folder saved relative"); + check(body.find("card1 = \"saves/card1.mcd\"") != std::string::npos, "card 1 saved relative"); + check(body.find(outside.generic_string()) != std::string::npos, "outside card stays absolute"); + + const PSXRecompV4::UserSettings back = PSXRecompV4::load_user_settings(settings); + check(back.has_disc_path && back.disc_path == fs::path("inputs/disc-1/Game (Europe).chd"), + "relative disc path reads back unchanged"); + check(back.has_memcard1_path && back.memcard1_path == fs::path("saves/card1.mcd"), + "relative card path reads back unchanged"); + + fs::remove_all(game.parent_path(), ec); + if (failures) return 1; + std::printf("portable_settings_paths_test: all checks passed\n"); + return 0; +} diff --git a/runtime/CMakeLists.txt b/runtime/CMakeLists.txt index 157fd8d2..662dd3da 100644 --- a/runtime/CMakeLists.txt +++ b/runtime/CMakeLists.txt @@ -1020,6 +1020,7 @@ if(BUILD_TESTING) add_test(NAME setup_bios_assets_test COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_setup_bios_assets.py) if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") add_test(NAME launch_disc_path_test COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_launch_disc_path.py --compiler ${CMAKE_CXX_COMPILER}) + add_test(NAME persisted_settings_paths_test COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_persisted_settings_paths.py --compiler ${CMAKE_CXX_COMPILER}) endif() add_test(NAME disc_picker_ascii_test COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_disc_picker_ascii.py) diff --git a/runtime/src/main.cpp b/runtime/src/main.cpp index 4078fdcd..d6b585b4 100644 --- a/runtime/src/main.cpp +++ b/runtime/src/main.cpp @@ -2450,6 +2450,17 @@ static std::filesystem::path sidecar_cfg_path(const char* argv0, const char* fil return exe_dir_from_argv(argv0) / filename; } +/* Relative paths in settings.toml and the bios.cfg / disc.cfg sidecars are + * relative to the game folder. Resolve them against the exe directory, never + * the working directory: a shortcut or frontend that starts the game from + * elsewhere must still find its own memory cards and BIOS. Absolute paths are + * returned unchanged; empty stays empty. */ +static std::filesystem::path anchor_on_exe_dir(const char* argv0, + const std::filesystem::path& p) { + if (p.empty()) return p; + return PSXRecompV4::host_resolve(exe_dir_from_argv(argv0), p); +} + static std::filesystem::path read_cached_path(const char* argv0, const char* filename) { std::ifstream f(sidecar_cfg_path(argv0, filename)); if (!f.is_open()) return {}; @@ -2458,13 +2469,17 @@ static std::filesystem::path read_cached_path(const char* argv0, const char* fil while (!line.empty() && (line.back() == '\n' || line.back() == '\r')) { line.pop_back(); } - return line.empty() ? std::filesystem::path{} : std::filesystem::path(line); + return line.empty() ? std::filesystem::path{} + : anchor_on_exe_dir(argv0, std::filesystem::path(line)); } static void write_cached_path(const char* argv0, const char* filename, const std::filesystem::path& path) { std::ofstream f(sidecar_cfg_path(argv0, filename), std::ios::trunc); - if (f.is_open()) f << path.string() << "\n"; + // Relative inside the game folder so a moved portable folder still works; + // read_cached_path anchors relative paths on the exe directory. + if (f.is_open()) + f << PSXRecompV4::relative_to_folder(path, exe_dir_from_argv(argv0)).generic_string() << "\n"; } static void launcher_warning(const char* title, const std::string& msg) { @@ -2650,6 +2665,12 @@ static std::filesystem::path normalize_disc_path_for_launch(const std::filesyste return PSXRecompV4::resolve_disc_path(path).mount; } +static std::filesystem::path resolve_persisted_disc_path( + const std::filesystem::path& path, const std::filesystem::path& exe_dir) { + if (path.empty()) return {}; + return normalize_disc_path_for_launch(PSXRecompV4::host_resolve(exe_dir, path)); +} + /* Which image of a MULTI-DISC set to mount, given the roster this build was * made from (game.toml [game] discs), the player's persisted [disc] selected * index, and the [disc] path the launcher last wrote. @@ -2937,7 +2958,7 @@ static std::filesystem::path resolve_disc_for_runtime(const std::filesystem::pat std::filesystem::path cached = read_cached_path(argv0, "disc.cfg"); if (!cached.empty()) { - cached = normalize_disc_path_for_launch(cached); + cached = resolve_persisted_disc_path(cached, exe_dir_from_argv(argv0)); } if (!cached.empty() && std::filesystem::exists(cached) && validate_disc_for_launch(cached, game_id)) { @@ -14082,12 +14103,12 @@ int main(int argc, char** argv) { g_hotkey_pad_fast_forward_toggle = normalize_hotkey_pad_binding( us.hotkey_pad_fast_forward_toggle, 0); if (us.has_bios_path && !bios_from_cli && !us.bios_path.empty()) { - settings_bios_storage = us.bios_path.string(); + settings_bios_storage = anchor_on_exe_dir(argv[0], us.bios_path).string(); bios_path = settings_bios_storage.c_str(); bios_explicit = true; } if (us.has_disc_path && !disc_override_path) - resolved_disc = normalize_disc_path_for_launch(us.disc_path); + resolved_disc = resolve_persisted_disc_path(us.disc_path, exe_dir_from_argv(argv[0])); /* Multi-disc precedence. [disc] selected is authoritative ONLY WHEN * PRESENT; absent it, [disc] path decides and the index is derived * from it. @@ -14120,9 +14141,11 @@ int main(int argc, char** argv) { if (idx >= 0) selected_disc_index = idx + 1; } } - if (us.has_memcard_dir) memcard_dir = us.memcard_dir; - if (us.has_memcard1_path) memcard1_path = us.memcard1_path; - if (us.has_memcard2_path) memcard2_path = us.memcard2_path; + /* Relative [memcard] values anchor on the exe directory; the + * per-game options file also lives in memcard_dir. */ + if (us.has_memcard_dir) memcard_dir = anchor_on_exe_dir(argv[0], us.memcard_dir); + if (us.has_memcard1_path) memcard1_path = anchor_on_exe_dir(argv[0], us.memcard1_path); + if (us.has_memcard2_path) memcard2_path = anchor_on_exe_dir(argv[0], us.memcard2_path); if (us.has_memcard1_enabled) memcard1_enabled = us.memcard1_enabled; if (us.has_memcard2_enabled) memcard2_enabled = us.memcard2_enabled; if (us.has_multitap_enabled) multitap_enabled = us.multitap_enabled; @@ -15455,8 +15478,8 @@ int main(int argc, char** argv) { } memcard1_enabled = seed.memcard1_enabled; memcard2_enabled = seed.memcard2_enabled; - if (seed.has_memcard1_path) memcard1_path = seed.memcard1_path; - if (seed.has_memcard2_path) memcard2_path = seed.memcard2_path; + if (seed.has_memcard1_path) memcard1_path = anchor_on_exe_dir(argv[0], seed.memcard1_path); + if (seed.has_memcard2_path) memcard2_path = anchor_on_exe_dir(argv[0], seed.memcard2_path); if (seed.has_language) resolved_language = seed.language; { const int n = std::min(PSX_MAX_PLAYERS, diff --git a/runtime/tests/test_persisted_settings_paths.py b/runtime/tests/test_persisted_settings_paths.py new file mode 100644 index 00000000..9dca4abf --- /dev/null +++ b/runtime/tests/test_persisted_settings_paths.py @@ -0,0 +1,126 @@ +"""Relative paths persisted in settings.toml and the bios.cfg / disc.cfg +sidecars resolve from the executable directory, not the working directory. + +Compiles the real adapters from main.cpp (normalize_disc_path_for_launch, +resolve_persisted_disc_path, anchor_on_exe_dir) into a probe and runs it from +an unrelated working directory. Also checks that every persisted-path consumer +in main.cpp goes through an adapter. No game data is used. +""" +import argparse +import os +from pathlib import Path +import shutil +import subprocess +import tempfile + +# Every place main.cpp reads a persisted path must anchor it on the exe +# directory. Each line below must appear in main.cpp exactly as written. +CALL_SITES = [ + 'resolved_disc = resolve_persisted_disc_path(us.disc_path, exe_dir_from_argv(argv[0]))', + 'cached = resolve_persisted_disc_path(cached, exe_dir_from_argv(argv0))', + ': anchor_on_exe_dir(argv0, std::filesystem::path(line));', + 'settings_bios_storage = anchor_on_exe_dir(argv[0], us.bios_path).string();', + 'memcard_dir = anchor_on_exe_dir(argv[0], us.memcard_dir);', + 'memcard1_path = anchor_on_exe_dir(argv[0], us.memcard1_path);', + 'memcard2_path = anchor_on_exe_dir(argv[0], us.memcard2_path);', + 'memcard1_path = anchor_on_exe_dir(argv[0], seed.memcard1_path);', + 'memcard2_path = anchor_on_exe_dir(argv[0], seed.memcard2_path);', +] + + +def run(args, **kwargs): + return subprocess.run(args, capture_output=True, text=True, + encoding='utf-8', errors='replace', **kwargs) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument('--compiler', default=os.environ.get('CXX', 'c++')) + args = parser.parse_args() + root = Path(__file__).resolve().parents[1] + text = (root / 'src/main.cpp').read_text(encoding='utf-8') + for site in CALL_SITES: + assert site in text, 'main.cpp no longer anchors: ' + site + adapters = [] + for name in ('normalize_disc_path_for_launch', 'resolve_persisted_disc_path', + 'anchor_on_exe_dir'): + start = text.index('static std::filesystem::path ' + name + '(') + adapters.append(text[start:text.index('\n}\n', start) + 2]) + with tempfile.TemporaryDirectory(prefix='persisted paths spaces ') as tmp: + base = Path(tmp) + product, cwd = base / 'moved product', base / 'unrelated cwd' + (product / 'inputs').mkdir(parents=True) + (cwd / 'inputs').mkdir(parents=True) + relative = 'inputs/Some Game [Disc 1] (v1.1).chd' + (product / relative).write_bytes(b'authored placeholder') + (cwd / relative).write_bytes(b'wrong cwd decoy') + source = base / 'probe.cpp' + # The probe's exe_dir_from_argv() stands in for the real one: the + # "argv0" it receives is the product directory itself. + source.write_text('''#include +#include +#include +#include "host_path.h" +#include "disc_path.h" +static std::filesystem::path exe_dir_from_argv(const char* dir) { + return std::filesystem::path(dir); +} +''' + '\n'.join(adapters) + ''' +int main(int argc, char** argv) { + (void)argc; + const std::string mode = argv[1]; + const std::filesystem::path path = argv[2]; + const std::filesystem::path resolved = + mode == "persisted" ? resolve_persisted_disc_path(path, argv[3]) + : mode == "anchor" ? anchor_on_exe_dir(argv[3], path) + : normalize_disc_path_for_launch(path); + // libstdc++'s generic_string() collapses a UNC "\\\\server" root to "/server"; + // print the native spelling with forward slashes so both C++ libraries agree. + std::string out = resolved.string(); + for (char& c : out) if (c == '\\\\') c = '/'; + std::cout << out; +} +''', encoding='utf-8') + compiler = shutil.which(args.compiler) or args.compiler + env = os.environ.copy() + env['PATH'] = str(Path(compiler).parent) + os.pathsep + env.get('PATH', '') + exe = base / ('probe.exe' if os.name == 'nt' else 'probe') + subprocess.run([compiler, '-std=c++17', '-Werror=return-type', '-I'+str(root/'include'), + '-I'+str(root.parent/'recompiler/include'), str(source), + str(root/'src/disc_path.cpp'), str(root/'src/cue_sheet.cpp'), '-o', str(exe)], + check=True, env=env) + + def probe(mode, path): + result = run([str(exe), mode, str(path), str(product)], cwd=cwd, env=env) + assert result.returncode == 0, (mode, path, result.stderr) + return result.stdout + + def check(mode, path, expected): + out = probe(mode, path) + assert out == expected, (mode, path, out, expected) + + def check_same(mode, path, expected): + out = probe(mode, path) + assert Path(out).resolve() == Path(expected).resolve(), (mode, path, out, expected) + + # [disc] path and disc.cfg + check('persisted', relative, (product/relative).as_posix()) + check('persisted', product/relative, (product/relative).as_posix()) + check('persisted', '', '') + check('persisted', 'inputs/missing.chd', (product/'inputs/missing.chd').as_posix()) + check('cli', relative, (cwd/relative).as_posix()) + if os.name == 'nt': + check('persisted', r'\\server\share\Some Game [Disc 1].chd', + '//server/share/Some Game [Disc 1].chd') + # [bios] path, bios.cfg and [memcard] dir/card1/card2 + check_same('anchor', '.', product) + check_same('anchor', 'cards/card1.mcd', product/'cards/card1.mcd') + check_same('anchor', 'bios/SCPH1001.BIN', product/'bios/SCPH1001.BIN') + check('anchor', product/'cards', (product/'cards').as_posix()) + check('anchor', '', '') + print('persisted paths: disc (relative, absolute, empty, missing, CLI cwd, ' + 'Windows UNC) and BIOS/memory card (".", relative, absolute, empty) passed') + + +if __name__ == '__main__': + main()