Fix `InvRef` bugs and add unit tests (#14591)

doc/lua_api.md

@@ -7456,6 +7456,7 @@ An `InvRef` is a reference to an inventory.
     * returns `false` on error (e.g. invalid `listname` or `size`)
 * `get_width(listname)`: get width of a list
 * `set_width(listname, width)`: set width of list; currently used for crafting
+    * returns `false` on error (e.g. invalid `listname` or `width`)
 * `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)`: returns full list (list of `ItemStack`s)

games/devtest/mods/unittests/init.lua

@@ -184,6 +184,7 @@ dofile(modpath .. "/itemstack_equals.lua")
 dofile(modpath .. "/content_ids.lua")
 dofile(modpath .. "/metadata.lua")
 dofile(modpath .. "/raycast.lua")
+dofile(modpath .. "/inventory.lua")
 
 --------------
 

games/devtest/mods/unittests/inventory.lua

@@ -0,0 +1,73 @@
+
+local item_with_meta = ItemStack({name = "air", meta = {test = "abc"}})
+
+local test_list = {
+	ItemStack("air"),
+	ItemStack(""),
+	ItemStack(item_with_meta),
+}
+
+local function compare_lists(a, b)
+	if not a or not b or #a ~= #b then
+		return false
+	end
+	for i=1, #a do
+		if not ItemStack(a[i]):equals(ItemStack(b[i])) then
+			return false
+		end
+	end
+	return true
+end
+
+local function test_inventory()
+	local inv = minetest.create_detached_inventory("test")
+
+	inv:set_lists({test = {""}})
+	assert(inv:get_list("test"))
+
+	assert(inv:get_size("test") == 1)
+	assert(inv:set_size("test", 3))
+	assert(not inv:set_size("test", -1))
+
+	assert(inv:get_width("test") == 0)
+	assert(inv:set_width("test", 3))
+	assert(not inv:set_width("test", -1))
+
+	inv:set_stack("test", 1, "air")
+	inv:set_stack("test", 3, item_with_meta)
+	assert(not inv:is_empty("test"))
+	assert(compare_lists(inv:get_list("test"), test_list))
+
+	assert(inv:add_item("test", "air") == ItemStack())
+	assert(inv:add_item("test", item_with_meta) == ItemStack())
+	assert(inv:get_stack("test", 1) == ItemStack("air 2"))
+
+	assert(inv:room_for_item("test", "air 99"))
+	inv:set_stack("test", 2, "air 99")
+	assert(not inv:room_for_item("test", "air 99"))
+	inv:set_stack("test", 2, "")
+
+	assert(inv:contains_item("test", "air"))
+	assert(not inv:contains_item("test", "air 99"))
+	assert(inv:contains_item("test", item_with_meta, true))
+
+	-- Items should be removed in reverse and combine with first stack removed
+	assert(inv:remove_item("test", "air") == item_with_meta)
+	item_with_meta:set_count(2)
+	assert(inv:remove_item("test", "air 2") == item_with_meta)
+	assert(inv:remove_item("test", "air") == ItemStack("air"))
+	assert(inv:is_empty("test"))
+
+	-- Failure of set_list(s) should not change inventory
+	local before = inv:get_list("test")
+	pcall(inv.set_lists, inv, {test = true})
+	pcall(inv.set_list, inv, "test", true)
+	local after = inv:get_list("test")
+	assert(compare_lists(before, after))
+
+	local location = inv:get_location()
+	assert(minetest.remove_detached_inventory("test"))
+	assert(not minetest.get_inventory(location))
+end
+
+unittests.register("test_inventory", test_inventory)

src/script/lua_api/l_inventory.cpp

@@ -151,19 +151,28 @@ int InvRef::l_set_width(lua_State *L)
 	NO_MAP_LOCK_REQUIRED;
 	InvRef *ref = checkObject<InvRef>(L, 1);
 	const char *listname = luaL_checkstring(L, 2);
+
 	int newwidth = luaL_checknumber(L, 3);
+	if (newwidth < 0) {
+		lua_pushboolean(L, false);
+		return 1;
+	}
+
 	Inventory *inv = getinv(L, ref);
 	if(inv == NULL){
-		return 0;
+		lua_pushboolean(L, false);
+		return 1;
 	}
 	InventoryList *list = inv->getList(listname);
 	if(list){
 		list->setWidth(newwidth);
 	} else {
-		return 0;
+		lua_pushboolean(L, false);
+		return 1;
 	}
 	reportInventoryChange(L, ref);
-	return 0;
+	lua_pushboolean(L, true);
+	return 1;
 }
 
 // get_stack(self, listname, i) -> itemstack
@@ -264,19 +273,19 @@ int InvRef::l_set_lists(lua_State *L)
 	}
 
 	// Make a temporary inventory in case reading fails
-	Inventory *tempInv(inv);
-	tempInv->clear();
+	Inventory tempInv(*inv);
+	tempInv.clear();
 
 	Server *server = getServer(L);
 
 	lua_pushnil(L);
 	luaL_checktype(L, 2, LUA_TTABLE);
 	while (lua_next(L, 2)) {
-		const char *listname = lua_tostring(L, -2);
-		read_inventory_list(L, -1, tempInv, listname, server);
+		const char *listname = luaL_checkstring(L, -2);
+		read_inventory_list(L, -1, &tempInv, listname, server);
 		lua_pop(L, 1);
 	}
-	inv = tempInv;
+	*inv = tempInv;
 	return 0;
 }