Skip to content

Commit 2d52540

Browse files
authored
Merge pull request #504 from OpenVicProject/distinguish_between_0_1_based_indexing
Clearly distinguish between 0- & 1-based indexing
2 parents dce2cea + 096ee5b commit 2d52540

File tree

12 files changed

+75
-57
lines changed

12 files changed

+75
-57
lines changed

src/openvic-simulation/console/ConsoleInstance.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ ProvinceInstance* ConsoleInstance::validate_province_index(std::string_view valu
208208
return nullptr;
209209
}
210210

211-
ProvinceInstance* province = instance_manager.get_map_instance().get_province_instance_by_index(*result);
211+
ProvinceInstance* province = instance_manager.get_map_instance().get_province_instance_by_index_1_based(*result);
212212

213213
if (province == nullptr) {
214214
write_error("Unknown province id");

src/openvic-simulation/history/ProvinceHistory.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ void ProvinceHistoryManager::lock_province_histories(MapDefinition const& map_de
181181

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

186186
history_map.sort_entries();
187187
}

src/openvic-simulation/map/MapDefinition.cpp

Lines changed: 43 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,17 @@ bool MapDefinition::add_province_definition(std::string_view identifier, colour_
5858
return false;
5959
}
6060
ProvinceDefinition new_province {
61-
identifier, colour, static_cast<ProvinceDefinition::index_t>(province_definitions.size() + 1)
61+
identifier, colour, static_cast<ProvinceDefinition::index_t>(province_definitions.size())
6262
};
63-
const ProvinceDefinition::index_t index = get_index_from_colour(colour);
64-
if (index != ProvinceDefinition::NULL_INDEX) {
63+
const ProvinceDefinition::index_t index_1_based = get_index_1_based_from_colour(colour);
64+
if (index_1_based != ProvinceDefinition::NULL_INDEX) {
6565
Logger::error(
66-
"Duplicate province colours: ", get_province_definition_by_index(index)->to_string(), " and ",
66+
"Duplicate province colours: ", get_province_definition_by_index_1_based(index_1_based)->to_string(), " and ",
6767
new_province.to_string()
6868
);
6969
return false;
7070
}
71-
colour_index_map[new_province.get_colour()] = new_province.get_index();
71+
colour_index_map[new_province.get_colour()] = new_province.get_index_1_based();
7272
return province_definitions.add_item(std::move(new_province));
7373
}
7474

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

127127
if (from.is_water()) {
128-
path_map_sea.try_add_point(to.get_index(), { to.centre.x, to.centre.y });
128+
path_map_sea.try_add_point(to.get_index_1_based(), { to.centre.x, to.centre.y });
129129
} else {
130-
path_map_sea.try_add_point(from.get_index(), { from.centre.x, from.centre.y });
130+
path_map_sea.try_add_point(from.get_index_1_based(), { from.centre.x, from.centre.y });
131131
}
132132
/* Connect points on pathfinding map */
133-
path_map_sea.connect_points(from.get_index(), to.get_index());
133+
path_map_sea.connect_points(from.get_index_1_based(), to.get_index_1_based());
134134
/* Land units can use transports to path on the sea */
135-
path_map_land.connect_points(from.get_index(), to.get_index());
135+
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
136136
/* Sea points are only valid for land units with a transport */
137137
if (from.is_water()) {
138-
path_map_land.set_point_disabled(from.get_index());
138+
path_map_land.set_point_disabled(from.get_index_1_based());
139139
} else {
140-
path_map_land.set_point_disabled(to.get_index());
140+
path_map_land.set_point_disabled(to.get_index_1_based());
141141
}
142142
} else if (from.is_water()) {
143143
/* Water-to-water adjacency */
144144
type = WATER;
145145

146146
/* Connect points on pathfinding map */
147-
path_map_sea.connect_points(from.get_index(), to.get_index());
147+
path_map_sea.connect_points(from.get_index_1_based(), to.get_index_1_based());
148148
/* Land units can use transports to path on the sea */
149-
path_map_land.connect_points(from.get_index(), to.get_index());
149+
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
150150
/* Sea points are only valid for land units with a transport */
151151
if (from.is_water()) {
152-
path_map_land.set_point_disabled(from.get_index());
152+
path_map_land.set_point_disabled(from.get_index_1_based());
153153
} else {
154-
path_map_land.set_point_disabled(to.get_index());
154+
path_map_land.set_point_disabled(to.get_index_1_based());
155155
}
156156
} else {
157157
/* Connect points on pathfinding map */
158-
path_map_land.connect_points(from.get_index(), to.get_index());
158+
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
159159
}
160160

