Fix #4861: If an Undoable object connects to the UndoSystem in the mi…

…ddle of an operation (stack != nullptr) then assign the stack to the UndoStackFiller on the fly.

libs/ObservedUndoable.h

@@ -59,25 +59,19 @@ class ObservedUndoable :
 	{
 		if (_undoStateSaver != nullptr)
 		{
-			//rMessage() << "Saving undoable's state of observed object '" << _debugName << "'\n";
-
 			_undoStateSaver->save(*this);
 		}
 	}
 
 	IUndoMementoPtr exportState() const
 	{
-		//rMessage() << "Exporting state of observed object '" << _debugName << "'\n";
-
 		return IUndoMementoPtr(new BasicUndoMemento<Copyable>(_object));
 	}
 
 	void importState(const IUndoMementoPtr& state)
 	{
 		save();
 
-		//rMessage() << "Importing state of observed object '" << _debugName << "'\n";
-
 		_importCallback(std::static_pointer_cast<BasicUndoMemento<Copyable> >(state)->data());
 	}
 };

plugins/entity/Doom3Entity.cpp

@@ -48,8 +48,6 @@ bool Doom3Entity::isOfType(const std::string& className)
 
 void Doom3Entity::importState(const KeyValues& keyValues)
 {
-	//rMessage() << "Importing state into entity " << getKeyValue("name") << ", got " << keyValues.size() << " key values\n";
-
 	// Remove the entity key values, one by one
 	while (_keyValues.size() > 0)
 	{
@@ -110,7 +108,6 @@ void Doom3Entity::connectUndoSystem(IMapFileChangeTracker& changeTracker)
 
 	for (const auto& keyValue : _keyValues)
 	{
-		//rMessage() << "Connecting keyValue: " << keyValue.first << " to Undo System\n";
 		keyValue.second->connectUndoSystem(changeTracker);
 	}
 
@@ -123,7 +120,6 @@ void Doom3Entity::disconnectUndoSystem(IMapFileChangeTracker& changeTracker)
 
 	for (const auto& keyValue : _keyValues)
 	{
-		//rMessage() << "Disconnecting keyValue: " << keyValue.first << " from Undo System\n";
 		keyValue.second->disconnectUndoSystem(changeTracker);
 	}
 
@@ -291,7 +287,6 @@ void Doom3Entity::insert(const std::string& key, const KeyValuePtr& keyValue)
 
 	if (_instanced)
 	{
-		//rMessage() << "Connecting new key value to undo system " << key << "\n";
 		i->second->connectUndoSystem(_undo.getUndoChangeTracker());
 	}
 }
@@ -303,8 +298,6 @@ void Doom3Entity::insert(const std::string& key, const std::string& value)
 
 	if (i != _keyValues.end())
     {
-		//rMessage() << "Storing value over existing key " << key << ": " << value << "\n";
-
 		// Key has been found
 		i->second->assign(value);
 
@@ -314,8 +307,6 @@ void Doom3Entity::insert(const std::string& key, const std::string& value)
 	}
 	else
 	{
-		//rMessage() << "Setting value of new key " << key << ": " << value << "\n";
-
 		// No key with that name found, create a new one
 		_undo.save();
 
@@ -331,8 +322,6 @@ void Doom3Entity::erase(const KeyValues::iterator& i)
 {
 	if (_instanced)
 	{
-		//rMessage() << "Disconnecting key value from undo system " << i->first << "\n";
-
 		i->second->disconnectUndoSystem(_undo.getUndoChangeTracker());
 	}
 

plugins/entity/KeyValue.cpp

@@ -72,8 +72,6 @@ void KeyValue::notify()
 
 void KeyValue::importState(const std::string& string) 
 {
-	//rMessage() << "Importing value from undo system " << string << "\n";
-
 	// Add ourselves to the Undo event observers, to get notified after all this has been finished
 	_undoHandler = GlobalUndoSystem().signal_postUndo().connect(
 		sigc::mem_fun(this, &KeyValue::onUndoRedoOperationFinished));

radiant/undo/UndoSystem.cpp

@@ -25,7 +25,8 @@ namespace
 
 // Constructor
 UndoSystem::UndoSystem() :
-	_undoLevels(64)
+	_undoLevels(64),
+	_activeUndoStack(nullptr)
 {}
 
 UndoSystem::~UndoSystem()
@@ -41,6 +42,13 @@ void UndoSystem::keyChanged()
 IUndoStateSaver* UndoSystem::getStateSaver(IUndoable& undoable, IMapFileChangeTracker& tracker)
 {
     auto result = _undoables.insert(std::make_pair(&undoable, UndoStackFiller(tracker)));
+
+	// If we're in the middle of an active undo operation, assign this to the tracker (#4861)
+	if (_activeUndoStack != nullptr)
+	{
+		result.first->second.setStack(_activeUndoStack);
+	}
+
     return &(result.first->second);
 }
 
@@ -271,7 +279,7 @@ void UndoSystem::startUndo()
 bool UndoSystem::finishUndo(const std::string& command)
 {
 	bool changed = _undoStack.finish(command);
-	setActiveUndoStack(NULL);
+	setActiveUndoStack(nullptr);
 	return changed;
 }
 
@@ -284,18 +292,18 @@ void UndoSystem::startRedo()
 bool UndoSystem::finishRedo(const std::string& command)
 {
 	bool changed = _redoStack.finish(command);
-	setActiveUndoStack(NULL);
+	setActiveUndoStack(nullptr);
 	return changed;
 }
 
 // Assigns the given stack to all of the Undoables listed in the map
 void UndoSystem::setActiveUndoStack(UndoStack* stack)
 {
-	//rMessage() << "Setting stack of " << _undoables.size() << " undoables.\n";
+	_activeUndoStack = stack;
 
 	for (UndoablesMap::value_type& pair : _undoables)
 	{
-		pair.second.setStack(stack);
+		pair.second.setStack(_activeUndoStack);
 	}
 }
 

radiant/undo/UndoSystem.h

@@ -40,6 +40,8 @@ class UndoSystem :
 	UndoStack _undoStack;
 	UndoStack _redoStack;
 
+	UndoStack* _activeUndoStack;
+
 	typedef std::map<IUndoable*, UndoStackFiller> UndoablesMap;
 	UndoablesMap _undoables;