aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--builtin/game/item.lua50
-rw-r--r--doc/lua_api.txt11
-rw-r--r--src/network/serverpackethandler.cpp44
-rw-r--r--src/script/cpp_api/s_item.cpp21
-rw-r--r--src/script/cpp_api/s_item.h14
-rw-r--r--src/script/lua_api/l_env.cpp2
-rw-r--r--src/util/Optional.h32
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; }
generated by cgit v1.2.3 (git 2.39.1) at 2024-12-26 04:43:53 +0000