Fixed JSON tilesets getting saved in TSX format

The problem was that when an external tileset was opened as part of a
map, there was no way for Tiled to derive the file format used by the
tileset. Upon saving, it would default to using the TSX format, which
was wrong if the tileset was loaded from a JSON file.

This is now fixed by remembering the file format at the Tileset level,
instead of in TilesetDocument (which libtiled doesn't know about).

The fallback to the TSX format has been removed, since actually it
should always be defined.

When reading an external tileset, there is still a fallback to TSX
format because external applications like tmxviewer and tmxrasterizer
depend on it.  It would be good to find some alternative for this
however, since this fallback is unable to set the correct format on the
loaded tileset.

This also fixes the Reload action being disabled for such tilesets and
even crashing Tiled when the file was changed on disk.

Finally, a small fix was made to the Broken Links handling when you try
to replace a tileset that couldn't be loaded with itself.

src/libtiled/tileset.cpp

@@ -28,8 +28,10 @@
  */
 
 #include "tileset.h"
-#include "tile.h"
+
 #include "terrain.h"
+#include "tile.h"
+#include "tilesetformat.h"
 
 #include <QBitmap>
 
@@ -62,6 +64,16 @@ Tileset::~Tileset()
     qDeleteAll(mTerrainTypes);
 }
 
