Fix items from losing functionality when manually inserted and/or pro…

…viding player light when used remotely (#69295) * Make both versions of restack handle magazines * Add `carrier` function to item_location to get the carrying Character * Remove non-working inv search cache call * Exit put_in() early if it fails There's no reason to do the rest if nothing was actually inserted * Make insert_item return ret_val<item *>, use for inv search caches Pocket unsealing is now handled inside item_contents::insert_item * Add items with charges to inv search caches on insertion * Make inventory's Organize Contents work with inv search caches * Revert how item_location::held_by works to fix NPC tests npc_shopkeeper_item_groups was throwing errors and fixing it was proving too complicated to wrap into this PR * Only cache transformed items if character is actually holding them * Fix shopkeeper test to allow simplifying held_by Co-authored-by: andrei <[email protected]> --------- Co-authored-by: andrei <[email protected]>
CleverRaven · Nov 13, 2023 · a8bf414 · a8bf414
1 parent d39ddd6
commit a8bf414
Show file tree

Hide file tree

Showing 15 changed files with 161 additions and 99 deletions.
diff --git a/src/activity_actor.cpp b/src/activity_actor.cpp
@@ -4287,7 +4287,7 @@ void insert_item_activity_actor::start( player_activity &act, Character &who )
 }
 
 static ret_val<void> try_insert( item_location &holster, drop_location &holstered_item,
-                                 int *charges_added )
+                                 int *charges_added, Character *carrier )
 {
     item &it = *holstered_item.first;
     ret_val<void> ret = ret_val<void>::make_failure( _( "item can't be stored there" ) );
@@ -4306,7 +4306,7 @@ static ret_val<void> try_insert( item_location &holster, drop_location &holstere
             return ret;
         }
 
-        return holster->put_in( it, item_pocket::pocket_type::CONTAINER, /*unseal_pockets=*/true );
+        return holster->put_in( it, item_pocket::pocket_type::CONTAINER, /*unseal_pockets=*/true, carrier );
     }
 
     ret = holster->can_contain_partial_directly( it );
@@ -4319,7 +4319,7 @@ static ret_val<void> try_insert( item_location &holster, drop_location &holstere
     }
     int charges_to_insert = std::min( holstered_item.second, max_parent_charges.value() );
     *charges_added = holster->fill_with( it, charges_to_insert, /*unseal_pockets=*/true,
-                                         /*allow_sealed=*/true, /*ignore_settings*/true, /*into_bottom*/true );
+                                         /*allow_sealed=*/true, /*ignore_settings*/true, /*into_bottom*/true, carrier );
     if( *charges_added <= 0 ) {
         return ret_val<void>::make_failure( _( "item can't be stored there" ) );
     }
