Skip to content

Add config option to use local video paths in gamelist.xml export#3082

Merged
gantoine merged 5 commits intorommapp:masterfrom
JamieeBond:feature/gamelist-local-video-config
Mar 8, 2026
Merged

Add config option to use local video paths in gamelist.xml export#3082
gantoine merged 5 commits intorommapp:masterfrom
JamieeBond:feature/gamelist-local-video-config

Conversation

@JamieeBond
Copy link
Contributor

Relating to #3069

Description
Add config value to use local videos in the gamelist.xml export, instead of YouTube, if set to true.

In the config, i created it as a new config array, to add some future proofing, rather than a standalone key, since gamelist is relevantly new, so config settings for the export or import can have a place to live:

 gamelist:
   export:
     local_video: true

Uses the same logic as here, to decide which video to use:

const localVideoPath = computed(() => {
if (!romsStore.isSimpleRom(rom)) return null;
// Only play video if boxart style is miximage
if (boxartStyle.value !== "miximage_path") return null;
const ssVideo = rom.ss_metadata?.video_path;
const gamelistVideo = rom.gamelist_metadata?.video_path;
return ssVideo || gamelistVideo || null;
});

Checklist
Please check all that apply.

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes

…tead of YouTube

Bump joserfc from 1.3.5 to 1.6.3
@greptile-apps
Copy link

greptile-apps bot commented Mar 7, 2026

Greptile Summary

This PR modifies backend/utils/gamelist_exporter.py to prefer locally stored video files (from ss_metadata.video_path or gamelist_metadata.video_path) over YouTube embed links when building the <video> element of a gamelist.xml export, falling back to YouTube only when no local path exists.

Key issue:

  • The PR title and description promise a config-gated option (gamelist.export.local_video: true) that lets users opt in to local video paths. The config flag was never implemented — no GAMELIST_EXPORT_LOCAL_VIDEO attribute was added to Config, _parse_config(), or _validate_config() in config_manager.py. The new behaviour is therefore unconditional, silently overriding YouTube video links with local paths for all existing installations, regardless of user preference.

Positive aspects:

  • The local-path resolution logic mirrors the existing frontend logic in useGameAnimation.ts (lines 195-201), keeping behaviour consistent between the UI and the export.
  • The fallback chain (ss_metadatagamelist_metadata → YouTube) is sensible.

Confidence Score: 2/5

  • Not safe to merge — the advertised config gate was not implemented, making this an unconditional breaking behaviour change.
  • The core intent of the PR (an opt-in config flag) is entirely absent from the implementation. All users will have their gamelist.xml video links silently switched from YouTube to local paths without any opt-in, which is the opposite of the described design. The config manager, its parsing, and validation are all untouched. Both backend/utils/gamelist_exporter.py and backend/config/config_manager.py need attention — the exporter needs the config flag guard, and the config manager needs the new GAMELIST_EXPORT_LOCAL_VIDEO attribute, parser entry, and validator.
  • backend/utils/gamelist_exporter.py (needs config guard), backend/config/config_manager.py (needs GAMELIST_EXPORT_LOCAL_VIDEO attribute, parser, and validator)

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["_create_game_element(rom)"] --> B{ss_metadata\nvideo_path?}
    B -- yes --> C["video = FRONTEND_RESOURCES_PATH/ss_metadata.video_path"]
    B -- no --> D{gamelist_metadata\nvideo_path?}
    D -- yes --> E["video = FRONTEND_RESOURCES_PATH/gamelist_metadata.video_path"]
    D -- no --> F{youtube_video_id?}
    F -- yes --> G["video = YOUTUBE_BASE_URL/embed/youtube_video_id"]
    F -- no --> H["No video element added"]

    style B fill:#f9a,stroke:#c33
    style D fill:#f9a,stroke:#c33
    note1["⚠️ Config flag\n(gamelist.export.local_video)\ndescribed in PR\nbut NOT implemented"]
    note1 -.-> B
Loading

Last reviewed commit: fcf015e

@gantoine gantoine self-requested a review March 8, 2026 00:07
@gantoine gantoine changed the title Add config option to use local video paths in gamelist.xml export ins… Add config option to use local video paths in gamelist.xml export Mar 8, 2026
@gantoine gantoine merged commit d595c12 into rommapp:master Mar 8, 2026
4 checks passed
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.

2 participants