feat: Auto Export Game List on scan#2987
Conversation
Summary of ChangesHello @ItzCobaltboy, 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 new feature that allows for the automatic export of Highlights
Changelog
Activity
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 new feature to automatically export gamelist.xml after a library scan, controlled by a new configuration option. The implementation is well-structured, covering configuration management, the scan workflow, and modifications to the gamelist exporter to support local exports without a request context. I have one suggestion to improve performance by refactoring a loop to avoid multiple database queries.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in configuration flag to automatically export ES-DE gamelist.xml files for scanned platforms after a library scan completes.
Changes:
- Introduces
gamelist.auto_export_on_scanconfig option (defaultfalse) with parsing + validation. - Hooks scan completion to optionally trigger local
gamelist.xmlexport per platform. - Updates
GamelistExporterAPIs to acceptrequest=Nonefor local exports.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| backend/config/config_manager.py | Adds new config field, parsing, validation, and persistence into config.yml. |
| backend/endpoints/sockets/scan.py | Triggers optional auto-export at end of scan when config is enabled. |
| backend/utils/gamelist_exporter.py | Allows request to be optional to support local export flows. |
Comments suppressed due to low confidence (2)
backend/utils/gamelist_exporter.py:176
- Request validation for non-local exports is only enforced inside
_create_game_element(). If a platform has zero ROMs (or all ROMs are filtered out),_create_game_element()is never called, soexport_platform_to_xml()/export_platform_to_file()can succeed even whenlocal_export=Falseandrequest=None. Add an upfront guard inexport_platform_to_xml()(and/orexport_platform_to_file()) to raise whenrequestis missing for non-local exports, regardless of ROM count.
def export_platform_to_xml(self, platform_id: int, request: Request | None) -> str:
"""Export a platform's ROMs to gamelist.xml format
Args:
platform_id: Platform ID to export
Returns:
XML string in gamelist.xml format
"""
platform = db_platform_handler.get_platform(platform_id)
if not platform:
raise ValueError(f"Platform with ID {platform_id} not found")
roms = db_rom_handler.get_roms_scalar(platform_ids=[platform_id])
backend/utils/gamelist_exporter.py:205
- The docstrings don’t reflect that
requestis now optional:export_platform_to_xml()doesn’t document therequestargument at all, andexport_platform_to_file()still implies it’s required. Update the docstrings to clarify whenrequestmay beNone(local exports) and when it’s mandatory (non-local exports).
def export_platform_to_xml(self, platform_id: int, request: Request | None) -> str:
"""Export a platform's ROMs to gamelist.xml format
Args:
platform_id: Platform ID to export
Returns:
XML string in gamelist.xml format
"""
platform = db_platform_handler.get_platform(platform_id)
if not platform:
raise ValueError(f"Platform with ID {platform_id} not found")
roms = db_rom_handler.get_roms_scalar(platform_ids=[platform_id])
# Create root element
root = Element("gameList")
for rom in roms:
if rom and not rom.missing_from_fs and rom.fs_name != "gamelist.xml":
game_element = self._create_game_element(rom, request=request)
root.append(game_element)
# Convert to XML string
indent(root, space=" ", level=0)
xml_str = tostring(root, encoding="unicode", xml_declaration=True)
log.info(f"Exported {len(roms)} ROMs for platform {platform.name}")
return xml_str
async def export_platform_to_file(
self,
platform_id: int,
request: Request | None,
) -> bool:
"""Export platform ROMs to gamelist.xml file in the platform's directory
Args:
platform_id: Platform ID to export
request: FastAPI request object for URL generation
Returns:
True if successful, False otherwise
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pulls database call out of the loop Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (3)
backend/utils/gamelist_exporter.py:166
- This module currently contains syntactically invalid f-strings in the
rom.ss_metadatablock (e.g.,rom.ss_metadata["box3d_path"]embedded inside a double-quoted f-string). As written, it will raise aSyntaxErrorat import time, which would break the/gamelist/exportendpoint and the new scan auto-export path. Please fix the quoting/formatting of those f-strings so the file parses correctly.
def export_platform_to_xml(self, platform_id: int, request: Request | None) -> str:
"""Export a platform's ROMs to gamelist.xml format
Args:
platform_id: Platform ID to export
backend/utils/gamelist_exporter.py:205
export_platform_to_file's docstring still describesrequestas required, but the signature now allowsNonefor local exports. Update the docstring to reflect thatrequestis optional only whenlocal_export=True, and ideally raise a clear error (or return False with a specific log) ifrequestis missing for non-local exports.
async def export_platform_to_file(
self,
platform_id: int,
request: Request | None,
) -> bool:
"""Export platform ROMs to gamelist.xml file in the platform's directory
Args:
platform_id: Platform ID to export
request: FastAPI request object for URL generation
Returns:
True if successful, False otherwise
"""
backend/utils/gamelist_exporter.py:170
export_platform_to_xmlnow accepts an optionalrequest, but the docstringArgs:section does not document therequestparameter or when it is required (non-local export) vs optional (local export). Please update the docstring so callers know the contract.
def export_platform_to_xml(self, platform_id: int, request: Request | None) -> str:
"""Export a platform's ROMs to gamelist.xml format
Args:
platform_id: Platform ID to export
Returns:
XML string in gamelist.xml format
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@ItzCobaltboy can you ping me when you've addressed he copilot comments? resolve any you don't want/don't think need to be changes. |
|
@gantoine Edit: I pushed a commit with files formatted, there should be no workflow issues now |
2 participants
Implementation for #2975
Description
Implements the Feature of automatic export of gamelist for each platform on Library scan. Adds a new config feature to enable it (Disabled by default)
Implementation details
gamelist.auto_export_on_scanon boot, default set to falsescan_platforms, callsGameListExporterfor exporting gamelist.xml locallyGameListExporterto acceptNoneiflocal_export=Trueis set, throws value error ifrequestis not passed for non local exportFiles changed
backend/config/config_manager.pybackend/endpoints/sockets/scan.pybackend/utils/gamelist_exporter.pyThis pull request introduces a new configuration option to automatically export
gamelist.xmlfiles for all platforms after a scan completes. The changes include updates to configuration management, validation, and the scan completion workflow, as well as improvements to the gamelist exporter utility to support optional request objects for local exports.Configuration management and validation:
GAMELIST_AUTO_EXPORT_ON_SCANboolean option to theConfigclass, parsing, and validation logic to control auto-export behavior.Scan workflow enhancements:
gamelist.xmlfor all scanned platforms using theGamelistExporter. Errors during export are logged per platform.Gamelist exporter improvements:
GamelistExportermethods (_create_game_element,export_platform_to_xml, andexport_platform_to_file) to accept an optionalrequestargument, raising an error if a non-local export is attempted without a request. This supports the new local auto-export workflow.AI Disclosure
The PR and Code is authored by me. ChatGPT was used for codebase comprehension and to sanity-check the implementation for potential misbehavior.
Checklist
Please check all that apply.