From: Kahrl <kahrl@gmx.net>
Date: Sat, 13 Jul 2013 22:55:47 +0000 (+0200)
Subject: Change ContentFeatures array to a vector
X-Git-Url: http://81.2.79.47:8989/gitweb/?a=commitdiff_plain;h=112dbba7c4d38bf780e4e2f05bdff106b52ce2f9;p=zefram%2Fminetest%2Fminetest_engine.git

Change ContentFeatures array to a vector
---

diff --git a/src/inventory.cpp b/src/inventory.cpp
index 928021c2..2ce50e01 100644
--- a/src/inventory.cpp
+++ b/src/inventory.cpp
@@ -175,7 +175,7 @@ void ItemStack::deSerialize(std::istream &is, IItemDefManager *itemdef)
 		// Convert old materials
 		if(material <= 0xff)
 			material = content_translate_from_19_to_internal(material);
-		if(material > MAX_CONTENT)
+		if(material > 0xfff)
 			throw SerializationError("Too large material number");
 		// Convert old id to name
 		NameIdMapping legacy_nimap;
@@ -194,7 +194,7 @@ void ItemStack::deSerialize(std::istream &is, IItemDefManager *itemdef)
 		is>>material;
 		u16 materialcount;
 		is>>materialcount;
-		if(material > MAX_CONTENT)
+		if(material > 0xfff)
 			throw SerializationError("Too large material number");
 		// Convert old id to name
 		NameIdMapping legacy_nimap;
diff --git a/src/itemdef.cpp b/src/itemdef.cpp
index f692ccf5..e7498ce5 100644
--- a/src/itemdef.cpp
+++ b/src/itemdef.cpp
@@ -519,7 +519,8 @@ public:
 
 		// Add the four builtin items:
 		//   "" is the hand
-		//   "unknown" is returned whenever an undefined item is accessed
+		//   "unknown" is returned whenever an undefined item
+		//     is accessed (is also the unknown node)
 		//   "air" is the air node
 		//   "ignore" is the ignore node
 
@@ -530,6 +531,7 @@ public:
 		m_item_definitions.insert(std::make_pair("", hand_def));
 
 		ItemDefinition* unknown_def = new ItemDefinition;
+		unknown_def->type = ITEM_NODE;
 		unknown_def->name = "unknown";
 		m_item_definitions.insert(std::make_pair("unknown", unknown_def));
 
diff --git a/src/mapnode.h b/src/mapnode.h
index 53e36b67..74b079c6 100644
--- a/src/mapnode.h
+++ b/src/mapnode.h
@@ -35,19 +35,23 @@ class INodeDefManager;
 	- Tile = TileSpec at some side of a node of some content type
 */
 typedef u16 content_t;
-#define MAX_CONTENT 0xfff
 
 /*
-	Ignored node.
+	The maximum node ID that can be registered by mods. This must
+	be significantly lower than the maximum content_t value, so that
+	there is enough room for dummy node IDs, which are created when
+	a MapBlock containing unknown node names is loaded from disk.
+*/
+#define MAX_REGISTERED_CONTENT 0xfffU
 
-	Anything that stores MapNodes doesn't have to preserve parameters
-	associated with this material.
-	
-	Doesn't create faces with anything and is considered being
-	out-of-map in the game map.
+/*
+	A solid walkable node with the texture unknown_node.png.
+
+	For example, used on the client to display unregistered node IDs
+	(instead of expanding the vector of node definitions each time
+	such a node is received).
 */
-#define CONTENT_IGNORE 127
-#define CONTENT_IGNORE_DEFAULT_PARAM 0
+#define CONTENT_UNKNOWN 125
 
 /*
 	The common material through which the player can walk and which
@@ -55,6 +59,18 @@ typedef u16 content_t;
 */
 #define CONTENT_AIR 126
 
