cataclysmbnteam · Coolthulhu · Apr 10, 2022 · Jun 3, 2022 · Jun 5, 2022 · Jun 23, 2022
diff --git a/src/activity_actor.cpp b/src/activity_actor.cpp
@@ -688,7 +688,7 @@ drop_activity_actor::drop_activity_actor( Character &ch, const drop_locations &i
         bool force_ground, const tripoint &relpos )
     : force_ground( force_ground ), relpos( relpos )
 {
-    this->items = pickup::reorder_for_dropping( ch, items );
+    this->items = pickup::reorder_for_dropping( ch, items, pickup::nonconst{} );
 }
 
 void drop_activity_actor::start( player_activity &act, Character & )
@@ -1123,7 +1123,7 @@ void wash_activity_actor::start( player_activity &act, Character & )
 stash_activity_actor::stash_activity_actor( Character &ch, const drop_locations &items,
         const tripoint &relpos ) : relpos( relpos )
 {
-    this->items = pickup::reorder_for_dropping( ch, items );
+    this->items = pickup::reorder_for_dropping( ch, items, pickup::nonconst{} );
 }
 
 void stash_activity_actor::start( player_activity &act, Character & )

diff --git a/src/activity_item_handling.cpp b/src/activity_item_handling.cpp
@@ -408,14 +408,14 @@ void put_into_vehicle_or_drop( Character &c, item_drop_reason reason, const std:
     drop_on_map( c, reason, items, where );
 }
 
