diff --git a/src/active_item_cache.cpp b/src/active_item_cache.cpp index 61aacbe5bfa26..de26dfc2d56ed 100644 --- a/src/active_item_cache.cpp +++ b/src/active_item_cache.cpp @@ -7,72 +7,75 @@ #include "debug.h" #include "item.h" #include "item_stack.h" +#include "safe_reference.h" -void active_item_cache::remove( item_stack::iterator it, point location ) +void active_item_cache::remove( const item *it ) { - const auto predicate = [&]( const item_reference & active_item ) { - return location == active_item.location && active_item.item_iterator == it; - }; - // The iterator is expected to be in this list, as it was added that way in `add`. - // But the processing_speed may have changed, so if it's not in the expected container, - // remove it from all containers to ensure no stale iterator remains. - auto &expected = active_items[it->processing_speed()]; - const auto iter = std::find_if( expected.begin(), expected.end(), predicate ); - if( iter != expected.end() ) { - expected.erase( iter ); - } else { - for( auto &e : active_items ) { - e.second.remove_if( predicate ); - } - } - if( active_item_set.erase( &*it ) == 0 ) { - // map erase returns number of elements erased - debugmsg( "The item isn't there!" ); - } + active_items[it->processing_speed()].remove_if( [it]( const item_reference & active_item ) { + item *const target = active_item.safe_ref.get(); + return !target || target == it; + } ); } -void active_item_cache::add( item_stack::iterator it, point location ) +void active_item_cache::add( safe_reference it, point location ) { - if( has( it, location ) ) { + item *const target = it.get(); + if( !target ) { + debugmsg( "Tried to add nonexistant item to the active item cache at (%d, %d)", location.x, + location.y ); return; } - active_items[it->processing_speed()].push_back( item_reference{ location, it, &*it } ); - active_item_set[ &*it ] = false; -} - -bool active_item_cache::has( item_stack::iterator it, point ) const -{ - return active_item_set.find( &*it ) != active_item_set.end(); + // If the item is alread in the cache for some reason, don't add a second reference + std::list &target_list = active_items[target->processing_speed()]; + if( std::find_if( target_list.begin(), + target_list.end(), [target]( const item_reference & active_item_ref ) { + return target == active_item_ref.safe_ref.get(); + } ) != target_list.end() ) { + return; + } + target_list.emplace_back( item_reference{ location, it } ); } -bool active_item_cache::has( const item_reference &itm ) const +bool active_item_cache::empty() const { - const auto found = active_item_set.find( itm.item_id ); - return found != active_item_set.end() && found->second; + return active_items.empty(); } -bool active_item_cache::empty() const +std::vector active_item_cache::get() { - return active_items.empty(); + std::vector all_cached_items; + for( std::pair> &kv : active_items ) { + for( std::list::iterator it = kv.second.begin(); it != kv.second.end(); ) { + if( it->safe_ref ) { + all_cached_items.emplace_back( *it ); + } else { + it = kv.second.erase( it ); + } + } + } + return all_cached_items; } -// get() only returns the first size() / processing_speed() elements of each list, rounded up. -// It relies on the processing logic to remove and reinsert the items to they -// move to the back of their respective lists (or to new lists). -// Otherwise only the first n items will ever be processed. -std::list active_item_cache::get() +std::vector active_item_cache::get_for_processing() { - std::list items_to_process; - for( auto &tuple : active_items ) { + std::vector items_to_process; + for( std::pair> &kv : active_items ) { // Rely on iteration logic to make sure the number is sane. - int num_to_process = tuple.second.size() / tuple.first; - for( auto &an_iter : tuple.second ) { - active_item_set[an_iter.item_id] = true; - items_to_process.push_back( an_iter ); - if( --num_to_process < 0 ) { - break; + int num_to_process = kv.second.size() / kv.first; + std::list::iterator it = kv.second.begin(); + for( ; it != kv.second.end() && num_to_process >= 0; ) { + if( it->safe_ref ) { + items_to_process.push_back( *it ); + --num_to_process; + ++it; + } else { + // The item has been destroyed, so remove the reference from the cache + it = kv.second.erase( it ); } } + // Rotate the returned items to the end of their list so that the items that weren't + // returned this time will be first in line on the next call + kv.second.splice( kv.second.end(), kv.second, kv.second.begin(), it ); } return items_to_process; } diff --git a/src/active_item_cache.h b/src/active_item_cache.h index 497d06bfc20f1..ab0422fc23f40 100644 --- a/src/active_item_cache.h +++ b/src/active_item_cache.h @@ -9,32 +9,52 @@ #include "enums.h" #include "item.h" #include "item_stack.h" +#include "safe_reference.h" // A struct used to uniquely identify an item within a submap or vehicle. struct item_reference { point location; - item_stack::iterator item_iterator; - // Do not access this from outside this module, it is only used as an ID for active_item_set. - item *item_id; + safe_reference safe_ref; }; class active_item_cache { private: std::unordered_map> active_items; - // Cache for fast lookup when we're iterating over the active items to verify the item is present. - // Key is item_id, value is whether it was returned in the last call to get - std::unordered_map active_item_set; public: - void remove( item_stack::iterator it, point location ); - void add( item_stack::iterator it, point location ); - bool has( item_stack::iterator it, point ) const; - // Use this one if there's a chance that the item being referenced has been invalidated. - bool has( const item_reference &itm ) const; + /** + * Removes the item if it is in the cache. Does nothing if the item is not in the cache. + * Relies on the fact that item::processing_speed() is a constant. + * Also removes any items that have been destroyed in the list containing it + */ + void remove( const item *it ); + + /** + * Adds the reference to the cache. Does nothing if the reference is already in the cache. + * Relies on the fact that item::processing_speed() is a constant. + */ + void add( safe_reference it, point location ); + + /** + * Returns true if the cache is empty + */ bool empty() const; - std::list get(); + /** + * Returns a vector of all cached active item references. + * Broken references are removed from the cache. + */ + std::vector get(); + + /** + * Returns the first size() / processing_speed() elements of each list, rounded up. + * Items returned are rotated to the back of their respective lists, otherwise only the + * first n items will ever be processed. + * Broken references are removed from the cache. + * Relies on the fact that item::processing_speed() is a constant. + */ + std::vector get_for_processing(); /** Subtract delta from every item_reference's location */ void subtract_locations( const point &delta ); diff --git a/src/map.cpp b/src/map.cpp index c22136a0a3bd3..60febe45d0312 100644 --- a/src/map.cpp +++ b/src/map.cpp @@ -51,6 +51,7 @@ #include "pathfinding.h" #include "projectile.h" #include "rng.h" +#include "safe_reference.h" #include "scent_map.h" #include "sounds.h" #include "string_formatter.h" @@ -4126,11 +4127,10 @@ map_stack::iterator map::i_rem( const tripoint &p, map_stack::const_iterator it point l; submap *const current_submap = get_submap_at( p, l ); - if( current_submap->active_items.has( it, l ) ) { - current_submap->active_items.remove( it, l ); - if( current_submap->active_items.empty() ) { - submaps_with_active_items.erase( abs_sub + tripoint( p.x / SEEX, p.y / SEEY, p.z ) ); - } + // remove from the active items cache (if it isn't there does nothing) + current_submap->active_items.remove( &*it ); + if( current_submap->active_items.empty() ) { + submaps_with_active_items.erase( abs_sub + tripoint( p.x / SEEX, p.y / SEEY, p.z ) ); } current_submap->update_lum_rem( l, *it ); @@ -4152,15 +4152,12 @@ void map::i_clear( const tripoint &p ) point l; submap *const current_submap = get_submap_at( p, l ); - for( auto item_it = current_submap->itm[l.x][l.y].begin(); - item_it != current_submap->itm[l.x][l.y].end(); ++item_it ) { - if( current_submap->active_items.has( item_it, l ) ) { - current_submap->active_items.remove( item_it, l ); - if( current_submap->active_items.empty() ) { - submaps_with_active_items.erase( - abs_sub + tripoint( p.x / SEEX, p.y / SEEY, p.z ) ); - } - } + for( item &it : current_submap->itm[l.x][l.y] ) { + // remove from the active items cache (if it isn't there does nothing) + current_submap->active_items.remove( &it ); + } + if( current_submap->active_items.empty() ) { + submaps_with_active_items.erase( abs_sub + tripoint( p.x / SEEX, p.y / SEEY, p.z ) ); } current_submap->lum[l.x][l.y] = 0; @@ -4382,7 +4379,7 @@ item &map::add_item( const tripoint &p, item new_item ) if( current_submap->active_items.empty() ) { submaps_with_active_items.insert( abs_sub + tripoint( p.x / SEEX, p.y / SEEY, p.z ) ); } - current_submap->active_items.add( new_pos, l ); + current_submap->active_items.add( new_pos->get_safe_reference(), l ); } return *new_pos; @@ -4446,7 +4443,7 @@ void map::make_active( item_location &loc ) submaps_with_active_items.insert( abs_sub + tripoint( loc.position().x / SEEX, loc.position().y / SEEY, loc.position().z ) ); } - current_submap->active_items.add( iter, l ); + current_submap->active_items.add( iter->get_safe_reference(), l ); } void map::update_lum( item_location &loc, bool add ) @@ -4468,38 +4465,27 @@ void map::update_lum( item_location &loc, bool add ) } } -static bool process_item( item_stack &items, item_stack::iterator &n, const tripoint &location, +static bool process_item( item_stack &items, safe_reference &safe_ref, + const tripoint &location, const bool activate, const float insulation, const temperature_flag flag ) { - // Process the item out of the map in case it destroys other items - // (for example because it is a grenade) since it might destroy itself - // and that would be bad. - item temp; - std::swap( temp, *n ); - - // Store a pointer to the target item so we can check if processing destroyed it. - item *const target = &*n; - - if( temp.process( nullptr, location, activate, insulation, flag ) ) { - // Item is to be destroyed so erase the null item in the map stack + if( safe_ref->process( nullptr, location, activate, insulation, flag ) ) { + // Item is to be destroyed so erase it from the map stack // unless it was already destroyed by processing. - item_stack::iterator it = items.get_iterator_from_pointer( target ); - if( it != items.end() ) { - items.erase( it ); + if( safe_ref ) { + items.erase( items.get_iterator_from_pointer( safe_ref.get() ) ); } return true; - } else { - // Item survived processing so replace it in the map stack - *n = std::move( temp ); - return false; } + // Item not destroyed + return false; } -static bool process_map_items( item_stack &items, item_stack::iterator &n, +static bool process_map_items( item_stack &items, safe_reference &safe_ref, const tripoint &location, const std::string &, const float insulation, const temperature_flag flag ) { - return process_item( items, n, location, false, insulation, flag ); + return process_item( items, safe_ref, location, false, insulation, flag ); } static void process_vehicle_items( vehicle &cur_veh, int part ) @@ -4589,21 +4575,22 @@ void map::process_items_in_submap( submap ¤t_submap, const tripoint &gridp // Get a COPY of the active item list for this submap. // If more are added as a side effect of processing, they are ignored this turn. // If they are destroyed before processing, they don't get processed. - std::list active_items = current_submap.active_items.get(); - const auto grid_offset = point {gridp.x * SEEX, gridp.y * SEEY}; - for( auto &active_item : active_items ) { - if( !current_submap.active_items.has( active_item ) ) { + std::vector active_items = current_submap.active_items.get_for_processing(); + const point grid_offset( gridp.x * SEEX, gridp.y * SEEY ); + for( item_reference &active_item_ref : active_items ) { + if( !active_item_ref.safe_ref.get() ) { + // The item was destroyed, so skip it. continue; } - const tripoint map_location = tripoint( grid_offset + active_item.location, gridp.z ); + const tripoint map_location = tripoint( grid_offset + active_item_ref.location, gridp.z ); // root cellars are special temperature_flag flag = temperature_flag::TEMP_NORMAL; if( g->m.ter( map_location ) == t_rootcellar ) { flag = temperature_flag::TEMP_ROOT_CELLAR; } - auto items = i_at( map_location ); - processor( items, active_item.item_iterator, map_location, signal, 1, flag ); + map_stack items = i_at( map_location ); + processor( items, active_item_ref.safe_ref, map_location, signal, 1, flag ); } } @@ -4642,28 +4629,29 @@ void map::process_items_in_vehicle( vehicle &cur_veh, submap ¤t_submap, co process_vehicle_items( cur_veh, vp.part_index() ); } - for( auto &active_item : cur_veh.active_items.get() ) { + for( item_reference &active_item_ref : cur_veh.active_items.get_for_processing() ) { if( empty( cargo_parts ) ) { return; - } else if( !cur_veh.active_items.has( active_item ) ) { + } else if( !active_item_ref.safe_ref.get() ) { + // The item was destroyed, so skip it. continue; } const auto it = std::find_if( begin( cargo_parts ), end( cargo_parts ), [&]( const vpart_reference & part ) { - return active_item.location == part.mount(); + return active_item_ref.location == part.mount(); } ); if( it == end( cargo_parts ) ) { continue; // Can't find a cargo part matching the active item. } - item_stack::iterator &item_iter = active_item.item_iterator; + const item &target = *active_item_ref.safe_ref; // Find the cargo part and coordinates corresponding to the current active item. const vehicle_part &pt = it->part(); const tripoint item_loc = it->pos(); auto items = cur_veh.get_items( static_cast( it->part_index() ) ); float it_insulation = 1.0; temperature_flag flag = temperature_flag::TEMP_NORMAL; - if( item_iter->has_temperature() || item_iter->is_food_container() ) { + if( target.has_temperature() || target.is_food_container() ) { const vpart_info &pti = pt.info(); if( engine_heater_is_on ) { flag = temperature_flag::TEMP_HEATER; @@ -4679,7 +4667,7 @@ void map::process_items_in_vehicle( vehicle &cur_veh, submap ¤t_submap, co flag = temperature_flag::TEMP_FREEZER; } } - if( !processor( items, item_iter, item_loc, signal, it_insulation, flag ) ) { + if( !processor( items, active_item_ref.safe_ref, item_loc, signal, it_insulation, flag ) ) { // If the item was NOT destroyed, we can skip the remainder, // which handles fallout from the vehicle being damaged. continue; @@ -5086,43 +5074,44 @@ std::list > map::get_rc_items( int x, int y, int z ) return rc_pairs; } -static bool trigger_radio_item( item_stack &items, item_stack::iterator &n, +static bool trigger_radio_item( item_stack &items, safe_reference &safe_ref, const tripoint &pos, const std::string &signal, const float, const temperature_flag flag ) { bool trigger_item = false; - if( n->has_flag( "RADIO_ACTIVATION" ) && n->has_flag( signal ) ) { + if( safe_ref->has_flag( "RADIO_ACTIVATION" ) && safe_ref->has_flag( signal ) ) { sounds::sound( pos, 6, sounds::sound_t::alarm, _( "beep." ), true, "misc", "beep" ); - if( n->has_flag( "RADIO_INVOKE_PROC" ) ) { + if( safe_ref->has_flag( "RADIO_INVOKE_PROC" ) ) { // Invoke twice: first to transform, then later to proc // Can't use process_item here - invalidates our iterator - n->process( nullptr, pos, true ); + safe_ref->process( nullptr, pos, true ); } - if( n->has_flag( "BOMB" ) ) { + if( safe_ref->has_flag( "BOMB" ) ) { // Set charges to 0 to ensure it detonates now - n->charges = 0; - n->item_counter = 0; + safe_ref->charges = 0; + safe_ref->item_counter = 0; } trigger_item = true; - } else if( n->has_flag( "RADIO_CONTAINER" ) && !n->contents.empty() ) { - auto it = std::find_if( n->contents.begin(), n->contents.end(), [ &signal ]( const item & c ) { + } else if( safe_ref->has_flag( "RADIO_CONTAINER" ) && !safe_ref->contents.empty() ) { + auto it = std::find_if( safe_ref->contents.begin(), + safe_ref->contents.end(), [ &signal ]( const item & c ) { return c.has_flag( signal ); } ); - if( it != n->contents.end() ) { - n->convert( it->typeId() ); - if( n->has_flag( "RADIO_INVOKE_PROC" ) ) { - n->process( nullptr, pos, true ); + if( it != safe_ref->contents.end() ) { + safe_ref->convert( it->typeId() ); + if( safe_ref->has_flag( "RADIO_INVOKE_PROC" ) ) { + safe_ref->process( nullptr, pos, true ); } // Clear possible mods to prevent unnecessary pop-ups. - n->contents.clear(); + safe_ref->contents.clear(); - n->charges = 0; + safe_ref->charges = 0; trigger_item = true; } } if( trigger_item ) { - return process_item( items, n, pos, true, 1, flag ); + return process_item( items, safe_ref, pos, true, 1, flag ); } return false; } @@ -5550,6 +5539,15 @@ void map::add_camp( const tripoint &p, const std::string &name ) g->validate_camps(); } +void map::update_submap_active_item_status( const tripoint &p ) +{ + point l; + submap *const current_submap = get_submap_at( p, l ); + if( current_submap->active_items.empty() ) { + submaps_with_active_items.erase( abs_sub + tripoint( p.x / SEEX, p.y / SEEY, p.z ) ); + } +} + void map::update_visibility_cache( const int zlev ) { visibility_variables_cache.variables_set = true; // Not used yet @@ -8402,7 +8400,7 @@ std::list map::get_active_items_in_radius( const tripoint ¢er continue; } - result.emplace_back( map_cursor( pos ), &*elem.item_iterator ); + result.emplace_back( map_cursor( pos ), elem.safe_ref.get() ); } } } diff --git a/src/map.h b/src/map.h index dc5f64448a3a0..90ce430827a58 100644 --- a/src/map.h +++ b/src/map.h @@ -1593,7 +1593,7 @@ class map * It's a really heinous function pointer so a typedef is the best * solution in this instance. */ - using map_process_func = bool ( * )( item_stack &, item_stack::iterator &, const tripoint &, + using map_process_func = bool ( * )( item_stack &, safe_reference &, const tripoint &, const std::string &, float, temperature_flag ); private: @@ -1674,6 +1674,8 @@ class map void update_visibility_cache( int zlev ); const visibility_variables &get_visibility_variables_cache() const; + void update_submap_active_item_status( const tripoint &p ); + // Clips the area to map bounds tripoint_range points_in_rectangle( const tripoint &from, const tripoint &to ) const; tripoint_range points_in_radius( const tripoint ¢er, size_t radius, size_t radiusz = 0 ) const; diff --git a/src/pickup.cpp b/src/pickup.cpp index 0371395e86f79..abb6e6881c41d 100644 --- a/src/pickup.cpp +++ b/src/pickup.cpp @@ -317,7 +317,7 @@ bool Pickup::do_pickup( std::vector &targets, std::vector &q quantities.pop_back(); if( !target ) { - debugmsg( "lost target item of ACT_DROP" ); + debugmsg( "lost target item of ACT_PICKUP" ); continue; } diff --git a/src/savegame_json.cpp b/src/savegame_json.cpp index 19c262d4d75bd..ebc92f3cf8027 100644 --- a/src/savegame_json.cpp +++ b/src/savegame_json.cpp @@ -2433,7 +2433,7 @@ void vehicle::deserialize( JsonIn &jsin ) auto end = vp.part().items.end(); for( ; it != end; ++it ) { if( it->needs_processing() ) { - active_items.add( it, vp.mount() ); + active_items.add( it->get_safe_reference(), vp.mount() ); } } } @@ -3497,7 +3497,7 @@ void submap::load( JsonIn &jsin, const std::string &member_name, bool rubpow_upd const cata::colony::iterator it = itm[p.x][p.y].insert( tmp ); if( tmp.needs_processing() ) { - active_items.add( it, p ); + active_items.add( it->get_safe_reference(), p ); } } } diff --git a/src/vehicle.cpp b/src/vehicle.cpp index 8f27fbcf02434..506bcd792ccd5 100644 --- a/src/vehicle.cpp +++ b/src/vehicle.cpp @@ -4358,12 +4358,7 @@ void vehicle::make_active( item_location &loc ) } // System insures that there is only one part in this vector. vehicle_part *cargo_part = cargo_parts.front(); - auto &item_stack = cargo_part->items; - auto item_index = std::find_if( item_stack.begin(), item_stack.end(), - [&target]( const item & i ) { - return &i == target; - } ); - active_items.add( item_index, cargo_part->mount ); + active_items.add( target->get_safe_reference(), cargo_part->mount ); } int vehicle::add_charges( int part, const item &itm ) @@ -4436,7 +4431,7 @@ cata::optional vehicle::add_item( int part, const item const vehicle_stack::iterator new_pos = parts[part].items.insert( itm_copy ); if( itm_copy.needs_processing() ) { - active_items.add( new_pos, parts[part].mount ); + active_items.add( new_pos->get_safe_reference(), parts[part].mount ); } invalidate_mass(); @@ -4458,9 +4453,8 @@ vehicle_stack::iterator vehicle::remove_item( int part, vehicle_stack::const_ite { cata::colony &veh_items = parts[part].items; - if( active_items.has( it, parts[part].mount ) ) { - active_items.remove( it, parts[part].mount ); - } + // remove from the active items cache (if it isn't there does nothing) + active_items.remove( &*it ); invalidate_mass(); return veh_items.erase( it ); diff --git a/src/visitable.cpp b/src/visitable.cpp index a2d7968b58b0b..4caafaec8aaa7 100644 --- a/src/visitable.cpp +++ b/src/visitable.cpp @@ -650,10 +650,8 @@ std::list visitable::remove_items_with( const for( auto iter = sub->itm[ offset.x ][ offset.y ].begin(); iter != sub->itm[ offset.x ][ offset.y ].end(); ) { if( filter( *iter ) ) { - // check for presence in the active items cache - if( sub->active_items.has( iter, offset ) ) { - sub->active_items.remove( iter, offset ); - } + // remove from the active items cache (if it isn't there does nothing) + sub->active_items.remove( &*iter ); // if necessary remove item from the luminosity map sub->update_lum_rem( offset, *iter ); @@ -673,6 +671,7 @@ std::list visitable::remove_items_with( const ++iter; } } + g->m.update_submap_active_item_status( *cur ); return res; } @@ -712,10 +711,8 @@ std::list visitable::remove_items_with( const vehicle_part &part = cur->veh.parts[ idx ]; for( auto iter = part.items.begin(); iter != part.items.end(); ) { if( filter( *iter ) ) { - // check for presence in the active items cache - if( cur->veh.active_items.has( iter, part.mount ) ) { - cur->veh.active_items.remove( iter, part.mount ); - } + // remove from the active items cache (if it isn't there does nothing) + cur->veh.active_items.remove( &*iter ); res.push_back( *iter ); iter = part.items.erase( iter );