Skip to content

Commit

Permalink
Do not count magazine as pocket in the pickup inventory (#73107)
Browse files Browse the repository at this point in the history
* Not count type mag/mag_well in pickup inventory.

* Apply the change for frozen_liquid.

* astyle-check

* Clang-tidy

* Fix item_cannot_contain_contents_it_already_has.

* Revert "Fix item_cannot_contain_contents_it_already_has."

This reverts commit b6dec3a.

* Try to pass the test case.

* Remove unused parameter.

* Add is_pick_up_inv back.
  • Loading branch information
osuphobia authored Apr 21, 2024
1 parent 8281fcf commit 3ceec3d
Show file tree
Hide file tree
Showing 12 changed files with 63 additions and 36 deletions.
6 changes: 3 additions & 3 deletions src/character.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3234,19 +3234,19 @@ bool Character::can_pickVolume( const item &it, bool, const item *avoid,
}

bool Character::can_pickVolume_partial( const item &it, bool, const item *avoid,
const bool ignore_pkt_settings ) const
const bool ignore_pkt_settings, const bool is_pick_up_inv ) const
{
item copy = it;
if( it.count_by_charges() ) {
copy.charges = 1;
}

if( ( avoid == nullptr || &weapon != avoid ) &&
weapon.can_contain( copy, false, false, ignore_pkt_settings ).success() ) {
weapon.can_contain( copy, false, false, ignore_pkt_settings, is_pick_up_inv ).success() ) {
return true;
}

return worn.can_pickVolume( copy, ignore_pkt_settings );
return worn.can_pickVolume( copy, ignore_pkt_settings, is_pick_up_inv );
}

bool Character::can_pickWeight( const item &it, bool safe ) const
Expand Down
2 changes: 1 addition & 1 deletion src/character.h
Original file line number Diff line number Diff line change
Expand Up @@ -2281,7 +2281,7 @@ class Character : public Creature, public visitable
bool can_pickVolume( const item &it, bool safe = false, const item *avoid = nullptr,
bool ignore_pkt_settings = true ) const;
bool can_pickVolume_partial( const item &it, bool safe = false, const item *avoid = nullptr,
bool ignore_pkt_settings = true ) const;
bool ignore_pkt_settings = true, bool is_pick_up_inv = false ) const;
bool can_pickWeight( const item &it, bool safe = true ) const;
bool can_pickWeight_partial( const item &it, bool safe = true ) const;

Expand Down
5 changes: 3 additions & 2 deletions src/character_attire.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1794,10 +1794,11 @@ void outfit::add_dependent_item( std::list<item *> &dependent, const item &it )
}
}

bool outfit::can_pickVolume( const item &it, const bool ignore_pkt_settings ) const
bool outfit::can_pickVolume( const item &it, const bool ignore_pkt_settings,
const bool is_pick_up_inv ) const
{
for( const item &w : worn ) {
if( w.can_contain( it, false, false, ignore_pkt_settings ).success() ) {
if( w.can_contain( it, false, false, ignore_pkt_settings, is_pick_up_inv ).success() ) {
return true;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/character_attire.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ class outfit
const body_part_set &worn_item_body_parts ) const;
// will someone get shocked by zapback
bool hands_conductive() const;
bool can_pickVolume( const item &it, bool ignore_pkt_settings = true ) const;
bool can_pickVolume( const item &it, bool ignore_pkt_settings = true,
bool is_pick_up_inv = false ) const;
side is_wearing_shoes( const bodypart_id &bp ) const;
bool is_barefoot() const;
item item_worn_with_flag( const flag_id &f, const bodypart_id &bp ) const;
Expand Down
18 changes: 10 additions & 8 deletions src/game_inventory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,11 +532,13 @@ class pickup_inventory_preset : public inventory_selector_preset
} else if( loc->is_frozen_liquid() ) {
ret_val<crush_tool_type> can_crush = you.can_crush_frozen_liquid( loc );

if( loc->has_flag( flag_SHREDDED ) ) { // NOLINT(bugprone-branch-clone)
return std::string();
} else if( !can_crush.success() ) {
return can_crush.str();
} else if( !you.can_pickVolume_partial( *loc, false, nullptr, false ) ) {
if( you.can_pickVolume_partial( *loc, false, nullptr, false, true ) ) {
if( loc->has_flag( flag_SHREDDED ) ) {// NOLINT(bugprone-branch-clone)
return std::string();
} else if( !can_crush.success() ) {
return can_crush.str();
}
} else {
item item_copy( *loc );
item_copy.charges = 1;
item_copy.set_flag( flag_SHREDDED );
Expand All @@ -549,11 +551,11 @@ class pickup_inventory_preset : public inventory_selector_preset
!ip->front().can_combine( item_copy ) ||
item_copy.typeId() != ip->front().typeId() ) ) ) {
return _( "Does not have any pocket for frozen liquids!" );
} else {
return std::string();
}
} else {
return std::string();
}
} else if( !you.can_pickVolume_partial( *loc, false, nullptr, false ) &&
} else if( !you.can_pickVolume_partial( *loc, false, nullptr, false, true ) &&
( skip_wield_check || you.has_wield_conflicts( *loc ) ) ) {
return _( "Does not fit in any pocket!" );
} else if( !you.can_pickWeight_partial( *loc, !get_option<bool>( "DANGEROUS_PICKUPS" ) ) ) {
Expand Down
2 changes: 1 addition & 1 deletion src/inventory_ui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ std::string inventory_holster_preset::get_denial( const item_location &it ) cons
item_copy.charges = 1;
item_location parent = it.has_parent() ? it.parent_item() : item_location();

ret_val<void> ret = holster->can_contain( item_copy, false, false, true, parent );
ret_val<void> ret = holster->can_contain( item_copy, false, false, true, false, parent );
if( !ret.success() ) {
return !ret.str().empty() ? ret.str() : "item can't be stored there";
}
Expand Down
20 changes: 12 additions & 8 deletions src/item.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10096,22 +10096,24 @@ ret_val<void> item::is_compatible( const item &it ) const

ret_val<void> item::can_contain_directly( const item &it ) const
{
return can_contain( it, false, false, true, item_location(), 10000000_ml, false );
return can_contain( it, false, false, true, false, item_location(), 10000000_ml, false );
}

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 bool ignore_pkt_settings, const bool is_pick_up_inv,
const item_location &parent_it,
units::volume remaining_parent_volume, const bool allow_nested ) const
{
int copies = 1;
return can_contain( it, copies, nested, ignore_rigidity,
ignore_pkt_settings, parent_it,
ignore_pkt_settings, is_pick_up_inv, parent_it,
remaining_parent_volume, allow_nested );
}

