Merge pull request #98 from OpenVic2Project/some-cleanup

Req comments + cleanup + c++ registry refactoring

14 files changed, 230 insertions, 152 deletions

diff --git a/extension/src/openvic2/GameAdvancementHook.cpp b/extension/src/openvic2/GameAdvancementHook.cpp
index c78847b..4b9bc25 100644
--- a/extension/src/openvic2/GameAdvancementHook.cpp
+++ b/extension/src/openvic2/GameAdvancementHook.cpp
@@ -65,3 +65,8 @@ void GameAdvancementHook::conditionallyAdvanceGame() {
 	}
 	if (refreshFunction) refreshFunction();
 }
+
+void GameAdvancementHook::reset() {
+	isPaused = true;
+	currentSpeed = 0;
+}
diff --git a/extension/src/openvic2/GameAdvancementHook.hpp b/extension/src/openvic2/GameAdvancementHook.hpp
index 572494a..07f8414 100644
--- a/extension/src/openvic2/GameAdvancementHook.hpp
+++ b/extension/src/openvic2/GameAdvancementHook.hpp
@@ -21,12 +21,12 @@ namespace OpenVic2 {
 		//A function pointer that advances the simulation, intended to be a capturing lambda or something similar. May need to be reworked later
 		AdvancementFunction triggerFunction;
 		RefreshFunction refreshFunction;
+		speed_t currentSpeed;
 
 		public:
 		bool isPaused;
-		speed_t currentSpeed;
 
-		GameAdvancementHook(AdvancementFunction tickFunction, RefreshFunction updateFunction, bool startPaused = false, speed_t startingSpeed = 0);
+		GameAdvancementHook(AdvancementFunction tickFunction, RefreshFunction updateFunction, bool startPaused = true, speed_t startingSpeed = 0);
 
 		void setSimulationSpeed(speed_t speed);
 		speed_t getSimulationSpeed() const;
@@ -37,5 +37,6 @@ namespace OpenVic2 {
 		GameAdvancementHook& operator++();
 		GameAdvancementHook& operator--();
 		void conditionallyAdvanceGame();
+		void reset();
 	};
 }
\ No newline at end of file
diff --git a/extension/src/openvic2/GameManager.cpp b/extension/src/openvic2/GameManager.cpp
index da742ca..78992f1 100644
--- a/extension/src/openvic2/GameManager.cpp
+++ b/extension/src/openvic2/GameManager.cpp
@@ -5,7 +5,7 @@
 using namespace OpenVic2;
 
 GameManager::GameManager(state_updated_func_t state_updated_callback)
