Data types, defaults, callback cleanup

2 files changed, 71 insertions, 64 deletions

diff --git a/src/openvic-simulation/units/Unit.cpp b/src/openvic-simulation/units/Unit.cpp
index 16719e6..8d519f1 100644
--- a/src/openvic-simulation/units/Unit.cpp
+++ b/src/openvic-simulation/units/Unit.cpp
@@ -1,6 +1,6 @@
 #include "Unit.hpp"
 
-#define UNIT_ARGS icon, sprite, active, type, floating_flag, priority, max_strength, \
+#define UNIT_ARGS icon, sprite, active, unit_type, floating_flag, priority, max_strength, \
 					default_organisation, maximum_speed, weighted_value, build_time, build_cost, supply_consumption, \
 					supply_cost
 #define LAND_ARGS reconnaissance, attack, defence, discipline, support, maneuver, siege
@@ -10,8 +10,8 @@
 using namespace OpenVic;
 using namespace OpenVic::NodeTools;
 
-Unit::Unit(std::string_view identifier, std::string_view category, UNIT_PARAMS) : HasIdentifier { identifier },
-	icon { icon }, category { category }, sprite { sprite }, active { active }, type { type },
+Unit::Unit(std::string_view identifier, type_t type, UNIT_PARAMS) : HasIdentifier { identifier },
+	icon { icon }, type { type }, sprite { sprite }, active { active }, unit_type { unit_type },
 	floating_flag { floating_flag }, priority { priority }, max_strength { max_strength },
 	default_organisation { default_organisation }, maximum_speed { maximum_speed }, weighted_value { weighted_value },
 	build_time { build_time }, build_cost { build_cost }, supply_consumption { supply_consumption }, supply_cost { supply_cost } {}
@@ -20,7 +20,11 @@ Unit::icon_t Unit::get_icon() const {
 	return icon;
 }
 
