Skip to content

feat: Continue integration and download of more artifacts#149

Draft
pawndev wants to merge 3 commits intorommapp:v4.7.0.0from
pawndev:feat/139/finalize-ES-gamelist-integration
Draft

feat: Continue integration and download of more artifacts#149
pawndev wants to merge 3 commits intorommapp:v4.7.0.0from
pawndev:feat/139/finalize-ES-gamelist-integration

Conversation

@pawndev
Copy link
Collaborator

@pawndev pawndev commented Feb 27, 2026

⚠️ Currently, all artifacts path from ss_metadata are wrong, because of the missing /assets/romm/resources prefix. Fix with 19814c2
⚠️ Not tested on a real device yet Tested on RGDS w/ RockNix and RG35xxsp w/ Knulli

This PR is part of the issue #139

Currently, all artifacts path from ss_metadata are wrong, because of the missing `/assets/romm/resources` prefix
@pawndev
Copy link
Collaborator Author

pawndev commented Mar 2, 2026

Resulting gamelist.xml like:

<?xml version="1.0" encoding="UTF-8"?>
<gameList>
    <game>
        <name>Pokémon Pearl Version (France)</name>
        <region>France</region>
        <desc>Pokémon Version Perle est un jeu de rôle sur Nintendo DS qui vous permet de devenir le plus grand des maîtres Pokémon en les attrapant tous, pour les rendre les plus compétitifs possible lors de combats en mode solo ou en multijoueur. Il est également possible de les échanger, y compris grâce au Wi-Fi.</desc>
        <md5>a2c9d259ad7262e5dcd5dd3ff08d798a</md5>
        <rating>0.8</rating>
        <marquee>/Users/ccoquelet/SBC/Knulli/share/roms/nds/images/Pokemon - Version Perle (France) (Rev 5)_marquee.png</marquee>
        <cheevosHash>c8824fdcd12fd53947a6d37a1569a492</cheevosHash>
        <image>/Users/ccoquelet/SBC/Knulli/share/roms/nds/images/Pokemon - Version Perle (France) (Rev 5).png</image>
        <manual>/Users/ccoquelet/SBC/Knulli/share/roms/nds/manuals/Pokemon - Version Perle (France) (Rev 5).pdf</manual>
        <players>1-4</players>
        <genre>Adventure, Role-playing (RPG), Turn-based strategy (TBS)</genre>
        <developer>Game Freak, Nintendo</developer>
        <scraperId>86743</scraperId>
        <cheevosId>25369</cheevosId>
        <releasedate>20060928T000000</releasedate>
        <thumbnail>/Users/ccoquelet/SBC/Knulli/share/roms/nds/images/Pokemon - Version Perle (France) (Rev 5).png</thumbnail>
        <video>/Users/ccoquelet/SBC/Knulli/share/roms/nds/videos/Pokemon - Version Perle (France) (Rev 5).mp4</video>
        <path>/Users/ccoquelet/SBC/Knulli/share/roms/nds/Pokemon - Version Perle (France) (Rev 5).nds</path>
    </game>
</gameList>

@pawndev
Copy link
Collaborator Author

pawndev commented Mar 2, 2026

image

@pawndev
Copy link
Collaborator Author

pawndev commented Mar 2, 2026

With bazels:
image