-	: clock{ [this]() { tick(); }, [this]() { update_state(); }, true }, today{ 1836 }, state_updated{ state_updated_callback } {}
+	: clock{ [this]() { tick(); }, [this]() { update_state(); } }, state_updated{ state_updated_callback } {}
 
 void GameManager::set_needs_update() {
 	needs_update = true;
@@ -27,8 +27,11 @@ void GameManager::tick() {
 	set_needs_update();
 }
 
-void GameManager::finished_loading_data() {
-	map.generate_province_buildings(building_manager);
+return_t GameManager::setup() {
+	clock.reset();
+	today = { 1836 };
+	set_needs_update();
+	return map.generate_province_buildings(building_manager);
 }
 
 Date const& GameManager::get_today() const {
diff --git a/extension/src/openvic2/GameManager.hpp b/extension/src/openvic2/GameManager.hpp
index cba0180..65cd566 100644
--- a/extension/src/openvic2/GameManager.hpp
+++ b/extension/src/openvic2/GameManager.hpp
@@ -13,7 +13,7 @@ namespace OpenVic2 {
 	private:
 		Date today;
 		state_updated_func_t state_updated;
-		bool needs_update = true;
+		bool needs_update;
 
 		void set_needs_update();
 		void update_state();
@@ -21,7 +21,7 @@ namespace OpenVic2 {
 	public:
 		GameManager(state_updated_func_t state_updated_callback);
 
-		void finished_loading_data();
+		return_t setup();
 
 		Date const& get_today() const;
 		return_t expand_building(Province::index_t province_index, std::string const& building_type_identifier);
diff --git a/extension/src/openvic2/Logger.cpp b/extension/src/openvic2/Logger.cpp
index f211e7e..56d74ab 100644
--- a/extension/src/openvic2/Logger.cpp
+++ b/extension/src/openvic2/Logger.cpp
@@ -7,9 +7,9 @@ using namespace OpenVic2;
 Logger::log_func_t Logger::info_func = [](std::string&& str) { std::cout << str; };
 Logger::log_func_t Logger::error_func = [](std::string&& str) { std::cerr << str; };
 
-const char* Logger::get_filename(const char* filepath) {
+char const* Logger::get_filename(char const* filepath) {
 	if (filepath == nullptr) return nullptr;
-	const char *last_slash = filepath;
+	char const* last_slash = filepath;
 	while (*filepath != '\0') {
 		if (*filepath == '\\' || *filepath == '/') last_slash = filepath + 1;
 		filepath++;
diff --git a/extension/src/openvic2/Logger.hpp b/extension/src/openvic2/Logger.hpp
index 749c67f..624df29 100644
--- a/extension/src/openvic2/Logger.hpp
+++ b/extension/src/openvic2/Logger.hpp
@@ -25,9 +25,9 @@ namespace OpenVic2 {
 			return source_location(f, l, n);
 		}
 
-		inline const char* file_name() const { return _file.c_str(); }
+		inline char const* file_name() const { return _file.c_str(); }
 		inline int line() const {return _line; }
-		inline const char* function_name() const { return _function.c_str(); }
+		inline char const* function_name() const { return _function.c_str(); }
 	};
 	#endif
 
@@ -42,7 +42,7 @@ namespace OpenVic2 {
 
 		static log_func_t info_func, error_func;
 
-		static const char* get_filename(const char* filepath);
+		static char const* get_filename(char const* filepath);
 
 		template <typename... Ts>
 		struct log {
diff --git a/extension/src/openvic2/Types.hpp b/extension/src/openvic2/Types.hpp
index b20db10..98e92ce 100644
--- a/extension/src/openvic2/Types.hpp
+++ b/extension/src/openvic2/Types.hpp
@@ -10,11 +10,91 @@ namespace OpenVic2 {
 	// This mirrors godot::Error, where `OK = 0` and `FAILED = 1`.
 	static constexpr return_t SUCCESS = false, FAILURE = true;
 
+	/*
+	 * Base class for objects with a non-empty string identifier,
+	 * uniquely named instances of which can be entered into an
+	 * IdentifierRegistry instance.
+	 */
 	class HasIdentifier {
-		std::string identifier;
+		const std::string identifier;
 	protected:
 		HasIdentifier(std::string const& new_identifier);
 	public:
+		HasIdentifier(HasIdentifier const&) = delete;
+		HasIdentifier(HasIdentifier&&) = default;
+		HasIdentifier& operator=(HasIdentifier const&) = delete;
+		HasIdentifier& operator=(HasIdentifier&&) = delete;
+
 		std::string const& get_identifier() const;
 	};
+
+	/*
+	 * Template for a list of objects with unique string identifiers that can
+	 * be locked to prevent any further additions. The template argument T is
+	 * the type of object that the registry will store, and the second part ensures
+	 * that HasIdentifier is a base class of T.
+	 */
+	template<class T, typename std::enable_if<std::is_base_of<HasIdentifier, T>::value>::type* = nullptr>
+	class IdentifierRegistry {
+		const std::string name;
+		std::vector<T> items;
+		bool locked = false;
+	public:
+		IdentifierRegistry(std::string const& new_name) : name(new_name) {}
+		return_t add_item(T&& item) {
+			if (locked) {
+				Logger::error("Cannot add item to the ", name, " registry - locked!");
+				return FAILURE;
+			}
+			T const* old_item = get_item_by_identifier(item.get_identifier());
+			if (old_item != nullptr) {
+				Logger::error("Cannot add item to the ", name, " registry - an item with the identifier \"", item.get_identifier(), "\" already exists!");
+				return FAILURE;
+			}
+			items.push_back(std::move(item));
+			return SUCCESS;
+		}
+		void lock(bool log = true) {
+			if (locked) {
+				Logger::error("Failed to lock ", name, " registry - already locked!");
+			} else {
+				locked = true;
+				if (log) Logger::info("Locked ", name, " registry after registering ", get_item_count(), " items");
+			}
+		}
+		bool is_locked() const {
+			return locked;
+		}
+		void reset() {
+			items.clear();
+			locked = false;
+		}
+		size_t get_item_count() const {
+			return items.size();
+		}
+		T* get_item_by_identifier(std::string const& identifier) {
+			if (!identifier.empty())
+				for (T& item : items)
+					if (item.get_identifier() == identifier) return &item;
+			return nullptr;
+		}
+		T const* get_item_by_identifier(std::string const& identifier) const {
+			if (!identifier.empty())
+				for (T const& item : items)
+					if (item.get_identifier() == identifier) return &item;
+			return nullptr;
+		}
+		T* get_item_by_index(size_t index) {
+			return index < items.size() ? &items[index] : nullptr;
+		}
+		T const* get_item_by_index(size_t index) const {
+			return index < items.size() ? &items[index] : nullptr;
+		}
+		std::vector<T>& get_items() {
+			return items;
+		}
+		std::vector<T> const& get_items() const {
+			return items;
+		}
+	};
 }
diff --git a/extension/src/openvic2/map/Building.cpp b/extension/src/openvic2/map/Building.cpp
index f453a0f..00e121b 100644
--- a/extension/src/openvic2/map/Building.cpp
+++ b/extension/src/openvic2/map/Building.cpp
@@ -3,10 +3,11 @@
 #include <cassert>
 
 #include "openvic2/Logger.hpp"
+#include "openvic2/map/Province.hpp"
 
 using namespace OpenVic2;
 
-Building::Building(BuildingType const& new_type) : type{ new_type } {}
+Building::Building(BuildingType const& new_type) : HasIdentifier{ new_type.get_identifier() }, type{ new_type } {}
 
 bool Building::_can_expand() const {
 	return level < type.get_max_level();
@@ -45,6 +46,9 @@ return_t Building::expand() {
 	return FAILURE;
 }
 
+/* REQUIREMENTS:
+ * MAP-71, MAP-74, MAP-77
+ */
 void Building::update_state(Date const& today) {
 	switch (expansion_state) {
 		case ExpansionState::Preparing:
@@ -84,13 +88,11 @@ Timespan BuildingType::get_build_time() const {
 	return build_time;
 }
 
+BuildingManager::BuildingManager() : building_types{ "building types" } {}
+
 return_t BuildingManager::add_building_type(std::string const& identifier, Building::level_t max_level, Timespan build_time) {
-	if (building_types_locked) {
-		Logger::error("The building type list has already been locked!");
-		return FAILURE;
-	}
 	if (identifier.empty()) {
-		Logger::error("Empty building type identifier!");
+		Logger::error("Invalid building type identifier - empty!");
 		return FAILURE;
 	}
 	if (max_level < 0) {
@@ -101,29 +103,22 @@ return_t BuildingManager::add_building_type(std::string const& identifier, Build
 		Logger::error("Invalid building type build time: ", build_time);
 		return FAILURE;
 	}
-	BuildingType new_building_type{ identifier, max_level, build_time };
-	BuildingType const* old_building_type = get_building_type_by_identifier(identifier);
-	if (old_building_type != nullptr) {
-		Logger::error("Duplicate building type identifiers: ", old_building_type->get_identifier(), " and ", identifier);
-		return FAILURE;
-	}
-	building_types.push_back(new_building_type);
-	return SUCCESS;
+	return building_types.add_item({ identifier, max_level, build_time });
 }
 
 void BuildingManager::lock_building_types() {
-	building_types_locked = true;
-	Logger::info("Locked building types after registering ", building_types.size());
+	building_types.lock();
 }
 
 BuildingType const* BuildingManager::get_building_type_by_identifier(std::string const& identifier) const {
-	if (!identifier.empty())
-		for (BuildingType const& building_type : building_types)
-			if (building_type.get_identifier() == identifier) return &building_type;
-	return nullptr;
+	return building_types.get_item_by_identifier(identifier);
 }
 
-void BuildingManager::generate_province_buildings(std::vector<Building>& buildings) const {
-	for (BuildingType const& type : building_types)
-		buildings.push_back(Building{ type });
+return_t BuildingManager::generate_province_buildings(Province& province) const {
+	return_t ret = SUCCESS;
+	province.reset_buildings();
+	for (BuildingType const& type : building_types.get_items())
+		if (province.add_building(type) != SUCCESS) ret = FAILURE;
+	province.lock_buildings();
+	return ret;
 }
diff --git a/extension/src/openvic2/map/Building.hpp b/extension/src/openvic2/map/Building.hpp
index 7a1a777..1305014 100644
--- a/extension/src/openvic2/map/Building.hpp
+++ b/extension/src/openvic2/map/Building.hpp
@@ -6,11 +6,16 @@
 #include "openvic2/Date.hpp"
 
 namespace OpenVic2 {
-	struct BuildingManager;
+	struct Province;
 	struct BuildingType;
 
-	struct Building {
-		friend struct BuildingManager;
+	/* REQUIREMENTS:
+	 * MAP-11, MAP-72, MAP-73
+	 * MAP-12, MAP-75, MAP-76
+	 * MAP-13, MAP-78, MAP-79
+	 */
+	struct Building : HasIdentifier {
+		friend struct Province;
 
 		using level_t = int8_t;
 
@@ -26,6 +31,8 @@ namespace OpenVic2 {
 
 		bool _can_expand() const;
 	public:
+		Building(Building&&) = default;
+
 		BuildingType const& get_type() const;
 		level_t get_level() const;
 		ExpansionState get_expansion_state() const;
@@ -38,26 +45,31 @@ namespace OpenVic2 {
 		void tick(Date const& today);
 	};
 
+	struct BuildingManager;
+
 	struct BuildingType : HasIdentifier {
 		friend struct BuildingManager;
 	private:
-		Building::level_t max_level;
-		Timespan build_time;
+		const Building::level_t max_level;
+		const Timespan build_time;
 
 		BuildingType(std::string const& new_identifier, Building::level_t new_max_level, Timespan new_build_time);
 	public:
+		BuildingType(BuildingType&&) = default;
+
 		Building::level_t get_max_level() const;
 		Timespan get_build_time() const;
 	};
 
 	struct BuildingManager {
 	private:
-		std::vector<BuildingType> building_types;
-		bool building_types_locked = false;
+		IdentifierRegistry<BuildingType> building_types;
 	public:
+		BuildingManager();
+
 		return_t add_building_type(std::string const& identifier, Building::level_t max_level, Timespan build_time);
 		void lock_building_types();
 		BuildingType const* get_building_type_by_identifier(std::string const& identifier) const;
-		void generate_province_buildings(std::vector<Building>& buildings) const;
+		return_t generate_province_buildings(Province& province) const;
 	};
 }
diff --git a/extension/src/openvic2/map/Map.cpp b/extension/src/openvic2/map/Map.cpp
index 1089d61..b5cf144 100644
--- a/extension/src/openvic2/map/Map.cpp
+++ b/extension/src/openvic2/map/Map.cpp
@@ -16,45 +16,36 @@ Mapmode::index_t Mapmode::get_index() const {
 	return index;
 }
 
-Mapmode::colour_func_t Mapmode::get_colour_func() const {
-	return colour_func;
+Province::colour_t Mapmode::get_colour(Map const& map, Province const& province) const {
+	return colour_func ? colour_func(map, province) : Province::NULL_COLOUR;
 }
 
+Map::Map() : provinces{ "provinces" }, regions{ "regions" }, mapmodes{ "mapmodes" } {}
+
 return_t Map::add_province(std::string const& identifier, Province::colour_t colour) {
-	if (provinces_locked) {
-		Logger::error("The map's province list has already been locked!");
-		return FAILURE;
-	}
-	if (provinces.size() >= Province::MAX_INDEX) {
+	if (provinces.get_item_count() >= Province::MAX_INDEX) {
 		Logger::error("The map's province list is full - there can be at most ", Province::MAX_INDEX, " provinces");
 		return FAILURE;
 	}
 	if (identifier.empty()) {
-		Logger::error("Empty province identifier for colour ", Province::colour_to_hex_string(colour));
+		Logger::error("Invalid province identifier - empty!");
 		return FAILURE;
 	}
 	if (colour == Province::NULL_COLOUR || colour > Province::MAX_COLOUR) {
 		Logger::error("Invalid province colour: ", Province::colour_to_hex_string(colour));
 		return FAILURE;
 	}
-	Province new_province{ static_cast<Province::index_t>(provinces.size() + 1), identifier, colour };
-	Province const* old_province = get_province_by_identifier(identifier);
-	if (old_province != nullptr) {
-		Logger::error("Duplicate province identifiers: ", old_province->to_string(), " and ", new_province.to_string());
-		return FAILURE;
-	}
-	old_province = get_province_by_colour(colour);
+	Province new_province{ static_cast<Province::index_t>(provinces.get_item_count() + 1), identifier, colour };
+	Province const* old_province = get_province_by_colour(colour);
 	if (old_province != nullptr) {
 		Logger::error("Duplicate province colours: ", old_province->to_string(), " and ", new_province.to_string());
 		return FAILURE;
 	}
-	provinces.push_back(new_province);
-	return SUCCESS;
+	return provinces.add_item(std::move(new_province));
 }
 
 void Map::lock_provinces() {
-	provinces_locked = true;
-	Logger::info("Locked provinces after registering ", provinces.size());
+	provinces.lock();
 }
 
 return_t Map::set_water_province(std::string const& identifier) {
@@ -82,20 +73,12 @@ void Map::lock_water_provinces() {
 }
 
 return_t Map::add_region(std::string const& identifier, std::vector<std::string> const& province_identifiers) {
-	if (regions_locked) {
-		Logger::error("The map's region list has already been locked!");
-		return FAILURE;
-	}
 	if (identifier.empty()) {
-		Logger::error("Empty region identifier!");
-		return FAILURE;
-	}
-	if (provinces.empty()) {
-		Logger::error("Empty province list for region ", identifier);
+		Logger::error("Invalid region identifier - empty!");
 		return FAILURE;
 	}
-	return_t ret = SUCCESS;
 	Region new_region{ identifier };
+	return_t ret = SUCCESS;
 	for (std::string const& province_identifier : province_identifiers) {
 		Province* province = get_province_by_identifier(province_identifier);
 		if (province != nullptr) {
@@ -106,8 +89,8 @@ return_t Map::add_region(std::string const& identifier, std::vector<std::string>
 				size_t other_region_index = reinterpret_cast<size_t>(province->get_region());
 				if (other_region_index != 0) {
 					other_region_index--;
-					if (other_region_index < regions.size())
-						Logger::error("Province ", province_identifier, " is already part of ", regions[other_region_index].get_identifier());
+					if (other_region_index < regions.get_item_count())
+						Logger::error("Province ", province_identifier, " is already part of ", regions.get_item_by_index(other_region_index)->get_identifier());
 					else
 						Logger::error("Province ", province_identifier, " is already part of an unknown region with index ", other_region_index);
 					ret = FAILURE;
@@ -122,65 +105,53 @@ return_t Map::add_region(std::string const& identifier, std::vector<std::string>
 		Logger::error("No valid provinces in region's list");
 		return FAILURE;
 	}
-	Region const* old_region = get_region_by_identifier(identifier);
-	if (old_region != nullptr) {
-		Logger::error("Duplicate region identifiers: ", old_region->get_identifier(), " and ", identifier);
-		return FAILURE;
-	}
 
-	regions.push_back(new_region);
 	// Used to detect provinces listed in multiple regions, will
 	// be corrected once regions is stable (i.e. lock_regions).
-	Region* tmp_region_index = reinterpret_cast<Region*>(regions.size());
+	Region* tmp_region_index = reinterpret_cast<Region*>(regions.get_item_count());
 	for (Province* province : new_region.get_provinces())
 		province->region = tmp_region_index;
+	if (regions.add_item(std::move(new_region)) != SUCCESS) ret = FAILURE;
 	return ret;
 }
 
 void Map::lock_regions() {
-	regions_locked = true;
-	for (Region& region : regions)
+	regions.lock();
+	for (Region& region : regions.get_items())
 		for (Province* province : region.get_provinces())
 			province->region = &region;
-	Logger::info("Locked regions after registering ", regions.size());
 }
 
 size_t Map::get_province_count() const {
-	return provinces.size();
+	return provinces.get_item_count();
 }
 
 Province* Map::get_province_by_index(Province::index_t index) {
-	return index != Province::NULL_INDEX && index <= provinces.size() ? &provinces[index - 1] : nullptr;
+	return index != Province::NULL_INDEX ? provinces.get_item_by_index(index - 1) : nullptr;
 }
 
 Province const* Map::get_province_by_index(Province::index_t index) const {
-	return index != Province::NULL_INDEX && index <= provinces.size() ? &provinces[index - 1] : nullptr;
+	return index != Province::NULL_INDEX ? provinces.get_item_by_index(index - 1) : nullptr;
 }
 
 Province* Map::get_province_by_identifier(std::string const& identifier) {
-	if (!identifier.empty())
-		for (Province& province : provinces)
-			if (province.get_identifier() == identifier) return &province;
-	return nullptr;
+	return provinces.get_item_by_identifier(identifier);
 }
 
 Province const* Map::get_province_by_identifier(std::string const& identifier) const {
-	if (!identifier.empty())
-		for (Province const& province : provinces)
-			if (province.get_identifier() == identifier) return &province;
-	return nullptr;
+	return provinces.get_item_by_identifier(identifier);
 }
 
 Province* Map::get_province_by_colour(Province::colour_t colour) {
 	if (colour != Province::NULL_COLOUR)
-		for (Province& province : provinces)
+		for (Province& province : provinces.get_items())
 			if (province.get_colour() == colour) return &province;
 	return nullptr;
 }
 
 Province const* Map::get_province_by_colour(Province::colour_t colour) const {
 	if (colour != Province::NULL_COLOUR)
-		for (Province const& province : provinces)
+		for (Province const& province : provinces.get_items())
 			if (province.get_colour() == colour) return &province;
 	return nullptr;
 }
@@ -191,17 +162,11 @@ Province::index_t Map::get_province_index_at(size_t x, size_t y) const {
 }
 
 Region* Map::get_region_by_identifier(std::string const& identifier) {
-	if (!identifier.empty())
-		for (Region& region : regions)
-			if (region.get_identifier() == identifier) return &region;
-	return nullptr;
+	return regions.get_item_by_identifier(identifier);
 }
 
 Region const* Map::get_region_by_identifier(std::string const& identifier) const {
-	if (!identifier.empty())
-		for (Region const& region : regions)
-			if (region.get_identifier() == identifier) return &region;
-	return nullptr;
+	return regions.get_item_by_identifier(identifier);
 }
 
 static Province::colour_t colour_at(uint8_t const* colour_data, int32_t idx) {
@@ -213,7 +178,7 @@ return_t Map::generate_province_index_image(size_t new_width, size_t new_height,
 		Logger::error("Province index image has already been generated!");
 		return FAILURE;
 	}
-	if (!provinces_locked) {
+	if (!provinces.is_locked()) {
 		Logger::error("Province index image cannot be generated until after provinces are locked!");
 		return FAILURE;
 	}
@@ -229,7 +194,7 @@ return_t Map::generate_province_index_image(size_t new_width, size_t new_height,
 	height = new_height;
 	province_index_image.resize(width * height);
 
-	std::vector<bool> province_checklist(provinces.size());
+	std::vector<bool> province_checklist(provinces.get_item_count());
 	return_t ret = SUCCESS;
 	std::unordered_set<Province::colour_t> unrecognised_colours;
 
@@ -269,7 +234,7 @@ return_t Map::generate_province_index_image(size_t new_width, size_t new_height,
 
 	for (size_t idx = 0; idx < province_checklist.size(); ++idx) {
 		if (!province_checklist[idx]) {
-			Logger::error("Province missing from shape image: ", provinces[idx].to_string());
+			Logger::error("Province missing from shape image: ", provinces.get_item_by_index(idx)->to_string());
 			ret = FAILURE;
 		}
 	}
@@ -290,36 +255,30 @@ std::vector<Province::index_t> const& Map::get_province_index_image() const {
 
 return_t Map::add_mapmode(std::string const& identifier, Mapmode::colour_func_t colour_func) {
 	if (identifier.empty()) {
-		Logger::error("Empty mapmode identifier!");
+		Logger::error("Invalid mapmode identifier - empty!");
 		return FAILURE;
 	}
 	if (colour_func == nullptr) {
 		Logger::error("Mapmode colour function is null for identifier: ", identifier);
 		return FAILURE;
 	}
-	Mapmode new_mapmode{ mapmodes.size(), identifier, colour_func };
-	Mapmode const* old_mapmode = get_mapmode_by_identifier(identifier);
-	if (old_mapmode != nullptr) {
-		Logger::error("Duplicate mapmode identifiers: ", old_mapmode->get_identifier(), " and ", identifier);
-		return FAILURE;
-	}
-	mapmodes.push_back(new_mapmode);
-	return SUCCESS;
+	return mapmodes.add_item({ mapmodes.get_item_count(), identifier, colour_func });
+}
+
+void Map::lock_mapmodes() {
+	mapmodes.lock();
 }
 
 size_t Map::get_mapmode_count() const {
-	return mapmodes.size();
+	return mapmodes.get_item_count();
 }
 
 Mapmode const* Map::get_mapmode_by_index(size_t index) const {
-	return index < mapmodes.size() ? &mapmodes[index] : nullptr;
+	return mapmodes.get_item_by_index(index);
 }
 
 Mapmode const* Map::get_mapmode_by_identifier(std::string const& identifier) const {
-	if (!identifier.empty())
-		for (Mapmode const& mapmode : mapmodes)
-			if (mapmode.get_identifier() == identifier) return &mapmode;
-	return nullptr;
+	return mapmodes.get_item_by_identifier(identifier);
 }
 
 return_t Map::generate_mapmode_colours(Mapmode::index_t index, uint8_t* target) const {
@@ -327,14 +286,14 @@ return_t Map::generate_mapmode_colours(Mapmode::index_t index, uint8_t* target)
 		Logger::error("Mapmode colour target pointer is null!");
 		return FAILURE;
 	}
-	if (index >= mapmodes.size()) {
+	Mapmode const* mapmode = mapmodes.get_item_by_index(index);
+	if (mapmode == nullptr) {
 		Logger::error("Invalid mapmode index: ", index);
 		return FAILURE;
 	}
-	Mapmode const& mapmode = mapmodes[index];
 	target += 4; // Skip past Province::NULL_INDEX
-	for (Province const& province : provinces) {
-		const Province::colour_t colour = mapmode.get_colour_func()(*this, province);
+	for (Province const& province : provinces.get_items()) {
+		const Province::colour_t colour = mapmode->get_colour(*this, province);
 		*target++ = (colour >> 16) & 0xFF;
 		*target++ = (colour >> 8) & 0xFF;
 		*target++ = colour & 0xFF;
@@ -343,17 +302,19 @@ return_t Map::generate_mapmode_colours(Mapmode::index_t index, uint8_t* target)
 	return SUCCESS;
 }
 
-void Map::generate_province_buildings(BuildingManager const& manager) {
-	for (Province& province : provinces)
-		manager.generate_province_buildings(province.buildings);
+return_t Map::generate_province_buildings(BuildingManager const& manager) {
+	return_t ret = SUCCESS;
+	for (Province& province : provinces.get_items())
+		if (manager.generate_province_buildings(province) != SUCCESS) ret = FAILURE;
+	return ret;
 }
 
 void Map::update_state(Date const& today) {
-	for (Province& province : provinces)
+	for (Province& province : provinces.get_items())
 		province.update_state(today);
 }
 
 void Map::tick(Date const& today) {
-	for (Province& province : provinces)
+	for (Province& province : provinces.get_items())
 		province.tick(today);
 }
diff --git a/extension/src/openvic2/map/Map.hpp b/extension/src/openvic2/map/Map.hpp
index b6e7ac2..ebc23be 100644
--- a/extension/src/openvic2/map/Map.hpp
+++ b/extension/src/openvic2/map/Map.hpp
@@ -12,13 +12,13 @@ namespace OpenVic2 {
 		using colour_func_t = std::function<Province::colour_t (Map const&, Province const&)>;
 		using index_t = size_t;
 	private:
-		index_t index;
-		colour_func_t colour_func;
+		const index_t index;
+		const colour_func_t colour_func;
 
 		Mapmode(index_t new_index, std::string const& new_identifier, colour_func_t new_colour_func);
 	public:
 		index_t get_index() const;
-		colour_func_t get_colour_func() const;
+		Province::colour_t get_colour(Map const& map, Province const& province) const;
 	};
 
 	/* REQUIREMENTS:
@@ -26,15 +26,17 @@ namespace OpenVic2 {
 	 */
 	struct Map {
 	private:
-		std::vector<Province> provinces;
-		std::vector<Region> regions;
-		bool provinces_locked = false, water_provinces_locked = false, regions_locked = false;
+		IdentifierRegistry<Province> provinces;
+		IdentifierRegistry<Region> regions;
+		IdentifierRegistry<Mapmode> mapmodes;
+		bool water_provinces_locked = false;
 		size_t water_province_count = 0;
 
 		size_t width = 0, height = 0;
 		std::vector<Province::index_t> province_index_image;
-		std::vector<Mapmode> mapmodes;
 	public:
+		Map();
+
 		return_t add_province(std::string const& identifier, Province::colour_t colour);
 		void lock_provinces();
 		return_t set_water_province(std::string const& identifier);
@@ -60,12 +62,13 @@ namespace OpenVic2 {
 		std::vector<Province::index_t> const& get_province_index_image() const;
 
 		return_t add_mapmode(std::string const& identifier, Mapmode::colour_func_t colour_func);
+		void lock_mapmodes();
 		size_t get_mapmode_count() const;
 		Mapmode const* get_mapmode_by_index(Mapmode::index_t index) const;
 		Mapmode const* get_mapmode_by_identifier(std::string const& identifier) const;
 		return_t generate_mapmode_colours(Mapmode::index_t index, uint8_t* target) const;
 
-		void generate_province_buildings(BuildingManager const& manager);
+		return_t generate_province_buildings(BuildingManager const& manager);
 
 		void update_state(Date const& today);
 		void tick(Date const& today);
diff --git a/extension/src/openvic2/map/Province.cpp b/extension/src/openvic2/map/Province.cpp
index c641b7e..4360bce 100644
--- a/extension/src/openvic2/map/Province.cpp
+++ b/extension/src/openvic2/map/Province.cpp
@@ -7,7 +7,7 @@
 using namespace OpenVic2;
 
 Province::Province(index_t new_index, std::string const& new_identifier, colour_t new_colour) :
-	HasIdentifier{ new_identifier }, index{ new_index }, colour{ new_colour } {
+	HasIdentifier{ new_identifier }, index{ new_index }, colour{ new_colour }, buildings{ "buildings" } {
 	assert(index != NULL_INDEX);
 	assert(colour != NULL_COLOUR);
 }
@@ -38,15 +38,26 @@ Province::life_rating_t Province::get_life_rating() const {
 	return life_rating;
 }
 
+return_t Province::add_building(BuildingType const& type) {
+	return buildings.add_item({ type });
+}
+
+void Province::lock_buildings() {
+	buildings.lock(false);
+}
+
+void Province::reset_buildings() {
+	buildings.reset();
+}
+
 std::vector<Building> const& Province::get_buildings() const {
-	return buildings;
+	return buildings.get_items();
 }
 
 return_t Province::expand_building(std::string const& building_type_identifier) {
-	for (Building& building : buildings)
-		if (building.get_type().get_identifier() == building_type_identifier)
-			return building.expand();
-	return FAILURE;
+	Building* building = buildings.get_item_by_identifier(building_type_identifier);
+	if (building == nullptr) return FAILURE;
+	return building->expand();
 }
 
 std::string Province::to_string() const {
@@ -56,12 +67,12 @@ std::string Province::to_string() const {
 }
 
 void Province::update_state(Date const& today) {
-	for (Building& building : buildings)
+	for (Building& building : buildings.get_items())
 		building.update_state(today);
 
 }
 
 void Province::tick(Date const& today) {
-	for (Building& building : buildings)
+	for (Building& building : buildings.get_items())
 		building.tick(today);
 }
diff --git a/extension/src/openvic2/map/Province.hpp b/extension/src/openvic2/map/Province.hpp
index 79c6861..aa0329c 100644
--- a/extension/src/openvic2/map/Province.hpp
+++ b/extension/src/openvic2/map/Province.hpp
@@ -19,22 +19,27 @@ namespace OpenVic2 {
 		static constexpr colour_t NULL_COLOUR = 0, MAX_COLOUR = 0xFFFFFF;
 		static constexpr index_t NULL_INDEX = 0, MAX_INDEX = 0xFFFF;
 	private:
-		index_t index;
-		colour_t colour;
+		const index_t index;
+		const colour_t colour;
 		Region* region = nullptr;
 		bool water = false;
 		life_rating_t life_rating = 0;
-		std::vector<Building> buildings;
+		IdentifierRegistry<Building> buildings;
 
 		Province(index_t new_index, std::string const& new_identifier, colour_t new_colour);
 	public:
 		static std::string colour_to_hex_string(colour_t colour);
 
+		Province(Province&&) = default;
+
 		index_t get_index() const;
 		colour_t get_colour() const;
 		Region* get_region() const;
 		bool is_water() const;
 		life_rating_t get_life_rating() const;
+		return_t add_building(BuildingType const& type);
+		void lock_buildings();
+		void reset_buildings();
 		std::vector<Building> const& get_buildings() const;
 		return_t expand_building(std::string const& building_type_identifier);
 		std::string to_string() const;
diff --git a/extension/src/openvic2/map/Region.hpp b/extension/src/openvic2/map/Region.hpp
index 2eec1cd..04564fc 100644
--- a/extension/src/openvic2/map/Region.hpp
+++ b/extension/src/openvic2/map/Region.hpp
@@ -23,6 +23,8 @@ namespace OpenVic2 {
 	private:
 		Region(std::string const& new_identifier);
 	public:
+		Region(Region&&) = default;
+
 		Province::colour_t get_colour() const;
 	};
 }