From 7bbb9b066a8cc079512ddd3e6b32475309f49fca Mon Sep 17 00:00:00 2001 From: est31 Date: Sun, 19 Jul 2015 02:27:12 +0200 Subject: [PATCH] MoveItemSomewhere double bugfix -> Fix bug where MoveSomewhere from an infinite source would fill the destination inventory with copies of itself. -> Fix bug where MoveSomewhere would needlessly call callbacks. -> Remove trailing whitespaces --- src/inventorymanager.cpp | 64 ++++++++++++++++++++++++---------------- src/inventorymanager.h | 16 +++++----- 2 files changed, 47 insertions(+), 33 deletions(-) diff --git a/src/inventorymanager.cpp b/src/inventorymanager.cpp index d23d1529d..bf5a7dd9d 100644 --- a/src/inventorymanager.cpp +++ b/src/inventorymanager.cpp @@ -171,7 +171,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame { Inventory *inv_from = mgr->getInventory(from_inv); Inventory *inv_to = mgr->getInventory(to_inv); - + if (!inv_from) { infostream << "IMoveAction::apply(): FAIL: source inventory not found: " << "from_inv=\""< src_can_take_count) @@ -348,7 +348,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame /* 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) { @@ -379,21 +379,28 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame list_to, to_i, count, !caused_by_move_somewhere); // If source is infinite, reset it's stack - if(src_can_take_count == -1){ - // If destination stack is of different type and there are leftover - // items, attempt to put the leftover items to a different place in the - // destination inventory. - // The client-side GUI will try to guess if this happens. - if(from_stack_was.name != to_stack_was.name){ - for(u32 i=0; igetSize(); i++){ - if(list_to->getItem(i).empty()){ - list_to->changeItem(i, to_stack_was); - break; + if (src_can_take_count == -1) { + // For the caused_by_move_somewhere == true case we didn't force-put the item, + // which guarantees there is no leftover, and code below would duplicate the + // (not replaced) to_stack_was item. + if (!caused_by_move_somewhere) { + // If destination stack is of different type and there are leftover + // items, attempt to put the leftover items to a different place in the + // destination inventory. + // The client-side GUI will try to guess if this happens. + if (from_stack_was.name != to_stack_was.name) { + for (u32 i = 0; i < list_to->getSize(); i++) { + if (list_to->getItem(i).empty()) { + list_to->changeItem(i, to_stack_was); + break; + } } } } - list_from->deleteItem(from_i); - list_from->addItem(from_i, from_stack_was); + if (move_count > 0) { + list_from->deleteItem(from_i); + list_from->addItem(from_i, from_stack_was); + } } // If destination is infinite, reset it's stack and take count from source if(dst_can_put_count == -1){ @@ -416,6 +423,13 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame << " i=" << to_i << 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) + if (caused_by_move_somewhere && move_count == 0) { + return; + } + /* Record rollback information */ @@ -454,7 +468,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame /* Report move to endpoints */ - + /* Detached inventories */ // Both endpoints are same detached @@ -507,7 +521,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame from_inv.p, from_list, from_i, src_item, player); } } - + mgr->setInventoryModified(from_inv, false); if(inv_from != inv_to) mgr->setInventoryModified(to_inv, false); @@ -567,7 +581,7 @@ IDropAction::IDropAction(std::istream &is) void IDropAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGameDef *gamedef) { Inventory *inv_from = mgr->getInventory(from_inv); - + if(!inv_from){ infostream<<"IDropAction::apply(): FAIL: source inventory not found: " <<"from_inv=\""<getItem(from_i); @@ -644,7 +658,7 @@ void IDropAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame infostream<<"Actually dropped no items"<