#5408: Remove the old pointer-to-member-based state machine in UndoFi…

…leChangeTracker. Remove the corresponding methods and the calls to it from UndoSystem.
codereader · Oct 25, 2021 · dc44cc0 · dc44cc0
1 parent 2144bcb
commit dc44cc0
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 70 deletions.
diff --git a/include/iundo.h b/include/iundo.h
@@ -95,9 +95,6 @@ class IUndoSystem
         virtual ~Tracker() {}
 
         virtual void clear() = 0;
-        virtual void begin() = 0;
-        virtual void undo() = 0;
-        virtual void redo() = 0;
 
         // Invoked when a non-empty operation has been recorded by the undo system
         virtual void onOperationRecorded() = 0;

diff --git a/include/mapfile.h b/include/mapfile.h
@@ -3,14 +3,21 @@
 #include "inode.h"
 #include <functional>
 
+/**
+ * The file change tracker monitors the changes made to a single map with the help
+ * of the root node's undo system. It fires the given callback function whenever 
+ * the change count is increased or decreased (this is happening on change, undo and redo).
+ * It also provides methods for the client code to check whether the current point 
+ * in the map's undo history corresponds to a saved state or not - this allows to keep 
+ * the map's modified flag up to date.
+ */
 class IMapFileChangeTracker
 {
 public:
     virtual ~IMapFileChangeTracker() {}
 
     virtual void save() = 0;
     virtual bool saved() const = 0;
-    virtual void changed() = 0;
     virtual void setChangedCallback(const std::function<void()>& changed) = 0;
     virtual std::size_t changes() const = 0;
 };
diff --git a/libs/UndoFileChangeTracker.h b/libs/UndoFileChangeTracker.h
@@ -2,7 +2,6 @@
 
 #include "iundo.h"
 #include "mapfile.h"
-#include "itextstream.h"
 #include <limits>
 #include <functional>
 
@@ -11,27 +10,18 @@ class UndoFileChangeTracker :
     public IMapFileChangeTracker
 {
 private:
-    const std::size_t MAPFILE_MAX_CHANGES;
+    constexpr static std::size_t MAPFILE_MAX_CHANGES = std::numeric_limits<std::size_t>::max();
 
 	std::size_t _size;
 	std::size_t _saved;
-	typedef void (UndoFileChangeTracker::*Pending)();
-	Pending _pending;
 	std::function<void()> _changed;
 
 public:
 	UndoFileChangeTracker() :
-        MAPFILE_MAX_CHANGES(std::numeric_limits<std::size_t>::max()),
 		_size(0),
-		_saved(MAPFILE_MAX_CHANGES),
-		_pending(0)
+		_saved(MAPFILE_MAX_CHANGES)
 	{}
 
-	void print()
-    {
-		rMessage() << "saved: " << _saved << " size: " << _size << std::endl;
-	}
-
 	void push() 
     {
         ++_size;
@@ -50,8 +40,10 @@ class UndoFileChangeTracker :
         }
 	}
 
-	void pushOperation() {
-		if (_size < _saved) {
+	void pushOperation()
+    {
+		if (_size < _saved)
+        {
 			// redo queue has been flushed.. it is now impossible to get back to the saved state via undo/redo
 			_saved = MAPFILE_MAX_CHANGES;
 		}
@@ -68,28 +60,6 @@ class UndoFileChangeTracker :
         }
 	}
 
-	void begin() override 
-    {
-		//_pending = Pending(&UndoFileChangeTracker::pushOperation);
-	}
-
-	void undo() override 
-    {
-		//_pending = Pending(&UndoFileChangeTracker::pop);
-	}
-
-	void redo() override 
-    {
-		//_pending = Pending(&UndoFileChangeTracker::push);
-	}
-
-    void changed() override {
-		if (_pending != 0) {
-			((*this).*_pending)();
-			_pending = 0;
-		}
-	}
-
     void save() override 
     {
 		_saved = _size;

diff --git a/radiantcore/undo/StackFiller.h b/radiantcore/undo/StackFiller.h
@@ -40,12 +40,13 @@ class UndoStackFiller :
 	{
         if (_stack != nullptr)
 		{
+#if 0
             // Optionally notify the change tracker
             if (_tracker != nullptr)
             {
                 _tracker->changed();
             }
-
+#endif
             // Export the Undoable's memento
 			_stack->save(undoable);
 

diff --git a/radiantcore/undo/UndoSystem.cpp b/radiantcore/undo/UndoSystem.cpp
@@ -52,7 +52,6 @@ void UndoSystem::start()
 		_undoStack.pop_front();
 	}
 	startUndo();
-	trackersBegin();
 }
 
 bool UndoSystem::operationStarted() const
@@ -97,7 +96,6 @@ void UndoSystem::undo()
 	rMessage() << "Undo: " << operation->getName() << std::endl;
 
 	startRedo();
-	trackersUndo();
 	operation->restoreSnapshot();
 	finishRedo(operation->getName());
 	_undoStack.pop_back();
@@ -133,7 +131,6 @@ void UndoSystem::redo()
 	rMessage() << "Redo: " << operation->getName() << std::endl;
 
 	startUndo();
-	trackersRedo();
 	operation->restoreSnapshot();
 	finishUndo(operation->getName());
 	_redoStack.pop_back();
@@ -156,7 +153,7 @@ void UndoSystem::clear()
 	setActiveUndoStack(nullptr);
 	_undoStack.clear();
 	_redoStack.clear();
-	trackersClear();
+    foreachTracker([&](Tracker& tracker) { tracker.clear(); });
 
 	// greebo: This is called on map shutdown, so don't clear the observers,
 	// there are some "persistent" observers like EntityInspector and ShaderClipboard
@@ -230,24 +227,4 @@ void UndoSystem::foreachTracker(const std::function<void(Tracker&)>& functor) co
 	});
 }
 
-void UndoSystem::trackersClear() const
-{
-	foreachTracker([&] (Tracker& tracker) { tracker.clear(); });
-}
-
-void UndoSystem::trackersBegin() const
-{
-	foreachTracker([&] (Tracker& tracker) { tracker.begin(); });
-}
-
-void UndoSystem::trackersUndo() const
-{
-	foreachTracker([&] (Tracker& tracker) { tracker.undo(); });
-}
-
-void UndoSystem::trackersRedo() const
-{
-	foreachTracker([&] (Tracker& tracker) { tracker.redo(); });
-}
-
 } // namespace undo
diff --git a/radiantcore/undo/UndoSystem.h b/radiantcore/undo/UndoSystem.h
@@ -99,11 +99,6 @@ class UndoSystem final :
 	void setActiveUndoStack(UndoStack* stack);
 
 	void foreachTracker(const std::function<void(Tracker&)>& functor) const;
-
-	void trackersClear() const;
-	void trackersBegin() const;
-	void trackersUndo() const;
-	void trackersRedo() const;
 };
 
 }