Skip to content

Commit

Permalink
Watch and reload all documents rather than just opened editors
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bjorn committed Jun 11, 2024
1 parent eb0dcd1 commit 839d9b3
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 75 deletions.
1 change: 0 additions & 1 deletion src/libtiled/imagecache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "imagecache.h"

#include "logginginterface.h"
#include "map.h"
#include "mapformat.h"
#include "minimaprenderer.h"

Expand Down
2 changes: 1 addition & 1 deletion src/tiled/abstractworldtool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion src/tiled/command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
22 changes: 6 additions & 16 deletions src/tiled/document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include "changeevents.h"
#include "containerhelpers.h"
#include "documentmanager.h"
#include "editableasset.h"
#include "logginginterface.h"
#include "object.h"
Expand All @@ -34,8 +35,6 @@

namespace Tiled {

QHash<QString, Document*> Document::sDocumentInstances;

Document::Document(DocumentType type, const QString &fileName,
QObject *parent)
: QObject(parent)
Expand All @@ -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);
Expand All @@ -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()
Expand All @@ -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);
}
Expand Down
17 changes: 4 additions & 13 deletions src/tiled/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -128,8 +128,6 @@ class Document : public QObject,

virtual void checkIssues() {}

static const QHash<QString, Document *> &documentInstances();

signals:
void changed(const ChangeEvent &change);
void saved();
Expand Down Expand Up @@ -186,17 +184,15 @@ class Document : public QObject,
bool mModified = false;
bool mChangedOnDisk = false;
bool mIgnoreBrokenLinks = false;

static QHash<QString, Document*> 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;
}
Expand Down Expand Up @@ -245,11 +241,6 @@ inline bool Document::isReadOnly() const
return mReadOnly;
}

inline const QHash<QString, Document *> &Document::documentInstances()
{
return sDocumentInstances;
}

using DocumentPtr = QSharedPointer<Document>;

} // namespace Tiled
113 changes: 75 additions & 38 deletions src/tiled/documentmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -871,23 +868,18 @@ 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<MapDocument*>(document.data())) {
for (const SharedTileset &tileset : mapDocument->map()->tilesets())
removeFromTilesetDocument(tileset, mapDocument);
} else if (auto tilesetDocument = qobject_cast<TilesetDocument*>(document.data())) {
if (tilesetDocument->mapDocuments().isEmpty()) {
mTilesetDocumentsModel->remove(tilesetDocument);
emit tilesetDocumentRemoved(tilesetDocument);
} else {
tilesetDocument->disconnect(this);
}
}

Expand All @@ -914,45 +906,63 @@ 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<MapDocument>()) {
if (auto mapDocument = qobject_cast<MapDocument*>(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<TilesetDocument*>(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));
return false;
}

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;
}
Expand Down Expand Up @@ -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<Document*>(sender());

// Update the tabs for all opened embedded tilesets
Document *document = static_cast<Document*>(sender());
if (MapDocument *mapDocument = qobject_cast<MapDocument*>(document)) {
for (const SharedTileset &tileset : mapDocument->map()->tilesets()) {
if (auto tilesetDocument = findTilesetDocument(tileset))
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

Expand Down Expand Up @@ -1270,6 +1275,38 @@ TilesetDocument *DocumentManager::openTilesetFile(const QString &path)
return i == -1 ? nullptr : qobject_cast<TilesetDocument*>(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];
Expand Down
Loading

0 comments on commit 839d9b3

Please sign in to comment.