Fix some issues with the filewatcher

include/core/map.h

@@ -127,6 +127,7 @@ private:
     bool m_hasUnsavedDataChanges = false;
     bool m_needsHealLocation = false;
     bool m_scriptsLoaded = false;
+    bool m_loggedScriptsFileError = false;
 
     QMap<Event::Group, QList<Event *>> m_events;
     QSet<Event *> m_ownedEvents; // for memory management

include/core/parseutil.h

@@ -80,7 +80,7 @@ public:
     static int getScriptLineNumber(const QString &filePath, const QString &scriptLabel);
     static int getRawScriptLineNumber(QString text, const QString &scriptLabel);
     static int getPoryScriptLineNumber(QString text, const QString &scriptLabel);
-    static QStringList getGlobalScriptLabels(const QString &filePath);
+    static QStringList getGlobalScriptLabels(const QString &filePath, QString *error = nullptr);
     static QStringList getGlobalRawScriptLabels(QString text);
     static QStringList getGlobalPoryScriptLabels(QString text);
     static QString removeStringLiterals(QString text);

include/core/utility.h

@@ -11,6 +11,7 @@ namespace Util {
     QString toDefineCase(QString input);
     QString toHexString(uint32_t value, int minLength = 0);
     QString toHtmlParagraph(const QString &text);
+    QString stripPrefix(const QString &s, const QString &prefix);
     Qt::Orientations getOrientation(bool xflip, bool yflip);
     QString replaceExtension(const QString &path, const QString &newExtension);
     void setErrorStylesheet(QLineEdit *lineEdit, bool isError);

include/project.h

@@ -294,6 +294,8 @@ private:
     // We can't display these layouts to the user, but we want to preserve the data when they save.
     QList<QJsonObject> failedLayoutsData;
 
+    QSet<QString> failedFileWatchPaths;
+
     const QRegularExpression re_gbapalExtension;
     const QRegularExpression re_bppExtension;
 
@@ -322,8 +324,8 @@ private:
     void setNewLayoutBlockdata(Layout *layout);
     void setNewLayoutBorder(Layout *layout);
 
-    void watchFile(const QString &filename);
-    void watchFiles(const QStringList &filenames);
+    bool watchFile(const QString &filename);
+    bool watchFiles(const QStringList &filenames);
     void ignoreWatchedFileTemporarily(const QString &filepath);
     void ignoreWatchedFilesTemporarily(const QStringList &filepaths);
     void recordFileChange(const QString &filepath);

src/core/map.cpp

@@ -149,12 +149,26 @@ void Map::invalidateScripts() {
 QStringList Map::getScriptLabels(Event::Group group) {
     if (!m_scriptsLoaded) {
         const QString scriptsFilepath = getScriptsFilepath();
-        m_scriptLabels = ParseUtil::getGlobalScriptLabels(scriptsFilepath);
+        QString error;
+        m_scriptLabels = ParseUtil::getGlobalScriptLabels(scriptsFilepath, &error);
         m_scriptsLoaded = true;
 
+        if (!error.isEmpty() && !m_loggedScriptsFileError) {
+            logWarn(QString("Failed to read scripts file '%1' for %2: %3")
+                            .arg(Util::stripPrefix(scriptsFilepath, projectConfig.projectDir() + "/"))
+                            .arg(m_name)
+                            .arg(error));
+            m_loggedScriptsFileError = true;
+        }
+
         // Track the scripts file for changes. Path may have changed, so stop tracking old files.
         m_scriptFileWatcher->removePaths(m_scriptFileWatcher->files());
-        m_scriptFileWatcher->addPath(scriptsFilepath);
+        if (!m_scriptFileWatcher->addPath(scriptsFilepath) && !m_loggedScriptsFileError) {
+            logWarn(QString("Failed to add scripts file '%1' to file watcher for %2.")
+                            .arg(Util::stripPrefix(scriptsFilepath, projectConfig.projectDir() + "/"))
+                            .arg(m_name));
+            m_loggedScriptsFileError = true;
+        }
     }
 
     QStringList scriptLabels = m_scriptLabels;

src/core/parseutil.cpp

@@ -1,6 +1,7 @@
 #include "log.h"
 #include "parseutil.h"
 #include "loadingscreen.h"
+#include "utility.h"
 
 #include <QRegularExpression>
 #include <QJsonDocument>
@@ -59,11 +60,7 @@ QString ParseUtil::createErrorMessage(const QString &message, const QString &exp
 void ParseUtil::updateSplashScreen(QString path) {
     if (!this->updatesSplashScreen)
         return;
-
-    if (path.startsWith(this->root)) {
-        path.remove(0, this->root.length());
-    }
-    porysplash->showLoadingMessage(path);
+    porysplash->showLoadingMessage(Util::stripPrefix(path, this->root));
 }
 
 QString ParseUtil::readTextFile(const QString &path, QString *error) {
@@ -101,8 +98,12 @@ QString ParseUtil::loadTextFile(const QString &path, QString *error) {
 bool ParseUtil::cacheFile(const QString &path, QString *error) {
     updateSplashScreen(path);
 
-    this->fileCache.insert(path, readTextFile(pathWithRoot(path), error));
-    return !error || error->isEmpty();
+    // We use an internal '_error' variable because we use the error output to track success,
+    // and we want to report success regardless of whether the caller provided 'error'.
+    QString _error;
+    this->fileCache.insert(path, readTextFile(pathWithRoot(path), &_error));
+    if (error) *error = _error;
+    return _error.isEmpty();
 }
 
 int ParseUtil::textFileLineCount(const QString &path) {
@@ -879,11 +880,11 @@ int ParseUtil::getPoryScriptLineNumber(QString text, const QString &scriptLabel)
     return 0;
 }
 
-QStringList ParseUtil::getGlobalScriptLabels(const QString &filePath) {
+QStringList ParseUtil::getGlobalScriptLabels(const QString &filePath, QString *error) {
     if (filePath.endsWith(".inc") || filePath.endsWith(".s"))
-        return getGlobalRawScriptLabels(readTextFile(filePath));
+        return getGlobalRawScriptLabels(readTextFile(filePath, error));
     else if (filePath.endsWith(".pory"))
-        return getGlobalPoryScriptLabels(readTextFile(filePath));
+        return getGlobalPoryScriptLabels(readTextFile(filePath, error));
     else
         return { };
 }

src/core/utility.cpp

@@ -48,6 +48,13 @@ QString Util::toHtmlParagraph(const QString &text) {
     return QString("<html><head/><body><p>%1</p></body></html>").arg(text);
 }
 
+QString Util::stripPrefix(const QString &s, const QString &prefix) {
+    if (!s.startsWith(prefix)) {
+        return s;
+    }
+    return QString(s).remove(0, prefix.length());
+}
+
 Qt::Orientations Util::getOrientation(bool xflip, bool yflip) {
     Qt::Orientations flags;
     if (xflip) flags |= Qt::Orientation::Horizontal;

src/project.cpp

@@ -232,6 +232,7 @@ bool Project::load() {
 
 void Project::resetFileCache() {
     this->parser.clearFileCache();
+    this->failedFileWatchPaths.clear();
 
     const QSet<QString> filepaths = {
         // Whenever we load a tileset we'll need to parse some data from these files, so we cache them to avoid the overhead of opening the files.
@@ -245,6 +246,7 @@ void Project::resetFileCache() {
         // We need separate sets of constants from these files
         projectConfig.getFilePath(ProjectFilePath::constants_map_types),
         projectConfig.getFilePath(ProjectFilePath::global_fieldmap),
+        projectConfig.getFilePath(ProjectFilePath::constants_weather),
     };
     for (const auto &path : filepaths) {
         if (this->parser.cacheFile(path)) {
@@ -365,9 +367,10 @@ QSet<QString> Project::getTopLevelMapFields() const {
 }
 
 QJsonDocument Project::readMapJson(const QString &mapName, QString *error) {
+    // Note: We are explicitly not adding mapFilepath to the fileWatcher here.
+    //       All map.json files are read at launch, and adding them all to the filewatcher
+    //       can easily exceed the 256 file limit that exists on some platforms.
     const QString mapFilepath = Map::getJsonFilepath(mapName);
-    watchFile(mapFilepath);
-
     QJsonDocument doc;
     if (!parser.tryParseJsonFile(&doc, mapFilepath, error)) {
         if (error) {
@@ -403,6 +406,7 @@ bool Project::loadMapData(Map* map) {
         logError(error);
         return false;
     }
+    watchFile(map->getJsonFilepath());
 
     QJsonObject mapObj = mapDoc.object();
 
@@ -741,17 +745,29 @@ bool Project::saveMapLayouts() {
     return true;
 }
 
-void Project::watchFile(const QString &filename) {
-    if (!filename.startsWith(this->root)) {
-        this->fileWatcher.addPath(QString("%1/%2").arg(this->root).arg(filename));
-    } else {
-        this->fileWatcher.addPath(filename);
+bool Project::watchFile(const QString &filename) {
+    QString filepath = filename.startsWith(this->root) ? filename : QString("%1/%2").arg(this->root).arg(filename);
+    if (!this->fileWatcher.addPath(filepath) && !this->fileWatcher.files().contains(filepath)) {
+        // We failed to watch the file, and this wasn't a file we were already watching.
+        // Log a warning, but only if A. we actually care that we failed, because 'monitor files' is enabled,
+        // B. we haven't logged a warning for this file yet, and C. we would have otherwise been able to watch it, because the file exists.
+        if (porymapConfig.monitorFiles && !this->failedFileWatchPaths.contains(filepath) && QFileInfo::exists(filepath)) {
+            this->failedFileWatchPaths.insert(filepath);
+            logWarn(QString("Failed to add '%1' to file watcher. Currently watching %2 files.")
+                            .arg(Util::stripPrefix(filepath, this->root))
+                            .arg(this->fileWatcher.files().length()));
+        }
+        return false;
     }
+    return true;
 }
 
-void Project::watchFiles(const QStringList &filenames) {
-    for (const auto &filename : filenames)
-        watchFile(filename);
+bool Project::watchFiles(const QStringList &filenames) {
+    bool success = true;
+    for (const auto &filename : filenames) {
+        if (!watchFile(filename)) success = false;
+    }
+    return success;
 }
 
 void Project::ignoreWatchedFileTemporarily(const QString &filepath) {
@@ -766,6 +782,14 @@ void Project::ignoreWatchedFilesTemporarily(const QStringList &filepaths) {
 }
 
 void Project::recordFileChange(const QString &filepath) {
+    // --From the Qt manual--
+    // Note: As a safety measure, many applications save an open file by writing a new file and then deleting the old one.
+    //       In your slot function, you can check watcher.files().contains(path).
+    //       If it returns false, check whether the file still exists and then call addPath() to continue watching it.
+    if (!this->fileWatcher.files().contains(filepath) && QFileInfo::exists(filepath)) {
+        this->fileWatcher.addPath(filepath);
+    }
+
     if (this->modifiedFiles.contains(filepath)) {
         // We already recorded a change to this file
         return;

src/ui/customscriptseditor.cpp

@@ -133,11 +133,9 @@ QString CustomScriptsEditor::getScriptFilepath(QListWidgetItem * item, bool abso
 
 void CustomScriptsEditor::setScriptFilepath(QListWidgetItem * item, QString filepath) const {
     auto widget = dynamic_cast<CustomScriptsListItem *>(ui->list->itemWidget(item));
-    if (!widget) return;
-
-    if (filepath.startsWith(this->baseDir))
-        filepath.remove(0, this->baseDir.length());
-    widget->ui->lineEdit_filepath->setText(filepath);
+    if (widget) {
+        widget->ui->lineEdit_filepath->setText(Util::stripPrefix(filepath, this->baseDir));
+    }
 }
 
 bool CustomScriptsEditor::getScriptEnabled(QListWidgetItem * item) const {
@@ -179,8 +177,7 @@ void CustomScriptsEditor::loadScript() {
 }
 
 void CustomScriptsEditor::displayNewScript(QString filepath) {
-    if (filepath.startsWith(this->baseDir))
-        filepath.remove(0, this->baseDir.length());
+    filepath = Util::stripPrefix(filepath, this->baseDir);
 
     // Verify new script path is not already in list
     for (int i = 0; i < ui->list->count(); i++) {

src/ui/newlocationdialog.cpp

@@ -30,9 +30,7 @@ void NewLocationDialog::onIdNameChanged(const QString &idName) {
     validateIdName(true);
 
     // Extract a presumed display name from the ID name
-    QString displayName = idName;
-    if (displayName.startsWith(namePrefix))
-        displayName.remove(0, namePrefix.length());
+    QString displayName = Util::stripPrefix(idName, namePrefix);
     displayName.replace("_", " ");
     ui->lineEdit_DisplayName->setText(displayName);
 }

src/ui/projectsettingseditor.cpp

@@ -765,9 +765,7 @@ QMap<QString,QString> ProjectSettingsEditor::getGlobalConstants() {
 
 // Display relative path if this file is in the project folder
 QString ProjectSettingsEditor::stripProjectDir(QString s) {
-    if (s.startsWith(this->baseDir))
-        s.remove(0, this->baseDir.length());
-    return s;
+    return Util::stripPrefix(s, this->baseDir);
 }
 
 void ProjectSettingsEditor::importDefaultPrefabsClicked(bool) {

src/ui/wildmonchart.cpp

@@ -121,9 +121,7 @@ void WildMonChart::readTable() {
         const QString groupName = this->tableIndexToGroupName.value(i);
 
         // Create species label (strip 'SPECIES_' prefix).
-        QString label = pokemon.species;
-        if (label.startsWith(speciesPrefix))
-            label.remove(0, speciesPrefix.length());
+        QString label = Util::stripPrefix(pokemon.species, speciesPrefix);
 
         // Add species/level frequency data
         Summary *summary = &this->speciesToGroupedData[label][groupName];