From d75510bde12945ce8d58029861c9d1330ff812e2 Mon Sep 17 00:00:00 2001 From: Markus Pointner Date: Sun, 3 Sep 2017 21:11:38 +0200 Subject: [PATCH] TheGamesDB scrapper should use GetGameList.php Previously GetGame.php was used, but GetGamesList.php is the "search" API call and more reliably returns the correct game. --- .../src/components/ScraperSearchComponent.cpp | 6 +- es-app/src/scrapers/GamesDBScraper.cpp | 68 +++++++++++++------ es-app/src/scrapers/GamesDBScraper.h | 13 +++- es-app/src/scrapers/Scraper.cpp | 8 ++- 4 files changed, 66 insertions(+), 29 deletions(-) diff --git a/es-app/src/components/ScraperSearchComponent.cpp b/es-app/src/components/ScraperSearchComponent.cpp index 89d954931..736cbe1be 100644 --- a/es-app/src/components/ScraperSearchComponent.cpp +++ b/es-app/src/components/ScraperSearchComponent.cpp @@ -231,11 +231,9 @@ void ScraperSearchComponent::onSearchDone(const std::vector mScraperResults = results; - const int end = results.size() > MAX_SCRAPER_RESULTS ? MAX_SCRAPER_RESULTS : results.size(); // at max display 5 - auto font = Font::get(FONT_SIZE_MEDIUM); unsigned int color = 0x777777FF; - if(end == 0) + if(results.empty()) { ComponentListRow row; row.addElement(std::make_shared(mWindow, "NO GAMES FOUND - SKIP", font, color), true); @@ -247,7 +245,7 @@ void ScraperSearchComponent::onSearchDone(const std::vector mGrid.resetCursor(); }else{ ComponentListRow row; - for(int i = 0; i < end; i++) + for(size_t i = 0; i < results.size(); i++) { row.elements.clear(); row.addElement(std::make_shared(mWindow, strToUpper(results.at(i).mdl.get("name")), font, color), true); diff --git a/es-app/src/scrapers/GamesDBScraper.cpp b/es-app/src/scrapers/GamesDBScraper.cpp index 47d4aa283..2a92e891f 100644 --- a/es-app/src/scrapers/GamesDBScraper.cpp +++ b/es-app/src/scrapers/GamesDBScraper.cpp @@ -74,31 +74,28 @@ const std::map gamesdb_platformid_map = boost::assign:: void thegamesdb_generate_scraper_requests(const ScraperSearchParams& params, std::queue< std::unique_ptr >& requests, std::vector& results) { - std::string path = "thegamesdb.net/api/GetGame.php?"; + std::string path; bool usingGameID = false; std::string cleanName = params.nameOverride; - if (cleanName.empty()) + if (!cleanName.empty() && cleanName.substr(0,3) == "id:") { - cleanName = params.game->getCleanName(); - path += "name=" + HttpReq::urlEncode(cleanName); - } - else - { - if (cleanName.substr(0,3) == "id:") { - std::string gameID = cleanName.substr(3,-1); - path += "id=" + HttpReq::urlEncode(gameID); - usingGameID = true; - } - else { - path += "exactname=" + HttpReq::urlEncode(cleanName); - } + std::string gameID = cleanName.substr(3); + path = "thegamesdb.net/api/GetGame.php?id=" + HttpReq::urlEncode(gameID); + usingGameID = true; + }else{ + if (cleanName.empty()) + cleanName = params.game->getCleanName(); + path += "thegamesdb.net/api/GetGamesList.php?name=" + HttpReq::urlEncode(cleanName); } - if(params.system->getPlatformIds().empty() || usingGameID) + if(usingGameID) { - // no platform specified, we're done + // if we have the ID already, we don't need the GetGameList request requests.push(std::unique_ptr(new TheGamesDBRequest(results, path))); + }else if(params.system->getPlatformIds().empty()){ + // no platform specified, we're done + requests.push(std::unique_ptr(new TheGamesDBRequest(requests, results, path))); }else{ // go through the list, we need to split this into multiple requests // because TheGamesDB API either sucks or I don't know how to use it properly... @@ -116,7 +113,7 @@ void thegamesdb_generate_scraper_requests(const ScraperSearchParams& params, std LOG(LogWarning) << "TheGamesDB scraper warning - no support for platform " << getPlatformName(*platformIt); } - requests.push(std::unique_ptr(new TheGamesDBRequest(results, path))); + requests.push(std::unique_ptr(new TheGamesDBRequest(requests, results, path))); } } } @@ -130,19 +127,27 @@ void TheGamesDBRequest::process(const std::unique_ptr& req, std::vector if(!parseResult) { std::stringstream ss; - ss << "GamesDBRequest - Error parsing XML. \n\t" << parseResult.description() << ""; + ss << "TheGamesDBRequest - Error parsing XML. \n\t" << parseResult.description() << ""; std::string err = ss.str(); setError(err); LOG(LogError) << err; return; } - pugi::xml_node data = doc.child("Data"); + if (isGameRequest()) + processGame(doc, results); + else + processList(doc, results); +} + +void TheGamesDBRequest::processGame(const pugi::xml_document& xmldoc, std::vector& results) +{ + pugi::xml_node data = xmldoc.child("Data"); std::string baseImageUrl = data.child("baseImgUrl").text().get(); pugi::xml_node game = data.child("Game"); - while(game && results.size() < MAX_SCRAPER_RESULTS) + if(game) { ScraperSearchResult result; @@ -179,6 +184,27 @@ void TheGamesDBRequest::process(const std::unique_ptr& req, std::vector } results.push_back(result); + } +} + +void TheGamesDBRequest::processList(const pugi::xml_document& xmldoc, std::vector& results) +{ + assert(mRequestQueue != nullptr); + + pugi::xml_node data = xmldoc.child("Data"); + pugi::xml_node game = data.child("Game"); + + // limit the number of results per platform, not in total. + // otherwise if the first platform returns >= 7 games + // but the second platform contains the relevant game, + // the relevant result would not be shown. + for(int i = 0; game && i < MAX_SCRAPER_RESULTS; i++) + { + std::string id = game.child("id").text().get(); + std::string path = "thegamesdb.net/api/GetGame.php?id=" + id; + + mRequestQueue->push(std::unique_ptr(new TheGamesDBRequest(results, path))); + game = game.next_sibling("Game"); } } diff --git a/es-app/src/scrapers/GamesDBScraper.h b/es-app/src/scrapers/GamesDBScraper.h index cf5f69ede..f692b131a 100644 --- a/es-app/src/scrapers/GamesDBScraper.h +++ b/es-app/src/scrapers/GamesDBScraper.h @@ -2,13 +2,24 @@ #include "scrapers/Scraper.h" +namespace pugi { class xml_document; } + void thegamesdb_generate_scraper_requests(const ScraperSearchParams& params, std::queue< std::unique_ptr >& requests, std::vector& results); class TheGamesDBRequest : public ScraperHttpRequest { public: - TheGamesDBRequest(std::vector& resultsWrite, const std::string& url) : ScraperHttpRequest(resultsWrite, url) {} + // ctor for a GetGameList request + TheGamesDBRequest(std::queue< std::unique_ptr >& requestsWrite, std::vector& resultsWrite, const std::string& url) : ScraperHttpRequest(resultsWrite, url), mRequestQueue(&requestsWrite) {} + // ctor for a GetGame request + TheGamesDBRequest(std::vector& resultsWrite, const std::string& url) : ScraperHttpRequest(resultsWrite, url), mRequestQueue(nullptr) {} + protected: void process(const std::unique_ptr& req, std::vector& results) override; + void processList(const pugi::xml_document& xmldoc, std::vector& results); + void processGame(const pugi::xml_document& xmldoc, std::vector& results); + bool isGameRequest() { return !mRequestQueue; } + + std::queue< std::unique_ptr >* mRequestQueue; }; diff --git a/es-app/src/scrapers/Scraper.cpp b/es-app/src/scrapers/Scraper.cpp index 5bc9b0ef6..72ec9e6d4 100644 --- a/es-app/src/scrapers/Scraper.cpp +++ b/es-app/src/scrapers/Scraper.cpp @@ -43,13 +43,15 @@ void ScraperSearchHandle::update() if(!mRequestQueue.empty()) { - auto& req = mRequestQueue.front(); - AsyncHandleStatus status = req->status(); + // a request can add more requests to the queue while running, + // so be careful with references into the queue + auto& req = *(mRequestQueue.front()); + AsyncHandleStatus status = req.status(); if(status == ASYNC_ERROR) { // propegate error - setError(req->getStatusString()); + setError(req.getStatusString()); // empty our queue while(!mRequestQueue.empty())