Merge pull request #36724 from ZhilkinSerg/sa-2020-01-05

Code optimizations reported by static code analysis (2020-01-05)

src/auto_pickup.cpp

@@ -576,7 +576,7 @@ void rule_list::create_rule( cache &map_items, const std::string &to_match )
 
 void player_settings::create_rule( const item *it )
 {
-    // @todo change it to be a reference
+    // @TODO: change it to be a reference
     global_rules.create_rule( map_items, *it );
     character_rules.create_rule( map_items, *it );
 }

src/calendar.cpp

@@ -118,7 +118,7 @@ time_point night_time( const time_point &p )
 
 time_point daylight_time( const time_point &p )
 {
-    // @TODO Actual dailight should start 18 degrees before sunrise
+    // @TODO: Actual dailight should start 18 degrees before sunrise
     return sunrise( p ) + 15_minutes;
 }
 

src/cata_io.h

@@ -279,7 +279,8 @@ class JsonObjectInputArchive : public JsonObject
         bool io( const std::string &name, T *&pointer,
                  const std::function<void( const std::string & )> &load,
                  const std::function<std::string( const T & )> &save, bool required = false ) {
-            ( void ) save; // Only used by the matching function in the output archive classes.
+            // Only used by the matching function in the output archive classes.
+            ( void ) save;
             std::string ident;
             if( !io( name, ident ) ) {
                 if( required ) {

src/cata_tiles.cpp

@@ -3124,7 +3124,7 @@ void cata_tiles::draw_explosion_frame()
         draw_from_id_string( exp_name, exp_pos + point( i, i ),
                              subtile, rotation++, LL_LIT, nv_goggles_activated );
         draw_from_id_string( exp_name, exp_pos + point( i, -i ),
-                             subtile, rotation++, LL_LIT, nv_goggles_activated );
+                             subtile, rotation, LL_LIT, nv_goggles_activated );
 
         subtile = edge;
         for( int j = 1 - i; j < 0 + i; j++ ) {

src/character.cpp

@@ -3829,7 +3829,7 @@ void Character::update_bodytemp()
             // Morale bonus for comfiness - only if actually comfy (not too warm/cold)
             // Spread the morale bonus in time.
             if( comfortable_warmth > 0 &&
-                // @todo make this simpler and use time_duration/time_point
+                // @TODO: make this simpler and use time_duration/time_point
                 to_turn<int>( calendar::turn ) % to_turns<int>( 1_minutes ) == to_turns<int>
                 ( 1_minutes * bp ) / to_turns<int>( 1_minutes * num_bp ) &&
                 get_effect_int( effect_cold, num_bp ) == 0 &&
@@ -4123,8 +4123,6 @@ hp_part Character::body_window( const std::string &menu_header,
         max_bp_name_len = std::max( max_bp_name_len, utf8_width( e.name ) );
     }
 
-
-
     uilist bmenu;
     bmenu.desc_enabled = true;
     bmenu.text = menu_header;
@@ -4203,7 +4201,7 @@ hp_part Character::body_window( const std::string &menu_header,
             std::pair<std::string, nc_color> h_bar = get_hp_bar( current_hp, maximal_hp, false );
             hp_str = colorize( h_bar.first, h_bar.second ) +
                      colorize( std::string( 5 - utf8_width( h_bar.first ), '.' ), c_white );
-        };
+        }
         msg += colorize( aligned_name, all_state_col ) + " " + hp_str;
 
         // BLEEDING block

src/clzones.cpp

@@ -561,7 +561,7 @@ void zone_manager::cache_vzones()
         const std::string &type_hash = elem->get_type_hash();
         auto &cache = area_cache[type_hash];
 
-        // @todo looks very similar to the above cache_data - maybe merge it?
+        // @TODO: looks very similar to the above cache_data - maybe merge it?
 
         // Draw marked area
         for( const tripoint &p : tripoint_range( elem->get_start_point(), elem->get_end_point() ) ) {

src/consumption.cpp

@@ -10,7 +10,7 @@
 #include "addiction.h"
 #include "avatar.h"
 #include "bionics.h"
-#include "calendar.h" // ticks_between
+#include "calendar.h"
 #include "cata_utility.h"
 #include "debug.h"
 #include "game.h"
@@ -347,8 +347,9 @@ std::pair<int, int> Character::fun_for( const item &comest ) const
 
     // As float to avoid rounding too many times
     float fun = comest.get_comestible_fun();
+    // Food doesn't taste as good when you're sick
     if( ( has_effect( effect_common_cold ) || has_effect( effect_flu ) ) && fun > 0 ) {
-        fun /= 3; // food doesn't taste as good when you're sick
+        fun /= 3;
     }
     // Rotten food should be pretty disgusting
     const float relative_rot = comest.get_relative_rot();
@@ -373,9 +374,10 @@ std::pair<int, int> Character::fun_for( const item &comest ) const
                 event.component_hash == comest.make_component_hash() ) {
                 fun -= comest.get_comestible()->monotony_penalty;
                 // This effect can't drop fun below 0, unless the food has the right flag.
+                // 0 is the lowest we'll go, no need to keep looping.
                 if( fun <= 0 && !comest.has_flag( "NEGATIVE_MONOTONY_OK" ) ) {
                     fun = 0;
-                    break; // 0 is the lowest we'll go, no need to keep looping.
+                    break;
                 }
             }
         }
@@ -395,8 +397,9 @@ std::pair<int, int> Character::fun_for( const item &comest ) const
         if( fun > 0 ) {
             fun *= 0.5;
         } else {
-            fun *= 1.25; // melted freezable food tastes 25% worse than frozen freezable food
-            // frozen freezable food... say that 5 times fast
+            // Melted freezable food tastes 25% worse than frozen freezable food.
+            // Frozen freezable food... say that 5 times fast
+            fun *= 1.25;
         }
     }
 
@@ -451,7 +454,7 @@ int Character::vitamin_mod( const vitamin_id &vit, int qty, bool capped )
     const auto &v = it->first.obj();
 
     if( qty > 0 ) {
-        // accumulations can never occur from food sources
+        // Accumulations can never occur from food sources
         it->second = std::min( it->second + qty, capped ? 0 : v.max() );
         update_vitamins( vit );
 
@@ -788,7 +791,7 @@ bool player::eat( item &food, bool force )
         add_msg_if_player( m_good, _( "Mmm, this %s tastes delicious…" ), food.tname() );
     }
     if( !consume_effects( food ) ) {
-        //Already consumed by using `food.type->invoke`?
+        // Already consumed by using `food.type->invoke`?
         if( charges_used > 0 ) {
             food.mod_charges( -charges_used );
         }
@@ -805,17 +808,19 @@ bool player::eat( item &food, bool force )
             has_trait( trait_id( "FANGS_SPIDER" ) ) ) {
             mealtime /= 2;
         } else if( has_trait( trait_id( "SHARKTEETH" ) ) ) {
-            //SHARKBAIT! HOO HA HA!
+            // SHARKBAIT! HOO HA HA!
             mealtime /= 3;
         } else if( has_trait( trait_id( "GOURMAND" ) ) ) {
             // Don't stack those two - that would be 25 moves per item
             mealtime -= 100;
         }
 
         if( has_trait( trait_id( "BEAK_HUM" ) ) && !drinkable ) {
-            mealtime += 200; // Much better than PROBOSCIS but still optimized for fluids
+            // Much better than PROBOSCIS but still optimized for fluids
+            mealtime += 200;
         } else if( has_trait( trait_id( "SABER_TEETH" ) ) ) {
-            mealtime += 250; // They get In The Way
+            // They get In The Way
+            mealtime += 250;
         }
 
         if( amorphous ) {
@@ -1003,7 +1008,7 @@ bool player::eat( item &food, bool force )
     if( food.has_flag( "URSINE_HONEY" ) && ( !crossed_threshold() ||
             has_trait( trait_id( "THRESH_URSINE" ) ) ) &&
         mutation_category_level["URSINE"] > 40 ) {
-        //Need at least 5 bear mutations for effect to show, to filter out mutations in common with other categories
+        // Need at least 5 bear mutations for effect to show, to filter out mutations in common with other categories
         int honey_fun = has_trait( trait_id( "THRESH_URSINE" ) ) ?
                         std::min( mutation_category_level["URSINE"] / 8, 20 ) :
                         mutation_category_level["URSINE"] / 12;
@@ -1015,7 +1020,7 @@ bool player::eat( item &food, bool force )
         add_morale( MORALE_HONEY, honey_fun, 100 );
     }
 
-    // chance to become parasitised
+    // Chance to become parasitised
     if( !will_vomit && !( has_bionic( bio_digestion ) || has_trait( trait_id( "PARAIMMUNE" ) ) ) ) {
         if( food.get_comestible()->parasites > 0 && !food.has_flag( "NO_PARASITES" ) &&
             one_in( food.get_comestible()->parasites ) ) {
@@ -1039,7 +1044,7 @@ bool player::eat( item &food, bool force )
         }
     }
 
-    // chance to get food poisoning from bacterial contamination
+    // Chance to get food poisoning from bacterial contamination
     if( !will_vomit && !has_bionic( bio_digestion ) ) {
         const int contamination = food.get_comestible()->contamination;
         if( rng( 1, 100 ) <= contamination ) {
@@ -1063,7 +1068,8 @@ bool player::eat( item &food, bool force )
 void player::modify_health( const islot_comestible &comest )
 {
     const int effective_health = comest.healthy;
-    const int health_cap = 200; // Effectively no cap on health modifiers from food and meds
+    // Effectively no cap on health modifiers from food and meds
+    const int health_cap = 200;
     mod_healthy_mod( effective_health, effective_health >= 0 ? health_cap : -health_cap );
 }
 
@@ -1137,7 +1143,7 @@ bool player::consume_effects( item &food )
     }
 
     if( has_trait( trait_id( "THRESH_PLANT" ) ) && food.type->can_use( "PLANTBLECH" ) ) {
-        // used to cap nutrition and thirst, but no longer
+        // Was used to cap nutrition and thirst, but no longer does this
         return false;
     }
     if( ( has_trait( trait_id( "HERBIVORE" ) ) || has_trait( trait_id( "RUMINANT" ) ) ) &&
@@ -1160,9 +1166,11 @@ bool player::consume_effects( item &food )
         add_msg( m_debug, "%d health from %0.2f%% rotten food", h_loss, rottedness );
     }
 
-    const auto nutr = nutrition_for( food ); // used in hibernation messages.
+    // Used in hibernation messages.
+    const auto nutr = nutrition_for( food );
     const bool skip_health = has_trait( trait_id( "PROJUNK2" ) ) && comest.healthy < 0;
-    if( !skip_health ) { // we can handle junk just fine
+    // We can handle junk just fine
+    if( !skip_health ) {
         modify_health( comest );
     }
     modify_stimulation( comest );
@@ -1172,18 +1180,18 @@ bool player::consume_effects( item &food )
     const bool hibernate = has_active_mutation( trait_id( "HIBERNATE" ) );
     if( hibernate ) {
         if( ( nutr > 0 && get_hunger() < -60 ) || ( comest.quench > 0 && get_thirst() < -60 ) ) {
-            //Tell the player what's going on
+            // Tell the player what's going on
             add_msg_if_player( _( "You gorge yourself, preparing to hibernate." ) );
             if( one_in( 2 ) ) {
-                //50% chance of the food tiring you
+                // 50% chance of the food tiring you
                 mod_fatigue( nutr );
             }
         }
         if( ( nutr > 0 && get_hunger() < -200 ) || ( comest.quench > 0 && get_thirst() < -200 ) ) {
-            //Hibernation should cut burn to 60/day
+            // Hibernation should cut burn to 60/day
             add_msg_if_player( _( "You feel stocked for a day or two.  Got your bed all ready and secured?" ) );
             if( one_in( 2 ) ) {
-                //And another 50%, intended cumulative
+                // And another 50%, intended cumulative
                 mod_fatigue( nutr );
             }
         }
@@ -1192,7 +1200,7 @@ bool player::consume_effects( item &food )
             add_msg_if_player(
                 _( "Mmm.  You can still fit some more in… but maybe you should get comfortable and sleep." ) );
             if( !one_in( 3 ) ) {
-                //Third check, this one at 66%
+                // Third check, this one at 66%
                 mod_fatigue( nutr );
             }
         }
@@ -1220,8 +1228,8 @@ bool player::consume_effects( item &food )
         }
         mod_hunger( 40 );
         mod_thirst( 40 );
-        //~slimespawns have *small voices* which may be the Nice equivalent
-        //~of the Rat King's ALL CAPS invective.  Probably shared-brain telepathy.
+        //~ slimespawns have *small voices* which may be the Nice equivalent
+        //~ of the Rat King's ALL CAPS invective.  Probably shared-brain telepathy.
         add_msg_if_player( m_good, _( "hey, you look like me!  let's work together!" ) );
     }
 
@@ -1252,7 +1260,7 @@ bool player::consume_effects( item &food )
         contained_food.base_volume() - std::max( water, 0_ml ),
         compute_effective_nutrients( contained_food )
     };
-    // maybe move tapeworm to digestion
+    // Maybe move tapeworm to digestion
     if( has_effect( efftype_id( "tapeworm" ) ) ) {
         ingested.nutr /= 2;
     }
@@ -1335,7 +1343,8 @@ bool player::can_feed_furnace_with( const item &it ) const
         return false;
     }
 
