Skip to content

Clearly distinguish between 0- & 1-based indexing #504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/openvic-simulation/console/ConsoleInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ ProvinceInstance* ConsoleInstance::validate_province_index(std::string_view valu
return nullptr;
}

ProvinceInstance* province = instance_manager.get_map_instance().get_province_instance_by_index(*result);
ProvinceInstance* province = instance_manager.get_map_instance().get_province_instance_by_index_1_based(*result);

if (province == nullptr) {
write_error("Unknown province id");
Expand Down
2 changes: 1 addition & 1 deletion src/openvic-simulation/history/ProvinceHistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ void ProvinceHistoryManager::lock_province_histories(MapDefinition const& map_de

memory::vector<bool> province_checklist(provinces.size());
for (auto [province, history_map] : mutable_iterator(province_histories)) {
province_checklist[province->get_index() - 1] = true;
province_checklist[province->get_index()] = true;

history_map.sort_entries();
}
Expand Down
86 changes: 43 additions & 43 deletions src/openvic-simulation/map/MapDefinition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,17 @@ bool MapDefinition::add_province_definition(std::string_view identifier, colour_
return false;
}
ProvinceDefinition new_province {
identifier, colour, static_cast<ProvinceDefinition::index_t>(province_definitions.size() + 1)
identifier, colour, static_cast<ProvinceDefinition::index_t>(province_definitions.size())
};
const ProvinceDefinition::index_t index = get_index_from_colour(colour);
if (index != ProvinceDefinition::NULL_INDEX) {
const ProvinceDefinition::index_t index_1_based = get_index_1_based_from_colour(colour);
if (index_1_based != ProvinceDefinition::NULL_INDEX) {
Logger::error(
"Duplicate province colours: ", get_province_definition_by_index(index)->to_string(), " and ",
"Duplicate province colours: ", get_province_definition_by_index_1_based(index_1_based)->to_string(), " and ",
new_province.to_string()
);
return false;
}
colour_index_map[new_province.get_colour()] = new_province.get_index();
colour_index_map[new_province.get_colour()] = new_province.get_index_1_based();
return province_definitions.add_item(std::move(new_province));
}

Expand Down Expand Up @@ -125,37 +125,37 @@ bool MapDefinition::add_standard_adjacency(ProvinceDefinition& from, ProvinceDef
to.coastal = !to.is_water();

if (from.is_water()) {
path_map_sea.try_add_point(to.get_index(), { to.centre.x, to.centre.y });
path_map_sea.try_add_point(to.get_index_1_based(), { to.centre.x, to.centre.y });
} else {
path_map_sea.try_add_point(from.get_index(), { from.centre.x, from.centre.y });
path_map_sea.try_add_point(from.get_index_1_based(), { from.centre.x, from.centre.y });
}
/* Connect points on pathfinding map */
path_map_sea.connect_points(from.get_index(), to.get_index());
path_map_sea.connect_points(from.get_index_1_based(), to.get_index_1_based());
/* Land units can use transports to path on the sea */
path_map_land.connect_points(from.get_index(), to.get_index());
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
/* Sea points are only valid for land units with a transport */
if (from.is_water()) {
path_map_land.set_point_disabled(from.get_index());
path_map_land.set_point_disabled(from.get_index_1_based());
} else {
path_map_land.set_point_disabled(to.get_index());
path_map_land.set_point_disabled(to.get_index_1_based());
}
} else if (from.is_water()) {
/* Water-to-water adjacency */
type = WATER;

/* Connect points on pathfinding map */
path_map_sea.connect_points(from.get_index(), to.get_index());
path_map_sea.connect_points(from.get_index_1_based(), to.get_index_1_based());
/* Land units can use transports to path on the sea */
path_map_land.connect_points(from.get_index(), to.get_index());
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
/* Sea points are only valid for land units with a transport */
if (from.is_water()) {
path_map_land.set_point_disabled(from.get_index());
path_map_land.set_point_disabled(from.get_index_1_based());
} else {
path_map_land.set_point_disabled(to.get_index());
path_map_land.set_point_disabled(to.get_index_1_based());
}
} else {
/* Connect points on pathfinding map */
path_map_land.connect_points(from.get_index(), to.get_index());
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
}

if (from_needs_adjacency) {
Expand Down Expand Up @@ -283,17 +283,17 @@ bool MapDefinition::add_special_adjacency(
}
*existing_adjacency = { &to, distance, type, through, data };
if (from.is_water() && to.is_water()) {
path_map_sea.connect_points(from.get_index(), to.get_index());
path_map_sea.connect_points(from.get_index_1_based(), to.get_index_1_based());
} else {
path_map_land.connect_points(from.get_index(), to.get_index());
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
}

if (from.is_water() || to.is_water()) {
path_map_land.connect_points(from.get_index(), to.get_index());
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
if (from.is_water()) {
path_map_land.set_point_disabled(from.get_index());
path_map_land.set_point_disabled(from.get_index_1_based());
} else {
path_map_land.set_point_disabled(to.get_index());
path_map_land.set_point_disabled(to.get_index_1_based());
}
}
return true;
Expand All @@ -305,17 +305,17 @@ bool MapDefinition::add_special_adjacency(
} else {
from.adjacencies.emplace_back(&to, distance, type, through, data);
if (from.is_water() && to.is_water()) {
path_map_sea.connect_points(from.get_index(), to.get_index());
path_map_sea.connect_points(from.get_index_1_based(), to.get_index_1_based());
} else {
path_map_land.connect_points(from.get_index(), to.get_index());
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
}

if (from.is_water() || to.is_water()) {
path_map_land.connect_points(from.get_index(), to.get_index());
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
if (from.is_water()) {
path_map_land.set_point_disabled(from.get_index());
path_map_land.set_point_disabled(from.get_index_1_based());
} else {
path_map_land.set_point_disabled(to.get_index());
path_map_land.set_point_disabled(to.get_index_1_based());
}
}
return true;
Expand Down Expand Up @@ -346,7 +346,7 @@ bool MapDefinition::set_water_province(std::string_view identifier) {
return false;
}
province->water = true;
path_map_sea.try_add_point(province->get_index(), path_map_land.get_point_position(province->get_index()));
path_map_sea.try_add_point(province->get_index_1_based(), path_map_land.get_point_position(province->get_index_1_based()));
return true;
}

Expand Down Expand Up @@ -430,27 +430,27 @@ bool MapDefinition::add_region(std::string_view identifier, memory::vector<Provi
return ret;
}

ProvinceDefinition::index_t MapDefinition::get_index_from_colour(colour_t colour) const {
ProvinceDefinition::index_t MapDefinition::get_index_1_based_from_colour(colour_t colour) const {
const colour_index_map_t::const_iterator it = colour_index_map.find(colour);
if (it != colour_index_map.end()) {
return it->second;
}
return ProvinceDefinition::NULL_INDEX;
}

ProvinceDefinition::index_t MapDefinition::get_province_index_at(ivec2_t pos) const {
ProvinceDefinition::index_t MapDefinition::get_province_index_1_based_at(ivec2_t pos) const {
if (pos.nonnegative() && pos.is_within_bound(dims)) {
return province_shape_image[get_pixel_index_from_pos(pos)].index;
return province_shape_image[get_pixel_index_from_pos(pos)].index_1_based;
}
return ProvinceDefinition::NULL_INDEX;
}

ProvinceDefinition* MapDefinition::get_province_definition_at(ivec2_t pos) {
return get_province_definition_by_index(get_province_index_at(pos));
return get_province_definition_by_index_1_based(get_province_index_1_based_at(pos));
}

ProvinceDefinition const* MapDefinition::get_province_definition_at(ivec2_t pos) const {
return get_province_definition_by_index(get_province_index_at(pos));
return get_province_definition_by_index_1_based(get_province_index_1_based_at(pos));
}

bool MapDefinition::set_max_provinces(ProvinceDefinition::index_t new_max_provinces) {
Expand Down Expand Up @@ -544,7 +544,7 @@ bool MapDefinition::load_province_definitions(std::span<const LineObject> lines)
}

ProvinceDefinition const& definition = province_definitions.back();
ret &= path_map_land.try_add_point(definition.get_index(), { definition.centre.x, definition.centre.y });
ret &= path_map_land.try_add_point(definition.get_index_1_based(), { definition.centre.x, definition.centre.y });
if (!ret) {
Logger::error("Province ", identifier, " could not be added to " _OV_STR(path_map_land));
}
Expand Down Expand Up @@ -810,27 +810,27 @@ bool MapDefinition::load_map_images(fs::path const& province_path, fs::path cons
for (pos.x = 0; pos.x < get_width(); ++pos.x) {
const size_t pixel_index = get_pixel_index_from_pos(pos);
const colour_t province_colour = colour_at(province_data, pixel_index);
ProvinceDefinition::index_t province_index = ProvinceDefinition::NULL_INDEX;
ProvinceDefinition::index_t province_index_1_based = ProvinceDefinition::NULL_INDEX;

if (pos.x > 0) {
const size_t jdx = pixel_index - 1;
if (colour_at(province_data, jdx) == province_colour) {
province_index = province_shape_image[jdx].index;
province_index_1_based = province_shape_image[jdx].index_1_based;
goto index_found;
}
}

if (pos.y > 0) {
const size_t jdx = pixel_index - get_width();
if (colour_at(province_data, jdx) == province_colour) {
province_index = province_shape_image[jdx].index;
province_index_1_based = province_shape_image[jdx].index_1_based;
goto index_found;
}
}

province_index = get_index_from_colour(province_colour);
province_index_1_based = get_index_1_based_from_colour(province_colour);

if (province_index == ProvinceDefinition::NULL_INDEX && !unrecognised_province_colours.contains(province_colour)) {
if (province_index_1_based == ProvinceDefinition::NULL_INDEX && !unrecognised_province_colours.contains(province_colour)) {
unrecognised_province_colours.insert(province_colour);
if (detailed_errors) {
Logger::warning(
Expand All @@ -840,19 +840,19 @@ bool MapDefinition::load_map_images(fs::path const& province_path, fs::path cons
}

index_found:
province_shape_image[pixel_index].index = province_index;
province_shape_image[pixel_index].index_1_based = province_index_1_based;

if (province_index != ProvinceDefinition::NULL_INDEX) {
const ProvinceDefinition::index_t array_index = province_index - 1;
if (province_index_1_based != ProvinceDefinition::NULL_INDEX) {
const ProvinceDefinition::index_t array_index = province_index_1_based - 1;
pixels_per_province[array_index]++;
pixel_position_sum_per_province[array_index] += static_cast<fvec2_t>(pos);
}

const TerrainTypeMapping::index_t terrain = terrain_data[pixel_index];
TerrainTypeMapping const* mapping = terrain_type_manager.get_terrain_type_mapping_for(terrain);
if (mapping != nullptr) {
if (province_index != ProvinceDefinition::NULL_INDEX) {
terrain_type_pixels_list[province_index - 1][&mapping->get_type()]++;
if (province_index_1_based != ProvinceDefinition::NULL_INDEX) {
terrain_type_pixels_list[province_index_1_based - 1][&mapping->get_type()]++;
}
if (mapping->get_has_texture() && terrain < terrain_type_manager.get_terrain_texture_limit()) {
province_shape_image[pixel_index].terrain = terrain + 1;
Expand Down
6 changes: 3 additions & 3 deletions src/openvic-simulation/map/MapDefinition.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ namespace OpenVic {
#pragma pack(push, 1)
/* Used to represent tightly packed 3-byte integer pixel information. */
struct shape_pixel_t {
ProvinceDefinition::index_t index;
ProvinceDefinition::index_t index_1_based;
TerrainTypeMapping::index_t terrain;
};
#pragma pack(pop)
Expand Down Expand Up @@ -73,7 +73,7 @@ namespace OpenVic {
PointMap PROPERTY_REF(path_map_land);
PointMap PROPERTY_REF(path_map_sea);

ProvinceDefinition::index_t get_index_from_colour(colour_t colour) const;
ProvinceDefinition::index_t get_index_1_based_from_colour(colour_t colour) const;
bool _generate_standard_province_adjacencies();

inline constexpr int32_t get_pixel_index_from_pos(ivec2_t pos) const {
Expand Down Expand Up @@ -106,7 +106,7 @@ namespace OpenVic {
size_t get_land_province_count() const;
size_t get_water_province_count() const;

ProvinceDefinition::index_t get_province_index_at(ivec2_t pos) const;
ProvinceDefinition::index_t get_province_index_1_based_at(ivec2_t pos) const;

private:
ProvinceDefinition* get_province_definition_at(ivec2_t pos);
Expand Down
4 changes: 2 additions & 2 deletions src/openvic-simulation/map/MapInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ MapInstance::MapInstance(
sea_pathing { *this } {}

ProvinceInstance& MapInstance::get_province_instance_from_definition(ProvinceDefinition const& province) {
return province_instances.get_items()[province.get_index() - 1];
return province_instances.get_items()[province.get_index()];
}

ProvinceInstance const& MapInstance::get_province_instance_from_definition(ProvinceDefinition const& province) const {
return province_instances.get_items()[province.get_index() - 1];
return province_instances.get_items()[province.get_index()];
}

void MapInstance::enable_canal(canal_index_t canal_index) {
Expand Down
4 changes: 2 additions & 2 deletions src/openvic-simulation/map/Mapmode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ bool MapmodeManager::generate_mapmode_colours(

if (map_instance.province_instances_are_locked()) {
for (ProvinceInstance const& province : map_instance.get_province_instances()) {
target_stripes[province.get_index()] = mapmode->get_base_stripe_colours(
target_stripes[province.get_province_definition().get_index_1_based()] = mapmode->get_base_stripe_colours(
map_instance, province, player_country, selected_province
);
}
Expand Down Expand Up @@ -327,7 +327,7 @@ bool MapmodeManager::setup_mapmodes() {
CountryInstance const* player_country, ProvinceInstance const* selected_province
) -> Mapmode::base_stripe_t {
const colour_argb_t::value_type f = colour_argb_t::colour_traits::component_from_fraction(
province.get_index(), map_instance.get_map_definition().get_province_definition_count() + 1
province.get_province_definition().get_index_1_based(), map_instance.get_map_definition().get_province_definition_count() + 1
);
return colour_argb_t::fill_as(f).with_alpha(ALPHA_VALUE);
}
Expand Down
2 changes: 1 addition & 1 deletion src/openvic-simulation/map/ProvinceDefinition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ ProvinceDefinition::ProvinceDefinition(
HasIndex { new_index } {}

memory::string ProvinceDefinition::to_string() const {
return memory::fmt::format("(#{}, {}, 0x{})", get_index(), get_identifier(), get_colour());
return memory::fmt::format("(#{}, {}, 0x{})", get_index_1_based(), get_identifier(), get_colour());
}

bool ProvinceDefinition::load_positions(
Expand Down
4 changes: 4 additions & 0 deletions src/openvic-simulation/map/ProvinceDefinition.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ namespace OpenVic {
return region != nullptr;
}

constexpr index_t get_index_1_based() const {
return get_index()+1;
}

/* The positions' y coordinates need to be inverted. */
bool load_positions(
MapDefinition const& map_definition, BuildingTypeManager const& building_type_manager, ast::NodeCPtr root
Expand Down
2 changes: 1 addition & 1 deletion src/openvic-simulation/misc/GameAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ bool GameActionManager::game_action_callback_expand_province_building(game_actio
return false;
}

ProvinceInstance* province = instance_manager.get_map_instance().get_province_instance_by_index(province_building->first);
ProvinceInstance* province = instance_manager.get_map_instance().get_province_instance_by_index_1_based(province_building->first);

if (OV_unlikely(province == nullptr)) {
Logger::error("GAME_ACTION_EXPAND_PROVINCE_BUILDING called with invalid province index: ", province_building->first);
Expand Down
4 changes: 2 additions & 2 deletions src/openvic-simulation/pathfinding/AStarPathing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ bool ArmyAStarPathing::_is_point_enabled(search_const_iterator it) const {
return true;
}

ProvinceInstance const* province = map_instance.get_province_instance_by_index(it->first);
ProvinceInstance const* province = map_instance.get_province_instance_by_index_1_based(it->first);

if (OV_unlikely(province == nullptr)) {
return true;
Expand All @@ -142,7 +142,7 @@ bool NavyAStarPathing::_is_point_enabled(search_const_iterator it) const {
return true;
}

ProvinceInstance const* province = map_instance.get_province_instance_by_index(it->first);
ProvinceInstance const* province = map_instance.get_province_instance_by_index_1_based(it->first);

if (OV_unlikely(province == nullptr)) {
return true;
Expand Down
1 change: 1 addition & 0 deletions src/openvic-simulation/types/HasIndex.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace OpenVic {
public:
using index_t = IndexT;
private:
// always 0-based, this may be used for indexing arrays
const index_t PROPERTY(index);

protected:
Expand Down
15 changes: 14 additions & 1 deletion src/openvic-simulation/types/IdentifierRegistry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,12 @@ public: \
#define IDENTIFIER_REGISTRY_NON_CONST_ACCESSORS_FULL_CUSTOM(singular, plural, registry, debug_name, index_offset) \
IDENTIFIER_REGISTRY_INTERNAL_SHARED(singular, plural, registry, index_offset,)

#define IF_NON_ZERO(value, conditional_part) IF_NON_ZERO_##value(conditional_part)
#define IF_NON_ZERO_0(conditional_part)
#define IF_NON_ZERO_1(conditional_part) conditional_part
#define CONCAT_FORCE_EXPAND(a, b) a##b
#define CONCAT_TOKENS(a, b) CONCAT_FORCE_EXPAND(a, b)

#define IDENTIFIER_REGISTRY_INTERNAL_SHARED(singular, plural, registry, index_offset, const_kw) \
constexpr decltype(registry)::external_value_type const_kw& get_front_##singular() const_kw { \
return registry.front(); \
Expand All @@ -639,7 +645,14 @@ public: \
constexpr T const_kw* get_cast_##singular##_by_identifier(std::string_view identifier) const_kw { \
return registry.get_cast_item_by_identifier<T>(identifier); \
} \
constexpr decltype(registry)::external_value_type const_kw* get_##singular##_by_index(std::size_t index) const_kw { \
IF_NON_ZERO(index_offset, \
constexpr decltype(registry)::external_value_type const_kw* get_##singular##_by_index_0_based(std::size_t index_0_based) const_kw { \
return index_0_based >= 0 ? registry.get_item_by_index(index_0_based) : nullptr; \
} \
) \
constexpr decltype(registry)::external_value_type const_kw* \
CONCAT_TOKENS(get_##singular##_by_index, IF_NON_ZERO(index_offset, _##index_offset##_based)) \
(std::size_t index) const_kw { \
return index >= index_offset ? registry.get_item_by_index(index - index_offset) : nullptr; \
} \
constexpr decltype(registry)::storage_type const_kw& get_##plural() const_kw { \
Expand Down
Loading