161161
if (from_needs_adjacency) {
@@ -283,17 +283,17 @@ bool MapDefinition::add_special_adjacency(
283283
}
284284
*existing_adjacency = { &to, distance, type, through, data };
285285
if (from.is_water() && to.is_water()) {
286-
path_map_sea.connect_points(from.get_index(), to.get_index());
286+
path_map_sea.connect_points(from.get_index_1_based(), to.get_index_1_based());
287287
} else {
288-
path_map_land.connect_points(from.get_index(), to.get_index());
288+
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
289289
}
290290

291291
if (from.is_water() || to.is_water()) {
292-
path_map_land.connect_points(from.get_index(), to.get_index());
292+
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
293293
if (from.is_water()) {
294-
path_map_land.set_point_disabled(from.get_index());
294+
path_map_land.set_point_disabled(from.get_index_1_based());
295295
} else {
296-
path_map_land.set_point_disabled(to.get_index());
296+
path_map_land.set_point_disabled(to.get_index_1_based());
297297
}
298298
}
299299
return true;
@@ -305,17 +305,17 @@ bool MapDefinition::add_special_adjacency(
305305
} else {
306306
from.adjacencies.emplace_back(&to, distance, type, through, data);
307307
if (from.is_water() && to.is_water()) {
308-
path_map_sea.connect_points(from.get_index(), to.get_index());
308+
path_map_sea.connect_points(from.get_index_1_based(), to.get_index_1_based());
309309
} else {
310-
path_map_land.connect_points(from.get_index(), to.get_index());
310+
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
311311
}
312312

313313
if (from.is_water() || to.is_water()) {
314-
path_map_land.connect_points(from.get_index(), to.get_index());
314+
path_map_land.connect_points(from.get_index_1_based(), to.get_index_1_based());
315315
if (from.is_water()) {
316-
path_map_land.set_point_disabled(from.get_index());
316+
path_map_land.set_point_disabled(from.get_index_1_based());
317317
} else {
318-
path_map_land.set_point_disabled(to.get_index());
318+
path_map_land.set_point_disabled(to.get_index_1_based());
319319
}
320320
}
321321
return true;
@@ -346,7 +346,7 @@ bool MapDefinition::set_water_province(std::string_view identifier) {
346346
return false;
347347
}
348348
province->water = true;
349-
path_map_sea.try_add_point(province->get_index(), path_map_land.get_point_position(province->get_index()));
349+
path_map_sea.try_add_point(province->get_index_1_based(), path_map_land.get_point_position(province->get_index_1_based()));
350350
return true;
351351
}
352352

@@ -430,27 +430,27 @@ bool MapDefinition::add_region(std::string_view identifier, memory::vector<Provi
430430
return ret;
431431
}
432432

433-
ProvinceDefinition::index_t MapDefinition::get_index_from_colour(colour_t colour) const {
433+
ProvinceDefinition::index_t MapDefinition::get_index_1_based_from_colour(colour_t colour) const {
434434
const colour_index_map_t::const_iterator it = colour_index_map.find(colour);
435435
if (it != colour_index_map.end()) {
436436
return it->second;
437437
}
438438
return ProvinceDefinition::NULL_INDEX;
439439
}
440440

441-
ProvinceDefinition::index_t MapDefinition::get_province_index_at(ivec2_t pos) const {
441+
ProvinceDefinition::index_t MapDefinition::get_province_index_1_based_at(ivec2_t pos) const {
442442
if (pos.nonnegative() && pos.is_within_bound(dims)) {
443-
return province_shape_image[get_pixel_index_from_pos(pos)].index;
443+
return province_shape_image[get_pixel_index_from_pos(pos)].index_1_based;
444444
}
445445
return ProvinceDefinition::NULL_INDEX;
446446
}
447447

448448
ProvinceDefinition* MapDefinition::get_province_definition_at(ivec2_t pos) {
449-
return get_province_definition_by_index(get_province_index_at(pos));
449+
return get_province_definition_by_index_1_based(get_province_index_1_based_at(pos));
450450
}
451451

452452
ProvinceDefinition const* MapDefinition::get_province_definition_at(ivec2_t pos) const {
453-
return get_province_definition_by_index(get_province_index_at(pos));
453+
return get_province_definition_by_index_1_based(get_province_index_1_based_at(pos));
454454
}
455455

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

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

815815
if (pos.x > 0) {
816816
const size_t jdx = pixel_index - 1;
817817
if (colour_at(province_data, jdx) == province_colour) {
818-
province_index = province_shape_image[jdx].index;
818+
province_index_1_based = province_shape_image[jdx].index_1_based;
819819
goto index_found;
820820
}
821821
}
822822

823823
if (pos.y > 0) {
824824
const size_t jdx = pixel_index - get_width();
825825
if (colour_at(province_data, jdx) == province_colour) {
826-
province_index = province_shape_image[jdx].index;
826+
province_index_1_based = province_shape_image[jdx].index_1_based;
827827
goto index_found;
828828
}
829829
}
830830

831-
province_index = get_index_from_colour(province_colour);
831+
province_index_1_based = get_index_1_based_from_colour(province_colour);
832832

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

842842
index_found:
843-
province_shape_image[pixel_index].index = province_index;
843+
province_shape_image[pixel_index].index_1_based = province_index_1_based;
844844

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

851851
const TerrainTypeMapping::index_t terrain = terrain_data[pixel_index];
852852
TerrainTypeMapping const* mapping = terrain_type_manager.get_terrain_type_mapping_for(terrain);
853853
if (mapping != nullptr) {
854-
if (province_index != ProvinceDefinition::NULL_INDEX) {
855-
terrain_type_pixels_list[province_index - 1][&mapping->get_type()]++;
854+
if (province_index_1_based != ProvinceDefinition::NULL_INDEX) {
855+
terrain_type_pixels_list[province_index_1_based - 1][&mapping->get_type()]++;
856856
}
857857
if (mapping->get_has_texture() && terrain < terrain_type_manager.get_terrain_texture_limit()) {
858858
province_shape_image[pixel_index].terrain = terrain + 1;

src/openvic-simulation/map/MapDefinition.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ namespace OpenVic {
4545
#pragma pack(push, 1)
4646
/* Used to represent tightly packed 3-byte integer pixel information. */
4747
struct shape_pixel_t {
48-
ProvinceDefinition::index_t index;
48+
ProvinceDefinition::index_t index_1_based;
4949
TerrainTypeMapping::index_t terrain;
5050
};
5151
#pragma pack(pop)
@@ -73,7 +73,7 @@ namespace OpenVic {
7373
PointMap PROPERTY_REF(path_map_land);
7474
PointMap PROPERTY_REF(path_map_sea);
7575

76-
ProvinceDefinition::index_t get_index_from_colour(colour_t colour) const;
76+
ProvinceDefinition::index_t get_index_1_based_from_colour(colour_t colour) const;
7777
bool _generate_standard_province_adjacencies();
7878

7979
inline constexpr int32_t get_pixel_index_from_pos(ivec2_t pos) const {
@@ -106,7 +106,7 @@ namespace OpenVic {
106106
size_t get_land_province_count() const;
107107
size_t get_water_province_count() const;
108108

109-
ProvinceDefinition::index_t get_province_index_at(ivec2_t pos) const;
109+
ProvinceDefinition::index_t get_province_index_1_based_at(ivec2_t pos) const;
110110

111111
private:
112112
ProvinceDefinition* get_province_definition_at(ivec2_t pos);

src/openvic-simulation/map/MapInstance.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ MapInstance::MapInstance(
1717
sea_pathing { *this } {}
1818

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

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

2727
void MapInstance::enable_canal(canal_index_t canal_index) {

src/openvic-simulation/map/Mapmode.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ bool MapmodeManager::generate_mapmode_colours(
8585

8686
if (map_instance.province_instances_are_locked()) {
8787
for (ProvinceInstance const& province : map_instance.get_province_instances()) {
88-
target_stripes[province.get_index()] = mapmode->get_base_stripe_colours(
88+
target_stripes[province.get_province_definition().get_index_1_based()] = mapmode->get_base_stripe_colours(
8989
map_instance, province, player_country, selected_province
9090
);
9191
}
@@ -327,7 +327,7 @@ bool MapmodeManager::setup_mapmodes() {
327327
CountryInstance const* player_country, ProvinceInstance const* selected_province
328328
) -> Mapmode::base_stripe_t {
329329
const colour_argb_t::value_type f = colour_argb_t::colour_traits::component_from_fraction(
330-
province.get_index(), map_instance.get_map_definition().get_province_definition_count() + 1
330+
province.get_province_definition().get_index_1_based(), map_instance.get_map_definition().get_province_definition_count() + 1
331331
);
332332
return colour_argb_t::fill_as(f).with_alpha(ALPHA_VALUE);
333333
}

src/openvic-simulation/map/ProvinceDefinition.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ ProvinceDefinition::ProvinceDefinition(
1818
HasIndex { new_index } {}
1919

2020
memory::string ProvinceDefinition::to_string() const {
21-
return memory::fmt::format("(#{}, {}, 0x{})", get_index(), get_identifier(), get_colour());
21+
return memory::fmt::format("(#{}, {}, 0x{})", get_index_1_based(), get_identifier(), get_colour());
2222
}
2323

2424
bool ProvinceDefinition::load_positions(

src/openvic-simulation/map/ProvinceDefinition.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ namespace OpenVic {
112112
return region != nullptr;
113113
}
114114

115+
constexpr index_t get_index_1_based() const {
116+
return get_index()+1;
117+
}
118+
115119
/* The positions' y coordinates need to be inverted. */
116120
bool load_positions(
117121
MapDefinition const& map_definition, BuildingTypeManager const& building_type_manager, ast::NodeCPtr root

src/openvic-simulation/misc/GameAction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ bool GameActionManager::game_action_callback_expand_province_building(game_actio
209209
return false;
210210
}
211211

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

214214
if (OV_unlikely(province == nullptr)) {
215215
Logger::error("GAME_ACTION_EXPAND_PROVINCE_BUILDING called with invalid province index: ", province_building->first);

src/openvic-simulation/pathfinding/AStarPathing.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ bool ArmyAStarPathing::_is_point_enabled(search_const_iterator it) const {
115115
return true;
116116
}
117117

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

120120
if (OV_unlikely(province == nullptr)) {
121121
return true;
@@ -142,7 +142,7 @@ bool NavyAStarPathing::_is_point_enabled(search_const_iterator it) const {
142142
return true;
143143
}
144144

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

147147
if (OV_unlikely(province == nullptr)) {
148148
return true;

0 commit comments

Comments
 (0)