Fix the documentation of InvRef:get_lists() and clean up code (#12150)

doc/lua_api.txt

@@ -6355,9 +6355,9 @@ An `InvRef` is a reference to an inventory.
 * `set_width(listname, width)`: set width of list; currently used for crafting
 * `get_stack(listname, i)`: get a copy of stack index `i` in list
 * `set_stack(listname, i, stack)`: copy `stack` to index `i` in list
-* `get_list(listname)`: return full list
+* `get_list(listname)`: return full list (list of `ItemStack`s)
 * `set_list(listname, list)`: set full list (size will not change)
-* `get_lists()`: returns list of inventory lists
+* `get_lists()`: returns table that maps listnames to inventory lists
 * `set_lists(lists)`: sets inventory lists (size will not change)
 * `add_item(listname, stack)`: add item somewhere in list, returns leftover
   `ItemStack`.

src/database/database-postgresql.cpp

@@ -495,7 +495,7 @@ void PlayerDatabasePostgreSQL::savePlayer(RemotePlayer *player)
 	execPrepared("remove_player_inventories", 1, rmvalues);
 	execPrepared("remove_player_inventory_items", 1, rmvalues);
 
-	std::vector<const InventoryList*> inventory_lists = sao->getInventory()->getLists();
+	const auto &inventory_lists = sao->getInventory()->getLists();
 	std::ostringstream oss;
 	for (u16 i = 0; i < inventory_lists.size(); i++) {
 		const InventoryList* list = inventory_lists[i];

src/database/database-sqlite3.cpp

@@ -476,7 +476,7 @@ void PlayerDatabaseSQLite3::savePlayer(RemotePlayer *player)
 	sqlite3_vrfy(sqlite3_step(m_stmt_player_remove_inventory_items), SQLITE_DONE);
 	sqlite3_reset(m_stmt_player_remove_inventory_items);
 
-	std::vector<const InventoryList*> inventory_lists = sao->getInventory()->getLists();
+	const auto &inventory_lists = sao->getInventory()->getLists();
 	std::ostringstream oss;
 	for (u16 i = 0; i < inventory_lists.size(); i++) {
 		const InventoryList *list = inventory_lists[i];

src/inventory.cpp

@@ -503,11 +503,6 @@ void InventoryList::deSerialize(std::istream &is)
 	throw SerializationError(ss.str());
 }
 
-InventoryList::InventoryList(const InventoryList &other)
-{
-	*this = other;
-}
-
 InventoryList & InventoryList::operator = (const InventoryList &other)
 {
 	m_items = other.m_items;
@@ -535,21 +530,6 @@ bool InventoryList::operator == (const InventoryList &other) const
 	return true;
 }
 
-const std::string &InventoryList::getName() const
-{
-	return m_name;
-}
-
-u32 InventoryList::getSize() const
-{
-	return m_items.size();
-}
-
-u32 InventoryList::getWidth() const
-{
-	return m_width;
-}
-
 u32 InventoryList::getUsedSlots() const
 {
 	u32 num = 0;
@@ -560,18 +540,6 @@ u32 InventoryList::getUsedSlots() const
 	return num;
 }
 
-const ItemStack& InventoryList::getItem(u32 i) const
-{
-	assert(i < m_size); // Pre-condition
-	return m_items[i];
-}
-
-ItemStack& InventoryList::getItem(u32 i)
-{
-	assert(i < m_size); // Pre-condition
-	return m_items[i];
-}
-
 ItemStack InventoryList::changeItem(u32 i, const ItemStack &newitem)
 {
 	if(i >= m_items.size())
@@ -960,16 +928,6 @@ InventoryList * Inventory::getList(const std::string &name)
 	return m_lists[i];
 }
 
-std::vector<const InventoryList*> Inventory::getLists()
-{
-	std::vector<const InventoryList*> lists;
-	lists.reserve(m_lists.size());
-	for (auto list : m_lists) {
-		lists.push_back(list);
-	}
-	return lists;
-}
-
 bool Inventory::deleteList(const std::string &name)
 {
 	s32 i = getListIndex(name);

src/inventory.h

@@ -198,7 +198,7 @@ public:
 	void serialize(std::ostream &os, bool incremental) const;
 	void deSerialize(std::istream &is);
 
-	InventoryList(const InventoryList &other);
+	InventoryList(const InventoryList &other) { *this = other; }
 	InventoryList & operator = (const InventoryList &other);
 	bool operator == (const InventoryList &other) const;
 	bool operator != (const InventoryList &other) const
@@ -206,15 +206,25 @@ public:
 		return !(*this == other);
 	}
 
-	const std::string &getName() const;
+	const std::string &getName() const { return m_name; }
-	u32 getSize() const;
+	u32 getSize() const { return static_cast<u32>(m_items.size()); }
-	u32 getWidth() const;
+	u32 getWidth() const { return m_width; }
 	// Count used slots
 	u32 getUsedSlots() const;
 
 	// Get reference to item
-	const ItemStack& getItem(u32 i) const;
+	const ItemStack &getItem(u32 i) const
-	ItemStack& getItem(u32 i);
+	{
+		assert(i < m_size); // Pre-condition
+		return m_items[i];
+	}
+	ItemStack &getItem(u32 i)
+	{
+		assert(i < m_size); // Pre-condition
+		return m_items[i];
+	}
+	// Get reference to all items
+	const std::vector<ItemStack> &getItems() const { return m_items; }
 	// Returns old item. Parameter can be an empty item.
 	ItemStack changeItem(u32 i, const ItemStack &newitem);
 	// Delete item
@@ -271,7 +281,7 @@ public:
 private:
 	std::vector<ItemStack> m_items;
 	std::string m_name;
-	u32 m_size;
+	u32 m_size; // always the same as m_items.size()
 	u32 m_width = 0;
 	IItemDefManager *m_itemdef;
 	bool m_dirty = true;
@@ -301,7 +311,7 @@ public:
 	InventoryList * addList(const std::string &name, u32 size);
 	InventoryList * getList(const std::string &name);
 	const InventoryList * getList(const std::string &name) const;
-	std::vector<const InventoryList*> getLists();
+	const std::vector<InventoryList *> &getLists() const { return m_lists; }
 	bool deleteList(const std::string &name);
 	// A shorthand for adding items. Returns leftover item (possibly empty).
 	ItemStack addItem(const std::string &listname, const ItemStack &newitem)

src/script/common/c_content.cpp

@@ -1351,17 +1351,22 @@ void push_tool_capabilities(lua_State *L,
 }
 
 /******************************************************************************/
-void push_inventory_list(lua_State *L, Inventory *inv, const char *name)
+void push_inventory_list(lua_State *L, const InventoryList &invlist)
 {
-	InventoryList *invlist = inv->getList(name);
+	push_items(L, invlist.getItems());
-	if(invlist == NULL){
+}
-		lua_pushnil(L);
+
-		return;
+/******************************************************************************/
+void push_inventory_lists(lua_State *L, const Inventory &inv)
+{
+	const auto &lists = inv.getLists();
+	lua_createtable(L, 0, lists.size());
+	for(const InventoryList *list : lists) {
+		const std::string &name = list->getName();
+		lua_pushlstring(L, name.c_str(), name.size());
+		push_inventory_list(L, *list);
+		lua_rawset(L, -3);
 	}
-	std::vector<ItemStack> items;
-	for(u32 i=0; i<invlist->getSize(); i++)
-		items.push_back(invlist->getItem(i));
-	push_items(L, items);
 }
 
 /******************************************************************************/

src/script/common/c_content.h

@@ -55,6 +55,7 @@ struct ObjectProperties;
 struct SimpleSoundSpec;
 struct ServerSoundParams;
 class Inventory;
+class InventoryList;
 struct NodeBox;
 struct ContentFeatures;
 struct TileDef;
@@ -120,8 +121,9 @@ void               push_object_properties    (lua_State *L,
                                               ObjectProperties *prop);
 
 void               push_inventory_list       (lua_State *L,
-                                              Inventory *inv,
+                                              const InventoryList &invlist);
-                                              const char *name);
+void               push_inventory_lists      (lua_State *L,
+                                              const Inventory &inv);
 void               read_inventory_list       (lua_State *L, int tableindex,
                                               Inventory *inv, const char *name,
                                               Server *srv, int forcesize=-1);

src/script/cpp_api/s_client.cpp

@@ -281,15 +281,7 @@ bool ScriptApiClient::on_inventory_open(Inventory *inventory)
 	lua_getglobal(L, "core");
 	lua_getfield(L, -1, "registered_on_inventory_open");
 
-	std::vector<const InventoryList*> lists = inventory->getLists();
+	push_inventory_lists(L, *inventory);
-	std::vector<const InventoryList*>::iterator iter = lists.begin();
-	lua_createtable(L, 0, lists.size());
-	for (; iter != lists.end(); iter++) {
-		const char* name = (*iter)->getName().c_str();
-		lua_pushstring(L, name);
-		push_inventory_list(L, inventory, name);
-		lua_rawset(L, -3);
-	}
 
 	try {
 		runCallbacks(1, RUN_CALLBACKS_MODE_OR);

src/script/lua_api/l_inventory.cpp

@@ -214,11 +214,16 @@ int InvRef::l_get_list(lua_State *L)
 	InvRef *ref = checkobject(L, 1);
 	const char *listname = luaL_checkstring(L, 2);
 	Inventory *inv = getinv(L, ref);
-	if(inv){
+	if (!inv) {
-		push_inventory_list(L, inv, listname);
-	} else {
 		lua_pushnil(L);
+		return 1;
 	}
+	InventoryList *invlist = inv->getList(listname);
+	if (!invlist) {
+		lua_pushnil(L);
+		return 1;
+	}
+	push_inventory_list(L, *invlist);
 	return 1;
 }
 
@@ -242,7 +247,7 @@ int InvRef::l_set_list(lua_State *L)
 	return 0;
 }
 
-// get_lists(self) -> list of InventoryLists
+// get_lists(self) -> table that maps listnames to InventoryLists
 int InvRef::l_get_lists(lua_State *L)
 {
 	NO_MAP_LOCK_REQUIRED;
@@ -251,15 +256,7 @@ int InvRef::l_get_lists(lua_State *L)
 	if (!inv) {
 		return 0;
 	}
-	std::vector<const InventoryList*> lists = inv->getLists();
+	push_inventory_lists(L, *inv);
-	std::vector<const InventoryList*>::iterator iter = lists.begin();
-	lua_createtable(L, 0, lists.size());
-	for (; iter != lists.end(); iter++) {
-		const char* name = (*iter)->getName().c_str();
-		lua_pushstring(L, name);
-		push_inventory_list(L, inv, name);
-		lua_rawset(L, -3);
-	}
 	return 1;
 }
 

src/script/lua_api/l_nodemeta.cpp

@@ -130,15 +130,11 @@ void NodeMetaRef::handleToTable(lua_State *L, Metadata *_meta)
 	NodeMetadata *meta = (NodeMetadata *) _meta;
 
 	// inventory
-	lua_newtable(L);
 	Inventory *inv = meta->getInventory();
 	if (inv) {
-		std::vector<const InventoryList *> lists = inv->getLists();
+		push_inventory_lists(L, *inv);
-		for(std::vector<const InventoryList *>::const_iterator
+	} else {
-				i = lists.begin(); i != lists.end(); ++i) {
+		lua_newtable(L);
-			push_inventory_list(L, inv, (*i)->getName().c_str());
-			lua_setfield(L, -2, (*i)->getName().c_str());
-		}
 	}
 	lua_setfield(L, -2, "inventory");
 }