BRNSystems/minetest
1
0
Fork 0
forked from Mirrorlandia_minetest/minetest
Code Pull Requests 1 Activity

Inventory: prevent item loss when stacking oversized ItemStacks (#14072)

Browse Source
This commit is contained in:
SmallJoker 2023-12-15 10:24:04 +01:00 committed by GitHub
parent c871b6dd4e
commit 94a54375e2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 45 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/inventory.cpp
View File

@ -777,20 +777,18 @@ u32 InventoryList::moveItem(u32 i, InventoryList *dest, u32 dest_i,
return 0; return 0;
// Try to add the item to destination list // Try to add the item to destination list
u32 oldcount = item1.count; count = item1.count;
item1 = dest->addItem(dest_i, item1); item1 = dest->addItem(dest_i, item1);
// If something is returned, the item was not fully added // If something is returned, the item was not fully added
if (!item1.empty()) { if (!item1.empty()) {
// If olditem is returned, nothing was added. bool nothing_added = (item1.count == count);
bool nothing_added = (item1.count == oldcount);
// If something else is returned, part of the item was left unadded. // Add any leftover stack back to the source stack.
// Add the other part back to the source item item1.add(getItem(i).count); // leftover + source count
addItem(i, item1); changeItem(i, item1); // do NOT use addItem to allow oversized stacks!
// If olditem is returned, nothing was added. // Swap if no items could be moved
// Swap the items
if (nothing_added && swap_if_needed) { if (nothing_added && swap_if_needed) {
// Tell that we swapped // Tell that we swapped
if (did_swap != NULL) { if (did_swap != NULL) {
@ -802,9 +800,10 @@ u32 InventoryList::moveItem(u32 i, InventoryList *dest, u32 dest_i,
ItemStack item2 = dest->changeItem(dest_i, item1); ItemStack item2 = dest->changeItem(dest_i, item1);
// Put item from destination list to the source list // Put item from destination list to the source list
changeItem(i, item2); changeItem(i, item2);
item1.clear(); // no leftover
} }
} }
return (oldcount - item1.count); return (count - item1.count);
} }
void InventoryList::checkResizeLock() void InventoryList::checkResizeLock()

4
src/unittest/helpers/helper_moveaction.lua
View File

@ -1,4 +1,8 @@
minetest.register_allow_player_inventory_action(function(player, action, inventory, info) minetest.register_allow_player_inventory_action(function(player, action, inventory, info)
if action == "move" then
return info.count
end
if info.stack:get_name() == "default:water" then if info.stack:get_name() == "default:water" then
return 0 return 0
end end

35
src/unittest/test_moveaction.cpp
View File

@ -36,6 +36,7 @@ public:
void runTests(IGameDef *gamedef); void runTests(IGameDef *gamedef);
void testMove(ServerActiveObject *obj, IGameDef *gamedef); void testMove(ServerActiveObject *obj, IGameDef *gamedef);
void testMoveFillStack(ServerActiveObject *obj, IGameDef *gamedef);
void testMoveSomewhere(ServerActiveObject *obj, IGameDef *gamedef); void testMoveSomewhere(ServerActiveObject *obj, IGameDef *gamedef);
void testMoveUnallowed(ServerActiveObject *obj, IGameDef *gamedef); void testMoveUnallowed(ServerActiveObject *obj, IGameDef *gamedef);
void testMovePartial(ServerActiveObject *obj, IGameDef *gamedef); void testMovePartial(ServerActiveObject *obj, IGameDef *gamedef);
@ -51,14 +52,24 @@ void TestMoveAction::runTests(IGameDef *gamedef)
MockServer server; MockServer server;
ServerScripting server_scripting(&server); ServerScripting server_scripting(&server);
server_scripting.loadMod(Server::getBuiltinLuaPath() + DIR_DELIM "init.lua", BUILTIN_MOD_NAME); try {
server_scripting.loadMod(std::string(HELPERS_PATH) + DIR_DELIM "helper_moveaction.lua", BUILTIN_MOD_NAME); server_scripting.loadMod(Server::getBuiltinLuaPath() + DIR_DELIM "init.lua", BUILTIN_MOD_NAME);
server_scripting.loadMod(
std::string(HELPERS_PATH) + DIR_DELIM "helper_moveaction.lua", BUILTIN_MOD_NAME
);
} catch (ModError &e) {
// Print backtrace in case of syntax errors
rawstream << e.what() << std::endl;
num_tests_failed = 1;
return;
}
MetricsBackend mb; MetricsBackend mb;
ServerEnvironment server_env(nullptr, &server_scripting, &server, "", &mb); ServerEnvironment server_env(nullptr, &server_scripting, &server, "", &mb);
MockServerActiveObject obj(&server_env); MockServerActiveObject obj(&server_env);
TEST(testMove, &obj, gamedef); TEST(testMove, &obj, gamedef);
TEST(testMoveFillStack, &obj, gamedef);
TEST(testMoveSomewhere, &obj, gamedef); TEST(testMoveSomewhere, &obj, gamedef);
TEST(testMoveUnallowed, &obj, gamedef); TEST(testMoveUnallowed, &obj, gamedef);
TEST(testMovePartial, &obj, gamedef); TEST(testMovePartial, &obj, gamedef);
@ -95,6 +106,26 @@ void TestMoveAction::testMove(ServerActiveObject *obj, IGameDef *gamedef)
UASSERT(inv.p2.getList("main")->getItem(0).getItemString() == "default:stone 20"); UASSERT(inv.p2.getList("main")->getItem(0).getItemString() == "default:stone 20");
} }
void TestMoveAction::testMoveFillStack(ServerActiveObject *obj, IGameDef *gamedef)
{
MockInventoryManager inv(gamedef);
auto list = inv.p1.addList("main", 10);
list->changeItem(0, parse_itemstack("default:stone 209"));
list->changeItem(1, parse_itemstack("default:stone 90")); // 9 free slots
apply_action("Move 209 player:p1 main 0 player:p1 main 1", &inv, obj, gamedef);
UASSERT(list->getItem(0).getItemString() == "default:stone 200");
UASSERT(list->getItem(1).getItemString() == "default:stone 99");
// Trigger stack swap
apply_action("Move 200 player:p1 main 0 player:p1 main 1", &inv, obj, gamedef);
UASSERT(list->getItem(0).getItemString() == "default:stone 99");
UASSERT(list->getItem(1).getItemString() == "default:stone 200");
}
void TestMoveAction::testMoveSomewhere(ServerActiveObject *obj, IGameDef *gamedef) void TestMoveAction::testMoveSomewhere(ServerActiveObject *obj, IGameDef *gamedef)
{ {
MockInventoryManager inv(gamedef); MockInventoryManager inv(gamedef);