From 8880df9fea4ee8b931f3e71e1393f6b26c194ba3 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Sun, 23 Jun 2019 10:04:03 +0100 Subject: [PATCH] Fix cash card stacking and display (#31732) * 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 --- src/advanced_inv.cpp | 9 ++++++--- src/inventory_ui.cpp | 4 ++-- src/item.cpp | 13 +++++++++---- src/item.h | 2 ++ src/pickup.cpp | 2 +- tests/item_test.cpp | 10 ++++++++++ 6 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/advanced_inv.cpp b/src/advanced_inv.cpp index b13a71ac5cc94..101c148dd424a 100644 --- a/src/advanced_inv.cpp +++ b/src/advanced_inv.cpp @@ -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; @@ -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_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; } diff --git a/src/inventory_ui.cpp b/src/inventory_ui.cpp index 2a81865db76f0..7a6327278a2c8 100644 --- a/src/inventory_ui.cpp +++ b/src/inventory_ui.cpp @@ -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, @@ -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 ); diff --git a/src/item.cpp b/src/item.cpp index d7f444de780aa..48e842fd48200 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -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() ); @@ -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; } @@ -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 ) { @@ -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(); @@ -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; diff --git a/src/item.h b/src/item.h index 8a3c41538d6fc..b962c6f6ad86a 100644 --- a/src/item.h +++ b/src/item.h @@ -280,6 +280,8 @@ class item : public visitable */ bool ready_to_revive( const tripoint &pos ) const; + bool is_money() const; + /** * Returns the default color of the item (e.g. @ref itype::color). */ diff --git a/src/pickup.cpp b/src/pickup.cpp index 1f13b520834ae..0371395e86f79 100644 --- a/src/pickup.cpp +++ b/src/pickup.cpp @@ -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; diff --git a/tests/item_test.cpp b/tests/item_test.cpp index 1f156353dcff2..8d27eba4a5fb2 100644 --- a/tests/item_test.cpp +++ b/tests/item_test.cpp @@ -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]" )