Don't try to shoot with empty gun

src/avatar_action.cpp

@@ -755,7 +755,7 @@ void avatar_action::fire_wielded_weapon( avatar &you )
         return;
     } else if( !weapon->is_gun() ) {
         return;
-    } else if( weapon->ammo_data() &&
+    } else if( weapon->has_ammo_data() &&
                !weapon->ammo_types().count( weapon->loaded_ammo().ammo_type() ) ) {
         add_msg( m_info, _( "The %s can't be fired while loaded with incompatible ammunition %s" ),
                  weapon->tname(), weapon->ammo_current()->nname( 1 ) );

src/game_inventory.cpp

@@ -1688,7 +1688,7 @@ class weapon_inventory_preset: public inventory_selector_preset
 
                 const int total_damage = loc->gun_damage( true ).total_damage();
 
-                if( loc->ammo_data() && loc->ammo_remaining() ) {
+                if( loc->has_ammo() ) {
                     const int basic_damage = loc->gun_damage( false ).total_damage();
                     const damage_instance &damage = loc->ammo_data()->ammo->damage;
                     if( !damage.empty() ) {

src/item.cpp

@@ -2962,7 +2962,7 @@ void item::ammo_info( std::vector<iteminfo> &info, const iteminfo_query *parts,
                       bool /* debug */ ) const
 {
     // Skip this section for guns and items without ammo data
-    if( is_gun() || !ammo_data() ||
+    if( is_gun() || !has_ammo_data() ||
         !parts->test( iteminfo_parts::AMMO_REMAINING_OR_TYPES ) ) {
         return;
     }
@@ -3416,7 +3416,7 @@ void item::gun_info( const item *mod, std::vector<iteminfo> &info, const iteminf
         }
     }
 
-    if( mod->ammo_data() && parts->test( iteminfo_parts::AMMO_REMAINING ) ) {
+    if( mod->has_ammo_data() && parts->test( iteminfo_parts::AMMO_REMAINING ) ) {
         info.emplace_back( "AMMO", _( "Ammunition: " ), string_format( "<stat>%s</stat>",
                            mod->ammo_data()->nname( mod->ammo_remaining() ) ) );
     }
@@ -4838,7 +4838,7 @@ void item::tool_info( std::vector<iteminfo> &info, const iteminfo_query *parts,
                            _( "* This tool <info>runs on bionic power</info>." ) );
     } else if( has_flag( flag_BURNOUT ) && parts->test( iteminfo_parts::TOOL_BURNOUT ) ) {
         int percent_left = 0;
-        if( ammo_data() != nullptr ) {
+        if( has_ammo_data() ) {
             // Candle items that use ammo instead of charges
             // The capacity should never be zero but clang complains about it
             percent_left = 100 * ammo_remaining() / std::max( ammo_capacity( ammo_data()->ammo->type ), 1 );
@@ -10590,7 +10590,7 @@ int item::gun_dispersion( bool with_ammo, bool with_scaling ) const
     int dispPerDamage = get_option< int >( "DISPERSION_PER_GUN_DAMAGE" );
     dispersion_sum += damage_level() * dispPerDamage;
     dispersion_sum = std::max( dispersion_sum, 0 );
-    if( with_ammo && ammo_data() ) {
+    if( with_ammo && has_ammo() ) {
         dispersion_sum += ammo_data()->ammo->dispersion_considering_length( barrel_length() );
     }
     if( !with_scaling ) {
@@ -10643,7 +10643,7 @@ damage_instance item::gun_damage( bool with_ammo, bool shot ) const
         ret.add( mod->type->gunmod->damage.di_considering_length( bl ) );
     }
 
-    if( with_ammo && ammo_data() ) {
+    if( with_ammo && has_ammo() ) {
         if( shot ) {
             ret.add( ammo_data()->ammo->shot_damage.di_considering_length( bl ) );
         } else {
@@ -10728,7 +10728,7 @@ int item::gun_recoil( const Character &p, bool bipod, bool ideal_strength ) cons
     handling = std::pow( wt, 0.8 ) * std::pow( handling, 1.2 );
 
     int qty = type->gun->recoil;
-    if( ammo_data() ) {
+    if( has_ammo() ) {
         qty += ammo_data()->ammo->recoil;
     }
 
@@ -10763,7 +10763,7 @@ int item::gun_range( bool with_ammo ) const
         ret += mod->type->gunmod->range;
         range_multiplier *= mod->type->gunmod->range_multiplier;
     }
-    if( with_ammo && ammo_data() ) {
+    if( with_ammo && has_ammo() ) {
         ret += ammo_data()->ammo->range;
         range_multiplier *= ammo_data()->ammo->range_multiplier;
     }
@@ -11179,6 +11179,56 @@ int item::activation_consume( int qty, const tripoint &pos, Character *carrier )
     return ammo_consume( qty * ammo_required(), pos, carrier );
 }
 
+bool item::has_ammo() const
+{
+    const item *mag = magazine_current();
+    if( mag ) {
+        return mag->has_ammo();
+    }
+
+    if( is_ammo() ) {
+        return true;
+    }
+
+    if( is_magazine() ) {
+        return !contents.empty();
+    }
+
+    auto mods = is_gun() ? gunmods() : toolmods();
+    for( const item *e : mods ) {
+        if( !e->type->mod->ammo_modifier.empty() && e->has_ammo() ) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+bool item::has_ammo_data() const
+{
+    const item *mag = magazine_current();
+    if( mag ) {
+        return mag->has_ammo_data();
+    }
+
+    if( is_ammo() ) {
+        return true;
+    }
+
+    if( is_magazine() ) {
+        return !contents.empty();
+    }
+
+    auto mods = is_gun() ? gunmods() : toolmods();
+    for( const item *e : mods ) {
+        if( !e->type->mod->ammo_modifier.empty() && e->has_ammo_data() ) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 const itype *item::ammo_data() const
 {
     const item *mag = magazine_current();
@@ -11350,7 +11400,7 @@ std::set<ammo_effect_str_id> item::ammo_effects( bool with_ammo ) const
     }
 
     std::set<ammo_effect_str_id> res = type->gun->ammo_effects;
-    if( with_ammo && ammo_data() ) {
+    if( with_ammo && has_ammo() ) {
         res.insert( ammo_data()->ammo->ammo_effects.begin(), ammo_data()->ammo->ammo_effects.end() );
     }
 

src/item.h

@@ -2564,6 +2564,12 @@ class item : public visitable
          */
         int activation_consume( int qty, const tripoint &pos, Character *carrier );
 
+        // Returns whether the item has ammo in it, either directly or via a selected magazine, which
+        // contrasts with ammo_data(), which just returns the magazine data if a magazine is selected,
+        // regardless of whether that magazine is empty or not.
+        bool has_ammo() const;
+        // Cheaper way to just check if ammo_data exists if the data is to be just discarded afterwards.
+        bool has_ammo_data() const;
         /** Specific ammo data, returns nullptr if item is neither ammo nor loaded with any */
         const itype *ammo_data() const;
         /** Specific ammo type, returns "null" if item is neither ammo nor loaded with any */

src/npc_attack.cpp

@@ -6,6 +6,7 @@
 #include "dialogue.h"
 #include "flag.h"
 #include "item.h"
+#include "itype.h"
 #include "line.h"
 #include "magic.h"
 #include "magic_spell_effect_helpers.h"
@@ -465,8 +466,9 @@ void npc_attack_gun::use( npc &source, const tripoint_bub_ms &location ) const
 
 bool npc_attack_gun::can_use( const npc &source ) const
 {
-    // can't attack with something you can't wield
-    return source.is_wielding( *gunmode ) || source.can_wield( *gunmode ).success();
+    // can't attack with something you can't wield or which lacks ammo.
+    return ( source.is_wielding( *gunmode ) || source.can_wield( *gunmode ).success() )
+           && gun.has_ammo();
 }
 
 int npc_attack_gun::base_time_penalty( const npc &source ) const

src/ranged.cpp

@@ -807,8 +807,8 @@ bool Character::handle_gun_damage( item &it )
         it.inc_damage();
     }
     if( !it.has_flag( flag_PRIMITIVE_RANGED_WEAPON ) ) {
-        if( it.ammo_data() != nullptr && ( ( it.ammo_data()->ammo->recoil < it.min_cycle_recoil() ) ||
-                                           ( it.has_fault_flag( "BAD_CYCLING" ) && one_in( 16 ) ) ) &&
+        if( it.has_ammo() && ( ( it.ammo_data()->ammo->recoil < it.min_cycle_recoil() ) ||
+                               ( it.has_fault_flag( "BAD_CYCLING" ) && one_in( 16 ) ) ) &&
             it.faults_potential().count( fault_gun_chamber_spent ) ) {
             add_msg_player_or_npc( m_bad, _( "Your %s fails to cycle!" ),
                                    _( "<npcname>'s %s fails to cycle!" ),
@@ -2172,12 +2172,12 @@ static projectile make_gun_projectile( const item &gun )
 
     auto &fx = proj.proj_effects;
 
-    if( ( gun.ammo_data() && gun.ammo_data()->phase == phase_id::LIQUID ) ||
+    if( ( gun.has_ammo_data() && gun.ammo_data()->phase == phase_id::LIQUID ) ||
         fx.count( ammo_effect_SHOT ) || fx.count( ammo_effect_BOUNCE ) ) {
         fx.insert( ammo_effect_WIDE );
     }
 
-    if( gun.ammo_data() ) {
+    if( gun.has_ammo() ) {
         const auto &ammo = gun.ammo_data()->ammo;
 
         // Some projectiles have a chance of being recoverable
@@ -2312,7 +2312,7 @@ item::sound_data item::gun_noise( const bool burst ) const
     }
 
     int noise = type->gun->loudness;
-    if( ammo_data() ) {
+    if( has_ammo() ) {
         noise += ammo_data()->ammo->loudness;
     }
     for( const item *mod : gunmods() ) {

tests/reload_magazine_test.cpp

@@ -72,7 +72,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re
     CHECK( mag->ammo_types().count( gun_ammo ) );
     CHECK( mag->ammo_capacity( gun_ammo ) == mag_cap );
     CHECK( mag->ammo_current().is_null() );
-    CHECK( mag->ammo_data() == nullptr );
+    CHECK( !mag->has_ammo_data() );
 
     GIVEN( "An empty magazine" ) {
         CHECK( mag->ammo_remaining() == 0 );
@@ -96,7 +96,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re
 
                 AND_THEN( "the current ammo is updated" ) {
                     REQUIRE( mag->ammo_current() == ammo_id );
-                    REQUIRE( mag->ammo_data() );
+                    REQUIRE( mag->has_ammo_data() );
                 }
                 AND_THEN( "the magazine is filled to capacity" ) {
                     REQUIRE( mag->remaining_ammo_capacity() == 0 );
@@ -126,7 +126,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re
 
                 AND_THEN( "the current ammo is updated" ) {
                     REQUIRE( mag->ammo_current() == ammo_id );
-                    REQUIRE( mag->ammo_data() );
+                    REQUIRE( mag->has_ammo_data() );
                 }
                 AND_THEN( "the magazine is filled with the correct quantity" ) {
                     REQUIRE( mag->ammo_remaining() == mag_cap - 2 );
@@ -206,7 +206,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re
         CHECK( gun->ammo_capacity( gun_ammo ) == 0 );
         CHECK( gun->ammo_remaining() == 0 );
         CHECK( gun->ammo_current().is_null() );
-        CHECK( gun->ammo_data() == nullptr );
+        CHECK( !gun->has_ammo_data() );
 
         WHEN( "the gun is reloaded with an incompatible magazine" ) {
             item_location mag = player_character.i_add( item( bad_mag ) );
@@ -237,7 +237,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re
                 AND_THEN( "the gun contains no ammo" ) {
                     REQUIRE( gun->ammo_current().is_null() );
                     REQUIRE( gun->ammo_remaining() == 0 );
-                    REQUIRE( gun->ammo_data() == nullptr );
+                    REQUIRE( !gun->has_ammo_data() );
                 }
             }
         }
@@ -264,7 +264,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re
                 AND_THEN( "the gun contains the correct amount and type of ammo" ) {
                     REQUIRE( gun->ammo_remaining() == mag_cap - 2 );
                     REQUIRE( gun->ammo_current() == ammo_id );
-                    REQUIRE( gun->ammo_data() );
+                    REQUIRE( gun->has_ammo_data() );
                 }
 
                 AND_WHEN( "the guns magazine is further reloaded with compatible but different ammo" ) {
@@ -359,7 +359,7 @@ TEST_CASE( "reload_revolver", "[visitable] [item] [item_location] [reload]" )
         CHECK( gun->ammo_capacity( gun_ammo ) == mag_cap );
         CHECK( gun->ammo_remaining() == 0 );
         CHECK( gun->ammo_current().is_null() );
-        CHECK( gun->ammo_data() == nullptr );
+        CHECK( !gun->has_ammo_data() );
 
         WHEN( "the gun is reloaded with incompatible ammo" ) {
             item_location ammo = player_character.i_add( item( bad_ammo ) );
@@ -380,7 +380,7 @@ TEST_CASE( "reload_revolver", "[visitable] [item] [item_location] [reload]" )
 
                 AND_THEN( "the current ammo is updated" ) {
                     REQUIRE( gun->ammo_current() == ammo_id );
-                    REQUIRE( gun->ammo_data() );
+                    REQUIRE( gun->has_ammo_data() );
                 }
                 AND_THEN( "the gun is filled to capacity" ) {
                     REQUIRE( gun->remaining_ammo_capacity() == 0 );
@@ -410,7 +410,7 @@ TEST_CASE( "reload_revolver", "[visitable] [item] [item_location] [reload]" )
 
                 AND_THEN( "the current ammo is updated" ) {
                     REQUIRE( gun->ammo_current() == ammo_id );
-                    REQUIRE( gun->ammo_data() );
+                    REQUIRE( gun->has_ammo_data() );
                 }
                 AND_THEN( "the gun is filled with the correct quantity" ) {
                     REQUIRE( gun->ammo_remaining() == mag_cap - 2 );