Fix some issues with minetest.clear_craft (#8712)

* Fix some issues with minetest.clear_craft

- Fix memory leak
- Fix crafts with an output count not being cleared when clearing by
  input.
- Fix recipe list being reversed when clearing by input.

* Add CraftInput::empty()

diff --git a/games/minimal/mods/test/crafting.lua b/games/minimal/mods/test/crafting.lua
new file mode 100644 (file)
index 0000000..8964bd2
--- /dev/null
+++ b/games/minimal/mods/test/crafting.lua
@@ -0,0 +1,71 @@
+local function test_clear_craft()
+       minetest.log("info", "Testing clear_craft")
+       -- Clearing by output
+       minetest.register_craft({
+               output = "foo",
+               recipe = {{"bar"}}
+       })
+       minetest.register_craft({
+               output = "foo 4",
+               recipe = {{"foo", "bar"}}
+       })
+       assert(#minetest.get_all_craft_recipes("foo") == 2)
+       minetest.clear_craft({output="foo"})
+       assert(minetest.get_all_craft_recipes("foo") == nil)
+       -- Clearing by input
+       minetest.register_craft({
+               output = "foo 4",
+               recipe = {{"foo", "bar"}}
+       })
+       assert(#minetest.get_all_craft_recipes("foo") == 1)
+       minetest.clear_craft({recipe={{"foo", "bar"}}})
+       assert(minetest.get_all_craft_recipes("foo") == nil)
+end
+test_clear_craft()
+
+local function test_get_craft_result()
+       minetest.log("info", "test_get_craft_result()")
+       -- normal
+       local input = {
+               method = "normal",
+               width = 2,
+               items = {"", "default:coal_lump", "", "default:stick"}
+       }
+       minetest.log("info", "torch crafting input: "..dump(input))
+       local output, decremented_input = minetest.get_craft_result(input)
+       minetest.log("info", "torch crafting output: "..dump(output))
+       minetest.log("info", "torch crafting decremented input: "..dump(decremented_input))
+       assert(output.item)
+       minetest.log("info", "torch crafting output.item:to_table(): "..dump(output.item:to_table()))
+       assert(output.item:get_name() == "default:torch")
+       assert(output.item:get_count() == 4)
+       -- fuel
+       local input = {
+               method = "fuel",
+               width = 1,
+               items = {"default:coal_lump"}
+       }
+       minetest.log("info", "coal fuel input: "..dump(input))
+       local output, decremented_input = minetest.get_craft_result(input)
+       minetest.log("info", "coal fuel output: "..dump(output))
+       minetest.log("info", "coal fuel decremented input: "..dump(decremented_input))
+       assert(output.time)
+       assert(output.time > 0)
+       -- cook
+       local input = {
+               method = "cooking",
+               width = 1,
+               items = {"default:cobble"}
+       }
+       minetest.log("info", "cobble cooking input: "..dump(output))
+       local output, decremented_input = minetest.get_craft_result(input)
+       minetest.log("info", "cobble cooking output: "..dump(output))
+       minetest.log("info", "cobble cooking decremented input: "..dump(decremented_input))
+       assert(output.time)
+       assert(output.time > 0)
+       assert(output.item)
+       minetest.log("info", "cobble cooking output.item:to_table(): "..dump(output.item:to_table()))
+       assert(output.item:get_name() == "default:stone")
+       assert(output.item:get_count() == 1)
+end
+test_get_craft_result()

diff --git a/games/minimal/mods/test/init.lua b/games/minimal/mods/test/init.lua
index 73fb6e6b588697d4f6293528e77d95844d74ab0b..4e2a510869cbfd0829e872becefc1aea68ec74a8 100644 (file)
--- a/games/minimal/mods/test/init.lua
+++ b/games/minimal/mods/test/init.lua
@@ -9,5 +9,7 @@ pseudo = PseudoRandom(13)
 assert(pseudo:next() == 22290)
 assert(pseudo:next() == 13854)
 
-dofile(minetest.get_modpath("test") .. "/player.lua")
-dofile(minetest.get_modpath("test") .. "/formspec.lua")
+local modpath = minetest.get_modpath("test")
+dofile(modpath .. "/player.lua")
+dofile(modpath .. "/formspec.lua")
+dofile(modpath .. "/crafting.lua")

diff --git a/games/minimal/mods/test/player.lua b/games/minimal/mods/test/player.lua
index e66539eac973046730139b53587e69552a9df468..563d0d9859bcc0c8abceecb2d91332ce49fe6151 100644 (file)
--- a/games/minimal/mods/test/player.lua
+++ b/games/minimal/mods/test/player.lua
@@ -74,51 +74,3 @@ local function run_player_tests(player)
        minetest.chat_send_all("All tests pass!")
 end
 minetest.register_on_joinplayer(run_player_tests)
-
-
-local function test_get_craft_result()
-       minetest.log("info", "test_get_craft_result()")
-       -- normal
-       local input = {
-               method = "normal",
-               width = 2,
-               items = {"", "default:coal_lump", "", "default:stick"}
-       }
-       minetest.log("info", "torch crafting input: "..dump(input))
-       local output, decremented_input = minetest.get_craft_result(input)
-       minetest.log("info", "torch crafting output: "..dump(output))
-       minetest.log("info", "torch crafting decremented input: "..dump(decremented_input))
-       assert(output.item)
-       minetest.log("info", "torch crafting output.item:to_table(): "..dump(output.item:to_table()))
-       assert(output.item:get_name() == "default:torch")
-       assert(output.item:get_count() == 4)
-       -- fuel
-       local input = {
-               method = "fuel",
-               width = 1,
-               items = {"default:coal_lump"}
-       }
-       minetest.log("info", "coal fuel input: "..dump(input))
-       local output, decremented_input = minetest.get_craft_result(input)
-       minetest.log("info", "coal fuel output: "..dump(output))
-       minetest.log("info", "coal fuel decremented input: "..dump(decremented_input))
-       assert(output.time)
-       assert(output.time > 0)
-       -- cook
-       local input = {
-               method = "cooking",
-               width = 1,
-               items = {"default:cobble"}
-       }
-       minetest.log("info", "cobble cooking input: "..dump(output))
-       local output, decremented_input = minetest.get_craft_result(input)
-       minetest.log("info", "cobble cooking output: "..dump(output))
-       minetest.log("info", "cobble cooking decremented input: "..dump(decremented_input))
-       assert(output.time)
-       assert(output.time > 0)
-       assert(output.item)
-       minetest.log("info", "cobble cooking output.item:to_table(): "..dump(output.item:to_table()))
-       assert(output.item:get_name() == "default:stone")
-       assert(output.item:get_count() == 1)
-end
-test_get_craft_result()

diff --git a/src/craftdef.cpp b/src/craftdef.cpp
index 24b7437cbe7ec15bfaba220fd8b31f15abaf8581..0181ceb60a6305674035bf2167e73e7c89ccc91b 100644 (file)
--- a/src/craftdef.cpp
+++ b/src/craftdef.cpp
@@ -287,6 +287,15 @@ std::string craftDumpMatrix(const std::vector<ItemStack> &items,
        CraftInput
 */
 
+bool CraftInput::empty() const
+{
+       for (const auto &item : items) {
+               if (!item.empty())
+                       return false;
+       }
+       return true;
+}
+
 std::string CraftInput::dump() const
 {
        std::ostringstream os(std::ios::binary);
@@ -906,18 +915,7 @@ public:
                        std::vector<ItemStack> &output_replacement, bool decrementInput,
                        IGameDef *gamedef) const
        {
-               output.item = "";
-               output.time = 0;
-
-               // If all input items are empty, abort.
-               bool all_empty = true;
-               for (const auto &item : input.items) {
-                       if (!item.empty()) {
-                               all_empty = false;
-                               break;
-                       }
-               }
-               if (all_empty)
+               if (input.empty())
                        return false;
 
                std::vector<std::string> input_names;
@@ -1002,84 +1000,48 @@ public:
                return recipes;
        }
 
-       virtual bool clearCraftRecipesByOutput(const CraftOutput &output, IGameDef *gamedef)
+       virtual bool clearCraftsByOutput(const CraftOutput &output, IGameDef *gamedef)
        {
-               auto vec_iter = m_output_craft_definitions.find(output.item);
+               auto to_clear = m_output_craft_definitions.find(output.item);
 
-               if (vec_iter == m_output_craft_definitions.end())
+               if (to_clear == m_output_craft_definitions.end())
                        return false;
 
-               std::vector<CraftDefinition*> &vec = vec_iter->second;
-               for (auto def : vec) {
+               for (auto def : to_clear->second) {
                        // Recipes are not yet hashed at this point
-                       std::vector<CraftDefinition*> &unhashed_inputs_vec = m_craft_defs[(int) CRAFT_HASH_TYPE_UNHASHED][0];
-                       std::vector<CraftDefinition*> new_vec_by_input;
-                       /* We will preallocate necessary memory addresses, so we don't need to reallocate them later.
-                               This would save us some performance. */
-                       new_vec_by_input.reserve(unhashed_inputs_vec.size());
-                       for (auto &i2 : unhashed_inputs_vec) {
-                               if (def != i2) {
-                                       new_vec_by_input.push_back(i2);
-                               }
-                       }
-                       m_craft_defs[(int) CRAFT_HASH_TYPE_UNHASHED][0].swap(new_vec_by_input);
+                       std::vector<CraftDefinition *> &defs = m_craft_defs[(int)CRAFT_HASH_TYPE_UNHASHED][0];
+                       defs.erase(std::remove(defs.begin(), defs.end(), def), defs.end());
+                       delete def;
                }
-               m_output_craft_definitions.erase(output.item);
+               m_output_craft_definitions.erase(to_clear);
                return true;
        }
 
-       virtual bool clearCraftRecipesByInput(CraftMethod craft_method, unsigned int craft_grid_width,
-               const std::vector<std::string> &recipe, IGameDef *gamedef)
+       virtual bool clearCraftsByInput(const CraftInput &input, IGameDef *gamedef)
        {
-               bool all_empty = true;
-               for (const auto &i : recipe) {
-                       if (!i.empty()) {
-                               all_empty = false;
-                               break;
-                       }
-               }
-               if (all_empty)
+               if (input.empty())
                        return false;
 
-               CraftInput input(craft_method, craft_grid_width, craftGetItems(recipe, gamedef));
                // Recipes are not yet hashed at this point
-               std::vector<CraftDefinition*> &unhashed_inputs_vec = m_craft_defs[(int) CRAFT_HASH_TYPE_UNHASHED][0];
-               std::vector<CraftDefinition*> new_vec_by_input;
+               std::vector<CraftDefinition *> &defs = m_craft_defs[(int)CRAFT_HASH_TYPE_UNHASHED][0];
+               std::vector<CraftDefinition *> new_defs;
                bool got_hit = false;
-               for (std::vector<CraftDefinition*>::size_type
-                               i = unhashed_inputs_vec.size(); i > 0; i--) {
-                       CraftDefinition *def = unhashed_inputs_vec[i - 1];
-                       /* If the input doesn't match the recipe definition, this recipe definition later
-                               will be added back in source map. */
+               for (auto def : defs) {
                        if (!def->check(input, gamedef)) {
-                               new_vec_by_input.push_back(def);
+                               new_defs.push_back(def);
                                continue;
                        }
-                       CraftOutput output = def->getOutput(input, gamedef);
                        got_hit = true;
-                       auto vec_iter = m_output_craft_definitions.find(output.item);
-                       if (vec_iter == m_output_craft_definitions.end())
+                       std::string output = def->getOutput(input, gamedef).item;
+                       delete def;
+                       auto it = m_output_craft_definitions.find(craftGetItemName(output, gamedef));
+                       if (it == m_output_craft_definitions.end())
                                continue;
-                       std::vector<CraftDefinition*> &vec = vec_iter->second;
-                       std::vector<CraftDefinition*> new_vec_by_output;
-                       /* We will preallocate necessary memory addresses, so we don't need
-                               to reallocate them later. This would save us some performance. */
-                       new_vec_by_output.reserve(vec.size());
-                       for (auto &vec_i : vec) {
-                               /* If pointers from map by input and output are not same,
-                                       we will add 'CraftDefinition*' to a new vector. */
-                               if (def != vec_i) {
-                                       /* Adding dereferenced iterator value (which are
-                                               'CraftDefinition' reference) to a new vector. */
-                                       new_vec_by_output.push_back(vec_i);
-                               }
-                       }
-                       // Swaps assigned to current key value with new vector for output map.
-                       m_output_craft_definitions[output.item].swap(new_vec_by_output);
+                       std::vector<CraftDefinition *> &outdefs = it->second;
+                       outdefs.erase(std::remove(outdefs.begin(), outdefs.end(), def), outdefs.end());
                }
                if (got_hit)
-                       // Swaps value with new vector for input map.
-                       m_craft_defs[(int) CRAFT_HASH_TYPE_UNHASHED][0].swap(new_vec_by_input);
+                       defs.swap(new_defs);
 
                return got_hit;
        }

diff --git a/src/craftdef.h b/src/craftdef.h
index 37ae6df4353b24466961433835faa69556f5f7ba..5971a89bf80a3f356f30c6c5cb9491835c72c52a 100644 (file)
--- a/src/craftdef.h
+++ b/src/craftdef.h
@@ -80,6 +80,9 @@ struct CraftInput
                method(method_), width(width_), items(items_)
        {}
 
+       // Returns true if all items are empty.
+       bool empty() const;
+
        std::string dump() const;
 };
 
@@ -431,9 +434,8 @@ public:
        virtual std::vector<CraftDefinition*> getCraftRecipes(CraftOutput &output,
                        IGameDef *gamedef, unsigned limit=0) const=0;
 
-       virtual bool clearCraftRecipesByOutput(const CraftOutput &output, IGameDef *gamedef) = 0;
-       virtual bool clearCraftRecipesByInput(CraftMethod craft_method,
-                       unsigned int craft_grid_width, const std::vector<std::string> &recipe, IGameDef *gamedef) = 0;
+       virtual bool clearCraftsByOutput(const CraftOutput &output, IGameDef *gamedef) = 0;
+       virtual bool clearCraftsByInput(const CraftInput &input, IGameDef *gamedef) = 0;
 
        // Print crafting recipes for debugging
        virtual std::string dump() const=0;

diff --git a/src/script/lua_api/l_craft.cpp b/src/script/lua_api/l_craft.cpp
index 0899b945e99980201d10fcdbdfc836e54fb360c7..18622ee00826faf939986a6a29e32fb233788cbc 100644 (file)
--- a/src/script/lua_api/l_craft.cpp
+++ b/src/script/lua_api/l_craft.cpp
@@ -294,7 +294,7 @@ int ModApiCraft::l_clear_craft(lua_State *L)
        std::string type = getstringfield_default(L, table, "type", "shaped");
        CraftOutput c_output(output, 0);
        if (!output.empty()) {
-               if (craftdef->clearCraftRecipesByOutput(c_output, getServer(L))) {
+               if (craftdef->clearCraftsByOutput(c_output, getServer(L))) {
                        lua_pushboolean(L, true);
                        return 1;
                }
@@ -351,7 +351,13 @@ int ModApiCraft::l_clear_craft(lua_State *L)
                throw LuaError("Unknown crafting definition type: \"" + type + "\"");
        }
 
-       if (!craftdef->clearCraftRecipesByInput(method, width, recipe, getServer(L))) {
+       std::vector<ItemStack> items;
+       items.reserve(recipe.size());
+       for (const auto &item : recipe)
+               items.emplace_back(item, 1, 0, getServer(L)->idef());
+       CraftInput input(method, width, items);
+
+       if (!craftdef->clearCraftsByInput(input, getServer(L))) {
                warningstream << "No craft recipe matches input" << std::endl;
                lua_pushboolean(L, false);
                return 1;