Implement metadata-aware version of InvRef:remove_item() (#15771)

builtin/game/features.lua

@@ -45,6 +45,7 @@ core.features = {
 	abm_without_neighbors = true,
 	biome_weights = true,
 	particle_blend_clip = true,
+	remove_item_match_meta = true,
 }
 
 function core.has_feature(arg)

doc/lua_api.md

@@ -5689,6 +5689,8 @@ Utilities
       biome_weights = true,
       -- Particles can specify a "clip" blend mode (5.11.0)
       particle_blend_clip = true,
+      -- The `match_meta` optional parameter is available for `InvRef:remove_item()` (5.12.0)
+      remove_item_match_meta = true,
   }
   ```
 
@@ -7872,13 +7874,15 @@ An `InvRef` is a reference to an inventory.
   can be fully added to the list
 * `contains_item(listname, stack, [match_meta])`: returns `true` if
   the stack of items can be fully taken from the list.
-  If `match_meta` is false, only the items' names are compared
-  (default: `false`).
-* `remove_item(listname, stack)`: take as many items as specified from the
-  list, returns the items that were actually removed (as an `ItemStack`)
-  -- note that any item metadata is ignored, so attempting to remove a specific
-  unique item this way will likely remove the wrong one -- to do that use
-  `set_stack` with an empty `ItemStack`.
+    * If `match_meta` is `true`, item metadata is also considered when comparing
+      items. Otherwise, only the items names are compared. Default: `false`
+    * The method ignores wear.
+* `remove_item(listname, stack, [match_meta])`: take as many items as specified from the
+  list, returns the items that were actually removed (as an `ItemStack`).
+    * If `match_meta` is `true` (available since feature `remove_item_match_meta`),
+      item metadata is also considered when comparing items. Otherwise, only the
+      items names are compared. Default: `false`
+    * The method ignores wear.
 * `get_location()`: returns a location compatible to
   `core.get_inventory(location)`.
     * returns `{type="undefined"}` in case location is not known

games/devtest/mods/unittests/inventory.lua

@@ -1,10 +1,11 @@
-
-local item_with_meta = ItemStack({name = "air", meta = {test = "abc"}})
+local function get_stack_with_meta(count)
+	return ItemStack({name = "air", count = count, meta = {test = "abc"}})
+end
 
 local test_list = {
 	ItemStack("air"),
 	ItemStack(""),
-	ItemStack(item_with_meta),
+	ItemStack(get_stack_with_meta(1)),
 }
 
 local function compare_lists(a, b)
@@ -34,12 +35,12 @@ local function test_inventory()
 	assert(not inv:set_width("test", -1))
 
 	inv:set_stack("test", 1, "air")
-	inv:set_stack("test", 3, item_with_meta)
+	inv:set_stack("test", 3, get_stack_with_meta(1))
 	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:add_item("test", get_stack_with_meta(1)) == ItemStack())
 	assert(inv:get_stack("test", 1) == ItemStack("air 2"))
 
 	assert(inv:room_for_item("test", "air 99"))
@@ -48,16 +49,28 @@ local function test_inventory()
 	inv:set_stack("test", 2, "")
 
 	assert(inv:contains_item("test", "air"))
+	assert(inv:contains_item("test", "air 4"))
+	assert(not inv:contains_item("test", "air 5"))
 	assert(not inv:contains_item("test", "air 99"))
-	assert(inv:contains_item("test", item_with_meta, true))
+	assert(inv:contains_item("test", "air 2", true))
+	assert(not inv:contains_item("test", "air 3", true))
+	assert(inv:contains_item("test", get_stack_with_meta(2), true))
+	assert(not inv:contains_item("test", get_stack_with_meta(3), 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") == get_stack_with_meta(1))
+	assert(inv:remove_item("test", "air 2") == get_stack_with_meta(2))
 	assert(inv:remove_item("test", "air") == ItemStack("air"))
 	assert(inv:is_empty("test"))
 
+	inv:set_stack("test", 1, "air 3")
+	inv:set_stack("test", 3, get_stack_with_meta(2))
+	assert(inv:remove_item("test", "air 4", true) == ItemStack("air 3"))
+	inv:set_stack("test", 1, "air 3")
+	assert(inv:remove_item("test", get_stack_with_meta(3), true) == get_stack_with_meta(2))
+	assert(inv:remove_item("test", "air 3", true) == ItemStack("air 3"))
+	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})

src/inventory.cpp

@@ -693,11 +693,11 @@ bool InventoryList::containsItem(const ItemStack &item, bool match_meta) const
 	return false;
 }
 
-ItemStack InventoryList::removeItem(const ItemStack &item)
+ItemStack InventoryList::removeItem(const ItemStack &item, bool match_meta)
 {
 	ItemStack removed;
 	for (auto i = m_items.rbegin(); i != m_items.rend(); ++i) {
-		if (i->name == item.name) {
+		if (i->name == item.name && (!match_meta || i->metadata == item.metadata)) {
 			u32 still_to_remove = item.count - removed.count;
 			ItemStack leftover = removed.addItem(i->takeItem(still_to_remove),
 					m_itemdef);

src/inventory.h

@@ -262,7 +262,7 @@ public:
 	// If not as many items exist as requested, removes as
 	// many as possible.
 	// Returns the items that were actually removed.
-	ItemStack removeItem(const ItemStack &item);
+	ItemStack removeItem(const ItemStack &item, bool match_meta);
 
 	// Takes some items from a slot.
 	// If there are not enough, takes as many as it can.

src/script/lua_api/l_inventory.cpp

@@ -319,9 +319,7 @@ int InvRef::l_contains_item(lua_State *L)
 	const char *listname = luaL_checkstring(L, 2);
 	ItemStack item = read_item(L, 3, getServer(L)->idef());
 	InventoryList *list = getlist(L, ref, listname);
-	bool match_meta = false;
-	if (lua_isboolean(L, 4))
-		match_meta = readParam<bool>(L, 4);
+	bool match_meta = readParam<bool>(L, 4, false);
 	if (list) {
 		lua_pushboolean(L, list->containsItem(item, match_meta));
 	} else {
@@ -330,7 +328,7 @@ int InvRef::l_contains_item(lua_State *L)
 	return 1;
 }
 
-// remove_item(self, listname, itemstack or itemstring or table or nil) -> itemstack
+// remove_item(self, listname, itemstack or itemstring or table or nil, [match_meta]) -> itemstack
 // Returns the items that were actually removed
 int InvRef::l_remove_item(lua_State *L)
 {
@@ -339,8 +337,9 @@ int InvRef::l_remove_item(lua_State *L)
 	const char *listname = luaL_checkstring(L, 2);
 	ItemStack item = read_item(L, 3, getServer(L)->idef());
 	InventoryList *list = getlist(L, ref, listname);
+	bool match_meta = readParam<bool>(L, 4, false);
 	if(list){
-		ItemStack removed = list->removeItem(item);
+		ItemStack removed = list->removeItem(item, match_meta);
 		if(!removed.empty())
 			reportInventoryChange(L, ref);
 		LuaItemStack::create(L, removed);