Skip to content

Commit

Permalink
Fix cash card stacking and display (#31732)
Browse files Browse the repository at this point in the history
* Treat inventory more like map in AIM

In advanced inventory, when handling a stack from a character inventory,
it was stored only as a pointer-to-first-item-and-count.  Store it
instead as a list-of-item-pointers.  This provides more consistency in
rendering between inventory and map items.  In particular, cash cards
were previously rendering differently.

* Add item::is_money()

After the recent change to allow multiple ammo types, cash cards needed
different code to identify them as such.  This check was happening in
many places, some of which were updated, but not all.

Add a new function item::is_money() to make factor out this logic and
use it in all the places.

This fixes various issues related to cash cards like them not stacking
any more or strange messages when you pick them up.

* Add tests for cash card stacking
  • Loading branch information
jbytheway authored and ZhilkinSerg committed Jun 23, 2019
1 parent 3cc5996 commit 8880df9
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 10 deletions.
9 changes: 6 additions & 3 deletions src/advanced_inv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ void advanced_inventory::print_items( advanced_inventory_pane &pane, bool active
stolen = true;
}
}
if( it.ammo_types().count( ammotype( "money" ) ) ) {
if( it.is_money() ) {
//Count charges
// TODO: transition to the item_location system used for the normal inventory
unsigned int charges_total = 0;
Expand Down Expand Up @@ -974,8 +974,11 @@ void advanced_inventory_pane::add_items_from_area( advanced_inv_area &square,
if( square.id == AIM_INVENTORY ) {
const invslice &stacks = u.inv.slice();
for( size_t x = 0; x < stacks.size(); ++x ) {
auto &an_item = stacks[x]->front();
advanced_inv_listitem it( &an_item, x, stacks[x]->size(), square.id, false );
std::list<item *> item_pointers;
for( item &i : *stacks[x] ) {
item_pointers.push_back( &i );
}
advanced_inv_listitem it( item_pointers, x, square.id, false );
if( is_filtered( *it.items.front() ) ) {
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions src/inventory_ui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class selection_column_preset: public inventory_selector_preset
} else if( available_count != 1 ) {
res << available_count << ' ';
}
if( entry.location->ammo_types().count( ammotype( "money" ) ) ) {
if( entry.location->is_money() ) {
if( entry.chosen_count > 0 && entry.chosen_count < available_count ) {
//~ In the following string, the %s is the amount of money on the selected cards as passed by the display money function, out of the total amount of money on the cards, which is specified by the format_money function")
res << string_format( _( "%s of %s" ), entry.location->display_money( entry.chosen_count,
Expand Down Expand Up @@ -232,7 +232,7 @@ std::string inventory_selector_preset::get_caption( const inventory_entry &entry
{
const size_t count = entry.get_stack_size();
const std::string disp_name =
( entry.location->ammo_types().count( ammotype( "money" ) ) ) ?
( entry.location->is_money() ) ?
entry.location->display_money( count,
entry.location.charges_in_stack( count ) ) : entry.location->display_name( count );

Expand Down
13 changes: 9 additions & 4 deletions src/item.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ item &item::ammo_set( const itype_id &ammo, int qty )
}

// handle reloadable tools and guns with no specific ammo type as special case
if( ( ammo == "null" && ammo_types().empty() ) || ammo_types().count( ammotype( "money" ) ) ) {
if( ( ammo == "null" && ammo_types().empty() ) || is_money() ) {
if( ( is_tool() || is_gun() ) && magazine_integral() ) {
curammo = nullptr;
charges = std::min( qty, ammo_capacity() );
Expand Down Expand Up @@ -633,7 +633,7 @@ bool item::stacks_with( const item &rhs, bool check_components ) const
if( type != rhs.type ) {
return false;
}
if( charges != 0 && rhs.charges != 0 && ammo_current() == "money" ) {
if( charges != 0 && rhs.charges != 0 && is_money() ) {
// Dealing with nonempty cash cards
return true;
}
Expand Down Expand Up @@ -3416,7 +3416,7 @@ std::string item::display_name( unsigned int quantity ) const
}

if( amount || show_amt ) {
if( ammo_current() == "money" ) {
if( is_money() ) {
amt = string_format( " $%.2f", amount / 100.0 );
} else {
if( max_amount != 0 ) {
Expand Down Expand Up @@ -4416,6 +4416,11 @@ bool item::ready_to_revive( const tripoint &pos ) const
return false;
}

bool item::is_money() const
{
return ammo_types().count( ammotype( "money" ) );
}

bool item::count_by_charges() const
{
return type->count_by_charges();
Expand Down Expand Up @@ -5516,7 +5521,7 @@ bool item::operator<( const item &other ) const
!other.contents.empty() ? &other.contents.front() : &other;

if( me->typeId() == rhs->typeId() ) {
if( me->ammo_current() == "money" ) {
if( me->is_money() ) {
return me->charges > rhs->charges;
}
return me->charges < rhs->charges;
Expand Down
2 changes: 2 additions & 0 deletions src/item.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ class item : public visitable<item>
*/
bool ready_to_revive( const tripoint &pos ) const;

bool is_money() const;

/**
* Returns the default color of the item (e.g. @ref itype::color).
*/
Expand Down
2 changes: 1 addition & 1 deletion src/pickup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ void Pickup::pick_up( const tripoint &p, int min, from_where get_items_from )
stealing = true;
}
}
if( stacked_here[true_it].front()->ammo_current() == "money" ) {
if( stacked_here[true_it].front()->is_money() ) {
//Count charges
// TODO: transition to the item_location system used for the inventory
unsigned int charges_total = 0;
Expand Down
10 changes: 10 additions & 0 deletions tests/item_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ TEST_CASE( "gun_layer", "[item]" )
CHECK( gun.get_layer() == BELTED_LAYER );
}

TEST_CASE( "stacking_cash_cards", "[item]" )
{
// Differently-charged cash cards should stack if neither is zero.
item cash0( "cash_card", calendar::time_of_cataclysm, 0 );
item cash1( "cash_card", calendar::time_of_cataclysm, 1 );
item cash2( "cash_card", calendar::time_of_cataclysm, 2 );
CHECK( !cash0.stacks_with( cash1 ) );
CHECK( cash1.stacks_with( cash2 ) );
}

// second minute hour day week season year

TEST_CASE( "stacking_over_time", "[item]" )
Expand Down

0 comments on commit 8880df9

Please sign in to comment.