New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ScreenScraper.fr as scraping source #519

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@cmitu
Copy link

cmitu commented Jan 9, 2019

I've added a new scraping source in Emulationstation - ScreenScraper.fr.

The new scraper is loosely based on the existing TGDB scraper source, but the API is a bit different. Some comments about this new module

  • ScreenScraper doesn't seem to have a 'search game' function in the api. A search request for a game should contain the ROM name and a checksum - so that matching would work better. Right now the module only searches by name.
    The result is that scraping would only return 1 result max (ideally the ROM itself), unlike TGDB that could return a list and the user could decide which result would import.

I plan to add checksum support, but right now there isn't one.

  • The PR appears in GH as 'Can't automatically merge' because of the commits yesterday in the PlatformId files - but I incorporated those changes, so it shouldn't be any conflict.

  • I'm using soft tabs in the module, but I can change to tabs proper if needed - I tried to keep the existing files with hard tabs, but I think I missed one.

  • I had to modify the Scraper.cpp since ScreenScraper's download URL for artwork doesn't include the filename at the end, so the extension of the image downloaded is taken from the API search response, not from the download link.

  • I plan on adding support in configuring the Region and Language (since ScreenScraper supports those), right now they're hardcoded in a config class. I intend to also create a configuration parameter to choose what artwork type to download (right now is 2d boxart, but we can use screenshot, 3D boxart, etc.).

  • I did a limited testing for about 20 systems, having well known ROM names, and it seems to be fine. More testing is needed for the rest of the platforms (ScummVM, PC, Apple, Saturn, etc.) but the no-intro names for NES, SNES, Atari, Genesis, TurboGrafx, PS1 and a few other platform tested should work fine.

Any feedback and testig is welcomed.

@cmitu cmitu referenced this pull request Jan 9, 2019

Open

New ES stable release #518

@joolswills

This comment has been minimized.

Copy link
Member

joolswills commented Jan 10, 2019

Thanks. This needs rebasing currently. Also some of the files contain the wrong indentation - please check the reset of the code (which uses tabs for indent).

@cmitu

This comment has been minimized.

Copy link

cmitu commented Jan 10, 2019

I've done a rebase locally, to get rid of the conflict merge, but didn't push it on Github.
I'll clean the indentation and rebase.

@cmitu cmitu force-pushed the cmitu:screenscraper branch from 6a35bcc to 88beb8b Jan 11, 2019

@cmitu

This comment has been minimized.

Copy link

cmitu commented Jan 11, 2019

@joolswills I replaced the spaces with tabs and rebased.
It should apply cleanly to the current master now.

@joolswills

This comment has been minimized.

Copy link
Member

joolswills commented Jan 16, 2019

Thanks. Anyone have any comments ? If not I will merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment