Skip to content

Commit

Permalink
Fix nested pockets pickup
Browse files Browse the repository at this point in the history
There are quite some discrepancies between can_contain and
best_pocket functionality. This commit looks to fix that and actually
allow nested pockets to work properly as long as there is space.
fixes CleverRaven#54725
  • Loading branch information
robkuijper committed Jan 30, 2022
1 parent 7684355 commit bb0212a
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 44 deletions.
11 changes: 1 addition & 10 deletions src/character.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2943,16 +2943,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
Expand Down
13 changes: 8 additions & 5 deletions src/item.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9315,24 +9315,27 @@ ret_val<bool> item::is_compatible( const item &it ) const
return contents.is_compatible( it );
}

ret_val<bool> item::can_contain( const item &it ) const
ret_val<bool> item::can_contain( const item &it, const bool nested ) const
{
if( this == &it ) {
// does the set of all sets contain itself?
return ret_val<bool>::make_failure();
}
if( nested && !this->is_container() ) {
return ret_val<bool>::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<bool>::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<bool>::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
Expand All @@ -9350,10 +9353,10 @@ bool item::can_contain_partial( const item &it ) const
}

std::pair<item_location, item_pocket *> 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 )
Expand Down
6 changes: 4 additions & 2 deletions src/item.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> can_contain( const item &it ) const;
ret_val<bool> 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<item_location, item_pocket *> 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;
Expand Down
42 changes: 18 additions & 24 deletions src/item_contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -550,13 +550,12 @@ void item_contents::force_insert_item( const item &it, item_pocket::pocket_type
}

std::pair<item_location, item_pocket *> 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)
{
if( !can_contain( it ).success() ) {
return { item_location(), nullptr };
}
std::pair<item_location, item_pocket *> 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<item_location, item_pocket*> ret = { parent, nullptr };
for( item_pocket &pocket : contents ) {
if( !pocket.is_type( item_pocket::pocket_type::CONTAINER ) ) {
// best pocket is for picking stuff up.
Expand All @@ -568,29 +567,24 @@ std::pair<item_location, item_pocket *> 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;
const auto 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<item_location, item_pocket *> 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;
Expand Down
4 changes: 3 additions & 1 deletion src/item_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<item_location, item_pocket *> 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;
Expand Down
23 changes: 21 additions & 2 deletions src/item_pocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,7 @@ ret_val<item_pocket::contain_code> item_pocket::is_compatible( const item &it )

if( it.length() < data->min_item_length ) {
return ret_val<item_pocket::contain_code>::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 ) {
Expand Down Expand Up @@ -1349,7 +1349,7 @@ ret_val<item_pocket::contain_code> item_pocket::can_contain( const item &it ) co
}
} else if( size() == 1 && contents.front().made_of( phase_id::GAS ) ) {
return ret_val<item_pocket::contain_code>::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" ) );
}


Expand Down Expand Up @@ -1801,6 +1801,25 @@ ret_val<item_pocket::contain_code> 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 : this->all_items_top() ) {
if ( contained_item == &it || contained_item == avoid ) {
continue;
}
const auto nested_pocket = contained_item->best_pocket( it, parent, avoid,
allow_sealed, ignore_settings, /*nested=*/true );
if ( nested_pocket.second != nullptr &&
( ret == nullptr || ret->better_pocket( *nested_pocket.second, it, /*nested=*/true) ) ) {
ret = nested_pocket.second;
}
}
return ret;
}

int item_pocket::obtain_cost( const item &it ) const
{
if( has_item( it ) ) {
Expand Down
14 changes: 14 additions & 0 deletions src/item_pocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<item>
std::list<item> &edit_contents();

Expand Down

0 comments on commit bb0212a

Please sign in to comment.