Data types, defaults, callback cleanup

3 files changed, 67 insertions, 58 deletions

diff --git a/src/openvic-simulation/map/Building.cpp b/src/openvic-simulation/map/Building.cpp
index 4e26ab9..6f4c099 100644
--- a/src/openvic-simulation/map/Building.cpp
+++ b/src/openvic-simulation/map/Building.cpp
@@ -5,7 +5,7 @@
 using namespace OpenVic;
 using namespace OpenVic::NodeTools;
 
-Building::Building(std::string_view identifier, ARGS) : HasIdentifier { identifier }, ModifierValue { std::move(modifiers) }, type { type },
+Building::Building(std::string_view identifier, BuildingType const& type, ARGS) : HasIdentifier { identifier }, ModifierValue { std::move(modifiers) }, type { type },
 	on_completion { on_completion }, completion_size { completion_size }, max_level { max_level }, goods_cost { goods_cost }, cost { cost }, build_time { build_time },
 	visibility { visibility }, on_map { on_map }, default_enabled { default_enabled }, production_type { production_type }, pop_build_factory { pop_build_factory },
 	strategic_factory { strategic_factory }, advanced_factory { advanced_factory }, fort_level { fort_level }, naval_capacity { naval_capacity },
@@ -17,8 +17,8 @@ BuildingType const& Building::get_type() const {
 	return type;
 }
 
