Skip to content

Commit

Permalink
Refactor -- Misc. code improvements via Infer.
Browse files Browse the repository at this point in the history
  • Loading branch information
Espyo committed Oct 10, 2024
1 parent 32c2586 commit 83b51d4
Show file tree
Hide file tree
Showing 22 changed files with 54 additions and 63 deletions.
2 changes: 2 additions & 0 deletions Source/documents/Misc. info.txt
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,8 @@ Procedure for every version release
valgrind --leak-check=yes ./pikifen 2> valgrind.txt
Use clang analyze and correct problems
make CXX=clang++ -f Source/gnu_make/makefile analyze
Use Infer and correct problems
infer run -o ~/Desktop/infer-out --report-block-list-files-containing "libs/imgui" -- make -f Source/gnu_make/makefile debug
Confirm that there are no superfluous #includes (how?)
Code cleanup
Confirm that no line goes over 80 characters
Expand Down
5 changes: 3 additions & 2 deletions Source/source/animation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,8 @@ void animation_database::calculate_hitbox_span() {
* report errors.
*/
void animation_database::create_conversions(
vector<std::pair<size_t, string> > conversions, const data_node* file
const vector<std::pair<size_t, string> >& conversions,
const data_node* file
) {
pre_named_conversions.clear();

Expand Down Expand Up @@ -496,7 +497,7 @@ void animation_database::load_from_data_node(data_node* node) {
vector<string> hazards_strs =
semicolon_list_to_vector(cur_hitbox.hazards_str);
for(size_t hs = 0; hs < hazards_strs.size(); hs++) {
string hazard_name = hazards_strs[hs];
const string& hazard_name = hazards_strs[hs];
if(game.content.hazards.find(hazard_name) == game.content.hazards.end()) {
game.errors.report(
"Unknown hazard \"" + hazard_name + "\"!",
Expand Down
3 changes: 2 additions & 1 deletion Source/source/animation.h
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,8 @@ class animation_database : public content {
size_t find_body_part(const string &name) const;
void calculate_hitbox_span();
void create_conversions(
vector<std::pair<size_t, string> > conversions, const data_node* file
const vector<std::pair<size_t, string> >& conversions,
const data_node* file
);
void delete_sprite(size_t idx);
void fill_sound_idx_caches(mob_type* mt_ptr);
Expand Down
2 changes: 1 addition & 1 deletion Source/source/area/geometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,7 @@ TRIANGULATION_ERROR get_polys(
* @return Whether it is an outer polygon.
*/
bool get_polys_is_outer(
vertex* v_ptr, const sector* s_ptr, unordered_set<edge*> edges_left,
vertex* v_ptr, const sector* s_ptr, const unordered_set<edge*>& edges_left,
bool doing_first_polygon
) {
if(doing_first_polygon) {
Expand Down
2 changes: 1 addition & 1 deletion Source/source/area/geometry.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ TRIANGULATION_ERROR get_polys(
sector* s_ptr, vector<polygon>* outers, vector<vector<polygon>>* inners
);
bool get_polys_is_outer(
vertex* v_ptr, const sector* s_ptr, unordered_set<edge*> edges_left,
vertex* v_ptr, const sector* s_ptr, const unordered_set<edge*>& edges_left,
bool doing_first_polygon
);
vertex* get_rightmost_vertex(const unordered_set<edge*> &edges);
Expand Down
2 changes: 1 addition & 1 deletion Source/source/edge_offset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ void update_offset_effect_buffer(
*/
void update_offset_effect_caches (
vector<edge_offset_cache> &caches,
unordered_set<vertex*> vertexes_to_update,
const unordered_set<vertex*>& vertexes_to_update,
offset_effect_checker_t checker,
offset_effect_length_getter_t length_getter,
offset_effect_color_getter_t color_getter
Expand Down
8 changes: 2 additions & 6 deletions Source/source/functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ bool are_walls_between(
float ignore_walls_below_z, bool* out_impassable_walls
) {
point bb_tl(std::min(p1.x, p2.x), std::min(p1.y, p2.y));
point bb_br(std::max(p1.x, p1.x), std::max(p2.y, p2.y));
point bb_br(std::max(p1.x, p2.x), std::max(p1.y, p2.y));

set<edge*> candidate_edges;
if(
Expand Down Expand Up @@ -694,11 +694,7 @@ vector<std::pair<int, string> > get_weather_table(data_node* node) {

for(size_t p = 0; p < n_points; p++) {
data_node* point_node = node->get_child(p);

int point_time = s2i(point_node->name);
string point_value = point_node->value;

table.push_back(make_pair(point_time, point_value));
table.push_back(make_pair(s2i(point_node->name), point_node->value));
}

sort(
Expand Down
2 changes: 1 addition & 1 deletion Source/source/functions.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ void update_offset_effect_buffer(
);
void update_offset_effect_caches (
vector<edge_offset_cache> &caches,
unordered_set<vertex*> vertexes_to_update,
const unordered_set<vertex*>& vertexes_to_update,
offset_effect_checker_t checker,
offset_effect_length_getter_t length_getter,
offset_effect_color_getter_t color_getter
Expand Down
10 changes: 5 additions & 5 deletions Source/source/game_states/animation_editor/editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,7 @@ void animation_editor::zoom_everything_cmd(float input_value) {

sprite* s_ptr = cur_sprite;
if(!s_ptr && cur_anim_i.valid_frame()) {
string name =
const string& name =
cur_anim_i.cur_anim->frames[cur_anim_i.cur_frame_idx].sprite_name;
size_t s_pos = anims.find_sprite(name);
if(s_pos != INVALID) s_ptr = anims.sprites[s_pos];
Expand Down Expand Up @@ -920,7 +920,7 @@ void animation_editor::rename_animation(
return;
}

string old_name = anim->name;
const string& old_name = anim->name;

//Check if the name is the same.
if(new_name == old_name) {
Expand Down Expand Up @@ -969,7 +969,7 @@ void animation_editor::rename_body_part(
return;
}

string old_name = part->name;
const string& old_name = part->name;

//Check if the name is the same.
if(new_name == old_name) {
Expand Down Expand Up @@ -1026,7 +1026,7 @@ void animation_editor::rename_sprite(
return;
}

string old_name = spr->name;
const string& old_name = spr->name;

//Check if the name is the same.
if(new_name == old_name) {
Expand Down Expand Up @@ -1500,7 +1500,7 @@ void animation_editor::update_hitboxes() {
//Add missing hitboxes.
for(size_t b = 0; b < anims.body_parts.size(); b++) {
bool hitbox_found = false;
string name = anims.body_parts[b]->name;
const string& name = anims.body_parts[b]->name;

for(size_t h = 0; h < s_ptr->hitboxes.size(); h++) {
if(s_ptr->hitboxes[h].body_part_name == name) {
Expand Down
23 changes: 8 additions & 15 deletions Source/source/game_states/area_editor/editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -570,22 +570,17 @@ void area_editor::create_mob_under_cursor() {
sub_state = EDITOR_SUB_STATE_NONE;
point hotspot = snap_point(game.mouse_cursor.w_pos);

string custom_cat_name_to_use = last_mob_custom_cat_name;
mob_type* type_to_use = last_mob_type;
if(custom_cat_name_to_use.empty()) {
custom_cat_name_to_use =
if(last_mob_custom_cat_name.empty()) {
last_mob_custom_cat_name =
game.config.pikmin_order[0]->custom_category_name;
type_to_use =
last_mob_type =
game.config.pikmin_order[0];
}

game.cur_area_data.mob_generators.push_back(
new mob_gen(hotspot, type_to_use)
new mob_gen(hotspot, last_mob_type)
);

last_mob_custom_cat_name = custom_cat_name_to_use;
last_mob_type = type_to_use;

selected_mobs.insert(game.cur_area_data.mob_generators.back());

set_status("Created object.");
Expand Down Expand Up @@ -1921,20 +1916,18 @@ void area_editor::load_area(
const AREA_TYPE requested_area_type,
bool from_backup, bool should_update_history
) {
string new_area_name = requested_area_folder_name;

clear_current_area();

::load_area(
new_area_name, requested_area_type, true, from_backup
requested_area_folder_name, requested_area_type, true, from_backup
);

//Calculate texture suggestions.
map<string, size_t> texture_uses_map;
vector<std::pair<string, size_t> > texture_uses_vector;

for(size_t s = 0; s < game.cur_area_data.sectors.size(); s++) {
string n = game.cur_area_data.sectors[s]->texture_info.file_name;
const string& n = game.cur_area_data.sectors[s]->texture_info.file_name;
if(n.empty()) continue;
texture_uses_map[n]++;
}
Expand Down Expand Up @@ -1973,13 +1966,13 @@ void area_editor::load_area(
if(should_update_history) {
update_history(
get_base_area_folder_path(requested_area_type, true) + "/" +
new_area_name
requested_area_folder_name
);
save_options(); //Save the history in the options.
}

set_status(
"Loaded area \"" + new_area_name + "\" " +
"Loaded area \"" + requested_area_folder_name + "\" " +
(from_backup ? "from a backup " : "") +
"successfully."
);
Expand Down
5 changes: 2 additions & 3 deletions Source/source/game_states/area_editor/geometry_logic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1903,11 +1903,10 @@ bool area_editor::merge_sectors(sector* s1, sector* s2) {
void area_editor::merge_vertex(
const vertex* v1, vertex* v2, unordered_set<sector*>* affected_sectors
) {
vector<edge*> edges = v1->edges;
//Find out what to do with every edge of the dragged vertex.
for(size_t e = 0; e < edges.size(); e++) {
for(size_t e = 0; e < v1->edges.size(); e++) {

edge* e_ptr = edges[e];
edge* e_ptr = v1->edges[e];
vertex* other_vertex = e_ptr->get_other_vertex(v1);

if(other_vertex == v2) {
Expand Down
2 changes: 1 addition & 1 deletion Source/source/game_states/area_menu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ void area_menu_state::load() {
"/" + actual_name + "/" + AREA_DATA_FILE_NAME
);
if(data.file_was_opened) {
string s = data.get_child_by_name("name")->value;
const string& s = data.get_child_by_name("name")->value;
if(!s.empty()) {
actual_name = s;
}
Expand Down
10 changes: 5 additions & 5 deletions Source/source/game_states/editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1534,9 +1534,9 @@ void editor::process_gui_unsaved_changes_dialog() {
ImGui::SameLine(0.0f, 10);
if(ImGui::Button("Save", ImVec2(180, 30))) {
close_top_dialog();
const std::function<bool()> save_callback =
const std::function<bool()>& save_callback =
changes_mgr.get_unsaved_warning_save_callback();
const std::function<void()> action_callback =
const std::function<void()>& action_callback =
changes_mgr.get_unsaved_warning_action_callback();
if(save_callback()) {
action_callback();
Expand Down Expand Up @@ -2029,9 +2029,9 @@ bool editor::changes_manager::ask_if_unsaved(
ed->key_check(ev->keyboard.keycode, ALLEGRO_KEY_S, true)
) {
ed->close_top_dialog();
const std::function<bool()> save_callback =
const std::function<bool()>& save_callback =
this->get_unsaved_warning_save_callback();
const std::function<void()> action_callback =
const std::function<void()>& action_callback =
this->get_unsaved_warning_action_callback();
if(save_callback()) {
action_callback();
Expand All @@ -2040,7 +2040,7 @@ bool editor::changes_manager::ask_if_unsaved(
ed->key_check(ev->keyboard.keycode, ALLEGRO_KEY_D, true)
) {
ed->close_top_dialog();
const std::function<void()> action_callback =
const std::function<void()>& action_callback =
this->get_unsaved_warning_action_callback();
action_callback();
}
Expand Down
4 changes: 2 additions & 2 deletions Source/source/game_states/gameplay/drawing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ void gameplay_state::draw_big_msg() {

} case BIG_MESSAGE_MISSION_CLEAR:
case BIG_MESSAGE_MISSION_FAILED: {
const string TEXT =
const string& TEXT =
cur_big_msg == BIG_MESSAGE_MISSION_CLEAR ?
GAMEPLAY::BIG_MSG_MISSION_CLEAR_TEXT :
GAMEPLAY::BIG_MSG_MISSION_FAILED_TEXT;
Expand Down Expand Up @@ -1752,9 +1752,9 @@ ALLEGRO_BITMAP* gameplay_state::draw_to_bitmap() {
//Figure out the scale that will fit on the image.
float area_w = max_x - min_x + game.maker_tools.area_image_padding;
float area_h = max_y - min_y + game.maker_tools.area_image_padding;
float scale = 1.0f;
float final_bmp_w = game.maker_tools.area_image_size;
float final_bmp_h = final_bmp_w;
float scale;

if(area_w > area_h) {
scale = game.maker_tools.area_image_size / area_w;
Expand Down
2 changes: 1 addition & 1 deletion Source/source/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ void init_controls() {
const vector<player_action_type> &action_types =
game.controls.get_all_player_action_types();
for(size_t a = 0; a < action_types.size(); a++) {
string def = action_types[a].default_bind_str;
const string& def = action_types[a].default_bind_str;
if(def.empty()) continue;

control_bind bind;
Expand Down
14 changes: 7 additions & 7 deletions Source/source/libs/data_file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,32 +532,32 @@ bool data_node::remove(data_node* node_to_remove) {
* @brief Saves a node into a new text file. Line numbers are ignored.
* If you don't provide a file name, it'll use the node's file name.
*
* @param file_path Path to the file to save to.
* @param destination_file_path Path to the file to save to.
* @param children_only If true, only save the nodes inside this node.
* @param include_empty_values If true, even nodes with an empty value
* will be saved.
* @param encrypted If true, the file must be encrypted.
* @return Whether it succeded.
*/
bool data_node::save_file(
string file_path, bool children_only,
string destination_file_path, bool children_only,
bool include_empty_values, bool encrypted
) const {

if(file_path == "") file_path = this->file_path;
if(destination_file_path == "") destination_file_path = this->file_path;

//Create any missing folders.
size_t next_slash_pos = file_path.find('/', 0);
size_t next_slash_pos = destination_file_path.find('/', 0);
while(next_slash_pos != string::npos) {
string path_so_far = file_path.substr(0, next_slash_pos);
string path_so_far = destination_file_path.substr(0, next_slash_pos);
if(!al_make_directory(path_so_far.c_str())) {
return false;
}
next_slash_pos = file_path.find('/', next_slash_pos + 1);
next_slash_pos = destination_file_path.find('/', next_slash_pos + 1);
}

//Save the file.
ALLEGRO_FILE* file = al_fopen(file_path.c_str(), "w");
ALLEGRO_FILE* file = al_fopen(destination_file_path.c_str(), "w");
if(file) {
if(children_only) {
for(size_t c = 0; c < children.size(); c++) {
Expand Down
2 changes: 1 addition & 1 deletion Source/source/libs/data_file.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class data_node {
bool names_only_after_root = false
);
bool save_file(
string file_path = "", bool children_only = true,
string destination_file_path = "", bool children_only = true,
bool include_empty_values = false,
bool encrypted = false
) const;
Expand Down
2 changes: 1 addition & 1 deletion Source/source/mob_script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ mob_event::mob_event(
#define r(name, number) \
else if(n == (name)) type = (number)

string n = node->name;
const string& n = node->name;
if(n == "on_enter") type = MOB_EV_ON_ENTER;
r("on_leave", MOB_EV_ON_LEAVE);
r("on_tick", MOB_EV_ON_TICK);
Expand Down
7 changes: 2 additions & 5 deletions Source/source/mob_script_action.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2370,22 +2370,19 @@ bool assert_actions(
set<string> labels;
for(size_t a = 0; a < actions.size(); a++) {
if(actions[a]->action->type == MOB_ACTION_LABEL) {

string name = actions[a]->args[0];

const string& name = actions[a]->args[0];
if(labels.find(name) != labels.end()) {
game.errors.report(
"There are multiple labels called \"" + name + "\"!", dn
);
return false;
}

labels.insert(name);
}
}
for(size_t a = 0; a < actions.size(); a++) {
if(actions[a]->action->type == MOB_ACTION_GOTO) {
string name = actions[a]->args[0];
const string& name = actions[a]->args[0];
if(labels.find(name) == labels.end()) {
game.errors.report(
"There is no label called \"" + name + "\", even though "
Expand Down
2 changes: 1 addition & 1 deletion Source/source/mobs/mob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2311,7 +2311,7 @@ void mob::get_sprite_bitmap_effects(
) {
float damage_squash_time_ratio =
damage_squash_time / MOB::DAMAGE_SQUASH_DURATION;
float damage_scale_y = 1.0f;
float damage_scale_y;
if(damage_squash_time_ratio > 0.5) {
damage_scale_y =
interpolate_number(
Expand Down
5 changes: 3 additions & 2 deletions Source/source/utils/drawing_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -544,9 +544,10 @@ void get_multiline_text_dimensions(
int* out_width, int* out_height, int* out_line_height
) {
int lh = al_get_font_line_height(font);
size_t n_lines = lines.size();

if(out_height) *out_height = std::max(0, (int) ((lh + 1) * n_lines) - 1);
if(out_height) {
*out_height = std::max(0, (int) ((lh + 1) * lines.size()) - 1);
}

if(out_width) {
int largest_w = 0;
Expand Down
Loading

0 comments on commit 83b51d4

Please sign in to comment.