Merge pull request #218 from OpenVicProject/array-dict-tweaks

Pre-allocate and use std::move for Godot Arrays and Dictionaries

7 files changed, 98 insertions, 64 deletions

diff --git a/extension/src/openvic-extension/classes/GFXPieChartTexture.hpp b/extension/src/openvic-extension/classes/GFXPieChartTexture.hpp
index f1fbe1a..8683b10 100644
--- a/extension/src/openvic-extension/classes/GFXPieChartTexture.hpp
+++ b/extension/src/openvic-extension/classes/GFXPieChartTexture.hpp
@@ -54,12 +54,14 @@ namespace OpenVic {
 				return lhs.first < rhs.first;
 			});
 			godot_pie_chart_data_t array;
-			for (auto const& [key, val] : sorted_dist) {
+			ERR_FAIL_COND_V(array.resize(sorted_dist.size()) != OK, {});
+			for (size_t idx = 0; idx < array.size(); ++idx) {
+				auto const& [key, val] = sorted_dist[idx];
 				Dictionary sub_dict;
 				sub_dict[_slice_identifier_key()] = Utilities::std_view_to_godot_string(key->get_identifier());
 				sub_dict[_slice_colour_key()] = Utilities::to_godot_color(key->get_colour());
 				sub_dict[_slice_weight_key()] = val.to_float();
-				array.push_back(sub_dict);
+				array[idx] = std::move(sub_dict);
 			}
 			return array;
 		}
