From 6b575c0bb37559bcdbeb1c9025d7162d8debc9d4 Mon Sep 17 00:00:00 2001 From: bombasticSlacks Date: Sun, 14 Aug 2022 12:43:45 -0300 Subject: [PATCH 1/3] Nesting big bag tests --- data/mods/TEST_DATA/items.json | 19 +++++++++++++++++++ src/item.cpp | 6 +++--- src/item.h | 5 ++++- src/item_contents.cpp | 6 +++++- src/item_contents.h | 5 ++++- tests/item_pocket_test.cpp | 23 +++++++++++++++++++++++ 6 files changed, 58 insertions(+), 6 deletions(-) diff --git a/data/mods/TEST_DATA/items.json b/data/mods/TEST_DATA/items.json index bc266cd4de18f..b90749f2f6c8c 100644 --- a/data/mods/TEST_DATA/items.json +++ b/data/mods/TEST_DATA/items.json @@ -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", diff --git a/src/item.cpp b/src/item.cpp index a8e580e8667e2..739cbc89d8687 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -10213,7 +10213,7 @@ ret_val item::is_compatible( const item &it ) const ret_val 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() ) ) { @@ -10243,7 +10243,7 @@ ret_val 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::make_success(); } } @@ -10251,7 +10251,7 @@ ret_val item::can_contain( const item &it, const bool nested, 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 diff --git a/src/item.h b/src/item.h index 6fb21a7b8e0ce..c0a316ce5fa6b 100644 --- a/src/item.h +++ b/src/item.h @@ -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 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; /*@}*/ diff --git a/src/item_contents.cpp b/src/item_contents.cpp index abc425bcd74ed..6ac12894fff00 100644 --- a/src/item_contents.cpp +++ b/src/item_contents.cpp @@ -926,7 +926,8 @@ ret_val item_contents::can_contain_rigid( const item &it, return ret; } -ret_val item_contents::can_contain( const item &it, const bool ignore_pkt_settings ) const +ret_val item_contents::can_contain( const item &it, const bool ignore_pkt_settings, + units::volume remaining_parent_volume ) const { ret_val ret = ret_val::make_failure( _( "is not a container" ) ); for( const item_pocket &pocket : contents ) { @@ -938,6 +939,9 @@ ret_val item_contents::can_contain( const item &it, const bool ignore_pkt_ ret = ret_val::make_failure( _( "denied by pocket auto insert settings" ) ); continue; } + if( !pocket.rigid() && it.volume() > remaining_parent_volume ) { + continue; + } const ret_val pocket_contain_code = pocket.can_contain( it ); if( pocket_contain_code.success() ) { return ret_val::make_success(); diff --git a/src/item_contents.h b/src/item_contents.h index a1973151f35c6..48fd3509b5c15 100644 --- a/src/item_contents.h +++ b/src/item_contents.h @@ -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 can_contain( const item &it, bool ignore_pkt_settings = true ) const; + ret_val can_contain( const item &it, bool ignore_pkt_settings = true, + units::volume remaining_parent_volume = 10000000_ml ) const; ret_val can_contain_rigid( const item &it, bool ignore_pkt_settings = true ) const; bool can_contain_liquid( bool held_or_ground ) const; diff --git a/tests/item_pocket_test.cpp b/tests/item_pocket_test.cpp index 56335a809d76c..a0bed7bcbf12c 100644 --- a/tests/item_pocket_test.cpp +++ b/tests/item_pocket_test.cpp @@ -50,6 +50,7 @@ Item_spawn_data_wallet_science_stylish_full( "wallet_science_stylish_full" ); static const item_group_id Item_spawn_data_wallet_stylish_full( "wallet_stylish_full" ); static const itype_id itype_test_backpack( "test_backpack" ); +static const itype_id itype_test_mini_backpack( "test_mini_backpack" ); static const itype_id itype_test_socks( "test_socks" ); static const itype_id itype_test_watertight_open_sealed_container_1L( "test_watertight_open_sealed_container_1L" ); @@ -2659,3 +2660,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() ); + +} From 338f3503ec0bf94cf3e3519193d7b5dd3722f3ba Mon Sep 17 00:00:00 2001 From: bombasticSlacks Date: Sun, 14 Aug 2022 17:44:05 -0300 Subject: [PATCH 2/3] one more check for length this time --- src/item.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/item.cpp b/src/item.cpp index 739cbc89d8687..3ae277af8735d 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -10229,10 +10229,17 @@ ret_val item::can_contain( const item &it, const bool nested, it.contents.bigger_on_the_inside( it.volume() ) ) { return ret_val::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 = From 4bbdf510a089e94bc74bd6f886b87836406d3ea0 Mon Sep 17 00:00:00 2001 From: Dillon Matchett Date: Mon, 15 Aug 2022 08:38:25 -0300 Subject: [PATCH 3/3] Update tests/item_pocket_test.cpp --- tests/item_pocket_test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/item_pocket_test.cpp b/tests/item_pocket_test.cpp index a0bed7bcbf12c..8a2158dfae414 100644 --- a/tests/item_pocket_test.cpp +++ b/tests/item_pocket_test.cpp @@ -50,7 +50,6 @@ Item_spawn_data_wallet_science_stylish_full( "wallet_science_stylish_full" ); static const item_group_id Item_spawn_data_wallet_stylish_full( "wallet_stylish_full" ); static const itype_id itype_test_backpack( "test_backpack" ); -static const itype_id itype_test_mini_backpack( "test_mini_backpack" ); static const itype_id itype_test_socks( "test_socks" ); static const itype_id itype_test_watertight_open_sealed_container_1L( "test_watertight_open_sealed_container_1L" );