Government salaries and social income #363
Conversation
wvpm
force-pushed
the
government_salaries_and_social_income
branch
19 times, most recently
from
6e6b63b to
3ca3458
Compare
wvpm
force-pushed
the
government_salaries_and_social_income
branch
7 times, most recently
from
04e3b44 to
98e6a59
Compare
wvpm
force-pushed
the
government_salaries_and_social_income
branch
from
98e6a59 to
3f67419
Compare
| @@ -13,6 +13,7 @@ | |||
| #include "openvic-simulation/politics/Rebel.hpp" | |||
| #include "openvic-simulation/utility/Logger.hpp" | |||
| #include "openvic-simulation/utility/TslHelper.hpp" | |||
| #include "dataloader/NodeTools.hpp" | |||
There was a problem hiding this comment.
I expect this include was added by clangd automatically, you can disable that behaviour by adding this to your user settings JSON file:
"clangd.arguments": [
"-header-insertion=never"
]|
|
||
| public: | ||
| void update_costs(PopType const& pop_type, PopsDefines const& pop_defines, GoodInstanceManager const& good_instance_manager); | ||
| constexpr fixed_point_t get_social_income_form_base() const { |
There was a problem hiding this comment.
Should this be get_social_income_from_base?
| IndexedMap<PopType, SharedPopTypeValues> PROPERTY(shared_pop_type_values); | ||
|
|
||
| public: | ||
| SharedCountryValues( |
There was a problem hiding this comment.
Best to give this a default move constructor so shared_pop_type_values doesn't ever get copied unnecessarily
| if ((pop_type.get_##need_category##_needs_income_types() & ADMINISTRATION) == ADMINISTRATION) { \ | ||
| administration_salary_base += base_##need_category##_need_costs; \ | ||
| } \ | ||
| if ((pop_type.get_##need_category##_needs_income_types() & EDUCATION) == EDUCATION) { \ | ||
| education_salary_base += base_##need_category##_need_costs; \ | ||
| } \ | ||
| if ((pop_type.get_##need_category##_needs_income_types() & MILITARY) == MILITARY) { \ | ||
| military_salary_base += base_##need_category##_need_costs; \ | ||
| } |
There was a problem hiding this comment.
All the if statements here could use share_income_type rather than manually doing the bitwise operations
|
|
||
| fixed_point_t PROPERTY(administrative_efficiency); | ||
| constexpr fixed_point_t get_corruption_cost_multiplier() const { | ||
| return 2 - administrative_efficiency; |
There was a problem hiding this comment.
Is there definitely no need for a std::max(0, ...) here or any other bounds check?
| constexpr SharedCountryValues const& get_shared_country_values() const { | ||
| return shared_country_values; | ||
| } |
There was a problem hiding this comment.
Why not just put PROPERTY around the shared_country_values declaration above?
| projected_administration_spending_unscaled_by_slider += size * calculate_administration_salary_base(pop_type, shared_country_values); | ||
| projected_education_spending_unscaled_by_slider += size * calculate_education_salary_base(pop_type, shared_country_values); | ||
| projected_military_spending_unscaled_by_slider += size * calculate_military_salary_base(pop_type, shared_country_values); | ||
| projected_pensions_spending_unscaled_by_slider += size * calculate_pensions_base(pop_type, shared_country_values); | ||
| projected_unemployment_subsidies_spending_unscaled_by_slider += pop_type_unemployed_count[pop_type] | ||
| * calculate_unemployment_subsidies_base(pop_type, shared_country_values); |
There was a problem hiding this comment.
As mentioned in previous comments, the functions called here involve many repeated lookups/calculations which could be avoided with caching
| } | ||
| } | ||
|
|
||
| for (auto const& [good_instance, data] : goods_data) { |
There was a problem hiding this comment.
| for (auto const& [good_instance, data] : goods_data) { | |
| for (auto& data : goods_data.get_values()) { |
You can iterate over an IndexedMap's values directly
| @@ -1691,6 +1822,68 @@ void CountryInstance::report_output(ProductionType const& production_type, const | |||
| good_data.production_per_production_type[&production_type] += quantity; | |||
| } | |||
|
|
|||
| void CountryInstance::request_salaries_and_welfare(Pop& pop, SharedCountryValues const& shared_country_values) const { | |||
There was a problem hiding this comment.
This function could also benefit from caching rather than always looking up/calculating new values in every function call
| } | ||
|
|
||
| #undef DO_FOR_ALL_NEED_CATEGORIES |
There was a problem hiding this comment.
This and the PopNeedsMacro.hpp include at the top of the file can be removed
Implements administration, education & military spending as well as pensions and unemployment subsidies.