Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nesting big bag tests and fixes for disapearing items #60151

Merged
merged 3 commits into from
Aug 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions data/mods/TEST_DATA/items.json
Original file line number Diff line number Diff line change
Expand Up @@ -1699,6 +1699,25 @@
"flags": [ "BELTED" ],
"armor": [ { "encumbrance": [ 2, 15 ], "coverage": 30, "covers": [ "torso" ] } ]
},
{
"id": "test_mini_backpack",
"type": "ARMOR",
"name": "TEST backpack",
"description": "A small backpack designed for testing nesting issues. Good storage for a little encumbrance.",
"weight": "633 g",
"volume": "2 L",
"price": 3900,
"price_postapoc": 16000,
"material": [ "cotton" ],
"symbol": "[",
"looks_like": "ragpouch",
"color": "green",
"pocket_data": [ { "pocket_type": "CONTAINER", "max_contains_volume": "3 L", "max_contains_weight": "30 kg", "moves": 300 } ],
"warmth": 6,
"material_thickness": 2,
"flags": [ "BELTED" ],
"armor": [ { "encumbrance": [ 2, 15 ], "coverage": 30, "covers": [ "torso" ] } ]
},
{
"id": "test_briefcase",
"repairs_like": "backpack_hiking",
Expand Down
13 changes: 10 additions & 3 deletions src/item.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10213,7 +10213,7 @@ ret_val<void> item::is_compatible( const item &it ) const

ret_val<void> item::can_contain( const item &it, const bool nested,
const bool ignore_rigidity, const bool ignore_pkt_settings,
const item_location &parent_it ) const
const item_location &parent_it, units::volume remaining_parent_volume ) const
{
if( this == &it || ( parent_it.where() != item_location::type::invalid &&
this == parent_it.get_item() ) ) {
Expand All @@ -10229,10 +10229,17 @@ ret_val<void> item::can_contain( const item &it, const bool nested,
it.contents.bigger_on_the_inside( it.volume() ) ) {
return ret_val<void>::make_failure();
}

for( const item_pocket *pkt : contents.get_all_contained_pockets() ) {
if( pkt->empty() ) {
continue;
}

// early exit for max length no nested item is gonna fix this
if( pkt->max_containable_length() < it.length() ) {
continue;
}

// If the current pocket has restrictions or blacklists the item,
// try the nested pocket regardless of whether it's soft or rigid.
const bool ignore_rigidity =
Expand All @@ -10243,15 +10250,15 @@ ret_val<void> item::can_contain( const item &it, const bool nested,
continue;
}
if( internal_it->can_contain( it, true, ignore_rigidity, ignore_pkt_settings,
parent_it ).success() ) {
parent_it, pkt->remaining_volume() ).success() ) {
return ret_val<void>::make_success();
}
}
}

return nested && !ignore_rigidity ?
contents.can_contain_rigid( it, ignore_pkt_settings ) :
contents.can_contain( it, ignore_pkt_settings );
contents.can_contain( it, ignore_pkt_settings, remaining_parent_volume );
}

bool item::can_contain( const itype &tp ) const
Expand Down
5 changes: 4 additions & 1 deletion src/item.h
Original file line number Diff line number Diff line change
Expand Up @@ -1546,12 +1546,15 @@ class item : public visitable
* @param it the item being put in
* @param nested whether or not the current call is nested (used recursively).
* @param ignore_pkt_settings whether to ignore pocket autoinsert settings
* @param remaining_parent_volume the ammount of space in the parent pocket,
* needed to make sure we dont try to nest items which can't fit in the nested pockets
*/
/*@{*/
ret_val<void> can_contain( const item &it, bool nested = false,
bool ignore_rigidity = false,
bool ignore_pkt_settings = true,
const item_location &parent_it = item_location() ) const;
const item_location &parent_it = item_location(),
units::volume remaining_parent_volume = 10000000_ml ) const;
bool can_contain( const itype &tp ) const;
bool can_contain_partial( const item &it ) const;
/*@}*/
Expand Down
6 changes: 5 additions & 1 deletion src/item_contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,8 @@ ret_val<void> item_contents::can_contain_rigid( const item &it,
return ret;
}

ret_val<void> item_contents::can_contain( const item &it, const bool ignore_pkt_settings ) const
ret_val<void> item_contents::can_contain( const item &it, const bool ignore_pkt_settings,
units::volume remaining_parent_volume ) const
{
ret_val<void> ret = ret_val<void>::make_failure( _( "is not a container" ) );
for( const item_pocket &pocket : contents ) {
Expand All @@ -938,6 +939,9 @@ ret_val<void> item_contents::can_contain( const item &it, const bool ignore_pkt_
ret = ret_val<void>::make_failure( _( "denied by pocket auto insert settings" ) );
continue;
}
if( !pocket.rigid() && it.volume() > remaining_parent_volume ) {
continue;
}
const ret_val<item_pocket::contain_code> pocket_contain_code = pocket.can_contain( it );
if( pocket_contain_code.success() ) {
return ret_val<void>::make_success();
Expand Down
5 changes: 4 additions & 1 deletion src/item_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ class item_contents
* physical pockets.
* @param it the item being put in
* @param ignore_pkt_settings whether to ignore pocket autoinsert settings
* @param remaining_parent_volume if we are nesting things without concern for rigidity we need to be careful about overfilling pockets
* this tracks the remaining volume of any parent pockets
*/
ret_val<void> can_contain( const item &it, bool ignore_pkt_settings = true ) const;
ret_val<void> can_contain( const item &it, bool ignore_pkt_settings = true,
units::volume remaining_parent_volume = 10000000_ml ) const;
ret_val<void> can_contain_rigid( const item &it, bool ignore_pkt_settings = true ) const;
bool can_contain_liquid( bool held_or_ground ) const;

Expand Down
22 changes: 22 additions & 0 deletions tests/item_pocket_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2659,3 +2659,25 @@ TEST_CASE( "Sawed off fits in large holster", "[item][pocket]" )
CHECK( large_holster.can_contain( double_barrel ).success() );

}

// this tests for cases where we try to find a nested pocket for items (when a parent pocket has some restrictions) and find a massive bag inside the parent pocket
// need to make sure we don't try to fit things larger than the parent pockets remaining volume inside the child pocket if it is non-rigid
TEST_CASE( "bag with restrictions and nested bag doesn't fit too large items", "[item][pocket]" )
{
item backpack( "test_backpack" );
item backpack_two( "test_backpack" );
item mini_backpack( "test_mini_backpack" );

mini_backpack.put_in( backpack, item_pocket::pocket_type::CONTAINER );
REQUIRE( !mini_backpack.is_container_empty() );
REQUIRE( mini_backpack.only_item().typeId() == backpack.typeId() );

// need to set a setting on the pocket for this to work since that's when nesting starts trying weird stuff
mini_backpack.get_contents().get_all_standard_pockets().front()->settings.set_disabled( true );

// check if the game thinks the mini bag can contain the second bag (it can't)
// but that the bag could otherwise fit if not in the parent pocket
CHECK( backpack.can_contain( backpack_two ).success() );
CHECK( !mini_backpack.can_contain( backpack_two ).success() );

}