Fix wrong reported item counts for inventory actions using Shift-Move (#10930)

This commit is contained in:
Lars Müller 2021-02-21 20:02:23 +01:00 committed by GitHub
parent c12e9cdcba
commit 051e4c2b00
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 43 additions and 6 deletions

@ -23,6 +23,18 @@ minetest.register_node("chest:chest", {
local inv = meta:get_inventory() local inv = meta:get_inventory()
return inv:is_empty("main") return inv:is_empty("main")
end, end,
allow_metadata_inventory_put = function(pos, listname, index, stack, player)
minetest.chat_send_player(player:get_player_name(), "Allow put: " .. stack:to_string())
return stack:get_count()
end,
allow_metadata_inventory_take = function(pos, listname, index, stack, player)
minetest.chat_send_player(player:get_player_name(), "Allow take: " .. stack:to_string())
return stack:get_count()
end,
on_metadata_inventory_put = function(pos, listname, index, stack, player)
minetest.chat_send_player(player:get_player_name(), "On put: " .. stack:to_string())
end,
on_metadata_inventory_take = function(pos, listname, index, stack, player)
minetest.chat_send_player(player:get_player_name(), "On take: " .. stack:to_string())
end,
}) })

@ -301,6 +301,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
if (!list_to->getItem(dest_i).empty()) { if (!list_to->getItem(dest_i).empty()) {
to_i = dest_i; to_i = dest_i;
apply(mgr, player, gamedef); apply(mgr, player, gamedef);
assert(move_count <= count);
count -= move_count; count -= move_count;
} }
} }
@ -352,10 +353,12 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
bool allow_swap = !list_to->itemFits(to_i, src_item, &restitem) bool allow_swap = !list_to->itemFits(to_i, src_item, &restitem)
&& restitem.count == src_item.count && restitem.count == src_item.count
&& !caused_by_move_somewhere; && !caused_by_move_somewhere;
move_count = src_item.count - restitem.count;
// Shift-click: Cannot fill this stack, proceed with next slot // Shift-click: Cannot fill this stack, proceed with next slot
if (caused_by_move_somewhere && restitem.count == src_item.count) if (caused_by_move_somewhere && move_count == 0) {
return; return;
}
if (allow_swap) { if (allow_swap) {
// Swap will affect the entire stack if it can performed. // Swap will affect the entire stack if it can performed.
@ -384,9 +387,16 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
src_can_take_count = dst_can_put_count = 0; src_can_take_count = dst_can_put_count = 0;
} else { } else {
// Take from one inventory, put into another // Take from one inventory, put into another
int src_item_count = src_item.count;
if (caused_by_move_somewhere)
// When moving somewhere: temporarily use the actual movable stack
// size to ensure correct callback execution.
src_item.count = move_count;
dst_can_put_count = allowPut(src_item, player); dst_can_put_count = allowPut(src_item, player);
src_can_take_count = allowTake(src_item, player); src_can_take_count = allowTake(src_item, player);
if (caused_by_move_somewhere)
// Reset source item count
src_item.count = src_item_count;
bool swap_expected = allow_swap; bool swap_expected = allow_swap;
allow_swap = allow_swap allow_swap = allow_swap
&& (src_can_take_count == -1 || src_can_take_count >= src_item.count) && (src_can_take_count == -1 || src_can_take_count >= src_item.count)
@ -416,12 +426,17 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
count = src_can_take_count; count = src_can_take_count;
if (dst_can_put_count != -1 && count > dst_can_put_count) if (dst_can_put_count != -1 && count > dst_can_put_count)
count = dst_can_put_count; count = dst_can_put_count;
/* Limit according to source item count */ /* Limit according to source item count */
if (count > list_from->getItem(from_i).count) if (count > list_from->getItem(from_i).count)
count = list_from->getItem(from_i).count; count = list_from->getItem(from_i).count;
/* If no items will be moved, don't go further */ /* If no items will be moved, don't go further */
if (count == 0) { if (count == 0) {
if (caused_by_move_somewhere)
// Set move count to zero, as no items have been moved
move_count = 0;
// Undo client prediction. See 'clientApply' // Undo client prediction. See 'clientApply'
if (from_inv.type == InventoryLocation::PLAYER) if (from_inv.type == InventoryLocation::PLAYER)
list_from->setModified(); list_from->setModified();
@ -438,6 +453,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
<<" list=\""<<to_list<<"\"" <<" list=\""<<to_list<<"\""
<<" i="<<to_i <<" i="<<to_i
<<std::endl; <<std::endl;
return; return;
} }
@ -455,6 +471,8 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
bool did_swap = false; bool did_swap = false;
move_count = list_from->moveItem(from_i, move_count = list_from->moveItem(from_i,
list_to, to_i, count, allow_swap, &did_swap); list_to, to_i, count, allow_swap, &did_swap);
if (caused_by_move_somewhere)
count = old_count;
assert(allow_swap == did_swap); assert(allow_swap == did_swap);
// If source is infinite, reset it's stack // If source is infinite, reset it's stack
@ -503,8 +521,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
<< std::endl; << std::endl;
// If we are inside the move somewhere loop, we don't need to report // If we are inside the move somewhere loop, we don't need to report
// anything if nothing happened (perhaps we don't need to report // anything if nothing happened
// anything for caused_by_move_somewhere == true, but this way its safer)
if (caused_by_move_somewhere && move_count == 0) if (caused_by_move_somewhere && move_count == 0)
return; return;
@ -558,7 +575,15 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
} }
mgr->setInventoryModified(from_inv); mgr->setInventoryModified(from_inv);
} else { } else {
int src_item_count = src_item.count;
if (caused_by_move_somewhere)
// When moving somewhere: temporarily use the actual movable stack
// size to ensure correct callback execution.
src_item.count = move_count;
onPutAndOnTake(src_item, player); onPutAndOnTake(src_item, player);
if (caused_by_move_somewhere)
// Reset source item count
src_item.count = src_item_count;
if (did_swap) { if (did_swap) {
// Item is now placed in source list // Item is now placed in source list
src_item = list_from->getItem(from_i); src_item = list_from->getItem(from_i);