diff --git a/src/openvic-simulation/economy/BuildingType.cpp b/src/openvic-simulation/economy/BuildingType.cpp index 2325b734..c1681b24 100644 --- a/src/openvic-simulation/economy/BuildingType.cpp +++ b/src/openvic-simulation/economy/BuildingType.cpp @@ -89,12 +89,12 @@ bool BuildingType::can_be_built_in( if (is_limited_to_one_per_state) { State const* state = location.get_state(); if (state != nullptr) { - for (ProvinceInstance const* province_in_state : state->get_provinces()) { - if (province_in_state == nullptr || *province_in_state == location) { + for (ProvinceInstance const& province_in_state : state->get_provinces()) { + if (province_in_state == location) { continue; } - const building_level_t other_building_level = province_in_state->get_buildings()[province_building_index.value()].get_level(); + const building_level_t other_building_level = province_in_state.get_buildings()[province_building_index.value()].get_level(); if (other_building_level > building_level_t(0)) { return false; } diff --git a/src/openvic-simulation/economy/production/ResourceGatheringOperation.cpp b/src/openvic-simulation/economy/production/ResourceGatheringOperation.cpp index f2b7dd65..2ac1e6dd 100644 --- a/src/openvic-simulation/economy/production/ResourceGatheringOperation.cpp +++ b/src/openvic-simulation/economy/production/ResourceGatheringOperation.cpp @@ -400,8 +400,7 @@ void ResourceGatheringOperation::pay_employees(memory::vector& re upper_limit ); - for (Pop* owner_pop_ptr : *owner_pops_cache_nullable) { - Pop& owner_pop = *owner_pop_ptr; + for (Pop& owner_pop : *owner_pops_cache_nullable) { const fixed_point_t income_for_this_pop = std::max( revenue_left * owner_share.mul_div(owner_pop.get_size(), total_owner_count_in_state_cache), fixed_point_t::epsilon //revenue > 0 is already checked, so rounding up diff --git a/src/openvic-simulation/economy/production/ResourceGatheringOperation.hpp b/src/openvic-simulation/economy/production/ResourceGatheringOperation.hpp index 9cbf97b8..29adf0b9 100644 --- a/src/openvic-simulation/economy/production/ResourceGatheringOperation.hpp +++ b/src/openvic-simulation/economy/production/ResourceGatheringOperation.hpp @@ -1,5 +1,7 @@ #pragma once +#include + #include "openvic-simulation/economy/production/Employee.hpp" #include "openvic-simulation/types/fixed_point/FixedPoint.hpp" #include "openvic-simulation/types/IndexedFlatMap.hpp" @@ -25,7 +27,7 @@ namespace OpenVic { ProvinceInstance* location_ptr = nullptr; pop_sum_t total_owner_count_in_state_cache = 0; pop_sum_t total_worker_count_in_province_cache = 0; - memory::vector const* owner_pops_cache_nullable = nullptr; + memory::vector> const* owner_pops_cache_nullable = nullptr; ProductionType const* PROPERTY_RW(production_type_nullable); fixed_point_t PROPERTY(revenue_yesterday); diff --git a/src/openvic-simulation/map/MapDefinition.cpp b/src/openvic-simulation/map/MapDefinition.cpp index 216a4cf6..e81098b8 100644 --- a/src/openvic-simulation/map/MapDefinition.cpp +++ b/src/openvic-simulation/map/MapDefinition.cpp @@ -371,13 +371,14 @@ bool MapDefinition::set_water_province(std::string_view identifier) { return false; } - ProvinceDefinition* province = get_province_definition_by_identifier(identifier); + ProvinceDefinition* const province_ptr = get_province_definition_by_identifier(identifier); - if (province == nullptr) { + if (province_ptr == nullptr) { spdlog::error_s("Unrecognised water province identifier: {}", identifier); return false; } - if (province->is_water()) { + ProvinceDefinition& province = *province_ptr; + if (province.is_water()) { spdlog::warn_s("Province {} is already a water province!", identifier); return true; } @@ -385,8 +386,11 @@ bool MapDefinition::set_water_province(std::string_view identifier) { spdlog::error_s("Failed to add province {} to water province set!", identifier); return false; } - province->water = true; - path_map_sea.try_add_point(province->get_province_number(), path_map_land.get_point_position(province->get_province_number())); + province.water = true; + path_map_sea.try_add_point( + province.get_province_number(), + path_map_land.get_point_position(province.get_province_number()) + ); return true; } @@ -417,7 +421,11 @@ size_t MapDefinition::get_water_province_count() const { return water_provinces.size(); } -bool MapDefinition::add_region(std::string_view identifier, memory::vector&& provinces, colour_t colour) { +bool MapDefinition::add_region( + std::string_view identifier, + memory::vector>&& provinces, + colour_t colour +) { if (identifier.empty()) { spdlog::error_s("Invalid region identifier - empty!"); return false; @@ -428,8 +436,7 @@ bool MapDefinition::add_region(std::string_view identifier, memory::vectorhas_region()) { - region.add_province(province_definition_ptr); + for (ProvinceDefinition const& province_definition : provinces) { + if (!province_definition.has_region()) { + region.add_province(province_definition); } } } @@ -467,8 +474,8 @@ bool MapDefinition::add_region(std::string_view identifier, memory::vectorregion = ®ion; + for (ProvinceDefinition const& province_definition : region.get_provinces()) { + get_mutable_province_definition(province_definition).region = ®ion; } } return ret; @@ -640,10 +647,10 @@ bool MapDefinition::load_region_file(ast::NodeCPtr root, std::span bool { - memory::vector provinces; + memory::vector> provinces; bool ret = expect_list_reserve_length( - provinces, expect_province_definition_identifier(vector_callback_pointer(provinces)) + provinces, expect_province_definition_identifier(vector_emplace_callback(provinces)) )(region_node); ret &= add_region(region_identifier, std::move(provinces), colours[regions.size() % colours.size()]); @@ -1124,10 +1131,10 @@ bool MapDefinition::load_climate_file(ModifierManager const& modifier_manager, a ret &= expect_list_reserve_length(*cur_climate, expect_province_definition_identifier( [cur_climate, &identifier](ProvinceDefinition& province) { if (province.climate != cur_climate) { - cur_climate->add_province(&province); + cur_climate->add_province(province); if (province.climate != nullptr) { Climate* old_climate = const_cast(province.climate); - old_climate->remove_province(&province); + old_climate->remove_province(province); spdlog::warn_s( "Province with id {} found in multiple climates: {} and {}", province, identifier, *old_climate @@ -1168,13 +1175,13 @@ bool MapDefinition::load_continent_file(ModifierManager const& modifier_manager, } ModifierValue values; - memory::vector prov_list; + memory::vector> prov_list; bool ret = NodeTools::expect_dictionary_keys_and_default( modifier_manager.expect_base_province_modifier(values), "provinces", ONE_EXACTLY, expect_list_reserve_length(prov_list, expect_province_definition_identifier( [&prov_list](ProvinceDefinition const& province) -> bool { if (province.continent == nullptr) { - prov_list.emplace_back(&province); + prov_list.emplace_back(province); } else { spdlog::warn_s("Province {} found in multiple continents", province); } @@ -1194,8 +1201,8 @@ bool MapDefinition::load_continent_file(ModifierManager const& modifier_manager, continent.add_provinces(prov_list); continent.lock(); - for (ProvinceDefinition const* prov : continent.get_provinces()) { - remove_province_definition_const(prov)->continent = &continent; + for (ProvinceDefinition const& prov : continent.get_provinces()) { + get_mutable_province_definition(prov).continent = &continent; } return ret; diff --git a/src/openvic-simulation/map/MapDefinition.hpp b/src/openvic-simulation/map/MapDefinition.hpp index ed4c02dd..c52099ac 100644 --- a/src/openvic-simulation/map/MapDefinition.hpp +++ b/src/openvic-simulation/map/MapDefinition.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -114,20 +115,23 @@ namespace OpenVic { private: ProvinceDefinition* get_province_definition_at(ivec2_t pos); - /* This provides a safe way to remove the const qualifier of a ProvinceDefinition const*, via a non-const Map. - * It uses a const_cast (the fastest/simplest solution), but this could also be done without it by looking up the - * ProvinceDefinition* using the ProvinceDefinition const*'s index. Requiring a non-const Map ensures that this - * function can only be used where the ProvinceDefinition* could already be accessed by other means, such as the + /* This provides a safe way to remove the const qualifier of a ProvinceDefinition const&, via a non-const Map. + * It looks up the ProvinceDefinition& using the ProvinceDefinition const&'s index. Requiring a non-const Map ensures that this + * function can only be used where the ProvinceDefinition& could already be accessed by other means, such as the * index method, preventing misleading code, or in the worst case undefined behaviour. */ - constexpr ProvinceDefinition* remove_province_definition_const(ProvinceDefinition const* province) { - return const_cast(province); + constexpr ProvinceDefinition& get_mutable_province_definition(ProvinceDefinition const& province) { + return *province_definitions.get_item_by_index(province.index); } public: ProvinceDefinition const* get_province_definition_at(ivec2_t pos) const; bool set_max_provinces(ProvinceDefinition::index_t new_max_provinces); - bool add_region(std::string_view identifier, memory::vector&& provinces, colour_t colour); + bool add_region( + std::string_view identifier, + memory::vector>&& provinces, + colour_t colour + ); bool load_province_definitions(std::span lines); /* Must be loaded after adjacencies so we know what provinces are coastal, and so can have a port */ diff --git a/src/openvic-simulation/map/ProvinceInstance.cpp b/src/openvic-simulation/map/ProvinceInstance.cpp index 5f81baab..158405a3 100644 --- a/src/openvic-simulation/map/ProvinceInstance.cpp +++ b/src/openvic-simulation/map/ProvinceInstance.cpp @@ -53,7 +53,7 @@ ModifierSum const& ProvinceInstance::get_owner_modifier_sum() const { return owner->get_modifier_sum(); } -memory::vector const& ProvinceInstance::get_pops_cache_by_type(PopType const& key) const { +memory::vector> const& ProvinceInstance::get_pops_cache_by_type(PopType const& key) const { return pops_cache_by_type.at(key); } @@ -215,7 +215,7 @@ void ProvinceInstance::_update_pops(MilitaryDefines const& military_defines) { has_unaccepted_pops = false; - for (memory::vector& pops_cache : pops_cache_by_type.get_values()) { + for (memory::vector>& pops_cache : pops_cache_by_type.get_values()) { pops_cache.clear(); } @@ -227,7 +227,7 @@ void ProvinceInstance::_update_pops(MilitaryDefines const& military_defines) { : is_owner_core() ? fixed_point_t::_1 : military_defines.get_pop_size_per_regiment_non_core_multiplier(); for (Pop& pop : pops) { - pops_cache_by_type.at(pop.get_type()).push_back(&pop); + pops_cache_by_type.at(pop.get_type()).push_back(pop); pop.update_gamestate(military_defines, owner, pop_size_per_regiment_multiplier); add_pops_aggregate(pop); if (pop.get_culture_status() == Pop::culture_status_t::UNACCEPTED) { diff --git a/src/openvic-simulation/map/ProvinceInstance.hpp b/src/openvic-simulation/map/ProvinceInstance.hpp index 8db9059e..55b1e273 100644 --- a/src/openvic-simulation/map/ProvinceInstance.hpp +++ b/src/openvic-simulation/map/ProvinceInstance.hpp @@ -1,5 +1,7 @@ #pragma once +#include + #include #include "openvic-simulation/core/portable/ForwardableSpan.hpp" @@ -122,7 +124,7 @@ namespace OpenVic { bool convert_rgo_worker_pops_to_equivalent(ProductionType const& production_type); void initialise_rgo(); - OV_IFLATMAP_PROPERTY(PopType, memory::vector, pops_cache_by_type); + OV_IFLATMAP_PROPERTY(PopType, memory::vector>, pops_cache_by_type); public: ProvinceDefinition const& province_definition; diff --git a/src/openvic-simulation/map/Region.cpp b/src/openvic-simulation/map/Region.cpp index 4e5cb9ec..615788f5 100644 --- a/src/openvic-simulation/map/Region.cpp +++ b/src/openvic-simulation/map/Region.cpp @@ -5,19 +5,14 @@ using namespace OpenVic; -bool ProvinceSet::add_province(ProvinceDefinition const* province) { +bool ProvinceSet::add_province(ProvinceDefinition const& province) { if (locked) { spdlog::error_s("Cannot add province to province set - locked!"); return false; } - if (province == nullptr) { - spdlog::error_s("Cannot add province to province set - null province!"); - return false; - } - if (contains_province(province)) { - spdlog::warn_s("Cannot add province {} to province set - already in the set!", *province); + spdlog::warn_s("Cannot add province {} to province set - already in the set!", province); return false; } @@ -26,20 +21,15 @@ bool ProvinceSet::add_province(ProvinceDefinition const* province) { return true; } -bool ProvinceSet::remove_province(ProvinceDefinition const* province) { +bool ProvinceSet::remove_province(ProvinceDefinition const& province) { if (locked) { spdlog::error_s("Cannot remove province from province set - locked!"); return false; } - if (province == nullptr) { - spdlog::error_s("Cannot remove province from province set - null province!"); - return false; - } - const decltype(provinces)::const_iterator it = std::find(provinces.begin(), provinces.end(), province); if (it == provinces.end()) { - spdlog::warn_s("Cannot remove province {} from province set - not in the set!", *province); + spdlog::warn_s("Cannot remove province {} from province set - not in the set!", province); return false; } @@ -92,8 +82,8 @@ void ProvinceSet::reserve_more(size_t size) { OpenVic::reserve_more(*this, size); } -bool ProvinceSet::contains_province(ProvinceDefinition const* province) const { - return province != nullptr && std::find(provinces.begin(), provinces.end(), province) != provinces.end(); +bool ProvinceSet::contains_province(ProvinceDefinition const& province) const { + return std::find(provinces.begin(), provinces.end(), province) != provinces.end(); } ProvinceSetModifier::ProvinceSetModifier(std::string_view new_identifier, ModifierValue&& new_values, modifier_type_t new_type) diff --git a/src/openvic-simulation/map/Region.hpp b/src/openvic-simulation/map/Region.hpp index 8c6833d6..abed3a3e 100644 --- a/src/openvic-simulation/map/Region.hpp +++ b/src/openvic-simulation/map/Region.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -12,21 +13,20 @@ namespace OpenVic { struct ProvinceSet { private: - memory::vector SPAN_PROPERTY(provinces); + memory::vector> SPAN_PROPERTY(provinces); bool locked = false; public: /* Returns true if the province is successfully added, false if not (including if it's already in the set). */ - bool add_province(ProvinceDefinition const* province); + bool add_province(ProvinceDefinition const& province); template - requires std::convertible_to, ProvinceDefinition const*> + requires std::convertible_to, std::reference_wrapper> bool add_provinces(Container const& new_provinces) { reserve_more(new_provinces.size()); bool ret = true; - - for (ProvinceDefinition const* province : new_provinces) { + for (ProvinceDefinition const& province : new_provinces) { ret &= add_province(province); } @@ -34,7 +34,7 @@ namespace OpenVic { } /* Returns true if the province is successfully removed, false if not (including if it's not in the set). */ - bool remove_province(ProvinceDefinition const* province); + bool remove_province(ProvinceDefinition const& province); void lock(bool log = false); bool is_locked() const; void reset(); @@ -43,7 +43,7 @@ namespace OpenVic { size_t capacity() const; void reserve(size_t size); void reserve_more(size_t size); - bool contains_province(ProvinceDefinition const* province) const; + bool contains_province(ProvinceDefinition const& province) const; }; struct ProvinceSetModifier : Modifier, ProvinceSet { diff --git a/src/openvic-simulation/map/State.cpp b/src/openvic-simulation/map/State.cpp index 51cf019e..6fa3b584 100644 --- a/src/openvic-simulation/map/State.cpp +++ b/src/openvic-simulation/map/State.cpp @@ -16,7 +16,7 @@ using namespace OpenVic; State::State( StateSet const& new_state_set, ProvinceInstance* new_capital, - memory::vector&& new_provinces, + memory::vector>&& new_provinces, colony_status_t new_colony_status, forwardable_span strata_keys, forwardable_span pop_type_keys, @@ -46,24 +46,24 @@ CountryInstance* State::get_owner() const { : capital->get_owner(); } -memory::vector const& State::get_pops_cache_by_type(PopType const& pop_type) const { +memory::vector> const& State::get_pops_cache_by_type(PopType const& pop_type) const { return pops_cache_by_type.at(pop_type); } void State::update_gamestate() { clear_pops_aggregate(); - for (memory::vector& pops_cache : pops_cache_by_type.get_values()) { + for (memory::vector>& pops_cache : pops_cache_by_type.get_values()) { pops_cache.clear(); } coastal = false; - for (ProvinceInstance* const province : provinces) { - coastal |= province->province_definition.is_coastal(); - add_pops_aggregate(*province); + for (ProvinceInstance& province : provinces) { + coastal |= province.province_definition.is_coastal(); + add_pops_aggregate(province); - for (auto const& [pop_type, province_pops_of_type] : province->get_pops_cache_by_type()) { - memory::vector& state_pops_of_type = pops_cache_by_type.at(pop_type); + for (auto const& [pop_type, province_pops_of_type] : province.get_pops_cache_by_type()) { + memory::vector>& state_pops_of_type = pops_cache_by_type.at(pop_type); state_pops_of_type.insert( state_pops_of_type.end(), province_pops_of_type.begin(), @@ -113,10 +113,9 @@ void State::_update_country() { previous_country_ptr = owner_ptr; } -/* Whether two provinces in the same region should be grouped into the same state or not. - * (Assumes both provinces non-null.) */ -static bool provinces_belong_in_same_state(ProvinceInstance const* lhs, ProvinceInstance const* rhs) { - return lhs->get_owner() == rhs->get_owner() && lhs->get_colony_status() == rhs->get_colony_status(); +// Whether two provinces in the same region should be grouped into the same state or not. +static bool provinces_belong_in_same_state(ProvinceInstance const& lhs, ProvinceInstance const& rhs) { + return lhs.get_owner() == rhs.get_owner() && lhs.get_colony_status() == rhs.get_colony_status(); } StateSet::StateSet(Region const& new_region) : region { new_region } {} @@ -140,14 +139,13 @@ bool StateManager::add_state_set( OV_ERR_FAIL_COND_V_MSG(region.is_meta, false, memory::fmt::format("Cannot use meta region \"{}\" as state template!", region)); OV_ERR_FAIL_COND_V_MSG(region.empty(), false, memory::fmt::format("Cannot use empty region \"{}\" as state template!", region)); - memory::vector> temp_provinces; + memory::vector>> temp_provinces; - for (ProvinceDefinition const* province : region.get_provinces()) { - - ProvinceInstance* province_instance = &map_instance.get_province_instance_by_definition(*province); + for (ProvinceDefinition const& province : region.get_provinces()) { + ProvinceInstance& province_instance = map_instance.get_province_instance_by_definition(province); // add to existing state if shared owner & status... - for (memory::vector& provinces : temp_provinces) { + for (memory::vector>& provinces : temp_provinces) { if (provinces_belong_in_same_state(provinces.front(), province_instance)) { provinces.push_back(province_instance); // jump to the end of the outer loop, skipping the new state code @@ -170,18 +168,18 @@ bool StateManager::add_state_set( // Reserve space for the maximum number of states (one per province) state_set.states.reserve(region.size()); - for (memory::vector& provinces : temp_provinces) { - ProvinceInstance* capital = provinces.front(); + for (memory::vector>& provinces : temp_provinces) { + ProvinceInstance& capital = provinces.front(); State& state = *state_set.states.emplace( /* TODO: capital province logic */ - state_set, capital, - std::move(provinces), capital->get_colony_status(), + state_set, &capital, + std::move(provinces), capital.get_colony_status(), strata_keys, pop_type_keys, ideology_keys ); - for (ProvinceInstance* province : state.get_provinces()) { - province->set_state(&state); + for (ProvinceInstance& province : state.get_provinces()) { + province.set_state(&state); } } diff --git a/src/openvic-simulation/map/State.hpp b/src/openvic-simulation/map/State.hpp index 5eb3178c..038c11b0 100644 --- a/src/openvic-simulation/map/State.hpp +++ b/src/openvic-simulation/map/State.hpp @@ -1,8 +1,9 @@ #pragma once -#include +#include #include +#include #include "openvic-simulation/population/PopsAggregate.hpp" #include "openvic-simulation/types/ColonyStatus.hpp" @@ -33,12 +34,12 @@ namespace OpenVic { CountryInstance* previous_country_ptr = nullptr; ProvinceInstance* PROPERTY_PTR(capital); - memory::vector SPAN_PROPERTY(provinces); + memory::vector> SPAN_PROPERTY(provinces); colony_status_t PROPERTY(colony_status); fixed_point_t PROPERTY(industrial_power); bool PROPERTY_CUSTOM_PREFIX(coastal, is, false); - OV_IFLATMAP_PROPERTY(PopType, memory::vector, pops_cache_by_type); + OV_IFLATMAP_PROPERTY(PopType, memory::vector>, pops_cache_by_type); void _update_country(); @@ -48,7 +49,7 @@ namespace OpenVic { State( StateSet const& new_state_set, ProvinceInstance* new_capital, - memory::vector&& new_provinces, + memory::vector>&& new_provinces, colony_status_t new_colony_status, forwardable_span strata_keys, forwardable_span pop_type_keys,