Various minor enhancements, fixes and doc updates (#204)

* Various minor enhancements, fixes and doc updates

* Fix logic error in screenshot scrape of TGDB
* Update Screenscraper thread count messages and general numeric options range checks
* Enable Skyscraper unit tests in CI workflow and fix unittest for test/settings
* Documentation addition when using folder options (input, gamelist, media folder) in frontend INI section.
* Path outputs with ~ instead verbatim home path
* Ensure Python3 is present when using update_skyscraper script

Author: Gemba

README.md

@@ -89,6 +89,19 @@ The scriptmodule will also automagically install programmable completion for the
 
 _Qt5 is end-of-life by end of May 2025_: You can still use and compile Skyscraper with Qt5 with no restrictions, when your setup does not provide Qt6. However, if you have the option to use Qt6 I strongly recommend it, Skyscraper works well with Qt6. Use Qt6 especially when you use Skyscraper on a recent Linux distribution, macOS or Windows. In some cases you may have to use `qmake6` instead of `qmake`. If you find a mismatch in the build scripts, please file an issue.
 
+In general you should have these packages installed:
+
+```
+$ sudo apt install \
+    make \
+    g++ \
+    gcc \
+    git \
+    python3
+```
+
+Python3 is needed for the `update_skyscraper.sh` script and if you want to use the Python scripts in the supplementary/ folder.
+
 #### Linux <img src="docs/resources/os/Tux.svg" width="20px">
 _For Qt6_:
 ```bash
@@ -100,8 +113,6 @@ _For Qt5 (these are legacy installation prerequisites!)_: Skyscraper needs Qt5.1
 ```bash
 $ sudo apt update
 $ sudo apt install qtbase5-dev qtchooser qt5-qmake qtbase5-dev-tools libqt5sql5-sqlite p7zip-full
-# You may need these too, if they are not installed already
-$ sudo apt install make g++ gcc git
 ```
 To install Qt5 (disencouraged) or Qt6 (preferred) on other Linux distributions, please refer to their documentation.
 

docs/CLIHELP.md

@@ -191,13 +191,13 @@ Skyscraper -p snes -s import
 
 Read more about [each scraping module](SCRAPINGMODULES.md).
 
-### -t <1-8>
+### -t <1-...>
 
-Sets the desired number of parallel threads to be run when scraping. By default it is set to 4.
+Sets the desired number of parallel threads to be run when scraping. By default it is set to 4. You can not set this higher than the [ideal thread count](https://doc.qt.io/qt-6/qthread.html#idealThreadCount), which is usually the output of `nproc` on your system.
 
 !!! note
 
-    Some modules have maximum allowed threads. If you set this higher than the allowed value, it will be auto-adjusted.
+    Some scraping modules have maximum allowed threads. If you set this higher than the allowed value, it will be auto-adjusted. For gamelist creation runs you may set it to the maximum.
 
 **Example(s)**
 

docs/CONFIGINI.md

@@ -149,7 +149,7 @@ This is an alphabetical index of all configuration options their usage level and
 
 Sets the rom input folder. By default Skyscraper will look for roms in the `/home/<USER>/RetroPie/roms/<PLATFORM>` folder. If your roms are located in a non-default location, you can set the input path using this option.
 
-!!! note
+!!! note "Path Appended With `<PLATFORM>`"
 
     If this is set in the `[main]` or `[<FRONTEND>]` section it will automatically add `/<PLATFORM>` to the end of the path. If you want better control consider adding it to a `[<PLATFORM>]` section instead where it will be used as is.
 
@@ -158,13 +158,20 @@ Allowed in sections: `[main]`, `[<PLATFORM>]`, `[<FRONTEND>]`
 
 The default is valid for the most frontends, very few [frontends](FRONTENDS.md) do use a different default value.
 
+!!! tip "Usage in `[<FRONTEND>]` Section"
+
+    When you use this option in a `[<FRONTEND>]` section other than
+    the default `emulationstation`, then it is only evaluated when you provide
+    either `-f` command line parameter or have set the [`frontend`](#frontend)
+    option during a scraping run. 
+
 ---
 
 #### gameListFolder
 
 Sets the game list export folder. By default Skyscraper exports the game list to the same directory as the rom input folder. This enables you to change that to a non-default location.
 
-!!! note
+!!! note "Path Appended With `<PLATFORM>`"
 
     If this is set in the `[main]` or `[<FRONTEND>]` section it will automatically add `/<PLATFORM>` to the end of the path. If you want better control consider adding it to a `[<PLATFORM>]` section instead where it will be used as is.
 
@@ -173,6 +180,13 @@ Allowed in sections: `[main]`, `[<PLATFORM>]`, `[<FRONTEND>]`
 
 The default is valid for the most frontends, very few [frontends](FRONTENDS.md) do use a different default value.
 
+!!! tip "Usage in `[<FRONTEND>]` Section"
+
+    When you use this option in a `[<FRONTEND>]` section other than
+    the default `emulationstation`, then it is only evaluated when you provide
+    either `-f` command line parameter or have set the [`frontend`](#frontend)
+    option during a scraping run. 
+
 ---
 
 #### gameListFilename
@@ -199,7 +213,7 @@ Sets the artwork / media output folder. By default (ie. if the option `mediaFold
 
 Read more about the [artwork compositing](ARTWORK.md).
 
-!!! note
+!!! note "Path Appended With `<PLATFORM>`"
 
     If this is set in the `[main]` or `[<FRONTEND>]` section it will automatically add `/<PLATFORM>` to the end of the path. If you want better control consider adding it to a `[<PLATFORM>]` section instead where it will be used as is.
 
@@ -208,6 +222,13 @@ Allowed in sections: `[main]`, `[<PLATFORM>]`, `[<FRONTEND>]`
 
 The default is valid for the most frontends, very few [frontends](FRONTENDS.md) do use a different default value.
 
+!!! tip "Usage in `[<FRONTEND>]` Section"
+
+    When you use this option in a `[<FRONTEND>]` section other than
+    the default `emulationstation`, then it is only evaluated when you provide
+    either `-f` command line parameter or have set the [`frontend`](#frontend)
+    option during a scraping run. 
+
 ---
 
 #### mediaFolderHidden
@@ -635,7 +656,7 @@ Allowed in sections: `[main]`, `[<PLATFORM>]`, `[<SCRAPER>]`
 
 #### threads
 
-Sets the desired number of parallel threads to be run when scraping. Some modules have maximum allowed threads. If you set this higher than the allowed value, it will be auto-adjusted. By default it is set to 4.
+Sets the desired number of parallel threads to be run when scraping or creating the gamelist. Some modules have maximum allowed threads. If you set this higher than the allowed value, it will be auto-adjusted. By default it is set to 4. You can not set this higher than the [ideal thread count](https://doc.qt.io/qt-6/qthread.html#idealThreadCount), which is usually the output of `nproc` on your Linux system.
 
 Default value: `4`  
 Allowed in sections: `[main]`, `[<PLATFORM>]`, `[<SCRAPER>]`

src/cache.cpp

@@ -1179,12 +1179,15 @@ void Cache::assembleReport(const Settings &config, const QString filter) {
 
     QString dateTime = QDateTime::currentDateTime().toString("yyyyMMdd");
     for (const auto &resType : resTypeList) {
-        QFile reportFile(reportsDir.absolutePath() + "/report-" +
-                         config.platform + "-missing_" + resType + "-" +
-                         dateTime + ".txt");
+        QString rFn = QString("{1}/report-{2}-missing_{3}-{4}.txt")
+                          .arg(reportsDir.absolutePath())
+                          .arg(config.platform)
+                          .arg(resType)
+                          .arg(dateTime);
+        QFile reportFile(rFn);
         printf("Report filename: '\033[1;32m%s\033[0m'\nAssembling report, "
                "please wait...",
-               reportFile.fileName().toStdString().c_str());
+               Config::pathToCStr(rFn));
         if (reportFile.open(QIODevice::WriteOnly)) {
             int missing = 0;
             int dots = 0;

src/config.cpp

@@ -383,4 +383,12 @@ QString &Config::expandHomePath(QString &path) {
         // "~account/folder" not supported
     }
     return path;
+}
+
+const char *Config::pathToCStr(QString &in) {
+    QString ret = in;
+#ifndef Q_OS_WIN
+    ret = ret.replace(QDir::homePath(), "~");
+#endif
+    return ret.toUtf8().constData();
 }

src/config.h

@@ -46,6 +46,7 @@ namespace Config {
     QString lexicallyRelativePath(const QString &base, const QString &other);
     QString lexicallyNormalPath(const QString &pathWithDots);
     QString &expandHomePath(QString &path);
+    const char *pathToCStr(QString &in);
 } // namespace Config
 
 #endif // CONFIG_H

src/screenscraper.cpp

@@ -169,9 +169,9 @@ void ScreenScraper::getSearchResults(QList<GameEntry> &gameEntries,
                                            "191;167;200;198;192;228;169;156"),
                          "****");
             data.replace(config->password.toUtf8(), "****");
-            QFile errorResponse(
-                Config::getSkyFolder(Config::SkyFolderType::LOG) +
-                "/screenscraper_error.txt");
+            QString errFile = Config::getSkyFolder(Config::SkyFolderType::LOG) %
+                              "/screenscraper_error.txt";
+            QFile errorResponse(errFile);
             if (errorResponse.open(QIODevice::WriteOnly)) {
                 if (data.length() > 64) {
                     if (data.startsWith("****I****l**** "
@@ -182,16 +182,12 @@ void ScreenScraper::getSearchResults(QList<GameEntry> &gameEntries,
                             "\nEN: Mandatory fields are missing in the URL.\n");
                     }
                     errorResponse.write(data);
-                    printf("The erroneous answer was written to "
-                           "'%s'. "
-                           "If this file contains game data, please consider "
-                           "filing a bug report at "
+                    printf("The erroneous answer was written to '%s'. If you "
+                           "expected to scrape game data and this error "
+                           "persists, please consider filing a bug report at "
                            "'https://github.com/Gemba/skyscraper/issues' and "
                            "attach that file.\n",
-                           QFileInfo(errorResponse)
-                               .absoluteFilePath()
-                               .toStdString()
-                               .c_str());
+                           Config::pathToCStr(errFile));
                 }
                 errorResponse.close();
             }

src/settings.cpp

@@ -27,6 +27,7 @@
 #include <QDebug>
 #include <QFileInfo>
 #include <QStringBuilder>
+#include <QThread>
 #include <filesystem>
 
 static inline bool isArcadePlatform(const QString &platform) {
@@ -146,7 +147,7 @@ void RuntimeCfg::applyConfigIni(CfgType type, QSettings *settings,
         default:;
         }
         qInfo() << "Section type" << type << "[" << section << "]"
-                << "has surplus key(s) (=ignored): " << invalid;
+                << "has surplus key(s), which will be ignored: " << invalid;
     }
 
     for (auto k : retained) {
@@ -495,19 +496,17 @@ void RuntimeCfg::applyConfigIni(CfgType type, QSettings *settings,
             bool intOk;
             int v = ss.toInt(&intOk);
             if (!intOk) {
-                printf("\033[1;31mConversion of %s to integer failed for key "
-                       "%s. Please fix in config.ini.\n\033[0m",
-                       ss.toString().toUtf8().constData(),
-                       k.toUtf8().constData());
+                printf(
+                    "\033[1;31mConversion of %s to integer failed for option "
+                    "%s. Please fix in config.ini.\n\033[0m",
+                    ss.toString().toUtf8().constData(), k.toUtf8().constData());
                 exit(1);
             }
             if (k == "jpgQuality") {
                 if (0 < v && v <= 100) {
                     config->jpgQuality = v;
                 } else {
-                    printf("\033[1;33mValue of %d is out of range and is "
-                           "ignored! Consult the documentation.\n\033[0m",
-                           v);
+                    outOfRange(k, v);
                 }
                 continue;
             }
@@ -519,9 +518,7 @@ void RuntimeCfg::applyConfigIni(CfgType type, QSettings *settings,
                 if (0 < v && v <= 200) {
                     config->maxFails = v;
                 } else {
-                    printf("\033[1;33mValue of %d is out of range and is "
-                           "ignored! Consult the documentation.\n\033[0m",
-                           v);
+                    outOfRange(k, v);
                 }
                 continue;
             }
@@ -532,24 +529,24 @@ void RuntimeCfg::applyConfigIni(CfgType type, QSettings *settings,
                     config->minMatch = v;
                     config->minMatchSet = true;
                 } else {
-                    printf("\033[1;33mValue of %d is out of range and is "
-                           "ignored! Consult the documentation.\n\033[0m",
-                           v);
+                    outOfRange(k, v);
                 }
                 continue;
             }
             if (k == "threads") {
-                config->threads = v;
-                config->threadsSet = true;
+                if (v > 0 && v <= QThread::idealThreadCount()) {
+                    config->threads = v;
+                    config->threadsSet = true;
+                } else {
+                    outOfRange(k, v);
+                }
                 continue;
             }
             if (k == "verbosity") {
                 if (0 < v && v <= 3) {
                     config->verbosity = v;
                 } else {
-                    printf("\033[1;33mValue of %d is out of range and is "
-                           "ignored! Consult the documentation.\n\033[0m",
-                           v);
+                    outOfRange(k, v);
                 }
                 continue;
             }
@@ -567,7 +564,8 @@ void RuntimeCfg::applyCli(bool &inputFolderSet, bool &gameListFolderSet,
         parser->value("l").toInt() <= 10000) {
         config->maxLength = parser->value("l").toInt();
     }
-    if (parser->isSet("t") && parser->value("t").toInt() <= 8) {
+    if (parser->isSet("t") && parser->value("t").toInt() > 0 &&
+        parser->value("t").toInt() <= QThread::idealThreadCount()) {
         config->threads = parser->value("t").toInt();
         config->threadsSet = true;
     }
@@ -913,4 +911,10 @@ QString RuntimeCfg::getAllExtensionsOfPlatform() {
     return Platform::get()
         .getFormats(config->platform, config->extensions, config->addExtensions)
         .remove('*');
+}
+
+void RuntimeCfg::outOfRange(QString &k, int v) {
+    printf("\033[1;33mValue of %d is out of range for option %s and is "
+           "ignored! Consult the documentation.\n\033[0m",
+           v, k.toStdString().c_str());
 }

src/settings.h

@@ -195,6 +195,7 @@ class RuntimeCfg : public QObject {
     QString toAbsolutePath(bool isCliOpt, QString optionVal);
     QString parseExtensions(const QString &optionVal);
     QString getAllExtensionsOfPlatform();
+    void outOfRange(QString &k, int v);
 
     Settings *config;
     const QCommandLineParser *parser;