Prep work to make it easier to test and integrate new cores #75
Conversation
- Show actionable error messages instead of silent failures - Catch core crashes during load with segfault recovery - Capture libretro error logs for display to user - Add fallback input polling for misbehaving cores - Fix GFX_wrapText to preserve existing newlines (for multi-line messages) - Fix memory leak (missing fclose on malloc failure) - Clean up temp directories after archive extraction
- Add merge_configs() to generate-paks.sh for base + platform merging - Platform configs now sparse (only unique settings, 1-2 lines each) - Delete 12 redundant configs, remove invalid gkdpixel platform - Update Emus README documenting merge behavior
There was a problem hiding this comment.
Pull request overview
This pull request implements preparatory refactoring to streamline testing and integration of new emulator cores. The changes establish a sparse configuration system, improve error handling for core loading failures, and add robust input polling to handle misbehaving cores.
Key changes:
- Introduces a build-time config merging system where platform-specific configs contain only overrides, with base configs providing all defaults
- Adds comprehensive fatal error handling with segfault recovery during core loading and user-friendly error messages
- Implements fallback input polling to ensure menu responsiveness even when cores don't properly call the input callback
Reviewed changes
Copilot reviewed 46 out of 52 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| workspace/all/paks/MinUI/launch.sh.template | Clarifies LOG_SYNC documentation for crash-safe logging |
| workspace/all/paks/Emus/skeleton/SYSTEM/common/bin/arm64/.gitkeep | Placeholder for future arm64 binary support |
| workspace/all/paks/Emus/skeleton/SYSTEM/common/bin/arm/.gitkeep | Placeholder for future arm binary support |
| workspace/all/paks/Emus/cores.json | Removes VICE emulator cores (C128, VIC, PLUS4, PET) from active configuration |
| workspace/all/paks/Emus/configs//default.cfg | Removes redundant platform-specific configs, moving common settings to base configs |
| workspace/all/paks/Emus/README.md | Documents the new sparse config system with merge semantics and provides philosophy and examples |
| workspace/all/minarch/minarch_loop_*.inc | Adds input polling reset and fallback polling to ensure menu responsiveness |
| workspace/all/minarch/minarch.c | Implements fatal error display, segfault recovery, improved error messages, and temp directory cleanup |
| workspace/all/common/gfx_text.c | Enhances text wrapping to preserve existing newlines for multi-line error messages |
| tests/unit/all/common/test_gfx_text.c | Adds comprehensive tests for newline-preserving text wrapping behavior |
| skeleton/BASE/Bios/*/.gitkeep | Placeholder directories for future VICE emulator BIOS files |
| scripts/generate-paks.sh | Implements config merge function with last-one-wins semantics for sparse platform overrides |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
workspace/all/minarch/minarch.c
Outdated
| // Not in core_load - restore default handler and re-raise | ||
| signal(SIGSEGV, SIG_DFL); | ||
| raise(SIGSEGV); | ||
| } | ||
|
|
There was a problem hiding this comment.
Calling signal() from within a signal handler is not async-signal-safe according to POSIX. If a SIGSEGV occurs outside of core loading, this handler could invoke undefined behavior. Consider using only async-signal-safe functions like _exit() or simply avoiding this code path by ensuring the handler is only installed when in_core_load is true.
| // Not in core_load - restore default handler and re-raise | |
| signal(SIGSEGV, SIG_DFL); | |
| raise(SIGSEGV); | |
| } | |
| // Not in core_load - terminate immediately using async-signal-safe function | |
| _exit(128 + SIGSEGV); | |
| } |
workspace/all/minarch/minarch.c
Outdated
| // (7z is never natively supported by libretro cores) | ||
| if (!supports_archive || suffixMatch(".7z", game.path)) { | ||
| // Extract if core doesn't support the archive format natively | ||
| if (!supports_archive) { |
There was a problem hiding this comment.
The special case handling for .7z files was removed. The old code extracted 7z archives even when cores claimed to support archives natively, because libretro cores never support 7z format. Without this check, 7z files will be passed directly to cores that claim archive support, causing them to fail loading. Consider restoring the condition: if (!supports_archive || suffixMatch(".7z", game.path))
| if (!supports_archive) { | |
| if (!supports_archive || suffixMatch(".7z", game.path)) { |
| *last_space = '\n'; | ||
| line_start = last_space + 1; | ||
| last_space = NULL; | ||
| lines++; |
There was a problem hiding this comment.
After wrapping at a space, the pointer 'p' continues from its current position, which may be far ahead of the new line_start. This causes the loop to keep measuring the full span from line_start to p on each iteration until p advances past where it wrapped. Consider resetting p to line_start after wrapping to avoid redundant measurements and potential incorrect behavior.
- New generate-scaling-configs.py script calculates optimal scaling per platform/core - Uses 95% fill threshold for large screens, 100% for small (<3") - minarch.c now forces Sharp pixels for Native/Cropped modes automatically - Generates 23 platform configs (only where integer scaling works well) - Cropped override for FC/SFC on square screens
No description provided.