Fix pocket obtain shenanigans (#40322)

src/avatar_action.cpp

@@ -1235,16 +1235,11 @@ void avatar_action::use_item( avatar &you, item_location &loc )
         if( loc->has_flag( flag_ALLOWS_REMOTE_USE ) ) {
             use_in_place = true;
         } else {
-            const int obtain_cost = loc.obtain_cost( you );
             loc = loc.obtain( you );
             if( !loc ) {
                 debugmsg( "Failed to obtain target item" );
                 return;
             }
-
-            // TODO: the following comment is inaccurate and this mechanic needs to be rexamined
-            // This method only handles items in the inventory, so refund the obtain cost.
-            you.mod_moves( obtain_cost );
         }
     }
 

src/item_location.cpp

@@ -92,6 +92,7 @@ class item_location::impl
             ensure_unpacked();
             return !!what;
         }
+
     private:
         void ensure_unpacked() const {
             if( needs_unpacking ) {
@@ -551,8 +552,16 @@ class item_location::impl::item_in_container : public item_location::impl
             item obj = target()->split( qty );
             if( !obj.is_null() ) {
                 return item_location( ch, &ch.i_add( obj, should_stack ) );
+            } else if( container.held_by( ch ) ) {
+                // we don't need to move it in this case, it's in a pocket
+                // we just charge the obtain cost and leave it in place. otherwise
+                // it's liable to end up back in the same pocket, where shenanigans ensue
+                return item_location( container, target() );
             } else {
                 item *inv = &ch.i_add( *target(), should_stack );
+                if( inv->is_null() ) {
+                    debugmsg( "failed to add item to character inventory while obtaining from container" );
+                }
                 remove_item();
                 return item_location( ch, inv );
             }
@@ -685,6 +694,14 @@ item_location item_location::parent_item() const
     return item_location::nowhere;
 }
 
+bool item_location::has_parent() const
+{
+    if( where() == type::container ) {
+        return !!ptr->parent_item();
+    }
+    return false;
+}
+
 item_location::type item_location::where() const
 {
     return ptr->where();
@@ -738,3 +755,13 @@ void item_location::set_should_stack( bool should_stack ) const
 {
     ptr->should_stack = should_stack;
 }
+
+bool item_location::held_by( Character &who ) const
+{
+    if( where() == type::character && g->critter_at<Character>( position() ) == &who ) {
+        return true;
+    } else if( has_parent() ) {
+        return parent_item().held_by( who );
+    }
+    return false;
+}

src/item_location.h

@@ -90,6 +90,16 @@ class item_location
         /** returns the parent item, or an invalid location if it has no parent */
         item_location parent_item() const;
 
+        /** returns true if the item is in the inventory of the given character **/
+        bool held_by( Character &who ) const;
+
+        /**
+         * true if this item location can and does have a parent
+         *
+         * exists because calling parent_item() naively causes debug messages
+         **/
+        bool has_parent() const;
+
     private:
         class impl;