+/*
+	Ignored node.
+	
+	Unloaded chunks are considered to consist of this. Several other
+	methods return this when an error occurs. Also, during
+	map generation this means the node has not been set yet.
+	
+	Doesn't create faces with anything and is considered being
+	out-of-map in the game map.
+*/
+#define CONTENT_IGNORE 127
+
 enum LightBank
 {
 	LIGHTBANK_DAY,
diff --git a/src/nodedef.cpp b/src/nodedef.cpp
index 4b2fe164..b2641516 100644
--- a/src/nodedef.cpp
+++ b/src/nodedef.cpp
@@ -27,6 +27,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "log.h"
 #include "settings.h"
 #include "nameidmapping.h"
+#include "util/numeric.h"
 #include "util/serialize.h"
 //#include "profiler.h" // For TimeTaker
 
@@ -361,13 +362,26 @@ class CNodeDefManager: public IWritableNodeDefManager
 public:
 	void clear()
 	{
+		m_content_features.clear();
 		m_name_id_mapping.clear();
 		m_name_id_mapping_with_aliases.clear();
+		m_group_to_items.clear();
+		m_next_id = 0;
 
-		for(u16 i=0; i<=MAX_CONTENT; i++)
+		u32 initial_length = 0;
+		initial_length = MYMAX(initial_length, CONTENT_UNKNOWN + 1);
+		initial_length = MYMAX(initial_length, CONTENT_AIR + 1);
+		initial_length = MYMAX(initial_length, CONTENT_IGNORE + 1);
+		m_content_features.resize(initial_length);
+
+		// Set CONTENT_UNKNOWN
 		{
-			ContentFeatures &f = m_content_features[i];
-			f.reset(); // Reset to defaults
+			ContentFeatures f;
+			f.name = "unknown";
+			// Insert directly into containers
+			content_t c = CONTENT_UNKNOWN;
+			m_content_features[c] = f;
+			addNameIdMapping(c, f.name);
 		}
 
 		// Set CONTENT_AIR
@@ -387,6 +401,7 @@ public:
 			m_content_features[c] = f;
 			addNameIdMapping(c, f.name);
 		}
+
 		// Set CONTENT_IGNORE
 		{
 			ContentFeatures f;
@@ -406,16 +421,6 @@ public:
 			addNameIdMapping(c, f.name);
 		}
 	}
-	// CONTENT_IGNORE = not found
-	content_t getFreeId()
-	{
-		for(u32 i=0; i<=0xffff; i++){
-			const ContentFeatures &f = m_content_features[i];
-			if(f.name == "")
-				return i;
-		}
-		return CONTENT_IGNORE;
-	}
 	CNodeDefManager()
 	{
 		clear();
@@ -426,16 +431,15 @@ public:
 	virtual IWritableNodeDefManager* clone()
 	{
 		CNodeDefManager *mgr = new CNodeDefManager();
-		for(u16 i=0; i<=MAX_CONTENT; i++)
-		{
-			mgr->set(i, get(i));
-		}
+		*mgr = *this;
 		return mgr;
 	}
 	virtual const ContentFeatures& get(content_t c) const
 	{
-		assert(c <= MAX_CONTENT);
-		return m_content_features[c];
+		if(c < m_content_features.size())
+			return m_content_features[c];
+		else
+			return m_content_features[CONTENT_UNKNOWN];
 	}
 	virtual const ContentFeatures& get(const MapNode &n) const
 	{
@@ -468,7 +472,6 @@ public:
 		}
 		std::string group = name.substr(6);
 
-#if 1	// Optimized version, takes less than 1 microsecond at -O1
 		std::map<std::string, GroupItems>::const_iterator
 			i = m_group_to_items.find(group);
 		if (i == m_group_to_items.end())
@@ -480,50 +483,67 @@ public:
 			if ((*j).second != 0)
 				result.insert((*j).first);
 		}
-#else	// Old version, takes about ~150-200us at -O1
-		for(u16 id=0; id<=MAX_CONTENT; id++)
-		{
-			const ContentFeatures &f = m_content_features[id];
-			if(f.name == "") // Quickly discard undefined nodes
-				continue;
-			if(itemgroup_get(f.groups, group) != 0)
-				result.insert(id);
-		}
-#endif
 		//printf("getIds: %dus\n", t.stop());
 	}
 	virtual const ContentFeatures& get(const std::string &name) const
 	{
-		content_t id = CONTENT_IGNORE;
+		content_t id = CONTENT_UNKNOWN;
 		getId(name, id);
 		return get(id);
 	}
