changeset 338:d266506ff4f9

Bug fix. It turned out the instance tree contained ghost instances, since InstanceTree.removeInstance sometimes failed. This caused those random crashes in UH. Now the InstanceTree enforces that remove/add Instance work in pairs. A new Exception is raised in case this ever goes wrong again. (InconsitencyDetected) Furthermore the removeInstancheChangeListener stuff had a fix to become reentrant. It is not clear wether this was shadowed by the aforementioned bug or was never triggered.
author phoku@33b003aa-7bff-0310-803a-e67f0ece8222
date Mon, 24 Aug 2009 18:32:03 +0000
parents f9aca52c7c45
children 0fd74235b34d
files engine/core/model/structures/instance.cpp engine/core/model/structures/instancetree.cpp engine/core/model/structures/instancetree.h engine/core/util/base/exception.h
diffstat 4 files changed, 24 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/engine/core/model/structures/instance.cpp	Mon Aug 24 16:06:30 2009 +0000
+++ b/engine/core/model/structures/instance.cpp	Mon Aug 24 18:32:03 2009 +0000
@@ -152,9 +152,17 @@
 		if (source.m_changeinfo != ICHANGE_NO_CHANGES) {
 			std::vector<InstanceChangeListener*>::iterator i = m_changelisteners.begin();
 			while (i != m_changelisteners.end()) {
-				(*i)->onInstanceChanged(&source, source.m_changeinfo);
+				if (NULL != *i)
+				{
+					(*i)->onInstanceChanged(&source, source.m_changeinfo);
+				}
 				++i;
 			}
+			// Really remove "removed" listeners.
+			m_changelisteners.erase(
+				std::remove(m_changelisteners.begin(),m_changelisteners.end(),
+					(InstanceChangeListener*)NULL),
+				m_changelisteners.end());
 		}
 	}
 
@@ -242,7 +250,7 @@
 		std::vector<InstanceChangeListener*>::iterator i = m_activity->m_changelisteners.begin();
 		while (i != m_activity->m_changelisteners.end()) {
 			if ((*i) == listener) {
-				m_activity->m_changelisteners.erase(i);
+				*i = NULL;
 				return;
 			}
 			++i;
@@ -285,7 +293,7 @@
 		m_activity->m_actioninfo->m_target = new Location(leader->getLocationRef());
 		m_activity->m_actioninfo->m_speed = speed;
 		m_activity->m_actioninfo->m_leader = leader;
-                leader->addDeleteListener(this);
+		leader->addDeleteListener(this);
 		setFacingLocation(*m_activity->m_actioninfo->m_target);
 		FL_DBG(_log, LMsg("starting action ") <<  action_name << " from" << m_location << " to " << *m_activity->m_actioninfo->m_target << " with speed " << speed);
 	}
--- a/engine/core/model/structures/instancetree.cpp	Mon Aug 24 16:06:30 2009 +0000
+++ b/engine/core/model/structures/instancetree.cpp	Mon Aug 24 18:32:03 2009 +0000
@@ -28,6 +28,7 @@
 // These includes are split up in two parts, separated by one empty line
 // First block: files included from the FIFE root src directory
 // Second block: files included from the same folder
+#include "util/base/exception.h"
 #include "model/structures/instance.h"
 #include "util/structures/rect.h"
 
@@ -44,22 +45,28 @@
 
 	bool InstanceTree::addInstance(Instance* instance) {
 		ModelCoordinate coords = instance->getLocationRef().getLayerCoordinates();
-		InstanceList& list = m_tree.find_container(coords.x,coords.y,0,0)->data();
+		InstanceTreeNode * node = m_tree.find_container(coords.x,coords.y,0,0);
+		InstanceList& list = node->data();
 		list.push_back(instance);
+		if( m_reverse.find(instance) != m_reverse.end() )
+			throw new InconsistencyDetected("Duplicate Instance.");
+		m_reverse[instance] = node;
 		return true;
 	}
 
 	bool InstanceTree::removeInstance(Instance* instance) {
 		ModelCoordinate coords = instance->getLocationRef().getLayerCoordinates();
-		InstanceList& list = m_tree.find_container(coords.x,coords.y, 0, 0)->data();
-
+		InstanceTreeNode * node = m_reverse[instance];
+		if( !node )
+			throw new InconsistencyDetected("Removing Ghost Instance.");
+		m_reverse.erase(instance);
+		InstanceList& list = node->data();
 		for(InstanceList::iterator i = list.begin(); i != list.end(); ++i) {
 			if((*i) == instance) {
 				list.erase(i);
 				return true;
 			}
 		}
-
 		return false;
 	}
 
--- a/engine/core/model/structures/instancetree.h	Mon Aug 24 16:06:30 2009 +0000
+++ b/engine/core/model/structures/instancetree.h	Mon Aug 24 18:32:03 2009 +0000
@@ -94,7 +94,7 @@
 
 	private:
 		InstanceQuadTree m_tree;
-
+		std::map<Instance*,InstanceTreeNode*> m_reverse;
 	};
 
 }
--- a/engine/core/util/base/exception.h	Mon Aug 24 16:06:30 2009 +0000
+++ b/engine/core/util/base/exception.h	Mon Aug 24 18:32:03 2009 +0000
@@ -83,6 +83,7 @@
 	FIFE_EXCEPTION_DECL(ScriptException, "Error related to scripting functionality");
 	FIFE_EXCEPTION_DECL(EventException, "Error related to event functionality");
 	FIFE_EXCEPTION_DECL(GuiException, "Error related to gui functionality");
+	FIFE_EXCEPTION_DECL(InconsistencyDetected, "An inconsistency in FIFE internals was detected. Please report this is a FIFE Bug.");
 	
 	/** @bug The memory allocation in @c std::string might fail, resulting in terminate. */
 	FIFE_EXCEPTION_DECL(OutOfMemory, "Buy more ram ;)");