Optimize loading of pockets with many items inside (#68051)

* Adjust simplified `can_contain` and use for item deserialization

* Apply suggestions from code review

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Clean up ablative checks in `can_contain`

* Replace overfilled pocket debugmsg with in-world messages

* Simplify `can_holster`

* Overflow items when hauled

Stress tested this and saw no CPU impact

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

src/activity_actor.cpp

@@ -2086,6 +2086,9 @@ void move_items_activity_actor::do_turn( player_activity &act, Character &who )
 
         // Check that we can pick it up.
         if( target->made_of_from_type( phase_id::SOLID ) ) {
+            // Spill out any invalid contents first
+            target.overflow();
+
             item &leftovers = *target;
             // Make a copy to be put in the destination location
             item newit = leftovers;

src/item_contents.cpp

@@ -653,7 +653,7 @@ void item_contents::read_mods( const item_contents &read_input )
 }
 
 void item_contents::combine( const item_contents &read_input, const bool convert,
-                             const bool into_bottom, bool restack_charges )
+                             const bool into_bottom, bool restack_charges, bool ignore_contents )
 {
     std::vector<item> uninserted_items;
     size_t pocket_index = 0;
@@ -671,7 +671,7 @@ void item_contents::combine( const item_contents &read_input, const bool convert
                     pocket.is_type( item_pocket::pocket_type::MAGAZINE_WELL ) ) {
                     ++pocket_index;
                     for( const item *it : pocket.all_items_top() ) {
-                        insert_item( *it, pocket.get_pocket_data()->type );
+                        insert_item( *it, pocket.get_pocket_data()->type, ignore_contents );
                     }
                     continue;
                 } else if( pocket.is_type( item_pocket::pocket_type::MOD ) ) {
@@ -688,7 +688,7 @@ void item_contents::combine( const item_contents &read_input, const bool convert
                 } else if( pocket.saved_type() == item_pocket::pocket_type::MIGRATION ||
                            pocket.saved_type() == item_pocket::pocket_type::CORPSE ) {
                     for( const item *it : pocket.all_items_top() ) {
-                        insert_item( *it, pocket.saved_type() );
+                        insert_item( *it, pocket.saved_type(), ignore_contents );
                     }
                     ++pocket_index;
                     continue;
@@ -699,7 +699,7 @@ void item_contents::combine( const item_contents &read_input, const bool convert
 
             for( const item *it : pocket.all_items_top() ) {
                 const ret_val<item_pocket::contain_code> inserted = current_pocket_iter->insert_item( *it,
-                        into_bottom, restack_charges );
+                        into_bottom, restack_charges, ignore_contents );
                 if( !inserted.success() ) {
                     uninserted_items.push_back( *it );
                     debugmsg( "error: item %s cannot fit into pocket while loading: %s",
@@ -720,7 +720,7 @@ void item_contents::combine( const item_contents &read_input, const bool convert
     }
 
     for( const item &uninserted_item : uninserted_items ) {
-        insert_item( uninserted_item, item_pocket::pocket_type::MIGRATION );
+        insert_item( uninserted_item, item_pocket::pocket_type::MIGRATION, ignore_contents );
     }
 }
 
@@ -813,7 +813,7 @@ int item_contents::insert_cost( const item &it ) const
 }
 
 ret_val<item_pocket *> item_contents::insert_item( const item &it,
-        item_pocket::pocket_type pk_type )
+        item_pocket::pocket_type pk_type, bool ignore_contents )
 {
     if( pk_type == item_pocket::pocket_type::LAST ) {
         // LAST is invalid, so we assume it will be a regular container
@@ -828,7 +828,8 @@ ret_val<item_pocket *> item_contents::insert_item( const item &it,
         return ret_val<item_pocket *>::make_failure( nullptr, _( "Can't store anything in this." ) );
     }
 
-    ret_val<item_pocket::contain_code> pocket_contain_code = pocket.value()->insert_item( it );
+    ret_val<item_pocket::contain_code> pocket_contain_code = pocket.value()->insert_item( it, false,
+            true, ignore_contents );
     if( pocket_contain_code.success() ) {
         return pocket;
     }
@@ -2440,8 +2441,7 @@ float item_contents::relative_encumbrance() const
         nonrigid_max_volume += pocket.max_contains_volume() * modifier;
     }
     if( nonrigid_volume > nonrigid_max_volume ) {
-        debugmsg( "volume exceeds capacity (%sml > %sml)",
-                  to_milliliter( nonrigid_volume ), to_milliliter( nonrigid_max_volume ) );
+        // volume exceeds capacity and will spill until 1 or lower if picked up, so assume 1
         return 1;
     }
     if( nonrigid_max_volume == 0_ml ) {

src/item_contents.h

@@ -250,12 +250,14 @@ class item_contents
          * With CONTAINER, MAGAZINE, or MAGAZINE_WELL pocket types, items must fit the pocket's
          * volume, length, weight, ammo type, and all other physical restrictions.  This is
          * synonymous with the success of item_contents::can_contain with that item.
+         * If ignore_contents is true, will disregard other pocket contents for these checks.
          *
          * For the MOD, CORPSE, SOFTWARE, CABLE, and MIGRATION pocket types, if contents have such a
          * pocket, items will be successfully inserted without regard to volume, length, or any
          * other restrictions, since these pockets are not considered to be normal "containers".
          */
-        ret_val<item_pocket *> insert_item( const item &it, item_pocket::pocket_type pk_type );
+        ret_val<item_pocket *> insert_item( const item &it, item_pocket::pocket_type pk_type,
+                                            bool ignore_contents = false );
         void force_insert_item( const item &it, item_pocket::pocket_type pk_type );
         bool can_unload_liquid() const;
 
@@ -360,7 +362,7 @@ class item_contents
         // reads the items in the MOD pocket first
         void read_mods( const item_contents &read_input );
         void combine( const item_contents &read_input, bool convert = false, bool into_bottom = false,
-                      bool restack_charges = true );
+                      bool restack_charges = true, bool ignore_contents = false );
 
         void serialize( JsonOut &json ) const;
         void deserialize( const JsonObject &data );

src/item_pocket.cpp

@@ -1228,6 +1228,11 @@ void item_pocket::contents_info( std::vector<iteminfo> &info, int pocket_number,
                                            contains_weight() ) );
     }
 
+    if( remaining_volume() < 0_ml || remaining_weight() < 0_gram ) {
+        info.emplace_back( "DESCRIPTION",
+                           colorize( _( "This pocket is over capacity and will spill if moved!" ), c_red ) );
+    }
+
     // ablative pockets have their contents displayed earlier in the UI
     if( !is_ablative() ) {
         std::vector<std::pair<item const *, int>> counted_contents;
@@ -1403,26 +1408,20 @@ ret_val<item_pocket::contain_code> item_pocket::is_compatible( const item &it )
 }
 
 ret_val<item_pocket::contain_code> item_pocket::can_contain( const item &it,
-        int &copies_remaining ) const
-{
-    return _can_contain( it, copies_remaining, true );
-}
-
-ret_val<item_pocket::contain_code> item_pocket::can_contain( const item &it ) const
+        int &copies_remaining, bool ignore_contents ) const
 {
-    int copies = 1;
-    return _can_contain( it, copies, true );
+    return _can_contain( it, copies_remaining, ignore_contents );
 }
 
-ret_val<item_pocket::contain_code> item_pocket::can_contain_skip_space_checks(
-    const item &it ) const
+ret_val<item_pocket::contain_code> item_pocket::can_contain( const item &it,
+        bool ignore_contents ) const
 {
     int copies = 1;
-    return _can_contain( it, copies, false );
+    return _can_contain( it, copies, ignore_contents );
 }
 
 ret_val<item_pocket::contain_code> item_pocket::_can_contain( const item &it,
-        int &copies_remaining, const bool check_for_enough_space ) const
+        int &copies_remaining, const bool ignore_contents ) const
 {
     ret_val<item_pocket::contain_code> compatible = is_compatible( it );
 
@@ -1480,8 +1479,8 @@ ret_val<item_pocket::contain_code> item_pocket::_can_contain( const item &it,
                    contain_code::ERR_GAS, _( "can't put non gas into pocket with gas" ) );
     }
 