+	// returns CONTENT_IGNORE if no free ID found
+	content_t allocateId()
+	{
+		for(content_t id = m_next_id;
+				id >= m_next_id; // overflow?
+				++id){
+			while(id >= m_content_features.size()){
+				m_content_features.push_back(ContentFeatures());
+			}
+			const ContentFeatures &f = m_content_features[id];
+			if(f.name == ""){
+				m_next_id = id + 1;
+				return id;
+			}
+		}
+		// If we arrive here, an overflow occurred in id.
+		// That means no ID was found
+		return CONTENT_IGNORE;
+	}
 	// IWritableNodeDefManager
-	virtual void set(content_t c, const ContentFeatures &def)
+	virtual content_t set(const std::string &name,
+			const ContentFeatures &def)
 	{
-		verbosestream<<"registerNode: registering content id \""<<c
-				<<"\": name=\""<<def.name<<"\""<<std::endl;
-		assert(c <= MAX_CONTENT);
-		// Don't allow redefining CONTENT_IGNORE (but allow air)
-		if(def.name == "ignore" || c == CONTENT_IGNORE){
-			infostream<<"registerNode: WARNING: Ignoring "
+		assert(name != "");
+		assert(name == def.name);
+
+		// Don't allow redefining ignore (but allow air and unknown)
+		if(name == "ignore"){
+			infostream<<"NodeDefManager: WARNING: Ignoring "
 					<<"CONTENT_IGNORE redefinition"<<std::endl;
-			return;
+			return CONTENT_IGNORE;
 		}
-		// Check that the special contents are not redefined as different id
-		// because it would mess up everything
-		if((def.name == "ignore" && c != CONTENT_IGNORE) ||
-			(def.name == "air" && c != CONTENT_AIR)){
-			errorstream<<"registerNode: IGNORING ERROR: "
-					<<"trying to register built-in type \""
-					<<def.name<<"\" as different id"<<std::endl;
-			return;
+
+		content_t id = CONTENT_IGNORE;
+		bool found = m_name_id_mapping.getId(name, id);  // ignore aliases
+		if(!found){
+			// Get new id
+			id = allocateId();
+			if(id == CONTENT_IGNORE){
+				infostream<<"NodeDefManager: WARNING: Absolute "
+						<<"limit reached"<<std::endl;
+				return CONTENT_IGNORE;
+			}
+			assert(id != CONTENT_IGNORE);
+			addNameIdMapping(id, name);
 		}
-		m_content_features[c] = def;
-		if(def.name != "")
-			addNameIdMapping(c, def.name);
+		m_content_features[id] = def;
+		verbosestream<<"NodeDefManager: registering content id \""<<id
+				<<"\": name=\""<<def.name<<"\""<<std::endl;
 
 		// Add this content to the list of all groups it belongs to
+		// FIXME: This should remove a node from groups it no longer
+		// belongs to when a node is re-registered
 		for (ItemGroupList::const_iterator i = def.groups.begin();
 			i != def.groups.end(); ++i) {
 			std::string group_name = i->first;
@@ -531,28 +551,13 @@ public:
 			std::map<std::string, GroupItems>::iterator
 				j = m_group_to_items.find(group_name);
 			if (j == m_group_to_items.end()) {
-				m_group_to_items[group_name].push_back(std::make_pair(c, i->second));
+				m_group_to_items[group_name].push_back(
+						std::make_pair(id, i->second));
 			} else {
 				GroupItems &items = j->second;
-				items.push_back(std::make_pair(c, i->second));
+				items.push_back(std::make_pair(id, i->second));
 			}
 		}
-	}
-	virtual content_t set(const std::string &name,
-			const ContentFeatures &def)
-	{
-		assert(name == def.name);
-		u16 id = CONTENT_IGNORE;
-		bool found = m_name_id_mapping.getId(name, id);  // ignore aliases
-		if(!found){
-			// Get some id
-			id = getFreeId();
-			if(id == CONTENT_IGNORE)
-				return CONTENT_IGNORE;
-			if(name != "")
-				addNameIdMapping(id, name);
-		}
-		set(id, def);
 		return id;
 	}
 	virtual content_t allocateDummy(const std::string &name)
@@ -589,7 +594,7 @@ public:
 		bool new_style_leaves = g_settings->getBool("new_style_leaves");
 		bool opaque_water = g_settings->getBool("opaque_water");
 
-		for(u16 i=0; i<=MAX_CONTENT; i++)
+		for(u32 i=0; i<m_content_features.size(); i++)
 		{
 			ContentFeatures *f = &m_content_features[i];
 
@@ -766,9 +771,10 @@ public:
 		writeU8(os, 1); // version
 		u16 count = 0;
 		std::ostringstream os2(std::ios::binary);
-		for(u16 i=0; i<=MAX_CONTENT; i++)
+		for(u32 i=0; i<m_content_features.size(); i++)
 		{
-			if(i == CONTENT_IGNORE || i == CONTENT_AIR)
+			if(i == CONTENT_IGNORE || i == CONTENT_AIR
+					|| i == CONTENT_UNKNOWN)
 				continue;
 			ContentFeatures *f = &m_content_features[i];
 			if(f->name == "")
@@ -779,6 +785,8 @@ public:
 			std::ostringstream wrapper_os(std::ios::binary);
 			f->serialize(wrapper_os, protocol_version);
 			os2<<serializeString(wrapper_os.str());
+
+			assert(count + 1 > count); // must not overflow
 			count++;
 		}
 		writeU16(os, count);
@@ -792,24 +800,43 @@ public:
 			throw SerializationError("unsupported NodeDefinitionManager version");
 		u16 count = readU16(is);
 		std::istringstream is2(deSerializeLongString(is), std::ios::binary);
+		ContentFeatures f;
 		for(u16 n=0; n<count; n++){
 			u16 i = readU16(is2);
-			if(i > MAX_CONTENT){
-				errorstream<<"ContentFeatures::deSerialize(): "
-						<<"Too large content id: "<<i<<std::endl;
-				continue;
-			}
-			/*// Do not deserialize special types
-			if(i == CONTENT_IGNORE || i == CONTENT_AIR)
-				continue;*/
-			ContentFeatures *f = &m_content_features[i];
+
 			// Read it from the string wrapper
 			std::string wrapper = deSerializeString(is2);
 			std::istringstream wrapper_is(wrapper, std::ios::binary);
-			f->deSerialize(wrapper_is);
-			verbosestream<<"deserialized "<<f->name<<std::endl;
-			if(f->name != "")
-				addNameIdMapping(i, f->name);
+			f.deSerialize(wrapper_is);
+
+			// Check error conditions
+			if(i == CONTENT_IGNORE || i == CONTENT_AIR
+					|| i == CONTENT_UNKNOWN){
+				infostream<<"NodeDefManager::deSerialize(): WARNING: "
+					<<"not changing builtin node "<<i
+					<<std::endl;
+				continue;
+			}
+			if(f.name == ""){
+				infostream<<"NodeDefManager::deSerialize(): WARNING: "
+					<<"received empty name"<<std::endl;
+				continue;
+			}
+			u16 existing_id;
+			bool found = m_name_id_mapping.getId(f.name, existing_id);  // ignore aliases
+			if(found && i != existing_id){
+				infostream<<"NodeDefManager::deSerialize(): WARNING: "
+					<<"already defined with different ID: "
+					<<f.name<<std::endl;
+				continue;
+			}
+
+			// All is ok, add node definition with the requested ID
+			if(i >= m_content_features.size())
+				m_content_features.resize((u32)(i) + 1);
+			m_content_features[i] = f;
+			addNameIdMapping(i, f.name);
+			verbosestream<<"deserialized "<<f.name<<std::endl;
 		}
 	}
 private:
@@ -820,7 +847,7 @@ private:
 	}
 private:
 	// Features indexed by id
-	ContentFeatures m_content_features[MAX_CONTENT+1];
+	std::vector<ContentFeatures> m_content_features;
 	// A mapping for fast converting back and forth between names and ids
 	NameIdMapping m_name_id_mapping;
 	// Like m_name_id_mapping, but only from names to ids, and includes
@@ -831,6 +858,8 @@ private:
 	// that belong to it.  Necessary for a direct lookup in getIds().
 	// Note: Not serialized.
 	std::map<std::string, GroupItems> m_group_to_items;
+	// Next possibly free id
+	content_t m_next_id;
 };
 
 IWritableNodeDefManager* createNodeDefManager()
