changeset 145:e7a431577c95

Cleaned the basic model up. Code is cleaner now and a bit faster. Some code path were never tested in depth :-( Added a 'time_to_load' attribute to XMLMapLoader, which records the seconds it took to load a map. Down from 6s to 5.6s for reio de hola. Yay!
author phoku@33b003aa-7bff-0310-803a-e67f0ece8222
date Thu, 09 Oct 2008 08:23:13 +0000
parents d2f1e81fbe2c
children 54b3984e1afc
files engine/core/model/model.cpp engine/core/model/model.h engine/core/model/model.i engine/extensions/loaders.py engine/extensions/serializers/xmlmap.py
diffstat 5 files changed, 91 insertions(+), 61 deletions(-) [+]
line wrap: on
line diff
--- a/engine/core/model/model.cpp	Thu Oct 09 06:18:36 2008 +0000
+++ b/engine/core/model/model.cpp	Thu Oct 09 08:23:13 2008 +0000
@@ -42,13 +42,14 @@
 
 	Model::Model(): 
 		FifeClass(),
+		m_last_namespace(NULL),
 		m_timeprovider(NULL) {
 	}
 
 	Model::~Model() {
 		purge(m_maps);
 		for(std::list<namespace_t>::iterator nspace = m_namespaces.begin(); nspace != m_namespaces.end(); ++nspace)
-			purge(nspace->second);
+			purge_map(nspace->second);
 		purge(m_pathers);
 		purge(m_created_grids);
 		purge(m_adopted_grids);
@@ -128,76 +129,68 @@
 	}
 
 	std::list<std::string> Model::getNamespaces() const {
-		std::list<std::string> lst;
+		std::list<std::string> namespace_list;
 		std::list<namespace_t>::const_iterator nspace = m_namespaces.begin();
 		for(; nspace != m_namespaces.end(); ++nspace) {
-			lst.push_back(nspace->first);
+			namespace_list.push_back(nspace->first);
 		}
-
-		return lst;
+		return namespace_list;
 	}
 
 	Object* Model::createObject(const std::string& identifier, const std::string& name_space, Object* parent) {
-
-		std::list<namespace_t>::iterator nspace = m_namespaces.begin();
-		for(; nspace != m_namespaces.end(); ++nspace) {
-			if(nspace->first == name_space) break;
+		// Find or create namespace
+		namespace_t* nspace = selectNamespace(name_space);
+		if(!nspace) {
+			m_namespaces.push_back(namespace_t(name_space,objectmap_t()));
+			nspace = selectNamespace(name_space);
 		}
 
-		if(nspace == m_namespaces.end()) {
-			m_namespaces.push_back(namespace_t(name_space,std::list<Object*>()));
-			nspace = m_namespaces.end();
-			--nspace;
+		// Check for nameclashes
+		objectmap_t::const_iterator it = nspace->second.find(identifier);
+		if( it != nspace->second.end() ) {
+			throw NameClash(identifier);
 		}
 
-		std::list<Object*>::const_iterator it = nspace->second.begin();
-		for(; it != nspace->second.end(); ++it) {
-			if(identifier == (*it)->getId())
-				throw NameClash(identifier);
-		}
-
+		// Finally insert & create
 		Object* object = new Object(identifier, name_space, parent);
-		nspace->second.push_back(object);
+		nspace->second[identifier] = object;
 		return object;
 	}
 
 	bool Model::deleteObject(Object* object) {
+		// WARNING: This code has obviously not been tested (thoroughly).
+
+		// Check if any instances exist. If yes - bail out.
 		std::list<Layer*>::const_iterator jt;
 		std::vector<Instance*>::const_iterator kt;
 		for(std::list<Map*>::iterator it = m_maps.begin(); it != m_maps.end(); ++it) {
 			for(jt = (*it)->getLayers().begin(); jt != (*it)->getLayers().end(); ++jt) {
 				for(kt = (*jt)->getInstances().begin(); kt != (*jt)->getInstances().end(); ++kt) {
 					Object* o = (*kt)->getObject();
-					while(o != 0) {
-						if(o == object)
-							return false;
+					if(o == object) {
+						return false;
 					}
 				}
 			}
 		}
 
-		std::string name_space = object->getNamespace();
-		std::list<namespace_t>::iterator nspace = m_namespaces.begin();
-		for(; nspace != m_namespaces.end(); ++nspace) {
-			if(nspace->first == name_space) break;
-		}
-
-		if(nspace == m_namespaces.end())
+		// Check if the namespace exists
+		namespace_t* nspace = selectNamespace(object->getNamespace());
+		if(!nspace)
 			return true;
 
-		std::list<Object*>::iterator it = nspace->second.begin();
-		for(; it != nspace->second.end(); ++it) {
-			if(*it == object) {
-				delete *it;
-				nspace->second.erase(it);
-				return true;
-			}
+		// If yes - delete+erase object.
+		objectmap_t::iterator it = nspace->second.find(object->getId());
+		if( it != nspace->second.end()) {
+			delete it->second;
+			nspace->second.erase(it);
 		}
 
 		return true;
 	}
 
 	bool Model::deleteObjects() {
+		// If we have layers with instances - bail out.
 		std::list<Layer*>::const_iterator jt;
 		for(std::list<Map*>::iterator it = m_maps.begin(); it != m_maps.end(); ++it) {
 			for(jt = (*it)->getLayers().begin(); jt != (*it)->getLayers().end(); ++jt) {
@@ -206,42 +199,63 @@
 			}
 		}
 
+		// Otherwise delete every object in every namespace
 		std::list<namespace_t>::iterator nspace = m_namespaces.begin();
 		while(nspace != m_namespaces.end()) {
-			std::list<Object*>::iterator it = nspace->second.begin();
+			objectmap_t::iterator it = nspace->second.begin();
 			for(; it != nspace->second.end(); ++it) {
-				delete *it;
+				delete it->second;
 			}
 			nspace = m_namespaces.erase(nspace);
 		}
+		m_last_namespace = 0;
 		return true;
 	}
 
 	Object* Model::getObject(const std::string& id, const std::string& name_space) {
-		std::list<namespace_t>::iterator nspace = m_namespaces.begin();
-		for(; nspace != m_namespaces.end(); ++nspace) {
-			if(nspace->first == name_space) {
-				std::list<Object*>::iterator obj = nspace->second.begin();
-				for(; obj != nspace->second.end(); ++obj)
-					if((*obj)->getId() == id) return *obj;
-			}
+		namespace_t* nspace = selectNamespace(name_space);
+		if(nspace) {
+			objectmap_t::iterator it = nspace->second.find(id);
+			if( it !=  nspace->second.end() )
+				return it->second;
 		}
-		for(; nspace != m_namespaces.end(); ++nspace) {
-			std::list<Object*>::iterator obj = nspace->second.begin();
-			for(; obj != nspace->second.end(); ++obj)
-				if((*obj)->getId() == id) return *obj;
-		}
- 
 		return 0;
 	}
 
-	const std::list<Object*>& Model::getObjects(const std::string& name_space) const {
+	std::list<Object*> Model::getObjects(const std::string& name_space) const {
+		std::list<Object*> object_list;
+		const namespace_t* nspace = selectNamespace(name_space);
+		if(nspace) {
+			objectmap_t::const_iterator it = nspace->second.begin();
+			for(; it != nspace->second.end(); ++it )
+				object_list.push_back(it->second);
+			return object_list;
+		}
+		throw NotFound(name_space);
+	}
+
+	const Model::namespace_t* Model::selectNamespace(const std::string& name_space) const {
 		std::list<namespace_t>::const_iterator nspace = m_namespaces.begin();
 		for(; nspace != m_namespaces.end(); ++nspace) {
-			if(nspace->first == name_space) return nspace->second;
+			if( nspace->first == name_space ) {
+				return &(*nspace);
+			}
 		}
+		return 0;
+	}
 
-		throw NotFound(name_space);
+	Model::namespace_t* Model::selectNamespace(const std::string& name_space) {
+		if( m_last_namespace && m_last_namespace->first == name_space )
+			return m_last_namespace;
+		std::list<namespace_t>::iterator nspace = m_namespaces.begin();
+		for(; nspace != m_namespaces.end(); ++nspace) {
+			if( nspace->first == name_space ) {
+				m_last_namespace = &(*nspace);
+				return m_last_namespace;
+			}
+		}
+		m_last_namespace = 0;
+		return 0;
 	}
 
 	void Model::update() {
--- a/engine/core/model/model.h	Thu Oct 09 06:18:36 2008 +0000
+++ b/engine/core/model/model.h	Thu Oct 09 08:23:13 2008 +0000
@@ -107,14 +107,13 @@
 		 */
 		bool deleteObjects();
 
-		/** Get an object by its id. If no namespace is specified, the namespaces are searched in order
-		 * and the first matching object is returned. Returns 0 if object is not found.
+		/** Get an object by its id. Returns 0 if object is not found.
 		 */
 		Object* getObject(const std::string& id, const std::string& name_space);
 
 		/** Get all the objects in the given namespace.
 		 */
-		const std::list<Object*>& getObjects(const std::string& name_space) const;
+		std::list<Object*> getObjects(const std::string& name_space) const;
 
 		/** Adds pather to model. Moves ownership to model
 		 */
@@ -150,9 +149,19 @@
 
 		std::list<Map*> m_maps;
 
-		typedef std::pair<std::string, std::list<Object*> > namespace_t;
+		typedef std::map<std::string,Object*> objectmap_t;
+		typedef std::pair<std::string,objectmap_t> namespace_t;
 		std::list<namespace_t> m_namespaces;
 
+		/// Used to remember last 'selected' namespace.
+		namespace_t* m_last_namespace;
+
+		/// Convenience function to retrieve a pointer to a namespace or NULL if it doesn't exist
+		namespace_t* selectNamespace(const std::string& name_space);
+
+		/// Convenience function to retrieve a pointer to a namespace or NULL if it doesn't exist
+		const namespace_t* selectNamespace(const std::string& name_space) const;
+
 		std::vector<AbstractPather*> m_pathers;
 		std::vector<CellGrid*> m_adopted_grids;
 		std::vector<CellGrid*> m_created_grids;
--- a/engine/core/model/model.i	Thu Oct 09 06:18:36 2008 +0000
+++ b/engine/core/model/model.i	Thu Oct 09 08:23:13 2008 +0000
@@ -55,7 +55,7 @@
 		bool deleteObject(Object*);
 		bool deleteObjects();
 		Object* getObject(const std::string& id, const std::string& name_space);
-		const std::list<Object*>& getObjects(const std::string& name_space) const;
+		std::list<Object*> getObjects(const std::string& name_space) const;
 
 		size_t getNumMaps() const;
 		void deleteMaps();
--- a/engine/extensions/loaders.py	Thu Oct 09 06:18:36 2008 +0000
+++ b/engine/extensions/loaders.py	Thu Oct 09 08:23:13 2008 +0000
@@ -22,7 +22,10 @@
 	@return	map	: map object
 	"""
 	map_loader = XMLMapLoader(engine, callback)
-	return map_loader.loadResource(fife.ResourceLocation(path))
+	map = map_loader.loadResource(fife.ResourceLocation(path))
+	#print "--- Loading map took: ", map_loader.time_to_load, " seconds."
+	return map
+
 
 def loadImportFile(path, engine):
 	object_loader = XMLObjectLoader(engine.getImagePool(), engine.getAnimationPool(), engine.getModel(), engine.getVFS())
--- a/engine/extensions/serializers/xmlmap.py	Thu Oct 09 06:18:36 2008 +0000
+++ b/engine/extensions/serializers/xmlmap.py	Thu Oct 09 08:23:13 2008 +0000
@@ -6,6 +6,7 @@
 
 import loaders
 from serializers import *
+import time
 
 FORMAT = '1.0'
 
@@ -33,6 +34,7 @@
 		self.anim_pool = self.engine.getAnimationPool()
 		self.map = None
 		self.source = None
+		self.time_to_load = 0
 
 		self.nspace = None
 
@@ -40,6 +42,7 @@
 		raise SyntaxError(''.join(['File: ', self.source, ' . ', msg]))
 
 	def loadResource(self, location):
+		start_time = time.time()
 		self.source = location.getFilename()
 		f = self.vfs.open(self.source)
 		f.thisown = 1
@@ -47,6 +50,7 @@
 		root = tree.getroot()
 			
 		map = self.parse_map(root)
+		self.time_to_load = time.time() - start_time
 		return map
 
 	def parse_map(self, mapelt):