Importing metadata from local launchbox installation#2922
Importing metadata from local launchbox installation#2922gantoine merged 23 commits intorommapp:masterfrom
Conversation
Summary of ChangesHello @gravelfreeman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a major enhancement to the LaunchBox integration within RomM, shifting from an exclusive remote metadata fetching model to one that prioritizes and leverages local LaunchBox installations. Users can now mount their local LaunchBox directories to import game metadata, images, and manuals, reducing reliance on external scraping. The changes include robust logic for identifying and processing local XML files and media, alongside a new user interface control to manage the interplay between local and remote data sources, offering greater flexibility and efficiency for managing game libraries. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a major overhaul of the Launchbox handler to support importing metadata from a local Launchbox installation, in addition to the existing remote database scraping. This is a great feature for users who have already curated their local libraries. The changes are extensive, touching the backend scanning logic, the Launchbox metadata handler, and the frontend UI to add a toggle for this new functionality. The implementation correctly prioritizes local metadata and media, with a fallback to remote sources.
My review focuses on the new logic in launchbox_handler.py, where I've identified a few areas for improvement regarding exception handling, performance, and potential data loss from aggressive filtering. These are important to address to ensure the new feature is robust and reliable. The rest of the changes, including the frontend updates and plumbing the new setting through the backend, look solid.
There was a problem hiding this comment.
Pull request overview
This PR adds support for importing metadata, images, and manuals from local LaunchBox installations, allowing users to reuse existing LaunchBox libraries instead of re-scraping metadata. The implementation includes a frontend toggle to control whether remote metadata fetching is enabled, and backend logic to parse local XML files and resolve media assets from the mounted LaunchBox directory structure.
Changes:
- Added UI toggle for LaunchBox remote metadata fetching with local storage persistence
- Implemented local XML parsing for LaunchBox platform data with caching
- Added media file resolution for images, screenshots, and manuals from local LaunchBox directories
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/views/Scan.vue | Added Remote switch toggle to control LaunchBox remote metadata fetching |
| frontend/src/locales/en_US/scan.json | Added English translation for LaunchBox remote feature |
| frontend/src/locales/fr_FR/scan.json | Added French translation for LaunchBox remote feature |
| backend/handler/metadata/launchbox_handler.py | Major overhaul with local XML parsing, media resolution, and hybrid local/remote metadata support |
| backend/handler/scan_handler.py | Added launchbox_remote_enabled parameter propagation through scan functions |
| backend/endpoints/sockets/scan.py | Threaded launchbox_remote_enabled parameter through WebSocket scan handlers |
| backend/tasks/scheduled/update_launchbox_metadata.py | Changed condition to use LAUNCHBOX_API_ENABLED directly instead of handler check |
| .dockerignore | Added comprehensive ignore patterns for Docker build optimization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
gantoine
changed the title
gantoine
changed the title
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-implemented feature: the ability to import metadata and media directly from a local LaunchBox installation. The extensive refactoring in launchbox_handler.py has improved modularity and clarity, breaking down complex logic into smaller, more manageable functions. The new caching mechanism for local XML files, along with the robust handling of local media paths and prioritization over remote sources, demonstrates a thorough approach to the feature. The integration with the existing scanning process and the addition of a UI toggle for remote LaunchBox fetching are also well-executed. This feature will greatly benefit users with existing LaunchBox libraries.
Greptile SummaryThis PR refactors the LaunchBox handler from a single file into a structured package ( Key changes:
Notable new finding: Additional notes:
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[scan_rom called] --> B{launchbox platform?}
B -- No --> Z[return LaunchboxRom null]
B -- Yes --> C{ScanType.UPDATE\n& rom.launchbox_id\n& remote_enabled?}
C -- Yes --> D[get_rom_by_id\nremote only]
C -- No --> E[get_rom\nfs_name + platform_slug]
E --> F[LocalSource.get_rom\nparse XML / mtime cache]
F -- local match --> G[build local_media_req\nplatform_name + fs_name + title]
F -- no match --> H{LAUNCHBOX_TAG_REGEX\nin filename?}
H -- yes --> I[get_rom_by_id\nby embedded ID tag]
I -- found --> R[return ROM]
I -- not found --> J{remote_available?}
H -- no --> J
J -- No --> Z
J -- Yes --> K[RemoteSource.get_rom\nRedis name index]
K -- found --> L[build remote_media_req\nplatform_name=None ⚠️\nfs_name= ⚠️]
K -- not found --> Z
G --> M[fetch_images remote]
L --> M
G --> N[_build_local_media_context\nplatform + stems + regions]
L --> O[_build_local_media_context\nplatform_name=None → returns None ⚠️]
N --> P[_get_cover / _get_screenshots\n_get_images / _get_manuals\nlocal files override remote]
O --> Q[ALL local image lookups\nskipped silently ⚠️]
P --> R
Q --> R
M --> N
M --> O
Last reviewed commit: aa4abe6 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| else: | ||
| search_term = fs_name | ||
|
|
||
| search_term = search_term.lower() | ||
|
|
||
| # Check if game is scummvm shortname | ||
| if platform_slug == UPS.SCUMMVM: | ||
| search_term = await self._scummvm_format(search_term) | ||
| fallback_rom = LaunchboxRom(launchbox_id=None, name=search_term) | ||
|
|
||
| index_entry = await self._remote.get_rom( | ||
| search_term, | ||
| platform_slug, |
There was a problem hiding this comment.
Local images silently skipped for remote-only matched ROMs
When a ROM has no local XML entry but is matched via the remote source, remote_media_req is constructed with platform_name=None and fs_name="". Inside _build_local_media_context, the very first guard is:
if not req.platform_name:
return NoneThis causes None to be returned immediately, so every downstream image function (_get_cover, _get_screenshots, _get_images) skips local lookups entirely. A user who mounts a full local LaunchBox image library under /romm/launchbox/Images/ will receive no locally-cached artwork for any ROM that wasn't indexed in their local XML — even though the images are present on disk.
The fix is to thread both platform_name and the original fs_name through remote_media_req so the local media context can be built:
# In remote_media_req: add platform_name and fs_name params
def remote_media_req(
*,
platform_name: str | None,
fs_name: str,
remote: dict | None,
remote_images: list[dict] | None,
remote_enabled: bool,
) -> MediaRequest:
title = ((remote or {}).get("Name") or "").strip()
return MediaRequest(
platform_name,
fs_name,
title,
None,
remote_images,
remote_enabled,
)And at the call site in get_rom:
platform_name = get_platform(platform_slug).get("name")
media_req = remote_media_req(
platform_name=platform_name,
fs_name=fs_name,
remote=index_entry,
remote_images=remote_images,
remote_enabled=remote_available,
)| preferred_regions.extend( | ||
| [r.strip() for r in region_hint.split(",") if r.strip()] | ||
| ) | ||
|
|
||
| return { | ||
| "base": base, | ||
| "stems": stems, | ||
| "preferred_regions": preferred_regions, | ||
| } |
There was a problem hiding this comment.
Combined region_hint string added as first (unmatchable) search path
When region_hint contains a comma (e.g., "North America, Europe"), the full joined string is appended to preferred_regions before the split parts:
preferred_regions.append(region_hint) # → "North America, Europe"
if "," in region_hint:
preferred_regions.extend([...]) # → "North America", "Europe"The first entry "North America, Europe" will never match a real LaunchBox directory (which uses individual region names). It is tried first for every stem, burning iterations on a path that cannot exist. Consider only appending the split parts when a comma is present:
if region_hint:
if "," in region_hint:
preferred_regions.extend(
[r.strip() for r in region_hint.split(",") if r.strip()]
)
else:
preferred_regions.append(region_hint)
Description
Launchbox handler overhaul. RomM currently only fetches metadata from the LaunchBox database, but many users already have a well-maintained local LaunchBox library. This lets RomM reuse that work instead of scraping everything again.
Supported :
Requires implementation :
How does RomM access your local LaunchBox installation?
You can mount it to
/tempas read only either the full root of Launchbox folder, or just the Data, Images, and Manuals directories as follow :/temp/Data/temp/Images/temp/ManualsI added a
Remoteswitch in the frontend so you can toggle remote metadata/media fetching.If switch is OFF and there's no
/tempfolder Launchbox handler will exit.If switch is OFF and there's
/tempfolder Launchbox handler will search for platforms in mounted Launchbox structure under/temp/Data/Platform/<Platform Name.xml>.If switch is ON and there's no
/tempfolder Launchbox handler will scrape platforms in your defaultromslibrary.If switch is ON and there's
/tempfolder Launchbox handler will search for platforms in mounted Launchbox structure under/temp/Data/Platform/<Platform Name.xml>and match roms with the local Launchbox metadata in priority and then fallback toRemotemetadata.If you prefer, you can also copy platform XML files one by one into
/temp/Data/<Platform Name>.xmlto import platforms individually.Checklist
Please check all that apply.
Screenshots (if applicable)