-    if( it.charges_per_volume( furnace_max_volume ) < 1 ) { // not even one charge fits
+    // Not even one charge fits
+    if( it.charges_per_volume( furnace_max_volume ) < 1 ) {
         return false;
     }
 
@@ -1418,7 +1427,8 @@ bool player::fuel_bionic_with( item &it )
     const std::string new_charge = std::to_string( loadable + loaded );
 
     it.charges -= loadable;
-    set_value( it.typeId(), new_charge );// type and amount of fuel
+    // Type and amount of fuel
+    set_value( it.typeId(), new_charge );
     update_fuel_storage( it.typeId() );
     add_msg_player_or_npc( m_info,
                            //~ %1$i: charge number, %2$s: item name, %3$s: bionics name
@@ -1469,7 +1479,8 @@ int player::get_acquirable_energy( const item &it, rechargeable_cbm cbm ) const
                 const double n_stacks = static_cast<double>( it.charges_per_volume( furnace_max_volume ) ) /
                                         it.type->stack_size;
                 consumed_vol = it.type->volume * n_stacks;
-                consumed_mass = it.type->weight * 10 * n_stacks; // it.type->weight is in 10g units?
+                // it.type->weight is in 10g units?
+                consumed_mass = it.type->weight * 10 * n_stacks;
             }
             int amount = ( consumed_vol / 250_ml + consumed_mass / 1_gram ) / 9;
 
@@ -1513,7 +1524,7 @@ bool player::can_consume( const item &it ) const
     if( can_consume_as_is( it ) ) {
         return true;
     }
-    // checking NO_RELOAD to prevent consumption of `battery` when contained in `battery_car` (#20012)
+    // Checking NO_RELOAD to prevent consumption of `battery` when contained in `battery_car` (#20012)
     return !it.is_container_empty() && !it.has_flag( "NO_RELOAD" ) &&
            can_consume_as_is( it.contents.front() );
 }
