vcmi: modernize lib/mapping

lib/mapping/CCampaignHandler.cpp

@@ -32,30 +32,12 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-CCampaignHeader::CCampaignHeader()
-	: version(0), mapVersion(0), difficultyChoosenByPlayer(0), music(0), filename(), loadFromLod(0)
-{
-
-}
-
-CScenarioTravel::STravelBonus::STravelBonus()
-	:type(SPELL), info1(0), info2(0), info3(0)
-{
-
-}
-
 bool CScenarioTravel::STravelBonus::isBonusForHero() const
 {
 	return type == SPELL || type == MONSTER || type == ARTIFACT || type == SPELL_SCROLL || type == PRIMARY_SKILL
 		|| type == SECONDARY_SKILL;
 }
 
-CScenarioTravel::CScenarioTravel()
-	:whatHeroKeeps(0), startOptions(0), playerColor(0)
-{
-
-}
-
 CCampaignHeader CCampaignHandler::getHeader( const std::string & name)
 {
 	std::vector<ui8> cmpgn = getFile(name, true)[0];
@@ -98,27 +80,31 @@ std::unique_ptr<CCampaign> CCampaignHandler::getCampaign( const std::string & na
 
 		std::string scenarioName = name.substr(0, name.find('.'));
 		boost::to_lower(scenarioName);
-		scenarioName += ':' + boost::lexical_cast<std::string>(g-1);
+		scenarioName += ':' + std::to_string(g - 1);
 
 		//set map piece appropriately, convert vector to string
 		ret->mapPieces[scenarioID].assign(reinterpret_cast< const char* >(file[g].data()), file[g].size());
 		CMapService mapService;
-		ret->scenarios[scenarioID].scenarioName = mapService.loadMapHeader((const ui8*)ret->mapPieces[scenarioID].c_str(), (int)ret->mapPieces[scenarioID].size(), scenarioName)->name;
+		auto hdr = mapService.loadMapHeader(
+			reinterpret_cast<const ui8 *>(ret->mapPieces[scenarioID].c_str()),
+			static_cast<int>(ret->mapPieces[scenarioID].size()),
+			scenarioName);
+		ret->scenarios[scenarioID].scenarioName = hdr->name;
 		scenarioID++;
 	}
 
 	// handle campaign specific discrepancies
 	if(name == "DATA/AB.H3C")
 	{
-		ret->scenarios[6].keepHeroes.push_back(HeroTypeID(155)); // keep hero Xeron for map 7 To Kill A Hero
+		ret->scenarios[6].keepHeroes.emplace_back(155); // keep hero Xeron for map 7 To Kill A Hero
 	}
 	else if(name == "DATA/FINAL.H3C")
 	{
 		// keep following heroes for map 8 Final H
-		ret->scenarios[7].keepHeroes.push_back(HeroTypeID(148)); // Gelu
-		ret->scenarios[7].keepHeroes.push_back(HeroTypeID(27)); // Gem
-		ret->scenarios[7].keepHeroes.push_back(HeroTypeID(102)); // Crag Hack
-		ret->scenarios[7].keepHeroes.push_back(HeroTypeID(96)); // Yog
+		ret->scenarios[7].keepHeroes.emplace_back(148); // Gelu
+		ret->scenarios[7].keepHeroes.emplace_back(27); // Gem
+		ret->scenarios[7].keepHeroes.emplace_back(102); // Crag Hack
+		ret->scenarios[7].keepHeroes.emplace_back(96); // Yog
 	}
 
 	return ret;
@@ -357,33 +343,16 @@ bool CCampaign::conquerable( int whichScenario ) const
 	return true;
 }
 
-CCampaign::CCampaign()
-{
-
-}
-
-CCampaignScenario::SScenarioPrologEpilog::SScenarioPrologEpilog()
-	: hasPrologEpilog(false), prologVideo(0), prologMusic(0), prologText()
-{
-
-}
-
-CCampaignScenario::CCampaignScenario()
-	: mapName(), scenarioName(), packedMapSize(0), regionColor(0), difficulty(0), conquered(false)
-{
-
-}
-
 bool CCampaignScenario::isNotVoid() const
 {
-	return mapName.size() > 0;
+	return !mapName.empty();
 }
 
