#5408: No IMapFileChangeTracker reference is needed anymore when acqu…

…iring an undo state saver.
codereader · Oct 25, 2021 · 1381730 · 1381730
1 parent d228c9c
commit 1381730
Show file tree

Hide file tree

Showing 32 changed files with 108 additions and 158 deletions.
diff --git a/include/iundo.h b/include/iundo.h
@@ -9,8 +9,6 @@
 #include <memory>
 #include <sigc++/signal.h>
 
-class IMapFileChangeTracker;
-
 /** 
  * greebo: An UndoMemento has to be allocated on the heap
  * and contains all the information that is needed to describe
@@ -60,9 +58,8 @@ class IUndoSystem
     using Ptr = std::shared_ptr<IUndoSystem>;
 
 	// Undoable objects need to call this to get hold of a StateSaver instance
-	// which will take care of exporting and saving the state. The passed map file change 
-    // tracker will be notified when the state is saved.
-    virtual IUndoStateSaver* getStateSaver(IUndoable& undoable, IMapFileChangeTracker& tracker) = 0;
+	// which will take care of exporting and saving the state.
+    virtual IUndoStateSaver* getStateSaver(IUndoable& undoable) = 0;
 	virtual void releaseStateSaver(IUndoable& undoable) = 0;
 
 	virtual std::size_t size() const = 0;

diff --git a/libs/ObservedUndoable.h b/libs/ObservedUndoable.h
@@ -1,7 +1,6 @@
 #pragma once
 
 #include "iundo.h"
-#include "imapfilechangetracker.h"
 #include <functional>
 #include "BasicUndoMemento.h"
 
@@ -17,41 +16,31 @@ class ObservedUndoable :
 	Copyable& _object;
 	ImportCallback _importCallback;
 	IUndoStateSaver* _undoStateSaver;
-    IMapFileChangeTracker* _changeTracker;
 
 	std::string _debugName;
 
 public:
 	ObservedUndoable<Copyable>(Copyable& object, const ImportCallback& importCallback) :
 		_object(object), 
 		_importCallback(importCallback), 
-        _undoStateSaver(nullptr),
-        _changeTracker(nullptr)
+        _undoStateSaver(nullptr)
 	{}
 
 	ObservedUndoable<Copyable>(Copyable& object, const ImportCallback& importCallback, const std::string& debugName) :
 		_object(object),
 		_importCallback(importCallback),
 		_undoStateSaver(nullptr),
-		_changeTracker(nullptr),
 		_debugName(debugName)
 	{}
 
-    IMapFileChangeTracker& getUndoChangeTracker()
-    {
-        return *_changeTracker;
-    }
-
-	void connectUndoSystem(IMapFileChangeTracker& changeTracker)
+	void connectUndoSystem()
 	{
-        _changeTracker = &changeTracker;
-		_undoStateSaver = GlobalUndoSystem().getStateSaver(*this, changeTracker);
+		_undoStateSaver = GlobalUndoSystem().getStateSaver(*this);
 	}
 
-    void disconnectUndoSystem(IMapFileChangeTracker& map)
+    void disconnectUndoSystem()
 	{
         _undoStateSaver = nullptr;
-        _changeTracker = nullptr;
 		GlobalUndoSystem().releaseStateSaver(*this);
 	}
 

diff --git a/libs/scene/BasicRootNode.h b/libs/scene/BasicRootNode.h
@@ -1,7 +1,6 @@
 #pragma once
 
 #include "imap.h"
-#include "imapfilechangetracker.h"
 #include "ilayer.h"
 #include "ientity.h"
 #include "iundo.h"

diff --git a/libs/scene/Node.cpp b/libs/scene/Node.cpp
@@ -283,14 +283,14 @@ void Node::onRemoveFromScene(IMapRootNode& root)
 	_instantiated = false;
 }
 
-void Node::connectUndoSystem(IMapFileChangeTracker& changeTracker)
+void Node::connectUndoSystem()
 {
-    _children.connectUndoSystem(changeTracker);
+    _children.connectUndoSystem();
 }
 
-void Node::disconnectUndoSystem(IMapFileChangeTracker& changeTracker)
+void Node::disconnectUndoSystem()
 {
-    _children.disconnectUndoSystem(changeTracker);
+    _children.disconnectUndoSystem();
 }
 
 TraversableNodeSet& Node::getTraversable() {

diff --git a/libs/scene/Node.h b/libs/scene/Node.h
@@ -204,8 +204,8 @@ class Node :
 
 	TraversableNodeSet& getTraversable();
 
-    virtual void connectUndoSystem(IMapFileChangeTracker& changeTracker);
-    virtual void disconnectUndoSystem(IMapFileChangeTracker& changeTracker);
+    virtual void connectUndoSystem();
+    virtual void disconnectUndoSystem();
 
 	// Clears the TraversableNodeSet
 	virtual void removeAllChildNodes();

diff --git a/libs/scene/SelectableNode.cpp b/libs/scene/SelectableNode.cpp
@@ -28,7 +28,7 @@ SelectableNode::~SelectableNode()
 
 void SelectableNode::onInsertIntoScene(IMapRootNode& root)
 {
-	connectUndoSystem(root.getUndoChangeTracker());
+	connectUndoSystem();
 
 	Node::onInsertIntoScene(root);
 
@@ -51,7 +51,7 @@ void SelectableNode::onRemoveFromScene(IMapRootNode& root)
 {
 	setSelected(false);
 
-	disconnectUndoSystem(root.getUndoChangeTracker());
+	disconnectUndoSystem();
 
 	// When a node is removed from the scene with a non-empty group assignment
 	// we do notify the SelectionGroup to remove ourselves, but we keep the ID list
@@ -233,19 +233,19 @@ void SelectableNode::onSelectionStatusChange(bool changeGroupStatus)
 	}
 }
 
-void SelectableNode::connectUndoSystem(IMapFileChangeTracker& changeTracker)
+void SelectableNode::connectUndoSystem()
 {
-	_undoStateSaver = GlobalUndoSystem().getStateSaver(*this, changeTracker);
+	_undoStateSaver = GlobalUndoSystem().getStateSaver(*this);
 
-	Node::connectUndoSystem(changeTracker);
+	Node::connectUndoSystem();
 }
 
-void SelectableNode::disconnectUndoSystem(IMapFileChangeTracker& changeTracker)
+void SelectableNode::disconnectUndoSystem()
 {
 	_undoStateSaver = nullptr;
 	GlobalUndoSystem().releaseStateSaver(*this);
 
-	Node::disconnectUndoSystem(changeTracker);
+	Node::disconnectUndoSystem();
 }
 
 void SelectableNode::undoSave()

diff --git a/libs/scene/SelectableNode.h b/libs/scene/SelectableNode.h
@@ -65,8 +65,8 @@ class SelectableNode :
      */
 	virtual void onSelectionStatusChange(bool changeGroupStatus);
 
