Merge pull request #711 from OpenVicProject/refactor_politics_reference_wrapper

Refactor politics and leftovers to use reference_wrapper

Author: wvpm

src/headless/main.cpp

@@ -260,32 +260,31 @@ static bool run_headless(fs::path const& root, memory::vector<memory::string>& m
 	SPDLOG_INFO("===== Ranking system test... =====");
 	if (game_manager.get_instance_manager()) {
 		const auto print_ranking_list = [ //
-		](std::string_view title, OpenVic::forwardable_span<CountryInstance* const> countries) -> void {
+		](std::string_view title, OpenVic::forwardable_span<const std::reference_wrapper<CountryInstance>> countries) -> void {
 			memory::string countries_str;
-			for (CountryInstance* country : countries) {
+			for (CountryInstance& country : countries) {
 				countries_str += fmt::format(
 					"\n\t{} - Total #{} ({}), Prestige #{} ({}), Industry #{} ({}), Military #{} ({})", //
-					*country, //
-					country->get_total_rank(), country->total_score.get_untracked().to_string(1), //
-					country->get_prestige_rank(), country->get_prestige_untracked().to_string(1),
-					country->get_industrial_rank(), country->get_industrial_power_untracked().to_string(1),
-					country->get_military_rank(), country->military_power.get_untracked().to_string(1)
+					country, //
+					country.get_total_rank(), country.total_score.get_untracked().to_string(1), //
+					country.get_prestige_rank(), country.get_prestige_untracked().to_string(1),
+					country.get_industrial_rank(), country.get_industrial_power_untracked().to_string(1),
+					country.get_military_rank(), country.military_power.get_untracked().to_string(1)
 				);
 			}
 			SPDLOG_INFO("{}:{}", title, countries_str);
 		};
 
-		CountryInstanceManager const& country_instance_manager =
-			game_manager.get_instance_manager()->get_country_instance_manager();
+		CountryInstanceManager const& country_instance_manager = game_manager.get_instance_manager()->get_country_instance_manager();
 
-		OpenVic::forwardable_span<CountryInstance* const> great_powers = country_instance_manager.get_great_powers();
+		OpenVic::forwardable_span<const std::reference_wrapper<CountryInstance>> great_powers = country_instance_manager.get_great_powers();
 		print_ranking_list("Great Powers", great_powers);
 		print_ranking_list("Secondary Powers", country_instance_manager.get_secondary_powers());
 		print_ranking_list("All countries", country_instance_manager.get_total_ranking());
 
 		SPDLOG_INFO("===== RGO test... =====");
 		for (size_t i = 0; i < std::min<size_t>(3, great_powers.size()); ++i) {
-			CountryInstance const& great_power = *great_powers[i];
+			CountryInstance const& great_power = great_powers[i];
 			ProvinceInstance const* const capital_province = great_power.get_capital();
 			if (capital_province == nullptr) {
 				spdlog::warn_s("{} has no capital ProvinceInstance set.", great_power);

src/openvic-simulation/core/template/Concepts.hpp

@@ -4,7 +4,6 @@
 #include <cstddef>
 #include <memory>
 #include <string_view>
-#include <type_traits>
 
 #include <type_safe/strong_typedef.hpp>
 
@@ -235,4 +234,4 @@ namespace OpenVic {
 	concept strict_regular_invocable_r = std::regular_invocable<F, Args...> && requires(F f, Args&&... args) {
 		{ f(static_cast<Args>(args)...) } -> std::same_as<Return>;
 	};
-}
+}

src/openvic-simulation/core/template/Functional.hpp

@@ -0,0 +1,49 @@
+#pragma once
+
+#include <functional>
+
+#include <concepts/type_traits.hpp>
+
+// bug fix for std::hash & std::equal_to to work with std::reference_wrapper
+namespace std {
+	template <typename T>
+	struct hash<std::reference_wrapper<T>> {
+		using BaseType = std::remove_cv_t<T>;
+
+		[[nodiscard]] size_t operator()(T const& val) const noexcept {
+			return std::hash<BaseType>{}(val);
+		}
+	};
+}
+
+// bug fix for concepts/type_traits.hpp to work with std::reference_wrapper
+namespace concepts {
+	namespace detail {
+		template<typename T>
+		constexpr bool _is_ref_wrapper = false;
+		
+		template<typename T>
+		constexpr bool _is_ref_wrapper<std::reference_wrapper<T>> = true;
+		
+		template<typename R, typename T, typename RQual, typename TQual>
+		concept _ref_wrap_common_reference_with = _is_ref_wrapper<R>
+			&& requires { typename std::common_reference_t<typename R::type&, TQual>; }
+			&& std::convertible_to<RQual, std::common_reference_t<typename R::type&, TQual>>;
+	}
+
+	template<typename R, typename T, template<typename> class RQual, template<typename> class TQual>
+	requires detail::_ref_wrap_common_reference_with<R, T, RQual<R>, TQual<T>>
+	&& (!detail::_ref_wrap_common_reference_with<T, R, TQual<T>, RQual<R>>)
+	struct basic_common_reference<R, T, RQual, TQual>
+	{
+		using type = std::common_reference_t<typename R::type&, TQual<T>>;
+	};
+
+	template<typename T, typename R, template<typename> class TQual, template<typename> class RQual>
+	requires detail::_ref_wrap_common_reference_with<R, T, RQual<R>, TQual<T>>
+	&& (!detail::_ref_wrap_common_reference_with<T, R, TQual<T>, RQual<R>>)
+	struct basic_common_reference<T, R, TQual, RQual>
+	{
+		using type = std::common_reference_t<typename R::type&, TQual<T>>;
+	};
+}

src/openvic-simulation/country/CountryDefinition.cpp

@@ -159,7 +159,7 @@ node_callback_t CountryDefinitionManager::load_country_party(
 
 						return politics_manager.get_issue_manager().expect_party_policy_identifier(
 							[&party_policy_group, &policy](PartyPolicy const& party_policy) -> bool {
-								if (&party_policy.issue_group == &party_policy_group) {
+								if (&party_policy.group == &party_policy_group) {
 									policy = &party_policy;
 									return true;
 								}
@@ -168,7 +168,7 @@ node_callback_t CountryDefinitionManager::load_country_party(
 								spdlog::warn_s(
 									"Invalid party policy \"{}\", group is \"{}\" when \"{}\" was expected.",
 									party_policy,
-									party_policy.issue_group,
+									party_policy.group,
 									party_policy_group
 								);
 								return true;

src/openvic-simulation/country/CountryInstance.cpp

@@ -61,51 +61,51 @@ static constexpr size_t DAYS_OF_BALANCE_HISTORY = 30;
 static constexpr colour_t ERROR_COLOUR = colour_t::from_integer(0xFF0000);
 
 CountryInstance::CountryInstance(
-	CountryDefinition const* new_country_definition,
-	SharedCountryValues* new_shared_country_values,
-	CountryInstanceDeps const* country_instance_deps
+	CountryDefinition const& new_country_definition,
+	SharedCountryValues& new_shared_country_values,
+	CountryInstanceDeps const& country_instance_deps
 ) : FlagStrings { "country" },
-	HasIndex { new_country_definition->index },
+	HasIndex { new_country_definition.index },
 	PopsAggregate {
-		country_instance_deps->stratas,
-		country_instance_deps->pop_types,
-		country_instance_deps->ideologies
+		country_instance_deps.stratas,
+		country_instance_deps.pop_types,
+		country_instance_deps.ideologies
 	},
 	/* Main attributes */
-	country_definition { *new_country_definition },
-	shared_country_values { *new_shared_country_values },
-
-	country_relations_manager { country_instance_deps->country_relations_manager },
-	game_rules_manager { country_instance_deps->game_rules_manager },
-	good_instance_manager { country_instance_deps->good_instance_manager },
-	market_instance { country_instance_deps->market_instance },
-	modifier_effect_cache { country_instance_deps->modifier_effect_cache },
-	unit_type_manager { country_instance_deps->unit_type_manager },
+	country_definition { new_country_definition },
+	shared_country_values { new_shared_country_values },
+
+	country_relations_manager { country_instance_deps.country_relations_manager },
+	game_rules_manager { country_instance_deps.game_rules_manager },
+	good_instance_manager { country_instance_deps.good_instance_manager },
+	market_instance { country_instance_deps.market_instance },
+	modifier_effect_cache { country_instance_deps.modifier_effect_cache },
+	unit_type_manager { country_instance_deps.unit_type_manager },
 
-	fallback_date_for_never_completing_research { country_instance_deps->fallback_date_for_never_completing_research },
-	country_defines { country_instance_deps->country_defines },
-	diplomacy_defines { country_instance_deps->diplomacy_defines },
-	economy_defines { country_instance_deps->economy_defines },
-	military_defines { country_instance_deps->military_defines },
+	fallback_date_for_never_completing_research { country_instance_deps.fallback_date_for_never_completing_research },
+	country_defines { country_instance_deps.country_defines },
+	diplomacy_defines { country_instance_deps.diplomacy_defines },
+	economy_defines { country_instance_deps.economy_defines },
+	military_defines { country_instance_deps.military_defines },
 
 	colour { ERROR_COLOUR },
 
 	/* Production */
-	building_type_unlock_levels { country_instance_deps->building_types },
+	building_type_unlock_levels { country_instance_deps.building_types },
 
 	/* Budget */
 	balance_history{DAYS_OF_BALANCE_HISTORY},
-	taxable_income_by_pop_type { country_instance_deps->pop_types },
+	taxable_income_by_pop_type { country_instance_deps.pop_types },
 	effective_tax_rate_by_strata {
-		country_instance_deps->stratas,
+		country_instance_deps.stratas,
 		[this](Strata const& strata)->auto {
 			return [this,&strata](DependencyTracker& tracker)->fixed_point_t {
 				return tax_efficiency.get(tracker) * tax_rate_slider_value_by_strata.at(strata).get_value(tracker);
 			};
 		}
 	},
 	administration_salary_base_by_pop_type{
-		country_instance_deps->pop_types,
+		country_instance_deps.pop_types,
 		[this](PopType const& pop_type)->auto {
 			return [this,&pop_type](DependencyTracker& tracker)->fixed_point_t {
 				return corruption_cost_multiplier.get(tracker)
@@ -115,7 +115,7 @@ CountryInstance::CountryInstance(
 		}
 	},
 	education_salary_base_by_pop_type{
-		country_instance_deps->pop_types,
+		country_instance_deps.pop_types,
 		[this](PopType const& pop_type)->auto {
 			return [this,&pop_type](DependencyTracker& tracker)->fixed_point_t {
 				return corruption_cost_multiplier.get(tracker)
@@ -125,7 +125,7 @@ CountryInstance::CountryInstance(
 		}
 	},
 	military_salary_base_by_pop_type{
-		country_instance_deps->pop_types,
+		country_instance_deps.pop_types,
 		[this](PopType const& pop_type)->auto {
 			return [this,&pop_type](DependencyTracker& tracker)->fixed_point_t {
 				return corruption_cost_multiplier.get(tracker)
@@ -135,7 +135,7 @@ CountryInstance::CountryInstance(
 		}
 	},
 	social_income_variant_base_by_pop_type{
-		country_instance_deps->pop_types,
+		country_instance_deps.pop_types,
 		[this](PopType const& pop_type)->auto {
 			return [this,&pop_type](DependencyTracker& tracker)->fixed_point_t {
 				return corruption_cost_multiplier.get(tracker)
@@ -144,26 +144,26 @@ CountryInstance::CountryInstance(
 			};
 		}
 	},
-	tax_rate_slider_value_by_strata { country_instance_deps->stratas },
+	tax_rate_slider_value_by_strata { country_instance_deps.stratas },
 
 	/* Technology */
-	technology_unlock_levels { country_instance_deps->technologies },
-	invention_unlock_levels { country_instance_deps->inventions },
+	technology_unlock_levels { country_instance_deps.technologies },
+	invention_unlock_levels { country_instance_deps.inventions },
 
 	/* Politics */
-	upper_house_proportion_by_ideology { country_instance_deps->ideologies },
-	reforms { country_instance_deps->reforms },
-	flag_overrides_by_government_type { country_instance_deps->government_types },
-	crime_unlock_levels { country_instance_deps->crimes },
+	upper_house_proportion_by_ideology { country_instance_deps.ideologies },
+	reforms { country_instance_deps.reforms },
+	flag_overrides_by_government_type { country_instance_deps.government_types },
+	crime_unlock_levels { country_instance_deps.crimes },
 
 	/* Trade */
-	goods_data { country_instance_deps->good_instances },
+	goods_data { country_instance_deps.good_instances },
 
 	/* Diplomacy */
 
 	/* Military */
-	regiment_type_unlock_levels { country_instance_deps->regiment_types },
-	ship_type_unlock_levels { country_instance_deps->ship_types },
+	regiment_type_unlock_levels { country_instance_deps.regiment_types },
+	ship_type_unlock_levels { country_instance_deps.ship_types },
 
 	/* DerivedState */
 	flag_government_type { [this](DependencyTracker& tracker)->GovernmentType const* {
@@ -247,7 +247,7 @@ CountryInstance::CountryInstance(
 	}
 
 	// army, navy and construction spending have minimums defined in EconomyDefines and always have an unmodified max (1.0).
-	EconomyDefines const& economy_defines = country_instance_deps->economy_defines;
+	EconomyDefines const& economy_defines = country_instance_deps.economy_defines;
 	army_spending_slider_value.set_bounds(economy_defines.get_minimum_army_spending_slider_value(), 1);
 	army_spending_slider_value.set_value(1);
 
@@ -274,7 +274,7 @@ CountryInstance::CountryInstance(
 	tariff_rate_slider_value.set_bounds(0, 0);
 	tariff_rate_slider_value.set_value(0);
 
-	update_parties_for_votes(new_country_definition);
+	update_parties_for_votes(&new_country_definition);
 
 	for (BuildingType const& building_type : building_type_unlock_levels.get_keys()) {
 		if (building_type.is_enabled_by_default) {
@@ -393,17 +393,17 @@ void CountryInstance::set_alliance_with(CountryInstance& country, bool alliance)
 }
 
 bool CountryInstance::is_at_war_with(CountryInstance const& country) const {
-	return war_enemies.contains(&country);
+	return war_enemies.contains(country);
 }
 
 void CountryInstance::set_at_war_with(CountryInstance& country, bool at_war) {
 	country_relations_manager.set_at_war_with(this, &country, at_war);
 	if (at_war) {
-		war_enemies.insert(&country);
-		country.war_enemies.insert(this);
+		war_enemies.emplace(country);
+		country.war_enemies.emplace(*this);
 	} else {
-		war_enemies.unordered_erase(&country);
-		country.war_enemies.unordered_erase(this);
+		war_enemies.unordered_erase(country);
+		country.war_enemies.unordered_erase(*this);
 	}
 }
 
@@ -639,7 +639,7 @@ bool CountryInstance::set_ruling_party(CountryParty const& new_ruling_party) {
 }
 
 bool CountryInstance::add_reform(Reform const& new_reform) {
-	ReformGroup const& reform_group = new_reform.get_reform_group();
+	ReformGroup const& reform_group = new_reform.group;
 	Reform const*& reform = reforms.at(reform_group);
 
 	if (reform != &new_reform) {
@@ -1211,7 +1211,7 @@ bool CountryInstance::can_research_tech(Technology const& technology, const Date
 
 	const Technology::area_index_t index_in_area = technology.index_in_area;
 
-	return index_in_area == 0 || is_technology_unlocked(*technology.area.get_technologies()[index_in_area - 1]);
+	return index_in_area == 0 || is_technology_unlocked(technology.area.get_technologies()[index_in_area - 1]);
 }
 
 void CountryInstance::start_research(Technology const& technology, const Date today) {
@@ -1233,7 +1233,7 @@ void CountryInstance::apply_foreign_investments(
 	fixed_point_map_t<CountryDefinition const*> const& investments, CountryInstanceManager const& country_instance_manager
 ) {
 	for (auto const& [country, money_invested] : investments) {
-		foreign_investments[&country_instance_manager.get_country_instance_by_definition(*country)] = money_invested;
+		foreign_investments[country_instance_manager.get_country_instance_by_definition(*country)] = money_invested;
 	}
 }
 
@@ -1320,16 +1320,17 @@ void CountryInstance::_update_production() {
 	industrial_power_from_states.clear();
 	industrial_power_from_investments.clear();
 
-	for (State const* state : states) {
-		const fixed_point_t state_industrial_power = state->get_industrial_power();
+	for (State const* state_ptr : states) {
+		State const& state = *state_ptr;
+		const fixed_point_t state_industrial_power = state.get_industrial_power();
 		if (state_industrial_power != 0) {
 			industrial_power += state_industrial_power;
 			industrial_power_from_states.emplace_back(state, state_industrial_power);
 		}
 	}
 
 	for (auto const& [country, money_invested] : foreign_investments) {
-		if (country->exists()) {
+		if (country.get().exists()) {
 			const fixed_point_t investment_industrial_power = fixed_point_t::mul_div(
 				money_invested,
 				country_defines.get_country_investment_industrial_score_factor(),

src/openvic-simulation/country/CountryInstance.hpp

@@ -154,10 +154,10 @@ namespace OpenVic {
 
 		/* Production */
 		OV_STATE_PROPERTY(fixed_point_t, industrial_power);
-		memory::vector<std::pair<State const*, fixed_point_t>> SPAN_PROPERTY(industrial_power_from_states);
-		memory::vector<std::pair<CountryInstance const*, fixed_point_t>> SPAN_PROPERTY(industrial_power_from_investments);
+		memory::vector<std::pair<std::reference_wrapper<const State>, fixed_point_t>> SPAN_PROPERTY(industrial_power_from_states);
+		memory::vector<std::pair<std::reference_wrapper<const CountryInstance>, fixed_point_t>> SPAN_PROPERTY(industrial_power_from_investments);
 		size_t PROPERTY(industrial_rank, 0);
-		fixed_point_map_t<CountryInstance const*> PROPERTY(foreign_investments);
+		fixed_point_map_t<std::reference_wrapper<const CountryInstance>> PROPERTY(foreign_investments);
 		OV_IFLATMAP_PROPERTY(BuildingType, building_level_t, building_type_unlock_levels);
 		// TODO - total amount of each good produced
 
@@ -345,7 +345,7 @@ namespace OpenVic {
 		// The last time this country lost a war, i.e. accepted a peace offer sent from their offer tab or the enemy's demand
 		// tab, even white peace. Used for the "has_recently_lost_war" condition (true if the date is less than 5 years ago).
 		Date PROPERTY(last_war_loss_date);
-		vector_ordered_set<CountryInstance const*> PROPERTY(war_enemies);
+		vector_ordered_set<std::reference_wrapper<const CountryInstance>> PROPERTY(war_enemies);
 		OV_STATE_PROPERTY(CountryInstance const*, sphere_owner, nullptr);
 		// TODO - colonial power, current wars
 
@@ -397,11 +397,10 @@ namespace OpenVic {
 		memory::vector<technology_unlock_level_t> SPAN_PROPERTY(unit_variant_unlock_levels);
 
 	public:
-		//pointers instead of references to allow construction via std::tuple
 		CountryInstance(
-			CountryDefinition const* new_country_definition,
-			SharedCountryValues* new_shared_country_values,
-			CountryInstanceDeps const* country_instance_deps
+			CountryDefinition const& new_country_definition,
+			SharedCountryValues& new_shared_country_values,
+			CountryInstanceDeps const& country_instance_deps
 		);
 		CountryInstance(CountryInstance const&) = delete;
 		CountryInstance& operator=(CountryInstance const&) = delete;