Store RiverType by value, handle by reference

client/mapHandler.cpp

@@ -180,9 +180,9 @@ void CMapHandler::initTerrainGraphics()
 	{
 		terrainFiles[terrain.name] = terrain.tilesFilename;
 	}
-	for(const auto * river : VLC->terrainTypeHandler->rivers())
+	for(const auto & river : VLC->terrainTypeHandler->rivers())
 	{
-		riverFiles[river->fileName] = river->fileName;
+		riverFiles[river.fileName] = river.fileName;
 	}
 	for(const auto * road : VLC->terrainTypeHandler->roads())
 	{

lib/GameConstants.h

@@ -871,6 +871,7 @@ namespace River
 	enum ERiver : ui8
 	{
 		NO_RIVER = 0,
+		FIRST_REGULAR_RIVER = 1,
 		WATER_RIVER = 1,
 		ICY_RIVER = 2,
 		MUD_RIVER = 3,

lib/Terrain.cpp

@@ -31,10 +31,6 @@ TerrainTypeHandler::TerrainTypeHandler()
 
 TerrainTypeHandler::~TerrainTypeHandler()
 {
-	for (const auto * river : riverTypes)
-	{
-		delete river;
-	}
 	for (const auto * road : roadTypes)
 	{
 		delete road;
@@ -217,8 +213,8 @@ void TerrainTypeHandler::initTerrains(const std::vector<std::string> & allConfig
 
 void TerrainTypeHandler::initRivers(const std::vector<std::string> & allConfigs)
 {
-	riverTypes.resize(River::ORIGINAL_RIVER_COUNT, nullptr); //make space for original rivers
-	riverTypes[River::NO_RIVER] = new RiverType(); //default
+	riverTypes.resize(River::ORIGINAL_RIVER_COUNT); //make space for original rivers
+	//First object will be default
 
 	for (auto & mod : allConfigs)
 	{
@@ -228,21 +224,21 @@ void TerrainTypeHandler::initRivers(const std::vector<std::string> & allConfigs)
 		JsonNode rivs(mod, ResourceID("config/rivers.json"));
 		for (auto & river : rivs.Struct())
 		{
-			auto * info = new RiverType();
+			RiverType info;
 
-			info->fileName = river.second["animation"].String();
-			info->code = river.second["code"].String();
-			info->deltaName = river.second["delta"].String();
+			info.fileName = river.second["animation"].String();
+			info.code = river.second["code"].String();
+			info.deltaName = river.second["delta"].String();
 			//info->movementCost = river.second["moveCost"].Integer();
 
 			if (!river.second["originalRiverId"].isNull())
 			{
-				info->id = static_cast<TRiverId>(river.second["originalRiverId"].Float());
-				riverTypes[info->id] = info;
+				info.id = static_cast<TRiverId>(river.second["originalRiverId"].Float());
+				riverTypes[info.id] = info;
 			}
 			else
 			{
-				info->id = static_cast<TRiverId>(riverTypes.size());
+				info.id = static_cast<TRiverId>(riverTypes.size());
 				riverTypes.push_back(info);
 			}
 		}
@@ -302,10 +298,9 @@ void TerrainTypeHandler::recreateTerrainMaps()
 
 void TerrainTypeHandler::recreateRiverMaps()
 {
-	for (const RiverType * riverInfo : riverTypes)
+	for (size_t i = River::FIRST_REGULAR_RIVER ; i < riverTypes.size(); i++)
 	{
-		if (riverInfo->id == River::NO_RIVER)
-			continue;
+		const auto* riverInfo = &riverTypes[i];
 
 		riverInfoByName[riverInfo->fileName] = riverInfo;
 		riverInfoByCode[riverInfo->code] = riverInfo;
@@ -328,11 +323,11 @@ void TerrainTypeHandler::recreateRoadMaps()
 
 const std::vector<TerrainType> & TerrainTypeHandler::terrains() const
 {
-	//FIXME: somehow make it non-copyable? Pointers must point t original data and not its copy
+	//FIXME: somehow make it non-copyable? Pointers must point to original data and not its copy
 	return objects;
 }
 
-const std::vector<RiverType*>& TerrainTypeHandler::rivers() const
+const std::vector<RiverType>& TerrainTypeHandler::rivers() const
 {
 	return riverTypes;
 }
@@ -487,6 +482,16 @@ RiverType::RiverType(const std::string & fileName, const std::string & code, TRi
 {
 }
 
+RiverType& RiverType::operator=(const RiverType& other)
+{
+	fileName = other.fileName;
+	code = other.code;
+	deltaName = other.deltaName;
+	id = other.id;
+
+	return *this;
+}
+
 RoadType::RoadType(const std::string& fileName, const std::string& code, TRoadId id):
 	fileName(fileName),
 	code(code),

lib/Terrain.h

@@ -49,7 +49,7 @@ public:
 	
 	TerrainType(const std::string & name = "");
 
-	TerrainType& operator=(const TerrainType & _type);
+	TerrainType& operator=(const TerrainType & other);
 	
 	bool operator==(const TerrainType & other);
 	bool operator!=(const TerrainType & other);
@@ -98,6 +98,8 @@ public:
 
 	RiverType(const std::string & fileName = "", const std::string & code = "", TRiverId id = River::NO_RIVER);
 
+	RiverType& operator=(const RiverType & other);
+
 	template <typename Handler> void serialize(Handler& h, const int version)
 	{
 		h & fileName;
@@ -140,7 +142,7 @@ public:
 	const TerrainType * getInfoByCode(const std::string & terrainCode) const;
 	const TerrainType * getInfoById(TTerrainId id) const;
 
-	const std::vector<RiverType *> & rivers() const;
+	const std::vector<RiverType> & rivers() const;
 	const RiverType * getRiverByName(const std::string & riverName) const;
 	const RiverType * getRiverByCode(const std::string & riverCode) const;
 	const RiverType * getRiverById(TRiverId id) const;
@@ -167,7 +169,7 @@ public:
 private:
 
 	std::vector<TerrainType> objects;
-	std::vector<RiverType *> riverTypes;
+	std::vector<RiverType> riverTypes;
 	std::vector<RoadType *> roadTypes;
 
 	std::unordered_map<std::string, const TerrainType*> terrainInfoByName;

lib/mapping/CDrawRoadsOperation.cpp

@@ -341,7 +341,7 @@ void CDrawRoadsOperation::executeTile(TerrainTile & tile)
 
 void CDrawRiversOperation::executeTile(TerrainTile & tile)
 {
-	tile.riverType = VLC->terrainTypeHandler->rivers()[riverType];
+	tile.riverType = const_cast<RiverType*>(&VLC->terrainTypeHandler->rivers()[riverType]);
 }
 
 bool CDrawRoadsOperation::canApplyPattern(const LinePattern & pattern) const

lib/mapping/CMap.cpp

@@ -126,7 +126,7 @@ CCastleEvent::CCastleEvent() : town(nullptr)
 TerrainTile::TerrainTile():
 	terType(nullptr),
 	terView(0),
-	riverType(VLC->terrainTypeHandler->rivers()[0]),
+	riverType(const_cast<RiverType*>(&VLC->terrainTypeHandler->rivers()[0])),
 	riverDir(0),
 	roadType(VLC->terrainTypeHandler->roads()[0]),
 	roadDir(0),

lib/mapping/MapFormatH3M.cpp

@@ -937,7 +937,7 @@ void CMapLoaderH3M::readTerrain()
 				auto & tile = map->getTile(pos);
 				tile.terType = const_cast<TerrainType*>(&terrains[reader.readUInt8()]);
 				tile.terView = reader.readUInt8();
-				tile.riverType = rivers[reader.readUInt8()];
+				tile.riverType = const_cast<RiverType*>(&rivers[reader.readUInt8()]);
 				tile.riverDir = reader.readUInt8();
 				tile.roadType = roads[reader.readUInt8()];
 				tile.roadDir = reader.readUInt8();

lib/rmg/RiverPlacer.cpp

@@ -321,8 +321,8 @@ void RiverPlacer::preprocess()
 void RiverPlacer::connectRiver(const int3 & tile)
 {
 	auto riverType = VLC->terrainTypeHandler->terrains()[zone.getTerrainType()].river;
-	auto river = VLC->terrainTypeHandler->rivers()[riverType];
-	if(river->id == River::NO_RIVER)
+	const auto & river = VLC->terrainTypeHandler->rivers()[riverType];
+	if(river.id == River::NO_RIVER)
 		return;
 	
 	rmg::Area roads;
@@ -379,9 +379,10 @@ void RiverPlacer::connectRiver(const int3 & tile)
 		if(tmplates.size() > 3)
 		{
 			if(tmplates.size() % 4 != 0)
-				throw rmgException(boost::to_string(boost::format("River templates for (%d,%d) at terrain %s, river %s are incorrect") % RIVER_DELTA_ID % RIVER_DELTA_SUBTYPE % zone.getTerrainType() % river));
+				throw rmgException(boost::to_string(boost::format("River templates for (%d,%d) at terrain %s, river %s are incorrect") %
+					RIVER_DELTA_ID % RIVER_DELTA_SUBTYPE % zone.getTerrainType() % river.code));
 			
-			std::string targetTemplateName = river->deltaName + std::to_string(deltaOrientations[pos]) + ".def";
+			std::string targetTemplateName = river.deltaName + std::to_string(deltaOrientations[pos]) + ".def";
 			for(auto & templ : tmplates)
 			{
 				if(templ->animationFile == targetTemplateName)