-std::string_view Building::get_on_completion() const {
-	return on_completion; 
+std::string const& Building::get_on_completion() const {
+	return on_completion;
 }
 
 fixed_point_t Building::get_completion_size() const {
@@ -29,7 +29,7 @@ Building::level_t Building::get_max_level() const {
 	return max_level;
 }
 
-std::map<const Good*, fixed_point_t> const& Building::get_goods_cost() const {
+std::map<Good const*, fixed_point_t> const& Building::get_goods_cost() const {
 	return goods_cost;
 }
 
@@ -77,7 +77,7 @@ uint64_t Building::get_naval_capacity() const {
 	return naval_capacity;
 }
 
-std::vector<uint64_t> const& Building::get_colonial_points() const {
+std::vector<fixed_point_t> const& Building::get_colonial_points() const {
 	return colonial_points;
 }
 
@@ -184,52 +184,72 @@ bool BuildingManager::add_building_type(std::string_view identifier) {
 	return building_types.add_item({ identifier });
 }
 
-bool BuildingManager::add_building(std::string_view identifier, ARGS) {
+bool BuildingManager::add_building(std::string_view identifier, BuildingType const* type, ARGS) {
 	if (identifier.empty()) {
 		Logger::error("Invalid building identifier - empty!");
 		return false;
 	}
+	if (type == nullptr) {
+		Logger::error("Invalid building type for ", identifier, ": null");
+		return false;
+	}
 
 	return buildings.add_item({
-		identifier, type, on_completion, completion_size, max_level, goods_cost, cost, build_time, visibility, on_map, default_enabled,
-		production_type, pop_build_factory, strategic_factory, advanced_factory, fort_level, naval_capacity, colonial_points, in_province, one_per_state, 
+		identifier, *type, on_completion, completion_size, max_level, goods_cost, cost, build_time, visibility, on_map, default_enabled,
+		production_type, pop_build_factory, strategic_factory, advanced_factory, fort_level, naval_capacity, colonial_points, in_province, one_per_state,
 		colonial_range, infrastructure, movement_cost, local_ship_build, spawn_railway_track, sail, steam, capital, port, std::move(modifiers)
 	});
 }
 
 bool BuildingManager::load_buildings_file(GoodManager const& good_manager, ProductionTypeManager const& production_type_manager, ModifierManager const& modifier_manager, ast::NodeCPtr root) {
-	bool ret = expect_dictionary([this, &good_manager, &production_type_manager, &modifier_manager](std::string_view key, ast::NodeCPtr value) -> bool {
-		std::string_view type, on_completion = "", production_type;
+	bool ret = expect_dictionary_reserve_length(buildings, [this](std::string_view, ast::NodeCPtr value) -> bool {
+		return expect_key("type", expect_identifier(
+			[this](std::string_view identifier) -> bool {
+				if (!building_types.has_identifier(identifier)) {
+					return building_types.add_item({ identifier });
+				}
+				return true;
+			}
+		))(value);
+	})(root);
+	lock_building_types();
+
+	ret &= expect_dictionary([this, &good_manager, &production_type_manager, &modifier_manager](std::string_view key, ast::NodeCPtr value) -> bool {
+		BuildingType const* type = nullptr;
+		ProductionType const* production_type = nullptr;
+		std::string_view on_completion;
 		fixed_point_t completion_size = 0, cost = 0, infrastructure = 0, movement_cost = 0, colonial_range = 0, local_ship_build = 0;
-		Building::level_t max_level, fort_level = 0;
-		std::map<const Good*, fixed_point_t> goods_cost;
-		uint32_t build_days;
-		bool visibility, on_map, default_enabled, pop_build_factory, strategic_factory, advanced_factory, in_province, one_per_state, spawn_railway_track, sail, steam, capital, port;
-		default_enabled = pop_build_factory = strategic_factory = advanced_factory = in_province = one_per_state = spawn_railway_track = sail = steam = capital = port = false;
+		Building::level_t max_level = 0, fort_level = 0;
+		std::map<Good const*, fixed_point_t> goods_cost;
+		Timespan build_time;
+		bool visibility = false, on_map = false, default_enabled = false, pop_build_factory = false, strategic_factory = false, advanced_factory = false;
+		bool in_province = false, one_per_state = false, spawn_railway_track = false, sail = false, steam = false, capital = false, port = false;
 		uint64_t naval_capacity = 0;
-		std::vector<uint64_t> colonial_points;
+		std::vector<fixed_point_t> colonial_points;
 		ModifierValue modifiers;
-		
+
 		bool ret = modifier_manager.expect_modifier_value_and_keys(move_variable_callback(modifiers),
-			"type", ONE_EXACTLY, expect_identifier(assign_variable_callback(type)),
+			"type", ONE_EXACTLY, expect_building_type_identifier(assign_variable_callback_pointer(type)),
 			"on_completion", ZERO_OR_ONE, expect_identifier(assign_variable_callback(on_completion)),
 			"completion_size", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(completion_size)),
-			"max_level", ONE_EXACTLY, expect_uint(assign_variable_callback(max_level)),
-			"goods_cost", ONE_EXACTLY, good_manager.expect_good_decimal_map(assign_variable_callback(goods_cost)),
+			"max_level", ONE_EXACTLY, expect_uint(assign_variable_callback_uint(max_level)),
+			"goods_cost", ONE_EXACTLY, good_manager.expect_good_decimal_map(move_variable_callback(goods_cost)),
 			"cost", ZERO_OR_MORE, expect_fixed_point(assign_variable_callback(cost)),
-			"time", ONE_EXACTLY, expect_uint(assign_variable_callback_uint("building build time", build_days)),
+			"time", ONE_EXACTLY, expect_timespan(assign_variable_callback(build_time)),
 			"visibility", ONE_EXACTLY, expect_bool(assign_variable_callback(visibility)),
 			"onmap", ONE_EXACTLY, expect_bool(assign_variable_callback(on_map)),
 			"default_enabled", ZERO_OR_ONE, expect_bool(assign_variable_callback(default_enabled)),
-			"production_type", ZERO_OR_ONE, expect_identifier(assign_variable_callback(production_type)),
+			"production_type", ZERO_OR_ONE, production_type_manager.expect_production_type_identifier(assign_variable_callback_pointer(production_type)),
 			"pop_build_factory", ZERO_OR_ONE, expect_bool(assign_variable_callback(pop_build_factory)),
 			"strategic_factory", ZERO_OR_ONE, expect_bool(assign_variable_callback(strategic_factory)),
 			"advanced_factory", ZERO_OR_ONE, expect_bool(assign_variable_callback(advanced_factory)),
-			"fort_level", ZERO_OR_ONE, expect_uint(assign_variable_callback_uint("building fort level", fort_level)),
-			"naval_capacity", ZERO_OR_ONE, expect_uint(assign_variable_callback_uint("building naval capacity", naval_capacity)),
-			"colonial_points", ZERO_OR_ONE, expect_list(expect_uint([&colonial_points](uint64_t points) -> bool {
-				return colonial_points.emplace_back(points);
-			})), "province", ZERO_OR_ONE, expect_bool(assign_variable_callback(in_province)),
+			"fort_level", ZERO_OR_ONE, expect_uint(assign_variable_callback_uint(fort_level)),
+			"naval_capacity", ZERO_OR_ONE, expect_uint(assign_variable_callback_uint(naval_capacity)),
+			"colonial_points", ZERO_OR_ONE, expect_list(expect_fixed_point([&colonial_points](fixed_point_t points) -> bool {
+				colonial_points.push_back(points);
+				return true;
+			})),
+			"province", ZERO_OR_ONE, expect_bool(assign_variable_callback(in_province)),
 			"one_per_state", ZERO_OR_ONE, expect_bool(assign_variable_callback(one_per_state)),
 			"colonial_range", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(colonial_range)),
 			"infrastructure", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(infrastructure)),
@@ -242,25 +262,14 @@ bool BuildingManager::load_buildings_file(GoodManager const& good_manager, Produ
 			"port", ZERO_OR_ONE, expect_bool(assign_variable_callback(port))
 		)(value);
 
-		BuildingType const* type_ref = building_types.get_item_by_identifier(type);
-		if (type_ref == nullptr) {
-			building_types.add_item({ type });
-			type_ref = building_types.get_item_by_identifier(type);
-		}
-
-		Timespan build_time = Timespan(build_days);
-
-		ProductionType const* production_type_ref = production_type_manager.get_production_type_by_identifier(production_type);
-
 		ret &= add_building(
-			key, *type_ref, on_completion, completion_size, max_level, goods_cost, cost, build_time, visibility, on_map, default_enabled,
-			production_type_ref, pop_build_factory, strategic_factory, advanced_factory, fort_level, naval_capacity, colonial_points, in_province,
+			key, type, on_completion, completion_size, max_level, goods_cost, cost, build_time, visibility, on_map, default_enabled,
+			production_type, pop_build_factory, strategic_factory, advanced_factory, fort_level, naval_capacity, colonial_points, in_province,
 			one_per_state, colonial_range, infrastructure, movement_cost, local_ship_build, spawn_railway_track, sail, steam, capital, port, std::move(modifiers)
 		);
 
 		return ret;
 	})(root);
-	lock_building_types();
 	lock_buildings();
 
 	return ret;
diff --git a/src/openvic-simulation/map/Building.hpp b/src/openvic-simulation/map/Building.hpp
index 1c2ded0..9b445bf 100644
--- a/src/openvic-simulation/map/Building.hpp
+++ b/src/openvic-simulation/map/Building.hpp
@@ -7,10 +7,10 @@
 #include "openvic-simulation/economy/ProductionType.hpp"
 #include "openvic-simulation/Modifier.hpp"
 
-#define ARGS BuildingType const& type, std::string_view on_completion, fixed_point_t completion_size, level_t max_level, \
-				std::map<const Good*, fixed_point_t> goods_cost, fixed_point_t cost, Timespan build_time, bool visibility, bool on_map, bool default_enabled, \
+#define ARGS std::string_view on_completion, fixed_point_t completion_size, level_t max_level, \
+				std::map<Good const*, fixed_point_t> goods_cost, fixed_point_t cost, Timespan build_time, bool visibility, bool on_map, bool default_enabled, \
 				ProductionType const* production_type, bool pop_build_factory, bool strategic_factory, bool advanced_factory, level_t fort_level, \
-				uint64_t naval_capacity, std::vector<uint64_t> colonial_points, bool in_province, bool one_per_state, fixed_point_t colonial_range, \
+				uint64_t naval_capacity, std::vector<fixed_point_t> colonial_points, bool in_province, bool one_per_state, fixed_point_t colonial_range, \
 				fixed_point_t infrastructure, fixed_point_t movement_cost, fixed_point_t local_ship_build, bool spawn_railway_track, bool sail, bool steam, \
 				bool capital, bool port, ModifierValue&& modifiers
 
@@ -27,19 +27,19 @@ namespace OpenVic {
 	struct Building : HasIdentifier, ModifierValue {
 		friend struct BuildingManager;
 
-		using level_t = uint8_t;
+		using level_t = int8_t;
 
 	private:
 		BuildingType const& type;
-		const std::string_view on_completion; //probably sound played on completion
+		const std::string on_completion; //probably sound played on completion
 		const fixed_point_t completion_size;
 		const level_t max_level;
-		const std::map<const Good*, fixed_point_t> goods_cost;
+		const std::map<Good const*, fixed_point_t> goods_cost;
 		const fixed_point_t cost;
 		const Timespan build_time; //time
 		const bool visibility;
 		const bool on_map; //onmap
-		
+
 		const bool default_enabled;
 		ProductionType const* production_type;
 		const bool pop_build_factory;
@@ -49,7 +49,7 @@ namespace OpenVic {
 		const level_t fort_level; //probably the step-per-level
 
 		const uint64_t naval_capacity;
-		const std::vector<uint64_t> colonial_points;
+		const std::vector<fixed_point_t> colonial_points;
 		const bool in_province; //province
 		const bool one_per_state;
 		const fixed_point_t colonial_range;
@@ -64,16 +64,16 @@ namespace OpenVic {
 		const bool capital; //only in naval base
 		const bool port; //only in naval base
 
-		Building(std::string_view identifier, ARGS);
+		Building(std::string_view identifier, BuildingType const& type, ARGS);
 
 	public:
 		Building(Building&&) = default;
 
 		BuildingType const& get_type() const;
-		std::string_view get_on_completion() const;
+		std::string const& get_on_completion() const;
 		fixed_point_t get_completion_size() const;
 		level_t get_max_level() const;
-		std::map<const Good*, fixed_point_t> const& get_goods_cost() const;
+		std::map<Good const*, fixed_point_t> const& get_goods_cost() const;
 		fixed_point_t get_cost() const;
 		Timespan get_build_time() const;
 		bool has_visibility() const;
@@ -84,15 +84,15 @@ namespace OpenVic {
 		bool is_pop_built_factory() const;
 		bool is_strategic_factory() const;
 		bool is_advanced_factory() const;
-		
+
 		level_t get_fort_level() const;
 
 		uint64_t get_naval_capacity() const;
-		std::vector<uint64_t> const& get_colonial_points() const;
+		std::vector<fixed_point_t> const& get_colonial_points() const;
 		bool is_in_province() const;
 		bool is_one_per_state() const;
 		fixed_point_t get_colonial_range() const;
-		
+
 		fixed_point_t get_infrastructure() const;
 		fixed_point_t get_movement_cost() const;
 		fixed_point_t get_local_ship_build() const;
@@ -119,7 +119,7 @@ namespace OpenVic {
 	struct BuildingInstance : HasIdentifier { //used in the actual game
 		friend struct BuildingManager;
 		using level_t = Building::level_t;
-	
+
 	private:
 		Building const& building;
 
@@ -164,7 +164,7 @@ namespace OpenVic {
 		bool add_building_type(std::string_view identifier);
 		IDENTIFIER_REGISTRY_ACCESSORS(BuildingType, building_type)
 
-		bool add_building(std::string_view identifier, ARGS);
+		bool add_building(std::string_view identifier, BuildingType const* type, ARGS);
 		IDENTIFIER_REGISTRY_ACCESSORS(Building, building)
 
 		bool load_buildings_file(GoodManager const& good_manager, ProductionTypeManager const& production_type_manager, ModifierManager const& modifier_manager, ast::NodeCPtr root);
diff --git a/src/openvic-simulation/map/TerrainType.cpp b/src/openvic-simulation/map/TerrainType.cpp
index 24cfb7d..2438df6 100644
--- a/src/openvic-simulation/map/TerrainType.cpp
+++ b/src/openvic-simulation/map/TerrainType.cpp
@@ -117,7 +117,7 @@ bool TerrainTypeManager::_load_terrain_type_mapping(std::string_view mapping_key
 				return false;
 			}
 		)),
-		"priority", ZERO_OR_ONE, expect_uint(assign_variable_callback_uint("terrain type mapping priority", priority)),
+		"priority", ZERO_OR_ONE, expect_uint(assign_variable_callback_uint(priority)),
 		"has_texture", ZERO_OR_ONE, expect_bool(assign_variable_callback(has_texture))
 	)(mapping_value);
 	if (has_texture) {
@@ -153,7 +153,7 @@ bool TerrainTypeManager::load_terrain_types(ModifierManager const& modifier_mana
 			if (key == "terrain") {
 				if (!terrain) {
 					terrain = true;
-					return expect_uint(assign_variable_callback_uint("terrain texture limit", terrain_texture_limit))(value);
+					return expect_uint(assign_variable_callback_uint(terrain_texture_limit))(value);
 				} else {
 					Logger::error("Duplicate terrain key!");
 					return false;