Allow reloading guns with mixed ammo (#54028)

src/item_pocket.cpp

@@ -241,20 +241,40 @@ void item_pocket::restack()
     if( contents.size() <= 1 ) {
         return;
     }
-    for( auto outer_iter = contents.begin(); outer_iter != contents.end(); ++outer_iter ) {
-        if( !outer_iter->count_by_charges() ) {
-            continue;
-        }
-        for( auto inner_iter = contents.begin(); inner_iter != contents.end(); ) {
-            if( outer_iter == inner_iter || !inner_iter->count_by_charges() ) {
-                ++inner_iter;
+    if( is_type( item_pocket::pocket_type::MAGAZINE ) ) {
+        // Restack magazine contents in a way that preserves order of items
+        for( auto iter = contents.begin(); iter != contents.end(); ) {
+            if( !iter->count_by_charges() ) {
                 continue;
             }
-            if( outer_iter->combine( *inner_iter ) ) {
-                inner_iter = contents.erase( inner_iter );
-                outer_iter = contents.begin();
+
+            auto next = std::next( iter, 1 );
+            if( next == contents.end() ) {
+                break;
+            }
+
+            if( iter->combine( *next ) ) {
+                iter = contents.erase( next );
             } else {
-                ++inner_iter;
+                ++iter;
+            }
+        }
+    } else {
+        for( auto outer_iter = contents.begin(); outer_iter != contents.end(); ++outer_iter ) {
+            if( !outer_iter->count_by_charges() ) {
+                continue;
+            }
+            for( auto inner_iter = contents.begin(); inner_iter != contents.end(); ) {
+                if( outer_iter == inner_iter || !inner_iter->count_by_charges() ) {
+                    ++inner_iter;
+                    continue;
+                }
+                if( outer_iter->combine( *inner_iter ) ) {
+                    inner_iter = contents.erase( inner_iter );
+                    outer_iter = contents.begin();
+                } else {
+                    ++inner_iter;
+                }
             }
         }
     }
@@ -1416,8 +1436,9 @@ bool item_pocket::can_reload_with( const item &ammo, const bool now ) const
 
         if( is_type( item_pocket::pocket_type::MAGAZINE ) ) {
             // Reloading is refused if
-            // Pocket contains ammo that can't combine (empty casings ignored)
-            // Pocket is full of ammo
+            // Pocket is full of ammo (casings are ignored)
+            // Ammos are of different ammo type
+            // If either of ammo is liquid while the other is not or they are different types
 
             if( full( false ) ) {
                 return false;
@@ -1427,12 +1448,16 @@ bool item_pocket::can_reload_with( const item &ammo, const bool now ) const
                 if( loaded->has_flag( flag_CASING ) ) {
                     continue;
                 }
-                // This is a *very* cut down version of item::stacks_with()
-                bool cant_combine = loaded->type != ammo.type || loaded->active != ammo.active ||
-                                    loaded->made_of( phase_id::LIQUID ) != ammo.made_of( phase_id::LIQUID );
-                if( cant_combine ) {
+                if( loaded->ammo_type() != ammo.ammo_type() ) {
                     return false;
                 }
+                if( loaded->made_of( phase_id::LIQUID ) || ammo.made_of( phase_id::LIQUID ) ) {
+                    bool cant_combine = !loaded->made_of( phase_id::LIQUID ) || !ammo.made_of( phase_id::LIQUID ) ||
+                                        loaded->type != ammo.type;
+                    if( cant_combine ) {
+                        return false;
+                    }
+                }
             }
 
             return true;
@@ -1765,10 +1790,11 @@ ret_val<item_pocket::contain_code> item_pocket::insert_item( const item &it )
 {
     const ret_val<item_pocket::contain_code> ret = !is_standard_type() ?
             ret_val<item_pocket::contain_code>::make_success() : can_contain( it );
+
     if( ret.success() ) {
-        contents.push_back( it );
+        contents.push_front( it );
+        restack();
     }
-    restack();
     return ret;
 }
 

src/item_pocket.h

@@ -344,7 +344,7 @@ class item_pocket
         bool same_contents( const item_pocket &rhs ) const;
         /** stacks like items inside the pocket */
         void restack();
-        /** same as above, except returns the stack where input item was placed */
+        /** same as restack(), except returns the stack where input item was placed */
         item *restack( /*const*/ item *it );
         bool has_item_stacks_with( const item &it ) const;
 

tests/reload_magazine_test.cpp

@@ -31,7 +31,7 @@ static const itype_id itype_stanag30( "stanag30" );
 static const itype_id itype_sw_619( "sw_619" );
 
 // NOLINTNEXTLINE(readability-function-size)
-TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location]" )
+TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [reload]" )
 {
     const itype_id gun_id = itype_m4_carbine;
     const ammotype gun_ammo = ammo_223;
@@ -179,9 +179,9 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location]" )
                 item &ammo = player_character.i_add( item( alt_ammo ) );
                 bool ok = mag.reload( player_character, item_location( player_character, &ammo ),
                                       mag.ammo_capacity( gun_ammo ) );
-                THEN( "further reloading should fail" ) {
-                    REQUIRE_FALSE( ok );
-                    REQUIRE( mag.ammo_remaining() == mag_cap - 2 );
+                THEN( "further reloading should be succesful" ) {
+                    REQUIRE( ok );
+                    REQUIRE( mag.ammo_remaining() == mag_cap );
                 }
             }
 
@@ -277,9 +277,9 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location]" )
                     item &ammo = player_character.i_add( item( alt_ammo, calendar::turn, 10 ) );
                     bool ok = gun.magazine_current()->reload( player_character, item_location( player_character,
                               &ammo ), 10 );
-                    THEN( "further reloading should fail" ) {
-                        REQUIRE_FALSE( ok );
-                        REQUIRE( gun.ammo_remaining() == mag_cap - 2 );
+                    THEN( "further reloading should be succesful" ) {
+                        REQUIRE( ok );
+                        REQUIRE( gun.ammo_remaining() == mag_cap );
                     }
                 }
 
@@ -337,7 +337,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location]" )
     }
 }
 
-TEST_CASE( "reload_revolver", "[visitable] [item] [item_location]" )
+TEST_CASE( "reload_revolver", "[visitable] [item] [item_location] [reload]" )
 {
     const itype_id gun_id = itype_sw_619;
     const ammotype gun_ammo = ammo_38;

tests/reloading_test.cpp

@@ -110,7 +110,7 @@ TEST_CASE( "reload_magazines", "[reload]" )
 
         SECTION( "with one ammo of different type" ) {
             item ammo( "223" );
-            test_reloading( mag, ammo, false );
+            test_reloading( mag, ammo );
         }
 
         SECTION( "with wrong ammo" ) {
@@ -178,6 +178,11 @@ TEST_CASE( "reload_gun_with_casings", "[reload],[gun]" )
 
         SECTION( "with one ammo of different type" ) {
             item ammo( "bp_40sw" );
+            test_reloading( gun, ammo );
+        }
+
+        SECTION( "with one ammo of different ammo type" ) {
+            item ammo( "10mm_fmj" );
             test_reloading( gun, ammo, false );
         }