minetest
/
minetest
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix item duplication if player dies during interact callback (alternative) (#11662)

master
sfan5 2021-10-25 20:30:27 +02:00 committed by GitHub
parent d4b89eb106
commit 660e63dbae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 118 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
builtin/game/item.lua
View File

@ -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)

11
doc/lua_api.txt
View File

@ -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.

44
src/network/serverpackethandler.cpp
View File

@ -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);
}

21
src/script/cpp_api/s_item.cpp
View File

@ -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;

14
src/script/cpp_api/s_item.h
View File

@ -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);

2
src/script/lua_api/l_env.cpp
View File

@ -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;

32
src/util/Optional.h
View File

@ -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; }