diff --git a/src/character.cpp b/src/character.cpp index c52c3282535aa..50f60a73829b1 100644 --- a/src/character.cpp +++ b/src/character.cpp @@ -2945,16 +2945,7 @@ bool Character::can_pickVolume_partial( const item &it, bool, const item *avoid copy.charges = 1; } - const item weapon = get_wielded_item(); - if( ( avoid == nullptr || &weapon != avoid ) && weapon.can_contain( copy ).success() ) { - return true; - } - for( const item &w : worn ) { - if( ( avoid == nullptr || &w != avoid ) && w.can_contain( copy ).success() ) { - return true; - } - } - return false; + return can_pickVolume( copy, avoid ); } bool Character::can_pickWeight( const item &it, bool safe ) const diff --git a/src/character.h b/src/character.h index 23f8a8d4458ac..2b82dbdc35d5a 100644 --- a/src/character.h +++ b/src/character.h @@ -1234,11 +1234,6 @@ class Character : public Creature, public visitable int get_mod( const trait_id &mut, const std::string &arg ) const; /** Applies skill-based boosts to stats **/ void apply_skill_boost(); - /** - * What is the best pocket to put @it into? - * the pockets in @avoid do not count - */ - std::pair best_pocket( const item &it, const item *avoid ); protected: void on_move( const tripoint_abs_ms &old_pos ) override; @@ -1950,6 +1945,16 @@ class Character : public Creature, public visitable bool can_pickVolume_partial( const item &it, bool safe = false, const item *avoid = nullptr ) const; bool can_pickWeight( const item &it, bool safe = true ) const; bool can_pickWeight_partial( const item &it, bool safe = true ) const; + + /** + * What is the best pocket to put @it into? + * @param it the item to try and find a pocket for. + * @param avoid pockets in this item are not taken into account. + * + * @returns nullptr in the value of the returned pair if no valid pocket was found. + */ + std::pair best_pocket( const item &it, const item *avoid = nullptr ); + /** * Checks if character stats and skills meet minimum requirements for the item. * Prints an appropriate message if requirements not met. diff --git a/src/item.cpp b/src/item.cpp index 51648801548f6..36b1bbe84f7a1 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -9324,24 +9324,27 @@ ret_val item::is_compatible( const item &it ) const return contents.is_compatible( it ); } -ret_val item::can_contain( const item &it ) const +ret_val item::can_contain( const item &it, const bool nested ) const { if( this == &it ) { // does the set of all sets contain itself? return ret_val::make_failure(); } + if( nested && !this->is_container() ) { + return ret_val::make_failure(); + } // disallow putting portable holes into bags of holding if( contents.bigger_on_the_inside( volume() ) && it.contents.bigger_on_the_inside( it.volume() ) ) { return ret_val::make_failure(); } for( const item *internal_it : contents.all_items_top( item_pocket::pocket_type::CONTAINER ) ) { - if( internal_it->contents.can_contain_rigid( it ).success() ) { + if( internal_it->can_contain( it, true ).success() ) { return ret_val::make_success(); } } - return contents.can_contain( it ); + return nested ? contents.can_contain_rigid( it ) : contents.can_contain( it ); } bool item::can_contain( const itype &tp ) const @@ -9359,10 +9362,10 @@ bool item::can_contain_partial( const item &it ) const } std::pair item::best_pocket( const item &it, item_location &parent, - const item *avoid, const bool allow_sealed, const bool ignore_settings ) + const item *avoid, const bool allow_sealed, const bool ignore_settings, const bool nested ) { item_location nested_location( parent, this ); - return contents.best_pocket( it, nested_location, avoid, allow_sealed, ignore_settings ); + return contents.best_pocket( it, nested_location, avoid, allow_sealed, ignore_settings, nested ); } bool item::spill_contents( Character &c ) diff --git a/src/item.h b/src/item.h index 7f37aa715318f..15625cdc2418d 100644 --- a/src/item.h +++ b/src/item.h @@ -1484,14 +1484,16 @@ class item : public visitable /** * Can the pocket contain the specified item? * @param it the item being put in + * @param nested whether or not the current call is nested (used recursively). */ /*@{*/ - ret_val can_contain( const item &it ) const; + ret_val can_contain( const item &it, const bool nested = false ) const; bool can_contain( const itype &tp ) const; bool can_contain_partial( const item &it ) const; /*@}*/ std::pair best_pocket( const item &it, item_location &parent, - const item *avoid = nullptr, bool allow_sealed = false, bool ignore_settings = false ); + const item *avoid = nullptr, bool allow_sealed = false, bool ignore_settings = false, + bool nested = false ); units::length max_containable_length( bool unrestricted_pockets_only = false ) const; units::length min_containable_length() const; diff --git a/src/item_contents.cpp b/src/item_contents.cpp index 13a926e800a72..37ea1139de304 100644 --- a/src/item_contents.cpp +++ b/src/item_contents.cpp @@ -550,10 +550,12 @@ void item_contents::force_insert_item( const item &it, item_pocket::pocket_type } std::pair item_contents::best_pocket( const item &it, - item_location &parent, const item *avoid, const bool allow_sealed, const bool ignore_settings ) + item_location &parent, const item *avoid, const bool allow_sealed, const bool ignore_settings, + const bool nested ) { - std::pair ret; - ret.second = nullptr; + // @TODO: this could be made better by doing a plain preliminary volume check. + // if the total volume of the parent is not sufficient, a child won't have enough either. + std::pair ret = { parent, nullptr }; for( item_pocket &pocket : contents ) { if( !pocket.is_type( item_pocket::pocket_type::CONTAINER ) ) { // best pocket is for picking stuff up. @@ -565,29 +567,24 @@ std::pair item_contents::best_pocket( const item & // that needs to be something a player explicitly does continue; } - if( !pocket.can_contain( it ).success() ) { - continue; - } if( !ignore_settings && !pocket.settings.accepts_item( it ) ) { // Item forbidden by whitelist / blacklist continue; } - if( ret.second == nullptr || ret.second->better_pocket( pocket, it ) ) { - // this pocket is the new candidate for "best" - ret.first = parent; + item_pocket *const nested_content_pocket = + pocket.best_pocket_in_contents( parent, it, avoid, allow_sealed, ignore_settings ); + if( nested_content_pocket != nullptr ) { + // item fits in pockets contents, no need to check the pocket itself. + // this gives nested pockets priority over parent pockets. + ret.second = nested_content_pocket; + continue; + } + if( !pocket.can_contain( it ).success() || ( nested && !pocket.rigid() ) ) { + // non-rigid nested pocket makes no sense, item should also be able to fit in parent. + continue; + } + if( ret.second == nullptr || ret.second->better_pocket( pocket, it, /*nested=*/nested ) ) { ret.second = &pocket; - // check all pockets within to see if they are better - for( item *contained : pocket.all_items_top() ) { - if( contained == avoid ) { - continue; - } - std::pair internal_pocket = - contained->best_pocket( it, parent, avoid, /*allow_sealed=*/false, /*ignore_settings=*/false ); - if( internal_pocket.second != nullptr && - ret.second->better_pocket( *internal_pocket.second, it, true ) ) { - ret = internal_pocket; - } - } } } return ret; diff --git a/src/item_contents.h b/src/item_contents.h index b1b6f8dc7d773..e35a701910cdc 100644 --- a/src/item_contents.h +++ b/src/item_contents.h @@ -36,10 +36,12 @@ class item_contents /** * returns an item_location and pointer to the best pocket that can contain the item @it + * checks all items contained in every pocket * only checks CONTAINER pocket type */ std::pair best_pocket( const item &it, item_location &parent, - const item *avoid = nullptr, bool allow_sealed = false, bool ignore_settings = false ); + const item *avoid = nullptr, bool allow_sealed = false, bool ignore_settings = false, + bool nested = false ); units::length max_containable_length( bool unrestricted_pockets_only = false ) const; units::length min_containable_length() const; diff --git a/src/item_pocket.cpp b/src/item_pocket.cpp index 88a94107ba54c..a15ad6679a23d 100644 --- a/src/item_pocket.cpp +++ b/src/item_pocket.cpp @@ -1274,7 +1274,7 @@ ret_val item_pocket::is_compatible( const item &it ) if( it.length() < data->min_item_length ) { return ret_val::make_failure( - contain_code::ERR_TOO_BIG, _( "item is too short" ) ); + contain_code::ERR_TOO_SMALL, _( "item is too short" ) ); } if( it.volume() < data->min_item_volume ) { @@ -1349,7 +1349,7 @@ ret_val item_pocket::can_contain( const item &it ) co } } else if( size() == 1 && contents.front().made_of( phase_id::GAS ) ) { return ret_val::make_failure( - contain_code::ERR_LIQUID, _( "can't put non gas into pocket with gas" ) ); + contain_code::ERR_GAS, _( "can't put non gas into pocket with gas" ) ); } @@ -1801,6 +1801,26 @@ ret_val item_pocket::insert_item( const item &it ) return ret; } +item_pocket *item_pocket::best_pocket_in_contents( + item_location &parent, const item &it, const item *avoid, + const bool allow_sealed, const bool ignore_settings ) +{ + item_pocket *ret = nullptr; + + for( item &contained_item : contents ) { + if( &contained_item == &it || &contained_item == avoid ) { + continue; + } + item_pocket *nested_pocket = contained_item.best_pocket( it, parent, avoid, + allow_sealed, ignore_settings, /*nested=*/true ).second; + if( nested_pocket != nullptr && + ( ret == nullptr || ret->better_pocket( *nested_pocket, it, /*nested=*/true ) ) ) { + ret = nested_pocket; + } + } + return ret; +} + int item_pocket::obtain_cost( const item &it ) const { if( has_item( it ) ) { diff --git a/src/item_pocket.h b/src/item_pocket.h index af35214fb1e60..63a2b7182e5bb 100644 --- a/src/item_pocket.h +++ b/src/item_pocket.h @@ -317,6 +317,20 @@ class item_pocket void add( const item &it, item **ret = nullptr ); bool can_unload_liquid() const; + /** + * @brief Check contents of pocket to see if it contains a valid item/pocket to store the given item. + * @param ret Used to cache and return a pocket if a valid one was found. + * @param pocket The @a item_pocket pocket to recursively look through. + * @param parent The parent item location, most likely provided by initial call to @a best_pocket. + * @param it The item to try and store. + * @param avoid Item to avoid trying to search for/inside of. + * + * @returns A non-nullptr if a suitable pocket is found. + */ + item_pocket *best_pocket_in_contents( + item_location &parent, const item &it, const item *avoid, + const bool allow_sealed, const bool ignore_settings ); + // only available to help with migration from previous usage of std::list std::list &edit_contents(); diff --git a/tests/item_pocket_test.cpp b/tests/item_pocket_test.cpp index b0c9908727556..99a357329910c 100644 --- a/tests/item_pocket_test.cpp +++ b/tests/item_pocket_test.cpp @@ -9,6 +9,7 @@ #include "calendar.h" #include "cata_catch.h" +#include "character.h" #include "debug.h" #include "enums.h" #include "flag.h" @@ -18,12 +19,17 @@ #include "item_pocket.h" #include "itype.h" #include "optional.h" +#include "player_helpers.h" #include "ret_val.h" #include "type_id.h" #include "units.h" #include "value_ptr.h" static const ammotype ammo_test_9mm( "test_9mm" ); +static const itype_id itype_test_backpack( "test_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" ); static const item_pocket::pocket_type pocket_container = item_pocket::pocket_type::CONTAINER; // Pocket Tests @@ -1519,6 +1525,25 @@ TEST_CASE( "pocket favorites allow or restrict containers", "[pocket][favorite][ } } +/** + * Add the given item to the best pocket of the dummy character. + * @param dummy The character to add the item to. + * @param it The item to add to the characters' pockets. + * + * @returns A pointer to the newly added item, if successfull. + */ +static item *add_item_to_best_pocket( Character &dummy, const item &it ) +{ + item_pocket *pocket = dummy.best_pocket( it ).second; + REQUIRE( pocket ); + + item *ret = nullptr; + pocket->add( it, &ret ); + REQUIRE( pocket->has_item( *ret ) ); + + return ret; +} + // Character::best_pocket // - See if wielded item can hold it - start with this as default // - For each worn item, see if best_pocket is better; if so, use it @@ -1530,11 +1555,138 @@ TEST_CASE( "pocket favorites allow or restrict containers", "[pocket][favorite][ // (Second argument is `avoid` item pointer, not parent item location) TEST_CASE( "character best pocket", "[pocket][character][best]" ) { - // TODO: - // Includes wielded item - // Includes worn items - // Uses best of multiple worn items - // Skips avoided items + item_location loc; + Character &dummy = get_player_character(); + + // reset dummy equipment before each test. + clear_avatar(); + + WHEN( "the player is wielding a container" ) { + item socks( itype_test_socks ); + item container( itype_test_watertight_open_sealed_container_1L ); + + // wield the container item. + dummy.set_wielded_item( container ); + + THEN( "the player can stash an item in it" ) { + // check to see if the item can be stored in any pocket. + CHECK( dummy.can_stash_partial( socks ) ); + + // and check to see if the item is actually stored in the wielded container. + std::pair pocket = dummy.best_pocket( socks ); + REQUIRE( pocket.second ); + CHECK( *pocket.second == *get_only_pocket( container ) ); + } + } + + WHEN( "the player is wearing a container" ) { + item socks( itype_test_socks ); + item backpack( itype_test_backpack ); + + // wear the backpack item. + REQUIRE( dummy.wear_item( backpack, false, false ) ); + + THEN( "the player can stash an item in it" ) { + // check to see if the item can be stored in any pocket. + CHECK( dummy.can_stash_partial( socks ) ); + + // and check to see if the item is actually stored in the wielded container. + std::pair pocket = dummy.best_pocket( socks ); + REQUIRE( pocket.second ); + CHECK( *pocket.second == *get_only_pocket( backpack ) ); + } + } + + WHEN( "wielding- and wearing a container" ) { + item socks( itype_test_socks ); + item container_wear( itype_test_watertight_open_sealed_container_1L ); + item container_wield( itype_test_watertight_open_sealed_container_1L ); + + // wield- and wear the respective container items. + REQUIRE( dummy.wield( container_wield ) ); + REQUIRE( dummy.wear_item( container_wear, false, false ) ); + + THEN( "the wielded container has priority" ) { + // check to see if the item can be stored in any pocket. + CHECK( dummy.can_stash_partial( socks ) ); + + // and try to find a fitting pocket. + // then check to see if it is, indeed, the wielded container. + std::pair pocket = dummy.best_pocket( socks ); + REQUIRE( pocket.second ); + CHECK( *pocket.second == *get_only_pocket( container_wield ) ); + } + } + + WHEN( "wearing a container with a nested rigid container" ) { + item socks( itype_test_socks ); + item backpack( itype_test_backpack ); + item container( itype_test_watertight_open_sealed_container_1L ); + + // wear the backpack item. + REQUIRE( dummy.wear_item( backpack ) ); + + // nest the rigid container inside of the worn backpack. + add_item_to_best_pocket( dummy, container ); + + THEN( "best pocket will be in nested rigid container" ) { + // check to see if the item can be stored in any pocket. + CHECK( dummy.can_stash_partial( socks ) ); + + // and try to find a fitting pocket. + // then check to see if it is, indeed, the nested container. + std::pair best_pocket = dummy.best_pocket( socks ); + REQUIRE( best_pocket.second ); + CHECK( *best_pocket.second == *get_only_pocket( container ) ); + } + } + + WHEN( "wearing a container with a nested non-rigid container" ) { + item socks( itype_test_socks ); + item backpack( itype_test_backpack ); + item backpack_nested( itype_test_backpack ); + + // wear the backpack item. + REQUIRE( dummy.wear_item( backpack ) ); + + // nest the non-rigid container inside of the worn backpack. + add_item_to_best_pocket( dummy, backpack_nested ); + + THEN( "best pocket will be in worn container" ) { + // check to see if the item can be stored in any pocket. + CHECK( dummy.can_stash_partial( socks ) ); + + // and try to find a fitting pocket. + // then check to see if it is, indeed, the worn container. + std::pair best_pocket = dummy.best_pocket( socks ); + REQUIRE( best_pocket.second ); + CHECK( *best_pocket.second == *get_only_pocket( backpack ) ); + } + } + + WHEN( "wearing a container with a nested rigid container which should be avoided" ) { + item socks( itype_test_socks ); + item backpack( itype_test_backpack ); + item container( itype_test_watertight_open_sealed_container_1L ); + + // wear the backpack item. + REQUIRE( dummy.wear_item( backpack ) ); + + // nest the rigid container inside of the worn backpack. + item *nested_container = add_item_to_best_pocket( dummy, container ); + + THEN( "best pocket for new item will be in worn container" ) { + // check to see if the item can be stored in any pocket. + CHECK( dummy.can_stash_partial( socks ) ); + + // and try to find a fitting pocket. + // then check to see if it is, indeed, the worn backpack + // when the nested (preferable) container should be avoided. + std::pair best_pocket = dummy.best_pocket( socks, nested_container ); + REQUIRE( best_pocket.second ); + CHECK( *best_pocket.second == *get_only_pocket( backpack ) ); + } + } } TEST_CASE( "guns and gunmods", "[pocket][gunmod]" )