@@ -4331,6 +4331,7 @@ void insert_item_activity_actor::finish( player_activity &act, Character &who )
 {
     bool success = false;
     bool bulk_load = false;
+    Character *carrier = holster.carrier();
     drop_location &holstered_item = items.front();
     if( holstered_item.first ) {
         success = true;
@@ -4343,20 +4344,19 @@ void insert_item_activity_actor::finish( player_activity &act, Character &who )
         ret_val<void> ret = ret_val<void>::make_failure( _( "item can't be stored there" ) );
 
         if( !it.count_by_charges() ) {
-            ret = try_insert( holster, holstered_item, nullptr );
+            ret = try_insert( holster, holstered_item, nullptr, carrier );
 
             if( ret.success() ) {
                 //~ %1$s: item to put in the container, %2$s: container to put item in
                 who.add_msg_if_player( string_format( _( "You put your %1$s into the %2$s." ),
                                                       holstered_item.first->display_name(), holster->type->nname( 1 ) ) );
-                who.add_to_inv_search_caches( *holstered_item.first );
                 handler.add_unsealed( holster );
                 handler.unseal_pocket_containing( holstered_item.first );
                 holstered_item.first.remove_item();
             }
         } else {
             int charges_added = 0;
-            ret = try_insert( holster, holstered_item, &charges_added );
+            ret = try_insert( holster, holstered_item, &charges_added, carrier );
 
             if( ret.success() ) {
                 item copy( it );
@@ -4570,10 +4570,9 @@ void reload_activity_actor::finish( player_activity &act, Character &who )
         case 2:
         default:
             // In player inventory and player is wielding something.
-            if( loc.held_by( who ) ) {
-                add_msg( m_neutral, _( "The %s no longer fits in your inventory so you drop it instead." ),
-                         reloadable_name );
-            }
+            loc.carrier()->add_msg_if_player( m_neutral,
+                                              _( "The %s no longer fits in your inventory so you drop it instead." ),
+                                              reloadable_name );
             get_map().add_item_or_charges( loc.position(), reloadable );
             loc.remove_item();
             break;

diff --git a/src/character.cpp b/src/character.cpp
@@ -12453,7 +12453,8 @@ void Character::store( item_pocket *pocket, item &put, bool penalties, int base_
     }
     moves -= std::max( item_store_cost( put, null_item_reference(), penalties, base_cost ),
                        pocket->obtain_cost( put ) );
-    pocket->insert_item( i_rem( &put ) );
+    ret_val<item *> result = pocket->insert_item( i_rem( &put ) );
+    result.value()->on_pickup( *this );
     calc_encumbrance();
 }
 

diff --git a/src/character_attire.cpp b/src/character_attire.cpp
@@ -2442,7 +2442,8 @@ void outfit::add_stash( Character &guy, const item &newit, int &remaining_charge
         item_location carried_item = guy.get_wielded_item();
         if( carried_item && !carried_item->has_pocket_type( item_pocket::pocket_type::MAGAZINE ) &&
             carried_item->can_contain_partial( newit ).success() ) {
-            int used_charges = carried_item->fill_with( newit, remaining_charges, false, false, false );
+            int used_charges = carried_item->fill_with( newit, remaining_charges, /*unseal_pockets=*/false,
+                               /*allow_sealed=*/false, /*ignore_settings=*/false, /*into_bottom*/false, &guy );
             remaining_charges -= used_charges;
         }
         // Crawl Next : worn items

diff --git a/src/game_inventory.cpp b/src/game_inventory.cpp
@@ -487,13 +487,9 @@ class liquid_inventory_selector_preset : public inventory_selector_preset
             if( location.get_item() == avoid ) {
                 return false;
             }
-            if( location.where() == item_location::type::character ) {
-                Character *character = get_creature_tracker().creature_at<Character>( location.position() );
-                if( character == nullptr ) {
-                    debugmsg( "Invalid location supplied to the liquid filter: no character found." );
-                    return false;
-                }
-                return location->get_remaining_capacity_for_liquid( liquid, *character ) > 0;
+            Character *carrier = location.carrier();
+            if( carrier != nullptr ) {
+                return location->get_remaining_capacity_for_liquid( liquid, *carrier ) > 0;
             }
             const bool allow_buckets = location.where() == item_location::type::map;
             return location->get_remaining_capacity_for_liquid( liquid, allow_buckets ) > 0;

diff --git a/src/item.cpp b/src/item.cpp
@@ -648,7 +648,7 @@ item &item::convert( const itype_id &new_type, Character *carrier )
         countdown_point = calendar::turn + type->countdown_interval;
         active = true;
     }
-    if( carrier ) {
+    if( carrier && carrier->has_item( *this ) ) {
         carrier->on_item_acquire( *this );
     }
 
@@ -1614,25 +1614,22 @@ int item::insert_cost( const item &it ) const
 }
 
 ret_val<void> item::put_in( const item &payload, item_pocket::pocket_type pk_type,
-                            const bool unseal_pockets )
+                            const bool unseal_pockets, Character *carrier )
 {
-    ret_val<item_pocket *> result = contents.insert_item( payload, pk_type );
+    ret_val<item *> result = contents.insert_item( payload, pk_type, false, unseal_pockets );
     if( !result.success() ) {
         debugmsg( "tried to put an item (%s) count (%d) in a container (%s) that cannot contain it: %s",
                   payload.typeId().str(), payload.count(), typeId().str(), result.str() );
+        return ret_val<void>::make_failure( result.str() );
     }
     if( pk_type == item_pocket::pocket_type::MOD ) {
         update_modified_pockets();
     }
-    if( unseal_pockets && result.success() ) {
-        result.value()->unseal();
+    if( carrier ) {
+        result.value()->on_pickup( *carrier );
     }
     on_contents_changed();
-    if( result.success() ) {
-        return ret_val<void>::make_success( result.str() );
-    } else {
-        return ret_val<void>::make_failure( result.str() );
-    }
+    return ret_val<void>::make_success( result.str() );
 }
 
 void item::force_insert_item( const item &it, item_pocket::pocket_type pk_type )
@@ -12104,7 +12101,8 @@ int item::fill_with( const item &contained, const int amount,
                      const bool unseal_pockets,
                      const bool allow_sealed,
                      const bool ignore_settings,
-                     const bool into_bottom )
+                     const bool into_bottom,
+                     Character *carrier )
 {
     if( amount <= 0 ) {
         return 0;
@@ -12148,11 +12146,13 @@ int item::fill_with( const item &contained, const int amount,
             }
         }
 
-        if( !pocket->insert_item( contained_item, into_bottom ).success() ) {
+        ret_val<item *> result = pocket->insert_item( contained_item, into_bottom );
+        if( !result.success() ) {
             if( count_by_charges ) {
-                debugmsg( "charges per remaining pocket volume does not fit in that very volume" );
+                debugmsg( "charges per remaining pocket volume does not fit in that very volume: %s",
+                          result.str() );
             } else {
-                debugmsg( "best pocket for item cannot actually contain the item" );
+                debugmsg( "best pocket for item cannot actually contain the item: %s", result.str() );
             }
             break;
         }
@@ -12164,6 +12164,9 @@ int item::fill_with( const item &contained, const int amount,
         if( unseal_pockets ) {
             pocket->unseal();
         }
+        if( carrier ) {
+            result.value()->on_pickup( *carrier );
+        }
     }
     if( num_contained == 0 ) {
         debugmsg( "tried to put an item (%s, amount %d) in a container (%s) that cannot contain it",

diff --git a/src/item.h b/src/item.h
@@ -865,7 +865,8 @@ class item : public visitable
                        bool unseal_pockets = false,
                        bool allow_sealed = false,
                        bool ignore_settings = false,
-                       bool into_bottom = false );
+                       bool into_bottom = false,
+                       Character *carrier = nullptr );
 
         /**
          * How much more of this liquid (in charges) can be put in this container.
@@ -932,7 +933,7 @@ class item : public visitable
          * Puts the given item into this one.
          */
         ret_val<void> put_in( const item &payload, item_pocket::pocket_type pk_type,
-                              bool unseal_pockets = false );
+                              bool unseal_pockets = false, Character *carrier = nullptr );
         void force_insert_item( const item &it, item_pocket::pocket_type pk_type );
 
         /**

diff --git a/src/item_contents.cpp b/src/item_contents.cpp
@@ -698,8 +698,8 @@ void item_contents::combine( const item_contents &read_input, const bool convert
             std::advance( current_pocket_iter, pocket_index );
 
             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, ignore_contents );
+                const ret_val<item *> inserted = current_pocket_iter->insert_item( *it,
+                                                 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",
@@ -812,8 +812,8 @@ 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, bool ignore_contents )
+ret_val<item *> item_contents::insert_item( const item &it,
+        item_pocket::pocket_type pk_type, bool ignore_contents, const bool unseal_pockets )
 {
     if( pk_type == item_pocket::pocket_type::LAST ) {
         // LAST is invalid, so we assume it will be a regular container
@@ -822,18 +822,20 @@ ret_val<item_pocket *> item_contents::insert_item( const item &it,
 
     ret_val<item_pocket *> pocket = find_pocket_for( it, pk_type );
     if( !pocket.success() ) {
-        return pocket;
+        return ret_val<item *>::make_failure( nullptr, pocket.str() );
     }
     if( pocket.value()->is_forbidden() ) {
-        return ret_val<item_pocket *>::make_failure( nullptr, _( "Can't store anything in this." ) );
+        return ret_val<item *>::make_failure( nullptr, _( "Can't store anything in this." ) );
     }
 
-    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;
+    ret_val<item *> inserted = pocket.value()->insert_item( it, false, true, ignore_contents );
+    if( inserted.success() ) {
+        if( unseal_pockets ) {
+            pocket.value()->unseal();
+        }
+        return inserted;
     }
-    return ret_val<item_pocket *>::make_failure( nullptr, pocket_contain_code.str() );
+    return ret_val<item *>::make_failure( nullptr, inserted.str() );
 }
 
 void item_contents::force_insert_item( const item &it, item_pocket::pocket_type pk_type )

diff --git a/src/item_contents.h b/src/item_contents.h
@@ -256,8 +256,8 @@ class item_contents
          * 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,
-                                            bool ignore_contents = false );
+        ret_val<item *> insert_item( const item &it, item_pocket::pocket_type pk_type,
+                                     bool ignore_contents = false, bool unseal_pockets = false );
         void force_insert_item( const item &it, item_pocket::pocket_type pk_type );
         bool can_unload_liquid() const;
 

diff --git a/src/item_location.cpp b/src/item_location.cpp
@@ -89,6 +89,7 @@ class item_location::impl
             return nullptr;
         }
         virtual tripoint position() const = 0;
+        virtual Character *carrier() const = 0;
         virtual std::string describe( const Character * ) const = 0;
         virtual item_location obtain( Character &, int ) = 0;
         virtual units::volume volume_capacity() const = 0;
@@ -143,6 +144,10 @@ class item_location::impl::nowhere : public item_location::impl
             return tripoint_min;
         }
 
+        Character *carrier() const override {
+            return nullptr;
+        }
+
         std::string describe( const Character * ) const override {
             debugmsg( "invalid use of nowhere item_location" );
             return "";
@@ -218,6 +223,10 @@ class item_location::impl::item_on_map : public item_location::impl
             return cur.pos();
         }
 
+        Character *carrier() const override {
+            return nullptr;
+        }
+
         std::string describe( const Character *ch ) const override {
             std::string res = get_map().name( cur.pos() );
             if( ch ) {
@@ -343,6 +352,13 @@ class item_location::impl::item_on_person : public item_location::impl
             return who->pos();
         }
 
+        Character *carrier() const override {
+            if( !ensure_who_unpacked() ) {
+                return nullptr;
+            }
+            return who;
+        }
+
         std::string describe( const Character *ch ) const override {
             if( !target() || !ensure_who_unpacked() ) {
                 return std::string();
@@ -469,6 +485,10 @@ class item_location::impl::item_on_vehicle : public item_location::impl
             return cur.veh.global_part_pos3( cur.part );
         }
 
+        Character *carrier() const override {
+            return nullptr;
+        }
+
         std::string describe( const Character *ch ) const override {
             const vpart_position part_pos( cur.veh, cur.part );
             std::string res;
@@ -613,6 +633,10 @@ class item_location::impl::item_in_container : public item_location::impl
             }
         }
 
+        Character *carrier() const override {
+            return container.carrier();
+        }
+
         std::string describe( const Character * ) const override {
             if( !target() ) {
                 return std::string();
@@ -1074,15 +1098,14 @@ void item_location::set_should_stack( bool should_stack ) const
     ptr->should_stack = should_stack;
 }
 
+Character *item_location::carrier() const
+{
+    return ptr->carrier();
+}
+
 bool item_location::held_by( Character const &who ) const
 {
-    if( where() == type::character &&
-        get_creature_tracker().creature_at<Character>( position() ) == &who ) {
-        return true;
-    } else if( has_parent() ) {
-        return parent_item().held_by( who );
-    }
-    return false;
+    return carrier() == &who;
 }
 
 units::volume item_location::volume_capacity() const

diff --git a/src/item_location.h b/src/item_location.h
@@ -110,6 +110,9 @@ class item_location
         item_location parent_item() const;
         item_pocket *parent_pocket() const;
 
+        /** returns the character whose inventory contains this item, nullptr if none **/
+        Character *carrier() const;
+
         /** returns true if the item is in the inventory of the given character **/
         bool held_by( Character const &who ) const;