-const CGHeroInstance * CCampaignScenario::strongestHero(PlayerColor owner)
+const CGHeroInstance * CCampaignScenario::strongestHero(const PlayerColor & owner)
 {
 	std::function<bool(JsonNode & node)> isOwned = [owner](JsonNode & node)
 	{
-		auto h = CCampaignState::crossoverDeserialize(node);
+		auto * h = CCampaignState::crossoverDeserialize(node);
 		bool result = h->tempOwner == owner;
 		vstd::clear_pointer(h);
 		return result;
@@ -392,7 +361,7 @@ const CGHeroInstance * CCampaignScenario::strongestHero(PlayerColor owner)
 
 	auto i = vstd::maxElementByFun(ownedHeroes, [](JsonNode & node)
 	{
-		auto h = CCampaignState::crossoverDeserialize(node);
+		auto * h = CCampaignState::crossoverDeserialize(node);
 		double result = h->getHeroStrength();
 		vstd::clear_pointer(h);
 		return result;
@@ -407,10 +376,10 @@ std::vector<CGHeroInstance *> CCampaignScenario::getLostCrossoverHeroes()
 	{
 		for(auto node2 : placedCrossoverHeroes)
 		{
-			auto hero = CCampaignState::crossoverDeserialize(node2);
+			auto * hero = CCampaignState::crossoverDeserialize(node2);
 			auto it = range::find_if(crossoverHeroes, [hero](JsonNode node)
 			{
-				auto h = CCampaignState::crossoverDeserialize(node);
+				auto * h = CCampaignState::crossoverDeserialize(node);
 				bool result = hero->subID == h->subID;
 				vstd::clear_pointer(h);
 				return result;
@@ -463,11 +432,6 @@ ui8 CCampaignState::currentBonusID() const
 	return chosenCampaignBonuses.at(*currentMap);
 }
 
-CCampaignState::CCampaignState()
-{
-
-}
-
 CCampaignState::CCampaignState( std::unique_ptr<CCampaign> _camp ) : camp(std::move(_camp))
 {
 	for(int i = 0; i < camp->scenarios.size(); i++)
@@ -484,11 +448,11 @@ CMap * CCampaignState::getMap(int scenarioId) const
 		scenarioId = currentMap.get();
 	std::string scenarioName = camp->header.filename.substr(0, camp->header.filename.find('.'));
 	boost::to_lower(scenarioName);
-	scenarioName += ':' + boost::lexical_cast<std::string>(scenarioId);
+	scenarioName += ':' + std::to_string(scenarioId);
 	std::string & mapContent = camp->mapPieces.find(scenarioId)->second;
-	auto buffer = reinterpret_cast<const ui8 *>(mapContent.data());
+	const auto * buffer = reinterpret_cast<const ui8 *>(mapContent.data());
 	CMapService mapService;
-	return mapService.loadMap(buffer, (int)mapContent.size(), scenarioName).release();
+	return mapService.loadMap(buffer, static_cast<int>(mapContent.size()), scenarioName).release();
 }
 
 std::unique_ptr<CMapHeader> CCampaignState::getHeader(int scenarioId) const
@@ -498,11 +462,11 @@ std::unique_ptr<CMapHeader> CCampaignState::getHeader(int scenarioId) const
 
 	std::string scenarioName = camp->header.filename.substr(0, camp->header.filename.find('.'));
 	boost::to_lower(scenarioName);
-	scenarioName += ':' + boost::lexical_cast<std::string>(scenarioId);
+	scenarioName += ':' + std::to_string(scenarioId);
 	std::string & mapContent = camp->mapPieces.find(scenarioId)->second;
-	auto buffer = reinterpret_cast<const ui8 *>(mapContent.data());
+	const auto * buffer = reinterpret_cast<const ui8 *>(mapContent.data());
 	CMapService mapService;
-	return mapService.loadMapHeader(buffer, (int)mapContent.size(), scenarioName);
+	return mapService.loadMapHeader(buffer, static_cast<int>(mapContent.size()), scenarioName);
 }
 
 std::shared_ptr<CMapInfo> CCampaignState::getMapInfo(int scenarioId) const
@@ -528,7 +492,7 @@ JsonNode CCampaignState::crossoverSerialize(CGHeroInstance * hero)
 CGHeroInstance * CCampaignState::crossoverDeserialize(JsonNode & node)
 {
 	JsonDeserializer handler(nullptr, node);
-	auto hero = new CGHeroInstance();
+	auto * hero = new CGHeroInstance();
 	hero->ID = Obj::HERO;
 	hero->serializeJsonOptions(handler);
 	return hero;
@@ -546,7 +510,7 @@ std::string CCampaignHandler::prologVideoName(ui8 index)
 std::string CCampaignHandler::prologMusicName(ui8 index)
 {
 	std::vector<std::string> music;
-	return VLC->generaltexth->translate("core.cmpmusic." + std::to_string(int(index)));
+	return VLC->generaltexth->translate("core.cmpmusic." + std::to_string(static_cast<int>(index)));
 }
 
 std::string CCampaignHandler::prologVoiceName(ui8 index)

lib/mapping/CCampaignHandler.h

@@ -35,16 +35,14 @@ namespace CampaignVersion
 class DLL_LINKAGE CCampaignHeader
 {
 public:
-	si32 version; //4 - RoE, 5 - AB, 6 - SoD and WoG
-	ui8 mapVersion; //CampText.txt's format
+	si32 version = 0; //4 - RoE, 5 - AB, 6 - SoD and WoG
+	ui8 mapVersion = 0; //CampText.txt's format
 	std::string name, description;
-	ui8 difficultyChoosenByPlayer;
-	ui8 music; //CmpMusic.txt, start from 0
+	ui8 difficultyChoosenByPlayer = 0;
+	ui8 music = 0; //CmpMusic.txt, start from 0
 
 	std::string filename;
-	ui8 loadFromLod; //if true, this campaign must be loaded fro, .lod file
-
-	CCampaignHeader();
+	ui8 loadFromLod = 0; //if true, this campaign must be loaded fro, .lod file
 
 	template <typename Handler> void serialize(Handler &h, const int formatVersion)
 	{
@@ -62,25 +60,23 @@ public:
 class DLL_LINKAGE CScenarioTravel
 {
 public:
-	ui8 whatHeroKeeps; //bitfield [0] - experience, [1] - prim skills, [2] - sec skills, [3] - spells, [4] - artifacts
+	ui8 whatHeroKeeps = 0; //bitfield [0] - experience, [1] - prim skills, [2] - sec skills, [3] - spells, [4] - artifacts
 	std::array<ui8, 19> monstersKeptByHero;
 	std::array<ui8, 18> artifsKeptByHero;
 
-	ui8 startOptions; //1 - start bonus, 2 - traveling hero, 3 - hero options
+	ui8 startOptions = 0; //1 - start bonus, 2 - traveling hero, 3 - hero options
 
-	ui8 playerColor; //only for startOptions == 1
+	ui8 playerColor = 0; //only for startOptions == 1
 
 	struct DLL_LINKAGE STravelBonus
 	{
 		enum EBonusType {SPELL, MONSTER, BUILDING, ARTIFACT, SPELL_SCROLL, PRIMARY_SKILL, SECONDARY_SKILL, RESOURCE,
 			HEROES_FROM_PREVIOUS_SCENARIO, HERO};
-		EBonusType type; //uses EBonusType
-		si32 info1, info2, info3; //purpose depends on type
+		EBonusType type = EBonusType::SPELL; //uses EBonusType
+		si32 info1 = 0, info2 = 0, info3 = 0; //purpose depends on type
 
 		bool isBonusForHero() const;
 
-		STravelBonus();
-
 		template <typename Handler> void serialize(Handler &h, const int formatVersion)
 		{
 			h & type;
@@ -92,8 +88,6 @@ public:
 
 	std::vector<STravelBonus> bonusesToChoose;
 
-	CScenarioTravel();
-
 	template <typename Handler> void serialize(Handler &h, const int formatVersion)
 	{
 		h & whatHeroKeeps;
@@ -111,13 +105,11 @@ class DLL_LINKAGE CCampaignScenario
 public:
 	struct DLL_LINKAGE SScenarioPrologEpilog
 	{
-		bool hasPrologEpilog;
-		ui8 prologVideo; // from CmpMovie.txt
-		ui8 prologMusic; // from CmpMusic.txt
+		bool hasPrologEpilog = false;
+		ui8 prologVideo = 0; // from CmpMovie.txt
+		ui8 prologMusic = 0; // from CmpMusic.txt
 		std::string prologText;
 
-		SScenarioPrologEpilog();
-
 		template <typename Handler> void serialize(Handler &h, const int formatVersion)
 		{
 			h & hasPrologEpilog;
@@ -129,11 +121,11 @@ public:
 
 	std::string mapName; //*.h3m
 	std::string scenarioName; //from header. human-readble
-	ui32 packedMapSize; //generally not used
+	ui32 packedMapSize = 0; //generally not used
 	std::set<ui8> preconditionRegions; //what we need to conquer to conquer this one (stored as bitfield in h3c)
-	ui8 regionColor;
-	ui8 difficulty;
-	bool conquered;
+	ui8 regionColor = 0;
+	ui8 difficulty = 0;
+	bool conquered = false;
 
 	std::string regionText;
 	SScenarioPrologEpilog prolog, epilog;
@@ -146,11 +138,9 @@ public:
 	void loadPreconditionRegions(ui32 regions);
 	bool isNotVoid() const;
 	// FIXME: due to usage of JsonNode I can't make these methods const
-	const CGHeroInstance * strongestHero(PlayerColor owner);
+	const CGHeroInstance * strongestHero(const PlayerColor & owner);
 	std::vector<CGHeroInstance *> getLostCrossoverHeroes(); /// returns a list of crossover heroes which started the scenario, but didn't complete it
 
-	CCampaignScenario();
-
 	template <typename Handler> void serialize(Handler &h, const int formatVersion)
 	{
 		h & mapName;
@@ -185,8 +175,6 @@ public:
 	}
 
 	bool conquerable(int whichScenario) const;
-
-	CCampaign();
 };
 
 class DLL_LINKAGE CCampaignState
@@ -212,9 +200,8 @@ public:
 	static JsonNode crossoverSerialize(CGHeroInstance * hero);
 	static CGHeroInstance * crossoverDeserialize(JsonNode & node);
 
-	CCampaignState();
+	CCampaignState() = default;
 	CCampaignState(std::unique_ptr<CCampaign> _camp);
-	~CCampaignState(){};
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{

lib/mapping/CDrawRoadsOperation.cpp

@@ -153,9 +153,9 @@ static bool ruleIsAny(const std::string & rule)
 #endif
 
 ///CDrawLinesOperation
-CDrawLinesOperation::CDrawLinesOperation(CMap * map, const CTerrainSelection & terrainSel, CRandomGenerator * gen):
+CDrawLinesOperation::CDrawLinesOperation(CMap * map, CTerrainSelection terrainSel, CRandomGenerator * gen):
 	CMapOperation(map),
-	terrainSel(terrainSel),
+	terrainSel(std::move(terrainSel)),
 	gen(gen)
 {
 }
@@ -232,7 +232,7 @@ void CDrawLinesOperation::flipPattern(LinePattern& pattern, int flip) const
 
 void CDrawLinesOperation::updateTiles(std::set<int3> & invalidated)
 {
-	for(int3 coord : invalidated)
+	for(const int3 & coord : invalidated)
 	{
 		TerrainTile & tile = map->getTile(coord);
 		ValidationResult result(false);

lib/mapping/CDrawRoadsOperation.h

@@ -39,9 +39,9 @@ protected:
 		bool result;
 		int flip;
 	};
-	
-	CDrawLinesOperation(CMap * map, const CTerrainSelection & terrainSel, CRandomGenerator * gen);
-	
+
+	CDrawLinesOperation(CMap * map, CTerrainSelection terrainSel, CRandomGenerator * gen);
+
 	virtual void executeTile(TerrainTile & tile) = 0;
 	virtual bool canApplyPattern(const CDrawLinesOperation::LinePattern & pattern) const = 0;
 	virtual bool needUpdateTile(const TerrainTile & tile) const = 0;

lib/mapping/CMap.cpp

@@ -86,7 +86,7 @@ EventCondition::EventCondition(EWinLoseType condition):
 {
 }
 
-EventCondition::EventCondition(EWinLoseType condition, si32 value, si32 objectType, int3 position):
+EventCondition::EventCondition(EWinLoseType condition, si32 value, si32 objectType, const int3 & position):
 	object(nullptr),
 	metaType(EMetaclass::INVALID),
 	value(value),
@@ -237,10 +237,6 @@ CMapHeader::CMapHeader() : version(EMapFormat::SOD), height(72), width(72),
 	players.resize(PlayerColor::PLAYER_LIMIT_I);
 }
 
-CMapHeader::~CMapHeader()
-{
-}
-
 ui8 CMapHeader::levels() const
 {
 	return (twoLevel ? 2 : 1);
@@ -378,7 +374,7 @@ bool CMap::isCoastalTile(const int3 & pos) const
 	if(isWaterTile(pos))
 		return false;
 
-	for (auto & dir : dirs)
+	for(const auto & dir : dirs)
 	{
 		const int3 hlp = pos + dir;
 
@@ -394,15 +390,7 @@ bool CMap::isCoastalTile(const int3 & pos) const
 
 bool CMap::isInTheMap(const int3 & pos) const
 {
-	if(pos.x < 0 || pos.y < 0 || pos.z < 0 || pos.x >= width || pos.y >= height
-			|| pos.z > (twoLevel ? 1 : 0))
-	{
-		return false;
-	}
-	else
-	{
-		return true;
-	}
+	return pos.x >= 0 && pos.y >= 0 && pos.z >= 0 && pos.x < width && pos.y < height && pos.z <= (twoLevel ? 1 : 0);
 }
 
 TerrainTile & CMap::getTile(const int3 & tile)
@@ -428,11 +416,11 @@ bool CMap::canMoveBetween(const int3 &src, const int3 &dst) const
 	return checkForVisitableDir(src, dstTile, dst) && checkForVisitableDir(dst, srcTile, src);
 }
 
-bool CMap::checkForVisitableDir(const int3 & src, const TerrainTile *pom, const int3 & dst ) const
+bool CMap::checkForVisitableDir(const int3 & src, const TerrainTile * pom, const int3 & dst) const
 {
 	if (!pom->entrableTerrain()) //rock is never accessible
 		return false;
-	for (auto obj : pom->visitableObjects) //checking destination tile
+	for(auto * obj : pom->visitableObjects) //checking destination tile
 	{
 		if(!vstd::contains(pom->blockingObjects, obj)) //this visitable object is not blocking, ignore
 			continue;
@@ -496,7 +484,7 @@ int3 CMap::guardingCreaturePosition (int3 pos) const
 	return int3(-1, -1, -1);
 }
 
-const CGObjectInstance * CMap::getObjectiveObjectFrom(int3 pos, Obj::EObj type)
+const CGObjectInstance * CMap::getObjectiveObjectFrom(const int3 & pos, Obj::EObj type)
 {
 	for (CGObjectInstance * object : getTile(pos).visitableObjects)
 	{
@@ -506,7 +494,7 @@ const CGObjectInstance * CMap::getObjectiveObjectFrom(int3 pos, Obj::EObj type)
 	// There is weird bug because of which sometimes heroes will not be found properly despite having correct position
 	// Try to workaround that and find closest object that we can use
 
-	logGlobal->error("Failed to find object of type %d at %s", int(type), pos.toString());
+	logGlobal->error("Failed to find object of type %d at %s", static_cast<int>(type), pos.toString());
 	logGlobal->error("Will try to find closest matching object");
 
 	CGObjectInstance * bestMatch = nullptr;
@@ -544,12 +532,12 @@ void CMap::checkForObjectives()
 
 				case EventCondition::HAVE_CREATURES:
 					boost::algorithm::replace_first(event.onFulfill, "%s", VLC->creh->objects[cond.objectType]->getNameSingularTranslated());
-					boost::algorithm::replace_first(event.onFulfill, "%d", boost::lexical_cast<std::string>(cond.value));
+					boost::algorithm::replace_first(event.onFulfill, "%d", std::to_string(cond.value));
 					break;
 
 				case EventCondition::HAVE_RESOURCES:
 					boost::algorithm::replace_first(event.onFulfill, "%s", VLC->generaltexth->restypes[cond.objectType]);
-					boost::algorithm::replace_first(event.onFulfill, "%d", boost::lexical_cast<std::string>(cond.value));
+					boost::algorithm::replace_first(event.onFulfill, "%d", std::to_string(cond.value));
 					break;
 
 				case EventCondition::HAVE_BUILDING:
@@ -559,14 +547,14 @@ void CMap::checkForObjectives()
 
 				case EventCondition::CONTROL:
 					if (isInTheMap(cond.position))
-						cond.object = getObjectiveObjectFrom(cond.position, Obj::EObj(cond.objectType));
+						cond.object = getObjectiveObjectFrom(cond.position, static_cast<Obj::EObj>(cond.objectType));
 
 					if (cond.object)
 					{
-						const CGTownInstance *town = dynamic_cast<const CGTownInstance*>(cond.object);
+						const auto * town = dynamic_cast<const CGTownInstance *>(cond.object);
 						if (town)
 							boost::algorithm::replace_first(event.onFulfill, "%s", town->getNameTranslated());
-						const CGHeroInstance *hero = dynamic_cast<const CGHeroInstance*>(cond.object);
+						const auto * hero = dynamic_cast<const CGHeroInstance *>(cond.object);
 						if (hero)
 							boost::algorithm::replace_first(event.onFulfill, "%s", hero->getNameTranslated());
 					}
@@ -574,11 +562,11 @@ void CMap::checkForObjectives()
 
 				case EventCondition::DESTROY:
 					if (isInTheMap(cond.position))
-						cond.object = getObjectiveObjectFrom(cond.position, Obj::EObj(cond.objectType));
+						cond.object = getObjectiveObjectFrom(cond.position, static_cast<Obj::EObj>(cond.objectType));
 
 					if (cond.object)
 					{
-						const CGHeroInstance *hero = dynamic_cast<const CGHeroInstance*>(cond.object);
+						const auto * hero = dynamic_cast<const CGHeroInstance *>(cond.object);
 						if (hero)
 							boost::algorithm::replace_first(event.onFulfill, "%s", hero->getNameTranslated());
 					}
@@ -607,8 +595,8 @@ void CMap::checkForObjectives()
 
 void CMap::addNewArtifactInstance(CArtifactInstance * art)
 {
-	art->id = ArtifactInstanceID((si32)artInstances.size());
-	artInstances.push_back(art);
+	art->id = ArtifactInstanceID(static_cast<si32>(artInstances.size()));
+	artInstances.emplace_back(art);
 }
 
 void CMap::eraseArtifactInstance(CArtifactInstance * art)
@@ -621,7 +609,7 @@ void CMap::eraseArtifactInstance(CArtifactInstance * art)
 void CMap::addNewQuestInstance(CQuest* quest)
 {
 	quest->qid = static_cast<si32>(quests.size());
-	quests.push_back(quest);
+	quests.emplace_back(quest);
 }
 
 void CMap::removeQuestInstance(CQuest * quest)
@@ -653,7 +641,7 @@ void CMap::setUniqueInstanceName(CGObjectInstance * obj)
 
 void CMap::addNewObject(CGObjectInstance * obj)
 {
-	if(obj->id != ObjectInstanceID((si32)objects.size()))
+	if(obj->id != ObjectInstanceID(static_cast<si32>(objects.size())))
 		throw std::runtime_error("Invalid object instance id");
 
 	if(obj->instanceName.empty())
@@ -662,7 +650,7 @@ void CMap::addNewObject(CGObjectInstance * obj)
 	if (vstd::contains(instanceNames, obj->instanceName))
 		throw std::runtime_error("Object instance name duplicated: "+obj->instanceName);
 
-	objects.push_back(obj);
+	objects.emplace_back(obj);
 	instanceNames[obj->instanceName] = obj;
 	addBlockVisTiles(obj);
 

lib/mapping/CMap.h

@@ -143,7 +143,7 @@ struct DLL_LINKAGE EventCondition
 	};
 
 	EventCondition(EWinLoseType condition = STANDARD_WIN);
-	EventCondition(EWinLoseType condition, si32 value, si32 objectType, int3 position = int3(-1, -1, -1));
+	EventCondition(EWinLoseType condition, si32 value, si32 objectType, const int3 & position = int3(-1, -1, -1));
 
 	const CGObjectInstance * object; // object that was at specified position or with instance name on start
 	EMetaclass metaType;
@@ -289,7 +289,7 @@ public:
 	static const int MAP_SIZE_GIANT = 252;
 
 	CMapHeader();
-	virtual ~CMapHeader();
+	virtual ~CMapHeader() = default;
 
 	ui8 levels() const;
 
@@ -357,7 +357,7 @@ public:
 	bool isWaterTile(const int3 & pos) const;
 
 	bool canMoveBetween(const int3 &src, const int3 &dst) const;
-	bool checkForVisitableDir( const int3 & src, const TerrainTile *pom, const int3 & dst ) const;
+	bool checkForVisitableDir(const int3 & src, const TerrainTile * pom, const int3 & dst) const;
 	int3 guardingCreaturePosition (int3 pos) const;
 
 	void addBlockVisTiles(CGObjectInstance * obj);
@@ -378,7 +378,7 @@ public:
 
 
 	/// Gets object of specified type on requested position
-	const CGObjectInstance * getObjectiveObjectFrom(int3 pos, Obj::EObj type);
+	const CGObjectInstance * getObjectiveObjectFrom(const int3 & pos, Obj::EObj type);
 	CGHeroInstance * getHero(int heroId);
 
 	/// Sets the victory/loss condition objectives ??

lib/mapping/CMapEditManager.cpp

@@ -106,7 +106,7 @@ void CMapUndoManager::onUndoRedo()
 
 void CMapUndoManager::setUndoCallback(std::function<void(bool, bool)> functor)
 {
-	undoCallback = functor;
+	undoCallback = std::move(functor);
 	onUndoRedo(); //inform immediately
 }
 
@@ -154,7 +154,7 @@ void CMapEditManager::insertObject(CGObjectInstance * obj)
 void CMapEditManager::insertObjects(std::set<CGObjectInstance*>& objects)
 {
 	auto composedOperation = std::make_unique<CComposedOperation>(map);
-	for (auto obj : objects)
+	for(auto * obj : objects)
 	{
 		composedOperation->addOperation(std::make_unique<CInsertObjectOperation>(map, obj));
 	}
@@ -174,7 +174,7 @@ void CMapEditManager::removeObject(CGObjectInstance * obj)
 void CMapEditManager::removeObjects(std::set<CGObjectInstance*> & objects)
 {
 	auto composedOperation = std::make_unique<CComposedOperation>(map);
-	for (auto obj : objects)
+	for(auto * obj : objects)
 	{
 		composedOperation->addOperation(std::make_unique<CRemoveObjectOperation>(map, obj));
 	}

lib/mapping/CMapInfo.cpp

@@ -44,13 +44,13 @@ void CMapInfo::mapInit(const std::string & fname)
 	countPlayers();
 }
 
-void CMapInfo::saveInit(ResourceID file)
+void CMapInfo::saveInit(const ResourceID & file)
 {
 	CLoadFile lf(*CResourceHandler::get()->getResourceName(file), MINIMAL_SERIALIZATION_VERSION);
 	lf.checkMagicBytes(SAVEGAME_MAGIC);
 
 	mapHeader = std::make_unique<CMapHeader>();
-	lf >> *(mapHeader.get()) >> scenarioOptionsOfSave;
+	lf >> *(mapHeader) >> scenarioOptionsOfSave;
 	fileURI = file.getName();
 	countPlayers();
 	std::time_t time = boost::filesystem::last_write_time(*CResourceHandler::get()->getResourceName(file));
@@ -62,7 +62,7 @@ void CMapInfo::saveInit(ResourceID file)
 
 void CMapInfo::campaignInit()
 {
-	campaignHeader = std::unique_ptr<CCampaignHeader>(new CCampaignHeader(CCampaignHandler::getHeader(fileURI)));
+	campaignHeader = std::make_unique<CCampaignHeader>(CCampaignHandler::getHeader(fileURI));
 }
 
 void CMapInfo::countPlayers()
@@ -81,8 +81,8 @@ void CMapInfo::countPlayers()
 	}
 
 	if(scenarioOptionsOfSave)
-		for (auto i = scenarioOptionsOfSave->playerInfos.cbegin(); i != scenarioOptionsOfSave->playerInfos.cend(); i++)
-			if(i->second.isControlledByHuman())
+		for(const auto & playerInfo : scenarioOptionsOfSave->playerInfos)
+			if(playerInfo.second.isControlledByHuman())
 				amountOfHumanPlayersInSave++;
 }
 
@@ -147,7 +147,8 @@ int CMapInfo::getMapSizeIconId() const
 
 std::pair<int, int> CMapInfo::getMapSizeFormatIconId() const
 {
-	int frame = -1, group = 0;
+	int frame = -1;
+	int group = 0;
 	switch(mapHeader->version)
 	{
 	case EMapFormat::ROE:

lib/mapping/CMapInfo.h

@@ -47,7 +47,7 @@ public:
 	CMapInfo &operator=(CMapInfo &&other);
 
 	void mapInit(const std::string & fname);
-	void saveInit(ResourceID file);
+	void saveInit(const ResourceID & file);
 	void campaignInit();
 	void countPlayers();
 	// TODO: Those must be on client-side

lib/mapping/CMapOperation.cpp

@@ -28,8 +28,7 @@ std::string CMapOperation::getLabel() const
 	return "";
 }
 
-
-MapRect CMapOperation::extendTileAround(const int3& centerPos) const
+MapRect CMapOperation::extendTileAround(const int3 & centerPos) const
 {
 	return MapRect(int3(centerPos.x - 1, centerPos.y - 1, centerPos.z), 3, 3);
 }
@@ -72,7 +71,7 @@ void CComposedOperation::redo()
 std::string CComposedOperation::getLabel() const
 {
 	std::string ret = "Composed operation: ";
-	for(auto & operation : operations)
+	for(const auto & operation : operations)
 	{
 		ret.append(operation->getLabel() + ";");
 	}
@@ -84,9 +83,9 @@ void CComposedOperation::addOperation(std::unique_ptr<CMapOperation>&& operation
 	operations.push_back(std::move(operation));
 }
 
-CDrawTerrainOperation::CDrawTerrainOperation(CMap* map, const CTerrainSelection& terrainSel, TerrainId terType, CRandomGenerator* gen):
+CDrawTerrainOperation::CDrawTerrainOperation(CMap * map, CTerrainSelection terrainSel, TerrainId terType, CRandomGenerator * gen):
 	CMapOperation(map),
-	terrainSel(terrainSel),
+	terrainSel(std::move(terrainSel)),
 	terType(terType),
 	gen(gen)
 {
@@ -150,18 +149,19 @@ void CDrawTerrainOperation::updateTerrainTypes()
 			// Blow up
 			auto rect = extendTileAroundSafely(centerPos);
 			std::set<int3> suitableTiles;
-			int invalidForeignTilesCnt = std::numeric_limits<int>::max(), invalidNativeTilesCnt = 0;
+			int invalidForeignTilesCnt = std::numeric_limits<int>::max();
+			int invalidNativeTilesCnt = 0;
 			bool centerPosValid = false;
 			rect.forEach([&](const int3& posToTest)
 				{
 					auto & terrainTile = map->getTile(posToTest);
 					if(centerTile.terType->getId() != terrainTile.terType->getId())
 					{
-						auto formerTerType = terrainTile.terType;
+						const auto * formerTerType = terrainTile.terType;
 						terrainTile.terType = centerTile.terType;
 						auto testTile = getInvalidTiles(posToTest);
 
-						int nativeTilesCntNorm = testTile.nativeTiles.empty() ? std::numeric_limits<int>::max() : (int)testTile.nativeTiles.size();
+						int nativeTilesCntNorm = testTile.nativeTiles.empty() ? std::numeric_limits<int>::max() : static_cast<int>(testTile.nativeTiles.size());
 
 						bool putSuitableTile = false;
 						bool addToSuitableTiles = false;
@@ -227,7 +227,7 @@ void CDrawTerrainOperation::updateTerrainTypes()
 			{
 				static const int3 directions[] = { int3(0, -1, 0), int3(-1, 0, 0), int3(0, 1, 0), int3(1, 0, 0),
 											int3(-1, -1, 0), int3(-1, 1, 0), int3(1, 1, 0), int3(1, -1, 0) };
-				for(auto & direction : directions)
+				for(const auto & direction : directions)
 				{
 					auto it = suitableTiles.find(centerPos + direction);
 					if (it != suitableTiles.end())
@@ -329,7 +329,7 @@ CDrawTerrainOperation::ValidationResult CDrawTerrainOperation::validateTerrainVi
 
 CDrawTerrainOperation::ValidationResult CDrawTerrainOperation::validateTerrainViewInner(const int3& pos, const TerrainViewPattern& pattern, int recDepth) const
 {
-	auto centerTerType = map->getTile(pos).terType;
+	const auto * centerTerType = map->getTile(pos).terType;
 	int totalPoints = 0;
 	std::string transitionReplacement;
 
@@ -387,7 +387,7 @@ CDrawTerrainOperation::ValidationResult CDrawTerrainOperation::validateTerrainVi
 
 		// Validate all rules per cell
 		int topPoints = -1;
-		for(auto & elem : pattern.data[i])
+		for(const auto & elem : pattern.data[i])
 		{
 			TerrainViewPattern::WeightedRule rule = elem;
 			if(!rule.isStandardRule())
@@ -420,7 +420,8 @@ CDrawTerrainOperation::ValidationResult CDrawTerrainOperation::validateTerrainVi
 			};
 
 			// Validate cell with the ruleset of the pattern
-			bool nativeTestOk, nativeTestStrongOk;
+			bool nativeTestOk = false;
+			bool nativeTestStrongOk = false;
 			nativeTestOk = nativeTestStrongOk = (rule.isNativeStrong() || rule.isNativeRule()) && !isAlien;
 
 			if(centerTerType->getId() == ETerrainId::DIRT)
@@ -497,21 +498,21 @@ CDrawTerrainOperation::InvalidTiles CDrawTerrainOperation::getInvalidTiles(const
 {
 	//TODO: this is very expensive function for RMG, needs optimization
 	InvalidTiles tiles;
-	auto centerTerType = map->getTile(centerPos).terType;
+	const auto * centerTerType = map->getTile(centerPos).terType;
 	auto rect = extendTileAround(centerPos);
 	rect.forEach([&](const int3& pos)
 		{
 			if(map->isInTheMap(pos))
 			{
-				auto ptrConfig = VLC->terviewh;
-				auto terType = map->getTile(pos).terType;
+				auto * ptrConfig = VLC->terviewh;
+				const auto * terType = map->getTile(pos).terType;
 				auto valid = validateTerrainView(pos, ptrConfig->getTerrainTypePatternById("n1")).result;
 
 				// Special validity check for rock & water
 				if(valid && (terType->isWater() || !terType->isPassable()))
 				{
 					static const std::string patternIds[] = { "s1", "s2" };
-					for(auto & patternId : patternIds)
+					for(const auto & patternId : patternIds)
 					{
 						valid = !validateTerrainView(pos, ptrConfig->getTerrainTypePatternById(patternId)).result;
 						if(!valid) break;
@@ -521,7 +522,7 @@ CDrawTerrainOperation::InvalidTiles CDrawTerrainOperation::getInvalidTiles(const
 				else if(!valid && (terType->isLand() && terType->isPassable()))
 				{
 					static const std::string patternIds[] = { "n2", "n3" };
-					for (auto & patternId : patternIds)
+					for(const auto & patternId : patternIds)
 					{
 						valid = validateTerrainView(pos, ptrConfig->getTerrainTypePatternById(patternId)).result;
 						if(valid) break;
@@ -542,8 +543,10 @@ CDrawTerrainOperation::InvalidTiles CDrawTerrainOperation::getInvalidTiles(const
 	return tiles;
 }
 
-CDrawTerrainOperation::ValidationResult::ValidationResult(bool result, const std::string& transitionReplacement)
-	: result(result), transitionReplacement(transitionReplacement), flip(0)
+CDrawTerrainOperation::ValidationResult::ValidationResult(bool result, std::string transitionReplacement)
+	: result(result)
+	, transitionReplacement(std::move(transitionReplacement))
+	, flip(0)
 {
 
 }
@@ -660,7 +663,7 @@ void CRemoveObjectOperation::undo()
 	try
 	{
 		//set new id, but do not rename object
-		obj->id = ObjectInstanceID((si32)map->objects.size());
+		obj->id = ObjectInstanceID(static_cast<si32>(map->objects.size()));
 		map->addNewObject(obj);
 	}
 	catch(const std::exception& e)

lib/mapping/CMapOperation.h

@@ -63,7 +63,7 @@ private:
 class CDrawTerrainOperation : public CMapOperation
 {
 public:
-	CDrawTerrainOperation(CMap * map, const CTerrainSelection & terrainSel, TerrainId terType, CRandomGenerator * gen);
+	CDrawTerrainOperation(CMap * map, CTerrainSelection terrainSel, TerrainId terType, CRandomGenerator * gen);
 
 	void execute() override;
 	void undo() override;
@@ -73,7 +73,7 @@ public:
 private:
 	struct ValidationResult
 	{
-		ValidationResult(bool result, const std::string & transitionReplacement = "");
+		ValidationResult(bool result, std::string transitionReplacement = "");
 
 		bool result;
 		/// The replacement of a T rule, either D or S.

lib/mapping/CMapService.cpp

@@ -70,7 +70,7 @@ void CMapService::saveMap(const std::unique_ptr<CMap> & map, boost::filesystem::
 		boost::filesystem::remove(fullPath);
 		boost::filesystem::ofstream tmp(fullPath, boost::filesystem::ofstream::binary);
 
-		tmp.write((const char *)serializeBuffer.getBuffer().data(),serializeBuffer.getSize());
+		tmp.write(reinterpret_cast<const char *>(serializeBuffer.getBuffer().data()), serializeBuffer.getSize());
 		tmp.flush();
 		tmp.close();
 	}
@@ -123,7 +123,7 @@ std::unique_ptr<IMapLoader> CMapService::getMapLoader(std::unique_ptr<CInputStre
 
 static JsonNode loadPatches(std::string path)
 {
-	JsonNode node = JsonUtils::assembleFromFiles(path);
+	JsonNode node = JsonUtils::assembleFromFiles(std::move(path));
 	for (auto & entry : node.Struct())
 		JsonUtils::validate(entry.second, "vcmi:mapHeader", "patch for " + entry.first);
 	return node;

lib/mapping/MapEditUtils.cpp

@@ -24,9 +24,8 @@ MapRect::MapRect() : x(0), y(0), z(0), width(0), height(0)
 
 }
 
-MapRect::MapRect(int3 pos, si32 width, si32 height) : x(pos.x), y(pos.y), z(pos.z), width(width), height(height)
+MapRect::MapRect(const int3 & pos, si32 width, si32 height): x(pos.x), y(pos.y), z(pos.z), width(width), height(height)
 {
-
 }
 
 MapRect MapRect::operator&(const MapRect& rect) const
@@ -97,15 +96,15 @@ CTerrainSelection::CTerrainSelection(CMap * map) : CMapSelection(map)
 
 void CTerrainSelection::selectRange(const MapRect& rect)
 {
-	rect.forEach([this](const int3 pos)
+	rect.forEach([this](const int3 & pos) 
 	{
-		this->select(pos);
+		this->select(pos); 
 	});
 }
 
 void CTerrainSelection::deselectRange(const MapRect& rect)
 {
-	rect.forEach([this](const int3 pos)
+	rect.forEach([this](const int3 & pos)
 	{
 		this->deselect(pos);
 	});
@@ -143,22 +142,26 @@ const std::string TerrainViewPattern::RULE_NATIVE = "N";
 const std::string TerrainViewPattern::RULE_NATIVE_STRONG = "N!";
 const std::string TerrainViewPattern::RULE_ANY = "?";
 
-TerrainViewPattern::TerrainViewPattern() : diffImages(false), rotationTypesCount(0), minPoints(0)
+TerrainViewPattern::TerrainViewPattern()
+	: diffImages(false)
+	, rotationTypesCount(0)
+	, minPoints(0)
+	, maxPoints(std::numeric_limits<int>::max())
 {
-	maxPoints = std::numeric_limits<int>::max();
 }
 
-TerrainViewPattern::WeightedRule::WeightedRule(std::string& Name) : points(0), name(Name)
+TerrainViewPattern::WeightedRule::WeightedRule(std::string & Name)
+	: points(0)
+	, name(Name)
+	, standardRule(TerrainViewPattern::RULE_ANY == Name || TerrainViewPattern::RULE_DIRT == Name || TerrainViewPattern::RULE_NATIVE == Name ||
+				   TerrainViewPattern::RULE_SAND == Name || TerrainViewPattern::RULE_TRANSITION == Name || TerrainViewPattern::RULE_NATIVE_STRONG == Name)
+	, anyRule(Name == TerrainViewPattern::RULE_ANY)
+	, dirtRule(Name == TerrainViewPattern::RULE_DIRT)
+	, sandRule(Name == TerrainViewPattern::RULE_SAND)
+	, transitionRule(Name == TerrainViewPattern::RULE_TRANSITION)
+	, nativeStrongRule(Name == TerrainViewPattern::RULE_NATIVE_STRONG)
+	, nativeRule(Name == TerrainViewPattern::RULE_NATIVE)
 {
-	standardRule = (TerrainViewPattern::RULE_ANY == Name || TerrainViewPattern::RULE_DIRT == Name
-		|| TerrainViewPattern::RULE_NATIVE == Name || TerrainViewPattern::RULE_SAND == Name
-		|| TerrainViewPattern::RULE_TRANSITION == Name || TerrainViewPattern::RULE_NATIVE_STRONG == Name);
-	anyRule = (Name == TerrainViewPattern::RULE_ANY);
-	dirtRule = (Name == TerrainViewPattern::RULE_DIRT);
-	sandRule = (Name == TerrainViewPattern::RULE_SAND);
-	transitionRule = (Name == TerrainViewPattern::RULE_TRANSITION);
-	nativeStrongRule = (Name == TerrainViewPattern::RULE_NATIVE_STRONG);
-	nativeRule = (Name == TerrainViewPattern::RULE_NATIVE);
 }
 
 void TerrainViewPattern::WeightedRule::setNative()
@@ -220,7 +223,7 @@ CTerrainViewPatternConfig::CTerrainViewPatternConfig()
 					TerrainViewPattern terGroupPattern = pattern;
 					auto mappingStr = mappingPair.second.String();
 					boost::algorithm::erase_all(mappingStr, " ");
-					auto colonIndex = mappingStr.find_first_of(":");
+					auto colonIndex = mappingStr.find_first_of(':');
 					const auto & flipMode = mappingStr.substr(0, colonIndex);
 					terGroupPattern.diffImages = TerrainViewPattern::FLIP_MODE_DIFF_IMAGES == &(flipMode[flipMode.length() - 1]);
 					if (terGroupPattern.diffImages)
@@ -235,8 +238,7 @@ CTerrainViewPatternConfig::CTerrainViewPatternConfig()
 					{
 						std::vector<std::string> range;
 						boost::split(range, mapping, boost::is_any_of("-"));
-						terGroupPattern.mapping.push_back(std::make_pair(boost::lexical_cast<int>(range[0]),
-							boost::lexical_cast<int>(range.size() > 1 ? range[1] : range[0])));
+						terGroupPattern.mapping.emplace_back(std::stoi(range[0]), std::stoi(range.size() > 1 ? range[1] : range[0]));
 					}
 
 					// Add pattern to the patterns map
@@ -266,11 +268,6 @@ CTerrainViewPatternConfig::CTerrainViewPatternConfig()
 	}
 }
 
-CTerrainViewPatternConfig::~CTerrainViewPatternConfig()
-{
-
-}
-
 const std::vector<CTerrainViewPatternConfig::TVPVector> & CTerrainViewPatternConfig::getTerrainViewPatterns(TerrainId terrain) const
 {
 	auto iter = terrainViewPatterns.find(VLC->terrainTypeHandler->getById(terrain)->terrainViewPatterns);
@@ -279,7 +276,7 @@ const std::vector<CTerrainViewPatternConfig::TVPVector> & CTerrainViewPatternCon
 	return iter->second;
 }
 
-boost::optional<const TerrainViewPattern &> CTerrainViewPatternConfig::getTerrainViewPatternById(std::string patternId, const std::string & id) const
+boost::optional<const TerrainViewPattern &> CTerrainViewPatternConfig::getTerrainViewPatternById(const std::string & patternId, const std::string & id) const
 {
 	auto iter = terrainViewPatterns.find(patternId);
 	const std::vector<TVPVector> & groupPatterns = (iter == terrainViewPatterns.end()) ? terrainViewPatterns.at("normal") : iter->second;
@@ -343,8 +340,7 @@ void CTerrainViewPatternConfig::flipPattern(TerrainViewPattern & pattern, int fl
 	}
 }
 
-
-void CTerrainViewPatternUtils::printDebuggingInfoAboutTile(const CMap * map, int3 pos)
+void CTerrainViewPatternUtils::printDebuggingInfoAboutTile(const CMap * map, const int3 & pos)
 {
 	logGlobal->debug("Printing detailed info about nearby map tiles of pos '%s'", pos.toString());
 	for (int y = pos.y - 2; y <= pos.y + 2; ++y)

lib/mapping/MapEditUtils.h

@@ -23,7 +23,7 @@ class CMap;
 struct DLL_LINKAGE MapRect
 {
 	MapRect();
-	MapRect(int3 pos, si32 width, si32 height);
+	MapRect(const int3 & pos, si32 width, si32 height);
 	si32 x, y, z;
 	si32 width, height;
 
@@ -215,10 +215,9 @@ public:
 	typedef std::vector<TerrainViewPattern> TVPVector;
 
 	CTerrainViewPatternConfig();
-	~CTerrainViewPatternConfig();
 
 	const std::vector<TVPVector> & getTerrainViewPatterns(TerrainId terrain) const;
-	boost::optional<const TerrainViewPattern &> getTerrainViewPatternById(std::string patternId, const std::string & id) const;
+	boost::optional<const TerrainViewPattern &> getTerrainViewPatternById(const std::string & patternId, const std::string & id) const;
 	boost::optional<const TVPVector &> getTerrainViewPatternsById(TerrainId terrain, const std::string & id) const;
 	const TVPVector * getTerrainTypePatternById(const std::string & id) const;
 	void flipPattern(TerrainViewPattern & pattern, int flip) const;
@@ -231,7 +230,7 @@ private:
 class DLL_LINKAGE CTerrainViewPatternUtils
 {
 public:
-	static void printDebuggingInfoAboutTile(const CMap * map, int3 pos);
+	static void printDebuggingInfoAboutTile(const CMap * map, const int3 & pos);
 };
 
 VCMI_LIB_NAMESPACE_END

lib/mapping/MapFormatH3M.cpp

@@ -38,9 +38,8 @@ CMapLoaderH3M::CMapLoaderH3M(CInputStream * stream) : map(nullptr), reader(strea
 {
 }
 
-CMapLoaderH3M::~CMapLoaderH3M()
-{
-}
+//must be instantiated in .cpp file for access to complete types of all member fields
+CMapLoaderH3M::~CMapLoaderH3M() = default;
 
 std::unique_ptr<CMap> CMapLoaderH3M::loadMap()
 {
@@ -67,7 +66,7 @@ void CMapLoaderH3M::init()
 	si64 temp_size = inputStream->getSize();
 	inputStream->seek(0);
 
-	auto  temp_buffer = new ui8[temp_size];
+	auto * temp_buffer = new ui8[temp_size];
 	inputStream->read(temp_buffer,temp_size);
 
 	// Compute checksum
@@ -85,47 +84,43 @@ void CMapLoaderH3M::init()
 		std::string name;
 		si64 time;
 
-		MapLoadingTime(std::string name, si64 time) : name(name),
-			time(time)
-		{
-
-		}
+		MapLoadingTime(std::string name, si64 time): name(std::move(name)), time(time) {}
 	};
 	std::vector<MapLoadingTime> times;
 
 	readHeader();
-	times.push_back(MapLoadingTime("header", sw.getDiff()));
+	times.emplace_back("header", sw.getDiff());
 
 	map->allHeroes.resize(map->allowedHeroes.size());
 
 	readDisposedHeroes();
-	times.push_back(MapLoadingTime("disposed heroes", sw.getDiff()));
+	times.emplace_back("disposed heroes", sw.getDiff());
 
 	readAllowedArtifacts();
-	times.push_back(MapLoadingTime("allowed artifacts", sw.getDiff()));
+	times.emplace_back("allowed artifacts", sw.getDiff());
 
 	readAllowedSpellsAbilities();
-	times.push_back(MapLoadingTime("allowed spells and abilities", sw.getDiff()));
+	times.emplace_back("allowed spells and abilities", sw.getDiff());
 
 	readRumors();
-	times.push_back(MapLoadingTime("rumors", sw.getDiff()));
+	times.emplace_back("rumors", sw.getDiff());
 
 	readPredefinedHeroes();
-	times.push_back(MapLoadingTime("predefined heroes", sw.getDiff()));
+	times.emplace_back("predefined heroes", sw.getDiff());
 
 	readTerrain();
-	times.push_back(MapLoadingTime("terrain", sw.getDiff()));
+	times.emplace_back("terrain", sw.getDiff());
 
 	readDefInfo();
-	times.push_back(MapLoadingTime("def info", sw.getDiff()));
+	times.emplace_back("def info", sw.getDiff());
 
 	readObjects();
-	times.push_back(MapLoadingTime("objects", sw.getDiff()));
+	times.emplace_back("objects", sw.getDiff());
 
 	readEvents();
-	times.push_back(MapLoadingTime("events", sw.getDiff()));
+	times.emplace_back("events", sw.getDiff());
 
-	times.push_back(MapLoadingTime("blocked/visitable tiles", sw.getDiff()));
+	times.emplace_back("blocked/visitable tiles", sw.getDiff());
 
 	// Print profiling times
 	if(IS_PROFILING_ENABLED)
@@ -149,7 +144,7 @@ void CMapLoaderH3M::readHeader()
 	//}
 
 	// Map version
-	mapHeader->version = (EMapFormat::EMapFormat)(reader.readUInt32());
+	mapHeader->version = static_cast<EMapFormat::EMapFormat>(reader.readUInt32());
 	if(mapHeader->version != EMapFormat::ROE && mapHeader->version != EMapFormat::AB && mapHeader->version != EMapFormat::SOD
 			&& mapHeader->version != EMapFormat::WOG)
 	{
@@ -306,7 +301,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 {
 	mapHeader->triggeredEvents.clear();
 
-	auto vicCondition = (EVictoryConditionType::EVictoryConditionType)reader.readUInt8();
+	auto vicCondition = static_cast<EVictoryConditionType::EVictoryConditionType>(reader.readUInt8());
 
 	EventCondition victoryCondition(EventCondition::STANDARD_WIN);
 	EventCondition defeatCondition(EventCondition::DAYS_WITHOUT_TOWN);
@@ -343,8 +338,8 @@ void CMapLoaderH3M::readVictoryLossConditions()
 		specialVictory.identifier = "specialVictory";
 		specialVictory.description.clear(); // TODO: display in quest window
 
-		mapHeader->victoryIconIndex = ui16(vicCondition);
-		mapHeader->victoryMessage = VLC->generaltexth->victoryConditions[size_t(vicCondition) + 1];
+		mapHeader->victoryIconIndex = static_cast<ui16>(vicCondition);
+		mapHeader->victoryMessage = VLC->generaltexth->victoryConditions[static_cast<size_t>(vicCondition) + 1];
 
 		bool allowNormalVictory = reader.readBool();
 		bool appliesToAI = reader.readBool();
@@ -404,9 +399,9 @@ void CMapLoaderH3M::readVictoryLossConditions()
 				EventCondition cond(EventCondition::HAVE_BUILDING);
 				cond.position = readInt3();
 				cond.objectType = BuildingID::TOWN_HALL + reader.readUInt8();
-				oper.expressions.push_back(cond);
+				oper.expressions.emplace_back(cond);
 				cond.objectType = BuildingID::FORT + reader.readUInt8();
-				oper.expressions.push_back(cond);
+				oper.expressions.emplace_back(cond);
 
 				specialVictory.effect.toOtherMessage = VLC->generaltexth->allTexts[283];
 				specialVictory.onFulfill = VLC->generaltexth->allTexts[282];
@@ -462,8 +457,8 @@ void CMapLoaderH3M::readVictoryLossConditions()
 		case EVictoryConditionType::TAKEDWELLINGS:
 			{
 				EventExpression::OperatorAll oper;
-				oper.expressions.push_back(EventCondition(EventCondition::CONTROL, 0, Obj::CREATURE_GENERATOR1));
-				oper.expressions.push_back(EventCondition(EventCondition::CONTROL, 0, Obj::CREATURE_GENERATOR4));
+				oper.expressions.emplace_back(EventCondition(EventCondition::CONTROL, 0, Obj::CREATURE_GENERATOR1));
+				oper.expressions.emplace_back(EventCondition(EventCondition::CONTROL, 0, Obj::CREATURE_GENERATOR4));
 
 				specialVictory.effect.toOtherMessage = VLC->generaltexth->allTexts[289];
 				specialVictory.onFulfill = VLC->generaltexth->allTexts[288];
@@ -501,7 +496,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 			EventExpression::OperatorAll oper;
 			EventCondition notAI(EventCondition::IS_HUMAN);
 			notAI.value = 1;
-			oper.expressions.push_back(notAI);
+			oper.expressions.emplace_back(notAI);
 			oper.expressions.push_back(specialVictory.trigger.get());
 			specialVictory.trigger = EventExpression(oper);
 		}
@@ -517,7 +512,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 	}
 
 	// Read loss conditions
-	auto lossCond = (ELossConditionType::ELossConditionType)reader.readUInt8();
+	auto lossCond = static_cast<ELossConditionType::ELossConditionType>(reader.readUInt8());
 	if (lossCond == ELossConditionType::LOSSSTANDARD)
 	{
 		mapHeader->defeatIconIndex = 3;
@@ -531,8 +526,8 @@ void CMapLoaderH3M::readVictoryLossConditions()
 		specialDefeat.identifier = "specialDefeat";
 		specialDefeat.description.clear(); // TODO: display in quest window
 
-		mapHeader->defeatIconIndex = ui16(lossCond);
-		mapHeader->defeatMessage = VLC->generaltexth->lossCondtions[size_t(lossCond) + 1];
+		mapHeader->defeatIconIndex = static_cast<ui16>(lossCond);
+		mapHeader->defeatMessage = VLC->generaltexth->lossCondtions[static_cast<size_t>(lossCond) + 1];
 
 		switch(lossCond)
 		{
@@ -543,7 +538,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 				cond.objectType = Obj::TOWN;
 				cond.position = readInt3();
 
-				noneOf.expressions.push_back(cond);
+				noneOf.expressions.emplace_back(cond);
 				specialDefeat.onFulfill = VLC->generaltexth->allTexts[251];
 				specialDefeat.trigger = EventExpression(noneOf);
 				break;
@@ -555,7 +550,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 				cond.objectType = Obj::HERO;
 				cond.position = readInt3();
 
-				noneOf.expressions.push_back(cond);
+				noneOf.expressions.emplace_back(cond);
 				specialDefeat.onFulfill = VLC->generaltexth->allTexts[253];
 				specialDefeat.trigger = EventExpression(noneOf);
 				break;
@@ -581,7 +576,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 		EventCondition isHuman(EventCondition::IS_HUMAN);
 		isHuman.value = 1;
 
-		allOf.expressions.push_back(isHuman);
+		allOf.expressions.emplace_back(isHuman);
 		allOf.expressions.push_back(specialDefeat.trigger.get());
 		specialDefeat.trigger = EventExpression(allOf);
 
@@ -756,7 +751,7 @@ void CMapLoaderH3M::readPredefinedHeroes()
 				int custom =  reader.readUInt8();
 				if(!custom) continue;
 
-				auto  hero = new CGHeroInstance();
+				auto * hero = new CGHeroInstance();
 				hero->ID = Obj::HERO;
 				hero->subID = z;
 
@@ -807,7 +802,7 @@ void CMapLoaderH3M::readPredefinedHeroes()
 						hero->pushPrimSkill(static_cast<PrimarySkill::PrimarySkill>(xx), reader.readUInt8());
 					}
 				}
-				map->predefinedHeroes.push_back(hero);
+				map->predefinedHeroes.emplace_back(hero);
 			}
 			break;
 		}
@@ -823,13 +818,15 @@ void CMapLoaderH3M::loadArtifactsOfHero(CGHeroInstance * hero)
 	// True if artifact set is not default (hero has some artifacts)
 	if(artSet)
 	{
-		if(hero->artifactsWorn.size() ||  hero->artifactsInBackpack.size())
-		{
-			logGlobal->warn("Hero %s at %s has set artifacts twice (in map properties and on adventure map instance). Using the latter set...", hero->getNameTranslated(), hero->pos.toString());
-			hero->artifactsInBackpack.clear();
-			while(hero->artifactsWorn.size())
+	if(!hero->artifactsWorn.empty() || !hero->artifactsInBackpack.empty())
+	{
+		logGlobal->warn("Hero %s at %s has set artifacts twice (in map properties and on adventure map instance). Using the latter set...",
+						hero->getNameTranslated(),
+						hero->pos.toString());
+		hero->artifactsInBackpack.clear();
+		while(!hero->artifactsWorn.empty())
 				hero->eraseArtSlot(hero->artifactsWorn.begin()->first);
-		}
+	}
 
 		for(int pom = 0; pom < 16; pom++)
 		{
@@ -865,7 +862,7 @@ void CMapLoaderH3M::loadArtifactsOfHero(CGHeroInstance * hero)
 		int amount = reader.readUInt16();
 		for(int ss = 0; ss < amount; ++ss)
 		{
-			loadArtifactToSlot(hero, GameConstants::BACKPACK_START + (int)hero->artifactsInBackpack.size());
+			loadArtifactToSlot(hero, GameConstants::BACKPACK_START + static_cast<int>(hero->artifactsInBackpack.size()));
 		}
 	}
 }
@@ -908,7 +905,7 @@ bool CMapLoaderH3M::loadArtifactToSlot(CGHeroInstance * hero, int slot)
 		}
 
 		// this is needed, because some H3M maps (last scenario of ROE map) contain invalid data like misplaced artifacts
-		auto artifact =  CArtifactInstance::createArtifact(map, aid);
+		auto * artifact = CArtifactInstance::createArtifact(map, aid);
 		auto artifactPos = ArtifactPosition(slot);
 		if (artifact->canBePutAt(ArtifactLocation(hero, artifactPos)))
 		{
@@ -945,7 +942,7 @@ void CMapLoaderH3M::readTerrain()
 				tile.roadDir = reader.readUInt8();
 				tile.extTileFlags = reader.readUInt8();
 				tile.blocked = !tile.terType->isPassable();
-				tile.visitable = 0;
+				tile.visitable = false;
 
 				assert(tile.terType->getId() != ETerrainId::NONE);
 			}
@@ -962,7 +959,7 @@ void CMapLoaderH3M::readDefInfo()
 	// Read custom defs
 	for(int idd = 0; idd < defAmount; ++idd)
 	{
-		auto tmpl = new ObjectTemplate;
+		auto * tmpl = new ObjectTemplate;
 		tmpl->readMap(reader);
 		templates.push_back(std::shared_ptr<const ObjectTemplate>(tmpl));
 	}
@@ -979,7 +976,7 @@ void CMapLoaderH3M::readObjects()
 		int3 objPos = readInt3();
 
 		int defnum = reader.readUInt32();
-		ObjectInstanceID idToBeGiven = ObjectInstanceID((si32)map->objects.size());
+		ObjectInstanceID idToBeGiven = ObjectInstanceID(static_cast<si32>(map->objects.size()));
 
 		std::shared_ptr<const ObjectTemplate> objTempl = templates.at(defnum);
 		reader.skip(5);
@@ -988,7 +985,7 @@ void CMapLoaderH3M::readObjects()
 		{
 		case Obj::EVENT:
 			{
-				auto  evnt = new CGEvent();
+				auto * evnt = new CGEvent();
 				nobj = evnt;
 
 				readMessageAndGuards(evnt->message, evnt);
@@ -1009,7 +1006,7 @@ void CMapLoaderH3M::readObjects()
 				int gabn = reader.readUInt8(); // Number of gained abilities
 				for(int oo = 0; oo < gabn; ++oo)
 				{
-					evnt->abilities.push_back(SecondarySkill(reader.readUInt8()));
+					evnt->abilities.emplace_back(reader.readUInt8());
 					evnt->abilityLevels.push_back(reader.readUInt8());
 				}
 
@@ -1018,18 +1015,18 @@ void CMapLoaderH3M::readObjects()
 				{
 					if(map->version == EMapFormat::ROE)
 					{
-						evnt->artifacts.push_back(ArtifactID(reader.readUInt8()));
+						evnt->artifacts.emplace_back(reader.readUInt8());
 					}
 					else
 					{
-						evnt->artifacts.push_back(ArtifactID(reader.readUInt16()));
+						evnt->artifacts.emplace_back(reader.readUInt16());
 					}
 				}
 
 				int gspel = reader.readUInt8(); // Number of gained spells
 				for(int oo = 0; oo < gspel; ++oo)
 				{
-					evnt->spells.push_back(SpellID(reader.readUInt8()));
+					evnt->spells.emplace_back(reader.readUInt8());
 				}
 
 				int gcre = reader.readUInt8(); //number of gained creatures
@@ -1061,7 +1058,7 @@ void CMapLoaderH3M::readObjects()
 		case Obj::RANDOM_MONSTER_L6:
 		case Obj::RANDOM_MONSTER_L7:
 			{
-				auto  cre = new CGCreature();
+				auto * cre = new CGCreature();
 				nobj = cre;
 
 				if(map->version > EMapFormat::ROE)
@@ -1070,7 +1067,7 @@ void CMapLoaderH3M::readObjects()
 					map->questIdentifierToId[cre->identifier] = idToBeGiven;
 				}
 
-				auto  hlp = new CStackInstance();
+				auto * hlp = new CStackInstance();
 				hlp->count = reader.readUInt16();
 
 				//type will be set during initialization
@@ -1084,7 +1081,7 @@ void CMapLoaderH3M::readObjects()
 					cre->message = reader.readString();
 					readResourses(cre->resources);
 
-					int artID;
+					int artID = 0;
 					if (map->version == EMapFormat::ROE)
 					{
 						artID = reader.readUInt8();
@@ -1125,7 +1122,7 @@ void CMapLoaderH3M::readObjects()
 		case Obj::OCEAN_BOTTLE:
 		case Obj::SIGN:
 			{
-				auto  sb = new CGSignBottle();
+				auto * sb = new CGSignBottle();
 				nobj = sb;
 				sb->message = reader.readString();
 				reader.skip(4);
@@ -1138,7 +1135,7 @@ void CMapLoaderH3M::readObjects()
 			}
 		case Obj::WITCH_HUT:
 			{
-				auto  wh = new CGWitchHut();
+				auto * wh = new CGWitchHut();
 				nobj = wh;
 
 				// in RoE we cannot specify it - all are allowed (I hope)
@@ -1175,7 +1172,7 @@ void CMapLoaderH3M::readObjects()
 			}
 		case Obj::SCHOLAR:
 			{
-				auto  sch = new CGScholar();
+				auto * sch = new CGScholar();
 				nobj = sch;
 				sch->bonusType = static_cast<CGScholar::EBonusType>(reader.readUInt8());
 				sch->bonusID = reader.readUInt8();
@@ -1185,7 +1182,7 @@ void CMapLoaderH3M::readObjects()
 		case Obj::GARRISON:
 		case Obj::GARRISON2:
 			{
-				auto  gar = new CGGarrison();
+				auto * gar = new CGGarrison();
 				nobj = gar;
 				nobj->setOwner(PlayerColor(reader.readUInt8()));
 				reader.skip(3);
@@ -1211,7 +1208,7 @@ void CMapLoaderH3M::readObjects()
 			{
 				auto artID = ArtifactID::NONE; //random, set later
 				int spellID = -1;
-				auto  art = new CGArtifact();
+				auto * art = new CGArtifact();
 				nobj = art;
 
 				readMessageAndGuards(art->message, art);
@@ -1233,7 +1230,7 @@ void CMapLoaderH3M::readObjects()
 		case Obj::RANDOM_RESOURCE:
 		case Obj::RESOURCE:
 			{
-				auto  res = new CGResource();
+				auto * res = new CGResource();
 				nobj = res;
 
 				readMessageAndGuards(res->message, res);
@@ -1275,7 +1272,7 @@ void CMapLoaderH3M::readObjects()
 		case Obj::SHRINE_OF_MAGIC_GESTURE:
 		case Obj::SHRINE_OF_MAGIC_THOUGHT:
 			{
-				auto  shr = new CGShrine();
+				auto * shr = new CGShrine();
 				nobj = shr;
 				ui8 raw_id = reader.readUInt8();
 
@@ -1293,7 +1290,7 @@ void CMapLoaderH3M::readObjects()
 			}
 		case Obj::PANDORAS_BOX:
 			{
-				auto  box = new CGPandoraBox();
+				auto * box = new CGPandoraBox();
 				nobj = box;
 				readMessageAndGuards(box->message, box);
 
@@ -1313,7 +1310,7 @@ void CMapLoaderH3M::readObjects()
 				int gabn = reader.readUInt8();//number of gained abilities
 				for(int oo = 0; oo < gabn; ++oo)
 				{
-					box->abilities.push_back(SecondarySkill(reader.readUInt8()));
+					box->abilities.emplace_back(reader.readUInt8());
 					box->abilityLevels.push_back(reader.readUInt8());
 				}
 				int gart = reader.readUInt8(); //number of gained artifacts
@@ -1321,17 +1318,17 @@ void CMapLoaderH3M::readObjects()
 				{
 					if(map->version > EMapFormat::ROE)
 					{
-						box->artifacts.push_back(ArtifactID(reader.readUInt16()));
+						box->artifacts.emplace_back(reader.readUInt16());
 					}
 					else
 					{
-						box->artifacts.push_back(ArtifactID(reader.readUInt8()));
+						box->artifacts.emplace_back(reader.readUInt8());
 					}
 				}
 				int gspel = reader.readUInt8(); //number of gained spells
 				for(int oo = 0; oo < gspel; ++oo)
 				{
-					box->spells.push_back(SpellID(reader.readUInt8()));
+					box->spells.emplace_back(reader.readUInt8());
 				}
 				int gcre = reader.readUInt8(); //number of gained creatures
 				readCreatureSet(&box->creatures, gcre);
@@ -1348,7 +1345,7 @@ void CMapLoaderH3M::readObjects()
 		case Obj::RANDOM_DWELLING_LVL: //same as castle, fixed level
 		case Obj::RANDOM_DWELLING_FACTION: //level range, fixed faction
 			{
-				auto dwelling = new CGDwelling();
+				auto * dwelling = new CGDwelling();
 				nobj = dwelling;
 				CSpecObjInfo * spec = nullptr;
 				switch(objTempl->id)
@@ -1370,7 +1367,7 @@ void CMapLoaderH3M::readObjects()
 				nobj->setOwner(PlayerColor(reader.readUInt32()));
 
 				//216 and 217
-				if (auto castleSpec = dynamic_cast<CCreGenAsCastleInfo *>(spec))
+				if(auto * castleSpec = dynamic_cast<CCreGenAsCastleInfo *>(spec))
 				{
 					castleSpec->instanceId = "";
 					castleSpec->identifier = reader.readUInt32();
@@ -1398,17 +1395,17 @@ void CMapLoaderH3M::readObjects()
 				}
 
 				//216 and 218
-				if (auto lvlSpec = dynamic_cast<CCreGenLeveledInfo *>(spec))
+				if(auto * lvlSpec = dynamic_cast<CCreGenLeveledInfo *>(spec))
 				{
-					lvlSpec->minLevel = std::max(reader.readUInt8(), ui8(1));
-					lvlSpec->maxLevel = std::min(reader.readUInt8(), ui8(7));
+					lvlSpec->minLevel = std::max(reader.readUInt8(), static_cast<ui8>(1));
+					lvlSpec->maxLevel = std::min(reader.readUInt8(), static_cast<ui8>(7));
 				}
 				dwelling->info = spec;
 				break;
 			}
 		case Obj::QUEST_GUARD:
 			{
-				auto  guard = new CGQuestGuard();
+				auto * guard = new CGQuestGuard();
 				readQuest(guard);
 				nobj = guard;
 				break;
@@ -1421,7 +1418,7 @@ void CMapLoaderH3M::readObjects()
 			}
 		case Obj::HERO_PLACEHOLDER: //hero placeholder
 			{
-				auto  hp = new CGHeroPlaceholder();
+				auto * hp = new CGHeroPlaceholder();
 				nobj = hp;
 
 				hp->setOwner(PlayerColor(reader.readUInt8()));
@@ -1522,7 +1519,7 @@ void CMapLoaderH3M::readCreatureSet(CCreatureSet * out, int number)
 	for(int ir = 0; ir < number; ++ir)
 	{
 		CreatureID creID;
-		int count;
+		int count = 0;
 
 		if (version)
 		{
@@ -1538,7 +1535,7 @@ void CMapLoaderH3M::readCreatureSet(CCreatureSet * out, int number)
 		if(creID == maxID)
 			continue;
 
-		auto  hlp = new CStackInstance();
+		auto * hlp = new CStackInstance();
 		hlp->count = count;
 
 		if(creID > maxID - 0xf)
@@ -1557,9 +1554,9 @@ void CMapLoaderH3M::readCreatureSet(CCreatureSet * out, int number)
 	out->validTypes(true);
 }
 
-CGObjectInstance * CMapLoaderH3M::readHero(ObjectInstanceID idToBeGiven, const int3 & initialPos)
+CGObjectInstance * CMapLoaderH3M::readHero(const ObjectInstanceID & idToBeGiven, const int3 & initialPos)
 {
-	auto nhi = new CGHeroInstance();
+	auto * nhi = new CGHeroInstance();
 
 	if(map->version > EMapFormat::ROE)
 	{
@@ -1635,7 +1632,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(ObjectInstanceID idToBeGiven, const i
 	bool hasSecSkills = reader.readBool();
 	if(hasSecSkills)
 	{
-		if(nhi->secSkills.size())
+		if(!nhi->secSkills.empty())
 		{
 			nhi->secSkills.clear();
 			//logGlobal->warn("Hero %s subID=%d has set secondary skills twice (in map properties and on adventure map instance). Using the latter set...", nhi->name, nhi->subID);
@@ -1685,7 +1682,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(ObjectInstanceID idToBeGiven, const i
 	if(map->version > EMapFormat::AB)
 	{
 		bool hasCustomSpells = reader.readBool();
-		if(nhi->spells.size())
+		if(!nhi->spells.empty())
 		{
 			nhi->clear();
 			logGlobal->warn("Hero %s subID=%d has spells set twice (in map properties and on adventure map instance). Using the latter set...", nhi->getNameTranslated(), nhi->subID);
@@ -1722,7 +1719,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(ObjectInstanceID idToBeGiven, const i
 			if(ps->size())
 			{
 				logGlobal->warn("Hero %s subID=%d has set primary skills twice (in map properties and on adventure map instance). Using the latter set...", nhi->getNameTranslated(), nhi->subID);
-				for(auto b : *ps)
+				for(const auto & b : *ps)
 					nhi->removeBonus(b);
 			}
 
@@ -1739,7 +1736,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(ObjectInstanceID idToBeGiven, const i
 
 CGSeerHut * CMapLoaderH3M::readSeerHut()
 {
-	auto  hut = new CGSeerHut();
+	auto * hut = new CGSeerHut();
 
 	if(map->version > EMapFormat::ROE)
 	{
@@ -1926,14 +1923,14 @@ void CMapLoaderH3M::readQuest(IQuestObject * guard)
 	guard->quest->firstVisitText = reader.readString();
 	guard->quest->nextVisitText = reader.readString();
 	guard->quest->completedText = reader.readString();
-	guard->quest->isCustomFirst = guard->quest->firstVisitText.size() > 0;
-	guard->quest->isCustomNext = guard->quest->nextVisitText.size() > 0;
-	guard->quest->isCustomComplete = guard->quest->completedText.size() > 0;
+	guard->quest->isCustomFirst = !guard->quest->firstVisitText.empty();
+	guard->quest->isCustomNext = !guard->quest->nextVisitText.empty();
+	guard->quest->isCustomComplete = !guard->quest->completedText.empty();
 }
 
 CGTownInstance * CMapLoaderH3M::readTown(int castleID)
 {
-	auto  nt = new CGTownInstance();
+	auto * nt = new CGTownInstance();
 	if(map->version > EMapFormat::ROE)
 	{
 		nt->identifier = reader.readUInt32();
@@ -1986,7 +1983,7 @@ CGTownInstance * CMapLoaderH3M::readTown(int castleID)
 				{
 					if(c == (c | static_cast<ui8>(std::pow(2., yy)))) //add obligatory spell even if it's banned on a map (?)
 					{
-						nt->obligatorySpells.push_back(SpellID(i * 8 + yy));
+						nt->obligatorySpells.emplace_back(i * 8 + yy);
 					}
 				}
 			}
@@ -2003,7 +2000,7 @@ CGTownInstance * CMapLoaderH3M::readTown(int castleID)
 			{
 				if(c != (c | static_cast<ui8>(std::pow(2., yy))) && map->allowedSpell[spellid]) //add random spell only if it's allowed on entire map
 				{
-					nt->possibleSpells.push_back(SpellID(spellid));
+					nt->possibleSpells.emplace_back(spellid);
 				}
 			}
 		}
@@ -2012,7 +2009,7 @@ CGTownInstance * CMapLoaderH3M::readTown(int castleID)
 	//TODO: allow customize new spells in towns
 	for (int i = SpellID::AFTER_LAST; i < VLC->spellh->objects.size(); ++i)
 	{
-		nt->possibleSpells.push_back(SpellID(i));
+		nt->possibleSpells.emplace_back(i);
 	}
 
 	// Read castle events
@@ -2067,7 +2064,7 @@ CGTownInstance * CMapLoaderH3M::readTown(int castleID)
 	return nt;
 }
 
-std::set<BuildingID> CMapLoaderH3M::convertBuildings(const std::set<BuildingID> h3m, int castleID, bool addAuxiliary)
+std::set<BuildingID> CMapLoaderH3M::convertBuildings(const std::set<BuildingID> & h3m, int castleID, bool addAuxiliary) const
 {
 	std::map<int, BuildingID> mapa;
 	std::set<BuildingID> ret;
@@ -2081,11 +2078,11 @@ std::set<BuildingID> CMapLoaderH3M::convertBuildings(const std::set<BuildingID>
 
 		if (town == castleID || town == -1)
 		{
-			mapa[(int)entry["h3"].Float()] = BuildingID((si32)entry["vcmi"].Float());
+			mapa[static_cast<int>(entry["h3"].Float())] = BuildingID(static_cast<si32>(entry["vcmi"].Float()));
 		}
 	}
 
-	for(auto & elem : h3m)
+	for(const auto & elem : h3m)
 	{
 		if(mapa[elem] >= 0)
 		{
@@ -2218,7 +2215,7 @@ void CMapLoaderH3M::readBitmask(std::vector<bool>& dest, const int byteCount, co
 	}
 }
 
-ui8 CMapLoaderH3M::reverse(ui8 arg)
+ui8 CMapLoaderH3M::reverse(ui8 arg) const
 {
 	ui8 ret = 0;
 	for(int i = 0; i < 8; ++i)
@@ -2244,7 +2241,7 @@ void CMapLoaderH3M::afterRead()
 
 			const CGObjectInstance * mainTown = nullptr;
 
-			for(auto obj : t.visitableObjects)
+			for(auto * obj : t.visitableObjects)
 			{
 				if(obj->ID == Obj::TOWN || obj->ID == Obj::RANDOM_TOWN)
 				{

lib/mapping/MapFormatH3M.h

@@ -164,7 +164,7 @@ private:
 	 * @param idToBeGiven the object id which should be set for the hero
 	 * @return a object instance
 	 */
-	CGObjectInstance * readHero(ObjectInstanceID idToBeGiven, const int3 & initialPos);
+	CGObjectInstance * readHero(const ObjectInstanceID & idToBeGiven, const int3 & initialPos);
 
 	/**
 	 * Reads a seer hut.
@@ -196,7 +196,7 @@ private:
 	 * @param addAuxiliary true if the village hall should be added
 	 * @return the converted buildings
 	 */
-	std::set<BuildingID> convertBuildings(const std::set<BuildingID> h3m, int castleID, bool addAuxiliary = true);
+	std::set<BuildingID> convertBuildings(const std::set<BuildingID> & h3m, int castleID, bool addAuxiliary = true) const;
 
 	/**
 	 * Reads events.
@@ -229,7 +229,7 @@ private:
 	 * @param arg the input argument
 	 * @return the reversed 8-bit integer
 	 */
-	ui8 reverse(ui8 arg);
+	ui8 reverse(ui8 arg) const;
 
 	/**
 	* Helper to read map position

lib/mapping/MapFormatJson.cpp

@@ -142,15 +142,15 @@ namespace TriggeredEventsDetail
 		auto rawId = vstd::find_pos(NMetaclass::names, source);
 
 		if(rawId >= 0)
-			return (EMetaclass)rawId;
+			return static_cast<EMetaclass>(rawId);
 		else
 			return EMetaclass::INVALID;
 	}
 
 	static std::string encodeIdentifier(EMetaclass metaType, si32 type)
 	{
-		std::string metaclassName = NMetaclass::names[(int)metaType];
-		std::string identifier = "";
+		std::string metaclassName = NMetaclass::names[static_cast<int>(metaType)];
+		std::string identifier;
 
 		switch(metaType)
 		{
@@ -189,7 +189,7 @@ namespace TriggeredEventsDetail
 			break;
 		}
 
-		return VLC->modh->makeFullIdentifier("", metaclassName, identifier);
+		return CModHandler::makeFullIdentifier("", metaclassName, identifier);
 	}
 
 	static EventCondition JsonToCondition(const JsonNode & node)
@@ -200,7 +200,7 @@ namespace TriggeredEventsDetail
 
 		auto pos = vstd::find_pos(conditionNames, conditionName);
 
-		event.condition = EventCondition::EWinLoseType(pos);
+		event.condition = static_cast<EventCondition::EWinLoseType>(pos);
 
 		if (node.Vector().size() > 1)
 		{
@@ -213,7 +213,10 @@ namespace TriggeredEventsDetail
 				{
 					//todo: support subtypes
 
-					std::string fullIdentifier = data["type"].String(), metaTypeName = "", scope = "" , identifier = "";
+					std::string fullIdentifier = data["type"].String();
+					std::string metaTypeName;
+					std::string scope;
+					std::string identifier;
 					CModHandler::parseIdentifier(fullIdentifier, scope, metaTypeName, identifier);
 
 					event.metaType = decodeMetaclass(metaTypeName);
@@ -255,7 +258,7 @@ namespace TriggeredEventsDetail
 
 			if (!data["position"].isNull())
 			{
-				auto & position = data["position"].Vector();
+				const auto & position = data["position"].Vector();
 				event.position.x = static_cast<si32>(position.at(0).Float());
 				event.position.y = static_cast<si32>(position.at(1).Float());
 				event.position.z = static_cast<si32>(position.at(2).Float());
@@ -349,19 +352,19 @@ CMapFormatJson::CMapFormatJson():
 
 }
 
-TerrainType * CMapFormatJson::getTerrainByCode( std::string code)
+TerrainType * CMapFormatJson::getTerrainByCode(const std::string & code)
 {
-	for ( auto const & object : VLC->terrainTypeHandler->objects)
+	for(const auto & object : VLC->terrainTypeHandler->objects)
 	{
-		if (object->shortIdentifier == code)
+		if(object->shortIdentifier == code)
 			return const_cast<TerrainType *>(object.get());
 	}
 	return nullptr;
 }
 
-RiverType * CMapFormatJson::getRiverByCode( std::string code)
+RiverType * CMapFormatJson::getRiverByCode(const std::string & code)
 {
-	for ( auto const & object : VLC->riverTypeHandler->objects)
+	for(const auto & object : VLC->riverTypeHandler->objects)
 	{
 		if (object->shortIdentifier == code)
 			return const_cast<RiverType *>(object.get());
@@ -369,9 +372,9 @@ RiverType * CMapFormatJson::getRiverByCode( std::string code)
 	return nullptr;
 }
 
-RoadType * CMapFormatJson::getRoadByCode( std::string code)
+RoadType * CMapFormatJson::getRoadByCode(const std::string & code)
 {
-	for ( auto const & object : VLC->roadTypeHandler->objects)
+	for(const auto & object : VLC->roadTypeHandler->objects)
 	{
 		if (object->shortIdentifier == code)
 			return const_cast<RoadType *>(object.get());
@@ -379,8 +382,7 @@ RoadType * CMapFormatJson::getRoadByCode( std::string code)
 	return nullptr;
 }
 
-
-void CMapFormatJson::serializeAllowedFactions(JsonSerializeFormat & handler, std::set<TFaction> & value)
+void CMapFormatJson::serializeAllowedFactions(JsonSerializeFormat & handler, std::set<TFaction> & value) const
 {
 	//TODO: unify allowed factions with others - make them std::vector<bool>
 
@@ -392,7 +394,7 @@ void CMapFormatJson::serializeAllowedFactions(JsonSerializeFormat & handler, std
 	{
 		for(auto faction : VLC->townh->objects)
 			if(faction->town && vstd::contains(value, faction->getIndex()))
-				temp[std::size_t(faction->getIndex())] = true;
+				temp[static_cast<std::size_t>(faction->getIndex())] = true;
 	}
 
 	handler.serializeLIC("allowedFactions", &FactionID::decode, &FactionID::encode, standard, temp);
@@ -402,7 +404,7 @@ void CMapFormatJson::serializeAllowedFactions(JsonSerializeFormat & handler, std
 		value.clear();
 		for (std::size_t i=0; i<temp.size(); i++)
 			if(temp[i])
-				value.insert((TFaction)i);
+				value.insert(static_cast<TFaction>(i));
 	}
 }
 
@@ -518,7 +520,7 @@ void CMapFormatJson::serializePlayerInfo(JsonSerializeFormat & handler)
 			{
 				if((obj->ID == Obj::HERO || obj->ID == Obj::RANDOM_HERO) && obj->tempOwner == PlayerColor(player))
 				{
-					CGHeroInstance * hero = dynamic_cast<CGHeroInstance *>(obj.get());
+					auto * hero = dynamic_cast<CGHeroInstance *>(obj.get());
 
 					auto heroes = handler.enterStruct("heroes");
 					if(hero)
@@ -558,7 +560,7 @@ void CMapFormatJson::serializePlayerInfo(JsonSerializeFormat & handler)
 				hname.heroId = -1;
 				std::string rawId = data["type"].String();
 
-				if(rawId != "")
+				if(!rawId.empty())
 					hname.heroId = HeroTypeID::decode(rawId);
 
 				hname.heroName = data["name"].String();
@@ -666,7 +668,7 @@ void CMapFormatJson::readTriggeredEvents(JsonDeserializer & handler)
 
 	mapHeader->triggeredEvents.clear();
 
-	for(auto & entry : input["triggeredEvents"].Struct())
+	for(const auto & entry : input["triggeredEvents"].Struct())
 	{
 		TriggeredEvent event;
 		event.identifier = entry.first;
@@ -675,7 +677,7 @@ void CMapFormatJson::readTriggeredEvents(JsonDeserializer & handler)
 	}
 }
 
-void CMapFormatJson::readTriggeredEvent(TriggeredEvent & event, const JsonNode & source)
+void CMapFormatJson::readTriggeredEvent(TriggeredEvent & event, const JsonNode & source) const
 {
 	using namespace TriggeredEventsDetail;
 
@@ -690,13 +692,13 @@ void CMapFormatJson::writeTriggeredEvents(JsonSerializer & handler)
 {
 	JsonNode triggeredEvents(JsonNode::JsonType::DATA_STRUCT);
 
-	for(auto event : mapHeader->triggeredEvents)
+	for(const auto & event : mapHeader->triggeredEvents)
 		writeTriggeredEvent(event, triggeredEvents[event.identifier]);
 
 	handler.serializeRaw("triggeredEvents", triggeredEvents, boost::none);
 }
 
-void CMapFormatJson::writeTriggeredEvent(const TriggeredEvent & event, JsonNode & dest)
+void CMapFormatJson::writeTriggeredEvent(const TriggeredEvent & event, JsonNode & dest) const
 {
 	using namespace TriggeredEventsDetail;
 
@@ -706,7 +708,7 @@ void CMapFormatJson::writeTriggeredEvent(const TriggeredEvent & event, JsonNode
 	if(!event.description.empty())
 		dest["description"].String() = event.description;
 
-	dest["effect"]["type"].String() = typeNames.at(size_t(event.effect.type));
+	dest["effect"]["type"].String() = typeNames.at(static_cast<size_t>(event.effect.type));
 
 	if(!event.effect.toOtherMessage.empty())
 		dest["effect"]["messageToSend"].String() = event.effect.toOtherMessage;
@@ -810,12 +812,12 @@ void CMapFormatJson::serializePredefinedHeroes(JsonSerializeFormat & handler)
 		{
 			auto predefinedHero = handler.enterStruct(p.first);
 
-			CGHeroInstance * hero = new CGHeroInstance();
+			auto * hero = new CGHeroInstance();
 			hero->ID = Obj::HERO;
-            hero->setHeroTypeName(p.first);
-            hero->serializeJsonDefinition(handler);
+			hero->setHeroTypeName(p.first);
+			hero->serializeJsonDefinition(handler);
 
-            map->predefinedHeroes.push_back(hero);
+			map->predefinedHeroes.emplace_back(hero);
 		}
 	}
 }
@@ -848,9 +850,7 @@ void CMapFormatJson::writeOptions(JsonSerializer & handler)
 }
 
 ///CMapPatcher
-CMapPatcher::CMapPatcher(JsonNode stream):
-	CMapFormatJson(),
-	input(stream)
+CMapPatcher::CMapPatcher(const JsonNode & stream): input(stream)
 {
 	//todo: update map patches and change this
 	fileVersionMajor = 0;
@@ -872,19 +872,17 @@ void CMapPatcher::readPatchData()
 }
 
 ///CMapLoaderJson
-CMapLoaderJson::CMapLoaderJson(CInputStream * stream):
-	CMapFormatJson(),
-	buffer(stream),
-	ioApi(new CProxyROIOApi(buffer)),
-	loader("", "_", ioApi)
+CMapLoaderJson::CMapLoaderJson(CInputStream * stream)
+	: buffer(stream)
+	, ioApi(new CProxyROIOApi(buffer))
+	, loader("", "_", ioApi)
 {
-
 }
 
 std::unique_ptr<CMap> CMapLoaderJson::loadMap()
 {
 	LOG_TRACE(logGlobal);
-	std::unique_ptr<CMap> result = std::unique_ptr<CMap>(new CMap());
+	std::unique_ptr<CMap> result = std::make_unique<CMap>();
 	map = result.get();
 	mapHeader = map;
 	readMap();
@@ -895,7 +893,7 @@ std::unique_ptr<CMapHeader> CMapLoaderJson::loadMapHeader()
 {
 	LOG_TRACE(logGlobal);
 	map = nullptr;
-	std::unique_ptr<CMapHeader> result = std::unique_ptr<CMapHeader>(new CMapHeader());
+	std::unique_ptr<CMapHeader> result = std::make_unique<CMapHeader>();
 	mapHeader = result.get();
 	readHeader(false);
 	return result;
@@ -1127,7 +1125,8 @@ void CMapLoaderJson::MapObjectLoader::construct()
 {
 	//TODO:consider move to ObjectTypeHandler
 	//find type handler
-	std::string typeName = configuration["type"].String(), subtypeName = configuration["subtype"].String();
+	std::string typeName = configuration["type"].String();
+	std::string subtypeName = configuration["subtype"].String();
 	if(typeName.empty())
 	{
 		logGlobal->error("Object type missing");
@@ -1157,7 +1156,7 @@ void CMapLoaderJson::MapObjectLoader::construct()
 
 	auto handler = VLC->objtypeh->getHandlerFor( CModHandler::scopeMap(), typeName, subtypeName);
 
-	auto appearance = new ObjectTemplate;
+	auto * appearance = new ObjectTemplate;
 
 	appearance->id = Obj(handler->getIndex());
 	appearance->subid = handler->getSubIndex();
@@ -1166,7 +1165,7 @@ void CMapLoaderJson::MapObjectLoader::construct()
 	// Will be destroyed soon and replaced with shared template
 	instance = handler->create(std::shared_ptr<const ObjectTemplate>(appearance));
 
-	instance->id = ObjectInstanceID((si32)owner->map->objects.size());
+	instance->id = ObjectInstanceID(static_cast<si32>(owner->map->objects.size()));
 	instance->instanceName = jsonKey;
 	instance->pos = pos;
 	owner->map->addNewObject(instance);
@@ -1184,7 +1183,7 @@ void CMapLoaderJson::MapObjectLoader::configure()
 	//artifact instance serialization requires access to Map object, handle it here for now
 	//todo: find better solution for artifact instance serialization
 
-	if(auto art = dynamic_cast<CGArtifact *>(instance))
+	if(auto * art = dynamic_cast<CGArtifact *>(instance))
 	{
 		auto artID = ArtifactID::NONE;
 		int spellID = -1;
@@ -1208,7 +1207,7 @@ void CMapLoaderJson::MapObjectLoader::configure()
 		art->storedArtifact = CArtifactInstance::createArtifact(owner->map, artID, spellID);
 	}
 
-	if(auto hero = dynamic_cast<CGHeroInstance *>(instance))
+	if(auto * hero = dynamic_cast<CGHeroInstance *>(instance))
 	{
 		auto o = handler.enterStruct("options");
 		hero->serializeJsonArtifacts(handler, "artifacts", owner->map);
@@ -1241,20 +1240,17 @@ void CMapLoaderJson::readObjects()
 }
 
 ///CMapSaverJson
-CMapSaverJson::CMapSaverJson(CInputOutputStream * stream):
-	CMapFormatJson(),
-	buffer(stream),
-	ioApi(new CProxyIOApi(buffer)),
-	saver(ioApi, "_")
+CMapSaverJson::CMapSaverJson(CInputOutputStream * stream)
+	: buffer(stream)
+	, ioApi(new CProxyIOApi(buffer))
+	, saver(ioApi, "_")
 {
 	fileVersionMajor = VERSION_MAJOR;
 	fileVersionMinor = VERSION_MINOR;
 }
 
-CMapSaverJson::~CMapSaverJson()
-{
-
-}
+//must be instantiated in .cpp file for access to complete types of all member fields
+CMapSaverJson::~CMapSaverJson() = default;
 
 void CMapSaverJson::addToArchive(const JsonNode & data, const std::string & filename)
 {
@@ -1267,7 +1263,7 @@ void CMapSaverJson::addToArchive(const JsonNode & data, const std::string & file
 		auto s = out.str();
 		std::unique_ptr<COutputStream> stream = saver.addFile(filename);
 
-		if (stream->write((const ui8*)s.c_str(), s.size()) != s.size())
+		if(stream->write(reinterpret_cast<const ui8 *>(s.c_str()), s.size()) != s.size())
 			throw std::runtime_error("CMapSaverJson::saveHeader() zip compression failed.");
 	}
 }
@@ -1323,13 +1319,13 @@ std::string CMapSaverJson::writeTerrainTile(const TerrainTile & tile)
 	out.setf(std::ios::dec, std::ios::basefield);
 	out.unsetf(std::ios::showbase);
 
-	out << tile.terType->shortIdentifier << (int)tile.terView << flipCodes[tile.extTileFlags % 4];
+	out << tile.terType->shortIdentifier << static_cast<int>(tile.terView) << flipCodes[tile.extTileFlags % 4];
 
 	if(tile.roadType->getId() != Road::NO_ROAD)
-		out << tile.roadType->shortIdentifier << (int)tile.roadDir << flipCodes[(tile.extTileFlags >> 4) % 4];
+		out << tile.roadType->shortIdentifier << static_cast<int>(tile.roadDir) << flipCodes[(tile.extTileFlags >> 4) % 4];
 
 	if(tile.riverType->getId() != River::NO_RIVER)
-		out << tile.riverType->shortIdentifier << (int)tile.riverDir << flipCodes[(tile.extTileFlags >> 2) % 4];
+		out << tile.riverType->shortIdentifier << static_cast<int>(tile.riverDir) << flipCodes[(tile.extTileFlags >> 2) % 4];
 
 	return out.str();
 }

lib/mapping/MapFormatJson.h

@@ -60,11 +60,11 @@ protected:
 
 	CMapFormatJson();
 
-	static TerrainType * getTerrainByCode( std::string code);
-	static RiverType * getRiverByCode( std::string code);
-	static RoadType * getRoadByCode( std::string code);
+	static TerrainType * getTerrainByCode(const std::string & code);
+	static RiverType * getRiverByCode(const std::string & code);
+	static RoadType * getRoadByCode(const std::string & code);
 
-	void serializeAllowedFactions(JsonSerializeFormat & handler, std::set<TFaction> & value);
+	void serializeAllowedFactions(JsonSerializeFormat & handler, std::set<TFaction> & value) const;
 
 	///common part of header saving/loading
 	void serializeHeader(JsonSerializeFormat & handler);
@@ -95,12 +95,12 @@ protected:
 	/**
 	 * Reads one of triggered events
 	 */
-	void readTriggeredEvent(TriggeredEvent & event, const JsonNode & source);
+	void readTriggeredEvent(TriggeredEvent & event, const JsonNode & source) const;
 
 	/**
 	 * Writes one of triggered events
 	 */
-	void writeTriggeredEvent(const TriggeredEvent & event, JsonNode & dest);
+	void writeTriggeredEvent(const TriggeredEvent & event, JsonNode & dest) const;
 
 	void writeDisposedHeroes(JsonSerializeFormat & handler);
 
@@ -132,7 +132,7 @@ public:
 	 *
 	 * @param stream. A stream containing the map data.
 	 */
-	CMapPatcher(JsonNode stream);
+	CMapPatcher(const JsonNode & stream);
 
 public: //IMapPatcher
 	/**