From 839d9b34d02ba5eab52abb8cab057a1c67a469cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thorbj=C3=B8rn=20Lindeijer?= Date: Tue, 28 May 2024 17:45:25 +0200 Subject: [PATCH] Watch and reload all documents rather than just opened editors When a map is loaded as part of a world, or a tileset is loaded as part of a map, but not open in its own editor, previously its file was not watched for changes. Now these files are also watched. When a change is detected and no unsaved changes are present, the affected map or tileset will be reloaded. Closes #1785 --- src/libtiled/imagecache.cpp | 1 - src/tiled/abstractworldtool.cpp | 2 +- src/tiled/command.cpp | 2 +- src/tiled/document.cpp | 22 ++----- src/tiled/document.h | 17 ++--- src/tiled/documentmanager.cpp | 113 +++++++++++++++++++++----------- src/tiled/documentmanager.h | 6 ++ src/tiled/mainwindow.cpp | 3 +- src/tiled/mapitem.cpp | 2 +- src/tiled/tilesetdock.cpp | 3 +- 10 files changed, 96 insertions(+), 75 deletions(-) diff --git a/src/libtiled/imagecache.cpp b/src/libtiled/imagecache.cpp index 39babdd01b..3ab341bf50 100644 --- a/src/libtiled/imagecache.cpp +++ b/src/libtiled/imagecache.cpp @@ -29,7 +29,6 @@ #include "imagecache.h" #include "logginginterface.h" -#include "map.h" #include "mapformat.h" #include "minimaprenderer.h" diff --git a/src/tiled/abstractworldtool.cpp b/src/tiled/abstractworldtool.cpp index 410104b9f8..fa138bdf2e 100644 --- a/src/tiled/abstractworldtool.cpp +++ b/src/tiled/abstractworldtool.cpp @@ -214,7 +214,7 @@ void AbstractWorldTool::showContextMenu(QGraphicsSceneMouseEvent *event) this, [=] { addAnotherMapToWorld(insertPos); }); if (targetDocument != nullptr && targetDocument != currentDocument) { - const QString targetFilename = targetDocument->fileName(); + const QString &targetFilename = targetDocument->fileName(); menu.addAction(QIcon(QLatin1String(":images/24/world-map-remove-this.png")), tr("Remove \"%1\" from World \"%2\"") .arg(targetDocument->displayName(), diff --git a/src/tiled/command.cpp b/src/tiled/command.cpp index 04ada6e5e3..21a36cbb19 100644 --- a/src/tiled/command.cpp +++ b/src/tiled/command.cpp @@ -73,7 +73,7 @@ static QString replaceVariables(const QString &string, bool quoteValues = true) // Perform variable replacement if (Document *document = DocumentManager::instance()->currentDocument()) { - const QString fileName = document->fileName(); + const QString &fileName = document->fileName(); QFileInfo fileInfo(fileName); const QString mapPath = fileInfo.absolutePath(); const QString projectPath = QFileInfo(ProjectManager::instance()->project().fileName()).absolutePath(); diff --git a/src/tiled/document.cpp b/src/tiled/document.cpp index f1ffa07b44..0686e697cb 100644 --- a/src/tiled/document.cpp +++ b/src/tiled/document.cpp @@ -22,6 +22,7 @@ #include "changeevents.h" #include "containerhelpers.h" +#include "documentmanager.h" #include "editableasset.h" #include "logginginterface.h" #include "object.h" @@ -34,8 +35,6 @@ namespace Tiled { -QHash Document::sDocumentInstances; - Document::Document(DocumentType type, const QString &fileName, QObject *parent) : QObject(parent) @@ -48,8 +47,7 @@ Document::Document(DocumentType type, const QString &fileName, mCanonicalFilePath = fileInfo.canonicalFilePath(); mReadOnly = fileInfo.exists() && !fileInfo.isWritable(); - if (!mCanonicalFilePath.isEmpty()) - sDocumentInstances.insert(mCanonicalFilePath, this); + DocumentManager::instance()->registerDocument(this); connect(mUndoStack, &QUndoStack::indexChanged, this, &Document::updateIsModified); connect(mUndoStack, &QUndoStack::cleanChanged, this, &Document::updateIsModified); @@ -61,11 +59,8 @@ Document::~Document() if (mCurrentObjectDocument) mCurrentObjectDocument->disconnect(this); - if (!mCanonicalFilePath.isEmpty()) { - auto i = sDocumentInstances.find(mCanonicalFilePath); - if (i != sDocumentInstances.end() && *i == this) - sDocumentInstances.erase(i); - } + if (auto manager = DocumentManager::maybeInstance()) + manager->unregisterDocument(this); } EditableAsset *Document::editable() @@ -88,19 +83,14 @@ void Document::setFileName(const QString &fileName) QString oldFileName = mFileName; - if (!mCanonicalFilePath.isEmpty()) { - auto i = sDocumentInstances.find(mCanonicalFilePath); - if (i != sDocumentInstances.end() && *i == this) - sDocumentInstances.erase(i); - } + DocumentManager::instance()->unregisterDocument(this); const QFileInfo fileInfo { fileName }; mFileName = fileName; mCanonicalFilePath = fileInfo.canonicalFilePath(); setReadOnly(fileInfo.exists() && !fileInfo.isWritable()); - if (!mCanonicalFilePath.isEmpty()) - sDocumentInstances.insert(mCanonicalFilePath, this); + DocumentManager::instance()->registerDocument(this); emit fileNameChanged(fileName, oldFileName); } diff --git a/src/tiled/document.h b/src/tiled/document.h index 14fcac25e6..a99b8049ac 100644 --- a/src/tiled/document.h +++ b/src/tiled/document.h @@ -67,8 +67,8 @@ class Document : public QObject, DocumentType type() const { return mType; } - QString fileName() const; - QString canonicalFilePath() const; + const QString &fileName() const; + const QString &canonicalFilePath() const; /** * Returns the name with which to display this document. It is the file name @@ -128,8 +128,6 @@ class Document : public QObject, virtual void checkIssues() {} - static const QHash &documentInstances(); - signals: void changed(const ChangeEvent &change); void saved(); @@ -186,17 +184,15 @@ class Document : public QObject, bool mModified = false; bool mChangedOnDisk = false; bool mIgnoreBrokenLinks = false; - - static QHash sDocumentInstances; }; -inline QString Document::fileName() const +inline const QString &Document::fileName() const { return mFileName; } -inline QString Document::canonicalFilePath() const +inline const QString &Document::canonicalFilePath() const { return mCanonicalFilePath; } @@ -245,11 +241,6 @@ inline bool Document::isReadOnly() const return mReadOnly; } -inline const QHash &Document::documentInstances() -{ - return sDocumentInstances; -} - using DocumentPtr = QSharedPointer; } // namespace Tiled diff --git a/src/tiled/documentmanager.cpp b/src/tiled/documentmanager.cpp index a06cfaf383..17d1bf12a4 100644 --- a/src/tiled/documentmanager.cpp +++ b/src/tiled/documentmanager.cpp @@ -173,7 +173,7 @@ DocumentManager::DocumentManager(QObject *parent) JumpToObject::activated = [this] (const JumpToObject &jump) { if (auto mapDocument = openMapFile(jump.mapFile)) { if (auto object = mapDocument->map()->findObjectById(jump.objectId)) { - mapDocument->focusMapObjectRequested(object); + emit mapDocument->focusMapObjectRequested(object); mapDocument->setSelectedObjects({ object }); } } @@ -209,7 +209,7 @@ DocumentManager::DocumentManager(QObject *parent) break; case Object::MapObjectType: if (auto object = mapDocument->map()->findObjectById(select.id)) { - mapDocument->focusMapObjectRequested(object); + emit mapDocument->focusMapObjectRequested(object); mapDocument->setSelectedObjects({ object }); obj = object; } @@ -356,7 +356,7 @@ void DocumentManager::restoreState() } /** - * Returns the current map document, or 0 when there is none. + * Returns the current document, or nullptr when there is none. */ Document *DocumentManager::currentDocument() const { @@ -385,7 +385,7 @@ MapView *DocumentManager::viewForDocument(MapDocument *mapDocument) const } /** - * Searches for a document with the given \a fileName and returns its + * Searches for an open document with the given \a fileName and returns its * index. Returns -1 when the document isn't open. */ int DocumentManager::findDocument(const QString &fileName) const @@ -570,9 +570,6 @@ int DocumentManager::insertDocument(int index, const DocumentPtr &document) } } - if (!document->fileName().isEmpty()) - mFileSystemWatcher->addPath(document->fileName()); - if (Editor *editor = mEditorForType.value(document->type())) editor->addDocument(documentPtr); @@ -639,7 +636,7 @@ DocumentPtr DocumentManager::loadDocument(const QString &fileName, { // Try to find it in already loaded documents QString canonicalFilePath = QFileInfo(fileName).canonicalFilePath(); - if (Document *doc = Document::documentInstances().value(canonicalFilePath)) + if (Document *doc = mDocumentByFileName.value(canonicalFilePath)) return doc->sharedFromThis(); if (!fileFormat) { @@ -831,7 +828,7 @@ void DocumentManager::closeOtherDocuments(int index) for (int i = mTabBar->count() - 1; i >= 0; --i) { if (i != index) - documentCloseRequested(i); + emit documentCloseRequested(i); if (!mMultiDocumentClose) return; @@ -849,7 +846,7 @@ void DocumentManager::closeDocumentsToRight(int index) mMultiDocumentClose = true; for (int i = mTabBar->count() - 1; i > index; --i) { - documentCloseRequested(i); + emit documentCloseRequested(i); if (!mMultiDocumentClose) return; @@ -871,14 +868,11 @@ void DocumentManager::closeDocumentAt(int index) mDocuments.removeAt(index); mTabBar->removeTab(index); + document->disconnect(this); + if (Editor *editor = mEditorForType.value(document->type())) editor->removeDocument(document.data()); - if (!document->fileName().isEmpty()) { - mFileSystemWatcher->removePath(document->fileName()); - document->setChangedOnDisk(false); - } - if (auto mapDocument = qobject_cast(document.data())) { for (const SharedTileset &tileset : mapDocument->map()->tilesets()) removeFromTilesetDocument(tileset, mapDocument); @@ -886,8 +880,6 @@ void DocumentManager::closeDocumentAt(int index) if (tilesetDocument->mapDocuments().isEmpty()) { mTilesetDocumentsModel->remove(tilesetDocument); emit tilesetDocumentRemoved(tilesetDocument); - } else { - tilesetDocument->disconnect(this); } } @@ -914,32 +906,47 @@ bool DocumentManager::reloadCurrentDocument() * Reloads the document at the given \a index. Will not ask the user whether to * save any changes! * - * Returns whether the document loaded successfully. + * Returns whether the document reloaded successfully. */ bool DocumentManager::reloadDocumentAt(int index) { const auto document = mDocuments.at(index); + return reloadDocument(document.data()); +} + +/** + * Reloads the given \a document. + * + * The document may not actually be open in any editor. It might be a map that + * is loaded as part of a world, or a tileset that is loaded as part of a map. + * + * Returns whether the document reloaded successfully. + */ +bool DocumentManager::reloadDocument(Document *document) +{ QString error; - if (auto mapDocument = document.objectCast()) { + if (auto mapDocument = qobject_cast(document)) { if (!mapDocument->reload(&error)) { emit reloadError(tr("%1:\n\n%2").arg(document->fileName(), error)); return false; } - const bool isCurrent = index == mTabBar->currentIndex(); + const bool isCurrent = document == currentDocument(); if (isCurrent) { if (mBrokenLinksModel->hasBrokenLinks()) mBrokenLinksWidget->show(); } - checkTilesetColumns(mapDocument.data()); + // Only check tileset columns for open maps since for other maps we + // may not have TilesetDocument instances created for their tilesets. + if (findDocument(document) != -1) + checkTilesetColumns(mapDocument); } else if (auto tilesetDocument = qobject_cast(document)) { if (tilesetDocument->isEmbedded()) { // For embedded tilesets, we need to reload the map - index = findDocument(tilesetDocument->mapDocuments().first()); - if (!reloadDocumentAt(index)) + if (!reloadDocument(tilesetDocument->mapDocuments().first())) return false; } else if (!tilesetDocument->reload(&error)) { emit reloadError(tr("%1:\n\n%2").arg(document->fileName(), error)); @@ -947,12 +954,15 @@ bool DocumentManager::reloadDocumentAt(int index) } tilesetDocument->setChangedOnDisk(false); + } else { + // We don't support reloading other document types at the moment + return false; } if (!isDocumentChangedOnDisk(currentDocument())) mFileChangedWarning->setVisible(false); - emit documentReloaded(document.data()); + emit documentReloaded(document); return true; } @@ -987,16 +997,12 @@ void DocumentManager::currentIndexChanged() emit currentDocumentChanged(document); } -void DocumentManager::fileNameChanged(const QString &fileName, - const QString &oldFileName) +void DocumentManager::fileNameChanged(const QString &/* fileName */, + const QString &/* oldFileName */) { - if (!fileName.isEmpty()) - mFileSystemWatcher->addPath(fileName); - if (!oldFileName.isEmpty()) - mFileSystemWatcher->removePath(oldFileName); + Document *document = static_cast(sender()); // Update the tabs for all opened embedded tilesets - Document *document = static_cast(sender()); if (MapDocument *mapDocument = qobject_cast(document)) { for (const SharedTileset &tileset : mapDocument->map()->tilesets()) { if (auto tilesetDocument = findTilesetDocument(tileset)) @@ -1128,13 +1134,12 @@ void DocumentManager::filesChanged(const QStringList &fileNames) void DocumentManager::fileChanged(const QString &fileName) { - const int index = findDocument(fileName); - - // Most likely the file was removed - if (index == -1) + const auto document = mDocumentByFileName.value(fileName); + if (!document) { + qWarning() << "Document not found for changed file:" << fileName; return; + } - const auto &document = mDocuments.at(index); const QFileInfo fileInfo { fileName }; // Always update potentially changed read-only state @@ -1145,8 +1150,8 @@ void DocumentManager::fileChanged(const QString &fileName) return; // Automatically reload when there are no unsaved changes - if (!isDocumentModified(document.data())) { - reloadDocumentAt(index); + if (!isDocumentModified(document)) { + reloadDocument(document); return; } @@ -1270,6 +1275,38 @@ TilesetDocument *DocumentManager::openTilesetFile(const QString &path) return i == -1 ? nullptr : qobject_cast(mDocuments.at(i).data()); } +void DocumentManager::registerDocument(Document *document) +{ + const QString &canonicalPath = document->canonicalFilePath(); + if (canonicalPath.isEmpty()) + return; + + // Always add path because FileSystemWatcher handles duplicates + mFileSystemWatcher->addPath(canonicalPath); + + const auto i = mDocumentByFileName.constFind(canonicalPath); + if (i != mDocumentByFileName.constEnd()) { + qWarning() << "Document already registered:" << canonicalPath; + return; + } + + mDocumentByFileName.insert(canonicalPath, document); +} + +void DocumentManager::unregisterDocument(Document *document) +{ + const QString &canonicalPath = document->canonicalFilePath(); + if (canonicalPath.isEmpty()) + return; + + // Always remove path because FileSystemWatcher handles duplicates + mFileSystemWatcher->removePath(canonicalPath); + + const auto i = mDocumentByFileName.constFind(canonicalPath); + if (i != mDocumentByFileName.constEnd() && *i == document) + mDocumentByFileName.erase(i); +} + WorldDocument *DocumentManager::ensureWorldDocument(const QString &fileName) { auto document = mWorldDocuments[fileName]; diff --git a/src/tiled/documentmanager.h b/src/tiled/documentmanager.h index 04b9386c71..70318c1793 100644 --- a/src/tiled/documentmanager.h +++ b/src/tiled/documentmanager.h @@ -68,6 +68,7 @@ class DocumentManager : public QObject ~DocumentManager() override; friend class MainWindow; + friend class Document; // for file watching public: static DocumentManager *instance(); @@ -123,6 +124,7 @@ class DocumentManager : public QObject bool reloadCurrentDocument(); bool reloadDocumentAt(int index); + bool reloadDocument(Document *document); void checkTilesetColumns(MapDocument *mapDocument); bool checkTilesetColumns(TilesetDocument *tilesetDocument); @@ -225,6 +227,9 @@ public slots: MapDocument *openMapFile(const QString &path); TilesetDocument *openTilesetFile(const QString &path); + void registerDocument(Document *document); + void unregisterDocument(Document *document); + QIcon mLockedIcon; QVector mDocuments; @@ -245,6 +250,7 @@ public slots: QUndoGroup *mUndoGroup; FileSystemWatcher *mFileSystemWatcher; + QHash mDocumentByFileName; static DocumentManager *mInstance; diff --git a/src/tiled/mainwindow.cpp b/src/tiled/mainwindow.cpp index 483dc5c743..853c87a181 100644 --- a/src/tiled/mainwindow.cpp +++ b/src/tiled/mainwindow.cpp @@ -2323,9 +2323,8 @@ void MainWindow::retranslateUi() void MainWindow::exportMapAs(MapDocument *mapDocument) { SessionOption lastUsedExportFilter { "map.lastUsedExportFilter" }; - QString fileName = mapDocument->fileName(); QString selectedFilter = lastUsedExportFilter; - auto exportDetails = chooseExportDetails(fileName, + auto exportDetails = chooseExportDetails(mapDocument->fileName(), mapDocument->lastExportFileName(), selectedFilter, this); diff --git a/src/tiled/mapitem.cpp b/src/tiled/mapitem.cpp index 51486a84d0..d4a0be4040 100644 --- a/src/tiled/mapitem.cpp +++ b/src/tiled/mapitem.cpp @@ -458,7 +458,7 @@ void MapItem::mapChanged() updateBoundingRect(); // When this map is part of a world, update that map's rect when necessary - const QString mapFileName = mapDocument()->fileName(); + const QString &mapFileName = mapDocument()->fileName(); if (const World *world = WorldManager::instance().worldForMap(mapFileName)) { if (world->canBeModified()) { const QRect currentRectInWorld = world->mapRect(mapFileName); diff --git a/src/tiled/tilesetdock.cpp b/src/tiled/tilesetdock.cpp index 8ac0db0aa0..8ef46bb7ff 100644 --- a/src/tiled/tilesetdock.cpp +++ b/src/tiled/tilesetdock.cpp @@ -937,9 +937,8 @@ void TilesetDock::tabContextMenuRequested(const QPoint &pos) QMenu menu; auto tilesetDocument = mTilesetDocuments.at(index); - const QString fileName = tilesetDocument->fileName(); - Utils::addFileManagerActions(menu, fileName); + Utils::addFileManagerActions(menu, tilesetDocument->fileName()); menu.addSeparator(); menu.addAction(mEditTileset->icon(), mEditTileset->text(), this, [tileset = tilesetDocument->tileset()] {