mapeditor · bjorn · Sep 3, 2024 · Aug 28, 2024 · Aug 29, 2024 · Aug 29, 2024
diff --git a/NEWS.md b/NEWS.md
@@ -3,6 +3,7 @@
 * Scripting: Added `FileFormat.nameFilter`
 * Scripting: Added `MapEditor.currentBrushChanged` signal
 * Scripting: Added `tiled.cursor` to create mouse cursor values
+* Fixed crash when accessing a world through a symlink (#4042)
 
 ### Tiled 1.11.0 (27 June 2024)
 

diff --git a/src/libtiled/world.cpp b/src/libtiled/world.cpp
@@ -46,10 +46,7 @@ namespace Tiled {
 
 void World::setMapRect(int mapIndex, const QRect &rect)
 {
-    if (maps[mapIndex].rect != rect) {
-        maps[mapIndex].rect = rect;
-        hasUnsavedChanges = true;
-    }
+    maps[mapIndex].rect = rect;
 }
 
 void World::removeMap(int mapIndex)
@@ -271,7 +268,7 @@ std::unique_ptr<World> World::load(const QString &fileName,
     QDir dir = QFileInfo(fileName).dir();
     std::unique_ptr<World> world(new World);
 
-    world->fileName = QFileInfo(fileName).canonicalFilePath();
+    world->fileName = fileName;
 
     const QJsonArray maps = object.value(QLatin1String("maps")).toArray();
     for (const QJsonValue &value : maps) {
@@ -380,8 +377,6 @@ bool World::save(World &world, QString *errorString)
     file.write(doc.toJson());
     file.close();
 
-    world.hasUnsavedChanges = false;
-
     return true;
 }
 

diff --git a/src/libtiled/world.h b/src/libtiled/world.h
@@ -80,7 +80,6 @@ class TILEDSHARED_EXPORT World : public Object
     QVector<WorldMapEntry> maps;
     QVector<WorldPattern> patterns;
     bool onlyShowAdjacentMaps = false;
-    bool hasUnsavedChanges = false;
 
     int mapIndex(const QString &fileName) const;
     void setMapRect(int mapIndex, const QRect &rect);

diff --git a/src/tiled/abstractworldtool.cpp b/src/tiled/abstractworldtool.cpp
@@ -142,13 +142,13 @@ void AbstractWorldTool::languageChangedImpl()
 void AbstractWorldTool::updateEnabledState()
 {
     const bool hasWorlds = !WorldManager::instance().worlds().isEmpty();
-    const World *world = constWorld(mapDocument());
-    setEnabled(mapDocument() && hasWorlds && (world == nullptr || world->canBeModified()));
+    const auto worldDocument = worldForMap(mapDocument());
+    setEnabled(mapDocument() && hasWorlds && (!worldDocument || worldDocument->world()->canBeModified()));
 
     // update toolbar actions
-    mAddMapToWorldAction->setEnabled(hasWorlds && world == nullptr);
-    mRemoveMapFromWorldAction->setEnabled(world != nullptr);
-    mAddAnotherMapToWorldAction->setEnabled(world != nullptr);
+    mAddMapToWorldAction->setEnabled(hasWorlds && !worldDocument);
+    mRemoveMapFromWorldAction->setEnabled(worldDocument);
+    mAddAnotherMapToWorldAction->setEnabled(worldDocument);
 }
 
 MapDocument *AbstractWorldTool::mapAt(const QPointF &pos) const
@@ -170,8 +170,8 @@ bool AbstractWorldTool::mapCanBeMoved(MapDocument *mapDocument) const
 {
     if (!mapDocument)
         return false;
-    const World *world = constWorld(mapDocument);
-    return world != nullptr && world->canBeModified();
+    auto worldDocument = worldForMap(mapDocument);
+    return worldDocument && worldDocument->world()->canBeModified();
 }
 
 QRect AbstractWorldTool::mapRect(MapDocument *mapDocument) const
@@ -184,60 +184,55 @@ QRect AbstractWorldTool::mapRect(MapDocument *mapDocument) const
     return rect;
 }
 
-const World *AbstractWorldTool::constWorld(MapDocument *mapDocument) const
+WorldDocument *AbstractWorldTool::worldForMap(MapDocument *mapDocument) const
 {
     if (!mapDocument)
         return nullptr;
-    return WorldManager::instance().worldForMap(mapDocument->fileName());
+    return WorldManager::instance().worldForMap(mapDocument->fileName()).data();
 }
 
 /**
  * Shows the context menu for adding/removing maps from worlds.
  */
 void AbstractWorldTool::showContextMenu(QGraphicsSceneMouseEvent *event)
 {
-    MapDocument *currentDocument = mapDocument();
-    MapDocument *targetDocument = targetMap();
-    const World *currentWorld = constWorld(currentDocument);
-    const World *targetWorld = constWorld(targetDocument);
-
-    const auto screenPos = event->screenPos();
-
     QMenu menu;
-    if (currentWorld) {
+
+    if (auto currentWorldDocument = worldForMap(mapDocument())) {
         QPoint insertPos = event->scenePos().toPoint();
-        insertPos += mapRect(currentDocument).topLeft();
+        insertPos += mapRect(mapDocument()).topLeft();
 
         menu.addAction(QIcon(QLatin1String(":images/24/world-map-add-other.png")),
                        tr("Add a Map to World \"%2\"")
-                       .arg(currentWorld->displayName()),
+                       .arg(currentWorldDocument->displayName()),
                        this, [=] { addAnotherMapToWorld(insertPos); });
 
-        if (targetDocument != nullptr && targetDocument != currentDocument) {
+        MapDocument *targetDocument = targetMap();
+        if (targetDocument != nullptr && targetDocument != mapDocument()) {
             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(),
-                                targetWorld->displayName()),
-                           this, [=] { removeFromWorld(targetFilename); });
+                                currentWorldDocument->displayName()),
+                           this, [=] { removeFromWorld(currentWorldDocument, targetFilename); });
         }
     } else {
         populateAddToWorldMenu(menu);
     }
 
-    menu.exec(screenPos);
+    menu.exec(event->screenPos());
 }
 
 void AbstractWorldTool::populateAddToWorldMenu(QMenu &menu)
 {
-    for (const World *world : WorldManager::instance().worlds()) {
-        if (!world->canBeModified())
+    for (auto &worldDocument : WorldManager::instance().worlds()) {
+        if (!worldDocument->world()->canBeModified())
             continue;
 
         auto action = menu.addAction(tr("Add \"%1\" to World \"%2\"")
                                      .arg(mapDocument()->displayName(),
-                                          world->displayName()),
-                                     this, [=] { addToWorld(world); });
+                                          worldDocument->displayName()),
+                                     this, [=] { addToWorld(worldDocument.data()); });
         action->setEnabled(!mapDocument()->fileName().isEmpty());
     }
 }