ret_val<void> item::can_contain( const item &it, int &copies_remaining, const bool nested,
const bool ignore_rigidity, const bool ignore_pkt_settings,
const item_location &parent_it, units::volume remaining_parent_volume,
const bool is_pick_up_inv, const item_location &parent_it,
units::volume remaining_parent_volume,
const bool allow_nested ) const
{
if( copies_remaining <= 0 ) {
Expand Down Expand Up @@ -10156,7 +10158,8 @@ ret_val<void> item::can_contain( const item &it, int &copies_remaining, const bo
continue;
}
auto ret = internal_it->can_contain( it, copies_remaining, true, ignore_nested_rigidity,
ignore_pkt_settings, parent_it, pkt->remaining_volume() );
ignore_pkt_settings, is_pick_up_inv, parent_it,
pkt->remaining_volume() );
if( copies_remaining <= 0 ) {
return ret;
}
Expand All @@ -10165,8 +10168,9 @@ ret_val<void> item::can_contain( const item &it, int &copies_remaining, const bo
}

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

ret_val<void> item::can_contain( const itype &tp ) const
Expand All @@ -10189,7 +10193,7 @@ ret_val<void> item::can_contain_partial_directly( const item &it ) const
if( i_copy.count_by_charges() ) {
i_copy.charges = 1;
}
return can_contain( i_copy, false, false, true, item_location(), 10000000_ml, false );
return can_contain( i_copy, false, false, true, false, item_location(), 10000000_ml, false );
}

std::pair<item_location, item_pocket *> item::best_pocket( const item &it, item_location &this_loc,
Expand Down
2 changes: 2 additions & 0 deletions src/item.h
Original file line number Diff line number Diff line change
Expand Up @@ -1701,12 +1701,14 @@ class item : public visitable
ret_val<void> can_contain( const item &it, bool nested = false,
bool ignore_rigidity = false,
bool ignore_pkt_settings = true,
bool is_pick_up_inv = false,
const item_location &parent_it = item_location(),
units::volume remaining_parent_volume = 10000000_ml,
bool allow_nested = true ) const;
ret_val<void> can_contain( const item &it, int &copies_remaining, bool nested = false,
bool ignore_rigidity = false,
bool ignore_pkt_settings = true,
bool is_pick_up_inv = false,
const item_location &parent_it = item_location(),
units::volume remaining_parent_volume = 10000000_ml,
bool allow_nested = true ) const;
Expand Down
27 changes: 20 additions & 7 deletions src/item_contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1055,17 +1055,23 @@ ret_val<void> item_contents::is_compatible( const item &it ) const
}

ret_val<void> item_contents::can_contain_rigid( const item &it,
const bool ignore_pkt_settings ) const
const bool ignore_pkt_settings, const bool is_pick_up_inv ) const
{
int copies = 1;
return can_contain_rigid( it, copies, ignore_pkt_settings );
return can_contain_rigid( it, copies, ignore_pkt_settings, is_pick_up_inv );
}

