From 05d5dc4cec37a64d01f1535c8e494a2a4a7e7036 Mon Sep 17 00:00:00 2001 From: OgelGames Date: Sun, 28 Apr 2024 08:13:44 +1000 Subject: [PATCH] Fix `InvRef` bugs and add unit tests (#14591) --- doc/lua_api.md | 1 + games/devtest/mods/unittests/init.lua | 1 + games/devtest/mods/unittests/inventory.lua | 73 ++++++++++++++++++++++ src/script/lua_api/l_inventory.cpp | 25 +++++--- 4 files changed, 92 insertions(+), 8 deletions(-) create mode 100644 games/devtest/mods/unittests/inventory.lua diff --git a/doc/lua_api.md b/doc/lua_api.md index 2ba75cbcb..43a103c3f 100644 --- a/doc/lua_api.md +++ b/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) diff --git a/games/devtest/mods/unittests/init.lua b/games/devtest/mods/unittests/init.lua index b01a6271c..6b2cd7a79 100644 --- a/games/devtest/mods/unittests/init.lua +++ b/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") -------------- diff --git a/games/devtest/mods/unittests/inventory.lua b/games/devtest/mods/unittests/inventory.lua new file mode 100644 index 000000000..8e1e16b3b --- /dev/null +++ b/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) diff --git a/src/script/lua_api/l_inventory.cpp b/src/script/lua_api/l_inventory.cpp index 422a080f9..311cc600f 100644 --- a/src/script/lua_api/l_inventory.cpp +++ b/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(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; }