Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/auto_pickup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
Expand Down
2 changes: 1 addition & 1 deletion src/calendar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion src/cata_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand Down
2 changes: 1 addition & 1 deletion src/cata_tiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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++ ) {
Expand Down
6 changes: 2 additions & 4 deletions src/character.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/clzones.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() ) ) {
Expand Down
72 changes: 42 additions & 30 deletions src/consumption.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
}
}
Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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 );

Expand Down Expand Up @@ -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 );
}
Expand All @@ -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 ) {
Expand Down Expand Up @@ -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;
Expand All @@ -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 ) ) {
Expand All @@ -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 ) {
Expand All @@ -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 );
}

Expand Down Expand Up @@ -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" ) ) ) &&
Expand All @@ -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 );
Expand All @@ -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 );
}
}
Expand All @@ -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 );
}
}
Expand Down Expand Up @@ -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!" ) );
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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() );
}
Expand All @@ -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;
}
6 changes: 3 additions & 3 deletions src/flat_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand All @@ -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();
}
Expand Down
9 changes: 3 additions & 6 deletions src/game.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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" );

Expand Down Expand Up @@ -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" );

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down