diff --git a/extension/src/openvic-extension/classes/MapMesh.cpp b/extension/src/openvic-extension/classes/MapMesh.cpp
index a557105..8db3b84 100644
--- a/extension/src/openvic-extension/classes/MapMesh.cpp
+++ b/extension/src/openvic-extension/classes/MapMesh.cpp
@@ -92,7 +92,7 @@ bool MapMesh::is_valid_uv_coord(godot::Vector2 const& uv) const {
 
 Array MapMesh::_create_mesh_array() const {
 	Array arr;
-	arr.resize(Mesh::ARRAY_MAX);
+	ERR_FAIL_COND_V(arr.resize(Mesh::ARRAY_MAX) != OK, {});
 
 	const int32_t vertex_count = (subdivide_w + 2) * (subdivide_d + 2);
 	const int32_t indice_count = (subdivide_w + 1) * (subdivide_d + 1) * 6;
@@ -103,11 +103,11 @@ Array MapMesh::_create_mesh_array() const {
 	PackedVector2Array uvs;
 	PackedInt32Array indices;
 
-	points.resize(vertex_count);
-	normals.resize(vertex_count);
-	tangents.resize(vertex_count * 4);
-	uvs.resize(vertex_count);
-	indices.resize(indice_count);
+	ERR_FAIL_COND_V(points.resize(vertex_count) != OK, {});
+	ERR_FAIL_COND_V(normals.resize(vertex_count) != OK, {});
+	ERR_FAIL_COND_V(tangents.resize(vertex_count * 4) != OK, {});
+	ERR_FAIL_COND_V(uvs.resize(vertex_count) != OK, {});
+	ERR_FAIL_COND_V(indices.resize(indice_count) != OK, {});
 
 	static const Vector3 normal { 0.0f, 1.0f, 0.0f };
 	const Size2 uv_size { 1.0f + 2.0f * repeat_proportion, 1.0f };
@@ -153,11 +153,11 @@ Array MapMesh::_create_mesh_array() const {
 		thisrow = point_index;
 	}
 
-	arr[Mesh::ARRAY_VERTEX] = points;
-	arr[Mesh::ARRAY_NORMAL] = normals;
-	arr[Mesh::ARRAY_TANGENT] = tangents;
-	arr[Mesh::ARRAY_TEX_UV] = uvs;
-	arr[Mesh::ARRAY_INDEX] = indices;
+	arr[Mesh::ARRAY_VERTEX] = std::move(points);
+	arr[Mesh::ARRAY_NORMAL] = std::move(normals);
+	arr[Mesh::ARRAY_TANGENT] = std::move(tangents);
+	arr[Mesh::ARRAY_TEX_UV] = std::move(uvs);
+	arr[Mesh::ARRAY_INDEX] = std::move(indices);
 
 	return arr;
 }
diff --git a/extension/src/openvic-extension/singletons/GameSingleton.cpp b/extension/src/openvic-extension/singletons/GameSingleton.cpp
index 8b19dc3..df0e9a0 100644
--- a/extension/src/openvic-extension/singletons/GameSingleton.cpp
+++ b/extension/src/openvic-extension/singletons/GameSingleton.cpp
@@ -196,11 +196,11 @@ Error GameSingleton::_update_colour_image() {
 	static constexpr int32_t colour_image_width = PROVINCE_INDEX_SQRT * sizeof(Mapmode::base_stripe_t) / sizeof(colour_argb_t);
 	/* Province count + null province, rounded up to next multiple of PROVINCE_INDEX_SQRT.
 	 * Rearranged from: (map.get_province_count() + 1) + (PROVINCE_INDEX_SQRT - 1) */
-	static const int32_t colour_image_height = (map.get_province_count() + PROVINCE_INDEX_SQRT) / PROVINCE_INDEX_SQRT;
+	const int32_t colour_image_height = (map.get_province_count() + PROVINCE_INDEX_SQRT) / PROVINCE_INDEX_SQRT;
 
 	static PackedByteArray colour_data_array;
-	static const int64_t colour_data_array_size = colour_image_width * colour_image_height * sizeof(colour_argb_t);
-	colour_data_array.resize(colour_data_array_size);
+	const int64_t colour_data_array_size = colour_image_width * colour_image_height * sizeof(colour_argb_t);
+	ERR_FAIL_COND_V(colour_data_array.resize(colour_data_array_size) != OK, FAILED);
 
 	Error err = OK;
 	if (!map.generate_mapmode_colours(mapmode_index, colour_data_array.ptrw())) {
@@ -286,19 +286,25 @@ Error GameSingleton::_load_map_images() {
 	}
 
 	Map::shape_pixel_t const* province_shape_data = game_manager.get_map().get_province_shape_image().data();
+
 	const Vector2i divided_dims = province_dims / image_subdivisions;
+	const int64_t subdivision_width = divided_dims.x * sizeof(Map::shape_pixel_t);
+	const int64_t subdivision_size = subdivision_width * divided_dims.y;
+
 	TypedArray<Image> province_shape_images;
-	province_shape_images.resize(image_subdivisions.x * image_subdivisions.y);
+	ERR_FAIL_COND_V(province_shape_images.resize(image_subdivisions.x * image_subdivisions.y) != OK, FAILED);
+
+	PackedByteArray index_data_array;
+	ERR_FAIL_COND_V(index_data_array.resize(subdivision_size) != OK, FAILED);
+
 	for (int32_t v = 0; v < image_subdivisions.y; ++v) {
 		for (int32_t u = 0; u < image_subdivisions.x; ++u) {
-			PackedByteArray index_data_array;
-			index_data_array.resize(divided_dims.x * divided_dims.y * sizeof(Map::shape_pixel_t));
 
 			for (int32_t y = 0; y < divided_dims.y; ++y) {
 				memcpy(
-					index_data_array.ptrw() + y * divided_dims.x * sizeof(Map::shape_pixel_t),
+					index_data_array.ptrw() + y * subdivision_width,
 					province_shape_data + (v * divided_dims.y + y) * province_dims.x + u * divided_dims.x,
-					divided_dims.x * sizeof(Map::shape_pixel_t)
+					subdivision_width
 				);
 			}
 
@@ -348,6 +354,7 @@ Error GameSingleton::_load_terrain_variants() {
 	const int32_t slice_size = sheet_width / SHEET_DIMS;
 
 	TypedArray<Image> terrain_images;
+	ERR_FAIL_COND_V(terrain_images.resize(SHEET_SIZE + 1) != OK, FAILED);
 	{
 		/* This is a placeholder image so that we don't have to branch to avoid looking up terrain index 0 (water).
 		 * It should never appear in game, and so is bright red to to make it obvious if it slips through. */
@@ -355,26 +362,25 @@ Error GameSingleton::_load_terrain_variants() {
 			{ 1.0f, 0.0f, 0.0f }, slice_size, slice_size, terrain_sheet->get_format()
 		);
 		ERR_FAIL_NULL_V_EDMSG(water_image, FAILED, "Failed to create water terrain image");
-		terrain_images.append(water_image);
+		terrain_images[0] = water_image;
 	}
-	Error err = OK;
+
 	for (int32_t idx = 0; idx < SHEET_SIZE; ++idx) {
 		const Rect2i slice { idx % SHEET_DIMS * slice_size, idx / SHEET_DIMS * slice_size, slice_size, slice_size };
 		const Ref<Image> terrain_image = terrain_sheet->get_region(slice);
-		if (terrain_image.is_null() || terrain_image->is_empty()) {
-			UtilityFunctions::push_error(
-				"Failed to extract terrain texture slice ", slice, " from ", terrain_texturesheet_path
-			);
-			err = FAILED;
-		}
-		terrain_images.append(terrain_image);
+
+		ERR_FAIL_COND_V_MSG(terrain_image.is_null() || terrain_image->is_empty(), FAILED, vformat(
+			"Failed to extract terrain texture slice %s from %s", slice, terrain_texturesheet_path
+		));
+
+		terrain_images[idx + 1] = terrain_image;
 	}
 
 	terrain_texture.instantiate();
 	ERR_FAIL_COND_V_MSG(
 		terrain_texture->create_from_images(terrain_images) != OK, FAILED, "Failed to create terrain texture array!"
 	);
-	return err;
+	return OK;
 }
 
 Error GameSingleton::_load_flag_images() {
diff --git a/extension/src/openvic-extension/singletons/LoadLocalisation.cpp b/extension/src/openvic-extension/singletons/LoadLocalisation.cpp
index 8860105..16ebe57 100644
--- a/extension/src/openvic-extension/singletons/LoadLocalisation.cpp
+++ b/extension/src/openvic-extension/singletons/LoadLocalisation.cpp
@@ -111,7 +111,7 @@ Error LoadLocalisation::load_localisation_dir(String const& dir_path) const {
 	ERR_FAIL_COND_V_MSG(
 		!DirAccess::dir_exists_absolute(dir_path), FAILED, vformat("Localisation directory does not exist: %s", dir_path)
 	);
-	PackedStringArray const dirs = DirAccess::get_directories_at(dir_path);
+	const PackedStringArray dirs = DirAccess::get_directories_at(dir_path);
 	ERR_FAIL_COND_V_MSG(
 		dirs.size() < 1, FAILED, vformat("Localisation directory does not contain any sub-directories: %s", dir_path)
 	);
diff --git a/extension/src/openvic-extension/singletons/MenuSingleton.cpp b/extension/src/openvic-extension/singletons/MenuSingleton.cpp
index 02fcd54..993549c 100644
--- a/extension/src/openvic-extension/singletons/MenuSingleton.cpp
+++ b/extension/src/openvic-extension/singletons/MenuSingleton.cpp
@@ -1,5 +1,7 @@
 #include "MenuSingleton.hpp"
 
+#include <godot_cpp/variant/utility_functions.hpp>
+
 #include <openvic-simulation/GameManager.hpp>
 
 #include "openvic-extension/classes/GFXPieChartTexture.hpp"
@@ -208,11 +210,17 @@ Dictionary MenuSingleton::get_province_info_from_index(int32_t index) const {
 	std::vector<Country const*> const& cores = province->get_cores();
 	if (!cores.empty()) {
 		PackedStringArray cores_array;
-		cores_array.resize(cores.size());
-		for (size_t idx = 0; idx < cores.size(); ++idx) {
-			cores_array[idx] = std_view_to_godot_string(cores[idx]->get_identifier());
+		if (cores_array.resize(cores.size()) == OK) {
+			for (size_t idx = 0; idx < cores.size(); ++idx) {
+				cores_array[idx] = std_view_to_godot_string(cores[idx]->get_identifier());
+			}
+			ret[province_info_cores_key] = std::move(cores_array);
+		} else {
+			UtilityFunctions::push_error(
+				"Failed to resize cores array to the correct size (", static_cast<int64_t>(cores.size()), ") for province ",
+				std_view_to_godot_string(province->get_identifier())
+			);
 		}
-		ret[province_info_cores_key] = cores_array;
 	}
 
 	static const StringName building_info_level_key = "level";
@@ -226,20 +234,26 @@ Dictionary MenuSingleton::get_province_info_from_index(int32_t index) const {
 		/* This system relies on the province buildings all being present in the right order. It will have to
 		 * be changed if we want to support variable combinations and permutations of province buildings. */
 		TypedArray<Dictionary> buildings_array;
-		buildings_array.resize(buildings.size());
-		for (size_t idx = 0; idx < buildings.size(); ++idx) {
-			BuildingInstance const& building = buildings[idx];
-
-			Dictionary building_dict;
-			building_dict[building_info_level_key] = static_cast<int32_t>(building.get_level());
-			building_dict[building_info_expansion_state_key] = static_cast<int32_t>(building.get_expansion_state());
-			building_dict[building_info_start_date_key] = std_to_godot_string(building.get_start_date().to_string());
-			building_dict[building_info_end_date_key] = std_to_godot_string(building.get_end_date().to_string());
-			building_dict[building_info_expansion_progress_key] = building.get_expansion_progress();
-
-			buildings_array[idx] = building_dict;
+		if (buildings_array.resize(buildings.size()) == OK) {
+			for (size_t idx = 0; idx < buildings.size(); ++idx) {
+				BuildingInstance const& building = buildings[idx];
+
+				Dictionary building_dict;
+				building_dict[building_info_level_key] = static_cast<int32_t>(building.get_level());
+				building_dict[building_info_expansion_state_key] = static_cast<int32_t>(building.get_expansion_state());
+				building_dict[building_info_start_date_key] = std_to_godot_string(building.get_start_date().to_string());
+				building_dict[building_info_end_date_key] = std_to_godot_string(building.get_end_date().to_string());
+				building_dict[building_info_expansion_progress_key] = building.get_expansion_progress();
+
+				buildings_array[idx] = std::move(building_dict);
+			}
+			ret[province_info_buildings_key] = std::move(buildings_array);
+		} else {
+			UtilityFunctions::push_error(
+				"Failed to resize buildings array to the correct size (", static_cast<int64_t>(buildings.size()),
+				") for province ", std_view_to_godot_string(province->get_identifier())
+			);
 		}
-		ret[province_info_buildings_key] = buildings_array;
 	}
 	return ret;
 }
diff --git a/extension/src/openvic-extension/singletons/PopulationMenu.cpp b/extension/src/openvic-extension/singletons/PopulationMenu.cpp
index f97cae6..a598ceb 100644
--- a/extension/src/openvic-extension/singletons/PopulationMenu.cpp
+++ b/extension/src/openvic-extension/singletons/PopulationMenu.cpp
@@ -575,9 +575,10 @@ TypedArray<Dictionary> MenuSingleton::get_population_menu_pop_rows(int32_t start
 	// TODO - monthly change
 
 	TypedArray<Dictionary> array;
+	ERR_FAIL_COND_V(array.resize(count) != OK, {});
 
-	for (int32_t idx = start; idx < start + count; ++idx) {
-		Pop const* pop = population_menu.filtered_pops[idx];
+	for (int32_t idx = 0; idx < count; ++idx) {
+		Pop const* pop = population_menu.filtered_pops[start + idx];
 		Dictionary pop_dict;
 
 		pop_dict[pop_size_key] = pop->get_size();
@@ -598,7 +599,7 @@ TypedArray<Dictionary> MenuSingleton::get_population_menu_pop_rows(int32_t start
 		pop_dict[pop_size_change_key] = pop->get_total_change();
 		pop_dict[pop_literacy_key] = pop->get_literacy().to_float();
 
-		array.push_back(pop_dict);
+		array[idx] = std::move(pop_dict);
 	}
 
 	return array;
@@ -619,9 +620,10 @@ PackedInt32Array MenuSingleton::get_population_menu_pop_filter_setup_info() {
 	ERR_FAIL_COND_V_MSG(population_menu.pop_filters.empty(), {}, "Failed to generate population menu pop filters!");
 
 	PackedInt32Array array;
+	ERR_FAIL_COND_V(array.resize(population_menu.pop_filters.size()) != OK, {});
 
-	for (auto const& [pop_type, filter] : population_menu.pop_filters) {
-		array.push_back(pop_type->get_sprite());
+	for (int32_t idx = 0; idx < array.size(); ++idx) {
+		array[idx] = population_menu.pop_filters.data()[idx].first->get_sprite();
 	}
 
 	return array;
@@ -633,13 +635,18 @@ TypedArray<Dictionary> MenuSingleton::get_population_menu_pop_filter_info() cons
 	static const StringName pop_filter_selected_key = "selected";
 
 	TypedArray<Dictionary> array;
+	ERR_FAIL_COND_V(array.resize(population_menu.pop_filters.size()) != OK, {});
+
+	for (int32_t idx = 0; idx < array.size(); ++idx) {
+		population_menu_t::pop_filter_t const& filter = population_menu.pop_filters.data()[idx].second;
 
-	for (auto const& [pop_type, filter] : population_menu.pop_filters) {
 		Dictionary filter_dict;
+
 		filter_dict[pop_filter_count_key] = filter.count;
 		filter_dict[pop_filter_change_key] = filter.promotion_demotion_change;
 		filter_dict[pop_filter_selected_key] = filter.selected;
-		array.push_back(filter_dict);
+
+		array[idx] = std::move(filter_dict);
 	}
 
 	return array;
@@ -688,17 +695,20 @@ void MenuSingleton::population_menu_deselect_all_pop_filters() {
 
 PackedStringArray MenuSingleton::get_population_menu_distribution_setup_info() const {
 	static const PackedStringArray distribution_names = []() -> PackedStringArray {
-		PackedStringArray array;
-
-		for (char const* name : std::array<char const*, population_menu_t::DISTRIBUTION_COUNT> {
+		constexpr std::array<char const*, population_menu_t::DISTRIBUTION_COUNT> NAMES {
 			/* Workforce (PopType)   */       "WORKFORCE_DISTTITLE",
 			/* Religion              */        "RELIGION_DISTTITLE",
 			/* Ideology              */        "IDEOLOGY_DISTTITLE",
 			/* Nationality (Culture) */     "NATIONALITY_DISTTITLE",
 			/* Issues                */ "DOMINANT_ISSUES_DISTTITLE",
 			/* Vote                  */      "ELECTORATE_DISTTITLE"
-		}) {
-			array.push_back(name);
+		};
+
+		PackedStringArray array;
+		ERR_FAIL_COND_V(array.resize(NAMES.size()) != OK, {});
+
+		for (int32_t idx = 0; idx < array.size(); ++idx) {
+			array[idx] = NAMES[idx];
 		}
 
 		return array;
@@ -709,9 +719,10 @@ PackedStringArray MenuSingleton::get_population_menu_distribution_setup_info() c
 
 TypedArray<Array> MenuSingleton::get_population_menu_distribution_info() const {
 	TypedArray<Array> array;
+	ERR_FAIL_COND_V(array.resize(population_menu.distributions.size()) != OK, {});
 
-	for (fixed_point_map_t<HasIdentifierAndColour const*> const& distribution : population_menu.distributions) {
-		array.push_back(GFXPieChartTexture::distribution_to_slices_array(distribution));
+	for (int32_t idx = 0; idx < array.size(); ++idx) {
+		array[idx] = GFXPieChartTexture::distribution_to_slices_array(population_menu.distributions[idx]);
 	}
 
 	return array;
diff --git a/extension/src/openvic-extension/utility/Utilities.cpp b/extension/src/openvic-extension/utility/Utilities.cpp
index 7450212..4389e95 100644
--- a/extension/src/openvic-extension/utility/Utilities.cpp
+++ b/extension/src/openvic-extension/utility/Utilities.cpp
@@ -80,7 +80,8 @@ static Ref<Image> load_dds_image(String const& path) {
 	);
 
 	PackedByteArray pixels;
-	pixels.resize(size);
+	ERR_FAIL_COND_V(pixels.resize(size) != OK, nullptr);
+
 	/* Index offset used to control whether we are reading */
 	const size_t rb_idx = 2 * needs_bgr_to_rgb;
 	uint8_t const* ptr = static_cast<uint8_t const*>(texture.data());