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

appgurueu · web-flow · commit 051e4c2b00a3 · 2021-02-21T20:02:23.000+01:00
diff --git a/games/devtest/mods/chest/init.lua b/games/devtest/mods/chest/init.lua
@@ -23,6 +23,18 @@ minetest.register_node("chest:chest", {
 		local inv = meta:get_inventory()
 		return inv:is_empty("main")
 	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,
 })
-
-
diff --git a/src/inventorymanager.cpp b/src/inventorymanager.cpp
@@ -301,6 +301,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 			if (!list_to->getItem(dest_i).empty()) {
 				to_i = dest_i;
 				apply(mgr, player, gamedef);
+				assert(move_count <= 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)
 		&& restitem.count == src_item.count
 		&& !caused_by_move_somewhere;
+	move_count = src_item.count - restitem.count;
 
 	// 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;
+	}
 
 	if (allow_swap) {
 		// 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;
 	} else {
 		// 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);
 		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;
 		allow_swap = allow_swap
 			&& (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;
 	if (dst_can_put_count != -1 && count > dst_can_put_count)
 		count = dst_can_put_count;
+
 	/* Limit according to source item count */
 	if (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 (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'
 		if (from_inv.type == InventoryLocation::PLAYER)
 			list_from->setModified();
@@ -438,6 +453,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 				<<" list=\""<<to_list<<"\""
 				<<" i="<<to_i
 				<<std::endl;
+
 		return;
 	}
 
@@ -455,6 +471,8 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 	bool did_swap = false;
 	move_count = list_from->moveItem(from_i,
 		list_to, to_i, count, allow_swap, &did_swap);
+	if (caused_by_move_somewhere)
+		count = old_count;
 	assert(allow_swap == did_swap);
 
 	// If source is infinite, reset it's stack
@@ -503,8 +521,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 			<< std::endl;
 
 	// 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 for caused_by_move_somewhere == true, but this way its safer)
+	// anything if nothing happened
 	if (caused_by_move_somewhere && move_count == 0)
 		return;
 
@@ -558,7 +575,15 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 		}
 		mgr->setInventoryModified(from_inv);
 	} 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);
+		if (caused_by_move_somewhere)
+			// Reset source item count
+			src_item.count = src_item_count;
 		if (did_swap) {
 			// Item is now placed in source list
 			src_item = list_from->getItem(from_i);
