Merge pull request #54987 from dseguin/fix_veh_fill

Prevent crash when filling empty tank
CleverRaven · Feb 3, 2022 · d3bd55c · d3bd55c
2 parents fa25c79 + 0eef328
commit d3bd55c
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 7 deletions.
diff --git a/src/vehicle.cpp b/src/vehicle.cpp
@@ -3325,9 +3325,9 @@ int vehicle::fuel_capacity( const itype_id &ftype ) const
 {
     return std::accumulate( parts.begin(), parts.end(), 0, [&ftype]( const int &lhs,
     const vehicle_part & rhs ) {
-        return lhs + ( ( rhs.is_available() &&
-                         rhs.ammo_current() == ftype ) ? rhs.ammo_capacity( item::find_type(
-                                     ftype )->ammo->type ) : 0 );
+        cata::value_ptr<islot_ammo> a_val = item::find_type( ftype )->ammo;
+        return lhs + ( ( rhs.is_available() && rhs.ammo_current() == ftype ) ?
+                       rhs.ammo_capacity( !!a_val ? a_val->type : ammotype::NULL_ID() ) : 0 );
     } );
 }
 

diff --git a/src/vehicle_part.cpp b/src/vehicle_part.cpp
@@ -280,7 +280,7 @@ int vehicle_part::ammo_set( const itype_id &ammo, int qty )
     // We often check if ammo is set to see if tank is empty, if qty == 0 don't set ammo
     if( is_tank() && qty != 0 ) {
         const itype *ammo_itype = item::find_type( ammo );
-        if( ammo_itype && ammo_itype->phase >= phase_id::LIQUID ) {
+        if( ammo_itype && ammo_itype->ammo && ammo_itype->phase >= phase_id::LIQUID ) {
             base.clear_items();
             const int limit = ammo_capacity( ammo_itype->ammo->type );
             // assuming "ammo" isn't really going into a magazine as this is a vehicle part
@@ -305,7 +305,7 @@ int vehicle_part::ammo_set( const itype_id &ammo, int qty )
 
     if( is_fuel_store() ) {
         const itype *ammo_itype = item::find_type( ammo );
-        if( ammo_itype ) {
+        if( ammo_itype && ammo_itype->ammo ) {
             base.ammo_set( ammo, qty >= 0 ? qty : ammo_capacity( ammo_itype->ammo->type ) );
             return base.ammo_remaining();
         }
@@ -412,7 +412,13 @@ bool vehicle_part::can_reload( const item &obj ) const
         return true; // empty tank
     }
 
-    return ammo_remaining() < ammo_capacity( ammo_current().obj().ammo->type );
+    // Despite checking for an empty tank, item::find_type can still turn up with an empty ammo pointer
+    if( cata::value_ptr<islot_ammo> a_val = item::find_type( ammo_current() )->ammo ) {
+        return ammo_remaining() < ammo_capacity( a_val->type );
+    }
+
+    // Nothing in tank
+    return ammo_capacity( obj.ammo_type() ) > 0;
 }
 
 void vehicle_part::process_contents( const tripoint &pos, const bool e_heater )
@@ -441,7 +447,13 @@ bool vehicle_part::fill_with( item &liquid, int qty )
         return false;
     }
 
-    int charges_max = ammo_capacity( item::find_type( ammo_current() )->ammo->type ) - ammo_remaining();
+    int charges_max = 0;
+    if( cata::value_ptr<islot_ammo> a_val = item::find_type( ammo_current() )->ammo ) {
+        charges_max = ammo_capacity( a_val->type ) - ammo_remaining();
+    } else {
+        // Nothing in tank
+        charges_max = ammo_capacity( liquid.ammo_type() );
+    }
     qty = qty < liquid.charges ? qty : liquid.charges;
 
     if( charges_max < liquid.charges ) {

diff --git a/tests/vehicle_part_test.cpp b/tests/vehicle_part_test.cpp
@@ -241,3 +241,34 @@ TEST_CASE( "verify_vehicle_tank_refill", "[vehicle]" )
     check_part_ammo_capacity( vpart_tank_test, itype_metal_tank_test, ammo_water, 240 );
     check_part_ammo_capacity( vpart_tank_test, itype_metal_tank_test, ammo_flammable, 60000 );
 }
+
+TEST_CASE( "check_capacity_fueltype_handling", "[vehicle]" )
+{
+    GIVEN( "tank is empty" ) {
+        vehicle_part vp( vpart_tank_test, "", point_zero, item( itype_metal_tank_test ) );
+        REQUIRE( vp.ammo_remaining() == 0 );
+        THEN( "ammo_current ammotype is always null" ) {
+            CHECK( vp.ammo_current().is_null() );
+            CHECK( !item::find_type( vp.ammo_current() )->ammo );
+            // Segmentation fault:
+            //vp.ammo_capacity( item::find_type( vp.ammo_current() )->ammo->type );
+        }
+        THEN( "using explicit ammotype for ammo_capacity returns expected value" ) {
+            CHECK( vp.ammo_capacity( ammo_flammable ) == 60000 );
+        }
+    }
+
+    GIVEN( "tank is not empty" ) {
+        vehicle_part vp( vpart_tank_test, "", point_zero, item( itype_metal_tank_test ) );
+        item tank( itype_metal_tank_test );
+        REQUIRE( tank.fill_with( item( itype_water_clean ), 100 ) == 100 );
+        vp.set_base( tank );
+        REQUIRE( vp.ammo_remaining() == 100 );
+
+        THEN( "ammo_current is not null" ) {
+            CHECK( vp.ammo_current() == itype_water_clean );
+            CHECK( !!item::find_type( vp.ammo_current() )->ammo );
+            CHECK( vp.ammo_capacity( item::find_type( vp.ammo_current() )->ammo->type ) == 240 );
+        }
+    }
+}