diff --git a/src/nodedef.h b/src/nodedef.h
index 7505cc12..e9ac2472 100644
--- a/src/nodedef.h
+++ b/src/nodedef.h
@@ -293,15 +293,14 @@ public:
 	virtual const ContentFeatures& get(content_t c) const=0;
 	virtual const ContentFeatures& get(const MapNode &n) const=0;
 	virtual bool getId(const std::string &name, content_t &result) const=0;
+	// If not found, returns CONTENT_IGNORE
 	virtual content_t getId(const std::string &name) const=0;
 	// Allows "group:name" in addition to regular node names
 	virtual void getIds(const std::string &name, std::set<content_t> &result)
 			const=0;
-	// If not found, returns the features of CONTENT_IGNORE
+	// If not found, returns the features of CONTENT_UNKNOWN
 	virtual const ContentFeatures& get(const std::string &name) const=0;
 
-	// Register node definition
-	virtual void set(content_t c, const ContentFeatures &def)=0;
 	// Register node definition by name (allocate an id)
 	// If returns CONTENT_IGNORE, could not allocate id
 	virtual content_t set(const std::string &name,
diff --git a/src/script/lua_api/l_item.cpp b/src/script/lua_api/l_item.cpp
index 4069c61c..6182c037 100644
--- a/src/script/lua_api/l_item.cpp
+++ b/src/script/lua_api/l_item.cpp
@@ -432,10 +432,15 @@ int ModApiItemMod::l_register_item_raw(lua_State *L)
 	idef->registerItem(def);
 
 	// Read the node definition (content features) and register it
-	if(def.type == ITEM_NODE)
-	{
+	if(def.type == ITEM_NODE){
 		ContentFeatures f = read_content_features(L, table);
-		ndef->set(f.name, f);
+		content_t id = ndef->set(f.name, f);
+
+		if(id > MAX_REGISTERED_CONTENT){
+			throw LuaError(L, "Number of registerable nodes ("
+					+ itos(MAX_REGISTERED_CONTENT+1)
+					+ ") exceeded (" + name + ")");
+		}
 	}
 
 	return 0; /* number of results */
diff --git a/src/test.cpp b/src/test.cpp
index ddbcee7f..7753e1d5 100644
--- a/src/test.cpp
+++ b/src/test.cpp
@@ -70,20 +70,18 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 	A few item and node definitions for those tests that need them
 */
 
-#define CONTENT_STONE 0
-#define CONTENT_GRASS 0x800
-#define CONTENT_TORCH 100
+static content_t CONTENT_STONE;
+static content_t CONTENT_GRASS;
+static content_t CONTENT_TORCH;
 
 void define_some_nodes(IWritableItemDefManager *idef, IWritableNodeDefManager *ndef)
 {
-	content_t i;
 	ItemDefinition itemdef;
 	ContentFeatures f;
 
 	/*
 		Stone
 	*/
-	i = CONTENT_STONE;
 	itemdef = ItemDefinition();
 	itemdef.type = ITEM_NODE;
 	itemdef.name = "default:stone";
@@ -99,12 +97,11 @@ void define_some_nodes(IWritableItemDefManager *idef, IWritableNodeDefManager *n
 		f.tiledef[i].name = "default_stone.png";
 	f.is_ground_content = true;
 	idef->registerItem(itemdef);
-	ndef->set(i, f);
+	CONTENT_STONE = ndef->set(f.name, f);
 
 	/*
 		Grass
 	*/
-	i = CONTENT_GRASS;
 	itemdef = ItemDefinition();
 	itemdef.type = ITEM_NODE;
 	itemdef.name = "default:dirt_with_grass";
@@ -122,12 +119,11 @@ void define_some_nodes(IWritableItemDefManager *idef, IWritableNodeDefManager *n
 		f.tiledef[i].name = "default_dirt.png^default_grass_side.png";
 	f.is_ground_content = true;
 	idef->registerItem(itemdef);
-	ndef->set(i, f);
+	CONTENT_GRASS = ndef->set(f.name, f);
 
 	/*
 		Torch (minimal definition for lighting tests)
 	*/
-	i = CONTENT_TORCH;
 	itemdef = ItemDefinition();
 	itemdef.type = ITEM_NODE;
 	itemdef.name = "default:torch";
@@ -138,7 +134,7 @@ void define_some_nodes(IWritableItemDefManager *idef, IWritableNodeDefManager *n
 	f.sunlight_propagates = true;
 	f.light_source = LIGHT_MAX-1;
 	idef->registerItem(itemdef);
-	ndef->set(i, f);
+	CONTENT_TORCH = ndef->set(f.name, f);
 }
 
 struct TestBase