-	virtual void connectUndoSystem(IMapFileChangeTracker& changeTracker) override;
-	virtual void disconnectUndoSystem(IMapFileChangeTracker& changeTracker) override;
+	virtual void connectUndoSystem() override;
+	virtual void disconnectUndoSystem() override;
 
 private:
 	void undoSave();

diff --git a/libs/scene/TraversableNodeSet.cpp b/libs/scene/TraversableNodeSet.cpp
@@ -4,7 +4,6 @@
 #include <algorithm>
 #include "LayerValidityCheckWalker.h"
 #include "BasicUndoMemento.h"
-#include "imapfilechangetracker.h"
 #include "Node.h"
 
 namespace scene
@@ -190,12 +189,12 @@ bool TraversableNodeSet::empty() const
 	return _children.empty();
 }
 
-void TraversableNodeSet::connectUndoSystem(IMapFileChangeTracker& changeTracker)
+void TraversableNodeSet::connectUndoSystem()
 {
-	_undoStateSaver = GlobalUndoSystem().getStateSaver(*this, changeTracker);
+	_undoStateSaver = GlobalUndoSystem().getStateSaver(*this);
 }
 
-void TraversableNodeSet::disconnectUndoSystem(IMapFileChangeTracker& changeTracker)
+void TraversableNodeSet::disconnectUndoSystem()
 {
     _undoStateSaver = nullptr;
 	GlobalUndoSystem().releaseStateSaver(*this);

diff --git a/libs/scene/TraversableNodeSet.h b/libs/scene/TraversableNodeSet.h
@@ -7,8 +7,6 @@
 #include <sigc++/connection.h>
 #include <sigc++/trackable.h>
 
-class IMapFileChangeTracker;
-
 namespace scene
 {
 
@@ -23,7 +21,7 @@ class Node;
  * as any child nodes are removed or inserted. When the user hits Undo, the UndoSystem sends back
  * the memento and asks the TraversableNodeSet to overwrite its current children with the saved state.
  */
-class TraversableNodeSet :
+class TraversableNodeSet final :
 	public IUndoable,
 	public util::Noncopyable,
 	public sigc::trackable
@@ -89,8 +87,8 @@ class TraversableNodeSet :
 	 */
 	bool empty() const;
 
-	void connectUndoSystem(IMapFileChangeTracker& changeTracker);
-    void disconnectUndoSystem(IMapFileChangeTracker& changeTracker);
+	void connectUndoSystem();
+    void disconnectUndoSystem();
 
 	// Undoable implementation
 	IUndoMementoPtr exportState() const;

diff --git a/radiantcore/brush/Brush.cpp b/radiantcore/brush/Brush.cpp
@@ -38,7 +38,6 @@ namespace {
 Brush::Brush(BrushNode& owner) :
     _owner(owner),
     _undoStateSaver(nullptr),
-    _mapFileChangeTracker(nullptr),
     _faceCentroidPoints(GL_POINTS),
     _uniqueVertexPoints(GL_POINTS),
     _uniqueEdgePoints(GL_POINTS),
@@ -54,7 +53,6 @@ Brush::Brush(BrushNode& owner) :
 Brush::Brush(BrushNode& owner, const Brush& other) :
     _owner(owner),
     _undoStateSaver(nullptr),
-    _mapFileChangeTracker(nullptr),
     _faceCentroidPoints(GL_POINTS),
     _uniqueVertexPoints(GL_POINTS),
     _uniqueEdgePoints(GL_POINTS),
@@ -144,27 +142,22 @@ void Brush::forEachVisibleFace(const std::function<void(Face&)>& functor) const
     }
 }
 
-void Brush::connectUndoSystem(IMapFileChangeTracker& changeTracker)
+void Brush::connectUndoSystem()
 {
     assert(_undoStateSaver == nullptr);
 
-    // Keep a reference around, we need it when faces are changing
-    _mapFileChangeTracker = &changeTracker;
+	_undoStateSaver = GlobalUndoSystem().getStateSaver(*this);
 
-	_undoStateSaver = GlobalUndoSystem().getStateSaver(*this, changeTracker);
-
-    // Notify each face that we have a tracker
-    forEachFace([&](Face& face) { face.connectUndoSystem(changeTracker); });
+    forEachFace([&](Face& face) { face.connectUndoSystem(); });
 }
 
-void Brush::disconnectUndoSystem(IMapFileChangeTracker& changeTracker)
+void Brush::disconnectUndoSystem()
 {
     assert(_undoStateSaver != nullptr);
 
     // Notify each face
-    forEachFace([&](Face& face) { face.disconnectUndoSystem(changeTracker); });
+    forEachFace([&](Face& face) { face.disconnectUndoSystem(); });
 
-    _mapFileChangeTracker = nullptr;
     _undoStateSaver = nullptr;
     GlobalUndoSystem().releaseStateSaver(*this);
 }
@@ -436,7 +429,7 @@ void Brush::push_back(Faces::value_type face) {
 
     if (_undoStateSaver)
     {
-        m_faces.back()->connectUndoSystem(*_mapFileChangeTracker);
+        m_faces.back()->connectUndoSystem();
     }
 
     for (Observers::iterator i = m_observers.begin(); i != m_observers.end(); ++i) {
@@ -449,7 +442,7 @@ void Brush::pop_back()
 {
     if (_undoStateSaver)
     {
-        m_faces.back()->disconnectUndoSystem(*_mapFileChangeTracker);
+        m_faces.back()->disconnectUndoSystem();
     }
 
     m_faces.pop_back();
@@ -463,7 +456,7 @@ void Brush::erase(std::size_t index)
 {
     if (_undoStateSaver)
     {
-        m_faces[index]->disconnectUndoSystem(*_mapFileChangeTracker);
+        m_faces[index]->disconnectUndoSystem();
     }
 
     m_faces.erase(m_faces.begin() + index);
@@ -506,7 +499,7 @@ void Brush::clear()
     undoSave();
     if (_undoStateSaver)
     {
-        forEachFace([&](Face& face) { face.disconnectUndoSystem(*_mapFileChangeTracker); });
+        forEachFace([&](Face& face) { face.disconnectUndoSystem(); });
     }
 
     m_faces.clear();

diff --git a/radiantcore/brush/Brush.h b/radiantcore/brush/Brush.h
@@ -94,7 +94,6 @@ class Brush :
 	typedef std::set<BrushObserver*> Observers;
 	Observers m_observers;
 	IUndoStateSaver* _undoStateSaver;
-	IMapFileChangeTracker* _mapFileChangeTracker;
 
 	// state
 	Faces m_faces;
@@ -177,8 +176,8 @@ class Brush :
     // but are forcedly visible due to the brush being selected)
     void forEachVisibleFace(const std::function<void(Face&)>& functor) const;
 
-	void connectUndoSystem(IMapFileChangeTracker& map);
-	void disconnectUndoSystem(IMapFileChangeTracker& map);
+	void connectUndoSystem();
+	void disconnectUndoSystem();
 
 	// Face observer callbacks
 	void onFacePlaneChanged();

diff --git a/radiantcore/brush/BrushNode.cpp b/radiantcore/brush/BrushNode.cpp
@@ -269,7 +269,7 @@ scene::INodePtr BrushNode::clone() const {
 
 void BrushNode::onInsertIntoScene(scene::IMapRootNode& root)
 {
-    m_brush.connectUndoSystem(root.getUndoChangeTracker());
+    m_brush.connectUndoSystem();
 	GlobalCounters().getCounter(counterBrushes).increment();
 
     // Update the origin information needed for transformations
@@ -289,7 +289,7 @@ void BrushNode::onRemoveFromScene(scene::IMapRootNode& root)
 	setSelectedComponents(false, selection::ComponentSelectionMode::Face);
 
 	GlobalCounters().getCounter(counterBrushes).decrement();
-    m_brush.disconnectUndoSystem(root.getUndoChangeTracker());
+    m_brush.disconnectUndoSystem();
 
 	SelectableNode::onRemoveFromScene(root);
 }

diff --git a/radiantcore/brush/Face.cpp b/radiantcore/brush/Face.cpp
@@ -146,16 +146,16 @@ void Face::realiseShader()
     _owner.onFaceShaderChanged();
 }
 
-void Face::connectUndoSystem(IMapFileChangeTracker& changeTracker)
+void Face::connectUndoSystem()
 {
     assert(!_undoStateSaver);
 
     _shader.setInUse(true);
 
-    _undoStateSaver = GlobalUndoSystem().getStateSaver(*this, changeTracker);
+    _undoStateSaver = GlobalUndoSystem().getStateSaver(*this);
 }
 
-void Face::disconnectUndoSystem(IMapFileChangeTracker& changeTracker)
+void Face::disconnectUndoSystem()
 {
     assert(_undoStateSaver);
     _undoStateSaver = nullptr;

diff --git a/radiantcore/brush/Face.h b/radiantcore/brush/Face.h
@@ -2,7 +2,6 @@
 
 #include "irender.h"
 #include "iundo.h"
-#include "imapfilechangetracker.h"
 #include "iselectiontest.h"
 #include <sigc++/connection.h>
 
@@ -87,8 +86,8 @@ class Face :
 	// greebo: Emits the updated normals to the Winding class.
 	void updateWinding();
 
-    void connectUndoSystem(IMapFileChangeTracker& changeTracker);
-    void disconnectUndoSystem(IMapFileChangeTracker& changeTracker);
+    void connectUndoSystem();
+    void disconnectUndoSystem();
 
 	void undoSave();