@@ -254,8 +249,8 @@ void AbstractWorldTool::addAnotherMapToWorldAtCenter()
 void AbstractWorldTool::addAnotherMapToWorld(QPoint insertPos)
 {
     MapDocument *map = mapDocument();
-    const World *world = constWorld(map);
-    if (!world)
+    auto worldDocument = worldForMap(map);
+    if (!worldDocument)
         return;
 
     const QDir dir = QFileInfo(map->fileName()).dir();
@@ -268,8 +263,8 @@ void AbstractWorldTool::addAnotherMapToWorld(QPoint insertPos)
     if (fileName.isEmpty())
         return;
 
-    const World *constWorldForSelectedMap = WorldManager::instance().worldForMap(fileName);
-    if (constWorldForSelectedMap) {
+    // Open the map if it's already part of one of the loaded worlds
+    if (WorldManager::instance().worldForMap(fileName)) {
         DocumentManager::instance()->openFile(fileName);
         return;
     }
@@ -286,23 +281,27 @@ void AbstractWorldTool::addAnotherMapToWorld(QPoint insertPos)
 
     const QRect rect { snapPoint(insertPos, map), QSize(0, 0) };
 
-    undoStack()->push(new AddMapCommand(world->fileName, fileName, rect));
+    auto undoStack = worldDocument->undoStack();
+    undoStack->push(new AddMapCommand(worldDocument, fileName, rect));
 }
 
 void AbstractWorldTool::removeCurrentMapFromWorld()
 {
-    removeFromWorld(mapDocument()->fileName());
+    if (auto currentWorldDocument = worldForMap(mapDocument()))
+        removeFromWorld(currentWorldDocument, mapDocument()->fileName());
 }
 
-void AbstractWorldTool::removeFromWorld(const QString &mapFileName)
+void AbstractWorldTool::removeFromWorld(WorldDocument *worldDocument,
+                                        const QString &mapFileName)
 {
     if (mapFileName.isEmpty())
         return;
 
-    undoStack()->push(new RemoveMapCommand(mapFileName));
+    QUndoStack *undoStack = worldDocument->undoStack();
+    undoStack->push(new RemoveMapCommand(worldDocument, mapFileName));
 }
 
-void AbstractWorldTool::addToWorld(const World *world)
+void AbstractWorldTool::addToWorld(WorldDocument *worldDocument)
 {
     MapDocument *document = mapDocument();
     if (document->fileName().isEmpty())
@@ -311,21 +310,20 @@ void AbstractWorldTool::addToWorld(const World *world)
     QRect rect = document->renderer()->mapBoundingRect();
 
     // Position the map alongside the last map by default
+    const World *world = worldDocument->world();
     if (!world->maps.isEmpty()) {
         const QRect &lastWorldRect = world->maps.last().rect;
         rect.moveTo(lastWorldRect.right() + 1, lastWorldRect.top());
     }
 
-    QUndoStack *undoStack = DocumentManager::instance()->ensureWorldDocument(world->fileName)->undoStack();
-    undoStack->push(new AddMapCommand(world->fileName, document->fileName(), rect));
+    QUndoStack *undoStack = worldDocument->undoStack();
+    undoStack->push(new AddMapCommand(worldDocument, document->fileName(), rect));
 }
 
 QUndoStack *AbstractWorldTool::undoStack()
 {
-    const World *world = constWorld(mapDocument());
-    if (!world)
-        return nullptr;
-    return DocumentManager::instance()->ensureWorldDocument(world->fileName)->undoStack();
+    auto worldDocument = worldForMap(mapDocument());
+    return worldDocument ? worldDocument->undoStack() : nullptr;
 }
 
 void AbstractWorldTool::populateToolBar(QToolBar *toolBar)

diff --git a/src/tiled/abstractworldtool.h b/src/tiled/abstractworldtool.h
@@ -30,6 +30,7 @@ namespace Tiled {
 class World;
 
 class SelectionRectangle;
+class WorldDocument;
 
 /**
  * A convenient base class for tools that work on object layers. Implements
@@ -74,8 +75,8 @@ class AbstractWorldTool : public AbstractTool
     void addAnotherMapToWorldAtCenter();
     void addAnotherMapToWorld(QPoint insertPos);
     void removeCurrentMapFromWorld();
-    void removeFromWorld(const QString &mapFileName);
-    void addToWorld(const World *world);
+    void removeFromWorld(WorldDocument *worldDocument, const QString &mapFileName);
+    void addToWorld(WorldDocument *worldDocument);
 
     QPoint snapPoint(QPoint point, MapDocument *document) const;
 
@@ -85,7 +86,7 @@ class AbstractWorldTool : public AbstractTool
 
     bool mapCanBeMoved(MapDocument *mapDocument) const;
     QRect mapRect(MapDocument *mapDocument) const;
-    const World *constWorld(MapDocument *mapDocument) const;
+    WorldDocument *worldForMap(MapDocument *mapDocument) const;
 
     void showContextMenu(QGraphicsSceneMouseEvent *);
 

diff --git a/src/tiled/changeworld.cpp b/src/tiled/changeworld.cpp
@@ -29,74 +29,90 @@
 
 namespace Tiled {
 
-AddMapCommand::AddMapCommand(const QString &worldName, const QString &mapName, const QRect &rect)
-    : QUndoCommand(QCoreApplication::translate("Undo Commands", "Add Map to World"))
-    , mWorldName(worldName)
+AddRemoveMapCommand::AddRemoveMapCommand(WorldDocument *worldDocument,
+                                         const QString &mapName,
+                                         const QRect &rect,
+                                         QUndoCommand *parent)
+    : QUndoCommand(parent)
+    , mWorldDocument(worldDocument)
     , mMapName(mapName)
     , mRect(rect)
 {
 }
 
-void AddMapCommand::undo()
+void AddRemoveMapCommand::addMap()
 {
-    WorldManager::instance().removeMap(mMapName);
+    auto world = mWorldDocument->world();
+    world->addMap(mMapName, mRect);
+    emit mWorldDocument->worldChanged();
 }
 
-void AddMapCommand::redo()
+void AddRemoveMapCommand::removeMap()
 {
-    WorldManager::instance().addMap(mWorldName, mMapName, mRect);
+    auto world = mWorldDocument->world();
+
+    const int index = world->mapIndex(mMapName);
+    if (index < 0)
+        return;
+
+    world->removeMap(index);
+    emit mWorldDocument->worldChanged();
 }
 
 
-RemoveMapCommand::RemoveMapCommand(const QString &mapName)
-    : QUndoCommand(QCoreApplication::translate("Undo Commands", "Remove Map from World"))
-    , mMapName(mapName)
+AddMapCommand::AddMapCommand(WorldDocument *worldDocument,
+                             const QString &mapName,
+                             const QRect &rect)
+    : AddRemoveMapCommand(worldDocument, mapName, rect)
 {
-    const WorldManager &manager = WorldManager::instance();
-    const World *world = manager.worldForMap(mMapName);
-    mPreviousRect = world->mapRect(mMapName);
-    mWorldName = world->fileName;
+    setText(QCoreApplication::translate("Undo Commands", "Add Map to World"));
 }
 
-void RemoveMapCommand::undo()
+
+RemoveMapCommand::RemoveMapCommand(WorldDocument *worldDocument, const QString &mapName)
+    : AddRemoveMapCommand(worldDocument, mapName, worldDocument->world()->mapRect(mapName))
 {
-    WorldManager::instance().addMap(mWorldName, mMapName, mPreviousRect);
+    setText(QCoreApplication::translate("Undo Commands", "Remove Map from World"));
 }
 
 void RemoveMapCommand::redo()
 {
     // ensure we're switching to a different map in case the current map is removed
     DocumentManager *manager = DocumentManager::instance();
     if (manager->currentDocument() && manager->currentDocument()->fileName() == mMapName) {
-        const World *world = WorldManager::instance().worldForMap(mMapName);
-        for (const WorldMapEntry &entry : world->allMaps()) {
+        for (const WorldMapEntry &entry : mWorldDocument->world()->allMaps()) {
             if (entry.fileName != mMapName) {
                 manager->switchToDocument(entry.fileName);
                 break;
             }
         }
     }
-    WorldManager::instance().removeMap(mMapName);
+
+    removeMap();
 }
 
 
-SetMapRectCommand::SetMapRectCommand(const QString &mapName, QRect rect)
+SetMapRectCommand::SetMapRectCommand(WorldDocument *worldDocument,
+                                     const QString &mapName,
+                                     const QRect &rect)
     : QUndoCommand(QCoreApplication::translate("Undo Commands", "Move Map"))
+    , mWorldDocument(worldDocument)
     , mMapName(mapName)
     , mRect(rect)
+    , mPreviousRect(mWorldDocument->world()->mapRect(mMapName))
 {
-    const WorldManager &manager = WorldManager::instance();
-    mPreviousRect = manager.worldForMap(mMapName)->mapRect(mMapName);
 }
 
-void SetMapRectCommand::undo()
+void SetMapRectCommand::setMapRect(const QRect &rect)
 {
-    WorldManager::instance().setMapRect(mMapName, mPreviousRect);
-}
+    auto world = mWorldDocument->world();
 
-void SetMapRectCommand::redo()
-{
-    WorldManager::instance().setMapRect(mMapName, mRect);
+    int index = world->mapIndex(mMapName);
+    if (index < 0)
+        return;
+
+    world->setMapRect(index, rect);
+    emit mWorldDocument->worldChanged();
 }
 
 } // namespace Tiled