-static std::list<pickup::act_item> convert_to_items( Character &p,
+static std::list<pickup::const_act_item> convert_to_items( const Character &p,
         const drop_locations &drop,
-        std::function<bool( item_location loc )> filter )
+        std::function<bool( const const_item_location &loc )> filter )
 {
-    std::list<pickup::act_item> res;
+    std::list<pickup::const_act_item> res;
 
     for( const drop_location &rec : drop ) {
-        const item_location loc = rec.loc;
+        const const_item_location loc = const_item_location( rec.loc );
         const int count = rec.count;
 
         if( !filter( loc ) ) {
@@ -438,7 +438,7 @@ static std::list<pickup::act_item> convert_to_items( Character &p,
                 }
                 const int qty = it.count_by_charges() ? std::min<int>( it.charges, count - obtained ) : 1;
                 obtained += qty;
-                item_location loc( p, const_cast<item *>( &it ) );
+                const_item_location loc = item_location::make_const( p, &it );
                 res.emplace_back( loc, qty, loc.obtain_cost( p, qty ) );
             }
         } else {
@@ -452,88 +452,119 @@ static std::list<pickup::act_item> convert_to_items( Character &p,
 namespace pickup
 {
 
+static std::list<pickup::act_item> recreate_as_nonconst(
+    const std::list<pickup::const_act_item> &l )
+{
+    std::list<pickup::act_item> ret;
+    std::transform( l.begin(), l.end(), std::back_inserter( ret ),
+    []( const pickup::const_act_item & ai ) {
+        // TODO: Make it safely recreate the location instead of hacking it with effectively-a-cast
+        return pickup::act_item( item_location( ai.loc ), ai.count, ai.consumed_moves );
+    } );
+    return ret;
+}
+
+std::list<act_item> reorder_for_dropping( Character &p, const drop_locations &drop, nonconst )
+{
+    return recreate_as_nonconst( reorder_for_dropping( p, drop ) );
+}
+
 // Prepares items for dropping by reordering them so that the drop
 // cost is minimal and "dependent" items get taken off first.
 // Implements the "backpack" logic.
-std::list<act_item> reorder_for_dropping( Character &p, const drop_locations &drop )
+std::list<const_act_item> reorder_for_dropping( const Character &p, const drop_locations &drop )
 {
-    std::list<act_item> res = convert_to_items( p, drop,
-    [&p]( item_location loc ) {
+    // Potentially slow: O(n*m), where n is inentory size and m is selection size
+    // Could be optimized by building a set of worn/wielded items beforehand
+    std::list<const_act_item> res = convert_to_items( p, drop,
+    [&p]( const const_item_location & loc ) {
         return p.is_wielding( *loc );
     } );
-    std::list<act_item> inv = convert_to_items( p, drop,
-    [&p]( item_location loc ) {
+    std::list<const_act_item> inv = convert_to_items( p, drop,
+    [&p]( const const_item_location & loc ) {
         return !p.is_wielding( *loc ) && !p.is_worn( *loc );
     } );
-    std::list<act_item> worn = convert_to_items( p, drop,
-    [&p]( item_location loc ) {
+    std::list<const_act_item> worn = convert_to_items( p, drop,
+    [&p]( const const_item_location & loc ) {
         return p.is_worn( *loc );
     } );
 
     // Sort inventory items by volume in ascending order
-    inv.sort( []( const act_item & first, const act_item & second ) {
+    inv.sort( []( const const_act_item & first, const const_act_item & second ) {
         return first.loc->volume() < second.loc->volume();
     } );
     // Add missing dependent worn items (if any).
     for( const auto &wait : worn ) {
-        for( item *dit : p.get_dependent_worn_items( *wait.loc ) ) {
+        for( const item *dit : p.get_dependent_worn_items( *wait.loc ) ) {
             const auto iter = std::find_if( worn.begin(), worn.end(),
-            [dit]( const act_item & ait ) {
+            [dit]( const const_act_item & ait ) {
                 return &*ait.loc == dit;
             } );
 
             if( iter == worn.end() ) {
                 // TODO: Use a calculated cost
-                const item_location loc( p, dit );
-                act_item act( loc, loc->count(), loc.obtain_cost( p, loc->count() ) );
-                worn.emplace_front( loc, loc->count(), loc.obtain_cost( p ) );
+                const const_item_location loc = item_location::make_const( p, dit );
+                int cost = item_handling::takeoff_cost( p, *loc ) * loc->count() +
+                           loc.obtain_cost( p, loc->count() );
+                const_act_item act( loc, loc->count(), cost );
+                worn.emplace_front( loc, loc->count(), cost );
             }
         }
     }
     // Sort worn items by storage in descending order, but dependent items always go first.
-    worn.sort( []( const act_item & first, const act_item & second ) {
+    worn.sort( []( const const_act_item & first, const const_act_item & second ) {
         return first.loc->is_worn_only_with( *second.loc )
                || ( first.loc->get_storage() > second.loc->get_storage()
                     && !second.loc->is_worn_only_with( *first.loc ) );
     } );
 
     // Avoid tumbling to the ground. Unload cleanly.
     units::volume dropped_inv_contents = std::accumulate( inv.begin(), inv.end(), 0_ml,
-    []( units::volume acc, const act_item & ait ) {
+    []( units::volume acc, const const_act_item & ait ) {
         return acc + ait.loc->volume();
     } );
     const units::volume dropped_worn_storage = std::accumulate( worn.begin(), worn.end(), 0_ml,
-    []( units::volume acc, const act_item & ait ) {
+    []( units::volume acc, const const_act_item & ait ) {
         return acc + ait.loc->get_storage();
     } );
-    std::set<int> inv_indices;
-    std::transform( inv.begin(), inv.end(), std::inserter( inv_indices, inv_indices.begin() ),
-    [&p]( const act_item & ait ) {
-        return p.get_item_position( &*ait.loc );
+    std::map<int, int> intentional_drops_by_position;
+    std::transform( inv.begin(), inv.end(),
+                    std::inserter( intentional_drops_by_position, intentional_drops_by_position.end() ),
+    [&p]( const const_act_item & ait ) {
+        return std::make_pair( p.get_item_position( &*ait.loc ), ait.count );
     } );
 
     units::volume excessive_volume = p.volume_carried() - dropped_inv_contents
                                      - p.volume_capacity_reduced_by( dropped_worn_storage );
     if( excessive_volume > 0_ml ) {
-        invslice old_inv = p.inv.slice();
+        const_invslice old_inv = p.inv.const_slice();
         for( size_t i = 0; i < old_inv.size() && excessive_volume > 0_ml; i++ ) {
             // TODO: Reimplement random dropping?
-            if( inv_indices.count( i ) != 0 ) {
-                continue;
-            }
-            std::list<item> &inv_stack = *old_inv[i];
-            for( item &item : inv_stack ) {
-                // Note: zero cost, but won't be contained on drop
-                act_item to_drop = act_item( item_location( p, &item ), item.count(), 0 );
-                inv.push_back( to_drop );
-                excessive_volume -= to_drop.loc->volume();
-                if( excessive_volume <= 0_ml ) {
-                    break;
+            // We possibly processed some of those items already, so we must skip those
+            auto iter = intentional_drops_by_position.find( i );
+            int to_skip = iter != intentional_drops_by_position.end() ?
+                          iter->second :
+                          0;
+            const std::list<item> &inv_stack = *old_inv[i];
+            for( const item &item : inv_stack ) {
+                int min = std::min( item.count(), to_skip );
+                to_skip = std::max( 0, to_skip - min );
+                if( to_skip == 0 ) {
+                    // Note: zero cost, but won't be contained on drop
+                    // Dropping the same item twice causes problems due to same item ptr
+                    // so we drop the full item instead
+                    // TODO: Split dropped item into intentional and implied parts, if both are present
+                    const_act_item to_drop = const_act_item( item_location::make_const( p, &item ), item.count(), 0 );
+                    inv.push_back( to_drop );
+                    excessive_volume -= to_drop.loc->volume();
+                    if( excessive_volume <= 0_ml ) {
+                        break;
+                    }
                 }
             }
         }
         // Need to re-sort
-        inv.sort( []( const act_item & first, const act_item & second ) {
+        inv.sort( []( const const_act_item & first, const const_act_item & second ) {
             return first.loc->volume() < second.loc->volume();
         } );
     }
@@ -608,12 +639,10 @@ std::list<item> obtain_and_tokenize_items( player &p, std::list<act_item> &items
         p.mod_moves( -ait.consumed_moves );
 
         if( p.is_worn( *ait.loc ) ) {
-            if( !p.takeoff( *ait.loc, &res ) ) {
-                // Skip item if failed to take it off
-                debugmsg( "Failed to obtain worn target item of ACT_DROP" );
-                items.pop_front();
-                continue;
-            }
+            res.emplace_back( *ait.loc );
+            // Can't use takeoff, it costs moves and we already paid them
+            p.i_rem( &*ait.loc );
+            // Hack alert! TODO: Handle failure!
         } else if( ait.loc->count_by_charges() ) {
             res.push_back( p.reduce_charges( const_cast<item *>( &*ait.loc ), ait.count ) );
         } else {

diff --git a/src/character.cpp b/src/character.cpp
@@ -2432,6 +2432,7 @@ item Character::i_rem( int pos )
         auto iter = worn.begin();
         std::advance( iter, worn_position_to_index( pos ) );
         tmp = *iter;
+        on_item_takeoff( *iter );
         tmp.on_takeoff( *this );
         worn.erase( iter );
         return tmp;
@@ -2445,7 +2446,7 @@ item Character::i_rem( const item *it )
         return &i == it;
     }, 1 );
     if( tmp.empty() ) {
-        debugmsg( "did not found item %s to remove it!", it->tname() );
+        debugmsg( "Did not find item %s to remove it!", it->tname() );
         return item();
     }
     return tmp.front();
@@ -2475,9 +2476,9 @@ bool Character::i_add_or_drop( item &it, int qty )
     return retval;
 }
 
-std::list<item *> Character::get_dependent_worn_items( const item &it ) const
+std::list<const item *> Character::get_dependent_worn_items( const item &it ) const
 {
-    std::list<item *> dependent;
+    std::list<const item *> dependent;
     // Adds dependent worn items recursively
     const std::function<void( const item &it )> add_dependent = [&]( const item & it ) {
         for( const item &wit : worn ) {
@@ -2490,7 +2491,7 @@ std::list<item *> Character::get_dependent_worn_items( const item &it ) const
             } );
             if( iter == dependent.end() ) { // Not in the list yet
                 add_dependent( wit );
-                dependent.push_back( const_cast<item *>( & wit ) );
+                dependent.push_back( &wit );
             }
         }
     };
@@ -9850,27 +9851,27 @@ void Character::on_item_wear( const item &it )
 {
     for( const trait_id &mut : it.mutations_from_wearing( *this ) ) {
         mutation_effect( mut );
-        recalc_sight_limits();
-        calc_encumbrance();
 
         // If the stamina is higher than the max (Languorous), set it back to max
         if( get_stamina() > get_stamina_max() ) {
             set_stamina( get_stamina_max() );
         }
     }
+    recalc_sight_limits();
+    calc_encumbrance();
     morale->on_item_wear( it );
 }
 
 void Character::on_item_takeoff( const item &it )
 {
     for( const trait_id &mut : it.mutations_from_wearing( *this ) ) {
         mutation_loss_effect( mut );
-        recalc_sight_limits();
-        calc_encumbrance();
         if( get_stamina() > get_stamina_max() ) {
             set_stamina( get_stamina_max() );
         }
     }
+    recalc_sight_limits();
+    calc_encumbrance();
     morale->on_item_takeoff( it );
 }
 

diff --git a/src/character.h b/src/character.h
@@ -1336,7 +1336,7 @@ class Character : public Creature, public visitable<Character>
 
         void drop_invalid_inventory();
         /** Returns all items that must be taken off before taking off this item */
-        std::list<item *> get_dependent_worn_items( const item &it ) const;
+        std::list<const item *> get_dependent_worn_items( const item &it ) const;
         /** Drops an item to the specified location */
         void drop( item_location loc, const tripoint &where );
         virtual void drop( const drop_locations &what, const tripoint &target, bool stash = false );
@@ -1504,7 +1504,11 @@ class Character : public Creature, public visitable<Character>
         std::string name;
         bool male = true;
 
+        // TODO: Encapsulate!
         std::list<item> worn;
+        const std::list<item> &get_worn() const {
+            return worn;
+        }
         std::array<int, num_hp_parts> damage_bandaged, damage_disinfected;
         bool nv_cached = false;
         // Means player sit inside vehicle on the tile he is now

diff --git a/src/game_inventory.cpp b/src/game_inventory.cpp
@@ -1481,18 +1481,12 @@ drop_locations game_menus::inv::multidrop( player &p )
 {
     p.inv.restack( p );
 
-    const inventory_filter_preset preset( [ &p ]( const item_location & location ) {
-        const item &itm = *location;
-        if( p.is_wielding( itm ) ) {
-            return p.can_unwield( itm ).success();
-        } else if( p.is_wearing( itm ) ) {
-            return p.can_takeoff( itm ).success();
-        } else {
-            return true;
-        }
-    } );
-
-    inventory_drop_selector inv_s( p, preset );
+    // TODO: Keep the filters
+    // const inventory_filter_preset preset( [ &p ]( const item_location & location ) {
+    //     return p.can_unwield( *location ).success();
+    // } );
+    // inventory_drop_selector inv_s( p, preset );
+    inventory_drop_selector inv_s( p );
 
     inv_s.add_character_items( p );
     inv_s.set_title( _( "Multidrop" ) );