<?xml version="1.0"?>
<gameList>
    <game>
        <path>/Users/ccoquelet/SBC/Knulli/share/roms/mastersystem/Streets of Rage II (Europe).sms</path>
        <md5>e680b108e67caf02e4fb77d419f48358</md5>
        <rating>0.6</rating>
        <releasedate>19930801T000000</releasedate>
        <bezel>/Users/ccoquelet/SBC/Knulli/share/roms/mastersystem/bezels/Streets of Rage II (Europe).png</bezel>
        <players>1</players>
        <developer>Japan System House, Samsung, Sega Enterprises, Ltd., Tec Toy</developer>
        <scraperId>777</scraperId>
        <desc>The 8-bit port of Streets of Rage 2, originally released for the Game Gear.</desc>
        <image>/Users/ccoquelet/SBC/Knulli/share/roms/mastersystem/images/Streets of Rage II (Europe).png</image>
        <marquee>/Users/ccoquelet/SBC/Knulli/share/roms/mastersystem/images/Streets of Rage II (Europe)_marquee.png</marquee>
        <video>/Users/ccoquelet/SBC/Knulli/share/roms/mastersystem/videos/Streets of Rage II (Europe).mp4</video>
        <genre>Hack and slash/Beat &apos;em up</genre>
        <cheevosHash>e680b108e67caf02e4fb77d419f48358</cheevosHash>
        <region>Europe</region>
        <cheevosId>11585</cheevosId>
        <name>Streets of Rage 2 (Europe)</name>
        <thumbnail>/Users/ccoquelet/SBC/Knulli/share/roms/mastersystem/images/Streets of Rage II (Europe).png</thumbnail>
    </game>
</gameList>

@pawndev pawndev self-assigned this Mar 2, 2026
@pawndev pawndev marked this pull request as ready for review March 2, 2026 22:34
@pawndev pawndev requested a review from BrandonKowalski March 2, 2026 22:34
@greptile-apps
Copy link

greptile-apps bot commented Mar 2, 2026

Greptile Summary

This PR adds support for downloading additional artifacts (marquee, video, bezel, manual) from Romm to EmulationStation-based custom firmwares (Knulli and ROCKNIX). It fixes the missing /assets/romm/resources prefix issue and adds RetroAchievements metadata to gamelists.

  • Implements download logic for marquee images, videos, bezels, and manuals
  • Adds UI settings for enabling/disabling each artifact type
  • Refactors ArtLocation from string to struct to handle multiple artifact paths
  • Adds proper URL path construction with asset prefix handling
  • Critical: Thumbnail download is not implemented despite having a UI setting
  • Error logging could be improved to show the actual path that caused failures

Confidence Score: 2/5

  • This PR has a critical missing implementation that affects user experience
  • The thumbnail download feature is advertised in the UI settings but not actually implemented in the download logic. Users can enable the setting but nothing will happen. The rest of the implementation looks solid with proper error handling and path construction.
  • Pay close attention to ui/download.go - the thumbnail download logic needs to be implemented to match the UI setting

Important Files Changed

Filename Overview
internal/config.go Added AdditionalDownloads config struct and directory accessor methods
internal/gamelist/add.go Refactored ArtLocation to struct supporting multiple artifact paths and added RetroAchievements metadata
romm/roms.go Added asset prefix handling, new artifact URL getters, and RetroAchievements fields; has error logging issue
ui/download.go Added download logic for marquee, video, bezel, and manual; thumbnail download is not implemented despite UI setting
ui/general_settings.go Added UI settings for EmulationStation additional downloads (marquee, video, thumbnail, bezel, manual)

Sequence Diagram

sequenceDiagram
    participant UI as UI (general_settings.go)
    participant Config as Config
    participant Download as DownloadScreen
    participant Rom as Rom (Romm API)
    participant CFW as CFW Directories
    participant FS as File System
    participant Gamelist as GameList

    UI->>Config: AdditionalDownloads settings
    Note over UI,Config: User enables marquee, video,<br/>bezel, manual (thumbnail not impl)
    
    Download->>Config: GetArtMarqueeDirectory(platform)
    Config->>CFW: GetArtMarqueeDirectory(romDir, ...)
    CFW-->>Config: Return art directory path
    Config-->>Download: Directory path
    
    Download->>Rom: GetMarqueeURL(host)
    Rom-->>Download: URL with RommAssetPrefix
    
    Download->>Rom: GetVideoURL(host)
    Rom-->>Download: Video URL
    
    Download->>Rom: GetBezelURL(host)
    Rom-->>Download: Bezel URL
    
    Download->>Rom: GetManualURL(host)
    Rom-->>Download: Manual URL
    
    Download->>FS: Download artifacts to directories
    Note over Download,FS: Downloads images (png),<br/>videos (mp4), manuals (pdf)
    
    Download->>Gamelist: AddRomGame(artLocation)
    Note over Gamelist: artLocation struct contains<br/>ImagePath, MarqueePath, VideoPath,<br/>BezelPath, ManualPath
    
    Gamelist->>FS: Write gamelist.xml with artifact paths
