diff --git a/src/activity_actor.cpp b/src/activity_actor.cpp index 12fffb89680eb..53ec8e8b8f27b 100644 --- a/src/activity_actor.cpp +++ b/src/activity_actor.cpp @@ -3496,6 +3496,12 @@ void unload_activity_actor::unload( Character &who, item_location &target ) it.on_contents_changed(); who.invalidate_weight_carried_cache(); handler.handle_by( who ); + // Warning: the above call to `contents_change_handler::handle_by` will + // call `Character::handle_contents_changed`, which might invalidate items + // and item_locations. See description for `::handle_contents_changed` + // in character.h . + // Therefore, it is important that we don't use `target` or `it` after here. + break; } } diff --git a/src/item_pocket.cpp b/src/item_pocket.cpp index 3444ca72cf722..c784ff2517f8b 100644 --- a/src/item_pocket.cpp +++ b/src/item_pocket.cpp @@ -803,6 +803,8 @@ void item_pocket::handle_liquid_or_spill( Character &guy, const item *avoid ) item i_copy( *iter ); guy.i_add_or_drop( i_copy, 1, avoid, &*iter ); iter = contents.erase( iter ); + guy.add_msg_if_player( m_warning, _( "The %s falls out of the %s." ), i_copy.display_name(), + get_name() ); } } } diff --git a/tests/item_pocket_test.cpp b/tests/item_pocket_test.cpp index ab3fbc0529216..0a6e1a69e16c5 100644 --- a/tests/item_pocket_test.cpp +++ b/tests/item_pocket_test.cpp @@ -2873,3 +2873,56 @@ TEST_CASE( "pocket_mods", "[pocket][toolmod][gunmod]" ) } } } + +// Reproduce previous segfault from https://github.com/CleverRaven/Cataclysm-DDA/issues/75156 +TEST_CASE( "unload_from_spillable_container", "[item][pocket]" ) +{ + clear_avatar(); + clear_map(); + avatar &u = get_avatar(); + map &here = get_map(); + item pill( "tums" ); // "antacid pill" + REQUIRE( u.wear_item( item( "backpack" ) ) ); + item_location backpack_loc = u.top_items_loc().front(); + item *backpack = backpack_loc.get_item(); + GIVEN( "spillable container contains two pillbottles, one with 4 pills and one with 1 pill" ) { + // Starting inventory looks like: + // backpack > + // steel_pan > + // bottle_plastic_small > antacid tablet (1) + // bottle_plastic_small > antacid tablet (4) + // It's a bit odd that we managed to put bottles into the frying + // pan in the first place, since the pan would normally reject that + // with a message stating that it would spill. + REQUIRE( backpack->put_in( item( "steel_pan" ), pocket_type::CONTAINER ).success() ); + item *steelpan = backpack->all_items_top().front(); + REQUIRE( steelpan->put_in( pill.in_its_container( 1 ), pocket_type::CONTAINER ).success() ); + REQUIRE( steelpan->put_in( pill.in_its_container( 4 ), pocket_type::CONTAINER ).success() ); + WHEN( "unload the pillbottle that only has one pill" ) { + item *bottle1 = steelpan->all_items_top().front(); + item_location steelpan_loc( backpack_loc, steelpan ); + item_location bottle1_loc( steelpan_loc, bottle1 ); + unload_activity_actor::unload( u, bottle1_loc ); + THEN( "pill is unloaded into bottle that previously had 4 pills, remaining empty bottle is kept" ) { + // Expected inventory after unloading should be: + // backpack > + // steel_pan + // bottle_plastic_small + // bottle_plastic_small > antacid tablet (5) + CHECK( here.i_at( u.pos_bub() ).size() == 0 ); // no items spilled to ground + CHECK( u.top_items_loc().size() == 1 ); // backpack is still only inventory item + const std::list backpack_items_list = backpack->all_items_top(); + const std::vector backpack_items_vec( backpack_items_list.begin(), + backpack_items_list.end() ); + CHECK( backpack_items_vec.size() == 3 ); + CHECK( backpack_items_vec[0]->typeId().str() == "steel_pan" ); + CHECK( backpack_items_vec[0]->empty() ); + CHECK( backpack_items_vec[1]->typeId().str() == "bottle_plastic_small" ); + CHECK( backpack_items_vec[1]->empty() ); + CHECK( backpack_items_vec[2]->typeId().str() == "bottle_plastic_small" ); + CHECK( backpack_items_vec[2]->all_items_top().size() == 5 ); + CHECK_FALSE( static_cast( bottle1_loc ) ); + } + } + } +}