From 573358b7581a7d10e2fd15567ac6495c6e268489 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Wed, 18 Jun 2025 14:20:19 -0400 Subject: [PATCH] Condense file watcher warning, reduce resource usage --- include/project.h | 4 ++- include/ui/preferenceeditor.h | 1 + src/core/map.cpp | 24 +++++++------ src/mainwindow.cpp | 1 + src/project.cpp | 67 +++++++++++++++++++++++++++-------- src/ui/preferenceeditor.cpp | 23 +++++++++--- 6 files changed, 89 insertions(+), 31 deletions(-) diff --git a/include/project.h b/include/project.h index 5c519bc3..ca7d9bbe 100644 --- a/include/project.h +++ b/include/project.h @@ -61,7 +61,6 @@ public: QMap metatileBehaviorMap; QMap metatileBehaviorMapInverse; ParseUtil parser; - QFileSystemWatcher fileWatcher; QSet modifiedFiles; bool usingAsmTilesets; QSet disabledSettingsNames; @@ -263,6 +262,7 @@ public: static QString getMapGroupPrefix(); private: + QPointer fileWatcher; QMap modifiedFileTimestamps; QMap facingDirections; QHash speciesToIconPath; @@ -332,6 +332,8 @@ private: void ignoreWatchedFilesTemporarily(const QStringList &filepaths); void recordFileChange(const QString &filepath); void resetFileCache(); + void resetFileWatcher(); + void logFileWatchStatus(); bool saveMapLayouts(); bool saveMapGroups(); diff --git a/include/ui/preferenceeditor.h b/include/ui/preferenceeditor.h index 64647f0e..74181c37 100644 --- a/include/ui/preferenceeditor.h +++ b/include/ui/preferenceeditor.h @@ -24,6 +24,7 @@ signals: void preferencesSaved(); void themeChanged(const QString &theme); void scriptSettingsChanged(bool on); + void reloadProjectRequested(); private: Ui::PreferenceEditor *ui; diff --git a/src/core/map.cpp b/src/core/map.cpp index 54d48c7b..6110a26f 100644 --- a/src/core/map.cpp +++ b/src/core/map.cpp @@ -163,17 +163,19 @@ QStringList Map::getScriptLabels(Event::Group group) { m_loggedScriptsFileError = true; } - if (!m_scriptFileWatcher) { - // Only create the file watcher when it's first needed (even an empty QFileSystemWatcher will consume system resources). - // The other option would be for Porymap to have a single global QFileSystemWatcher, but that has complications of its own. - m_scriptFileWatcher = new QFileSystemWatcher(this); - connect(m_scriptFileWatcher, &QFileSystemWatcher::fileChanged, this, &Map::invalidateScripts); - } - if (!m_scriptFileWatcher->files().contains(scriptsFilepath) && !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; + if (porymapConfig.monitorFiles) { + if (!m_scriptFileWatcher) { + // Only create the file watcher when it's first needed (even an empty QFileSystemWatcher will consume system resources). + // The other option would be for Porymap to have a single global QFileSystemWatcher, but that has complications of its own. + m_scriptFileWatcher = new QFileSystemWatcher(this); + connect(m_scriptFileWatcher, &QFileSystemWatcher::fileChanged, this, &Map::invalidateScripts); + } + if (!m_scriptFileWatcher->files().contains(scriptsFilepath) && !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; + } } m_scriptsLoaded = true; diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 8259c315..6fa4d4cc 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -2938,6 +2938,7 @@ void MainWindow::on_actionPreferences_triggered() { // require us to repopulate the EventFrames and redraw event pixmaps, respectively. connect(preferenceEditor, &PreferenceEditor::preferencesSaved, editor, &Editor::updateEvents); connect(preferenceEditor, &PreferenceEditor::scriptSettingsChanged, editor->project, &Project::readEventScriptLabels); + connect(preferenceEditor, &PreferenceEditor::reloadProjectRequested, this, &MainWindow::on_action_Reload_Project_triggered); } openSubWindow(preferenceEditor); diff --git a/src/project.cpp b/src/project.cpp index 1fab6300..99ad5583 100644 --- a/src/project.cpp +++ b/src/project.cpp @@ -32,9 +32,7 @@ int Project::num_pals_total = 13; Project::Project(QObject *parent) : QObject(parent) -{ - QObject::connect(&this->fileWatcher, &QFileSystemWatcher::fileChanged, this, &Project::recordFileChange); -} +{ } Project::~Project() { @@ -186,6 +184,7 @@ int Project::getSupportedMajorVersion(QString *errorOut) { bool Project::load() { this->parser.setUpdatesSplashScreen(true); + resetFileWatcher(); resetFileCache(); this->disabledSettingsNames.clear(); bool success = readGlobalConstants() @@ -225,6 +224,7 @@ bool Project::load() { initNewLayoutSettings(); initNewMapSettings(); applyParsedLimits(); + logFileWatchStatus(); } this->parser.setUpdatesSplashScreen(false); return success; @@ -232,7 +232,6 @@ bool Project::load() { void Project::resetFileCache() { this->parser.clearFileCache(); - this->failedFileWatchPaths.clear(); const QSet 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. @@ -749,16 +748,21 @@ bool Project::saveMapLayouts() { } bool Project::watchFile(const QString &filename) { + if (!porymapConfig.monitorFiles) + return true; + + if (!this->fileWatcher) { + // Only create the file watcher when it's first needed (even an empty QFileSystemWatcher will consume system resources). + this->fileWatcher = new QFileSystemWatcher(this); + QObject::connect(this->fileWatcher, &QFileSystemWatcher::fileChanged, this, &Project::recordFileChange); + } + 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)) { + 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)) { + // Record the filepath for logging later, assuming we should have been able to watch the file. + if (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; } @@ -774,8 +778,11 @@ bool Project::watchFiles(const QStringList &filenames) { } bool Project::stopFileWatch(const QString &filename) { + if (!this->fileWatcher) + return true; + QString filepath = filename.startsWith(this->root) ? filename : QString("%1/%2").arg(this->root).arg(filename); - return this->fileWatcher.removePath(filepath); + return this->fileWatcher->removePath(filepath); } void Project::ignoreWatchedFileTemporarily(const QString &filepath) { @@ -794,8 +801,8 @@ void Project::recordFileChange(const QString &filepath) { // 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->fileWatcher && !this->fileWatcher->files().contains(filepath) && QFileInfo::exists(filepath)) { + this->fileWatcher->addPath(filepath); } if (this->modifiedFiles.contains(filepath)) { @@ -815,6 +822,38 @@ void Project::recordFileChange(const QString &filepath) { emit fileChanged(filepath); } +// When calling 'watchFile' we record failures rather than log them immediately. +// We do this primarily to condense the warning if we fail to monitor any files. +void Project::logFileWatchStatus() { + if (!this->fileWatcher) + return; + + int numSuccessful = this->fileWatcher->files().length(); + int numAttempted = numSuccessful + this->failedFileWatchPaths.count(); + if (numAttempted == 0) + return; + + if (numSuccessful == 0) { + // We failed to watch every file we tried. As of writing this happens if Porymap is running + // on Windows and the project files are in WSL2. Rather than filling the log by + // outputting a warning for every file, just log that we failed to monitor any of them. + logWarn(QString("Failed to monitor project files")); + return; + } else { + logInfo(QString("Successfully monitoring %1/%2 project files").arg(numSuccessful).arg(numAttempted)); + } + + for (const auto &failedPath : this->failedFileWatchPaths) { + logWarn(QString("Failed to monitor project file '%1'").arg(failedPath)); + } +} + +void Project::resetFileWatcher() { + this->failedFileWatchPaths.clear(); + delete this->fileWatcher; + this->fileWatcher = nullptr; +} + bool Project::saveMapGroups() { QString mapGroupsFilepath = QString("%1/%2").arg(root).arg(projectConfig.getFilePath(ProjectFilePath::json_map_groups)); QFile mapGroupsFile(mapGroupsFilepath); diff --git a/src/ui/preferenceeditor.cpp b/src/ui/preferenceeditor.cpp index bda0ec98..4f4b0929 100644 --- a/src/ui/preferenceeditor.cpp +++ b/src/ui/preferenceeditor.cpp @@ -2,6 +2,7 @@ #include "ui_preferenceeditor.h" #include "config.h" #include "noscrollcombobox.h" +#include "message.h" #include #include @@ -87,6 +88,8 @@ void PreferenceEditor::updateFields() { } void PreferenceEditor::saveFields() { + bool needsProjectReload = false; + bool changedTheme = false; if (themeSelector->currentText() != porymapConfig.theme) { porymapConfig.theme = themeSelector->currentText(); @@ -100,7 +103,6 @@ void PreferenceEditor::saveFields() { porymapConfig.eventSelectionShapeMode = ui->radioButton_OnSprite->isChecked() ? QGraphicsPixmapItem::MaskShape : QGraphicsPixmapItem::BoundingRectShape; porymapConfig.textEditorOpenFolder = ui->lineEdit_TextEditorOpenFolder->text(); porymapConfig.textEditorGotoLine = ui->lineEdit_TextEditorGotoLine->text(); - porymapConfig.monitorFiles = ui->checkBox_MonitorProjectFiles->isChecked(); porymapConfig.reopenOnLaunch = ui->checkBox_OpenRecentProject->isChecked(); porymapConfig.checkForUpdates = ui->checkBox_CheckForUpdates->isChecked(); porymapConfig.eventDeleteWarningDisabled = ui->checkBox_DisableEventWarning->isChecked(); @@ -110,6 +112,11 @@ void PreferenceEditor::saveFields() { if (ui->checkBox_StatusWarnings->isChecked()) porymapConfig.statusBarLogTypes.insert(LogType::LOG_WARN); if (ui->checkBox_StatusInformation->isChecked()) porymapConfig.statusBarLogTypes.insert(LogType::LOG_INFO); + if (porymapConfig.monitorFiles != ui->checkBox_MonitorProjectFiles->isChecked()) { + porymapConfig.monitorFiles = ui->checkBox_MonitorProjectFiles->isChecked(); + needsProjectReload = true; + } + if (porymapConfig.applicationFont != this->applicationFont) { porymapConfig.applicationFont = this->applicationFont; changedTheme = true; @@ -119,13 +126,19 @@ void PreferenceEditor::saveFields() { changedTheme = true; } - porymapConfig.save(); - - emit preferencesSaved(); - if (changedTheme) { emit themeChanged(porymapConfig.theme); } + + porymapConfig.save(); + emit preferencesSaved(); + + if (needsProjectReload) { + auto message = QStringLiteral("Some changes will only take effect after reloading the project. Reload the project now?"); + if (QuestionMessage::show(message, this) == QMessageBox::Yes) { + emit reloadProjectRequested(); + } + } } void PreferenceEditor::dialogButtonClicked(QAbstractButton *button) {