+void Tileset::setFormat(TilesetFormat *format)
+{
+    mFormat = format;
+}
+
+TilesetFormat *Tileset::format() const
+{
+    return mFormat;
+}
+
 /**
  * Sets the tile size of this tileset. Affects how image is cut in loadImage.
  *
@@ -608,6 +620,7 @@ void Tileset::swap(Tileset &other)
     std::swap(mTerrainDistancesDirty, other.mTerrainDistancesDirty);
     std::swap(mLoaded, other.mLoaded);
     std::swap(mBackgroundColor, other.mBackgroundColor);
+    std::swap(mFormat, other.mFormat);
 
     // Don't swap mWeakPointer, since it's a reference to this.
 
@@ -640,6 +653,7 @@ SharedTileset Tileset::clone() const
     c->mTerrainDistancesDirty = mTerrainDistancesDirty;
     c->mLoaded = mLoaded;
     c->mBackgroundColor = mBackgroundColor;
+    c->mFormat = mFormat;
 
     QMapIterator<int, Tile*> tileIterator(mTiles);
     while (tileIterator.hasNext()) {

src/libtiled/tileset.h

@@ -34,18 +34,20 @@
 
 #include <QColor>
 #include <QList>
-#include <QVector>
+#include <QPixmap>
 #include <QPoint>
+#include <QPointer>
 #include <QSharedPointer>
 #include <QString>
-#include <QPixmap>
+#include <QVector>
 
 class QImage;
 
 namespace Tiled {
 
 class Tile;
 class Tileset;
+class TilesetFormat;
 class Terrain;
 
 typedef QSharedPointer<Tileset> SharedTileset;
@@ -111,6 +113,9 @@ class TILEDSHARED_EXPORT Tileset : public Object
     void setFileName(const QString &fileName);
     bool isExternal() const;
 
+    void setFormat(TilesetFormat *format);
+    TilesetFormat *format() const;
+
     int tileWidth() const;
     int tileHeight() const;
 
@@ -244,6 +249,7 @@ class TILEDSHARED_EXPORT Tileset : public Object
     bool mTerrainDistancesDirty;
     bool mLoaded;
     QColor mBackgroundColor;
+    QPointer<TilesetFormat> mFormat;
 
     QWeakPointer<Tileset> mWeakPointer;
 };

src/libtiled/tilesetformat.cpp

@@ -46,11 +46,13 @@ SharedTileset readTileset(const QString &fileName, QString *error)
                     *error = QString();
             }
 
+            if (tileset)
+                tileset->setFormat(format);
+
             return tileset;
         }
     }
 
-
     // Fall back to default reader (TSX format)
     MapReader reader;
     SharedTileset tileset = reader.readTileset(fileName);

src/tiled/brokenlinks.cpp

@@ -490,7 +490,7 @@ void BrokenLinksWidget::tryFixLink(const BrokenLink &link)
 
         // It could be, that we have already loaded this tileset.
         SharedTileset newTileset = TilesetManager::instance()->findTileset(fileName);
-        if (!newTileset) {
+        if (!newTileset || !newTileset->loaded()) {
             newTileset = Tiled::readTileset(fileName, &error);
 
             if (!newTileset) {

src/tiled/mainwindow.cpp

@@ -110,6 +110,9 @@ MainWindow::MainWindow(QWidget *parent, Qt::WindowFlags flags)
 {
     mUi->setupUi(this);
 
+    PluginManager::addObject(mTmxMapFormat);
+    PluginManager::addObject(mTsxTilesetFormat);
+
     ActionManager::registerAction(mUi->actionNewMap, "file.new_map");
     ActionManager::registerAction(mUi->actionNewTileset, "file.new_tileset");
 
@@ -121,9 +124,6 @@ MainWindow::MainWindow(QWidget *parent, Qt::WindowFlags flags)
 
     setCentralWidget(mDocumentManager->widget());
 
-    PluginManager::addObject(mTmxMapFormat);
-    PluginManager::addObject(mTsxTilesetFormat);
-
 #ifdef Q_OS_MAC
     MacSupport::addFullscreen(this);
 #endif
@@ -817,7 +817,7 @@ bool MainWindow::saveDocumentAs(Document *document)
     return saveDocument(document, fileName);
 }
 
-bool isEmbeddedTilesetDocument(Document *document)
+static bool isEmbeddedTilesetDocument(Document *document)
 {
     if (auto *tilesetDocument = qobject_cast<TilesetDocument*>(document))
         return tilesetDocument->isEmbedded();
@@ -1464,7 +1464,7 @@ void MainWindow::updateActions()
     mUi->actionExportAsImage->setEnabled(mapDocument);
     mUi->actionExport->setEnabled(mapDocument);
     mUi->actionExportAs->setEnabled(mapDocument);
-    mUi->actionReload->setEnabled(mapDocument || (tilesetDocument && tilesetDocument->readerFormat()));
+    mUi->actionReload->setEnabled(mapDocument || (tilesetDocument && tilesetDocument->canReload()));
     mUi->actionClose->setEnabled(document);
     mUi->actionCloseAll->setEnabled(document);
 

src/tiled/replacetileset.cpp

@@ -20,6 +20,7 @@
 
 #include "replacetileset.h"
 
+#include "map.h"
 #include "mapdocument.h"
 
 #include <QCoreApplication>
@@ -36,6 +37,7 @@ ReplaceTileset::ReplaceTileset(MapDocument *mapDocument,
     , mIndex(index)
     , mTileset(tileset)
 {
+    Q_ASSERT(mMapDocument->map()->tilesetAt(index) != tileset);
 }
 
 void ReplaceTileset::swap()

src/tiled/tilesetdocument.cpp

@@ -24,9 +24,9 @@
 #include "map.h"
 #include "terrain.h"
 #include "tile.h"
+#include "tilesetformat.h"
 #include "tilesetmanager.h"
 #include "tilesetterrainmodel.h"
-#include "tmxmapformat.h"
 
 #include <QCoreApplication>
 #include <QFileInfo>
@@ -94,11 +94,10 @@ TilesetDocument::~TilesetDocument()
 
 bool TilesetDocument::save(const QString &fileName, QString *error)
 {
-    TilesetFormat *tilesetFormat = mWriterFormat;
+    TilesetFormat *tilesetFormat = mTileset->format();
 
-    TsxTilesetFormat tsxTilesetFormat;
-    if (!tilesetFormat)
-        tilesetFormat = &tsxTilesetFormat;
+    if (!tilesetFormat || !(tilesetFormat->capabilities() & MapFormat::Write))
+        return false;
 
     // todo: workaround to avoid writing the tileset like an external tileset reference
     mTileset->setFileName(QString());
@@ -120,14 +119,17 @@ bool TilesetDocument::save(const QString &fileName, QString *error)
     return true;
 }
 
+bool TilesetDocument::canReload() const
+{
+    return !fileName().isEmpty() && mTileset->format();
+}
+
 bool TilesetDocument::reload(QString *error)
 {
-    auto format = mReaderFormat;
-    if (!format)
-        format = mWriterFormat;
+    if (!canReload())
+        return false;
 
-    // Either the file was saved or it was loaded, so now we must have a format
-    Q_ASSERT(format);
+    auto format = mTileset->format();
 
     SharedTileset tileset = format->read(fileName());
 
@@ -137,6 +139,8 @@ bool TilesetDocument::reload(QString *error)
         return false;
     }
 
+    tileset->setFormat(format);
+
     mUndoStack->push(new ReloadTileset(this, tileset));
     mUndoStack->setClean();
     mLastSaved = QFileInfo(fileName()).lastModified();
@@ -156,32 +160,19 @@ TilesetDocument *TilesetDocument::load(const QString &fileName,
         return nullptr;
     }
 
-    auto *document = new TilesetDocument(tileset, fileName);
-    document->setReaderFormat(format);
-    if (format->hasCapabilities(MapFormat::Write))
-        document->setWriterFormat(format);
+    tileset->setFormat(format);
 
-    return document;
-}
-
-TilesetFormat *TilesetDocument::readerFormat() const
-{
-    return mReaderFormat;
-}
-
-void TilesetDocument::setReaderFormat(TilesetFormat *format)
-{
-    mReaderFormat = format;
+    return new TilesetDocument(tileset, fileName);
 }
 
 FileFormat *TilesetDocument::writerFormat() const
 {
-    return mWriterFormat;
+    return mTileset->format();
 }
 
 void TilesetDocument::setWriterFormat(TilesetFormat *format)
 {
-    mWriterFormat = format;
+    mTileset->setFormat(format);
 }
 
 QString TilesetDocument::displayName() const

src/tiled/tilesetdocument.h

@@ -24,7 +24,6 @@
 #include "tileset.h"
 
 #include <QList>
-#include <QPointer>
 
 namespace Tiled {
 
@@ -48,6 +47,7 @@ class TilesetDocument : public Document
 
     bool save(const QString &fileName, QString *error = nullptr) override;
 
+    bool canReload() const;
     bool reload(QString *error);
 
     /**
@@ -58,9 +58,6 @@ class TilesetDocument : public Document
                                  TilesetFormat *format,
                                  QString *error = nullptr);
 
-    TilesetFormat *readerFormat() const;
-    void setReaderFormat(TilesetFormat *format);
-
     FileFormat *writerFormat() const override;
     void setWriterFormat(TilesetFormat *format);
 
@@ -149,9 +146,6 @@ private slots:
     SharedTileset mTileset;
     QList<MapDocument*> mMapDocuments;
 
-    QPointer<TilesetFormat> mReaderFormat;
-    QPointer<TilesetFormat> mWriterFormat;
-
     TilesetTerrainModel *mTerrainModel;
 
     QList<Tile*> mSelectedTiles;