From eb262c4b23e3af2c9bdc0ecf14833525de331fd1 Mon Sep 17 00:00:00 2001 From: Matthew Stanley <1379tech@gmail.com> Date: Mon, 27 Jul 2026 17:33:44 -0700 Subject: [PATCH] feat(mods): let a trusted plugin read its own option values The manifest schema already carries typed, validated, launcher-rendered, persisted options ([[option]] boolean/choice/integer), and the launcher provider already reads them. A trusted activation plugin could not: the callback takes no arguments, so a plugin was only ever an on/off switch. That forced a parameterised feature to be modelled as one feature per value, and `constraint` only expresses ordered_integer WITHIN a feature, so those pseudo-features could not even be made mutually exclusive. A game wanting "Widescreen: 16:9 / 21:9" or "Interpolation: 90/120/144/165/240" had no way to express it as one entry. psx_mod_option_value(package, feature, option, out, out_size) closes the gap against the value the player actually committed. It returns 0 with an empty string when the plan is not committed or the ids do not resolve, so a caller applies its own default rather than reading "" as a selection. Ids are explicit because registration is by plugin id alone and the callback carries no package/feature context. Also in this commit, from the same investigation: * Sustained overlay-autocompile failure is now LOUD. After 3 consecutive failed runs the runtime prints once to stdout: what failed, the exit code, that overlays are staying interpreted and frame times will be far worse than the build is capable of, what to check, and the compiler's own output tail. Previously the only trace was a `fails` counter reachable over TCP, so a wholly-interpreted title presented as "why is this slow" rather than as a message. Threshold is 3, not 1, because isolated failures (a shard racing a capture rewrite, a transient lock) are ordinary and self-heal. Measured cost of the silent case on Tomba!: p50 frame period 105ms, vs 3.1ms once shards existed. * The autocompile child's working directory is printed at startup beside cache and captures. Every relative path in overlay_autocompile_cmd resolves against it and it comes from game.toml's detected project root, so a game.toml staged outside the real root silently repoints the entire compile command with no other symptom. * A build tree whose cache still carries recomp-ui's OFF for RECOMP_UI_ENABLE_MODS now says so at configure time. 610234a's seed only fires on a fresh cache, and a stale OFF is indistinguishable from a deliberate one, so such trees kept producing Mods-less builds across reconfigures - the same "reads as a stale build" failure one level up, and invisible. The explicit-OFF contract is deliberate so this does not override it; it makes the state audible. Runtime ctest 16/16. Warning verified to fire on a tree configured with -DRECOMP_UI_ENABLE_MODS=OFF and to stay silent on a fresh configure. Co-Authored-By: Claude Opus 5 --- runtime/include/mod_plugins.h | 23 ++++++++++++++++ runtime/runtime.cmake | 23 ++++++++++++++++ runtime/src/autocompile.c | 50 +++++++++++++++++++++++++++++++++++ runtime/src/main.cpp | 13 +++++++-- runtime/src/mod_runtime.cpp | 22 +++++++++++++++ 5 files changed, 129 insertions(+), 2 deletions(-) diff --git a/runtime/include/mod_plugins.h b/runtime/include/mod_plugins.h index 7e44cc6a..1a5016da 100644 --- a/runtime/include/mod_plugins.h +++ b/runtime/include/mod_plugins.h @@ -24,6 +24,29 @@ int psx_mod_game_started(void); uint8_t psx_mod_read_byte(uint32_t address); void psx_mod_write_byte(uint32_t address, uint8_t value); +/* + * Read the committed value of one of this package's declared options, as the + * player left it in the launcher (or the manifest default when untouched). + * Writes a NUL-terminated string into `out` and returns 1; returns 0 with + * out[0] = '\0' when the plan is not committed, the ids do not resolve, or the + * value does not fit — the caller then applies its own default rather than + * treating an empty string as a selection. + * + * Why this exists: the manifest schema already carries typed, validated, + * launcher-rendered, persisted options ([[option]] boolean/choice/integer), but + * an activation callback takes no arguments and had no way to read them, so a + * trusted plugin could only ever be an on/off switch. A parameterised feature + * then had to be modelled as one feature per value — and `constraint` only + * expresses ordered_integer WITHIN a feature, so those pseudo-features could + * not even be made mutually exclusive. This closes that gap: one feature, one + * option, the plugin reads what was chosen. + * + * Ids are passed explicitly because registration is by plugin id alone and the + * callback carries no package/feature context. + */ +int psx_mod_option_value(const char* package_id, const char* feature_id, + const char* option_id, char* out, uint32_t out_size); + /* * Request a fixed host display aspect before renderer/window initialization. * Intended for activation callbacks that move a game's widescreen enhancement diff --git a/runtime/runtime.cmake b/runtime/runtime.cmake index 0926d6df..ba901aed 100644 --- a/runtime/runtime.cmake +++ b/runtime/runtime.cmake @@ -799,6 +799,29 @@ function(psxrecomp_add_runtime_target target) set(RECOMP_UI_ENABLE_MODS ON CACHE BOOL "Enable the recomp-ui Mods view (psxrecomp ships mod packages)") endif() + # The seed above only fires on a FRESH cache. A build tree configured + # before it existed already has RECOMP_UI_ENABLE_MODS=OFF in its cache, + # written by recomp-ui's own option(), and nothing can distinguish that + # stale default from a deliberate -DRECOMP_UI_ENABLE_MODS=OFF. Such a + # tree therefore keeps producing a Mods-less build across reconfigures, + # which is the same "reads as a stale build" failure the seed was added + # to prevent -- just one level up, and invisible. + # + # The explicit-OFF contract above is deliberate, so this does not + # override it. It makes the state audible instead: whoever sees a + # Mods-less build now gets told why and how to change it, rather than + # having to query the running game to discover the panel was compiled + # out. Deliberate opt-outs get one line per configure, which is the + # price of the two cases being genuinely indistinguishable. + if(NOT RECOMP_UI_ENABLE_MODS) + message(WARNING + "RECOMP_UI_ENABLE_MODS is OFF in this build tree, so the " + "launcher will have NO Mods page even for a title that ships a " + "catalog.\n" + " If that was not deliberate, this cache predates the " + "framework opting in: delete CMakeCache.txt (or just that " + "entry) to pick up the ON default.") + endif() include("${RECOMP_UI_ROOT}/recomp_ui.cmake") set(_psx_recomp_ui_args) diff --git a/runtime/src/autocompile.c b/runtime/src/autocompile.c index def9f1a8..6db99553 100644 --- a/runtime/src/autocompile.c +++ b/runtime/src/autocompile.c @@ -59,6 +59,25 @@ static uint64_t autocompile_now_ms(void) { #endif } +/* Loud-once threshold. One or two failures are ordinary (a shard racing a + * capture rewrite, a transient file lock) and self-heal on the next run. A + * SUSTAINED failure run means no overlay will ever go native, so the title + * runs wholly interpreted — a 10-30x frame-cost difference. That used to be + * visible only by querying autocompile_status over TCP and noticing fails == + * runs, which in practice meant it was discovered by someone saying "why is + * this slow" (the CWD-relative overlay_autocompile_cmd class of breakage: + * every build dir outside the game repo root silently degraded to the + * interpreter). Report it on stdout once, with the child's own output. */ +#define AC_LOUD_AFTER_FAILS 3u +static int s_reported_broken = 0; +/* Defined below, once the child-output tail ring it reads is declared. */ +static void autocompile_report_broken_once(void); +#ifndef _WIN32 +/* The compile spawner and its output ring are Windows-only in this file, so + * there is no child output to quote off-Windows and nothing to report. */ +static void autocompile_report_broken_once(void) { } +#endif + static void autocompile_note_failure(void) { s_fails++; if (s_consecutive_fails < 31u) s_consecutive_fails++; @@ -68,6 +87,7 @@ static void autocompile_note_failure(void) { uint64_t delay = AC_RETRY_INITIAL_MS << shift; if (delay > AC_RETRY_MAX_MS) delay = AC_RETRY_MAX_MS; s_retry_not_before_ms = autocompile_now_ms() + delay; + autocompile_report_broken_once(); } static void autocompile_note_success(void) { @@ -133,6 +153,36 @@ static char s_child_line[1024]; static int s_child_line_len = 0; static int s_child_line_overflow = 0; +/* See the forward declaration above for why this is loud. */ +static void autocompile_report_broken_once(void) { + if (s_reported_broken || s_consecutive_fails < AC_LOUD_AFTER_FAILS) return; + s_reported_broken = 1; + char tail[AC_OUT_CAP]; + int n = 0; + if (s_out_lock_init) { + EnterCriticalSection(&s_out_lock); + n = s_out_len < (int)sizeof tail - 1 ? s_out_len : (int)sizeof tail - 1; + if (n > 0) memcpy(tail, s_out + (s_out_len - n), (size_t)n); + LeaveCriticalSection(&s_out_lock); + } + tail[n > 0 ? n : 0] = '\0'; + fprintf(stdout, + "psxrecomp: WARNING: overlay autocompile has failed %u consecutive " + "runs (last exit %d).\n" + " Nothing is being compiled to native code, so overlay execution " + "stays in the interpreter and\n" + " frame times will be far worse than this build is capable of.\n" + " Check [runtime] overlay_autocompile_cmd in game.toml: every path in " + "it must resolve from the\n" + " process working directory, and the recompiler and " + "tools/compile_overlays.py it names must exist.\n" + " Last compiler output:\n%s%s", + s_consecutive_fails, (int)s_exit_code, + n > 0 ? tail : " (no output captured)\n", + (n > 0 && tail[n - 1] != '\n') ? "\n" : ""); + fflush(stdout); +} + /* Parse result markers while stdout is streaming, not from s_out after exit. * The configured child command may chain post-processing after * compile_overlays.py (coverage_vault.py is the production example). That diff --git a/runtime/src/main.cpp b/runtime/src/main.cpp index 8b1083ef..63fb8bea 100644 --- a/runtime/src/main.cpp +++ b/runtime/src/main.cpp @@ -5328,10 +5328,19 @@ int main(int argc, char** argv) { } autocompile_configure(ac_cmd->c_str(), ac_cwd.c_str()); overlay_autocapture_set_enabled(1); + /* cwd is printed because every relative path in + * overlay_autocompile_cmd resolves against it, and it is + * derived from game.toml's detected project root. Staging a + * game.toml somewhere that is not the real project root + * therefore repoints the whole compile command, and the only + * symptom is that nothing ever goes native and frame times + * are 10-30x worse. Printing the root makes that a one-line + * diagnosis instead of a TCP query against + * autocompile_status. */ std::fprintf(stdout, - "psxrecomp: overlay autocompile enabled (%s); cache=%s; captures=%s\n", + "psxrecomp: overlay autocompile enabled (%s); cache=%s; captures=%s; cwd=%s\n", overlay_backend_name(eff), cache_dir.c_str(), - captures_path.string().c_str()); + captures_path.string().c_str(), ac_cwd.c_str()); } code_provider_init(cfg_backend, gcc_avail); /* (sljit removed 2026-07-15: overlay_loader_apply_live_policy was diff --git a/runtime/src/mod_runtime.cpp b/runtime/src/mod_runtime.cpp index a168bae8..0adc09ad 100644 --- a/runtime/src/mod_runtime.cpp +++ b/runtime/src/mod_runtime.cpp @@ -979,6 +979,28 @@ extern "C" int psx_mod_game_started(void) { return fntrace_is_game_started(); } +extern "C" int psx_mod_option_value(const char* package_id, + const char* feature_id, + const char* option_id, + char* out, uint32_t out_size) { + using namespace PSXRecompV4; + if (out && out_size) out[0] = '\0'; + if (!package_id || !feature_id || !option_id || !out || out_size == 0) + return 0; + RuntimeMods& s = state(); + /* Activation runs after the final plan commit, so a committed plan is the + * precondition for a meaningful answer. Without one there is no selection + * to read and the caller must fall back to its own default rather than + * treat an empty string as a value. */ + if (!s.initialized || !s.plan.ok) return 0; + const std::string value = s.manager.feature_option_value( + package_id, feature_id, option_id); + if (value.empty()) return 0; + if (value.size() + 1 > (size_t)out_size) return 0; + std::memcpy(out, value.c_str(), value.size() + 1); + return 1; +} + extern "C" uint8_t psx_mod_read_byte(uint32_t address) { return psx_read_byte(address); }