From bb3cc4d4a1e8d0efdfc2b1a9ebc1c9f3f5e42230 Mon Sep 17 00:00:00 2001 From: Leon Styhre Date: Fri, 18 Dec 2020 16:35:19 +0100 Subject: [PATCH] Added logic and menu option for handling invalid media files during scraping. Also added a shortcut for defining the scraper searches and fixed an issue where games were automatically selected after refining the search. --- USERGUIDE.md | 8 +++++--- es-app/src/guis/GuiScraperMenu.cpp | 17 +++++++++++++++-- es-app/src/guis/GuiScraperSearch.cpp | 17 ++++++++++++++--- es-app/src/guis/GuiScraperSearch.h | 1 + es-app/src/scrapers/Scraper.cpp | 27 +++++++++++++++++++++++++++ es-core/src/Settings.cpp | 1 + 6 files changed, 63 insertions(+), 8 deletions(-) diff --git a/USERGUIDE.md b/USERGUIDE.md index f611b1b26..1747cb407 100644 --- a/USERGUIDE.md +++ b/USERGUIDE.md @@ -491,9 +491,7 @@ The single-game scraper is launched from the metadata editor. You navigate to a ### Multi-scraper -The multi-scraper is launched from the main menu, it's the first option on the menu actually. Here you can configure a number of scraping options, all which are explained in more depth below when covering the main menu entries. - -**_Tip: Pressing the 's' key on the keyboard is a quick-skip shortcut rather than having to navigate to the 'Skip' button and selecting it._** +The multi-scraper is launched from the main menu, it's the first option on the menu. Here you can configure a number of scraping options, all which are explained in more depth below when covering the main menu entries. ### Scraping process @@ -642,6 +640,10 @@ Currently only English or World are supported, not really significant at the mom Affects both overwriting of metadata as well as actual game media files on the filesystem. +**Halt on invalid media files** + +With this setting enabled, if any media files returned by the scraper seem to be invalid, the scraping is halted and an error message is presented where it's possible to retry or cancel the scraping of the specific game. In the case of multi-scraping it's also possible to skip the game and proceed to the next one in the queue. With this setting disabled, all media files will always be accepted and saved to disk. As of ES-DE v1.0 the file verification is crude as it's just checking if the file is less than 350 bytes in size which should indicate a server error response rather than a real media file. In some exceedingly rare situations, proper media files may be smaller than 350 bytes, and for those rare instances, simply disabling this setting temporarily allows these files to be scraped. Future versions of ES-DE will implement proper CRC/checksum verifications for ScreenScraper and possibly media file integrity checks for TheGamesDB (as this scraper service does not provide file checksums). + **Search using metadata name** By default ES-DE will perform scraper searches based on the game name that has been manually set in the metadata editor, or that has been previously scraped. If you prefer to search using the physical name of the game file or directory, then turn off this option. The default game name will correspond to the name of the physical file or directory, so for the first scraping of any given game, this option makes no difference. diff --git a/es-app/src/guis/GuiScraperMenu.cpp b/es-app/src/guis/GuiScraperMenu.cpp index a3c56a67d..2f1c2b88b 100644 --- a/es-app/src/guis/GuiScraperMenu.cpp +++ b/es-app/src/guis/GuiScraperMenu.cpp @@ -383,6 +383,20 @@ void GuiScraperMenu::openOtherSettings() } }); + // Halt scraping on invalid media files. + auto scraper_halt_on_invalid_media = std::make_shared(mWindow); + scraper_halt_on_invalid_media-> + setState(Settings::getInstance()->getBool("ScraperHaltOnInvalidMedia")); + s->addWithLabel("HALT ON INVALID MEDIA FILES", scraper_halt_on_invalid_media); + s->addSaveFunc([scraper_halt_on_invalid_media, s] { + if (scraper_halt_on_invalid_media->getState() != + Settings::getInstance()->getBool("ScraperHaltOnInvalidMedia")) { + Settings::getInstance()->setBool("ScraperHaltOnInvalidMedia", + scraper_halt_on_invalid_media->getState()); + s->setNeedsSaving(); + } + }); + // Search using metadata name. auto scraper_search_metadata_name = std::make_shared(mWindow); scraper_search_metadata_name-> @@ -602,8 +616,7 @@ bool GuiScraperMenu::input(InputConfig* config, Input input) if (GuiComponent::input(config, input)) return true; - if (config->isMappedTo("b", input) && - input.value != 0) { + if (config->isMappedTo("b", input) && input.value != 0) { delete this; return true; } diff --git a/es-app/src/guis/GuiScraperSearch.cpp b/es-app/src/guis/GuiScraperSearch.cpp index 933e5ac15..445ee8b73 100644 --- a/es-app/src/guis/GuiScraperSearch.cpp +++ b/es-app/src/guis/GuiScraperSearch.cpp @@ -44,6 +44,7 @@ GuiScraperSearch::GuiScraperSearch( mSearchType(type), mScrapeCount(scrapeCount), mScrapeRatings(false), + mRefinedSearch(false), mFoundGame(false) { addChild(&mGrid); @@ -487,8 +488,12 @@ bool GuiScraperSearch::input(InputConfig* config, Input input) return true; } - // Quick-skip option activated by pressing 's' on the keyboard. - if (config->getDeviceId() == DEVICE_KEYBOARD && input.id == SDLK_s && input.value != 0) + // Refine search. + if (config->isMappedTo("y", input) && input.value != 0) + openInputScreen(mLastSearch); + + // Skip game. + if (mScrapeCount > 1 && config->isMappedTo("x", input) && input.value != 0) mSkipCallback(); return GuiComponent::input(config, input); @@ -644,8 +649,10 @@ void GuiScraperSearch::updateThumbnail() // we are in semi-automatic mode with a single matching game result, we proceed // to immediately download the rest of the media files. if ((mSearchType == ALWAYS_ACCEPT_FIRST_RESULT || - (mSearchType == ACCEPT_SINGLE_MATCHES && mScraperResults.size() == 1)) && + (mSearchType == ACCEPT_SINGLE_MATCHES && mScraperResults.size() == 1 && + mRefinedSearch == false)) && mScraperResults.front().thumbnailDownloadStatus == COMPLETED) { + mRefinedSearch = false; if (mScraperResults.size() == 0) mSkipCallback(); else @@ -656,6 +663,7 @@ void GuiScraperSearch::updateThumbnail() void GuiScraperSearch::openInputScreen(ScraperSearchParams& params) { auto searchForFunc = [&](const std::string& name) { + mRefinedSearch = true; params.nameOverride = name; search(params); }; @@ -757,6 +765,9 @@ std::vector GuiScraperSearch::getHelpPrompts() { std::vector prompts; + prompts.push_back(HelpPrompt("y", "refine search")); + if (mScrapeCount > 1) + prompts.push_back(HelpPrompt("x", "skip")); if (mFoundGame) prompts.push_back(HelpPrompt("a", "accept result")); diff --git a/es-app/src/guis/GuiScraperSearch.h b/es-app/src/guis/GuiScraperSearch.h index 8f8d95e40..270437701 100644 --- a/es-app/src/guis/GuiScraperSearch.h +++ b/es-app/src/guis/GuiScraperSearch.h @@ -119,6 +119,7 @@ private: std::function mSkipCallback; std::function mCancelCallback; unsigned int mScrapeCount; + bool mRefinedSearch; bool mBlockAccept; bool mFoundGame; bool mScrapeRatings; diff --git a/es-app/src/scrapers/Scraper.cpp b/es-app/src/scrapers/Scraper.cpp index 81e732af1..70b6f3ca6 100644 --- a/es-app/src/scrapers/Scraper.cpp +++ b/es-app/src/scrapers/Scraper.cpp @@ -242,6 +242,20 @@ MDResolveHandle::MDResolveHandle(const ScraperSearchResult& result, if (mResult.thumbnailImageUrl == it->fileURL && mResult.thumbnailImageData.size() > 0) { + // This is just a temporary workaround to avoid saving media files to disk that + // are actually just containing error messages from the scraper service. The + // proper solution is to implement file checksum checks to determine if the + // server response contains valid media. The problem with this temporary + // solution is of course that any tiny media files of less than 300 bytes + // will not be saved to disk. + if (Settings::getInstance()->getBool("ScraperHaltOnInvalidMedia") && + mResult.thumbnailImageData.size() < 350) { + setError("The file '" + Utils::FileSystem::getFileName(filePath) + + "' returned by the scraper seems to be invalid as it's less than " + + "350 bytes in size."); + return; + } + // Remove any existing media file before attempting to write a new one. // This avoids the problem where there's already a file for this media type // with a different format/extension (e.g. game.jpg and we're going to write @@ -374,6 +388,19 @@ void MediaDownloadHandle::update() // Download is done, save it to disk. + // This is just a temporary workaround to avoid saving media files to disk that + // are actually just containing error messages from the scraper service. The + // proper solution is to implement file checksum checks to determine if the + // server response contains valid media. The problem with this temporary + // solution is of course that any tiny media files of less than 300 bytes + // will not be saved to disk. + if (Settings::getInstance()->getBool("ScraperHaltOnInvalidMedia") && + mReq->getContent().size() < 350) { + setError("The file '" + Utils::FileSystem::getFileName(mSavePath) + "' returned by the " + + "scraper seems to be invalid as it's less than 350 bytes in size."); + return; + } + // Remove any existing media file before attempting to write a new one. // This avoids the problem where there's already a file for this media type // with a different format/extension (e.g. game.jpg and we're going to write diff --git a/es-core/src/Settings.cpp b/es-core/src/Settings.cpp index 32ea9c287..061b02882 100644 --- a/es-core/src/Settings.cpp +++ b/es-core/src/Settings.cpp @@ -99,6 +99,7 @@ void Settings::setDefaults() mStringMap["ScraperRegion"] = { "eu", "eu" }; mStringMap["ScraperLanguage"] = { "en", "en" }; mBoolMap["ScraperOverwriteData"] = { true, true }; + mBoolMap["ScraperHaltOnInvalidMedia"] = { true, true }; mBoolMap["ScraperSearchMetadataName"] = { true, true }; mBoolMap["ScraperInteractive"] = { true, true }; mBoolMap["ScraperSemiautomatic"] = { true, true };