From 120155f312cb1977b9a325acc7c7679103eb3800 Mon Sep 17 00:00:00 2001 From: Paul Ouellette Date: Sat, 10 Aug 2019 17:28:00 -0400 Subject: [PATCH] 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() --- games/minimal/mods/test/crafting.lua | 71 +++++++++++++++++++ games/minimal/mods/test/init.lua | 6 +- games/minimal/mods/test/player.lua | 48 ------------- src/craftdef.cpp | 100 +++++++++------------------ src/craftdef.h | 8 ++- src/script/lua_api/l_craft.cpp | 10 ++- 6 files changed, 119 insertions(+), 124 deletions(-) create mode 100644 games/minimal/mods/test/crafting.lua diff --git a/games/minimal/mods/test/crafting.lua b/games/minimal/mods/test/crafting.lua new file mode 100644 index 000000000..8964bd25a --- /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 73fb6e6b5..4e2a51086 100644 --- 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 e66539eac..563d0d985 100644 --- 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 24b7437cb..0181ceb60 100644 --- a/src/craftdef.cpp +++ b/src/craftdef.cpp @@ -287,6 +287,15 @@ std::string craftDumpMatrix(const std::vector &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 &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 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 &vec = vec_iter->second; - for (auto def : vec) { + for (auto def : to_clear->second) { // Recipes are not yet hashed at this point - std::vector &unhashed_inputs_vec = m_craft_defs[(int) CRAFT_HASH_TYPE_UNHASHED][0]; - std::vector 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 &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 &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 &unhashed_inputs_vec = m_craft_defs[(int) CRAFT_HASH_TYPE_UNHASHED][0]; - std::vector new_vec_by_input; + std::vector &defs = m_craft_defs[(int)CRAFT_HASH_TYPE_UNHASHED][0]; + std::vector new_defs; bool got_hit = false; - for (std::vector::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 &vec = vec_iter->second; - std::vector 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 &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 37ae6df43..5971a89bf 100644 --- 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 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 &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 0899b945e..18622ee00 100644 --- 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 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;