#6107: Apply workaround to mark the map as modified when the layer hi…

…erarchy has been changed
codereader · Oct 1, 2022 · 372a39c · 372a39c
1 parent af55fdf
commit 372a39c
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 5 deletions.
diff --git a/radiantcore/map/Map.cpp b/radiantcore/map/Map.cpp
@@ -86,10 +86,11 @@ void Map::clearMapResource()
     setMapName(_(MAP_UNNAMED_STRING));
 }
 
-void Map::connectToUndoSystem()
+void Map::connectToRootNode()
 {
     _modifiedStatusListener.disconnect();
     _undoEventListener.disconnect();
+    _layerHierarchyChangedListener.disconnect();
 
     _modifiedStatusListener = _resource->signal_modifiedStatusChanged().connect(
         [this](bool newStatus) { setModified(newStatus); }
@@ -98,8 +99,18 @@ void Map::connectToUndoSystem()
     if (!_resource->getRootNode()) return;
 
     _undoEventListener = _resource->getRootNode()->getUndoSystem().signal_undoEvent().connect(
-        sigc::mem_fun(this, &Map::onUndoEvent)
+        sigc::mem_fun(*this, &Map::onUndoEvent)
     );
+
+    // This is a workaround - changing layer hierarchies is not an undoable operation
+    // and this by hitting undo or redo the status might be reset to "unmodified" anytime
+    _layerHierarchyChangedListener = _resource->getRootNode()->getLayerManager()
+        .signal_layerHierarchyChanged().connect(sigc::mem_fun(*this, &Map::onLayerHierarchyChanged));
+}
+
+void Map::onLayerHierarchyChanged()
+{
+    setModified(true);
 }
 
 void Map::onUndoEvent(IUndoSystem::EventType type, const std::string& operationName)
@@ -166,7 +177,7 @@ void Map::loadMapResourceFromLocation(const MapLocation& location)
         clearMapResource();
     }
 
-    connectToUndoSystem();
+    connectToRootNode();
 
     // Take the new node and insert it as map root
     GlobalSceneGraph().setRoot(_resource->getRootNode());
@@ -855,7 +866,7 @@ bool Map::saveAs()
         return false;
     }
 
-    connectToUndoSystem();
+    connectToRootNode();
 
     // Resource save was successful, notify about this name change
     rename(fileInfo.fullPath);

diff --git a/radiantcore/map/Map.h b/radiantcore/map/Map.h
@@ -77,6 +77,7 @@ class Map :
 
     sigc::connection _modifiedStatusListener;
     sigc::connection _undoEventListener;
+    sigc::connection _layerHierarchyChangedListener;
 
     // Point trace for leak detection
     std::unique_ptr<PointFile> _pointTrace;
@@ -289,8 +290,9 @@ class Map :
 	void emitMapEvent(MapEvent ev);
 
 	void clearMapResource();
-	void connectToUndoSystem();
+	void connectToRootNode();
 
+    void onLayerHierarchyChanged();
     void onUndoEvent(IUndoSystem::EventType type, const std::string& operationName);
 
     void cleanupMergeOperation();

diff --git a/test/LayerManipulation.cpp b/test/LayerManipulation.cpp
@@ -736,4 +736,19 @@ TEST_F(LayerTest, RemovingFromLayerMarksMapAsModified)
     performMoveOrAddToLayerTest(LayerAction::RemoveFromLayer);
 }
 
+TEST_F(LayerTest, SettingParentLayerMarksMapAsModified)
+{
+    loadMap("general_purpose.mapx");
+
+    auto& layerManager = GlobalMapModule().getRoot()->getLayerManager();
+    auto secondLayerId = layerManager.getLayerID("Second Layer");
+    auto thirdLayerId = layerManager.getLayerID("Third Layer");
+
+    EXPECT_FALSE(GlobalMapModule().isModified());
+
+    layerManager.setParentLayer(thirdLayerId, secondLayerId);
+
+    EXPECT_TRUE(GlobalMapModule().isModified());
+}
+
 }