Loading

Last reviewed commit: da32c9c

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +437 to +500

artMarqueeDir := config.GetArtMarqueeDirectory(gamePlatform)
if config.AdditionalDownloads.Marquee && artMarqueeDir != "" {
marqueeArtFileName := g.FsNameNoExt
// is cfw is ES based, use _marquee suffix to avoid conflicts with cover art
if cfw.GetCFW().IsBasedOnEmulationStation() {
marqueeArtFileName += "_marquee.png"
} else {
marqueeArtFileName += ".png"
}
marqueeArtLocation := filepath.Join(artMarqueeDir, marqueeArtFileName)
if marqueeURL := g.GetMarqueeURL(host); marqueeURL != "" {
gamelistRomEntry.ArtLocation.MarqueePath = marqueeArtLocation
artDownloads = append(artDownloads, artDownload{
URL: marqueeURL,
Location: marqueeArtLocation,
GameName: g.Name,
IsImage: true,
})
}
}

artVideoDir := config.GetArtVideoDirectory(gamePlatform)
if config.AdditionalDownloads.Video && artVideoDir != "" {
videoLocation := filepath.Join(artVideoDir, g.FsNameNoExt+".mp4")
if videoURL := g.GetVideoURL(host); videoURL != "" {
gamelistRomEntry.ArtLocation.VideoPath = videoLocation
artDownloads = append(artDownloads, artDownload{
URL: videoURL,
Location: videoLocation,
GameName: g.Name,
IsImage: false,
})
}
}

artBezelDir := config.GetArtBezelDirectory(gamePlatform)
if config.AdditionalDownloads.Bezel && artBezelDir != "" {
bezelArtLocation := filepath.Join(artBezelDir, artFileName)
if bezelURL := g.GetBezelURL(host); bezelURL != "" {
gamelistRomEntry.ArtLocation.BezelPath = bezelArtLocation
artDownloads = append(artDownloads, artDownload{
URL: bezelURL,
Location: bezelArtLocation,
GameName: g.Name,
IsImage: true,
})
}
}

manualDir := config.GetManualDirectory(gamePlatform)
if config.AdditionalDownloads.Manual && manualDir != "" {
manualLocation := filepath.Join(manualDir, g.FsNameNoExt+".pdf")
if manualURL := g.GetManualURL(host); manualURL != "" {
gamelistRomEntry.ArtLocation.ManualPath = manualLocation
artDownloads = append(artDownloads, artDownload{
URL: manualURL,
Location: manualLocation,
GameName: g.Name,
IsImage: false,
})
}
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thumbnail download not implemented - setting exists in UI but no download logic

The UI has a setting for AdditionalDownloads.Thumbnail (in general_settings.go), and there's a config field and directory getter (GetArtThumbnailDirectory), but there's no actual download code here for thumbnails. Users can enable the setting but nothing will be downloaded.

Comment on lines +362 to +364
if screenshotURL == "" && err != nil {
logger.Error("Error joining host URL with screenshot path", "error", err, "hostURL", host.ToLoggable(), "screenshotPath", screenshotURL)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error log shows empty screenshotURL instead of the path that caused the error

When url.JoinPath fails and returns an error, screenshotURL remains empty (checked by the condition). The log should show the actual path that failed - either r.UserScreenshots[0].URLPath or r.MergedScreenshots[0].

@pawndev pawndev marked this pull request as draft March 3, 2026 09:08
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