diff options
| -rw-r--r-- | builtin/game/item.lua | 50 | ||||
| -rw-r--r-- | doc/lua_api.txt | 11 | ||||
| -rw-r--r-- | src/network/serverpackethandler.cpp | 44 | ||||
| -rw-r--r-- | src/script/cpp_api/s_item.cpp | 21 | ||||
| -rw-r--r-- | src/script/cpp_api/s_item.h | 14 | ||||
| -rw-r--r-- | src/script/lua_api/l_env.cpp | 2 | ||||
| -rw-r--r-- | src/util/Optional.h | 32 |
7 files changed, 118 insertions, 56 deletions
diff --git a/builtin/game/item.lua b/builtin/game/item.lua index c495a67bd..039947584 100644 --- a/builtin/game/item.lua +++ b/builtin/game/item.lua @@ -499,34 +499,40 @@ function core.do_item_eat(hp_change, replace_with_item, itemstack, user, pointed return result end end + if itemstack:take_item():is_empty() then + return itemstack + end + local def = itemstack:get_definition() - if itemstack:take_item() ~= nil then - user:set_hp(user:get_hp() + hp_change) - - if def and def.sound and def.sound.eat then - core.sound_play(def.sound.eat, { - pos = user:get_pos(), - max_hear_distance = 16 - }, true) - end + if def and def.sound and def.sound.eat then + core.sound_play(def.sound.eat, { + pos = user:get_pos(), + max_hear_distance = 16 + }, true) + end - if replace_with_item then - if itemstack:is_empty() then - itemstack:add_item(replace_with_item) + -- Changing hp might kill the player causing mods to do who-knows-what to the + -- inventory, so do this before set_hp(). + if replace_with_item then + if itemstack:is_empty() then + itemstack:add_item(replace_with_item) + else + local inv = user:get_inventory() + -- Check if inv is null, since non-players don't have one + if inv and inv:room_for_item("main", {name=replace_with_item}) then + inv:add_item("main", replace_with_item) else - local inv = user:get_inventory() - -- Check if inv is null, since non-players don't have one - if inv and inv:room_for_item("main", {name=replace_with_item}) then - inv:add_item("main", replace_with_item) - else - local pos = user:get_pos() - pos.y = math.floor(pos.y + 0.5) - core.add_item(pos, replace_with_item) - end + local pos = user:get_pos() + pos.y = math.floor(pos.y + 0.5) + core.add_item(pos, replace_with_item) end end end - return itemstack + user:set_wielded_item(itemstack) + + user:set_hp(user:get_hp() + hp_change) + + return nil -- don't overwrite wield item a second time end function core.item_eat(hp_change, replace_with_item) diff --git a/doc/lua_api.txt b/doc/lua_api.txt index 69ac55493..e47df4686 100644 --- a/doc/lua_api.txt +++ b/doc/lua_api.txt @@ -5421,7 +5421,7 @@ Inventory * `minetest.remove_detached_inventory(name)` * Returns a `boolean` indicating whether the removal succeeded. * `minetest.do_item_eat(hp_change, replace_with_item, itemstack, user, pointed_thing)`: - returns left over ItemStack. + returns leftover ItemStack or nil to indicate no inventory change * See `minetest.item_eat` and `minetest.register_on_item_eat` Formspec @@ -7542,12 +7542,15 @@ Used by `minetest.register_node`, `minetest.register_craftitem`, and on_place = function(itemstack, placer, pointed_thing), -- When the 'place' key was pressed with the item in hand -- and a node was pointed at. - -- Shall place item and return the leftover itemstack. + -- Shall place item and return the leftover itemstack + -- or nil to not modify the inventory. -- The placer may be any ObjectRef or nil. -- default: minetest.item_place on_secondary_use = function(itemstack, user, pointed_thing), -- Same as on_place but called when not pointing at a node. + -- Function must return either nil if inventory shall not be modified, + -- or an itemstack to replace the original itemstack. -- The user may be any ObjectRef or nil. -- default: nil @@ -7559,8 +7562,8 @@ Used by `minetest.register_node`, `minetest.register_craftitem`, and on_use = function(itemstack, user, pointed_thing), -- default: nil -- When user pressed the 'punch/mine' key with the item in hand. - -- Function must return either nil if no item shall be removed from - -- inventory, or an itemstack to replace the original itemstack. + -- Function must return either nil if inventory shall not be modified, + -- or an itemstack to replace the original itemstack. -- e.g. itemstack:take_item(); return itemstack -- Otherwise, the function is free to do what it wants. -- The user may be any ObjectRef or nil. diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index dc7be0e23..d4bef3ca2 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -921,6 +921,13 @@ bool Server::checkInteractDistance(RemotePlayer *player, const f32 d, const std: return true; } +// Tiny helper to retrieve the selected item into an Optional +static inline void getWieldedItem(const PlayerSAO *playersao, Optional<ItemStack> &ret) +{ + ret = ItemStack(); + playersao->getWieldedItem(&(*ret)); +} + void Server::handleCommand_Interact(NetworkPacket *pkt) { /* @@ -1228,14 +1235,17 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) // Place block or right-click object case INTERACT_PLACE: { - ItemStack selected_item; - playersao->getWieldedItem(&selected_item, nullptr); + Optional<ItemStack> selected_item; + getWieldedItem(playersao, selected_item); // Reset build time counter if (pointed.type == POINTEDTHING_NODE && - selected_item.getDefinition(m_itemdef).type == ITEM_NODE) + selected_item->getDefinition(m_itemdef).type == ITEM_NODE) getClient(peer_id)->m_time_from_building = 0.0; + const bool had_prediction = !selected_item->getDefinition(m_itemdef). + node_placement_prediction.empty(); + if (pointed.type == POINTEDTHING_OBJECT) { // Right click object @@ -1248,11 +1258,9 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) << pointed_object->getDescription() << std::endl; // Do stuff - if (m_script->item_OnSecondaryUse( - selected_item, playersao, pointed)) { - if (playersao->setWieldedItem(selected_item)) { + if (m_script->item_OnSecondaryUse(selected_item, playersao, pointed)) { + if (selected_item.has_value() && playersao->setWieldedItem(*selected_item)) SendInventory(playersao, true); - } } pointed_object->rightClick(playersao); @@ -1260,7 +1268,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) // Placement was handled in lua // Apply returned ItemStack - if (playersao->setWieldedItem(selected_item)) + if (selected_item.has_value() && playersao->setWieldedItem(*selected_item)) SendInventory(playersao, true); } @@ -1272,8 +1280,7 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) RemoteClient *client = getClient(peer_id); v3s16 blockpos = getNodeBlockPos(pointed.node_abovesurface); v3s16 blockpos2 = getNodeBlockPos(pointed.node_undersurface); - if (!selected_item.getDefinition(m_itemdef - ).node_placement_prediction.empty()) { + if (had_prediction) { client->SetBlockNotSent(blockpos); if (blockpos2 != blockpos) client->SetBlockNotSent(blockpos2); @@ -1287,15 +1294,15 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) } // action == INTERACT_PLACE case INTERACT_USE: { - ItemStack selected_item; - playersao->getWieldedItem(&selected_item, nullptr); + Optional<ItemStack> selected_item; + getWieldedItem(playersao, selected_item); - actionstream << player->getName() << " uses " << selected_item.name + actionstream << player->getName() << " uses " << selected_item->name << ", pointing at " << pointed.dump() << std::endl; if (m_script->item_OnUse(selected_item, playersao, pointed)) { // Apply returned ItemStack - if (playersao->setWieldedItem(selected_item)) + if (selected_item.has_value() && playersao->setWieldedItem(*selected_item)) SendInventory(playersao, true); } @@ -1304,16 +1311,17 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) // Rightclick air case INTERACT_ACTIVATE: { - ItemStack selected_item; - playersao->getWieldedItem(&selected_item, nullptr); + Optional<ItemStack> selected_item; + getWieldedItem(playersao, selected_item); actionstream << player->getName() << " activates " - << selected_item.name << std::endl; + << selected_item->name << std::endl; pointed.type = POINTEDTHING_NOTHING; // can only ever be NOTHING if (m_script->item_OnSecondaryUse(selected_item, playersao, pointed)) { - if (playersao->setWieldedItem(selected_item)) + // Apply returned ItemStack + if (selected_item.has_value() && playersao->setWieldedItem(*selected_item)) SendInventory(playersao, true); } diff --git a/src/script/cpp_api/s_item.cpp b/src/script/cpp_api/s_item.cpp index 48dce14f3..b1916070e 100644 --- a/src/script/cpp_api/s_item.cpp +++ b/src/script/cpp_api/s_item.cpp @@ -59,13 +59,14 @@ bool ScriptApiItem::item_OnDrop(ItemStack &item, return true; } -bool ScriptApiItem::item_OnPlace(ItemStack &item, +bool ScriptApiItem::item_OnPlace(Optional<ItemStack> &ret_item, ServerActiveObject *placer, const PointedThing &pointed) { SCRIPTAPI_PRECHECKHEADER int error_handler = PUSH_ERROR_HANDLER(L); + const ItemStack &item = *ret_item; // Push callback function on stack if (!getItemCallback(item.name.c_str(), "on_place")) return false; @@ -82,22 +83,25 @@ bool ScriptApiItem::item_OnPlace(ItemStack &item, PCALL_RES(lua_pcall(L, 3, 1, error_handler)); if (!lua_isnil(L, -1)) { try { - item = read_item(L, -1, getServer()->idef()); + ret_item = read_item(L, -1, getServer()->idef()); } catch (LuaError &e) { throw WRAP_LUAERROR(e, "item=" + item.name); } + } else { + ret_item = nullopt; } lua_pop(L, 2); // Pop item and error handler return true; } -bool ScriptApiItem::item_OnUse(ItemStack &item, +bool ScriptApiItem::item_OnUse(Optional<ItemStack> &ret_item, ServerActiveObject *user, const PointedThing &pointed) { SCRIPTAPI_PRECHECKHEADER int error_handler = PUSH_ERROR_HANDLER(L); + const ItemStack &item = *ret_item; // Push callback function on stack if (!getItemCallback(item.name.c_str(), "on_use")) return false; @@ -109,22 +113,25 @@ bool ScriptApiItem::item_OnUse(ItemStack &item, PCALL_RES(lua_pcall(L, 3, 1, error_handler)); if(!lua_isnil(L, -1)) { try { - item = read_item(L, -1, getServer()->idef()); + ret_item = read_item(L, -1, getServer()->idef()); } catch (LuaError &e) { throw WRAP_LUAERROR(e, "item=" + item.name); } + } else { + ret_item = nullopt; } lua_pop(L, 2); // Pop item and error handler return true; } -bool ScriptApiItem::item_OnSecondaryUse(ItemStack &item, +bool ScriptApiItem::item_OnSecondaryUse(Optional<ItemStack> &ret_item, ServerActiveObject *user, const PointedThing &pointed) { SCRIPTAPI_PRECHECKHEADER int error_handler = PUSH_ERROR_HANDLER(L); + const ItemStack &item = *ret_item; if (!getItemCallback(item.name.c_str(), "on_secondary_use")) return false; @@ -134,10 +141,12 @@ bool ScriptApiItem::item_OnSecondaryUse(ItemStack &item, PCALL_RES(lua_pcall(L, 3, 1, error_handler)); if (!lua_isnil(L, -1)) { try { - item = read_item(L, -1, getServer()->idef()); + ret_item = read_item(L, -1, getServer()->idef()); } catch (LuaError &e) { throw WRAP_LUAERROR(e, "item=" + item.name); } + } else { + ret_item = nullopt; } lua_pop(L, 2); // Pop item and error handler return true; diff --git a/src/script/cpp_api/s_item.h b/src/script/cpp_api/s_item.h index 25a3501f9..5015d8bd4 100644 --- a/src/script/cpp_api/s_item.h +++ b/src/script/cpp_api/s_item.h @@ -21,6 +21,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "cpp_api/s_base.h" #include "irr_v3d.h" +#include "util/Optional.h" struct PointedThing; struct ItemStack; @@ -35,13 +36,20 @@ class ScriptApiItem : virtual public ScriptApiBase { public: + /* + * Functions with Optional<ItemStack> are for callbacks where Lua may + * want to prevent the engine from modifying the inventory after it's done. + * This has a longer backstory where on_use may need to empty the player's + * inventory without the engine interfering (see issue #6546). + */ + bool item_OnDrop(ItemStack &item, ServerActiveObject *dropper, v3f pos); - bool item_OnPlace(ItemStack &item, + bool item_OnPlace(Optional<ItemStack> &item, ServerActiveObject *placer, const PointedThing &pointed); - bool item_OnUse(ItemStack &item, + bool item_OnUse(Optional<ItemStack> &item, ServerActiveObject *user, const PointedThing &pointed); - bool item_OnSecondaryUse(ItemStack &item, + bool item_OnSecondaryUse(Optional<ItemStack> &item, ServerActiveObject *user, const PointedThing &pointed); bool item_OnCraft(ItemStack &item, ServerActiveObject *user, const InventoryList *old_craft_grid, const InventoryLocation &craft_inv); diff --git a/src/script/lua_api/l_env.cpp b/src/script/lua_api/l_env.cpp index 98f8861fa..2c709d31b 100644 --- a/src/script/lua_api/l_env.cpp +++ b/src/script/lua_api/l_env.cpp @@ -477,7 +477,7 @@ int ModApiEnvMod::l_place_node(lua_State *L) return 1; } // Create item to place - ItemStack item(ndef->get(n).name, 1, 0, idef); + Optional<ItemStack> item = ItemStack(ndef->get(n).name, 1, 0, idef); // Make pointed position PointedThing pointed; pointed.type = POINTEDTHING_NODE; diff --git a/src/util/Optional.h b/src/util/Optional.h index 9c2842b43..eda7fff89 100644 --- a/src/util/Optional.h +++ b/src/util/Optional.h @@ -19,6 +19,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #pragma once +#include <utility> #include "debug.h" struct nullopt_t @@ -43,18 +44,38 @@ class Optional public: Optional() noexcept {} Optional(nullopt_t) noexcept {} + Optional(const T &value) noexcept : m_has_value(true), m_value(value) {} + Optional(T &&value) noexcept : m_has_value(true), m_value(std::move(value)) {} + Optional(const Optional<T> &other) noexcept : m_has_value(other.m_has_value), m_value(other.m_value) + {} + Optional(Optional<T> &&other) noexcept : + m_has_value(other.m_has_value), m_value(std::move(other.m_value)) { + other.m_has_value = false; } - void operator=(nullopt_t) noexcept { m_has_value = false; } + Optional<T> &operator=(nullopt_t) noexcept { m_has_value = false; return *this; } - void operator=(const Optional<T> &other) noexcept + Optional<T> &operator=(const Optional<T> &other) noexcept { + if (&other == this) + return *this; m_has_value = other.m_has_value; m_value = other.m_value; + return *this; + } + + Optional<T> &operator=(Optional<T> &&other) noexcept + { + if (&other == this) + return *this; + m_has_value = other.m_has_value; + m_value = std::move(other.m_value); + other.m_has_value = false; + return *this; } T &value() @@ -71,6 +92,13 @@ public: const T &value_or(const T &def) const { return m_has_value ? m_value : def; } + // Unchecked access consistent with std::optional + T* operator->() { return &m_value; } + const T* operator->() const { return &m_value; } + + T& operator*() { return m_value; } + const T& operator*() const { return m_value; } + bool has_value() const noexcept { return m_has_value; } explicit operator bool() const { return m_has_value; } |