Skip to content

Fix psxrecomp_add_game_runtime for --no-recomp-ui headless builds - #199

Open
flyingclimber wants to merge 1 commit into
mstan:masterfrom
flyingclimber:fix/headless-no-recomp-ui-game-codegen
Open

Fix psxrecomp_add_game_runtime for --no-recomp-ui headless builds#199
flyingclimber wants to merge 1 commit into
mstan:masterfrom
flyingclimber:fix/headless-no-recomp-ui-game-codegen

Conversation

@flyingclimber

Copy link
Copy Markdown

Summary

psxrecomp_add_game_runtime() (added in #110) breaks with PSX_RECOMP_UI=OFF and real generated game C linked — a documented, supported config (docs/BUILDING.md: "PSX_RECOMP_UI OFF for headless/generated builds") that, as far as I can tell, nothing currently exercises. Found while bringing up a new title with recomp-ui intentionally left out for the first bring-up pass.

Two independent breaks in that combination:

  1. Declaration/call-site guard mismatch (runtime/src/main.cpp). main() calls psx_game_codegen_forward_if_built() whenever PSX_HAS_GAME_CODEGEN is set (i.e. any time real game C is linked), but the function is only declared under the narrower RECOMP_LAUNCHER guard. Any no-launcher build with generated game C fails to compile: use of undeclared identifier 'psx_game_codegen_forward_if_built'.

    Fix: introduced a precise PSX_HAS_CODEGEN_SETUP_HOST macro (set only when a codegen-setup host is actually linked, i.e. CODEGEN_SETUP_SOURCES was provided) and re-guarded both the declaration and the call site on it, instead of the previous mismatched guard pair.

  2. Unconditional recomp-ui dependency (runtime/runtime.cmake). psxrecomp_codegen_host.c was unconditionally appended to every game target's extras sources, but it unconditionally #includes recomp_launcher.h from the optional recomp-ui submodule. Any --no-recomp-ui build fails: recomp_launcher.h file not found.

    Fix: gated its inclusion on PSX_RECOMP_UI.

Why nothing caught this

psxrecomp_add_game_runtime() is currently used by exactly one shipped title — TombaRecomp — and Tomba always sets PSX_RECOMP_UI=ON. Every other title repo (ApeEscapeRecomp, Tomba2Recomp, MegaManX4/5/6Recomp, TsumuLightRecomp) calls the lower-level psxrecomp_add_runtime_target() directly and never reaches this code path at all.

Verification

  • New title, the actual broken config: built and ran end to end with PSX_RECOMP_UI=OFF, real generated game C, no launcher. OpenBIOS → game entry → continuous gameplay, clean atexit shutdown, no crash, no interp_unsupported faults.
  • Regression check on the one title that could actually regress: cloned TombaRecomp, pointed PSXRECOMP_ROOT at this branch via configure-time override (no submodule surgery, per docs/BUILDING.md#linking-the-framework), and built Tomba's real production config — PSX_RECOMP_UI=ON, ENABLE_SETUP_WIZARD, recomp-ui launcher, mod catalog, setup-host/no-disc mode. Clean build, ran to the launcher, no regressions. Both changes are conditionally gated on flags Tomba always sets the opposite way from the broken config, so this result was expected — but confirmed empirically rather than left as inference.
  • Ran the recompiler's ctest suite before/after: same pass rate (47/51; the 4 pre-existing failures are unrelated — a timing-sensitive overlay-pair-lock flake and three content checks against files that only exist when the recomp-ui/recomp-net submodules are populated).

Not covered here

CMAKE_CXX_STANDARD 17 (runtime/CMakeLists.txt) is also never inherited by a game repo that include()s runtime.cmake directly rather than add_subdirectory()-ing the framework's own CMakeLists.txt — this silently compiles text_xlate.cpp's std::filesystem usage against whatever the host compiler's default standard happens to be. Six of seven other title repos work around this by setting CMAKE_CXX_STANDARD 17 themselves. Happy to send a follow-up PR enforcing it inside runtime.cmake directly if that's preferred over per-repo workarounds — kept out of this PR to keep it focused on the one bug.

Test plan

  • ctest in recompiler/build — same pass/fail counts before and after
  • New headless title boots and runs continuously with the fix
  • TombaRecomp's production config builds and runs unchanged against this branch

psxrecomp_add_game_runtime() (added in mstan#110) is unusable with
PSX_RECOMP_UI=OFF and real generated game C linked — a documented,
supported config ("PSX_RECOMP_UI OFF for headless/generated builds",
BUILDING.md) that nothing currently exercises. Found while bringing up
a new title with recomp-ui intentionally left out initially.

Two independent breaks in that combination:

1. main.cpp calls psx_game_codegen_forward_if_built() from main()
   whenever PSX_HAS_GAME_CODEGEN is set (i.e. any time real game C is
   linked), but only *declares* it under the narrower RECOMP_LAUNCHER
   guard. Any no-launcher build with generated game C fails with
   "use of undeclared identifier". Introduced a precise
   PSX_HAS_CODEGEN_SETUP_HOST macro (set only when a codegen-setup
   host is actually linked) and re-guarded both the declaration and
   the call site on it instead of the mismatched pair of guards.

2. psxrecomp_codegen_host.c was unconditionally added to every game
   target's extras, but it unconditionally #includes recomp_launcher.h
   from the recomp-ui submodule. Any --no-recomp-ui build fails with
   "recomp_launcher.h file not found". Gated its inclusion on
   PSX_RECOMP_UI.

Verification:
- Built and ran a new title (headless: PSX_RECOMP_UI=OFF, real
  generated game C, no launcher) end to end: OpenBIOS -> game entry ->
  continuous gameplay, clean atexit shutdown, no crash.
- Regression-checked the one existing title that actually calls
  psxrecomp_add_game_runtime() (TombaRecomp; every other shipped title
  calls the lower-level psxrecomp_add_runtime_target directly and
  never reaches this code): built TombaRecomp's real production config
  (PSX_RECOMP_UI=ON, ENABLE_SETUP_WIZARD, recomp-ui launcher, mod
  catalog, setup-host/no-disc mode) against this branch via
  -DPSXRECOMP_ROOT override, no submodule surgery. Clean build, ran to
  the launcher with no regressions. Both changes are conditionally
  gated on flags Tomba always sets the opposite way, so this is
  expected and was confirmed empirically, not just by inspection.

Not covered by this PR: CMAKE_CXX_STANDARD 17 is also never inherited
by a game repo that include()s runtime.cmake directly (as opposed to
add_subdirectory()-ing runtime/CMakeLists.txt), which silently
compiles text_xlate.cpp's std::filesystem usage against whatever the
host compiler's default standard is. Six of seven other title repos
work around this by setting it themselves; happy to send a follow-up
if that's wanted here instead of per-repo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant