fix: calculate modes in gunmods too (#3614)

* refactor: extract `defmode_name`

* fix: account for empty modes in `confident_shoot_range`

* fix: do not attempt to calculate modes for gunmods

src/item_factory.cpp

@@ -166,6 +166,23 @@ static bool assign_coverage_from_json( const JsonObject &jo, const std::string &
     }
 }
 
+namespace
+{
+
+// TODO: add explicit action field to gun definitions
+auto defmode_name( itype &obj )
+{
+    if( obj.gun->clip == 1 ) {
+        return translate_marker( "manual" ); // break-type actions
+    } else if( obj.gun->skill_used == skill_id( "pistol" ) && obj.has_flag( flag_RELOAD_ONE ) ) {
+        return translate_marker( "revolver" );
+    } else {
+        return translate_marker( "semi" );
+    }
+};
+
+} //namespace
+
 void Item_factory::finalize_pre( itype &obj )
 {
     // TODO: separate repairing from reinforcing/enhancement
@@ -397,46 +414,41 @@ void Item_factory::finalize_pre( itype &obj )
         // replace those ammo types with that of the migrated ammo
         for( auto ammo_type_it = obj.gun->ammo.begin(); ammo_type_it != obj.gun->ammo.end(); ) {
             auto maybe_migrated = migrated_ammo.find( ammo_type_it->obj().default_ammotype() );
-            if( maybe_migrated != migrated_ammo.end() ) {
-                const ammotype old_ammo = *ammo_type_it;
-                // Remove the old ammotype add the migrated version
-                ammo_type_it = obj.gun->ammo.erase( ammo_type_it );
-                const ammotype &new_ammo = maybe_migrated->second;
-                obj.gun->ammo.insert( obj.gun->ammo.begin(), new_ammo );
-                // Migrate the compatible magazines
-                auto old_mag_it = obj.magazines.find( old_ammo );
-                if( old_mag_it != obj.magazines.end() ) {
-                    for( const itype_id &old_mag : old_mag_it->second ) {
-                        obj.magazines[new_ammo].insert( old_mag );
-                    }
-                    obj.magazines.erase( old_ammo );
-                }
-                // And the default magazines for each magazine type
-                auto old_default_mag_it = obj.magazine_default.find( old_ammo );
-                if( old_default_mag_it != obj.magazine_default.end() ) {
-                    const itype_id &old_default_mag = old_default_mag_it->second;
-                    obj.magazine_default[new_ammo] = old_default_mag;
-                    obj.magazine_default.erase( old_ammo );
-                }
-            } else {
+            if( maybe_migrated == migrated_ammo.end() ) {
                 ++ammo_type_it;
+                continue;
             }
-        }
 
-        // TODO: add explicit action field to gun definitions
-        const auto defmode_name = [&]() {
-            if( obj.gun->clip == 1 ) {
-                return translate_marker( "manual" ); // break-type actions
-            } else if( obj.gun->skill_used == skill_id( "pistol" ) && obj.has_flag( flag_RELOAD_ONE ) ) {
-                return translate_marker( "revolver" );
-            } else {
-                return translate_marker( "semi" );
+            const ammotype old_ammo = *ammo_type_it;
+
+            // Remove the old ammotype add the migrated version
+            ammo_type_it = obj.gun->ammo.erase( ammo_type_it );
+            const ammotype &new_ammo = maybe_migrated->second;
+            obj.gun->ammo.insert( obj.gun->ammo.begin(), new_ammo );
+
+            // Migrate the compatible magazines
+            auto old_mag_it = obj.magazines.find( old_ammo );
+            if( old_mag_it != obj.magazines.end() ) {
+                for( const itype_id &old_mag : old_mag_it->second ) {
+                    obj.magazines[new_ammo].insert( old_mag );
+                }
+                obj.magazines.erase( old_ammo );
+            }
+
+            // And the default magazines for each magazine type
+            auto old_default_mag_it = obj.magazine_default.find( old_ammo );
+            if( old_default_mag_it != obj.magazine_default.end() ) {
+                const itype_id &old_default_mag = old_default_mag_it->second;
+                obj.magazine_default[new_ammo] = old_default_mag;
+                obj.magazine_default.erase( old_ammo );
             }
-        };
+        }
+
+
 
         // if the gun doesn't have a DEFAULT mode then add one now
         obj.gun->modes.emplace( gun_mode_id( "DEFAULT" ),
-                                gun_modifier_data( defmode_name(), 1, std::set<std::string>() ) );
+                                gun_modifier_data( defmode_name( obj ), 1, std::set<std::string>() ) );
 
         // If a "gun" has a reach attack, give it an additional melee mode.
         if( obj.has_flag( flag_REACH_ATTACK ) ) {

src/npcmove.cpp

@@ -2022,18 +2022,20 @@ double npc::confidence_mult() const
 
 int npc::confident_shoot_range( const item &it, int recoil ) const
 {
-    int res = 0;
     if( !it.is_gun() ) {
-        return res;
+        return 0;
     }
     const auto gun_mode_cmp = []( const std::pair<gun_mode_id, gun_mode> &lhs,
     const std::pair<gun_mode_id, gun_mode> &rhs ) {
         return lhs.second.qty < rhs.second.qty;
     };
     std::map<gun_mode_id, gun_mode> modes = it.gun_all_modes();
+    if( modes.empty() ) {
+        debugmsg( "%s has no gun modes", it.tname() );
+        return 0;
+    }
     auto best = std::min_element( modes.begin(), modes.end(), gun_mode_cmp );
-    res = confident_gun_mode_range( ( *best ).second, recoil );
-    return res;
+    return confident_gun_mode_range( ( *best ).second, recoil );
 }
 
 int npc::confident_gun_mode_range( const gun_mode &gun, int at_recoil ) const
@@ -2074,9 +2076,9 @@ int npc::confident_throw_range( const item &thrown, Creature *target ) const
     return static_cast<int>( confident_range );
 }
 
-double item::ideal_ranged_dps( const Character &who, std::optional<gun_mode> &mode ) const
+auto item::ideal_ranged_dps( const Character &who, std::optional<gun_mode> &mode ) const -> double
 {
-    if( !is_gun() || !mode ) {
+    if( !is_gun() || is_gunmod() || !mode ) {
         return 0;
     }
     damage_instance gun_damage = this->gun_damage();

src/ranged.cpp

@@ -1955,13 +1955,15 @@ dispersion_sources ranged::get_weapon_dispersion( const Character &who, const it
     return dispersion;
 }
 
-std::pair<gun_mode_id, std::optional<gun_mode>> npc_ai::best_mode_for_range( const Character &who,
-        const item &firing,
-        int dist )
+auto npc_ai::best_mode_for_range(
+    const Character &who, const item &firing, int dist
+) -> std::pair<gun_mode_id, std::optional<gun_mode>>
 {
-    int shots = who.is_wielding( firing ) ? character_funcs::ammo_count_for( who,
-                firing ) : item_funcs::shots_remaining( who, firing );
-    if( !firing.is_gun() || shots == 0 ) {
+    const int shots = who.is_wielding( firing )
+                      ? character_funcs::ammo_count_for( who, firing )
+                      : item_funcs::shots_remaining( who, firing );
+
+    if( !firing.is_gun() || firing.is_gunmod() || shots == 0 ) {
         return std::make_pair( gun_mode_id(), std::nullopt );
     }
     int min_recoil = MAX_RECOIL;