@@ -1527,6 +1538,7 @@ item &player::get_consumable_from( item &it ) const
     }
 
     static item null_comestible;
-    null_comestible = item();   // Since it's not const.
+    // Since it's not const.
+    null_comestible = item();
     return null_comestible;
 }

src/flat_set.h

@@ -147,11 +147,11 @@ class flat_set : private Compare, Data
         }
 
         iterator insert( iterator, const value_type &value ) {
-            /// @todo Use insertion hint
+            /// @TODO: Use insertion hint
             return insert( value ).first;
         }
         iterator insert( iterator, value_type &&value ) {
-            /// @todo Use insertion hint
+            /// @TODO: Use insertion hint
             return insert( std::move( value ) ).first;
         }
         std::pair<iterator, bool> insert( const value_type &value ) {
@@ -171,7 +171,7 @@ class flat_set : private Compare, Data
 
         template<typename InputIt>
         void insert( InputIt first, InputIt last ) {
-            /// @todo could be faster when inserting only a few elements
+            /// @TODO: could be faster when inserting only a few elements
             Data::insert( end(), first, last );
             sort_data();
         }

src/game.cpp

@@ -210,14 +210,12 @@ static const efftype_id effect_winded( "winded" );
 
 static const bionic_id bio_remote( "bio_remote" );
 
-static const trait_id trait_GRAZER( "GRAZER" );
 static const trait_id trait_ILLITERATE( "ILLITERATE" );
 static const trait_id trait_INFIMMUNE( "INFIMMUNE" );
 static const trait_id trait_INFRESIST( "INFRESIST" );
 static const trait_id trait_LEG_TENT_BRACE( "LEG_TENT_BRACE" );
 static const trait_id trait_M_IMMUNE( "M_IMMUNE" );
 static const trait_id trait_PARKOUR( "PARKOUR" );
-static const trait_id trait_RUMINANT( "RUMINANT" );
 static const trait_id trait_VINES2( "VINES2" );
 static const trait_id trait_VINES3( "VINES3" );
 
@@ -3977,7 +3975,7 @@ void game::mon_info_update( )
     new_seen_mon.clear();
 
     static int previous_turn = 0;
-    // @todo change current_turn to time_point
+    // @TODO: change current_turn to time_point
     const int current_turn = to_turns<int>( calendar::turn - calendar::turn_zero );
     const int sm_ignored_turns = get_option<int>( "SAFEMODEIGNORETURNS" );
 
@@ -4144,7 +4142,6 @@ void game::mon_info_update( )
     mostseen = newseen;
 }
 
-
 void game::cleanup_dead()
 {
     // Dead monsters need to stay in the tracker until everything else that needs to die does so
@@ -4725,7 +4722,7 @@ monster *game::place_critter_at( const shared_ptr_fast<monster> mon, const tripo
 
 monster *game::place_critter_around( const mtype_id &id, const tripoint &center, const int radius )
 {
-    // @todo change this into an assert, it must never happen.
+    // @TODO: change this into an assert, it must never happen.
     if( id.is_null() ) {
         return nullptr;
     }
@@ -4756,7 +4753,7 @@ monster *game::place_critter_around( const shared_ptr_fast<monster> mon,
 
 monster *game::place_critter_within( const mtype_id &id, const tripoint_range &range )
 {
-    // @todo change this into an assert, it must never happen.
+    // @TODO: change this into an assert, it must never happen.
     if( id.is_null() ) {
         return nullptr;
     }