-Unit::sprite_t Unit::get_sprite() const {
+Unit::type_t Unit::get_type() const {
+	return type;
+}
+
+std::string const& Unit::get_sprite() const {
 	return sprite;
 }
 
@@ -28,8 +32,8 @@ bool Unit::is_active() const {
 	return active;
 }
 
-std::string_view Unit::get_type() const {
-	return type;
+std::string const& Unit::get_unit_type() const {
+	return unit_type;
 }
 
 bool Unit::has_floating_flag() const {
@@ -60,7 +64,7 @@ fixed_point_t Unit::get_weighted_value() const {
 	return weighted_value;
 }
 
-std::map<const Good*, fixed_point_t> const& Unit::get_build_cost() const {
+std::map<Good const*, fixed_point_t> const& Unit::get_build_cost() const {
 	return build_cost;
 }
 
@@ -68,11 +72,11 @@ fixed_point_t Unit::get_supply_consumption() const {
 	return supply_consumption;
 }
 
-std::map<const Good*, fixed_point_t> const& Unit::get_supply_cost() const {
+std::map<Good const*, fixed_point_t> const& Unit::get_supply_cost() const {
 	return supply_cost;
 }
 
-LandUnit::LandUnit(std::string_view identifier, UNIT_PARAMS, LAND_PARAMS) : Unit { identifier, "land", UNIT_ARGS },
+LandUnit::LandUnit(std::string_view identifier, UNIT_PARAMS, LAND_PARAMS) : Unit { identifier, type_t::LAND, UNIT_ARGS },
 	reconnaissance { reconnaissance }, attack { attack }, defence { defence }, discipline { discipline }, support { support },
 	maneuver { maneuver }, siege { siege } {}
 
@@ -104,7 +108,7 @@ fixed_point_t LandUnit::get_siege() const {
 	return siege;
 }
 
-NavalUnit::NavalUnit(std::string_view identifier, UNIT_PARAMS, NAVY_PARAMS) : Unit { identifier, "naval", UNIT_ARGS },
+NavalUnit::NavalUnit(std::string_view identifier, UNIT_PARAMS, NAVY_PARAMS) : Unit { identifier, type_t::NAVAL, UNIT_ARGS },
 	naval_icon { naval_icon }, sail { sail }, transport { transport }, capital { capital }, move_sound { move_sound },
 	select_sound { select_sound }, colonial_points { colonial_points }, build_overseas { build_overseas },
 	min_port_level { min_port_level }, limit_per_port { limit_per_port }, supply_consumption_score { supply_consumption_score },
@@ -122,11 +126,11 @@ bool NavalUnit::is_transport() const {
 	return transport;
 }
 
-NavalUnit::sound_t NavalUnit::get_move_sound() const {
+std::string const& NavalUnit::get_move_sound() const {
 	return move_sound;
 }
 
-NavalUnit::sound_t NavalUnit::get_select_sound() const {
+std::string const& NavalUnit::get_select_sound() const {
 	return select_sound;
 }
 
@@ -183,7 +187,7 @@ bool UnitManager::_check_shared_parameters(std::string_view identifier, UNIT_PAR
 		return false;
 	}
 
-	if (type.empty()) {
+	if (unit_type.empty()) {
 		Logger::error("Invalid unit type - empty!");
 		return false;
 	}
@@ -212,38 +216,36 @@ bool UnitManager::add_naval_unit(std::string_view identifier, UNIT_PARAMS, NAVY_
 }
 
 bool UnitManager::load_unit_file(GoodManager const& good_manager, ast::NodeCPtr root) {
-	return NodeTools::expect_dictionary([this, &good_manager](std::string_view key, ast::NodeCPtr value) -> bool {
-		Unit::icon_t icon;
-		std::string_view category, type;
-		Unit::sprite_t sprite;
-		bool active = true, floating_flag;
-		uint32_t priority, build_time_days;
-		fixed_point_t maximum_speed, max_strength, default_organisation, weighted_value, supply_consumption;
-		std::map<const Good*, fixed_point_t> build_cost, supply_cost;
+	return expect_dictionary([this, &good_manager](std::string_view key, ast::NodeCPtr value) -> bool {
+		Unit::icon_t icon = 0;
+		std::string_view type, unit_type, sprite;
+		bool active = true, floating_flag = false;
+		uint32_t priority = 0;
+		Timespan build_time;
+		fixed_point_t maximum_speed = 0, max_strength = 0, default_organisation = 0, weighted_value = 0, supply_consumption = 0;
+		std::map<Good const*, fixed_point_t> build_cost, supply_cost;
 
 		//shared
 		bool ret = expect_dictionary_keys(ALLOW_OTHER_KEYS,
-			"icon", ONE_EXACTLY, expect_uint(assign_variable_callback_uint("unit icon", icon)),
-			"type", ONE_EXACTLY, expect_identifier(assign_variable_callback(category)),
+			"icon", ONE_EXACTLY, expect_uint(assign_variable_callback_uint(icon)),
+			"type", ONE_EXACTLY, expect_identifier(assign_variable_callback(type)),
 			"sprite", ONE_EXACTLY, expect_identifier(assign_variable_callback(sprite)),
 			"active", ZERO_OR_ONE, expect_bool(assign_variable_callback(active)),
-			"unit_type", ONE_EXACTLY, expect_identifier(assign_variable_callback(type)),
+			"unit_type", ONE_EXACTLY, expect_identifier(assign_variable_callback(unit_type)),
 			"floating_flag", ONE_EXACTLY, expect_bool(assign_variable_callback(floating_flag)),
-			"priority", ONE_EXACTLY, expect_uint(assign_variable_callback_uint("unit priority", priority)),
+			"priority", ONE_EXACTLY, expect_uint(assign_variable_callback_uint(priority)),
 			"max_strength", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(max_strength)),
 			"default_organisation", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(default_organisation)),
 			"maximum_speed", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(maximum_speed)),
 			"weighted_value", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(weighted_value)),
-			"build_time", ONE_EXACTLY, expect_uint(assign_variable_callback_uint("unit build time", build_time_days)),
-			"build_cost", ONE_EXACTLY, good_manager.expect_good_decimal_map(assign_variable_callback(build_cost)),
-			"supply_consumption", ONE_EXACTLY, expect_fixed_point(move_variable_callback(supply_consumption)),
+			"build_time", ONE_EXACTLY, expect_timespan(assign_variable_callback(build_time)),
+			"build_cost", ONE_EXACTLY, good_manager.expect_good_decimal_map(move_variable_callback(build_cost)),
+			"supply_consumption", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(supply_consumption)),
 			"supply_cost", ONE_EXACTLY, good_manager.expect_good_decimal_map(move_variable_callback(supply_cost))
 		)(value);
 
-		Timespan build_time = { build_time_days };
-
-		if (category == "land") {
-			fixed_point_t reconnaissance, attack, defence, discipline, support, maneuver, siege;
+		if (type == "land") {
+			fixed_point_t reconnaissance = 0, attack = 0, defence = 0, discipline = 0, support = 0, maneuver = 0, siege = 0;
 
 			ret &= expect_dictionary_keys(ALLOW_OTHER_KEYS,
 				"reconnaissance", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(reconnaissance)),
@@ -258,16 +260,16 @@ bool UnitManager::load_unit_file(GoodManager const& good_manager, ast::NodeCPtr
 			ret &= add_land_unit(key, UNIT_ARGS, LAND_ARGS);
 
 			return ret;
-		} else if (category == "naval") {
-			Unit::icon_t naval_icon;
+		} else if (type == "naval") {
+			Unit::icon_t naval_icon = 0;
 			bool sail = false, transport = false, capital = false, build_overseas = false;
-			Unit::sound_t move_sound, select_sound; //TODO defaults for both
-			uint32_t min_port_level;
-			int32_t limit_per_port;
-			fixed_point_t fire_range, evasion, supply_consumption_score, hull, gun_power, colonial_points = 0, torpedo_attack = 0;
+			std::string_view move_sound, select_sound; //TODO defaults for both
+			uint32_t min_port_level = 0;
+			int32_t limit_per_port = 0;
+			fixed_point_t fire_range = 0, evasion = 0, supply_consumption_score = 0, hull = 0, gun_power = 0, colonial_points = 0, torpedo_attack = 0;
 
 			ret &= expect_dictionary_keys(ALLOW_OTHER_KEYS,
-				"naval_icon", ONE_EXACTLY, expect_uint(assign_variable_callback_uint("unit naval icon", naval_icon)),
+				"naval_icon", ONE_EXACTLY, expect_uint(assign_variable_callback_uint(naval_icon)),
 				"sail", ZERO_OR_ONE, expect_bool(assign_variable_callback(sail)),
 				"transport", ZERO_OR_ONE, expect_bool(assign_variable_callback(transport)),
 				"capital", ZERO_OR_ONE, expect_bool(assign_variable_callback(capital)),
@@ -275,8 +277,8 @@ bool UnitManager::load_unit_file(GoodManager const& good_manager, ast::NodeCPtr
 				"select_sound", ZERO_OR_ONE, expect_identifier(assign_variable_callback(select_sound)),
 				"colonial_points", ZERO_OR_ONE, expect_fixed_point(assign_variable_callback(colonial_points)),
 				"can_build_overseas", ZERO_OR_ONE, expect_bool(assign_variable_callback(build_overseas)),
-				"min_port_level", ONE_EXACTLY, expect_uint(assign_variable_callback_uint("unit min port level", min_port_level)),
-				"limit_per_port", ONE_EXACTLY, expect_int(assign_variable_callback_int("unit limit per port", limit_per_port)),
+				"min_port_level", ONE_EXACTLY, expect_uint(assign_variable_callback_uint(min_port_level)),
+				"limit_per_port", ONE_EXACTLY, expect_int(assign_variable_callback_int(limit_per_port)),
 				"supply_consumption_score", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(supply_consumption_score)),
 				"hull", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(hull)),
 				"gun_power", ONE_EXACTLY, expect_fixed_point(assign_variable_callback(gun_power)),
@@ -288,6 +290,9 @@ bool UnitManager::load_unit_file(GoodManager const& good_manager, ast::NodeCPtr
 			ret &= add_naval_unit(key, UNIT_ARGS, NAVY_ARGS);
 
 			return ret;
-		} else return false;
+		} else {
+			Logger::error("Invalid type for unit ", key, ": ", type);
+			return false;
+		}
 	})(root);
 }
\ No newline at end of file
diff --git a/src/openvic-simulation/units/Unit.hpp b/src/openvic-simulation/units/Unit.hpp
index 979ae70..3555cce 100644
--- a/src/openvic-simulation/units/Unit.hpp
+++ b/src/openvic-simulation/units/Unit.hpp
@@ -9,30 +9,32 @@
 #include "openvic-simulation/economy/Good.hpp"
 #include "openvic-simulation/types/Date.hpp"
 
-#define UNIT_PARAMS Unit::icon_t icon, Unit::sprite_t sprite, bool active, std::string_view type, \
+#define UNIT_PARAMS Unit::icon_t icon, std::string_view sprite, bool active, std::string_view unit_type, \
 					bool floating_flag, uint32_t priority, fixed_point_t max_strength, fixed_point_t default_organisation, \
 					fixed_point_t maximum_speed, fixed_point_t weighted_value, Timespan build_time, \
-					std::map<const Good*, fixed_point_t> build_cost, fixed_point_t supply_consumption, \
-					std::map<const Good*, fixed_point_t> supply_cost
+					std::map<Good const*, fixed_point_t> build_cost, fixed_point_t supply_consumption, \
+					std::map<Good const*, fixed_point_t> supply_cost
 #define LAND_PARAMS fixed_point_t reconnaissance, fixed_point_t attack, fixed_point_t defence, fixed_point_t discipline, \
 					fixed_point_t support, fixed_point_t maneuver, fixed_point_t siege
-#define NAVY_PARAMS Unit::icon_t naval_icon, bool sail, bool transport, bool capital, Unit::sound_t move_sound, \
-					Unit::sound_t select_sound, fixed_point_t colonial_points, bool build_overseas, uint32_t min_port_level, \
+#define NAVY_PARAMS Unit::icon_t naval_icon, bool sail, bool transport, bool capital, std::string_view move_sound, \
+					std::string_view select_sound, fixed_point_t colonial_points, bool build_overseas, uint32_t min_port_level, \
 					int32_t limit_per_port, fixed_point_t supply_consumption_score, fixed_point_t hull, fixed_point_t gun_power, \
 					fixed_point_t fire_range, fixed_point_t evasion, fixed_point_t torpedo_attack
 
 namespace OpenVic {
 	struct Unit : HasIdentifier {
 		using icon_t = uint32_t;
-		using sprite_t = std::string_view;
-		using sound_t = std::string_view;
+
+		enum struct type_t {
+			LAND, NAVAL
+		};
 
 	private:
-		const std::string_view category;
+		const type_t type;
 		const icon_t icon;
-		const sprite_t sprite;
+		const std::string sprite;
 		const bool active;
-		const std::string_view type;
+		const std::string unit_type;
 		const bool floating_flag;
 
 		const uint32_t priority;
@@ -42,21 +44,21 @@ namespace OpenVic {
 		const fixed_point_t weighted_value;
 
 		const Timespan build_time;
-		const std::map<const Good*, fixed_point_t> build_cost;
+		const std::map<Good const*, fixed_point_t> build_cost;
 		const fixed_point_t supply_consumption;
-		const std::map<const Good*, fixed_point_t> supply_cost;
+		const std::map<Good const*, fixed_point_t> supply_cost;
 
 	protected:
-		Unit(std::string_view identifier, std::string_view category, UNIT_PARAMS);
+		Unit(std::string_view identifier, type_t type, UNIT_PARAMS);
 
 	public:
 		Unit(Unit&&) = default;
 
 		icon_t get_icon() const;
-		std::string_view get_category() const;
-		sprite_t get_sprite() const;
+		type_t get_type() const;
+		std::string const& get_sprite() const;
 		bool is_active() const;
-		std::string_view get_type() const;
+		std::string const& get_unit_type() const;
 		bool has_floating_flag() const;
 
 		uint32_t get_priority() const;
@@ -66,9 +68,9 @@ namespace OpenVic {
 		fixed_point_t get_weighted_value() const;
 
 		Timespan get_build_time() const;
-		std::map<const Good*, fixed_point_t> const& get_build_cost() const;
+		std::map<Good const*, fixed_point_t> const& get_build_cost() const;
 		fixed_point_t get_supply_consumption() const;
-		std::map<const Good*, fixed_point_t> const& get_supply_cost() const;
+		std::map<Good const*, fixed_point_t> const& get_supply_cost() const;
 	};
 
 	struct LandUnit : Unit {
@@ -105,8 +107,8 @@ namespace OpenVic {
 		const bool sail;
 		const bool transport;
 		const bool capital;
-		const sound_t move_sound;
-		const sound_t select_sound;
+		const std::string move_sound;
+		const std::string select_sound;
 		const fixed_point_t colonial_points;
 		const bool build_overseas;
 		const uint32_t min_port_level;
@@ -128,8 +130,8 @@ namespace OpenVic {
 		bool can_sail() const;
 		bool is_transport() const;
 		bool is_capital() const;
-		sound_t get_move_sound() const;
-		sound_t get_select_sound() const;
+		std::string const& get_move_sound() const;
+		std::string const& get_select_sound() const;
 		fixed_point_t get_colonial_points() const;
 		bool can_build_overseas() const;
 		uint32_t get_min_port_level() const;