ret_val<void> item_contents::can_contain_rigid( const item &it, int &copies_remaining,
const bool ignore_pkt_settings ) const
const bool ignore_pkt_settings, const bool is_pick_up_inv ) const
{
ret_val<void> ret = ret_val<void>::make_failure( _( "is not a container" ) );
for( const item_pocket &pocket : contents ) {
// Only count container in pickup_inventory_preset.
if( is_pick_up_inv ) {
if( !pocket.is_type( pocket_type::CONTAINER ) ) {
continue;
}
}
if( pocket.is_type( pocket_type::MOD ) ||
pocket.is_type( pocket_type::CORPSE ) ||
pocket.is_type( pocket_type::MIGRATION ) ) {
Expand All @@ -1080,7 +1086,7 @@ ret_val<void> item_contents::can_contain_rigid( const item &it, int &copies_rema
continue;
}
const ret_val<item_pocket::contain_code> pocket_contain_code = pocket.can_contain( it,
copies_remaining );
copies_remaining, false );
if( copies_remaining <= 0 || pocket_contain_code.success() ) {
return ret_val<void>::make_success();
}
Expand All @@ -1090,14 +1096,15 @@ ret_val<void> item_contents::can_contain_rigid( const item &it, int &copies_rema
}

ret_val<void> item_contents::can_contain( const item &it, const bool ignore_pkt_settings,
units::volume remaining_parent_volume ) const
const bool is_pick_up_inv, units::volume remaining_parent_volume ) const
{
int copies = 1;
return can_contain( it, copies, ignore_pkt_settings, remaining_parent_volume );
return can_contain( it, copies, ignore_pkt_settings, is_pick_up_inv, remaining_parent_volume );
}

ret_val<void> item_contents::can_contain( const item &it, int &copies_remaining,
const bool ignore_pkt_settings, units::volume remaining_parent_volume ) const
const bool ignore_pkt_settings, const bool is_pick_up_inv,
units::volume remaining_parent_volume ) const
{
ret_val<void> ret = ret_val<void>::make_failure( _( "is not a container" ) );

Expand All @@ -1108,6 +1115,12 @@ ret_val<void> item_contents::can_contain( const item &it, int &copies_remaining,
if( pocket.is_forbidden() ) {
continue;
}
// Only count container in pickup_inventory_preset.
if( is_pick_up_inv ) {
if( !pocket.is_type( pocket_type::CONTAINER ) ) {
continue;
}
}
// mod, migration, corpse, and software aren't regular pockets.
if( !pocket.is_standard_type() ) {
continue;
Expand Down
8 changes: 6 additions & 2 deletions src/item_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,16 @@ class item_contents
* this tracks the remaining volume of any parent pockets
*/
ret_val<void> can_contain( const item &it, bool ignore_pkt_settings = true,
bool is_pick_up_inv = false,
units::volume remaining_parent_volume = 10000000_ml ) const;
ret_val<void> can_contain( const item &it, int &copies_remaining, bool ignore_pkt_settings = true,
bool is_pick_up_inv = false,
units::volume remaining_parent_volume = 10000000_ml ) const;
ret_val<void> can_contain_rigid( const item &it, bool ignore_pkt_settings = true ) const;
ret_val<void> can_contain_rigid( const item &it, bool ignore_pkt_settings = true,
bool is_pick_up_inv = false ) const;
ret_val<void> can_contain_rigid( const item &it, int &copies_remaining,
bool ignore_pkt_settings = true ) const;
bool ignore_pkt_settings = true,
bool is_pick_up_inv = false ) const;
bool can_contain_liquid( bool held_or_ground ) const;

bool contains_no_solids() const;
Expand Down
2 changes: 1 addition & 1 deletion src/iuse_actor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5025,7 +5025,7 @@ std::optional<int> link_up_actor::link_extend_cable( Character *p, item &it,
if( extended_copy ) {
// Check if there's another pocket on the same container that can hold the extended item, respecting pocket settings.
item_location parent = extended.parent_item();
if( parent->can_contain( *extended_ptr, false, false, false,
if( parent->can_contain( *extended_ptr, false, false, false, false,
item_location(), 10000000_ml, false ).success() ) {
if( !parent->put_in( *extended_ptr, pocket_type::CONTAINER ).success() ) {
debugmsg( "Failed to put %s inside %s!", extended_ptr->type_name(),
Expand Down
4 changes: 2 additions & 2 deletions tests/item_pocket_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2640,7 +2640,7 @@ TEST_CASE( "item_cannot_contain_contents_it_already_has", "[item][pocket]" )
}
CHECK( in_top );
CHECK( bottle_loc->can_contain( water_item ).success() );
CHECK( !bottle_loc->can_contain( water_item, false, false, true, bottle_loc ).success() );
CHECK( !bottle_loc->can_contain( water_item, false, false, true, false, bottle_loc ).success() );

// Check backpack containing bottle containing water
in_top = false;
Expand All @@ -2651,7 +2651,7 @@ TEST_CASE( "item_cannot_contain_contents_it_already_has", "[item][pocket]" )
}
CHECK( !in_top );
CHECK( backpack_loc->can_contain( water_item ).success() );
CHECK( !backpack_loc->can_contain( water_item, false, false, true, bottle_loc ).success() );
CHECK( !backpack_loc->can_contain( water_item, false, false, true, false, bottle_loc ).success() );
}

TEST_CASE( "Sawed_off_fits_in_large_holster", "[item][pocket]" )
Expand Down

0 comments on commit 3ceec3d

Please sign in to comment.