Minor enhancements and edge-case fixes (#181)

- fix edge case when romname contains roman and european numbers
- remove empty gamelist media subfolders after creating gamelist
- provide hint when game is 'almost' matched on how to support the matching
- provide hint/warning when exprected file is missing to complete artwork as defined in artwork XML file
- check well-formedness of artwork XML before entering threading
- test: remove surplus test
- 'Skyscraper -h' help texts adjusted

Author: Gemba

src/abstractscraper.cpp

@@ -684,7 +684,7 @@ void AbstractScraper::runPasses(QList<GameEntry> &gameEntries,
             }
             if (!candidates.empty()) {
                 candidates.sort(Qt::CaseInsensitive);
-                QString pad1 = candidates.length() > 3 ? "\n  " : " ";
+                QString pad1 = candidates.length() > 3 ? "\n  " : "";
                 QString pad2 = candidates.length() > 3 ? "\n  " : ", ";
                 debug.append(QString("Candidates: ") + pad1 +
                              candidates.join(pad2) + "\n");

src/cli.cpp

@@ -272,8 +272,8 @@ void Cli::createParser(QCommandLineParser *parser, QString platforms) {
     parser->addOption(uOption);
     parser->addOption(verbosityOption);
     parser->addVersionOption();
-    parser->addPositionalArgument(
-        "romfile", "Specific ROM to scrape, optionally.", "[<romfile>]");
+    parser->addPositionalArgument("romfile(s)", "Specific ROM(s) to scrape",
+                                  "[<romfile> [<romfile> [<romfile> ... ]]]");
 }
 
 void Cli::subCommandUsage(const QString subCmd) {

src/compositor.cpp

@@ -57,35 +57,33 @@
 
 Compositor::Compositor(Settings *config) { this->config = config; }
 
-bool Compositor::processXml() {
-    Layer newOutputs;
-
-    // Check document for errors before running through it
-    // FIXME: Segfault wenn fishy artwork file und threads > 1
-    // precheck in static methode auslagern (braucht eh nur artworkXml als param)
-    // und vor scraperworker spawn testen (nur wenn scraper == "cache")
+bool Compositor::preCheckArtworkXml(const QString &artworkXml) {
+    // Check document for errors before running through it with threads
     QDomDocument doc;
     QString eMsg;
     int eLine;
 #if QT_VERSION < 0x060800
-    if (!doc.setContent(config->artworkXml, false, &eMsg, &eLine)) {
+    if (!doc.setContent(artworkXml, false, &eMsg, &eLine)) {
 #else
-    QDomDocument::ParseResult p = QDomDocument::setContent(f.readAll());
+    QDomDocument::ParseResult p = doc.setContent(artworkXml);
     eMsg = p.errorMessage;
     eLine = p.errorLine;
     if (!p) {
 #endif
         qWarning() << "XML error:" << eMsg << "at line" << eLine;
         return false;
     }
+    return true;
+}
+
+void Compositor::processXml() {
     QXmlStreamReader xml(config->artworkXml);
+    Layer newOutputs;
 
     // Init recursive parsing
     addChildLayers(newOutputs, xml);
-
     // Assign global outputs to these new outputs
     outputs = newOutputs;
-    return true;
 }
 
 void Compositor::addChildLayers(Layer &layer, QXmlStreamReader &xml) {
@@ -374,7 +372,7 @@ void Compositor::saveAll(GameEntry &game, QString completeBaseName,
             if (!QDir().mkpath(fi.absolutePath())) {
                 qWarning() << "Path could not be created" << fi.absolutePath()
                            << " Check file permissions, gamelist binary data "
-                              "maybe incomplete.";
+                              "may be incomplete.";
             }
         }
 
@@ -419,6 +417,14 @@ void Compositor::processChildLayers(GameEntry &game, Layer &layer) {
             // If no meaningful canvas could be created, stop processing this
             // layer branch entirely
             if (thisLayer.canvas.isNull()) {
+                qWarning()
+                    << QString("Output of resource '%1' defined in artwork "
+                               "file, but no such data present for game '%2'. "
+                               "Output may not be as expected. To remediate "
+                               "this warning: Re-scrape to get the missing "
+                               "resource or adjust the artwork file.")
+                           .arg(thisLayer.resource)
+                           .arg(game.title);
                 continue;
             }
 

src/compositor.h

@@ -38,7 +38,8 @@ class Compositor : public QObject {
 
 public:
     Compositor(Settings *config);
-    bool processXml();
+    static bool preCheckArtworkXml(const QString &artworkXml);
+    void processXml();
     void saveAll(GameEntry &game, QString completeBaseName,
                  bool isBatocera = false);
     QString getSubpath(const QString &absPath);

src/nametools.cpp

@@ -279,6 +279,8 @@ int NameTools::getNumeral(const QString baseName) {
         QString roman = match.captured(1);
         numeral =
             arabicRomanNumerals().key(roman, QString::number(numeral)).toInt();
+        // 'Mega Man V MSU-1' (roman number wins)
+        return numeral;
     }
 
     // Check for european digits

src/scraperworker.cpp

@@ -104,12 +104,7 @@ void ScraperWorker::run() {
     platformOrig = config.platform;
 
     Compositor compositor(&config);
-    if (!compositor.processXml()) {
-        printf("Something went wrong when parsing artwork xml from '%s', "
-               "please check the file for errors. Now exiting...\n",
-               config.artworkConfig.toStdString().c_str());
-        exit(1);
-    }
+    compositor.processXml();
 
     while (queue->hasEntry()) {
         // takeEntry() also unlocks the mutex that was locked in hasEntry()
@@ -258,9 +253,18 @@ void ScraperWorker::run() {
         game.parNotes = NameTools::getUniqueNotes(game.parNotes, '(');
 
         if (!game.found) {
+            QString hint =
+                gameEntries.length() == 0
+                    ? " :("
+                    : QString(", but %1 candidate%1: Use --verbosity 3 or "
+                              "--flags interactive or --query '...' or set an "
+                              "alias (aliasMap.csv). =8^)")
+                          .arg(gameEntries.length() == 1 ? "" : "s")
+                          .arg(gameEntries.length());
             output.append(
-                QString("\033[1;33m---- Game '%1' not found :( ----\033[0m\n\n")
-                    .arg(info.completeBaseName()));
+                QString("\033[1;33m---- Game '%1' not found%2 ----\033[0m\n\n")
+                    .arg(info.completeBaseName())
+                    .arg(hint));
             game.resetMedia();
             if (!forceEnd) {
                 forceEnd = limitReached(output);

src/skyscraper.cpp

@@ -28,6 +28,7 @@
 #include "attractmode.h"
 #include "batocera.h"
 #include "cli.h"
+#include "compositor.h"
 #include "config.h"
 #include "emulationstation.h"
 #include "esde.h"
@@ -396,6 +397,14 @@ void Skyscraper::run() {
         }
     }
     printf("\n");
+
+    if (!Compositor::preCheckArtworkXml(config.artworkXml)) {
+        printf("Parsing artwork XML from '%s', failed, see above."
+               "Check the file for errors. Now exiting...\n",
+               config.artworkConfig.toStdString().c_str());
+        exit(1);
+    }
+
     if (!doCacheScraping) {
         printf("Starting scraping run on \033[1;32m%d\033[0m files using "
                "\033[1;32m%d\033[0m threads.\nSit back, relax and let me do "
@@ -445,7 +454,7 @@ void Skyscraper::prepareFileQueue() {
     QDir inputDir(config.inputFolder, getPlatformFileExtensions(), QDir::Name,
                   filter);
     if (!inputDir.exists()) {
-        printf("Input folder '\033[1;32m%s\033[0m' doesn't exist or can't be "
+        printf("Input folder '\033[1;31m%s\033[0m' doesn't exist or can't be "
                "accessed by current user. Please check path and permissions.\n",
                inputDir.absolutePath().toStdString().c_str());
         exit(1);
@@ -588,20 +597,17 @@ void Skyscraper::setFolder(const bool doCacheScraping, QString &outFolder,
 
 void Skyscraper::checkForFolder(QDir &folder, bool create) {
     if (!folder.exists()) {
-        printf("Folder '%s' doesn't exist",
-               folder.absolutePath().toStdString().c_str());
         if (create) {
-            printf(", trying to create it... ");
-            fflush(stdout);
-            if (folder.mkpath(folder.absolutePath())) {
-                printf("\033[1;32mSuccess!\033[0m\n");
-            } else {
-                printf("\033[1;32mFailed!\033[0m Please check path and "
-                       "permissions, now exiting...\n");
+            if (!folder.mkpath(folder.absolutePath())) {
+                printf(
+                    "Create folder '\033[1;31m%s\033[0m' failed! Please "
+                    "check path and filesystem permissions, now exiting...\n",
+                    folder.absolutePath().toStdString().c_str());
                 exit(1);
             }
         } else {
-            printf(", can't continue...\n");
+            printf("Folder '%s' doesn't exist, can't continue...\n",
+                   folder.absolutePath().toStdString().c_str());
             exit(1);
         }
     }
@@ -772,6 +778,18 @@ void Skyscraper::checkThreads() {
         printf("\n\n");
     }
 
+    if (doCacheScraping) {
+        QStringList mediaDirs = {
+            config.coversFolder,   config.screenshotsFolder,
+            config.wheelsFolder,   config.marqueesFolder,
+            config.texturesFolder, config.videosFolder,
+            config.manualsFolder,  config.fanartsFolder};
+        for (const auto &f : mediaDirs) {
+            QDir dir(f);
+            dir.rmdir(f);
+        }
+    }
+
     // All done, now clean up and exit to terminal
     emit finished();
 }
@@ -936,7 +954,7 @@ void Skyscraper::loadConfig(const QCommandLineParser &parser) {
         }
     }
 
-    // defaults are always absolute, thus input and mediafolder will be
+    // defaults are always absolute, thus input- and mediafolder will be
     // unchanged by these calls
     if (config.frontend == "pegasus") {
         config.inputFolder =
@@ -1016,10 +1034,7 @@ void Skyscraper::loadConfig(const QCommandLineParser &parser) {
         }
         if (requestedFileInfo.exists()) {
             QString romPath = requestedFileInfo.absoluteFilePath();
-            if (config.frontend == "emulationstation" ||
-                config.frontend == "esde") {
-                romPath = normalizePath(requestedFileInfo);
-            }
+            romPath = normalizePath(requestedFileInfo);
             if (!romPath.isEmpty()) {
                 cliFiles.append(romPath);
                 // Always set refresh and unattend true if user has supplied
@@ -1031,12 +1046,12 @@ void Skyscraper::loadConfig(const QCommandLineParser &parser) {
                 continue;
             }
         }
-        printf("Filename: '\033[1;32m%s\033[0m' requested either on "
-               "command line or with '--includefrom' neither found in platform "
-               "folder '%s' nor in current directory!\n\nPlease verify the "
-               "filename and try again...\n",
-               requestedFile.toStdString().c_str(),
-               config.platform.toStdString().c_str());
+        printf("Filename: '\033[1;33m%s\033[0m' requested either on command "
+               "line or with '--includefrom' not found in(side) the input "
+               "directory '\033[1;33m%s\033[0m'!\n\nPlease verify the filename "
+               "and try again...\n",
+               requestedFileInfo.fileName().toStdString().c_str(),
+               config.inputFolder.toStdString().c_str());
         exit(1);
     }
 
@@ -1116,8 +1131,10 @@ QString Skyscraper::normalizePath(QFileInfo fileInfo) {
     // normalize paths for single romfiles provided at the CLI.
     // format will be: config.inputFolder + relative-path-of-romfile
     QString canonicalRomPath = fileInfo.canonicalFilePath();
+
     // for Windows
     QString cleanRomPath = QDir::cleanPath(canonicalRomPath);
+
     QListIterator<QString> iter(cleanRomPath.split("/"));
     iter.toBack();
     QString relativeRomPath;

src/strtools.cpp

@@ -290,7 +290,7 @@ QString StrTools::getVersionHeader() {
         dashesString = dashesString % "-";
     }
 
-    return QString("\033[1;34m" % dashesString % "\033[0m\n\033[1;33m" %
+    return QString("\033[1;34m" % dashesString % "\033[0m\n\033[1m" %
                    headerString % "\033[0m\n\033[1;34m" % dashesString %
                    "\033[0m\n");
 }
@@ -364,7 +364,7 @@ QString StrTools::tidyText(QString text, bool ignoreBangs) {
     int ctr = 0;
     int tmpCtr = -1;
     bool itemize = false;
-    QRegularExpression re("^[\\*●]\\s+");
+    QRegularExpression re("^[\\*●•]\\s+");
     QString tmpPiggy;
     for (auto l : pars) {
         ctr++;

src/xmlreader.cpp

@@ -45,8 +45,7 @@ bool XmlReader::setFile(QString filename) {
 #if QT_VERSION < 0x060800
         if (setContent(f.readAll(), false, &eMsg, &eLine)) {
 #else
-        // FIXME: compile against 6.8 or later
-        QDomDocument::ParseResult p = QDomDocument::setContent(f.readAll());
+        QDomDocument::ParseResult p = setContent(f.readAll());
         eMsg = p.errorMessage;
         eLine = p.errorLine;
         if (p) {

test/nametools/test_nametools.cpp

@@ -67,6 +67,7 @@ private slots:
         }
         QCOMPARE(NameTools::getNumeral("Blarf 0"), 1);
         QCOMPARE(NameTools::getNumeral("Blarf -1"), 1);
+        QCOMPARE(NameTools::getNumeral("Mega Man V MSU-1"), 5);
     }
 
     void testGetSqrNotes() {