Code optimizations reported by static code analysis (2018-11-05)

src/clzones.cpp

@@ -232,7 +232,7 @@ void plot_options::deserialize( JsonObject &jo_zone )
     seed = jo_zone.get_string( "seed", "" );
 };
 
-cata::optional<std::string> zone_manager::query_name( std::string default_name ) const
+cata::optional<std::string> zone_manager::query_name( const std::string &default_name ) const
 {
     string_input_popup popup;
     popup

src/clzones.h

@@ -172,7 +172,7 @@ class zone_manager
         std::vector<zone_data> get_zones( const zone_type_id &type, const tripoint &where ) const;
         const zone_data *get_top_zone( const tripoint &where ) const;
         const zone_data *get_bottom_zone( const tripoint &where ) const;
-        cata::optional<std::string> query_name( std::string default_name = "" ) const;
+        cata::optional<std::string> query_name( const std::string &default_name = "" ) const;
         cata::optional<zone_type_id> query_type() const;
         void swap( zone_data &a, zone_data &b );
 

src/dialogue.h

@@ -153,7 +153,7 @@ struct talk_response {
                 /**
                  * Sets an effect to a function object and consequence to explicitly given one.
                  */
-                void set_effect_consequence( effect_fun_t eff, dialogue_consequence con );
+                void set_effect_consequence( const effect_fun_t &eff, dialogue_consequence con );
                 void set_effect_consequence( std::function<void( npc &p )> ptr, dialogue_consequence con );
 
 

src/dialogue_win.cpp

@@ -138,7 +138,8 @@ void dialogue_window::refresh_response_display()
     can_scroll_up = false;
 }
 
-void dialogue_window::display_responses( int const hilight_lines, std::vector<talk_data> responses,
+void dialogue_window::display_responses( int const hilight_lines,
+        const std::vector<talk_data> &responses,
         const long &ch )
 {
     if( text_only ) {

src/dialogue_win.h

@@ -18,7 +18,8 @@ class dialogue_window
         bool text_only = false;
 
         void clear_window_texts();
-        void display_responses( int hilight_lines, std::vector<talk_data> responses, const long &ch );
+        void display_responses( int hilight_lines, const std::vector<talk_data> &responses,
+                                const long &ch );
         void refresh_response_display();
         /**
          * Folds and adds the folded text to @ref history. Returns the number of added lines.

src/faction_camp.cpp

@@ -2302,7 +2302,7 @@ std::map<std::string, std::string> talk_function::camp_recipe_deck( const std::s
     return cooking_recipes;
 }
 
-int talk_function::camp_recipe_batch_max( const recipe making, const inventory &total_inv )
+int talk_function::camp_recipe_batch_max( const recipe &making, const inventory &total_inv )
 {
     int max_batch = 0;
     int max_checks = 9;
@@ -2752,9 +2752,8 @@ time_duration talk_function::companion_travel_time_calc( const std::vector<tripo
     return work + one_way * trips * 1_minutes;
 }
 
-int talk_function::om_carry_weight_to_trips( units::mass mass, units::volume volume,
-        units::mass carry_mass,
-        units::volume carry_volume )
+int talk_function::om_carry_weight_to_trips( const units::mass &mass, const units::volume &volume,
+        const units::mass &carry_mass, const units::volume &carry_volume )
 {
     int trips_m = 1 + mass / carry_mass;
     int trips_v = 1 + volume / carry_volume;

src/faction_camp.h

@@ -108,8 +108,8 @@ std::string camp_trip_description( time_duration total_time, time_duration worki
 /// Determines how many round trips a given NPC @ref comp will take to move all of the items @ref itms
 int om_carry_weight_to_trips( const std::vector<item *> &itms, npc *comp = nullptr );
 /// Determines how many trips it takes to move @ref mass and @ref volume of items with @ref carry_mass and @ref carry_volume moved per trip
-int om_carry_weight_to_trips( units::mass mass, units::volume volume, units::mass carry_mass,
-                              units::volume carry_volume );
+int om_carry_weight_to_trips( const units::mass &mass, const units::volume &volume,
+                              const units::mass &carry_mass, const units::volume &carry_volume );
 
 /// Improve the camp tile to the next level and pushes the camp manager onto his correct position in case he moved
 bool om_camp_upgrade( npc &p, const tripoint &omt_pos );
@@ -222,7 +222,7 @@ std::string name_mission_tabs( npc &p, const std::string &id, const std::string
 /// Creats a map of all the recipes that are available to a building at om_cur, "ALL" for all possible
 std::map<std::string, std::string> camp_recipe_deck( const std::string &om_cur );
 /// Determines what the absolute max (out of 9999) that can be crafted using inventory and food supplies
-int camp_recipe_batch_max( const recipe making, const inventory &total_inv );
+int camp_recipe_batch_max( const recipe &making, const inventory &total_inv );
 
 /*
  * check if a companion survives a random encounter

src/game.cpp

@@ -5632,7 +5632,7 @@ void game::exam_vehicle( vehicle &veh, int cx, int cy )
     }
 }
 
-bool game::forced_door_closing( const tripoint &p, const ter_id door_type, int bash_dmg )
+bool game::forced_door_closing( const tripoint &p, const ter_id &door_type, int bash_dmg )
 {
     // TODO: Z
     const int &x = p.x;

src/game.h

@@ -850,7 +850,7 @@ class game
         // will do so, if bash_dmg is greater than 0, items won't stop the door
         // from closing at all.
         // If the door gets closed the items on the door tile get moved away or destroyed.
-        bool forced_door_closing( const tripoint &p, const ter_id door_type, int bash_dmg );
+        bool forced_door_closing( const tripoint &p, const ter_id &door_type, int bash_dmg );
 
         //pixel minimap management
         int pixel_minimap_option;

src/input.cpp

@@ -1343,7 +1343,7 @@ std::vector<std::string> input_context::filter_strings_by_phrase(
     return filtered_strings;
 }
 
-void input_context::set_edittext( std::string s )
+void input_context::set_edittext( const std::string &s )
 {
     edittext = s;
 }

src/input.h

@@ -646,7 +646,7 @@ class input_context
         /**
         * Get/Set edittext to display IME unspecified string.
         */
-        void set_edittext( std::string s );
+        void set_edittext( const std::string &s );
         std::string get_edittext();
 
         void set_iso( bool mode = true );

src/inventory_ui.cpp

@@ -1263,7 +1263,7 @@ inventory_selector::stat display_stat( const std::string &caption, int cur_value
 
 inventory_selector::stats inventory_selector::get_weight_and_volume_stats(
     units::mass weight_carried, units::mass weight_capacity,
-    units::volume volume_carried, units::volume volume_capacity )
+    const units::volume &volume_carried, const units::volume &volume_capacity )
 {
     return {
         {

src/inventory_ui.h

@@ -492,7 +492,7 @@ class inventory_selector
 
         static stats get_weight_and_volume_stats(
             units::mass weight_carried, units::mass weight_capacity,
-            units::volume volume_carried, units::volume volume_capacity );
+            const units::volume &volume_carried, const units::volume &volume_capacity );
 
         /** Get stats to display in top right.
          *

src/item.cpp

@@ -626,7 +626,7 @@ bool item::merge_charges( const item &rhs )
     return true;
 }
 
-void item::put_in( item payload )
+void item::put_in( const item &payload )
 {
     contents.push_back( payload );
 }

src/item.h

@@ -624,7 +624,7 @@ class item : public visitable<item>
         /**
          * Puts the given item into this one, no checks are performed.
          */
-        void put_in( item payload );
+        void put_in( const item &payload );
 
         /** Stores a newly constructed item at the end of this item's contents */
         template<typename ... Args>

src/map.cpp

@@ -1078,7 +1078,7 @@ bool map::displace_water( const tripoint &p )
 
 // 2D overloads for furniture
 // To be removed once not needed
-void map::set( const int x, const int y, const ter_id new_terrain, const furn_id new_furniture )
+void map::set( const int x, const int y, const ter_id &new_terrain, const furn_id &new_furniture )
 {
     furn_set( x, y, new_furniture );
     ter_set( x, y, new_terrain );
@@ -1107,7 +1107,7 @@ furn_id map::furn( const int x, const int y ) const
     return current_submap->get_furn( lx, ly );
 }
 
-void map::furn_set( const int x, const int y, const furn_id new_furniture )
+void map::furn_set( const int x, const int y, const furn_id &new_furniture )
 {
     furn_set( tripoint( x, y, abs_sub.z ), new_furniture );
 }
@@ -1118,7 +1118,7 @@ std::string map::furnname( const int x, const int y )
 }
 // End of 2D overloads for furniture
 
-void map::set( const tripoint &p, const ter_id new_terrain, const furn_id new_furniture )
+void map::set( const tripoint &p, const ter_id &new_terrain, const furn_id &new_furniture )
 {
     furn_set( p, new_furniture );
     ter_set( p, new_terrain );
@@ -1160,7 +1160,7 @@ furn_id map::furn( const tripoint &p ) const
     return current_submap->get_furn( lx, ly );
 }
 
-void map::furn_set( const tripoint &p, const furn_id new_furniture )
+void map::furn_set( const tripoint &p, const furn_id &new_furniture )
 {
     if( !inbounds( p ) ) {
         return;
@@ -1254,7 +1254,7 @@ ter_id map::ter( const int x, const int y ) const
     return current_submap->get_ter( lx, ly );
 }
 
-bool map::ter_set( const int x, const int y, const ter_id new_terrain )
+bool map::ter_set( const int x, const int y, const ter_id &new_terrain )
 {
     return ter_set( tripoint( x, y, abs_sub.z ), new_terrain );
 }
@@ -1361,7 +1361,7 @@ bool map::is_harvestable( const tripoint &pos ) const
 /*
  * set terrain via string; this works for -any- terrain id
  */
-bool map::ter_set( const tripoint &p, const ter_id new_terrain )
+bool map::ter_set( const tripoint &p, const ter_id &new_terrain )
 {
     if( !inbounds( p ) ) {
         return false;
@@ -2356,13 +2356,13 @@ void map::make_rubble( const tripoint &p )
     make_rubble( p, f_rubble, false, t_dirt, false );
 }
 
-void map::make_rubble( const tripoint &p, const furn_id rubble_type, const bool items )
+void map::make_rubble( const tripoint &p, const furn_id &rubble_type, const bool items )
 {
     make_rubble( p, rubble_type, items, t_dirt, false );
 }
 
-void map::make_rubble( const tripoint &p, furn_id rubble_type, bool items, ter_id floor_type,
-                       bool overwrite )
+void map::make_rubble( const tripoint &p, const furn_id &rubble_type, const bool items,
+                       const ter_id &floor_type, bool overwrite )
 {
     if( overwrite ) {
         ter_set( p, floor_type );
@@ -3652,7 +3652,7 @@ bool map::open_door( const tripoint &p, const bool inside, const bool check_only
     return false;
 }
 
-void map::translate( const ter_id from, const ter_id to )
+void map::translate( const ter_id &from, const ter_id &to )
 {
     if( from == to ) {
         debugmsg( "map::translate %s => %s",
@@ -3674,7 +3674,7 @@ void map::translate( const ter_id from, const ter_id to )
 }
 
 //This function performs the translate function within a given radius of the player.
-void map::translate_radius( const ter_id from, const ter_id to, float radi, const tripoint &p,
+void map::translate_radius( const ter_id &from, const ter_id &to, float radi, const tripoint &p,
                             const bool same_submap )
 {
     if( from == to ) {

src/map.h

@@ -533,19 +533,19 @@ class map
         void move_vehicle( vehicle &veh, const tripoint &dp, const tileray &facing );
 
         // Furniture: 2D overloads
-        void set( const int x, const int y, const ter_id new_terrain, const furn_id new_furniture );
+        void set( const int x, const int y, const ter_id &new_terrain, const furn_id &new_furniture );
 
         std::string name( const int x, const int y );
         bool has_furn( const int x, const int y ) const;
 
         // Furniture at coordinates (x, y); {x|y}=(0, SEE{X|Y}*3]
         furn_id furn( const int x, const int y ) const;
 
-        void furn_set( const int x, const int y, const furn_id new_furniture );
+        void furn_set( const int x, const int y, const furn_id &new_furniture );
 
         std::string furnname( const int x, const int y );
         // Furniture: 3D
-        void set( const tripoint &p, const ter_id new_terrain, const furn_id new_furniture );
+        void set( const tripoint &p, const ter_id &new_terrain, const furn_id &new_furniture );
 
         std::string name( const tripoint &p );
         std::string disp_name( const tripoint &p );
@@ -558,15 +558,15 @@ class map
 
         furn_id furn( const tripoint &p ) const;
 
-        void furn_set( const tripoint &p, const furn_id new_furniture );
+        void furn_set( const tripoint &p, const furn_id &new_furniture );
 
         std::string furnname( const tripoint &p );
         bool can_move_furniture( const tripoint &pos, player *p = nullptr );
         // Terrain: 2D overloads
         // Terrain integer id at coordinates (x, y); {x|y}=(0, SEE{X|Y}*3]
         ter_id ter( const int x, const int y ) const;
 
-        bool ter_set( const int x, const int y, const ter_id new_terrain );
+        bool ter_set( const int x, const int y, const ter_id &new_terrain );
 
         std::string tername( const int x, const int y ) const; // Name of terrain at (x, y)
         // Terrain: 3D
@@ -581,7 +581,7 @@ class map
         const std::set<std::string> &get_harvest_names( const tripoint &p ) const;
         ter_id get_ter_transforms_into( const tripoint &p ) const;
 
-        bool ter_set( const tripoint &p, const ter_id new_terrain );
+        bool ter_set( const tripoint &p, const ter_id &new_terrain );
 
         std::string tername( const tripoint &p ) const;
 
@@ -694,10 +694,10 @@ class map
 
         /** Generates rubble at the given location, if overwrite is true it just writes on top of what currently exists
          *  floor_type is only used if there is a non-bashable wall at the location or with overwrite = true */
-        void make_rubble( const tripoint &p, furn_id rubble_type, bool items,
-                          ter_id floor_type, bool overwrite = false );
+        void make_rubble( const tripoint &p, const furn_id &rubble_type, const bool items,
+                          const ter_id &floor_type, bool overwrite = false );
         void make_rubble( const tripoint &p );
-        void make_rubble( const tripoint &p, furn_id rubble_type, bool items );
+        void make_rubble( const tripoint &p, const furn_id &rubble_type, const bool items );
 
         bool is_divable( const int x, const int y ) const;
         bool is_outside( const int x, const int y ) const;
@@ -743,9 +743,10 @@ class map
         void add_corpse( const tripoint &p );
 
         // Terrain changing functions
-        void translate( const ter_id from, const ter_id to ); // Change all instances of $from->$to
+        // Change all instances of $from->$to
+        void translate( const ter_id &from, const ter_id &to );
         // Change all instances $from->$to within this radius, optionally limited to locations in the same submap.
-        void translate_radius( const ter_id from, const ter_id to, const float radi, const tripoint &p,
+        void translate_radius( const ter_id &from, const ter_id &to, const float radi, const tripoint &p,
                                const bool same_submap = false );
         bool close_door( const tripoint &p, const bool inside, const bool check_only );
         bool open_door( const tripoint &p, const bool inside, const bool check_only = false );
@@ -1342,11 +1343,11 @@ class map
         void shift_traps( const tripoint &shift );
 
         void copy_grid( const tripoint &to, const tripoint &from );
-        void draw_map( const oter_id terrain_type, const oter_id t_north, const oter_id t_east,
-                       const oter_id t_south, const oter_id t_west, const oter_id t_neast,
-                       const oter_id t_seast, const oter_id t_swest, const oter_id t_nwest,
-                       const oter_id t_above, const oter_id t_below, const time_point &when, const float density,
-                       const int zlevel, const regional_settings *rsettings );
+        void draw_map( const oter_id &terrain_type, const oter_id &t_north, const oter_id &t_east,
+                       const oter_id &t_south, const oter_id &t_west, const oter_id &t_neast,
+                       const oter_id &t_seast, const oter_id &t_swest, const oter_id &t_nwest,
+                       const oter_id &t_above, const oter_id &t_below, const time_point &when,
+                       const float density, const int zlevel, const regional_settings *rsettings );
 
         void build_transparency_cache( int zlev );
     public:

src/mapgen.cpp

@@ -106,7 +106,7 @@ const mtype_id mon_zombie_soldier( "mon_zombie_soldier" );
 const mtype_id mon_zombie_spitter( "mon_zombie_spitter" );
 const mtype_id mon_zombie_tough( "mon_zombie_tough" );
 
-bool connects_to( oter_id there, int dir_from_here );
+bool connects_to( const oter_id &there, int dir_from_here );
 void science_room( map *m, int x1, int y1, int x2, int y2, int z, int rotate );
 void set_science_room( map *m, int x1, int y1, bool faces_right, const time_point &when );
 void silo_rooms( map *m );
@@ -2212,11 +2212,11 @@ void mapgen_function_lua::generate( map *m, const oter_id &terrain_type, const m
 // track down what is and isn't supposed to be carried around between bits of code.
 // I suggest that we break the function down into smaller parts
 
-void map::draw_map( const oter_id terrain_type, const oter_id t_north, const oter_id t_east,
-                    const oter_id t_south, const oter_id t_west, const oter_id t_neast,
-                    const oter_id t_seast, const oter_id t_swest, const oter_id t_nwest,
-                    const oter_id t_above, const oter_id t_below, const time_point &when, const float density,
-                    const int zlevel, const regional_settings *rsettings )
+void map::draw_map( const oter_id &terrain_type, const oter_id &t_north, const oter_id &t_east,
+                    const oter_id &t_south, const oter_id &t_west, const oter_id &t_neast,
+                    const oter_id &t_seast, const oter_id &t_swest, const oter_id &t_nwest,
+                    const oter_id &t_above, const oter_id &t_below, const time_point &when,
+                    const float density, const int zlevel, const regional_settings *rsettings )
 {
     static const mongroup_id GROUP_ZOMBIE( "GROUP_ZOMBIE" );
     static const mongroup_id GROUP_LAB( "GROUP_LAB" );
@@ -7336,7 +7336,7 @@ void map::rotate( int turns )
 }
 
 // Hideous function, I admit...
-bool connects_to( oter_id there, int dir )
+bool connects_to( const oter_id &there, int dir )
 {
     switch( dir ) {
         case 2:

src/mapgen.h

@@ -391,7 +391,7 @@ enum room_type {
 
 void house_room( map *m, room_type type, int x1, int y1, int x2, int y2, mapgendata &dat );
 // helpful functions
-bool connects_to( oter_id there, int dir );
+bool connects_to( const oter_id &there, int dir );
 void mapgen_rotate( map *m, oter_id terrain_type, bool north_is_down = false );
 // wrappers for map:: functions
 void line( map *m, const ter_id type, int x1, int y1, int x2, int y2 );

src/martialarts.cpp

@@ -886,7 +886,7 @@ bool player::can_melee() const
     } );
 }
 
-bool player::has_mabuff( mabuff_id id ) const
+bool player::has_mabuff( const mabuff_id &id ) const
 {
     return search_ma_buff_effect( *effects, [&id]( const ma_buff & b, const effect & ) {
         return b.id == id;