-    if( !check_for_enough_space ) {
-        // Skip all the checks that could result in NO_SPACE or CANNOT_SUPPORT errors.
+    if( ignore_contents ) {
+        // Skip all the checks against other pocket contents.
         if( it.weight() > weight_capacity() ) {
             return ret_val<item_pocket::contain_code>::make_failure(
                        contain_code::ERR_TOO_HEAVY, _( "item is too heavy" ) );
@@ -1504,17 +1503,17 @@ ret_val<item_pocket::contain_code> item_pocket::_can_contain( const item &it,
         }
     }
 
-    if( data->ablative && !contents.empty() ) {
-        if( contents.front().can_combine( it ) ) {
-            // Only items with charges can succeed here.
-            return ret_val<item_pocket::contain_code>::make_success();
-        } else {
-            return ret_val<item_pocket::contain_code>::make_failure(
-                       contain_code::ERR_NO_SPACE, _( "ablative pocket already contains a plate" ) );
+    if( data->ablative ) {
+        if( !contents.empty() ) {
+            if( contents.front().can_combine( it ) ) {
+                // Only items with charges can succeed here.
+                return ret_val<item_pocket::contain_code>::make_success();
+            } else {
+                return ret_val<item_pocket::contain_code>::make_failure(
+                           contain_code::ERR_NO_SPACE, _( "ablative pocket already contains a plate" ) );
+            }
         }
-    }
 
-    if( data->ablative ) {
         if( it.is_rigid() ) {
             for( const sub_bodypart_id &sbp : it.get_covered_sub_body_parts() ) {
                 if( it.is_bp_rigid( sbp ) && std::count( no_rigid.begin(), no_rigid.end(), sbp ) != 0 ) {
@@ -1783,7 +1782,7 @@ void item_pocket::overflow( const tripoint &pos, const item_location &loc )
 
         // if item has any contents, check it individually
         if( !iter->get_contents().empty_with_no_mods() ) {
-            if( !is_type( pocket_type::MIGRATION ) && can_contain_skip_space_checks( *iter ).success() ) {
+            if( !is_type( pocket_type::MIGRATION ) && can_contain( *iter, true ).success() ) {
                 ++iter;
             } else {
                 move_to_parent_pocket_recursive( pos, *iter, loc );
@@ -1796,7 +1795,7 @@ void item_pocket::overflow( const tripoint &pos, const item_location &loc )
         auto cont_copy_type = contained_type_validity.emplace( iter->typeId(), true );
         if( cont_copy_type.second ) {
             cont_copy_type.first->second = !is_type( pocket_type::MIGRATION ) &&
-                                           can_contain_skip_space_checks( *iter ).success();
+                                           can_contain( *iter, true ).success();
         }
         if( cont_copy_type.first->second ) {
             ++iter;
@@ -2141,10 +2140,10 @@ std::list<item> &item_pocket::edit_contents()
 }
 
 ret_val<item_pocket::contain_code> item_pocket::insert_item( const item &it,
-        const bool into_bottom, bool restack_charges )
+        const bool into_bottom, bool restack_charges, bool ignore_contents )
 {
     ret_val<item_pocket::contain_code> ret = !is_standard_type() ?
-            ret_val<item_pocket::contain_code>::make_success() : can_contain( it );
+            ret_val<item_pocket::contain_code>::make_success() : can_contain( it, ignore_contents );
 
     if( ret.success() ) {
         if( !into_bottom ) {

src/item_pocket.h

@@ -197,14 +197,13 @@ class item_pocket
         /**
          * Can the pocket contain the specified item?
          * @param it The item being put in
+         * @param ignore_contents If true, only check for compatible phase, size, and weight, skipping the more CPU-intensive checks against other contents. Optional, default false.
          * @param copies_remaining An optional integer reference that will be set to the number of item copies that won't fit
          */
-        ret_val<contain_code> can_contain( const item &it, int &copies_remaining ) const;
-        ret_val<contain_code> can_contain( const item &it ) const;
-        /**
-        * @brief A version of can_contain that skips the weight and volume check.
-        */
-        ret_val<contain_code> can_contain_skip_space_checks( const item &it ) const;
+        ret_val<contain_code> can_contain( const item &it, bool ignore_contents = false ) const;
+        ret_val<contain_code> can_contain( const item &it, int &copies_remaining,
+                                           bool ignore_contents = false ) const;
+
         bool can_contain_liquid( bool held_or_ground ) const;
         bool contains_phase( phase_id phase ) const;
 
@@ -331,7 +330,7 @@ class item_pocket
 
         // tries to put an item in the pocket. returns false if failure
         ret_val<contain_code> insert_item( const item &it, bool into_bottom = false,
-                                           bool restack_charges = true );
+                                           bool restack_charges = true, bool ignore_contents = false );
         /**
           * adds an item to the pocket with no checks
           * may create a new pocket
@@ -427,7 +426,7 @@ class item_pocket
         std::set<sub_bodypart_id> no_rigid;
 
         ret_val<contain_code> _can_contain( const item &it, int &copies_remaining,
-                                            bool check_for_enough_space ) const;
+                                            bool ignore_contents ) const;
 };
 
 /**

src/iuse_actor.cpp

@@ -2396,13 +2396,7 @@ void holster_actor::load( const JsonObject &obj )
 
 bool holster_actor::can_holster( const item &holster, const item &obj ) const
 {
-    if( !holster.can_contain( obj ).success() ) {
-        return false;
-    }
-    if( obj.active ) {
-        return false;
-    }
-    return holster.can_contain( obj ).success();
+    return obj.active ? false : holster.can_contain( obj ).success();
 }
 
 bool holster_actor::store( Character &you, item &holster, item &obj ) const

src/savegame_json.cpp

@@ -3222,7 +3222,7 @@ void item::deserialize( const JsonObject &data )
 
         contents.read_mods( read_contents );
         update_modified_pockets();
-        contents.combine( read_contents, false, true, false );
+        contents.combine( read_contents, false, true, false, true );
 
         if( data.has_object( "contents" ) ) {
             JsonObject tested = data.get_object( "contents" );