feat(mediadb): toml settings for scraper bundle path and asset stat checks#927
feat(mediadb): toml settings for scraper bundle path and asset stat checks#927BossRighteous wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR extends the gamelist.xml scraper to support custom gamelists with independent asset directories. Configuration enables specifying a custom gamelists path and an optional image statistics flag. Gamelist records now track a separate asset root path alongside the ROM root, enabling assets referenced in custom gamelist XML to resolve relative to the custom directory rather than the system root. The scraper also updates companion metadata handling to propagate asset paths through the metadata chain. ChangesCustom Gamelists and Asset Path Resolution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/database/scraper/gamelistxml/scraper_test.go (1)
1205-1213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd targeted tests for the newly introduced custom-gamelist and stat-gated asset flows.
This change only updates the
pathPropcall signature; it does not add coverage for the new branches introduced in this PR (loadCustomGamelistFile,AssetRootPathpropagation, andScraperStatGamelistImagestrue/false behavior inMapToDB). Please add tests for those paths before merge.As per coding guidelines, “Write tests for all new code — see TESTING.md and pkg/testing/README.md”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/database/scraper/gamelistxml/scraper_test.go` around lines 1205 - 1213, The test suite only updated the pathProp call signature and lacks coverage for the new branches; add targeted unit tests that exercise loadCustomGamelistFile behavior, propagation of AssetRootPath through pathProp/MapToDB, and both true/false cases of ScraperStatGamelistImages in MapToDB: create tests that (1) simulate a custom-gamelist file and assert loadCustomGamelistFile is invoked and parsed correctly, (2) set AssetRootPath and verify generated property paths from pathProp/MapToDB use that root (including mixed separators normalization), and (3) toggle ScraperStatGamelistImages and assert MapToDB includes or excludes stat-gated image entries accordingly; reference functions: loadCustomGamelistFile, AssetRootPath, pathProp, MapToDB, ScraperStatGamelistImages to locate and extend tests.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pkg/database/scraper/gamelistxml/scraper_test.go`:
- Around line 1205-1213: The test suite only updated the pathProp call signature
and lacks coverage for the new branches; add targeted unit tests that exercise
loadCustomGamelistFile behavior, propagation of AssetRootPath through
pathProp/MapToDB, and both true/false cases of ScraperStatGamelistImages in
MapToDB: create tests that (1) simulate a custom-gamelist file and assert
loadCustomGamelistFile is invoked and parsed correctly, (2) set AssetRootPath
and verify generated property paths from pathProp/MapToDB use that root
(including mixed separators normalization), and (3) toggle
ScraperStatGamelistImages and assert MapToDB includes or excludes stat-gated
image entries accordingly; reference functions: loadCustomGamelistFile,
AssetRootPath, pathProp, MapToDB, ScraperStatGamelistImages to locate and extend
tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fcfcb64a-5e6f-42c0-b11f-b8add912504f
📒 Files selected for processing (4)
pkg/config/config.gopkg/config/configscraper.gopkg/database/scraper/gamelistxml/scraper.gopkg/database/scraper/gamelistxml/scraper_test.go
|
they sound like good changes to me! i'm just getting a patch release ready but happy to to merge it after that |
2 participants
This is primarily to assist in batch storage and processing, so up to you if you think it's worthwhile.
All config fields optional
custom_gamelists_path (default "")
If defined, it will be used as another base path for gamelist lookups.
Where Launcher based system paths are derived from Folders[], which may have duplication and overlap (WonderSwan vs WonderSwanColor); this scan root will only check the literal SystemID string. I don't feel this is perfect but I think in the case of bundles this explicit Zaparoo system mapping is actually a better solution than continuing the fanout since it's not meant to be relative to actual roms/extension filters.
stat_gamelist_images (default false)
Recallbox gamelists have near guarantee that image paths defined were available when scraped. This can be trusted until it's time to load the asset via API.
Bundles would be made more flexible and convenient if defined image paths were first checked for existence.
Use case: batch gamelist.xmls could include game nodes with all known asset references and actual media could be managed from separate zips as requested.
All of these could exists without error or bad scrape props being injected. If a user installed box2d and screenshot zips those would be found without a big burden.
If you like or dislike anything proposed feel free to suggest changes! I'll improve coverage if the overall strategy looks okay
Summary by CodeRabbit
New Features
Tests