vcmi: modernize lib/mapObjects

lib/CArtHandler.cpp

@@ -410,17 +410,17 @@ CArtifact * CArtHandler::loadFromJson(const std::string & scope, const JsonNode
 	return art;
 }
 
-ArtifactPosition CArtHandler::stringToSlot(std::string slotName)
+ArtifactPosition::ArtifactPosition(std::string slotName):
+	num(ArtifactPosition::PRE_FIRST)
 {
 #define ART_POS(x) { #x, ArtifactPosition::x },
 	static const std::map<std::string, ArtifactPosition> artifactPositionMap = { ART_POS_LIST };
 #undef ART_POS
 	auto it = artifactPositionMap.find (slotName);
 	if (it != artifactPositionMap.end())
-		return it->second;
+		num = it->second;
 
 	logMod->warn("Warning! Artifact slot %s not recognized!", slotName);
-	return ArtifactPosition::PRE_FIRST;
 }
 
 void CArtHandler::addSlot(CArtifact * art, const std::string & slotID)
@@ -445,7 +445,7 @@ void CArtHandler::addSlot(CArtifact * art, const std::string & slotID)
 	}
 	else
 	{
-		auto slot = stringToSlot(slotID);
+		auto slot = ArtifactPosition(slotID);
 		if (slot != ArtifactPosition::PRE_FIRST)
 			art->possibleSlots[ArtBearer::HERO].push_back (slot);
 	}

lib/CArtHandler.h

@@ -249,8 +249,7 @@ public:
 
 	boost::optional<std::vector<CArtifact*>&> listFromClass(CArtifact::EartClass artifactClass);
 
-	ArtifactPosition stringToSlot(std::string slotName);
-	CArtifact::EartClass stringToClass(std::string className);
+	static CArtifact::EartClass stringToClass(std::string className); //TODO: rework EartClass to make this a constructor
 
 	/// Gets a artifact ID randomly and removes the selected artifact from this handler.
 	ArtifactID pickRandomArtifact(CRandomGenerator & rand, int flags);

lib/GameConstants.h

@@ -1034,6 +1034,8 @@ public:
 	ArtifactPosition(EArtifactPosition _num = PRE_FIRST) : num(_num)
 	{}
 
+	ArtifactPosition(std::string slotName);
+
 	ID_LIKE_CLASS_COMMON(ArtifactPosition, EArtifactPosition)
 
 	EArtifactPosition num;

lib/mapObjects/CArmedInstance.cpp

@@ -35,7 +35,6 @@ void CArmedInstance::randomizeArmy(int type)
 		assert(elem.second->valid(false));
 		assert(elem.second->armyObj == this);
 	}
-	return;
 }
 
 // Take Angelic Alliance troop-mixing freedom of non-evil units into account.
@@ -46,10 +45,11 @@ CArmedInstance::CArmedInstance()
 {
 }
 
-CArmedInstance::CArmedInstance(bool isHypotetic)
-	:CBonusSystemNode(isHypotetic), nonEvilAlignmentMix(this, nonEvilAlignmentMixSelector)
+CArmedInstance::CArmedInstance(bool isHypotetic):
+	CBonusSystemNode(isHypotetic),
+	nonEvilAlignmentMix(this, nonEvilAlignmentMixSelector),
+	battle(nullptr)
 {
-	battle = nullptr;
 }
 
 void CArmedInstance::updateMoraleBonusFromArmy()
@@ -71,7 +71,7 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 	const std::string undeadCacheKey = "type_UNDEAD";
 	static const CSelector undeadSelector = Selector::type()(Bonus::UNDEAD);
 
-	for(auto slot : Slots())
+	for(const auto & slot : Slots())
 	{
 		const CStackInstance * inst = slot.second;
 		const CCreature * creature  = VLC->creh->objects[inst->getCreatureID()];
@@ -110,7 +110,7 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 	}
 	else if (!factions.empty()) // no bonus from empty garrison
 	{
-	 	b->val = 2 - (si32)factionsInArmy;
+		b->val = 2 - static_cast<si32>(factionsInArmy);
 		description = boost::str(boost::format(VLC->generaltexth->arraytxt[114]) % factionsInArmy % b->val); //Troops of %d alignments %d
 		description = b->description.substr(0, description.size()-2);//trim value
 	}

lib/mapObjects/CBank.cpp

@@ -30,15 +30,10 @@ static std::string visitedTxt(const bool visited)
 	return VLC->generaltexth->allTexts[id];
 }
 
-CBank::CBank()
-{
-	daycounter = 0;
-	resetDuration = 0;
-}
-
-CBank::~CBank()
-{
-}
+//must be instantiated in .cpp file for access to complete types of all member fields
+CBank::CBank() = default;
+//must be instantiated in .cpp file for access to complete types of all member fields
+CBank::~CBank() = default;
 
 void CBank::initObj(CRandomGenerator & rand)
 {
@@ -55,10 +50,10 @@ std::string CBank::getHoverText(PlayerColor player) const
 
 void CBank::setConfig(const BankConfig & config)
 {
-	bc.reset(new BankConfig(config));
+	bc = std::make_unique<BankConfig>(config);
 	clear(); // remove all stacks, if any
 
-	for (auto & stack : config.guards)
+	for(const auto & stack : config.guards)
 		setCreature (SlotID(stacksCount()), stack.type->getId(), stack.count);
 }
 
@@ -194,7 +189,7 @@ void CBank::doVisit(const CGHeroInstance * hero) const
 				break;
 			}
 			cb->giveHeroBonus(&gbonus);
-			iw.components.push_back(Component(Component::MORALE, 0, -1, 0));
+			iw.components.emplace_back(Component::MORALE, 0, -1, 0);
 			iw.soundID = soundBase::GRAVEYARD;
 			break;
 		}
@@ -205,7 +200,7 @@ void CBank::doVisit(const CGHeroInstance * hero) const
 			gb.id = hero->id.getNum();
 			cb->giveHeroBonus(&gb);
 			textID = 107;
-			iw.components.push_back(Component(Component::LUCK, 0, -2, 0));
+			iw.components.emplace_back(Component::LUCK, 0, -2, 0);
 			break;
 		}
 		case Obj::CREATURE_BANK:
@@ -229,7 +224,7 @@ void CBank::doVisit(const CGHeroInstance * hero) const
 		{
 			if (bc->resources[it] != 0)
 			{
-				iw.components.push_back(Component(Component::RESOURCE, it, bc->resources[it], 0));
+				iw.components.emplace_back(Component::RESOURCE, it, bc->resources[it], 0);
 				loot << "%d %s";
 				loot.addReplacement(iw.components.back().val);
 				loot.addReplacement(MetaString::RES_NAMES, iw.components.back().subtype);
@@ -239,7 +234,7 @@ void CBank::doVisit(const CGHeroInstance * hero) const
 		//grant artifacts
 		for (auto & elem : bc->artifacts)
 		{
-			iw.components.push_back(Component(Component::ARTIFACT, elem, 0, 0));
+			iw.components.emplace_back(Component::ARTIFACT, elem, 0, 0);
 			loot << "%s";
 			loot.addReplacement(MetaString::ART_NAMES, elem);
 			cb->giveHeroNewArtifact(hero, VLC->arth->objects[elem], ArtifactPosition::FIRST_AVAILABLE);
@@ -276,14 +271,14 @@ void CBank::doVisit(const CGHeroInstance * hero) const
 			}
 			for(const SpellID & spellId : bc->spells)
 			{
-				auto spell = spellId.toSpell(VLC->spells());
+				const auto * spell = spellId.toSpell(VLC->spells());
 				iw.text.addTxt(MetaString::SPELL_NAME, spellId);
 				if(spell->getLevel() <= hero->maxSpellLevel())
 				{
 					if(hero->canLearnSpell(spell))
 					{
 						spells.insert(spellId);
-						iw.components.push_back(Component(Component::SPELL, spellId, 0, 0));
+						iw.components.emplace_back(Component::SPELL, spellId, 0, 0);
 					}
 				}
 				else
@@ -307,14 +302,14 @@ void CBank::doVisit(const CGHeroInstance * hero) const
 
 		//grant creatures
 		CCreatureSet ourArmy;
-		for (auto slot : bc->creatures)
+		for(const auto & slot : bc->creatures)
 		{
 			ourArmy.addToSlot(ourArmy.getSlotFor(slot.type->idNumber), slot.type->getId(), slot.count);
 		}
 
-		for (auto & elem : ourArmy.Slots())
+		for(const auto & elem : ourArmy.Slots())
 		{
-			iw.components.push_back(Component(*elem.second));
+			iw.components.emplace_back(*elem.second);
 			loot << "%s";
 			loot.addReplacement(*elem.second);
 		}

lib/mapObjects/CBank.h

@@ -28,7 +28,7 @@ class DLL_LINKAGE CBank : public CArmedInstance
 
 public:
 	CBank();
-	~CBank();
+	~CBank() override;
 
 	void setConfig(const BankConfig & bc);
 

lib/mapObjects/CGHeroInstance.cpp

@@ -38,7 +38,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 
 ///helpers
-static void showInfoDialog(const PlayerColor playerID, const ui32 txtID, const ui16 soundID = 0)
+static void showInfoDialog(const PlayerColor & playerID, const ui32 txtID, const ui16 soundID = 0)
 {
 	InfoWindow iw;
 	iw.soundID = soundID;
@@ -56,7 +56,7 @@ static void showInfoDialog(const CGHeroInstance* h, const ui32 txtID, const ui16
 static int lowestSpeed(const CGHeroInstance * chi)
 {
 	static const CSelector selectorSTACKS_SPEED = Selector::type()(Bonus::STACKS_SPEED);
-	static const std::string keySTACKS_SPEED = "type_" + std::to_string((si32)Bonus::STACKS_SPEED);
+	static const std::string keySTACKS_SPEED = "type_" + std::to_string(static_cast<si32>(Bonus::STACKS_SPEED));
 
 	if(!chi->stacksCount())
 	{
@@ -97,7 +97,7 @@ ui32 CGHeroInstance::getTileCost(const TerrainTile & dest, const TerrainTile & f
 		if(ret < GameConstants::BASE_MOVEMENT_COST)
 			ret = GameConstants::BASE_MOVEMENT_COST;
 	}
-	return (ui32)ret;
+	return static_cast<ui32>(ret);
 }
 
 TerrainId CGHeroInstance::getNativeTerrain() const
@@ -110,7 +110,7 @@ TerrainId CGHeroInstance::getNativeTerrain() const
 
 	TerrainId nativeTerrain = ETerrainId::ANY_TERRAIN;
 
-	for(auto stack : stacks)
+	for(const auto & stack : stacks)
 	{
 		TerrainId stackNativeTerrain = stack.second->type->getNativeTerrain(); //consider terrain bonuses e.g. Lodestar.
 
@@ -130,19 +130,19 @@ BattleField CGHeroInstance::getBattlefield() const
 	return BattleField::NONE;
 }
 
-ui8 CGHeroInstance::getSecSkillLevel(SecondarySkill skill) const
+ui8 CGHeroInstance::getSecSkillLevel(const SecondarySkill & skill) const
 {
-	for(auto & elem : secSkills)
+	for(const auto & elem : secSkills)
 		if(elem.first == skill)
 			return elem.second;
 	return 0;
 }
 
-void CGHeroInstance::setSecSkillLevel(SecondarySkill which, int val, bool abs)
+void CGHeroInstance::setSecSkillLevel(const SecondarySkill & which, int val, bool abs)
 {
 	if(getSecSkillLevel(which) == 0)
 	{
-		secSkills.push_back(std::pair<SecondarySkill,ui8>(which, val));
+		secSkills.emplace_back(which, val);
 		updateSkillBonus(which, val);
 	}
 	else
@@ -182,7 +182,7 @@ bool CGHeroInstance::canLearnSkill() const
 	return secSkills.size() < GameConstants::SKILL_PER_HERO;
 }
 
-bool CGHeroInstance::canLearnSkill(SecondarySkill which) const
+bool CGHeroInstance::canLearnSkill(const SecondarySkill & which) const
 {
 	if ( !canLearnSkill())
 		return false;
@@ -207,12 +207,12 @@ int CGHeroInstance::maxMovePoints(bool onLand) const
 
 int CGHeroInstance::maxMovePointsCached(bool onLand, const TurnInfo * ti) const
 {
-	int base;
+	int base = 0;
 
 	if(onLand)
 	{
 		// used function is f(x) = 66.6x + 1300, rounded to second digit, where x is lowest speed in army
-		static const int baseSpeed = 1300; // base speed from creature with 0 speed
+		static constexpr int baseSpeed = 1300; // base speed from creature with 0 speed
 
 		int armySpeed = lowestSpeed(this) * 20 / 3;
 
@@ -230,28 +230,25 @@ int CGHeroInstance::maxMovePointsCached(bool onLand, const TurnInfo * ti) const
 	const int subtype = onLand ? SecondarySkill::LOGISTICS : SecondarySkill::NAVIGATION;
 	const double modifier = ti->valOfBonuses(Bonus::SECONDARY_SKILL_PREMY, subtype) / 100.0;
 
-	return int(base * (1 + modifier)) + bonus;
+	return static_cast<int>(base * (1 + modifier)) + bonus;
 }
 
-CGHeroInstance::CGHeroInstance()
- : IBoatGenerator(this)
+CGHeroInstance::CGHeroInstance():
+	IBoatGenerator(this),
+	tacticFormationEnabled(false),
+	inTownGarrison(false),
+	isStanding(true),
+	moveDir(4),
+	mana(UNINITIALIZED_MANA),
+	movement(UNINITIALIZED_MOVEMENT),
+	portrait(UNINITIALIZED_PORTRAIT),
+	level(1),
+	exp(std::numeric_limits<TExpType>::max()),
+	sex(std::numeric_limits<ui8>::max())
 {
 	setNodeType(HERO);
 	ID = Obj::HERO;
-	tacticFormationEnabled = inTownGarrison = false;
-	mana = UNINITIALIZED_MANA;
-	movement = UNINITIALIZED_MOVEMENT;
-	portrait = UNINITIALIZED_PORTRAIT;
-	isStanding = true;
-	moveDir = 4;
-	level = 1;
-	exp = 0xffffffff;
-	visitedTown = nullptr;
-	type = nullptr;
-	boat = nullptr;
-	commander = nullptr;
-	sex = 0xff;
-	secSkills.push_back(std::make_pair(SecondarySkill::DEFAULT, -1));
+	secSkills.emplace_back(SecondarySkill::DEFAULT, -1);
 }
 
 PlayerColor CGHeroInstance::getOwner() const
@@ -259,7 +256,7 @@ PlayerColor CGHeroInstance::getOwner() const
 	return tempOwner;
 }
 
-void CGHeroInstance::initHero(CRandomGenerator & rand, HeroTypeID SUBID)
+void CGHeroInstance::initHero(CRandomGenerator & rand, const HeroTypeID & SUBID)
 {
 	subID = SUBID.getNum();
 	initHero(rand);
@@ -286,7 +283,7 @@ void CGHeroInstance::initHero(CRandomGenerator & rand)
 
 	if(!vstd::contains(spells, SpellID::PRESET)) //hero starts with a spell
 	{
-		for(auto spellID : type->spells)
+		for(const auto & spellID : type->spells)
 			spells.insert(spellID);
 	}
 	else //remove placeholder
@@ -484,11 +481,11 @@ ui8 CGHeroInstance::maxlevelsToWisdom() const
 	return type->heroClass->isMagicHero() ? 3 : 6;
 }
 
-CGHeroInstance::SecondarySkillsInfo::SecondarySkillsInfo()
+CGHeroInstance::SecondarySkillsInfo::SecondarySkillsInfo():
+	magicSchoolCounter(1),
+	wisdomCounter(1)
 {
 	rand.setSeed(0);
-	magicSchoolCounter = 1;
-	wisdomCounter = 1;
 }
 
 void CGHeroInstance::SecondarySkillsInfo::resetMagicSchoolCounter()
@@ -520,14 +517,14 @@ void CGHeroInstance::initObj(CRandomGenerator & rand)
 	}
 
 	//copy active (probably growing) bonuses from hero prototype to hero object
-	for(std::shared_ptr<Bonus> b : type->specialty)
+	for(const std::shared_ptr<Bonus> & b : type->specialty)
 		addNewBonus(b);
 	//dito for old-style bonuses -> compatibility for old savegames
 	for(SSpecialtyBonus & sb : type->specialtyDeprecated)
-		for(std::shared_ptr<Bonus> b : sb.bonuses)
+		for(const std::shared_ptr<Bonus> & b : sb.bonuses)
 			addNewBonus(b);
 	for(SSpecialtyInfo & spec : type->specDeprecated)
-		for(std::shared_ptr<Bonus> b : SpecialtyInfoToBonuses(spec, type->getIndex()))
+		for(const std::shared_ptr<Bonus> & b : SpecialtyInfoToBonuses(spec, type->getIndex()))
 			addNewBonus(b);
 
 	//initialize bonuses
@@ -539,19 +536,19 @@ void CGHeroInstance::initObj(CRandomGenerator & rand)
 void CGHeroInstance::recreateSecondarySkillsBonuses()
 {
 	auto secondarySkillsBonuses = getBonuses(Selector::sourceType()(Bonus::SECONDARY_SKILL));
-	for(auto bonus : *secondarySkillsBonuses)
+	for(const auto & bonus : *secondarySkillsBonuses)
 		removeBonus(bonus);
 
-	for(auto skill_info : secSkills)
+	for(const auto & skill_info : secSkills)
 		if(skill_info.second > 0)
 			updateSkillBonus(SecondarySkill(skill_info.first), skill_info.second);
 }
 
-void CGHeroInstance::updateSkillBonus(SecondarySkill which, int val)
+void CGHeroInstance::updateSkillBonus(const SecondarySkill & which, int val)
 {
 	removeBonuses(Selector::source(Bonus::SECONDARY_SKILL, which));
 	auto skillBonus = (*VLC->skillh)[which]->at(val).effects;
-	for (auto b : skillBonus)
+	for(const auto & b : skillBonus)
 		addNewBonus(std::make_shared<Bonus>(*b));
 }
 
@@ -579,12 +576,12 @@ double CGHeroInstance::getHeroStrength() const
 ui64 CGHeroInstance::getTotalStrength() const
 {
 	double ret = getFightingStrength() * getArmyStrength();
-	return (ui64) ret;
+	return static_cast<ui64>(ret);
 }
 
 TExpType CGHeroInstance::calculateXp(TExpType exp) const
 {
-	return (TExpType)(exp * (100 + valOfBonuses(Bonus::SECONDARY_SKILL_PREMY, SecondarySkill::LEARNING))/100.0);
+	return static_cast<TExpType>(exp * (100 + valOfBonuses(Bonus::SECONDARY_SKILL_PREMY, SecondarySkill::LEARNING)) / 100.0);
 }
 
 int32_t CGHeroInstance::getCasterUnitId() const
@@ -600,12 +597,12 @@ int32_t CGHeroInstance::getSpellSchoolLevel(const spells::Spell * spell, int32_t
 	{
 		int32_t thisSchool = std::max<int32_t>(
 			valOfBonuses(Bonus::SECONDARY_SKILL_PREMY, cnf.skill),
-			valOfBonuses(Bonus::MAGIC_SCHOOL_SKILL, 1 << ((ui8)cnf.id))); //FIXME: Bonus shouldn't be additive (Witchking Artifacts : Crown of Skies)
+			valOfBonuses(Bonus::MAGIC_SCHOOL_SKILL, 1 << (static_cast<ui8>(cnf.id)))); //FIXME: Bonus shouldn't be additive (Witchking Artifacts : Crown of Skies)
 		if(thisSchool > skill)
 		{
 			skill = thisSchool;
 			if(outSelectedSchool)
-				*outSelectedSchool = (ui8)cnf.id;
+				*outSelectedSchool = static_cast<ui8>(cnf.id);
 		}
 	});
 
@@ -621,8 +618,8 @@ int64_t CGHeroInstance::getSpellBonus(const spells::Spell * spell, int64_t base,
 {
 	//applying sorcery secondary skill
 
-	base = (int64_t)(base * (100 + valOfBonuses(Bonus::SECONDARY_SKILL_PREMY, SecondarySkill::SORCERY)) / 100.0);
-	base = (int64_t)(base * (100 + valOfBonuses(Bonus::SPELL_DAMAGE) + valOfBonuses(Bonus::SPECIFIC_SPELL_DAMAGE, spell->getIndex())) / 100.0);
+	base = static_cast<int64_t>(base * (100 + valOfBonuses(Bonus::SECONDARY_SKILL_PREMY, SecondarySkill::SORCERY)) / 100.0);
+	base = static_cast<int64_t>(base * (100 + valOfBonuses(Bonus::SPELL_DAMAGE) + valOfBonuses(Bonus::SPECIFIC_SPELL_DAMAGE, spell->getIndex())) / 100.0);
 
 	int maxSchoolBonus = 0;
 
@@ -631,17 +628,17 @@ int64_t CGHeroInstance::getSpellBonus(const spells::Spell * spell, int64_t base,
 		vstd::amax(maxSchoolBonus, valOfBonuses(cnf.damagePremyBonus));
 	});
 
-	base = (int64_t)(base * (100 + maxSchoolBonus) / 100.0);
+	base = static_cast<int64_t>(base * (100 + maxSchoolBonus) / 100.0);
 
 	if(affectedStack && affectedStack->creatureLevel() > 0) //Hero specials like Solmyr, Deemer
-		base = (int64_t)(base * double(100 + valOfBonuses(Bonus::SPECIAL_SPELL_LEV, spell->getIndex()) / affectedStack->creatureLevel()) / 100.0);
+		base = static_cast<int64_t>(base * static_cast<double>(100 + valOfBonuses(Bonus::SPECIAL_SPELL_LEV, spell->getIndex()) / affectedStack->creatureLevel()) / 100.0);
 
 	return base;
 }
 
 int64_t CGHeroInstance::getSpecificSpellBonus(const spells::Spell * spell, int64_t base) const
 {
-	base = (int64_t)(base * (100 + valOfBonuses(Bonus::SPECIFIC_SPELL_DAMAGE, spell->getIndex())) / 100.0);
+	base = static_cast<int64_t>(base * (100 + valOfBonuses(Bonus::SPECIFIC_SPELL_DAMAGE, spell->getIndex())) / 100.0);
 	return base;
 }
 
@@ -798,17 +795,17 @@ CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &b
 		TConstBonusListPtr improvedNecromancy = getBonuses(Selector::type()(Bonus::IMPROVED_NECROMANCY));
 		if(!improvedNecromancy->empty())
 		{
-			auto getCreatureID = [necromancyLevel](std::shared_ptr<Bonus> bonus) -> CreatureID
+			auto getCreatureID = [necromancyLevel](const std::shared_ptr<Bonus> & bonus) -> CreatureID
 			{
 				const CreatureID legacyTypes[] = {CreatureID::SKELETON, CreatureID::WALKING_DEAD, CreatureID::WIGHTS, CreatureID::LICHES};
 				return CreatureID(bonus->subtype >= 0 ? bonus->subtype : legacyTypes[necromancyLevel]);
 			};
 			int maxCasualtyLevel = 1;
-			for(auto & casualty : casualties)
+			for(const auto & casualty : casualties)
 				vstd::amax(maxCasualtyLevel, VLC->creh->objects[casualty.first]->level);
 			// pick best bonus available
 			std::shared_ptr<Bonus> topPick;
-			for(std::shared_ptr<Bonus> newPick : *improvedNecromancy)
+			for(const std::shared_ptr<Bonus> & newPick : *improvedNecromancy)
 			{
 				// addInfo[0] = required necromancy skill, addInfo[1] = required casualty level
 				if(newPick->additionalInfo[0] > necromancyLevel || newPick->additionalInfo[1] > maxCasualtyLevel)
@@ -819,7 +816,7 @@ CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &b
 				}
 				else
 				{
-					auto quality = [getCreatureID](std::shared_ptr<Bonus> pick) -> std::tuple<int, int, int>
+					auto quality = [getCreatureID](const std::shared_ptr<Bonus> & pick) -> std::tuple<int, int, int>
 					{
 						const CCreature * c = VLC->creh->objects[getCreatureID(pick)];
 						return std::tuple<int, int, int> {c->level, static_cast<int>(c->cost.marketValue()), -pick->additionalInfo[1]};
@@ -837,7 +834,7 @@ CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &b
 		// raise upgraded creature (at 2/3 rate) if no space available otherwise
 		if(getSlotFor(creatureTypeRaised) == SlotID())
 		{
-			for(CreatureID upgraded : VLC->creh->objects[creatureTypeRaised]->upgrades)
+			for(const CreatureID & upgraded : VLC->creh->objects[creatureTypeRaised]->upgrades)
 			{
 				if(getSlotFor(upgraded) != SlotID())
 				{
@@ -850,7 +847,7 @@ CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &b
 		// calculate number of creatures raised - low level units contribute at 50% rate
 		const double raisedUnitHealth = VLC->creh->objects[creatureTypeRaised]->MaxHealth();
 		double raisedUnits = 0;
-		for(auto & casualty : casualties)
+		for(const auto & casualty : casualties)
 		{
 			const CCreature * c = VLC->creh->objects[casualty.first];
 			double raisedFromCasualty = std::min(c->MaxHealth() / raisedUnitHealth, 1.0) * casualty.second * necromancySkill;
@@ -874,7 +871,7 @@ void CGHeroInstance::showNecromancyDialog(const CStackBasicDescriptor &raisedSta
 	InfoWindow iw;
 	iw.soundID = soundBase::pickup01 + rand.nextInt(6);
 	iw.player = tempOwner;
-	iw.components.push_back(Component(raisedStack));
+	iw.components.emplace_back(raisedStack);
 
 	if (raisedStack.count > 1) // Practicing the dark arts of necromancy, ... (plural)
 	{
@@ -1034,17 +1031,17 @@ bool CGHeroInstance::hasSpellbook() const
 	return getArt(ArtifactPosition::SPELLBOOK);
 }
 
-void CGHeroInstance::addSpellToSpellbook(SpellID spell)
+void CGHeroInstance::addSpellToSpellbook(const SpellID & spell)
 {
 	spells.insert(spell);
 }
 
-void CGHeroInstance::removeSpellFromSpellbook(SpellID spell)
+void CGHeroInstance::removeSpellFromSpellbook(const SpellID & spell)
 {
 	spells.erase(spell);
 }
 
-bool CGHeroInstance::spellbookContainsSpell(SpellID spell) const
+bool CGHeroInstance::spellbookContainsSpell(const SpellID & spell) const
 {
 	return vstd::contains(spells, spell);
 }
@@ -1128,7 +1125,7 @@ int CGHeroInstance::movementPointsAfterEmbark(int MPsBefore, int basicCost, bool
 
 EDiggingStatus CGHeroInstance::diggingStatus() const
 {
-	if((int)movement < maxMovePoints(true))
+	if(static_cast<int>(movement) < maxMovePoints(true))
 		return EDiggingStatus::LACK_OF_MOVEMENT;
 
 	return cb->getTile(visitablePos())->getDiggingStatus();
@@ -1145,7 +1142,7 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 	if (!skillsInfo.wisdomCounter)
 	{
 		if (canLearnSkill(SecondarySkill::WISDOM))
-			obligatorySkills.push_back(SecondarySkill::WISDOM);
+			obligatorySkills.emplace_back(SecondarySkill::WISDOM);
 	}
 	if (!skillsInfo.magicSchoolCounter)
 	{
@@ -1156,7 +1153,7 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 
 		std::shuffle(ss.begin(), ss.end(), skillsInfo.rand.getStdGenerator());
 
-		for (auto skill : ss)
+		for(const auto & skill : ss)
 		{
 			if (canLearnSkill(skill)) //only schools hero doesn't know yet
 			{
@@ -1168,12 +1165,14 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 
 	std::vector<SecondarySkill> skills;
 	//picking sec. skills for choice
-	std::set<SecondarySkill> basicAndAdv, expert, none;
+	std::set<SecondarySkill> basicAndAdv;
+	std::set<SecondarySkill> expert;
+	std::set<SecondarySkill> none;
 	for(int i = 0; i < VLC->skillh->size(); i++)
 		if (canLearnSkill(SecondarySkill(i)))
 			none.insert(SecondarySkill(i));
 
-	for(auto & elem : secSkills)
+	for(const auto & elem : secSkills)
 	{
 		if(elem.second < SecSkillLevel::EXPERT)
 			basicAndAdv.insert(elem.first);
@@ -1181,7 +1180,7 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 			expert.insert(elem.first);
 		none.erase(elem.first);
 	}
-	for (auto s : obligatorySkills) //don't duplicate them
+	for(const auto & s : obligatorySkills) //don't duplicate them
 	{
 		none.erase (s);
 		basicAndAdv.erase (s);
@@ -1192,11 +1191,11 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 	// 1) give obligatory skill
 	// 2) give any other new skill
 	// 3) upgrade existing
-	if (canLearnSkill() && obligatorySkills.size() > 0)
+	if(canLearnSkill() && !obligatorySkills.empty())
 	{
 		skills.push_back (obligatorySkills[0]);
 	}
-	else if(none.size() && canLearnSkill()) //hero have free skill slot
+	else if(!none.empty() && canLearnSkill()) //hero have free skill slot
 	{
 		skills.push_back(type->heroClass->chooseSecSkill(none, skillsInfo.rand)); //new skill
 		none.erase(skills.back());
@@ -1221,7 +1220,7 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 	{
 		skills.push_back (obligatorySkills[1]);
 	}
-	else if(none.size() && canLearnSkill())
+	else if(!none.empty() && canLearnSkill())
 	{
 		skills.push_back(type->heroClass->chooseSecSkill(none, skillsInfo.rand)); //give new skill
 		none.erase(skills.back());
@@ -1235,7 +1234,9 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 PrimarySkill::PrimarySkill CGHeroInstance::nextPrimarySkill(CRandomGenerator & rand) const
 {
 	assert(gainsLevel());
-	int randomValue = rand.nextInt(99), pom = 0, primarySkill = 0;
+	int randomValue = rand.nextInt(99);
+	int pom = 0;
+	int primarySkill = 0;
 	const auto isLowLevelHero = level < GameConstants::HERO_HIGH_LEVEL;
 	const auto & skillChances = isLowLevelHero ? type->heroClass->primarySkillLowLevel : type->heroClass->primarySkillHighLevel;
 
@@ -1266,7 +1267,7 @@ boost::optional<SecondarySkill> CGHeroInstance::nextSecondarySkill(CRandomGenera
 	if(!proposedSecondarySkills.empty())
 	{
 		std::vector<SecondarySkill> learnedSecondarySkills;
-		for(auto secondarySkill : proposedSecondarySkills)
+		for(const auto & secondarySkill : proposedSecondarySkills)
 		{
 			if(getSecSkillLevel(secondarySkill) > 0)
 			{
@@ -1325,7 +1326,7 @@ bool CGHeroInstance::gainsLevel() const
 	return exp >= static_cast<TExpType>(VLC->heroh->reqExp(level+1));
 }
 
-void CGHeroInstance::levelUp(std::vector<SecondarySkill> skills)
+void CGHeroInstance::levelUp(const std::vector<SecondarySkill> & skills)
 {
 	++level;
 
@@ -1339,7 +1340,7 @@ void CGHeroInstance::levelUp(std::vector<SecondarySkill> skills)
 
 	SecondarySkill spellSchools[] = {
 		SecondarySkill::FIRE_MAGIC, SecondarySkill::AIR_MAGIC, SecondarySkill::WATER_MAGIC, SecondarySkill::EARTH_MAGIC};
-	for(auto skill : spellSchools)
+	for(const auto & skill : spellSchools)
 	{
 		if(vstd::contains(skills, skill))
 		{
@@ -1411,7 +1412,7 @@ std::string CGHeroInstance::getHeroTypeName() const
 void CGHeroInstance::afterAddToMap(CMap * map)
 {
 	if(ID == Obj::HERO)
-		map->heroesOnMap.push_back(this);
+		map->heroesOnMap.emplace_back(this);
 }
 void CGHeroInstance::afterRemoveFromMap(CMap* map)
 {
@@ -1554,7 +1555,7 @@ void CGHeroInstance::serializeCommonOptions(JsonSerializeFormat & handler)
 		secSkills.clear();
 		if(skillMap.getType() == JsonNode::JsonType::DATA_NULL)
 		{
-			secSkills.push_back(std::pair<SecondarySkill,ui8>(SecondarySkill::DEFAULT, -1));
+			secSkills.emplace_back(SecondarySkill::DEFAULT, -1);
 		}
 		else
 		{
@@ -1577,7 +1578,7 @@ void CGHeroInstance::serializeCommonOptions(JsonSerializeFormat & handler)
 					continue;
 				}
 
-				secSkills.push_back(std::pair<SecondarySkill,ui8>(SecondarySkill(rawId), level));
+				secSkills.emplace_back(SecondarySkill(rawId), level);
 			}
 		}
 	}
@@ -1608,7 +1609,7 @@ void CGHeroInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 	handler.serializeBool<ui8>("tightFormation", formation, 1, 0, 0);
 
 	{
-		static const int NO_PATROLING = -1;
+		static constexpr int NO_PATROLING = -1;
 		int rawPatrolRadius = NO_PATROLING;
 
 		if(handler.saving)
@@ -1640,7 +1641,7 @@ bool CGHeroInstance::isMissionCritical() const
 		{
 			if ((condition.condition == EventCondition::CONTROL || condition.condition == EventCondition::HAVE_0) && condition.object)
 			{
-				auto hero = dynamic_cast<const CGHeroInstance*>(condition.object);
+				const auto * hero = dynamic_cast<const CGHeroInstance *>(condition.object);
 				return (hero != this);
 			}
 			else if(condition.condition == EventCondition::IS_HUMAN)

lib/mapObjects/CGHeroInstance.h

@@ -53,11 +53,12 @@ private:
 public:
 
 	//////////////////////////////////////////////////////////////////////////
-
-	ui8 moveDir; //format:	123
-					//		8 4
-					//		765
-	mutable ui8 isStanding, tacticFormationEnabled;
+	//format:   123
+	//          8 4
+	//          765
+	ui8 moveDir;
+	mutable ui8 isStanding;
+	mutable ui8 tacticFormationEnabled;
 
 	//////////////////////////////////////////////////////////////////////////
 
@@ -76,11 +77,11 @@ public:
 	bool inTownGarrison; // if hero is in town garrison
 	ConstTransitivePtr<CGTownInstance> visitedTown; //set if hero is visiting town or in the town garrison
 	ConstTransitivePtr<CCommanderInstance> commander;
-	const CGBoat *boat; //set to CGBoat when sailing
+	const CGBoat * boat = nullptr; //set to CGBoat when sailing
 
-	static const si32 UNINITIALIZED_PORTRAIT = -1;
-	static const si32 UNINITIALIZED_MANA = -1;
-	static const ui32 UNINITIALIZED_MOVEMENT = -1;
+	static constexpr si32 UNINITIALIZED_PORTRAIT = -1;
+	static constexpr si32 UNINITIALIZED_MANA = -1;
+	static constexpr ui32 UNINITIALIZED_MOVEMENT = -1;
 
 	//std::vector<const CArtifact*> artifacts; //hero's artifacts from bag
 	//std::map<ui16, const CArtifact*> artifWorn; //map<position,artifact_id>; positions: 0 - head; 1 - shoulders; 2 - neck; 3 - right hand; 4 - left hand; 5 - torso; 6 - right ring; 7 - left ring; 8 - feet; 9 - misc1; 10 - misc2; 11 - misc3; 12 - misc4; 13 - mach1; 14 - mach2; 15 - mach3; 16 - mach4; 17 - spellbook; 18 - misc5
@@ -157,15 +158,15 @@ public:
 
 	bool hasSpellbook() const;
 	int maxSpellLevel() const;
-	void addSpellToSpellbook(SpellID spell);
-	void removeSpellFromSpellbook(SpellID spell);
-	bool spellbookContainsSpell(SpellID spell) const;
+	void addSpellToSpellbook(const SpellID & spell);
+	void removeSpellFromSpellbook(const SpellID & spell);
+	bool spellbookContainsSpell(const SpellID & spell) const;
 	void removeSpellbook();
 	const std::set<SpellID> & getSpellsInSpellbook() const;
 	EAlignment::EAlignment getAlignment() const;
 	bool needsLastStack()const override;
 
-	ui32 getTileCost(const TerrainTile &dest, const TerrainTile &from, const TurnInfo * ti) const; //move cost - applying pathfinding skill, road and terrain modifiers. NOT includes diagonal move penalty, last move levelling
+	ui32 getTileCost(const TerrainTile & dest, const TerrainTile & from, const TurnInfo * ti) const; //move cost - applying pathfinding skill, road and terrain modifiers. NOT includes diagonal move penalty, last move levelling
 	TerrainId getNativeTerrain() const;
 	ui32 getLowestCreatureSpeed() const;
 	si32 manaRegain() const; //how many points of mana can hero regain "naturally" in one day
@@ -194,15 +195,15 @@ public:
 	/// Gets 0, 1 or 2 secondary skills which are proposed on hero level up.
 	std::vector<SecondarySkill> getLevelUpProposedSecondarySkills() const;
 
-	ui8 getSecSkillLevel(SecondarySkill skill) const; //0 - no skill
+	ui8 getSecSkillLevel(const SecondarySkill & skill) const; //0 - no skill
 
 	/// Returns true if hero has free secondary skill slot.
 	bool canLearnSkill() const;
-	bool canLearnSkill(SecondarySkill which) const;
+	bool canLearnSkill(const SecondarySkill & which) const;
 
 	void setPrimarySkill(PrimarySkill::PrimarySkill primarySkill, si64 value, ui8 abs);
-	void setSecSkillLevel(SecondarySkill which, int val, bool abs);// abs == 0 - changes by value; 1 - sets to value
-	void levelUp(std::vector<SecondarySkill> skills);
+	void setSecSkillLevel(const SecondarySkill & which, int val, bool abs); // abs == 0 - changes by value; 1 - sets to value
+	void levelUp(const std::vector<SecondarySkill> & skills);
 
 	int maxMovePoints(bool onLand) const;
 	//cached version is much faster, TurnInfo construction is costly
@@ -225,7 +226,7 @@ public:
 	void setType(si32 ID, si32 subID) override;
 
 	void initHero(CRandomGenerator & rand);
-	void initHero(CRandomGenerator & rand, HeroTypeID SUBID);
+	void initHero(CRandomGenerator & rand, const HeroTypeID & SUBID);
 
 	void putArtifact(ArtifactPosition pos, CArtifactInstance * art) override;
 	void putInBackpack(CArtifactInstance *art);
@@ -236,7 +237,7 @@ public:
 	ui8 maxlevelsToMagicSchool() const;
 	ui8 maxlevelsToWisdom() const;
 	void recreateSecondarySkillsBonuses();
-	void updateSkillBonus(SecondarySkill which, int val);
+	void updateSkillBonus(const SecondarySkill & which, int val);
 
 	bool hasVisions(const CGObjectInstance * target, const int subtype) const;
 	/// If this hero perishes, the scenario is failed

lib/mapObjects/CGMarket.cpp

@@ -40,17 +40,17 @@ bool IMarket::getOffer(int id1, int id2, int &val1, int &val2, EMarketMode::EMar
 		{
 			double effectiveness = std::min((getMarketEfficiency() + 1.0) / 20.0, 0.5);
 
-			double r = VLC->objh->resVals[id1], //value of given resource
-				g = VLC->objh->resVals[id2] / effectiveness; //value of wanted resource
+			double r = VLC->objh->resVals[id1]; //value of given resource
+			double g = VLC->objh->resVals[id2] / effectiveness; //value of wanted resource
 
 			if(r>g) //if given resource is more expensive than wanted
 			{
-				val2 = (int)ceil(r / g);
+				val2 = static_cast<int>(ceil(r / g));
 				val1 = 1;
 			}
 			else //if wanted resource is more expensive
 			{
-				val1 = (int)((g / r) + 0.5);
+				val1 = static_cast<int>((g / r) + 0.5);
 				val2 = 1;
 			}
 		}
@@ -60,17 +60,17 @@ bool IMarket::getOffer(int id1, int id2, int &val1, int &val2, EMarketMode::EMar
 			const double effectivenessArray[] = {0.0, 0.3, 0.45, 0.50, 0.65, 0.7, 0.85, 0.9, 1.0};
 			double effectiveness = effectivenessArray[std::min(getMarketEfficiency(), 8)];
 
-			double r = VLC->creh->objects[id1]->cost[6], //value of given creature in gold
-				g = VLC->objh->resVals[id2] / effectiveness; //value of wanted resource
+			double r = VLC->creh->objects[id1]->cost[6]; //value of given creature in gold
+			double g = VLC->objh->resVals[id2] / effectiveness; //value of wanted resource
 
 			if(r>g) //if given resource is more expensive than wanted
 			{
-				val2 = (int)ceil(r / g);
+				val2 = static_cast<int>(ceil(r / g));
 				val1 = 1;
 			}
 			else //if wanted resource is more expensive
 			{
-				val1 = (int)((g / r) + 0.5);
+				val1 = static_cast<int>((g / r) + 0.5);
 				val2 = 1;
 			}
 		}
@@ -82,27 +82,27 @@ bool IMarket::getOffer(int id1, int id2, int &val1, int &val2, EMarketMode::EMar
 	case EMarketMode::RESOURCE_ARTIFACT:
 		{
 			double effectiveness = std::min((getMarketEfficiency() + 3.0) / 20.0, 0.6);
-			double r = VLC->objh->resVals[id1], //value of offered resource
-				g = VLC->artifacts()->getByIndex(id2)->getPrice() / effectiveness; //value of bought artifact in gold
+			double r = VLC->objh->resVals[id1]; //value of offered resource
+			double g = VLC->artifacts()->getByIndex(id2)->getPrice() / effectiveness; //value of bought artifact in gold
 
 			if(id1 != 6) //non-gold prices are doubled
 				r /= 2;
 
-			val1 = std::max(1, (int)((g / r) + 0.5)); //don't sell arts for less than 1 resource
+			val1 = std::max(1, static_cast<int>((g / r) + 0.5)); //don't sell arts for less than 1 resource
 			val2 = 1;
 		}
 		break;
 	case EMarketMode::ARTIFACT_RESOURCE:
 		{
 			double effectiveness = std::min((getMarketEfficiency() + 3.0) / 20.0, 0.6);
-			double r = VLC->artifacts()->getByIndex(id1)->getPrice() * effectiveness,
-				g = VLC->objh->resVals[id2];
+			double r = VLC->artifacts()->getByIndex(id1)->getPrice() * effectiveness;
+			double g = VLC->objh->resVals[id2];
 
 // 			if(id2 != 6) //non-gold prices are doubled
 // 				r /= 2;
 
 			val1 = 1;
-			val2 = std::max(1, (int)((r / g) + 0.5)); //at least one resource is given in return
+			val2 = std::max(1, static_cast<int>((r / g) + 0.5)); //at least one resource is given in return
 		}
 		break;
 	case EMarketMode::CREATURE_EXP:
@@ -122,7 +122,7 @@ bool IMarket::getOffer(int id1, int id2, int &val1, int &val2, EMarketMode::EMar
 				return false;
 			}
 
-			static const int expPerClass[] = {1000, 1500, 3000, 6000};
+			static constexpr int expPerClass[] = {1000, 1500, 3000, 6000};
 			val2 = expPerClass[givenClass];
 		}
 		break;
@@ -171,15 +171,15 @@ const IMarket * IMarket::castFrom(const CGObjectInstance *obj, bool verbose)
 	switch(obj->ID)
 	{
 	case Obj::TOWN:
-		return static_cast<const CGTownInstance*>(obj);
+		return dynamic_cast<const CGTownInstance *>(obj);
 	case Obj::ALTAR_OF_SACRIFICE:
 	case Obj::BLACK_MARKET:
 	case Obj::TRADING_POST:
 	case Obj::TRADING_POST_SNOW:
 	case Obj::FREELANCERS_GUILD:
-		return static_cast<const CGMarket*>(obj);
+		return dynamic_cast<const CGMarket *>(obj);
 	case Obj::UNIVERSITY:
-		return static_cast<const CGUniversity*>(obj);
+		return dynamic_cast<const CGUniversity *>(obj);
 	default:
 		if(verbose)
 			logGlobal->error("Cannot cast to IMarket object with ID %d", obj->ID);
@@ -197,8 +197,8 @@ std::vector<EMarketMode::EMarketMode> IMarket::availableModes() const
 {
 	std::vector<EMarketMode::EMarketMode> ret;
 	for (int i = 0; i < EMarketMode::MARTKET_AFTER_LAST_PLACEHOLDER; i++)
-		if(allowsTrade((EMarketMode::EMarketMode)i))
-			ret.push_back((EMarketMode::EMarketMode)i);
+	if(allowsTrade(static_cast<EMarketMode::EMarketMode>(i)))
+		ret.push_back(static_cast<EMarketMode::EMarketMode>(i));
 
 	return ret;
 }

lib/mapObjects/CGPandoraBox.cpp

@@ -26,7 +26,7 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 ///helpers
-static void showInfoDialog(const PlayerColor playerID, const ui32 txtID, const ui16 soundID)
+static void showInfoDialog(const PlayerColor & playerID, const ui32 txtID, const ui16 soundID)
 {
 	InfoWindow iw;
 	iw.soundID = soundID;
@@ -41,13 +41,6 @@ static void showInfoDialog(const CGHeroInstance* h, const ui32 txtID, const ui16
 	showInfoDialog(playerID,txtID,soundID);
 }
 
-CGPandoraBox::CGPandoraBox()
-	: hasGuardians(false), gainedExp(0), manaDiff(0), moraleDiff(0), luckDiff(0)
-{
-
-}
-
-
 void CGPandoraBox::initObj(CRandomGenerator & rand)
 {
 	blockVisit = (ID==Obj::PANDORAS_BOX); //block only if it's really pandora's box (events also derive from that class)
@@ -70,7 +63,7 @@ void CGPandoraBox::giveContentsUpToExp(const CGHeroInstance *h) const
 	iw.player = h->getOwner();
 
 	bool changesPrimSkill = false;
-	for (auto & elem : primskills)
+	for(const auto & elem : primskills)
 	{
 		if(elem)
 		{
@@ -93,11 +86,11 @@ void CGPandoraBox::giveContentsUpToExp(const CGHeroInstance *h) const
 
 		if ((curLev && curLev < abilityLevels[i]) || abilityCanUseSlot)
 		{
-			unpossessedAbilities.push_back({ abilities[i], abilityLevels[i] });
+			unpossessedAbilities.emplace_back(abilities[i], abilityLevels[i]);
 		}
 	}
 
-	if(gainedExp || changesPrimSkill || unpossessedAbilities.size())
+	if(gainedExp || changesPrimSkill || !unpossessedAbilities.empty())
 	{
 		TExpType expVal = h->calculateXp(gainedExp);
 		//getText(iw,afterBattle,175,h); //wtf?
@@ -105,19 +98,19 @@ void CGPandoraBox::giveContentsUpToExp(const CGHeroInstance *h) const
 		iw.text.addReplacement(h->getNameTranslated());
 
 		if(expVal)
-			iw.components.push_back(Component(Component::EXPERIENCE,0,static_cast<si32>(expVal),0));
+			iw.components.emplace_back(Component::EXPERIENCE, 0, static_cast<si32>(expVal), 0);
 
 		for(int i=0; i<primskills.size(); i++)
 			if(primskills[i])
-				iw.components.push_back(Component(Component::PRIM_SKILL,i,primskills[i],0));
+				iw.components.emplace_back(Component::PRIM_SKILL, i, primskills[i], 0);
 
-		for(auto abilityData : unpossessedAbilities)
-			iw.components.push_back(Component(Component::SEC_SKILL, abilityData.first, abilityData.second, 0));
+		for(const auto & abilityData : unpossessedAbilities)
+			iw.components.emplace_back(Component::SEC_SKILL, abilityData.first, abilityData.second, 0);
 
 		cb->showInfoDialog(&iw);
 
 		//give sec skills
-		for (auto abilityData : unpossessedAbilities)
+		for(const auto & abilityData : unpossessedAbilities)
 			cb->changeSecSkill(h, abilityData.first, abilityData.second, true);
 
 		assert(h->secSkills.size() <= GameConstants::SKILL_PER_HERO);
@@ -149,7 +142,7 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 	iw.player = h->getOwner();
 
 	//TODO: reuse this code for Scholar skill
-	if(spells.size())
+	if(!spells.empty())
 	{
 		std::set<SpellID> spellsToGive;
 
@@ -162,10 +155,10 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 
 			for (; i != spells.cend(); i++)
 			{
-				auto spell = (*i).toSpell(VLC->spells());
+				const auto * spell = (*i).toSpell(VLC->spells());
 				if(h->canLearnSpell(spell))
 				{
-					iw.components.push_back(Component(Component::SPELL, *i, 0, 0));
+					iw.components.emplace_back(Component::SPELL, *i, 0, 0);
 					spellsToGive.insert(*i);
 				}
 				if(spellsToGive.size() == 8) //display up to 8 spells at once
@@ -193,7 +186,7 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 	if(manaDiff)
 	{
 		getText(iw,hadGuardians,manaDiff,176,177,h);
-		iw.components.push_back(Component(Component::PRIM_SKILL,5,manaDiff,0));
+		iw.components.emplace_back(Component::PRIM_SKILL, 5, manaDiff, 0);
 		cb->showInfoDialog(&iw);
 		cb->setManaPoints(h->id, h->mana + manaDiff);
 	}
@@ -201,7 +194,7 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 	if(moraleDiff)
 	{
 		getText(iw,hadGuardians,moraleDiff,178,179,h);
-		iw.components.push_back(Component(Component::MORALE,0,moraleDiff,0));
+		iw.components.emplace_back(Component::MORALE, 0, moraleDiff, 0);
 		cb->showInfoDialog(&iw);
 		GiveBonus gb;
 		gb.bonus = Bonus(Bonus::ONE_BATTLE,Bonus::MORALE,Bonus::OBJECT,moraleDiff,id.getNum(),"");
@@ -212,7 +205,7 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 	if(luckDiff)
 	{
 		getText(iw,hadGuardians,luckDiff,180,181,h);
-		iw.components.push_back(Component(Component::LUCK,0,luckDiff,0));
+		iw.components.emplace_back(Component::LUCK, 0, luckDiff, 0);
 		cb->showInfoDialog(&iw);
 		GiveBonus gb;
 		gb.bonus = Bonus(Bonus::ONE_BATTLE,Bonus::LUCK,Bonus::OBJECT,luckDiff,id.getNum(),"");
@@ -225,9 +218,9 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 	for(int i=0; i<resources.size(); i++)
 	{
 		if(resources[i] < 0)
-			iw.components.push_back(Component(Component::RESOURCE,i,resources[i],0));
+			iw.components.emplace_back(Component::RESOURCE, i, resources[i], 0);
 	}
-	if(iw.components.size())
+	if(!iw.components.empty())
 	{
 		getText(iw,hadGuardians,182,h);
 		cb->showInfoDialog(&iw);
@@ -238,9 +231,9 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 	for(int i=0; i<resources.size(); i++)
 	{
 		if(resources[i] > 0)
-			iw.components.push_back(Component(Component::RESOURCE,i,resources[i],0));
+			iw.components.emplace_back(Component::RESOURCE, i, resources[i], 0);
 	}
-	if(iw.components.size())
+	if(!iw.components.empty())
 	{
 		getText(iw,hadGuardians,183,h);
 		cb->showInfoDialog(&iw);
@@ -250,9 +243,9 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 	// 	getText(iw,afterBattle,183,h);
 	iw.text.addTxt(MetaString::ADVOB_TXT, 183); //% has found treasure
 	iw.text.addReplacement(h->getNameTranslated());
-	for(auto & elem : artifacts)
+	for(const auto & elem : artifacts)
 	{
-		iw.components.push_back(Component(Component::ARTIFACT,elem,0,0));
+		iw.components.emplace_back(Component::ARTIFACT, elem, 0, 0);
 		if(iw.components.size() >= 14)
 		{
 			cb->showInfoDialog(&iw);
@@ -261,14 +254,14 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 			iw.text.addReplacement(h->getNameTranslated());
 		}
 	}
-	if(iw.components.size())
+	if(!iw.components.empty())
 	{
 		cb->showInfoDialog(&iw);
 	}
 
 	cb->giveResources(h->getOwner(), resources);
 
-	for(auto & elem : artifacts)
+	for(const auto & elem : artifacts)
 		cb->giveHeroNewArtifact(h, VLC->arth->objects[elem],ArtifactPosition::FIRST_AVAILABLE);
 
 	iw.components.clear();
@@ -277,9 +270,9 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 	if(creatures.stacksCount())
 	{ //this part is taken straight from creature bank
 		MetaString loot;
-		for(auto & elem : creatures.Slots())
+		for(const auto & elem : creatures.Slots())
 		{ //build list of joined creatures
-			iw.components.push_back(Component(*elem.second));
+			iw.components.emplace_back(*elem.second);
 			loot << "%s";
 			loot.addReplacement(*elem.second);
 		}
@@ -295,7 +288,7 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 		cb->showInfoDialog(&iw);
 		cb->giveCreatures(this, h, creatures, false);
 	}
-	if(!hasGuardians && msg.size())
+	if(!hasGuardians && !msg.empty())
 	{
 		iw.text << msg;
 		cb->showInfoDialog(&iw);
@@ -304,7 +297,7 @@ void CGPandoraBox::giveContentsAfterExp(const CGHeroInstance *h) const
 
 void CGPandoraBox::getText( InfoWindow &iw, bool &afterBattle, int text, const CGHeroInstance * h ) const
 {
-	if(afterBattle || !message.size())
+	if(afterBattle || message.empty())
 	{
 		iw.text.addTxt(MetaString::ADVOB_TXT,text);//%s has lost treasure.
 		iw.text.addReplacement(h->getNameTranslated());
@@ -320,7 +313,7 @@ void CGPandoraBox::getText( InfoWindow &iw, bool &afterBattle, int val, int nega
 {
 	iw.components.clear();
 	iw.text.clear();
-	if(afterBattle || !message.size())
+	if(afterBattle || message.empty())
 	{
 		iw.text.addTxt(MetaString::ADVOB_TXT,val < 0 ? negative : positive); //%s's luck takes a turn for the worse / %s's luck increases
 		iw.text.addReplacement(h->getNameTranslated());
@@ -349,10 +342,10 @@ void CGPandoraBox::blockingDialogAnswered(const CGHeroInstance *hero, ui32 answe
 			showInfoDialog(hero,16,0);
 			cb->startBattleI(hero, this); //grants things after battle
 		}
-		else if(message.size() == 0 && resources.size() == 0
-			&& primskills.size() == 0 && abilities.size() == 0
-			&& abilityLevels.size() == 0 && artifacts.size() == 0
-			&& spells.size() == 0 && creatures.stacksCount() > 0
+		else if(message.empty() && resources.empty()
+			&& primskills.empty() && abilities.empty()
+			&& abilityLevels.empty() && artifacts.empty()
+			&& spells.empty() && creatures.stacksCount() > 0
 			&& gainedExp == 0 && manaDiff == 0 && moraleDiff == 0 && luckDiff == 0) //if it gives nothing without battle
 		{
 			showInfoDialog(hero,15,0);
@@ -375,12 +368,6 @@ void CGPandoraBox::afterSuccessfulVisit() const
 	cb->removeAfterVisit(this);
 }
 
-CGEvent::CGEvent()
-	: CGPandoraBox(), removeAfterVisit(false), availableFor(0), computerActivate(false), humanActivate(false)
-{
-
-}
-
 void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 {
 	CCreatureSet::serializeJson(handler, "guards", 7);
@@ -398,8 +385,8 @@ void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 
 		if(handler.saving)
 		{
-			for(int idx = 0; idx < primskills.size(); idx ++)
-				if(primskills[idx] != 0)
+			for(int primskill : primskills)
+				if(primskill != 0)
 					haveSkills = true;
 		}
 		else
@@ -456,7 +443,7 @@ void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 				continue;
 			}
 
-			abilities.push_back(SecondarySkill(rawId));
+			abilities.emplace_back(rawId);
 			abilityLevels.push_back(level);
 		}
 	}
@@ -487,7 +474,7 @@ void CGEvent::activated( const CGHeroInstance * h ) const
 	{
 		InfoWindow iw;
 		iw.player = h->tempOwner;
-		if(message.size())
+		if(!message.empty())
 			iw.text << message;
 		else
 			iw.text.addTxt(MetaString::ADVOB_TXT, 16);

lib/mapObjects/CGPandoraBox.h

@@ -21,13 +21,13 @@ class DLL_LINKAGE CGPandoraBox : public CArmedInstance
 {
 public:
 	std::string message;
-	mutable bool hasGuardians; //helper - after battle even though we have no stacks, allows us to know that there was battle
+	mutable bool hasGuardians = false; //helper - after battle even though we have no stacks, allows us to know that there was battle
 
 	//gained things:
-	ui32 gainedExp;
-	si32 manaDiff; //amount of gained / lost mana
-	si32 moraleDiff; //morale modifier
-	si32 luckDiff; //luck modifier
+	ui32 gainedExp = 0;
+	si32 manaDiff = 0; //amount of gained / lost mana
+	si32 moraleDiff = 0; //morale modifier
+	si32 luckDiff = 0; //luck modifier
 	TResources resources;//gained / lost resources
 	std::vector<si32> primskills;//gained / lost prim skills
 	std::vector<SecondarySkill> abilities; //gained abilities
@@ -36,7 +36,6 @@ public:
 	std::vector<SpellID> spells; //gained spells
 	CCreatureSet creatures; //gained creatures
 
-	CGPandoraBox();
 	void initObj(CRandomGenerator & rand) override;
 	void onHeroVisit(const CGHeroInstance * h) const override;
 	void battleFinished(const CGHeroInstance *hero, const BattleResult &result) const override;
@@ -73,10 +72,10 @@ private:
 class DLL_LINKAGE CGEvent : public CGPandoraBox  //event objects
 {
 public:
-	bool removeAfterVisit; //true if event is removed after occurring
-	ui8 availableFor; //players whom this event is available for
-	bool computerActivate; //true if computer player can activate this event
-	bool humanActivate; //true if human player can activate this event
+	bool removeAfterVisit = false; //true if event is removed after occurring
+	ui8 availableFor = 0; //players whom this event is available for
+	bool computerActivate = false; //true if computer player can activate this event
+	bool humanActivate = false; //true if human player can activate this event
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -87,7 +86,6 @@ public:
 		h & humanActivate;
 	}
 
-	CGEvent();
 	void onHeroVisit(const CGHeroInstance * h) const override;
 protected:
 	void serializeJsonOptions(JsonSerializeFormat & handler) override;

lib/mapObjects/CGTownInstance.cpp

@@ -35,12 +35,6 @@ CSpecObjInfo::CSpecObjInfo():
 
 }
 
-CCreGenAsCastleInfo::CCreGenAsCastleInfo():
-	CSpecObjInfo(),	asCastle(false),identifier(0)
-{
-
-}
-
 void CCreGenAsCastleInfo::serializeJson(JsonSerializeFormat & handler)
 {
 	handler.serializeString("sameAsTown", instanceId);
@@ -68,17 +62,10 @@ void CCreGenAsCastleInfo::serializeJson(JsonSerializeFormat & handler)
 	}
 }
 
-CCreGenLeveledInfo::CCreGenLeveledInfo():
-	CSpecObjInfo(),
-	minLevel(0), maxLevel(7)
-{
-
-}
-
 void CCreGenLeveledInfo::serializeJson(JsonSerializeFormat & handler)
 {
-	handler.serializeInt("minLevel", minLevel, ui8(1));
-	handler.serializeInt("maxLevel", maxLevel, ui8(7));
+	handler.serializeInt("minLevel", minLevel, static_cast<ui8>(1));
+	handler.serializeInt("maxLevel", maxLevel, static_cast<ui8>(7));
 
 	if(!handler.saving)
 	{
@@ -95,12 +82,6 @@ void CCreGenLeveledCastleInfo::serializeJson(JsonSerializeFormat & handler)
 	CCreGenLeveledInfo::serializeJson(handler);
 }
 
-CGDwelling::CGDwelling():
-	CArmedInstance()
-{
-	info = nullptr;
-}
-
 CGDwelling::~CGDwelling()
 {
 	vstd::clear_pointer(info);
@@ -116,7 +97,7 @@ void CGDwelling::initObj(CRandomGenerator & rand)
 			VLC->objtypeh->getHandlerFor(ID, subID)->configureObject(this, rand);
 
 			if (getOwner() != PlayerColor::NEUTRAL)
-				cb->gameState()->players[getOwner()].dwellings.push_back (this);
+				cb->gameState()->players[getOwner()].dwellings.emplace_back(this);
 
 			assert(!creatures.empty());
 			assert(!creatures[0].second.empty());
@@ -128,9 +109,9 @@ void CGDwelling::initObj(CRandomGenerator & rand)
 
 	case Obj::WAR_MACHINE_FACTORY:
 		creatures.resize(3);
-		creatures[0].second.push_back(CreatureID::BALLISTA);
-		creatures[1].second.push_back(CreatureID::FIRST_AID_TENT);
-		creatures[2].second.push_back(CreatureID::AMMO_CART);
+		creatures[0].second.emplace_back(CreatureID::BALLISTA);
+		creatures[1].second.emplace_back(CreatureID::FIRST_AID_TENT);
+		creatures[2].second.emplace_back(CreatureID::AMMO_CART);
 		break;
 
 	default:
@@ -170,7 +151,7 @@ void CGDwelling::setPropertyDer(ui8 what, ui32 val)
 					dwellings->erase (std::find(dwellings->begin(), dwellings->end(), this));
 				}
 				if (PlayerColor(val) != PlayerColor::NEUTRAL) //can new owner be neutral?
-					cb->gameState()->players[PlayerColor(val)].dwellings.push_back (this);
+					cb->gameState()->players[PlayerColor(val)].dwellings.emplace_back(this);
 			}
 			break;
 		case ObjProperty::AVAILABLE_CREATURE:
@@ -222,14 +203,14 @@ void CGDwelling::onHeroVisit( const CGHeroInstance * h ) const
 	{
 		bd.text.addTxt(MetaString::ADVOB_TXT, ID == Obj::CREATURE_GENERATOR1 ? 35 : 36); //{%s} Would you like to recruit %s? / {%s} Would you like to recruit %s, %s, %s, or %s?
 		bd.text.addReplacement(ID == Obj::CREATURE_GENERATOR1 ? MetaString::CREGENS : MetaString::CREGENS4, subID);
-		for(auto & elem : creatures)
+		for(const auto & elem : creatures)
 			bd.text.addReplacement(MetaString::CRE_PL_NAMES, elem.second[0]);
 	}
 	else if(ID == Obj::REFUGEE_CAMP)
 	{
 		bd.text.addTxt(MetaString::ADVOB_TXT, 35); //{%s} Would you like to recruit %s?
 		bd.text.addReplacement(MetaString::OBJ_NAMES, ID);
-		for(auto & elem : creatures)
+		for(const auto & elem : creatures)
 			bd.text.addReplacement(MetaString::CRE_PL_NAMES, elem.second[0]);
 	}
 	else if(ID == Obj::WAR_MACHINE_FACTORY)
@@ -261,7 +242,7 @@ void CGDwelling::newTurn(CRandomGenerator & rand) const
 	sac.tid = id;
 	for (size_t i = 0; i < creatures.size(); i++)
 	{
-		if(creatures[i].second.size())
+		if(!creatures[i].second.empty())
 		{
 			CCreature *cre = VLC->creh->objects[creatures[i].second[0]];
 			TQuantity amount = cre->growth * (1 + cre->valOfBonuses(Bonus::CREATURE_GROWTH_PERCENT)/100) + cre->valOfBonuses(Bonus::CREATURE_GROWTH);
@@ -539,33 +520,33 @@ GrowthInfo CGTownInstance::getGrowthInfo(int level) const
 	const int base = creature->growth;
 	int castleBonus = 0;
 
-	ret.entries.push_back(GrowthInfo::Entry(VLC->generaltexth->allTexts[590], base));// \n\nBasic growth %d"
+	ret.entries.emplace_back(VLC->generaltexth->allTexts[590], base); // \n\nBasic growth %d"
 
 	if (hasBuilt(BuildingID::CASTLE))
-		ret.entries.push_back(GrowthInfo::Entry(subID, BuildingID::CASTLE, castleBonus = base));
+		ret.entries.emplace_back(subID, BuildingID::CASTLE, castleBonus = base);
 	else if (hasBuilt(BuildingID::CITADEL))
-		ret.entries.push_back(GrowthInfo::Entry(subID, BuildingID::CITADEL, castleBonus = base / 2));
+		ret.entries.emplace_back(subID, BuildingID::CITADEL, castleBonus = base / 2);
 
 	if(town->hordeLvl.at(0) == level)//horde 1
 		if(hasBuilt(BuildingID::HORDE_1))
-			ret.entries.push_back(GrowthInfo::Entry(subID, BuildingID::HORDE_1, creature->hordeGrowth));
+			ret.entries.emplace_back(subID, BuildingID::HORDE_1, creature->hordeGrowth);
 
 	if(town->hordeLvl.at(1) == level)//horde 2
 		if(hasBuilt(BuildingID::HORDE_2))
-			ret.entries.push_back(GrowthInfo::Entry(subID, BuildingID::HORDE_2, creature->hordeGrowth));
+			ret.entries.emplace_back(subID, BuildingID::HORDE_2, creature->hordeGrowth);
 
 	//statue-of-legion-like bonus: % to base+castle
 	TConstBonusListPtr bonuses2 = getBonuses(Selector::type()(Bonus::CREATURE_GROWTH_PERCENT));
 	for(const auto & b : *bonuses2)
 	{
 		const auto growth = b->val * (base + castleBonus) / 100;
-		ret.entries.push_back(GrowthInfo::Entry(growth, b->Description(growth)));
+		ret.entries.emplace_back(growth, b->Description(growth));
 	}
 
 	//other *-of-legion-like bonuses (%d to growth cumulative with grail)
 	TConstBonusListPtr bonuses = getBonuses(Selector::type()(Bonus::CREATURE_GROWTH).And(Selector::subtype()(level)));
 	for(const auto & b : *bonuses)
-		ret.entries.push_back(GrowthInfo::Entry(b->val, b->Description()));
+		ret.entries.emplace_back(b->val, b->Description());
 
 	int dwellingBonus = 0;
 	if(const PlayerState *p = cb->getPlayerState(tempOwner, false))
@@ -573,10 +554,10 @@ GrowthInfo CGTownInstance::getGrowthInfo(int level) const
 		dwellingBonus = getDwellingBonus(creatures[level].second, p->dwellings);
 	}
 	if(dwellingBonus)
-		ret.entries.push_back(GrowthInfo::Entry(VLC->generaltexth->allTexts[591], dwellingBonus));// \nExternal dwellings %+d
+		ret.entries.emplace_back(VLC->generaltexth->allTexts[591], dwellingBonus); // \nExternal dwellings %+d
 
 	if(hasBuilt(BuildingID::GRAIL)) //grail - +50% to ALL (so far added) growth
-		ret.entries.push_back(GrowthInfo::Entry(subID, BuildingID::GRAIL, ret.totalGrowth() / 2));
+		ret.entries.emplace_back(subID, BuildingID::GRAIL, ret.totalGrowth() / 2);
 
 	return ret;
 }
@@ -597,11 +578,11 @@ int CGTownInstance::getDwellingBonus(const std::vector<CreatureID>& creatureIds,
 TResources CGTownInstance::dailyIncome() const
 {
 	TResources ret;
-	for (auto & p : town->buildings)
+	for(const auto & p : town->buildings)
 	{
 		BuildingID buildingUpgrade;
 
-		for (auto & p2 : town->buildings)
+		for(const auto & p2 : town->buildings)
 		{
 			if (p2.second->upgrade == p.first)
 			{
@@ -626,8 +607,14 @@ bool CGTownInstance::hasCapitol() const
 	return hasBuilt(BuildingID::CAPITOL);
 }
 
-CGTownInstance::CGTownInstance()
-	:CGDwelling(), IShipyard(this), IMarket(this), town(nullptr), builded(0), destroyed(0), identifier(0), alignment(0xff)
+CGTownInstance::CGTownInstance():
+	IShipyard(this),
+	IMarket(this),
+	town(nullptr),
+	builded(0),
+	destroyed(0),
+	identifier(0),
+	alignment(0xff)
 {
 	this->setNodeType(CBonusSystemNode::TOWN);
 }
@@ -652,12 +639,10 @@ int CGTownInstance::spellsAtLevel(int level, bool checkGuild) const
 
 bool CGTownInstance::needsLastStack() const
 {
-	if(garrisonHero)
-		return true;
-	else return false;
+	return garrisonHero != nullptr;
 }
 
-void CGTownInstance::setOwner(const PlayerColor player) const
+void CGTownInstance::setOwner(const PlayerColor & player) const
 {
 	removeCapitols(player);
 	cb->setOwner(this, player);
@@ -676,7 +661,7 @@ void CGTownInstance::onHeroVisit(const CGHeroInstance * h) const
 			if(!isBattleOutside && visitingHero && defendingHero == visitingHero)
 			{
 				//we have two approaches to merge armies: mergeGarrisonOnSiege() and used in the CGameHandler::garrisonSwap(ObjectInstanceID tid)
-				auto nodeSiege = defendingHero->whereShouldBeAttachedOnSiege(isBattleOutside);
+				auto * nodeSiege = defendingHero->whereShouldBeAttachedOnSiege(isBattleOutside);
 
 				if(nodeSiege == (CBonusSystemNode *)this)
 					cb->swapGarrisonOnSiege(this->id);
@@ -716,7 +701,7 @@ void CGTownInstance::onHeroVisit(const CGHeroInstance * h) const
 			InfoWindow iw;
 			iw.player = h->tempOwner;
 			iw.text << h->commander->getName();
-			iw.components.push_back(Component(*h->commander));
+			iw.components.emplace_back(*h->commander);
 			cb->showInfoDialog(&iw);
 		}
 	}
@@ -752,9 +737,9 @@ void CGTownInstance::initOverriddenBids()
 {
 	for(const auto & bid : builtBuildings)
 	{
-		auto & overrideThem = town->buildings.at(bid)->overrideBids;
+		const auto & overrideThem = town->buildings.at(bid)->overrideBids;
 
-		for(auto & overrideIt : overrideThem)
+		for(const auto & overrideIt : overrideThem)
 			overriddenBuildings.insert(overrideIt);
 	}
 }
@@ -807,9 +792,9 @@ TDmgRange CGTownInstance::getTowerDamageRange() const
 
 	// http://heroes.thelazy.net/wiki/Arrow_tower
 	// base damage, irregardless of town level
-	static const int baseDamage = 6;
+	static constexpr int baseDamage = 6;
 	// extra damage, for each building in town
-	static const int extraDamage = 1;
+	static constexpr int extraDamage = 1;
 
 	const int minDamage = baseDamage + extraDamage * getTownLevel();
 
@@ -825,9 +810,9 @@ TDmgRange CGTownInstance::getKeepDamageRange() const
 
 	// http://heroes.thelazy.net/wiki/Arrow_tower
 	// base damage, irregardless of town level
-	static const int baseDamage = 10;
+	static constexpr int baseDamage = 10;
 	// extra damage, for each building in town
-	static const int extraDamage = 2;
+	static constexpr int extraDamage = 2;
 
 	const int minDamage = baseDamage + extraDamage * getTownLevel();
 
@@ -862,10 +847,7 @@ void CGTownInstance::deleteTownBonus(BuildingID::EBuildingID bid)
 
 	bonusingBuildings.erase(bonusingBuildings.begin() + i);
 
-	if(isVisitingBonus)
-		delete (CTownBonus *)freeIt;
-	else if(isWeekBonus)
-		delete (COPWBonus *)freeIt;
+	delete freeIt;
 }
 
 void CGTownInstance::initObj(CRandomGenerator & rand) ///initialize town structures
@@ -894,7 +876,7 @@ void CGTownInstance::initObj(CRandomGenerator & rand) ///initialize town structu
 	updateAppearance();
 }
 
-bool CGTownInstance::hasBuiltInOldWay(ETownType::ETownType type, BuildingID bid) const
+bool CGTownInstance::hasBuiltInOldWay(ETownType::ETownType type, const BuildingID & bid) const
 {
 	return (this->town->faction != nullptr && this->town->faction->getIndex() == type && hasBuilt(bid));
 }
@@ -917,7 +899,7 @@ void CGTownInstance::newTurn(CRandomGenerator & rand) const
 			cb->setObjProperty (id, ObjProperty::BONUS_VALUE_SECOND, resVal);
 		}
 
-		auto manaVortex = getBonusingBuilding(BuildingSubID::MANA_VORTEX);
+		const auto * manaVortex = getBonusingBuilding(BuildingSubID::MANA_VORTEX);
 
 		if (manaVortex != nullptr)
 			cb->setObjProperty(id, ObjProperty::STRUCTURE_CLEAR_VISITORS, manaVortex->indexOnTV); //reset visitors for Mana Vortex
@@ -933,14 +915,14 @@ void CGTownInstance::newTurn(CRandomGenerator & rand) const
 		if (tempOwner == PlayerColor::NEUTRAL) //garrison growth for neutral towns
 			{
 				std::vector<SlotID> nativeCrits; //slots
-				for (auto & elem : Slots())
+				for(const auto & elem : Slots())
 				{
 					if (elem.second->type->faction == subID) //native
 					{
 						nativeCrits.push_back(elem.first); //collect matching slots
 					}
 				}
-				if (nativeCrits.size())
+				if(!nativeCrits.empty())
 				{
 					SlotID pos = *RandomGeneratorUtil::nextItem(nativeCrits, rand);
 					StackLocation sl(this, pos);
@@ -1011,12 +993,9 @@ void CGTownInstance::mergeGarrisonOnSiege() const
 		std::vector<SlotID> weakSlots;
 		auto stacksList = visitingHero->stacks;
 		std::pair<SlotID, CStackInstance *> pair;
-		while(stacksList.size())
+		while(!stacksList.empty())
 		{
-			pair = *vstd::minElementByFun(stacksList, [&](std::pair<SlotID, CStackInstance *> elem)
-			{
-				return elem.second->getPower();
-			});
+			pair = *vstd::minElementByFun(stacksList, [&](const std::pair<SlotID, CStackInstance *> & elem) { return elem.second->getPower(); });
 			if(powerLimit > pair.second->getPower() &&
 				(weakSlots.empty() || pair.second->getPower() == visitingHero->getStack(weakSlots.front()).getPower()))
 			{
@@ -1027,7 +1006,7 @@ void CGTownInstance::mergeGarrisonOnSiege() const
 				break;
 		}
 
-		if(weakSlots.size())
+		if(!weakSlots.empty())
 			return *std::max_element(weakSlots.begin(), weakSlots.end());
 
 		return SlotID();
@@ -1037,7 +1016,7 @@ void CGTownInstance::mergeGarrisonOnSiege() const
 
 	for(int i = 0; i < count; i++)
 	{
-		auto pair = *vstd::maxElementByFun(stacks, [&](std::pair<SlotID, CStackInstance *> elem)
+		auto pair = *vstd::maxElementByFun(stacks, [&](const std::pair<SlotID, CStackInstance *> & elem)
 		{
 			ui64 power = elem.second->getPower();
 			auto dst = visitingHero->getSlotFor(elem.second->getCreatureID());
@@ -1058,7 +1037,7 @@ void CGTownInstance::mergeGarrisonOnSiege() const
 	}
 }
 
-void CGTownInstance::removeCapitols (PlayerColor owner) const
+void CGTownInstance::removeCapitols(const PlayerColor & owner) const
 {
 	if (hasCapitol()) // search if there's an older capitol
 	{
@@ -1218,7 +1197,7 @@ void CGTownInstance::recreateBuildingsBonuses()
 	BonusList bl;
 	getExportedBonusList().getBonuses(bl, Selector::sourceType()(Bonus::TOWN_STRUCTURE));
 
-	for(auto b : bl)
+	for(const auto & b : bl)
 		removeBonus(b);
 
 	for(const auto & bid : builtBuildings)
@@ -1353,7 +1332,7 @@ const CArmedInstance * CGTownInstance::getUpperArmy() const
 
 const CGTownBuilding * CGTownInstance::getBonusingBuilding(BuildingSubID::EBuildingSubID subId) const
 {
-	for(const auto building : bonusingBuildings)
+	for(auto * const building : bonusingBuildings)
 	{
 		if(building->getBuildingSubtype() == subId)
 			return building;
@@ -1382,19 +1361,19 @@ bool CGTownInstance::hasBuilt(BuildingSubID::EBuildingSubID buildingID) const
 	return false;
 }
 
-bool CGTownInstance::hasBuilt(BuildingID buildingID) const
+bool CGTownInstance::hasBuilt(const BuildingID & buildingID) const
 {
 	return vstd::contains(builtBuildings, buildingID);
 }
 
-bool CGTownInstance::hasBuilt(BuildingID buildingID, int townID) const
+bool CGTownInstance::hasBuilt(const BuildingID & buildingID, int townID) const
 {
 	if (townID == town->faction->getIndex() || townID == ETownType::ANY)
 		return hasBuilt(buildingID);
 	return false;
 }
 
-TResources CGTownInstance::getBuildingCost(BuildingID buildingID) const
+TResources CGTownInstance::getBuildingCost(const BuildingID & buildingID) const
 {
 	if (vstd::contains(town->buildings, buildingID))
 		return town->buildings.at(buildingID)->resources;
@@ -1406,7 +1385,7 @@ TResources CGTownInstance::getBuildingCost(BuildingID buildingID) const
 
 }
 
-CBuilding::TRequired CGTownInstance::genBuildingRequirements(BuildingID buildID, bool deep) const
+CBuilding::TRequired CGTownInstance::genBuildingRequirements(const BuildingID & buildID, bool deep) const
 {
 	const CBuilding * building = town->buildings.at(buildID);
 
@@ -1422,7 +1401,7 @@ CBuilding::TRequired CGTownInstance::genBuildingRequirements(BuildingID buildID,
 		if (!hasBuilt(id))
 		{
 			if (deep)
-				requirements.expressions.push_back(id);
+				requirements.expressions.emplace_back(id);
 			else
 				return id;
 		}
@@ -1477,7 +1456,7 @@ void CGTownInstance::battleFinished(const CGHeroInstance * hero, const BattleRes
 	}
 }
 
-void CGTownInstance::onTownCaptured(const PlayerColor winner) const
+void CGTownInstance::onTownCaptured(const PlayerColor & winner) const
 {
 	setOwner(winner);
 	FoWChange fw;
@@ -1490,7 +1469,7 @@ void CGTownInstance::onTownCaptured(const PlayerColor winner) const
 void CGTownInstance::afterAddToMap(CMap * map)
 {
 	if(ID == Obj::TOWN)
-		map->towns.push_back(this);
+		map->towns.emplace_back(this);
 }
 
 void CGTownInstance::afterRemoveFromMap(CMap * map)
@@ -1596,10 +1575,10 @@ void CGTownInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 
 		if(handler.saving)
 		{
-			for(SpellID id : possibleSpells)
+			for(const SpellID & id : possibleSpells)
 				spellsLIC.any[id.num] = true;
 
-			for(SpellID id : obligatorySpells)
+			for(const SpellID & id : obligatorySpells)
 				spellsLIC.all[id.num] = true;
 		}
 
@@ -1611,14 +1590,14 @@ void CGTownInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 			for(si32 idx = 0; idx < spellsLIC.any.size(); idx++)
 			{
 				if(spellsLIC.any[idx])
-					possibleSpells.push_back(SpellID(idx));
+					possibleSpells.emplace_back(idx);
 			}
 
 			obligatorySpells.clear();
 			for(si32 idx = 0; idx < spellsLIC.all.size(); idx++)
 			{
 				if(spellsLIC.all[idx])
-					obligatorySpells.push_back(SpellID(idx));
+					obligatorySpells.emplace_back(idx);
 			}
 		}
 	}
@@ -1649,7 +1628,7 @@ int3 CGTownBuilding::getPosition() const
 	return town->getPosition();
 }
 
-COPWBonus::COPWBonus(BuildingID bid, BuildingSubID::EBuildingSubID subId, CGTownInstance * cgTown)
+COPWBonus::COPWBonus(const BuildingID & bid, BuildingSubID::EBuildingSubID subId, CGTownInstance * cgTown)
 {
 	bID = bid;
 	bType = subId;
@@ -1715,7 +1694,7 @@ void COPWBonus::onHeroVisit (const CGHeroInstance * h) const
 		}
 	}
 }
-CTownBonus::CTownBonus(BuildingID index, BuildingSubID::EBuildingSubID subId, CGTownInstance * cgTown)
+CTownBonus::CTownBonus(const BuildingID & index, BuildingSubID::EBuildingSubID subId, CGTownInstance * cgTown)
 {
 	bID = index;
 	bType = subId;
@@ -1743,31 +1722,31 @@ void CTownBonus::onHeroVisit (const CGHeroInstance * h) const
 		case BuildingSubID::KNOWLEDGE_VISITING_BONUS: //wall of knowledge
 			what = PrimarySkill::KNOWLEDGE;
 			val = 1;
-			iw.components.push_back(Component(Component::PRIM_SKILL, 3, 1, 0));
+			iw.components.emplace_back(Component::PRIM_SKILL, 3, 1, 0);
 			break;
 
 		case BuildingSubID::SPELL_POWER_VISITING_BONUS: //order of fire
 			what = PrimarySkill::SPELL_POWER;
 			val = 1;
-			iw.components.push_back(Component(Component::PRIM_SKILL, 2, 1, 0));
+			iw.components.emplace_back(Component::PRIM_SKILL, 2, 1, 0);
 			break;
 
 		case BuildingSubID::ATTACK_VISITING_BONUS: //hall of Valhalla
 			what = PrimarySkill::ATTACK;
 			val = 1;
-			iw.components.push_back(Component(Component::PRIM_SKILL, 0, 1, 0));
+			iw.components.emplace_back(Component::PRIM_SKILL, 0, 1, 0);
 			break;
 
 		case BuildingSubID::EXPERIENCE_VISITING_BONUS: //academy of battle scholars
 			what = PrimarySkill::EXPERIENCE;
 			val = static_cast<int>(h->calculateXp(1000));
-			iw.components.push_back(Component(Component::EXPERIENCE, 0, val, 0));
+			iw.components.emplace_back(Component::EXPERIENCE, 0, val, 0);
 			break;
 
 		case BuildingSubID::DEFENSE_VISITING_BONUS: //cage of warlords
 			what = PrimarySkill::DEFENSE;
 			val = 1;
-			iw.components.push_back(Component(Component::PRIM_SKILL, 1, 1, 0));
+			iw.components.emplace_back(Component::PRIM_SKILL, 1, 1, 0);
 			break;
 
 		case BuildingSubID::CUSTOM_VISITING_BONUS:
@@ -1795,7 +1774,7 @@ void CTownBonus::applyBonuses(CGHeroInstance * h, const BonusList & bonuses) con
 {
 	auto addToVisitors = false;
 
-	for(auto bonus : bonuses)
+	for(const auto & bonus : bonuses)
 	{
 		GiveBonus gb;
 		InfoWindow iw;
@@ -1828,16 +1807,15 @@ GrowthInfo::Entry::Entry(const std::string &format, int _count)
 	description = boost::str(boost::format(format) % count);
 }
 
-GrowthInfo::Entry::Entry(int subID, BuildingID building, int _count)
-	: count(_count)
+GrowthInfo::Entry::Entry(int subID, const BuildingID & building, int _count): count(_count)
 {
 	description = boost::str(boost::format("%s %+d") % (*VLC->townh)[subID]->town->buildings.at(building)->getNameTranslated() % count);
 }
 
-GrowthInfo::Entry::Entry(int _count, const std::string &fullDescription)
-	: count(_count)
+GrowthInfo::Entry::Entry(int _count, std::string fullDescription):
+	count(_count),
+	description(std::move(fullDescription))
 {
-	description = fullDescription;
 }
 
 CTownAndVisitingHero::CTownAndVisitingHero()
@@ -1854,7 +1832,7 @@ int GrowthInfo::totalGrowth() const
 	return ret;
 }
 
-const std::string CGTownBuilding::getVisitingBonusGreeting() const
+std::string CGTownBuilding::getVisitingBonusGreeting() const
 {
 	auto bonusGreeting = town->town->getGreeting(bType);
 
@@ -1894,7 +1872,7 @@ const std::string CGTownBuilding::getVisitingBonusGreeting() const
 	return bonusGreeting;
 }
 
-const std::string CGTownBuilding::getCustomBonusGreeting(const Bonus & bonus) const
+std::string CGTownBuilding::getCustomBonusGreeting(const Bonus & bonus) const
 {
 	if(bonus.type == Bonus::TOWN_MAGIC_WELL)
 	{
@@ -1904,16 +1882,17 @@ const std::string CGTownBuilding::getCustomBonusGreeting(const Bonus & bonus) co
 		return bonusGreeting;
 	}
 	auto bonusGreeting = std::string(VLC->generaltexth->translate("vcmi.townHall.greetingCustomBonus")); //"%s gives you +%d %s%s"
-	std::string param = "";
-	std::string until = "";
+	std::string param;
+	std::string until;
 
 	if(bonus.type == Bonus::MORALE)
 		param = VLC->generaltexth->allTexts[384];
 	else if(bonus.type == Bonus::LUCK)
 		param = VLC->generaltexth->allTexts[385];
 
-	until = bonus.duration == (ui16)Bonus::ONE_BATTLE
-		? VLC->generaltexth->translate("vcmi.townHall.greetingCustomUntil") : ".";
+	until = bonus.duration == static_cast<ui16>(Bonus::ONE_BATTLE) 
+			? VLC->generaltexth->translate("vcmi.townHall.greetingCustomUntil")
+			: ".";
 
 	boost::format fmt = boost::format(bonusGreeting) % bonus.description % bonus.val % param % until;
 	std::string greeting = fmt.str();

lib/mapObjects/CGTownInstance.h

@@ -35,9 +35,8 @@ public:
 class DLL_LINKAGE CCreGenAsCastleInfo : public virtual CSpecObjInfo
 {
 public:
-	CCreGenAsCastleInfo();
-	bool asCastle;
-	ui32 identifier;//h3m internal identifier
+	bool asCastle = false;
+	ui32 identifier = 0;//h3m internal identifier
 
 	std::vector<bool> allowedFactions;
 
@@ -48,8 +47,8 @@ public:
 class DLL_LINKAGE CCreGenLeveledInfo : public virtual CSpecObjInfo
 {
 public:
-	CCreGenLeveledInfo();
-	ui8 minLevel, maxLevel; //minimal and maximal level of creature in dwelling: <1, 7>
+	ui8 minLevel = 0;
+	ui8 maxLevel = 7; //minimal and maximal level of creature in dwelling: <1, 7>
 
 	void serializeJson(JsonSerializeFormat & handler) override;
 };
@@ -69,8 +68,7 @@ public:
 	CSpecObjInfo * info; //random dwelling options; not serialized
 	TCreaturesSet creatures; //creatures[level] -> <vector of alternative ids (base creature and upgrades, creatures amount>
 
-	CGDwelling();
-	virtual ~CGDwelling();
+	~CGDwelling() override;
 
 	void initRandomObjectInfo();
 protected:
@@ -99,9 +97,8 @@ class DLL_LINKAGE CGTownBuilding : public IObjectInterface
 {
 ///basic class for town structures handled as map objects
 public:
-	si32 indexOnTV; //identifies its index on towns vector
-	CGTownInstance *town;
-	CGTownBuilding() : bType(BuildingSubID::NONE), indexOnTV(0), town(nullptr) {};
+	si32 indexOnTV = 0; //identifies its index on towns vector
+	CGTownInstance *town = nullptr;
 
 	STRONG_INLINE
 	BuildingSubID::EBuildingSubID getBuildingSubtype() const
@@ -137,10 +134,10 @@ public:
 
 protected:
 	BuildingID bID; //from buildig list
-	BuildingSubID::EBuildingSubID bType;
+	BuildingSubID::EBuildingSubID bType = BuildingSubID::NONE;
 
-	const std::string getVisitingBonusGreeting() const;
-	const std::string getCustomBonusGreeting(const Bonus & bonus) const;
+	std::string getVisitingBonusGreeting() const;
+	std::string getCustomBonusGreeting(const Bonus & bonus) const;
 };
 
 class DLL_LINKAGE COPWBonus : public CGTownBuilding
@@ -150,8 +147,8 @@ public:
 	void setProperty(ui8 what, ui32 val) override;
 	void onHeroVisit (const CGHeroInstance * h) const override;
 
-	COPWBonus (BuildingID index, BuildingSubID::EBuildingSubID subId, CGTownInstance *TOWN);
-	COPWBonus () {};
+	COPWBonus(const BuildingID & index, BuildingSubID::EBuildingSubID subId, CGTownInstance * TOWN);
+	COPWBonus() = default;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -169,8 +166,8 @@ public:
 	void setProperty(ui8 what, ui32 val) override;
 	void onHeroVisit (const CGHeroInstance * h) const override;
 
-	CTownBonus (BuildingID index, BuildingSubID::EBuildingSubID subId, CGTownInstance *TOWN);
-	CTownBonus () {};
+	CTownBonus(const BuildingID & index, BuildingSubID::EBuildingSubID subId, CGTownInstance * TOWN);
+	CTownBonus() = default;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -195,8 +192,8 @@ struct DLL_LINKAGE GrowthInfo
 		int count;
 		std::string description;
 		Entry(const std::string &format, int _count);
-		Entry(int subID, BuildingID building, int _count);
-		Entry(int _count, const std::string &fullDescription);
+		Entry(int subID, const BuildingID & building, int _count);
+		Entry(int _count, std::string fullDescription);
 	};
 
 	std::vector<Entry> entries;
@@ -284,7 +281,7 @@ public:
 	const CArmedInstance *getUpperArmy() const; //garrisoned hero if present or the town itself
 
 	std::string getNameTranslated() const;
-	void setNameTranslated( std::string const & newName );
+	void setNameTranslated(const std::string & newName);
 
 	//////////////////////////////////////////////////////////////////////////
 
@@ -316,19 +313,19 @@ public:
 	//checks if special building with type buildingID is constructed
 	bool hasBuilt(BuildingSubID::EBuildingSubID buildingID) const;
 	//checks if building is constructed and town has same subID
-	bool hasBuilt(BuildingID buildingID) const;
-	bool hasBuilt(BuildingID buildingID, int townID) const;
+	bool hasBuilt(const BuildingID & buildingID) const;
+	bool hasBuilt(const BuildingID & buildingID, int townID) const;
 
-	TResources getBuildingCost(BuildingID buildingID) const;
+	TResources getBuildingCost(const BuildingID & buildingID) const;
 	TResources dailyIncome() const; //calculates daily income of this town
 	int spellsAtLevel(int level, bool checkGuild) const; //levels are counted from 1 (1 - 5)
 	bool armedGarrison() const; //true if town has creatures in garrison or garrisoned hero
 	int getTownLevel() const;
 
-	CBuilding::TRequired genBuildingRequirements(BuildingID build, bool deep = false) const;
+	CBuilding::TRequired genBuildingRequirements(const BuildingID & build, bool deep = false) const;
 
 	void mergeGarrisonOnSiege() const; // merge garrison into army of visiting hero
-	void removeCapitols (PlayerColor owner) const;
+	void removeCapitols(const PlayerColor & owner) const;
 	void clearArmy() const;
 	void addHeroToStructureVisitors(const CGHeroInstance *h, si64 structureInstanceID) const; //hero must be visiting or garrisoned in town
 	void deleteTownBonus(BuildingID::EBuildingID bid);
@@ -365,10 +362,10 @@ protected:
 	void serializeJsonOptions(JsonSerializeFormat & handler) override;
 
 private:
-	void setOwner(const PlayerColor owner) const;
-	void onTownCaptured(const PlayerColor winner) const;
+	void setOwner(const PlayerColor & owner) const;
+	void onTownCaptured(const PlayerColor & winner) const;
 	int getDwellingBonus(const std::vector<CreatureID>& creatureIds, const std::vector<ConstTransitivePtr<CGDwelling> >& dwellings) const;
-	bool hasBuiltInOldWay(ETownType::ETownType type, BuildingID bid) const;
+	bool hasBuiltInOldWay(ETownType::ETownType type, const BuildingID & bid) const;
 	bool townEnvisagesBuilding(BuildingSubID::EBuildingSubID bid) const;
 	bool isBonusingBuildingAdded(BuildingID::EBuildingID bid) const;
 	void tryAddOnePerWeekBonus(BuildingSubID::EBuildingSubID subID);

lib/mapObjects/CObjectClassesHandler.cpp

@@ -91,19 +91,19 @@ CObjectClassesHandler::CObjectClassesHandler()
 
 CObjectClassesHandler::~CObjectClassesHandler()
 {
-	for(auto p : objects)
+	for(auto * p : objects)
 		delete p;
 }
 
 std::vector<JsonNode> CObjectClassesHandler::loadLegacyData(size_t dataSize)
 {
 	CLegacyConfigParser parser("Data/Objects.txt");
-	size_t totalNumber = static_cast<size_t>(parser.readNumber()); // first line contains number of objects to read and nothing else
+	auto totalNumber = static_cast<size_t>(parser.readNumber()); // first line contains number of objects to read and nothing else
 	parser.endLine();
 
 	for (size_t i = 0; i < totalNumber; i++)
 	{
-		auto tmpl = new ObjectTemplate;
+		auto * tmpl = new ObjectTemplate;
 
 		tmpl->readTxt(parser);
 		parser.endLine();
@@ -225,7 +225,7 @@ std::string ObjectClass::getNameTranslated() const
 
 ObjectClass * CObjectClassesHandler::loadFromJson(const std::string & scope, const JsonNode & json, const std::string & name, size_t index)
 {
-	auto obj = new ObjectClass();
+	auto * obj = new ObjectClass();
 
 	obj->modScope = scope;
 	obj->identifier = name;
@@ -241,7 +241,7 @@ ObjectClass * CObjectClassesHandler::loadFromJson(const std::string & scope, con
 	{
 		if (!subData.second["index"].isNull())
 		{
-			std::string const & subMeta = subData.second["index"].meta;
+			const std::string & subMeta = subData.second["index"].meta;
 
 			if ( subMeta != "core")
 				logMod->warn("Object %s:%s.%s - attempt to load object with preset index! This option is reserved for built-in mod", subMeta, name, subData.first );
@@ -256,16 +256,16 @@ ObjectClass * CObjectClassesHandler::loadFromJson(const std::string & scope, con
 
 void CObjectClassesHandler::loadObject(std::string scope, std::string name, const JsonNode & data)
 {
-	auto object = loadFromJson(scope, data, name, objects.size());
+	auto * object = loadFromJson(scope, data, name, objects.size());
 	objects.push_back(object);
 	VLC->modh->identifiers.registerObject(scope, "object", name, object->id);
 }
 
 void CObjectClassesHandler::loadObject(std::string scope, std::string name, const JsonNode & data, size_t index)
 {
-	auto object = loadFromJson(scope, data, name, index);
+	auto * object = loadFromJson(scope, data, name, index);
 	assert(objects[(si32)index] == nullptr); // ensure that this id was not loaded before
-	objects[(si32)index] = object;
+	objects[static_cast<si32>(index)] = object;
 	VLC->modh->identifiers.registerObject(scope, "object", name, object->id);
 }
 
@@ -304,12 +304,12 @@ TObjectTypeHandler CObjectClassesHandler::getHandlerFor(si32 type, si32 subtype)
 	return objects.at(type)->objects.at(subtype);
 }
 
-TObjectTypeHandler CObjectClassesHandler::getHandlerFor(std::string scope, std::string type, std::string subtype) const
+TObjectTypeHandler CObjectClassesHandler::getHandlerFor(const std::string & scope, const std::string & type, const std::string & subtype) const
 {
 	boost::optional<si32> id = VLC->modh->identifiers.getIdentifier(scope, "object", type);
 	if(id)
 	{
-		auto object = objects[id.get()];
+		auto * object = objects[id.get()];
 		boost::optional<si32> subID = VLC->modh->identifiers.getIdentifier(scope, object->getJsonKey(), subtype);
 
 		if (subID)
@@ -330,7 +330,7 @@ std::set<si32> CObjectClassesHandler::knownObjects() const
 {
 	std::set<si32> ret;
 
-	for (auto entry : objects)
+	for(auto * entry : objects)
 		if (entry)
 			ret.insert(entry->id);
 
@@ -344,7 +344,7 @@ std::set<si32> CObjectClassesHandler::knownSubObjects(si32 primaryID) const
 
 	std::set<si32> ret;
 
-	for (auto entry : objects.at(primaryID)->objects)
+	for(const auto & entry : objects.at(primaryID)->objects)
 		if (entry)
 			ret.insert(entry->subtype);
 
@@ -357,7 +357,7 @@ void CObjectClassesHandler::beforeValidate(JsonNode & object)
 	{
 		if (object.Struct().count("subObjects"))
 		{
-			auto const & vector = object["subObjects"].Vector();
+			const auto & vector = object["subObjects"].Vector();
 
 			if (entry.second.Struct().count("index"))
 			{
@@ -377,12 +377,12 @@ void CObjectClassesHandler::beforeValidate(JsonNode & object)
 
 void CObjectClassesHandler::afterLoadFinalization()
 {
-	for(auto entry : objects)
+	for(auto * entry : objects)
 	{
 		if (!entry)
 			continue;
 
-		for(auto obj : entry->objects)
+		for(const auto & obj : entry->objects)
 		{
 			if (!obj)
 				continue;
@@ -403,7 +403,7 @@ void CObjectClassesHandler::afterLoadFinalization()
 
 std::string CObjectClassesHandler::getObjectName(si32 type, si32 subtype) const
 {
-	auto const handler = getHandlerFor(type, subtype);
+	const auto handler = getHandlerFor(type, subtype);
 	if (handler->hasNameTextID())
 		return handler->getNameTranslated();
 	else
@@ -431,16 +431,6 @@ std::string CObjectClassesHandler::getObjectHandlerName(si32 type) const
 	return objects.at(type)->handlerName;
 }
 
-AObjectTypeHandler::AObjectTypeHandler():
-	type(-1), subtype(-1)
-{
-
-}
-
-AObjectTypeHandler::~AObjectTypeHandler()
-{
-}
-
 std::string AObjectTypeHandler::getJsonKey() const
 {
 	return modScope + ':' + subTypeName;
@@ -491,7 +481,7 @@ void AObjectTypeHandler::init(const JsonNode & input)
 		entry.second.setType(JsonNode::JsonType::DATA_STRUCT);
 		JsonUtils::inherit(entry.second, base);
 
-		auto tmpl = new ObjectTemplate;
+		auto * tmpl = new ObjectTemplate;
 		tmpl->id = Obj(type);
 		tmpl->subid = subtype;
 		tmpl->stringID = entry.first; // FIXME: create "fullID" - type.object.template?
@@ -528,7 +518,7 @@ void AObjectTypeHandler::init(const JsonNode & input)
 	initTypeData(input);
 }
 
-bool AObjectTypeHandler::objectFilter(const CGObjectInstance *, std::shared_ptr<const ObjectTemplate>) const
+bool AObjectTypeHandler::objectFilter(const CGObjectInstance * obj, std::shared_ptr<const ObjectTemplate> tmpl) const
 {
 	return false; // by default there are no overrides
 }
@@ -566,7 +556,7 @@ SObjectSounds AObjectTypeHandler::getSounds() const
 	return sounds;
 }
 
-void AObjectTypeHandler::addTemplate(std::shared_ptr<const ObjectTemplate> templ)
+void AObjectTypeHandler::addTemplate(const std::shared_ptr<const ObjectTemplate> & templ)
 {
 	templates.push_back(templ);
 }
@@ -575,7 +565,7 @@ void AObjectTypeHandler::addTemplate(JsonNode config)
 {
 	config.setType(JsonNode::JsonType::DATA_STRUCT); // ensure that input is not null
 	JsonUtils::inherit(config, base);
-	auto tmpl = new ObjectTemplate;
+	auto * tmpl = new ObjectTemplate;
 	tmpl->id = Obj(type);
 	tmpl->subid = subtype;
 	tmpl->stringID.clear(); // TODO?
@@ -597,11 +587,11 @@ std::vector<std::shared_ptr<const ObjectTemplate>>AObjectTypeHandler::getTemplat
 {
 	std::vector<std::shared_ptr<const ObjectTemplate>> templates = getTemplates();
 	std::vector<std::shared_ptr<const ObjectTemplate>> filtered;
-
-	std::copy_if(templates.begin(), templates.end(), std::back_inserter(filtered), [&](std::shared_ptr<const ObjectTemplate> obj)
+	const auto cfun = [&](const std::shared_ptr<const ObjectTemplate> & obj)
 	{
 		return obj->canBePlacedAt(terrainType);
-	});
+	};
+	std::copy_if(templates.begin(), templates.end(), std::back_inserter(filtered), cfun);
 	// H3 defines allowed terrains in a weird way - artifacts, monsters and resources have faulty masks here
 	// Perhaps we should re-define faulty templates and remove this workaround (already done for resources)
 	if (type == Obj::ARTIFACT || type == Obj::MONSTER)

lib/mapObjects/CObjectClassesHandler.h

@@ -161,14 +161,13 @@ class DLL_LINKAGE AObjectTypeHandler : public boost::noncopyable
 
 protected:
 	void preInitObject(CGObjectInstance * obj) const;
-	virtual bool objectFilter(const CGObjectInstance *, std::shared_ptr<const ObjectTemplate>) const;
+	virtual bool objectFilter(const CGObjectInstance * obj, std::shared_ptr<const ObjectTemplate> tmpl) const;
 
 	/// initialization for classes that inherit this one
 	virtual void initTypeData(const JsonNode & input);
 public:
 
-	AObjectTypeHandler();
-	virtual ~AObjectTypeHandler();
+	virtual ~AObjectTypeHandler() = default;
 
 	si32 getIndex() const;
 	si32 getSubIndex() const;
@@ -185,7 +184,7 @@ public:
 	/// Returns object-specific name, if set
 	SObjectSounds getSounds() const;
 
-	void addTemplate(std::shared_ptr<const ObjectTemplate> templ);
+	void addTemplate(const std::shared_ptr<const ObjectTemplate> & templ);
 	void addTemplate(JsonNode config);
 
 	/// returns all templates matching parameters
@@ -315,7 +314,7 @@ public:
 
 	/// returns handler for specified object (ID-based). ObjectHandler keeps ownership
 	TObjectTypeHandler getHandlerFor(si32 type, si32 subtype) const;
-	TObjectTypeHandler getHandlerFor(std::string scope, std::string type, std::string subtype) const;
+	TObjectTypeHandler getHandlerFor(const std::string & scope, const std::string & type, const std::string & subtype) const;
 	TObjectTypeHandler getHandlerFor(CompoundMapObjectID compoundIdentifier) const;
 
 	std::string getObjectName(si32 type, si32 subtype) const;

lib/mapObjects/CObjectHandler.cpp

@@ -41,7 +41,7 @@ static void openWindow(const OpenWindow::EWindow type, const int id1, const int
 	IObjectInterface::cb->sendAndApply(&ow);
 }
 
-static void showInfoDialog(const PlayerColor playerID, const ui32 txtID, const ui16 soundID)
+static void showInfoDialog(const PlayerColor & playerID, const ui32 txtID, const ui16 soundID)
 {
 	InfoWindow iw;
 	iw.soundID = soundID;
@@ -72,12 +72,6 @@ void IObjectInterface::onHeroLeave(const CGHeroInstance * h) const
 void IObjectInterface::newTurn(CRandomGenerator & rand) const
 {}
 
-IObjectInterface::~IObjectInterface()
-{}
-
-IObjectInterface::IObjectInterface()
-{}
-
 void IObjectInterface::initObj(CRandomGenerator & rand)
 {}
 
@@ -122,6 +116,7 @@ CObjectHandler::CObjectHandler()
 	logGlobal->trace("\t\tDone loading resource prices!");
 }
 
+//TODO: remove constructor
 CGObjectInstance::CGObjectInstance():
 	pos(-1,-1,-1),
 	ID(Obj::NO_OBJ),
@@ -130,9 +125,9 @@ CGObjectInstance::CGObjectInstance():
 	blockVisit(false)
 {
 }
-CGObjectInstance::~CGObjectInstance()
-{
-}
+
+//must be instantiated in .cpp file for access to complete types of all member fields
+CGObjectInstance::~CGObjectInstance() = default;
 
 int32_t CGObjectInstance::getObjGroupIndex() const
 {
@@ -149,7 +144,7 @@ int3 CGObjectInstance::getPosition() const
 	return pos;
 }
 
-void CGObjectInstance::setOwner(PlayerColor ow)
+void CGObjectInstance::setOwner(const PlayerColor & ow)
 {
 	tempOwner = ow;
 }
@@ -283,12 +278,12 @@ int3 CGObjectInstance::getVisitableOffset() const
 	return appearance->getVisitableOffset();
 }
 
-void CGObjectInstance::giveDummyBonus(ObjectInstanceID heroID, ui8 duration) const
+void CGObjectInstance::giveDummyBonus(const ObjectInstanceID & heroID, ui8 duration) const
 {
 	GiveBonus gbonus;
 	gbonus.bonus.type = Bonus::NONE;
 	gbonus.id = heroID.getNum();
-	gbonus.bonus.duration = (Bonus::BonusDuration)duration;
+	gbonus.bonus.duration = static_cast<Bonus::BonusDuration>(duration);
 	gbonus.bonus.source = Bonus::OBJECT;
 	gbonus.bonus.sid = ID;
 	cb->giveHeroBonus(&gbonus);
@@ -302,7 +297,7 @@ std::string CGObjectInstance::getObjectName() const
 boost::optional<std::string> CGObjectInstance::getAmbientSound() const
 {
 	const auto & sounds = VLC->objtypeh->getObjectSounds(ID, subID).ambient;
-	if(sounds.size())
+	if(!sounds.empty())
 		return sounds.front(); // TODO: Support randomization of ambient sounds
 
 	return boost::none;
@@ -311,7 +306,7 @@ boost::optional<std::string> CGObjectInstance::getAmbientSound() const
 boost::optional<std::string> CGObjectInstance::getVisitSound() const
 {
 	const auto & sounds = VLC->objtypeh->getObjectSounds(ID, subID).visit;
-	if(sounds.size())
+	if(!sounds.empty())
 		return *RandomGeneratorUtil::nextItem(sounds, CRandomGenerator::getDefault());
 
 	return boost::none;
@@ -320,7 +315,7 @@ boost::optional<std::string> CGObjectInstance::getVisitSound() const
 boost::optional<std::string> CGObjectInstance::getRemovalSound() const
 {
 	const auto & sounds = VLC->objtypeh->getObjectSounds(ID, subID).removal;
-	if(sounds.size())
+	if(!sounds.empty())
 		return *RandomGeneratorUtil::nextItem(sounds, CRandomGenerator::getDefault());
 
 	return boost::none;
@@ -466,7 +461,7 @@ IBoatGenerator::EGeneratorState IBoatGenerator::shipyardStatus() const
 	const TerrainTile *t = IObjectInterface::cb->getTile(tile);
 	if(!t)
 		return TILE_BLOCKED; //no available water
-	else if(!t->blockingObjects.size())
+	else if(t->blockingObjects.empty())
 		return GOOD; //OK
 	else if(t->blockingObjects.front()->ID == Obj::BOAT)
 		return BOAT_ALREADY_BUILT; //blocked with boat
@@ -527,11 +522,11 @@ IShipyard * IShipyard::castFrom( CGObjectInstance *obj )
 
 	if(obj->ID == Obj::TOWN)
 	{
-		return static_cast<CGTownInstance*>(obj);
+		return dynamic_cast<CGTownInstance *>(obj);
 	}
 	else if(obj->ID == Obj::SHIPYARD)
 	{
-		return static_cast<CGShipyard*>(obj);
+		return dynamic_cast<CGShipyard *>(obj);
 	}
 	else
 	{

lib/mapObjects/CObjectHandler.h

@@ -28,15 +28,14 @@ class JsonNode;
 
 // This one teleport-specific, but has to be available everywhere in callbacks and netpacks
 // For now it's will be there till teleports code refactored and moved into own file
-typedef std::vector<std::pair<ObjectInstanceID, int3>> TTeleportExitsList;
+using TTeleportExitsList = std::vector<std::pair<ObjectInstanceID, int3>>;
 
 class DLL_LINKAGE IObjectInterface
 {
 public:
 	static IGameCallback *cb;
 
-	IObjectInterface();
-	virtual ~IObjectInterface();
+	virtual ~IObjectInterface() = default;
 
 	virtual int32_t getObjGroupIndex() const = 0;
 	virtual int32_t getObjTypeIndex() const = 0;
@@ -77,7 +76,7 @@ public:
 	const CGObjectInstance *o;
 
 	IBoatGenerator(const CGObjectInstance *O);
-	virtual ~IBoatGenerator() {}
+	virtual ~IBoatGenerator() = default;
 
 	virtual int getBoatType() const; //0 - evil (if a ship can be evil...?), 1 - good, 2 - neutral
 	virtual void getOutOffsets(std::vector<int3> &offsets) const =0; //offsets to obj pos when we boat can be placed
@@ -97,7 +96,6 @@ class DLL_LINKAGE IShipyard : public IBoatGenerator
 {
 public:
 	IShipyard(const CGObjectInstance *O);
-	virtual ~IShipyard() {}
 
 	virtual void getBoatCost(std::vector<si32> &cost) const;
 
@@ -132,8 +130,8 @@ public:
 	std::string typeName;
 	std::string subTypeName;
 
-	CGObjectInstance();
-	~CGObjectInstance();
+	CGObjectInstance(); //TODO: remove constructor
+	~CGObjectInstance() override;
 
 	int32_t getObjGroupIndex() const override;
 	int32_t getObjTypeIndex() const override;
@@ -145,7 +143,7 @@ public:
 	{
 		return this->tempOwner;
 	}
-	void setOwner(PlayerColor ow);
+	void setOwner(const PlayerColor & ow);
 
 	/** APPEARANCE ACCESSORS **/
 
@@ -195,7 +193,7 @@ public:
 	void initObj(CRandomGenerator & rand) override;
 	void onHeroVisit(const CGHeroInstance * h) const override;
 	/// method for synchronous update. Note: For new properties classes should override setPropertyDer instead
-	void setProperty(ui8 what, ui32 val) override final;
+	void setProperty(ui8 what, ui32 val) final;
 
 	virtual void afterAddToMap(CMap * map);
 	virtual void afterRemoveFromMap(CMap * map);
@@ -225,7 +223,7 @@ protected:
 	virtual void setPropertyDer(ui8 what, ui32 val);
 
 	/// Gives dummy bonus from this object to hero. Can be used to track visited state
-	void giveDummyBonus(ObjectInstanceID heroID, ui8 duration = Bonus::ONE_DAY) const;
+	void giveDummyBonus(const ObjectInstanceID & heroID, ui8 duration = Bonus::ONE_DAY) const;
 
 	///Serialize object-type specific options
 	virtual void serializeJsonOptions(JsonSerializeFormat & handler);

lib/mapObjects/CQuest.cpp

@@ -33,15 +33,25 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 std::map <PlayerColor, std::set <ui8> > CGKeys::playerKeyMap;
 
-CQuest::CQuest()
-	: qid(-1), missionType(MISSION_NONE), progress(NOT_ACTIVE), lastDay(-1), m13489val(0),
-	textOption(0), completedOption(0), stackDirection(0), heroPortrait(-1),
-	isCustomFirst(false), isCustomNext(false), isCustomComplete(false)
+//TODO: Remove constructor
+CQuest::CQuest():
+	qid(-1),
+	missionType(MISSION_NONE),
+	progress(NOT_ACTIVE),
+	lastDay(-1),
+	m13489val(0),
+	textOption(0),
+	completedOption(0),
+	stackDirection(0),
+	heroPortrait(-1),
+	isCustomFirst(false), 
+	isCustomNext(false),
+	isCustomComplete(false)
 {
 }
 
 ///helpers
-static void showInfoDialog(const PlayerColor playerID, const ui32 txtID, const ui16 soundID = 0)
+static void showInfoDialog(const PlayerColor & playerID, const ui32 txtID, const ui16 soundID = 0)
 {
 	InfoWindow iw;
 	iw.soundID = soundID;
@@ -102,7 +112,7 @@ bool CQuest::checkMissionArmy(const CQuest * q, const CCreatureSet * army)
 {
 	std::vector<CStackBasicDescriptor>::const_iterator cre;
 	TSlots::const_iterator it;
-	ui32 count;
+	ui32 count = 0;
 	ui32 slotsCount = 0;
 	bool hasExtraCreatures = false;
 	for(cre = q->m6creatures.begin(); cre != q->m6creatures.end(); ++cre)
@@ -116,10 +126,10 @@ bool CQuest::checkMissionArmy(const CQuest * q, const CCreatureSet * army)
 			}
 		}
 
-		if((TQuantity)count < cre->count) //not enough creatures of this kind
+		if(static_cast<TQuantity>(count) < cre->count) //not enough creatures of this kind
 			return false;
 
-		hasExtraCreatures |= (TQuantity)count > cre->count;
+		hasExtraCreatures |= static_cast<TQuantity>(count) > cre->count;
 	}
 
 	return hasExtraCreatures || slotsCount < army->Slots().size();
@@ -132,25 +142,23 @@ bool CQuest::checkQuest(const CGHeroInstance * h) const
 		case MISSION_NONE:
 			return true;
 		case MISSION_LEVEL:
-			if(m13489val <= h->level)
-				return true;
-			return false;
+			return m13489val <= h->level;
 		case MISSION_PRIMARY_STAT:
 			for(int i = 0; i < GameConstants::PRIMARY_SKILLS; ++i)
 			{
-				if(h->getPrimSkillLevel(static_cast<PrimarySkill::PrimarySkill>(i)) < (int)m2stats[i])
+				if(h->getPrimSkillLevel(static_cast<PrimarySkill::PrimarySkill>(i)) < static_cast<int>(m2stats[i]))
 					return false;
 			}
 			return true;
 		case MISSION_KILL_HERO:
 		case MISSION_KILL_CREATURE:
-			if (!h->cb->getObjByQuestIdentifier(m13489val))
+			if(!CGHeroInstance::cb->getObjByQuestIdentifier(m13489val))
 				return true;
 			return false;
 		case MISSION_ART:
 			// if the object was deserialized
 			if(artifactsRequirements.empty())
-				for(auto id : m5arts)
+				for(const auto & id : m5arts)
 					++artifactsRequirements[id];
 
 			for(const auto & elem : artifactsRequirements)
@@ -165,18 +173,14 @@ bool CQuest::checkQuest(const CGHeroInstance * h) const
 		case MISSION_RESOURCES:
 			for(Res::ERes i = Res::WOOD; i <= Res::GOLD; vstd::advance(i, +1)) //including Mithril ?
 			{	//Quest has no direct access to callback
-				if(h->cb->getResource (h->tempOwner, i) < (int)m7resources[i])
+				if(CGHeroInstance::cb->getResource(h->tempOwner, i) < static_cast<int>(m7resources[i]))
 					return false;
 			}
 			return true;
 		case MISSION_HERO:
-			if(m13489val == h->type->getIndex())
-				return true;
-			return false;
+			return m13489val == h->type->getIndex();
 		case MISSION_PLAYER:
-			if(m13489val == h->getOwner().getNum())
-				return true;
-			return false;
+			return m13489val == h->getOwner().getNum();
 		default:
 			return false;
 	}
@@ -200,7 +204,7 @@ void CQuest::getVisitText(MetaString &iwText, std::vector<Component> &components
 	switch (missionType)
 	{
 		case MISSION_LEVEL:
-			components.push_back(Component(Component::EXPERIENCE, 0, m13489val, 0));
+			components.emplace_back(Component::EXPERIENCE, 0, m13489val, 0);
 			if(!isCustom)
 				iwText.addReplacement(m13489val);
 			break;
@@ -211,7 +215,7 @@ void CQuest::getVisitText(MetaString &iwText, std::vector<Component> &components
 			{
 				if(m2stats[i])
 				{
-					components.push_back(Component(Component::PRIM_SKILL, i, m2stats[i], 0));
+					components.emplace_back(Component::PRIM_SKILL, i, m2stats[i], 0);
 					loot << "%d %s";
 					loot.addReplacement(m2stats[i]);
 					loot.addReplacement(VLC->generaltexth->primarySkillNames[i]);
@@ -222,19 +226,19 @@ void CQuest::getVisitText(MetaString &iwText, std::vector<Component> &components
 		}
 			break;
 		case MISSION_KILL_HERO:
-			components.push_back(Component(Component::HERO_PORTRAIT, heroPortrait, 0, 0));
+			components.emplace_back(Component::HERO_PORTRAIT, heroPortrait, 0, 0);
 			if(!isCustom)
 				addReplacements(iwText, text);
 			break;
 		case MISSION_HERO:
 			//FIXME: portrait may not match hero, if custom portrait was set in map editor
-			components.push_back(Component(Component::HERO_PORTRAIT, VLC->heroh->objects[m13489val]->imageIndex, 0, 0));
+			components.emplace_back(Component::HERO_PORTRAIT, VLC->heroh->objects[m13489val]->imageIndex, 0, 0);
 			if(!isCustom)
 				iwText.addReplacement(VLC->heroh->objects[m13489val]->getNameTextID());
 			break;
 		case MISSION_KILL_CREATURE:
 			{
-				components.push_back(Component(stackToKill));
+				components.emplace_back(stackToKill);
 				if(!isCustom)
 				{
 					addReplacements(iwText, text);
@@ -244,9 +248,9 @@ void CQuest::getVisitText(MetaString &iwText, std::vector<Component> &components
 		case MISSION_ART:
 		{
 			MetaString loot;
-			for(auto & elem : m5arts)
+			for(const auto & elem : m5arts)
 			{
-				components.push_back(Component(Component::ARTIFACT, elem, 0, 0));
+				components.emplace_back(Component::ARTIFACT, elem, 0, 0);
 				loot << "%s";
 				loot.addReplacement(MetaString::ART_NAMES, elem);
 			}
@@ -257,9 +261,9 @@ void CQuest::getVisitText(MetaString &iwText, std::vector<Component> &components
 		case MISSION_ARMY:
 		{
 			MetaString loot;
-			for(auto & elem : m6creatures)
+			for(const auto & elem : m6creatures)
 			{
-				components.push_back(Component(elem));
+				components.emplace_back(elem);
 				loot << "%s";
 				loot.addReplacement(elem);
 			}
@@ -274,7 +278,7 @@ void CQuest::getVisitText(MetaString &iwText, std::vector<Component> &components
 			{
 				if(m7resources[i])
 				{
-					components.push_back(Component (Component::RESOURCE, i, m7resources[i], 0));
+					components.emplace_back(Component::RESOURCE, i, m7resources[i], 0);
 					loot << "%d %s";
 					loot.addReplacement(m7resources[i]);
 					loot.addReplacement(MetaString::RES_NAMES, i);
@@ -285,7 +289,7 @@ void CQuest::getVisitText(MetaString &iwText, std::vector<Component> &components
 		}
 			break;
 		case MISSION_PLAYER:
-			components.push_back(Component (Component::FLAG, m13489val, 0, 0));
+			components.emplace_back(Component::FLAG, m13489val, 0, 0);
 			if(!isCustom)
 				iwText.addReplacement(VLC->generaltexth->colors[m13489val]);
 			break;
@@ -300,7 +304,7 @@ void CQuest::getRolloverText(MetaString &ms, bool onHover) const
 	if(onHover)
 		ms << "\n\n";
 
-	std::string questName  = missionName(Emission(missionType-1));
+	std::string questName = missionName(static_cast<Emission>(missionType - 1));
 	std::string questState = missionState(onHover ? 3 : 4);
 
 	ms << VLC->generaltexth->translate("core.seerhut.quest", questName, questState,textOption);
@@ -334,7 +338,7 @@ void CQuest::getRolloverText(MetaString &ms, bool onHover) const
 		case MISSION_ART:
 			{
 				MetaString loot;
-				for (auto & elem : m5arts)
+				for(const auto & elem : m5arts)
 				{
 					loot << "%s";
 					loot.addReplacement(MetaString::ART_NAMES, elem);
@@ -345,7 +349,7 @@ void CQuest::getRolloverText(MetaString &ms, bool onHover) const
 		case MISSION_ARMY:
 			{
 				MetaString loot;
-				for (auto & elem : m6creatures)
+				for(const auto & elem : m6creatures)
 				{
 					loot << "%s";
 					loot.addReplacement(elem);
@@ -408,7 +412,7 @@ void CQuest::getCompletionText(MetaString &iwText, std::vector<Component> &compo
 		case CQuest::MISSION_ART:
 		{
 			MetaString loot;
-			for (auto & elem : m5arts)
+			for(const auto & elem : m5arts)
 			{
 				loot << "%s";
 				loot.addReplacement(MetaString::ART_NAMES, elem);
@@ -420,7 +424,7 @@ void CQuest::getCompletionText(MetaString &iwText, std::vector<Component> &compo
 		case CQuest::MISSION_ARMY:
 		{
 			MetaString loot;
-			for (auto & elem : m6creatures)
+			for(const auto & elem : m6creatures)
 			{
 				loot << "%s";
 				loot.addReplacement(elem);
@@ -461,7 +465,7 @@ void CQuest::getCompletionText(MetaString &iwText, std::vector<Component> &compo
 	}
 }
 
-void CQuest::addArtifactID(ArtifactID id)
+void CQuest::addArtifactID(const ArtifactID & id)
 {
 	m5arts.push_back(id);
 	++artifactsRequirements[id];
@@ -477,9 +481,9 @@ void CQuest::serializeJson(JsonSerializeFormat & handler, const std::string & fi
 
 	if(!handler.saving)
 	{
-		isCustomFirst = firstVisitText.size() > 0;
-		isCustomNext = nextVisitText.size() > 0;
-		isCustomComplete = completedText.size() > 0;
+		isCustomFirst = !firstVisitText.empty();
+		isCustomNext = !nextVisitText.empty();
+		isCustomComplete = !completedText.empty();
 	}
 
 	static const std::vector<std::string> MISSION_TYPE_JSON =
@@ -509,7 +513,7 @@ void CQuest::serializeJson(JsonSerializeFormat & handler, const std::string & fi
 		break;
 	case MISSION_KILL_HERO:
 	case MISSION_KILL_CREATURE:
-		handler.serializeInstance<ui32>("killTarget", m13489val, ui32(-1));
+		handler.serializeInstance<ui32>("killTarget", m13489val, static_cast<ui32>(-1));
 		break;
 	case MISSION_ART:
 		//todo: ban artifacts
@@ -547,15 +551,6 @@ void CQuest::serializeJson(JsonSerializeFormat & handler, const std::string & fi
 
 }
 
-CGSeerHut::CGSeerHut() : IQuestObject(),
-	rewardType(NOTHING), rID(-1), rVal(-1)
-{
-	quest->lastDay = -1;
-	quest->isCustomFirst = false;
-	quest->isCustomNext = false;
-	quest->isCustomComplete = false;
-}
-
 void CGSeerHut::setObjToKill()
 {
 	if(quest->missionType == CQuest::MISSION_KILL_CREATURE)
@@ -646,19 +641,6 @@ void CQuest::addReplacements(MetaString &out, const std::string &base) const
 	}
 }
 
-IQuestObject::IQuestObject():
-	quest(new CQuest())
-{
-
-}
-
-IQuestObject::~IQuestObject()
-{
-	///Information about quest should remain accessible even if IQuestObject removed from map
-	///All CQuest objects are freed in CMap destructor
-	//delete quest;
-}
-
 bool IQuestObject::checkQuest(const CGHeroInstance* h) const
 {
 	return quest->checkQuest(h);
@@ -669,7 +651,7 @@ void IQuestObject::getVisitText (MetaString &text, std::vector<Component> &compo
 	quest->getVisitText (text,components, isCustom, FirstVisit, h);
 }
 
-void IQuestObject::afterAddToMapCommon(CMap * map)
+void IQuestObject::afterAddToMapCommon(CMap * map) const
 {
 	map->addNewQuestInstance(quest);
 }
@@ -679,26 +661,36 @@ void CGSeerHut::getCompletionText(MetaString &text, std::vector<Component> &comp
 	quest->getCompletionText (text, components, isCustom, h);
 	switch(rewardType)
 	{
-		case EXPERIENCE: components.push_back(Component (Component::EXPERIENCE, 0, (si32)h->calculateXp(rVal), 0));
-			break;
-		case MANA_POINTS: components.push_back(Component (Component::PRIM_SKILL, 5, rVal, 0));
-			break;
-		case MORALE_BONUS: components.push_back(Component (Component::MORALE, 0, rVal, 0));
-			break;
-		case LUCK_BONUS: components.push_back(Component (Component::LUCK, 0, rVal, 0));
-			break;
-		case RESOURCES: components.push_back(Component (Component::RESOURCE, rID, rVal, 0));
-			break;
-		case PRIMARY_SKILL: components.push_back(Component (Component::PRIM_SKILL, rID, rVal, 0));
-			break;
-		case SECONDARY_SKILL: components.push_back(Component (Component::SEC_SKILL, rID, rVal, 0));
-			break;
-		case ARTIFACT: components.push_back(Component (Component::ARTIFACT, rID, 0, 0));
-			break;
-		case SPELL: components.push_back(Component (Component::SPELL, rID, 0, 0));
-			break;
-		case CREATURE: components.push_back(Component (Component::CREATURE, rID, rVal, 0));
-			break;
+	case EXPERIENCE:
+		components.emplace_back(Component::EXPERIENCE, 0, static_cast<si32>(h->calculateXp(rVal)), 0);
+		break;
+	case MANA_POINTS:
+		components.emplace_back(Component::PRIM_SKILL, 5, rVal, 0);
+		break;
+	case MORALE_BONUS:
+		components.emplace_back(Component::MORALE, 0, rVal, 0);
+		break;
+	case LUCK_BONUS:
+		components.emplace_back(Component::LUCK, 0, rVal, 0);
+		break;
+	case RESOURCES:
+		components.emplace_back(Component::RESOURCE, rID, rVal, 0);
+		break;
+	case PRIMARY_SKILL:
+		components.emplace_back(Component::PRIM_SKILL, rID, rVal, 0);
+		break;
+	case SECONDARY_SKILL:
+		components.emplace_back(Component::SEC_SKILL, rID, rVal, 0);
+		break;
+	case ARTIFACT:
+		components.emplace_back(Component::ARTIFACT, rID, 0, 0);
+		break;
+	case SPELL:
+		components.emplace_back(Component::SPELL, rID, 0, 0);
+		break;
+	case CREATURE:
+		components.emplace_back(Component::CREATURE, rID, rVal, 0);
+		break;
 	}
 }
 
@@ -774,29 +766,29 @@ void CGSeerHut::onHeroVisit(const CGHeroInstance * h) const
 int CGSeerHut::checkDirection() const
 {
 	int3 cord = getCreatureToKill()->pos;
-	if ((double)cord.x/(double)cb->getMapSize().x < 0.34) //north
+	if(static_cast<double>(cord.x) / static_cast<double>(cb->getMapSize().x) < 0.34) //north
 	{
-		if ((double)cord.y/(double)cb->getMapSize().y < 0.34) //northwest
+		if(static_cast<double>(cord.y) / static_cast<double>(cb->getMapSize().y) < 0.34) //northwest
 			return 8;
-		else if ((double)cord.y/(double)cb->getMapSize().y < 0.67) //north
+		else if(static_cast<double>(cord.y) / static_cast<double>(cb->getMapSize().y) < 0.67) //north
 			return 1;
 		else //northeast
 			return 2;
 	}
-	else if ((double)cord.x/(double)cb->getMapSize().x < 0.67) //horizontal
+	else if(static_cast<double>(cord.x) / static_cast<double>(cb->getMapSize().x) < 0.67) //horizontal
 	{
-		if ((double)cord.y/(double)cb->getMapSize().y < 0.34) //west
+		if(static_cast<double>(cord.y) / static_cast<double>(cb->getMapSize().y) < 0.34) //west
 			return 7;
-		else if ((double)cord.y/(double)cb->getMapSize().y < 0.67) //central
+		else if(static_cast<double>(cord.y) / static_cast<double>(cb->getMapSize().y) < 0.67) //central
 			return 9;
 		else //east
 			return 3;
 	}
 	else //south
 	{
-		if ((double)cord.y/(double)cb->getMapSize().y < 0.34) //southwest
+		if(static_cast<double>(cord.y) / static_cast<double>(cb->getMapSize().y) < 0.34) //southwest
 			return 6;
-		else if ((double)cord.y/(double)cb->getMapSize().y < 0.67) //south
+		else if(static_cast<double>(cord.y) / static_cast<double>(cb->getMapSize().y) < 0.67) //south
 			return 5;
 		else //southeast
 			return 4;
@@ -815,9 +807,9 @@ void CGSeerHut::finishQuest(const CGHeroInstance * h, ui32 accept) const
 					if(!h->hasArt(elem))
 					{
 						// first we need to disassemble this backpack artifact
-						auto assembly = h->getAssemblyByConstituent(elem);
+						const auto * assembly = h->getAssemblyByConstituent(elem);
 						assert(assembly);
-						for(auto & ci : assembly->constituentsInfo)
+						for(const auto & ci : assembly->constituentsInfo)
 						{
 							cb->giveHeroNewArtifact(h, ci.art->artType, ArtifactPosition::PRE_FIRST);
 						}
@@ -833,7 +825,7 @@ void CGSeerHut::finishQuest(const CGHeroInstance * h, ui32 accept) const
 			case CQuest::MISSION_RESOURCES:
 				for (int i = 0; i < 7; ++i)
 				{
-					cb->giveResource(h->getOwner(), static_cast<Res::ERes>(i), -(int)quest->m7resources[i]);
+					cb->giveResource(h->getOwner(), static_cast<Res::ERes>(i), -static_cast<int>(quest->m7resources[i]));
 				}
 				break;
 			default:
@@ -906,7 +898,7 @@ const CGHeroInstance * CGSeerHut::getHeroToKill(bool allowNull) const
 	if(allowNull && !o)
 		return nullptr;
 	assert(o && (o->ID == Obj::HERO  ||  o->ID == Obj::PRISON));
-	return static_cast<const CGHeroInstance*>(o);
+	return dynamic_cast<const CGHeroInstance *>(o);
 }
 
 const CGCreature * CGSeerHut::getCreatureToKill(bool allowNull) const
@@ -915,7 +907,7 @@ const CGCreature * CGSeerHut::getCreatureToKill(bool allowNull) const
 	if(allowNull && !o)
 		return nullptr;
 	assert(o && o->ID == Obj::MONSTER);
-	return static_cast<const CGCreature*>(o);
+	return dynamic_cast<const CGCreature *>(o);
 }
 
 void CGSeerHut::blockingDialogAnswered(const CGHeroInstance *hero, ui32 answer) const
@@ -966,7 +958,10 @@ void CGSeerHut::serializeJsonOptions(JsonSerializeFormat & handler)
 	//todo: full reward format support after CRewardInfo integration
 
 	auto s = handler.enterStruct("reward");
-	std::string fullIdentifier, metaTypeName, scope, identifier;
+	std::string fullIdentifier;
+	std::string metaTypeName;
+	std::string scope;
+	std::string identifier;
 
 	if(handler.saving)
 	{
@@ -1115,18 +1110,15 @@ void CGKeys::setPropertyDer (ui8 what, ui32 val) //101-108 - enable key for play
 	if (what >= 101 && what <= (100 + PlayerColor::PLAYER_LIMIT_I))
 	{
 		PlayerColor player(what-101);
-		playerKeyMap[player].insert((ui8)val);
+		playerKeyMap[player].insert(static_cast<ui8>(val));
 	}
 	else
-		logGlobal->error("Unexpected properties requested to set: what=%d, val=%d", (int)what, val);
+		logGlobal->error("Unexpected properties requested to set: what=%d, val=%d", static_cast<int>(what), val);
 }
 
-bool CGKeys::wasMyColorVisited (PlayerColor player) const
+bool CGKeys::wasMyColorVisited(const PlayerColor & player) const
 {
-	if(playerKeyMap.count(player) && vstd::contains(playerKeyMap[player], subID))
-		return true;
-	else
-		return false;
+	return playerKeyMap.count(player) && vstd::contains(playerKeyMap[player], subID);
 }
 
 std::string CGKeys::getHoverText(PlayerColor player) const

lib/mapObjects/CQuest.h

@@ -19,6 +19,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 class CGCreature;
 
+
 class DLL_LINKAGE CQuest final
 {
 	mutable std::unordered_map<ArtifactID, unsigned, ArtifactID::hash> artifactsRequirements; // artifact ID -> required count
@@ -70,9 +71,11 @@ public:
 	si32 heroPortrait;
 
 	std::string firstVisitText, nextVisitText, completedText;
-	bool isCustomFirst, isCustomNext, isCustomComplete;
+	bool isCustomFirst;
+	bool isCustomNext;
+	bool isCustomComplete;
 
-	CQuest();
+	CQuest(); //TODO: Remove constructor
 
 	static bool checkMissionArmy(const CQuest * q, const CCreatureSet * army);
 	virtual bool checkQuest (const CGHeroInstance * h) const; //determines whether the quest is complete or not
@@ -81,7 +84,7 @@ public:
 	virtual void getRolloverText (MetaString &text, bool onHover) const; //hover or quest log entry
 	virtual void completeQuest (const CGHeroInstance * h) const {};
 	virtual void addReplacements(MetaString &out, const std::string &base) const;
-	void addArtifactID(ArtifactID id);
+	void addArtifactID(const ArtifactID & id);
 
 	bool operator== (const CQuest & quest) const
 	{
@@ -119,10 +122,11 @@ public:
 class DLL_LINKAGE IQuestObject
 {
 public:
-	CQuest * quest;
+	CQuest * quest = new CQuest();
 
-	IQuestObject();
-	virtual ~IQuestObject();
+	///Information about quest should remain accessible even if IQuestObject removed from map
+	///All CQuest objects are freed in CMap destructor
+	virtual ~IQuestObject() = default;
 	virtual void getVisitText (MetaString &text, std::vector<Component> &components, bool isCustom, bool FirstVisit, const CGHeroInstance * h = nullptr) const;
 	virtual bool checkQuest (const CGHeroInstance * h) const;
 
@@ -131,19 +135,18 @@ public:
 		h & quest;
 	}
 protected:
-	void afterAddToMapCommon(CMap * map);
+	void afterAddToMapCommon(CMap * map) const;
 };
 
 class DLL_LINKAGE CGSeerHut : public CArmedInstance, public IQuestObject //army is used when giving reward
 {
 public:
 	enum ERewardType {NOTHING, EXPERIENCE, MANA_POINTS, MORALE_BONUS, LUCK_BONUS, RESOURCES, PRIMARY_SKILL, SECONDARY_SKILL, ARTIFACT, SPELL, CREATURE};
-	ERewardType rewardType;
-	si32 rID; //reward ID
-	si32 rVal; //reward value
+	ERewardType rewardType = ERewardType::NOTHING;
+	si32 rID = -1; //reward ID
+	si32 rVal = -1; //reward value
 	std::string seerName;
 
-	CGSeerHut();
 	void initObj(CRandomGenerator & rand) override;
 	std::string getHoverText(PlayerColor player) const override;
 	void newTurn(CRandomGenerator & rand) const override;
@@ -172,7 +175,7 @@ public:
 		h & seerName;
 	}
 protected:
-	static const int OBJPROP_VISITED = 10;
+	static constexpr int OBJPROP_VISITED = 10;
 
 	void setPropertyDer(ui8 what, ui32 val) override;
 
@@ -182,7 +185,6 @@ protected:
 class DLL_LINKAGE CGQuestGuard : public CGSeerHut
 {
 public:
-	CGQuestGuard() = default;
 	void init(CRandomGenerator & rand) override;
 	void completeQuest (const CGHeroInstance * h) const override;
 
@@ -202,7 +204,7 @@ public:
 
 	static void reset();
 
-	bool wasMyColorVisited (PlayerColor player) const;
+	bool wasMyColorVisited(const PlayerColor & player) const;
 
 	std::string getObjectName() const override; //depending on color
 	std::string getHoverText(PlayerColor player) const override;
@@ -230,7 +232,6 @@ public:
 class DLL_LINKAGE CGBorderGuard : public CGKeys, public IQuestObject
 {
 public:
-	CGBorderGuard() = default;
 	void initObj(CRandomGenerator & rand) override;
 	void onHeroVisit(const CGHeroInstance * h) const override;
 	void blockingDialogAnswered(const CGHeroInstance *hero, ui32 answer) const override;
@@ -252,7 +253,6 @@ public:
 class DLL_LINKAGE CGBorderGate : public CGBorderGuard
 {
 public:
-	CGBorderGate() = default;
 	void onHeroVisit(const CGHeroInstance * h) const override;
 
 	bool passableFor(PlayerColor color) const override;

lib/mapObjects/CRewardableConstructor.cpp

@@ -32,7 +32,7 @@ namespace {
 
 	bool testForKey(const JsonNode & value, const std::string & key)
 	{
-		for( auto & reward : value["rewards"].Vector() )
+		for(const auto & reward : value["rewards"].Vector())
 		{
 			if (!reward[key].isNull())
 				return true;
@@ -116,9 +116,9 @@ void CRandomRewardObjectInfo::configureReward(CRewardableObject * object, CRando
 		bonus.sid = object->ID;
 		//TODO: bonus.description = object->getObjectName();
 		if (bonus.type == Bonus::MORALE)
-			reward.extraComponents.push_back(Component(Component::MORALE, 0, bonus.val, 0));
+			reward.extraComponents.emplace_back(Component::MORALE, 0, bonus.val, 0);
 		if (bonus.type == Bonus::LUCK)
-			reward.extraComponents.push_back(Component(Component::LUCK, 0, bonus.val, 0));
+			reward.extraComponents.emplace_back(Component::LUCK, 0, bonus.val, 0);
 	}
 
 	reward.primary = JsonRandom::loadPrimary(source["primary"], rng);
@@ -137,7 +137,7 @@ void CRandomRewardObjectInfo::configureReward(CRewardableObject * object, CRando
 		CreatureID from (VLC->modh->identifiers.getIdentifier (node.second.meta, "creature", node.first) .get());
 		CreatureID dest (VLC->modh->identifiers.getIdentifier (node.second.meta, "creature", node.second.String()).get());
 
-		reward.extraComponents.push_back(Component(Component::CREATURE, dest.getNum(), 0, 0));
+		reward.extraComponents.emplace_back(Component::CREATURE, dest.getNum(), 0, 0);
 
 		reward.creaturesChange[from] = dest;
 	}
@@ -302,10 +302,6 @@ bool CRandomRewardObjectInfo::givesBonuses() const
 	return testForKey(parameters, "bonuses");
 }
 
-CRewardableConstructor::CRewardableConstructor()
-{
-}
-
 void CRewardableConstructor::initTypeData(const JsonNode & config)
 {
 	objectInfo.init(config);
@@ -313,7 +309,7 @@ void CRewardableConstructor::initTypeData(const JsonNode & config)
 
 CGObjectInstance * CRewardableConstructor::create(std::shared_ptr<const ObjectTemplate> tmpl) const
 {
-	auto ret = new CRewardableObject();
+	auto * ret = new CRewardableObject();
 	preInitObject(ret);
 	ret->appearance = tmpl;
 	return ret;

lib/mapObjects/CRewardableConstructor.h

@@ -45,9 +45,6 @@ public:
 
 	void configureObject(CRewardableObject * object, CRandomGenerator & rng) const;
 
-	CRandomRewardObjectInfo()
-	{}
-
 	void init(const JsonNode & objectConfig);
 };
 
@@ -56,9 +53,8 @@ class DLL_LINKAGE CRewardableConstructor : public AObjectTypeHandler
 	CRandomRewardObjectInfo objectInfo;
 
 	void initTypeData(const JsonNode & config) override;
-public:
-	CRewardableConstructor();
 
+public:
 	CGObjectInstance * create(std::shared_ptr<const ObjectTemplate> tmpl = nullptr) const override;
 
 	void configureObject(CGObjectInstance * object, CRandomGenerator & rng) const override;

lib/mapObjects/CRewardableObject.cpp

@@ -36,10 +36,10 @@ bool CRewardLimiter::heroAllowed(const CGHeroInstance * hero) const
 			return false;
 	}
 
-	for(auto & reqStack : creatures)
+	for(const auto & reqStack : creatures)
 	{
 		size_t count = 0;
-		for (auto slot : hero->Slots())
+		for(const auto & slot : hero->Slots())
 		{
 			const CStackInstance * heroStack = slot.second;
 			if (heroStack->type == reqStack.type)
@@ -52,10 +52,10 @@ bool CRewardLimiter::heroAllowed(const CGHeroInstance * hero) const
 	if(!IObjectInterface::cb->getPlayerState(hero->tempOwner)->resources.canAfford(resources))
 		return false;
 
-	if(heroLevel > (si32)hero->level)
+	if(heroLevel > static_cast<si32>(hero->level))
 		return false;
 
-	if((TExpType)heroExperience > hero->exp)
+	if(static_cast<TExpType>(heroExperience) > hero->exp)
 		return false;
 
 	if(manaPoints > hero->mana)
@@ -66,44 +66,44 @@ bool CRewardLimiter::heroAllowed(const CGHeroInstance * hero) const
 
 	for(size_t i=0; i<primary.size(); i++)
 	{
-		if (primary[i] > hero->getPrimSkillLevel(PrimarySkill::PrimarySkill(i)))
+		if(primary[i] > hero->getPrimSkillLevel(static_cast<PrimarySkill::PrimarySkill>(i)))
 			return false;
 	}
 
-	for(auto & skill : secondary)
+	for(const auto & skill : secondary)
 	{
 		if (skill.second > hero->getSecSkillLevel(skill.first))
 			return false;
 	}
 
-	for(auto & spell : spells)
+	for(const auto & spell : spells)
 	{
 		if (!hero->spellbookContainsSpell(spell))
 			return false;
 	}
 
-	for(auto & art : artifacts)
+	for(const auto & art : artifacts)
 	{
 		if (!hero->hasArt(art))
 			return false;
 	}
 
-	for(auto & sublimiter : noneOf)
+	for(const auto & sublimiter : noneOf)
 	{
 		if (sublimiter->heroAllowed(hero))
 			return false;
 	}
 
-	for(auto & sublimiter : allOf)
+	for(const auto & sublimiter : allOf)
 	{
 		if (!sublimiter->heroAllowed(hero))
 			return false;
 	}
 
-	if ( anyOf.size() == 0 )
+	if(anyOf.empty())
 		return true;
 
-	for(auto & sublimiter : anyOf)
+	for(const auto & sublimiter : anyOf)
 	{
 		if (sublimiter->heroAllowed(hero))
 			return true;
@@ -146,7 +146,7 @@ void CRewardableObject::onHeroVisit(const CGHeroInstance *h) const
 		// grant reward afterwards. Note that it may remove object
 		grantReward(index, h, markAsVisit);
 	};
-	auto selectRewardsMessage = [&](std::vector<ui32> rewards, const MetaString & dialog) -> void
+	auto selectRewardsMessage = [&](const std::vector<ui32> & rewards, const MetaString & dialog) -> void
 	{
 		BlockingDialog sd(canRefuse, rewards.size() > 1);
 		sd.player = h->tempOwner;
@@ -200,7 +200,7 @@ void CRewardableObject::onHeroVisit(const CGHeroInstance *h) const
 			}
 		}
 
-		if(!objectRemovalPossible && getAvailableRewards(h, CRewardVisitInfo::EVENT_FIRST_VISIT).size() == 0)
+		if(!objectRemovalPossible && getAvailableRewards(h, CRewardVisitInfo::EVENT_FIRST_VISIT).empty())
 		{
 			ChangeObjectVisitors cov(ChangeObjectVisitors::VISITOR_ADD_TEAM, id, h->id);
 			cb->sendAndApply(&cov);
@@ -263,7 +263,7 @@ void CRewardableObject::grantRewardBeforeLevelup(const CRewardVisitInfo & info,
 
 	cb->giveResources(hero->tempOwner, info.reward.resources);
 
-	for(auto & entry : info.reward.secondary)
+	for(const auto & entry : info.reward.secondary)
 	{
 		int current = hero->getSecSkillLevel(entry.first);
 		if( (current != 0 && current < entry.second) ||
@@ -335,7 +335,7 @@ void CRewardableObject::grantRewardAfterLevelup(const CRewardVisitInfo & info, c
 		cb->giveHeroBonus(&gb);
 	}
 
-	for(ArtifactID art : info.reward.artifacts)
+	for(const ArtifactID & art : info.reward.artifacts)
 		cb->giveHeroNewArtifact(hero, VLC->arth->objects[art],ArtifactPosition::FIRST_AVAILABLE);
 
 	if(!info.reward.spells.empty())
@@ -346,11 +346,11 @@ void CRewardableObject::grantRewardAfterLevelup(const CRewardVisitInfo & info, c
 
 	if(!info.reward.creaturesChange.empty())
 	{
-		for (auto slot : hero->Slots())
+		for(const auto & slot : hero->Slots())
 		{
 			const CStackInstance * heroStack = slot.second;
 
-			for (auto & change : info.reward.creaturesChange)
+			for(const auto & change : info.reward.creaturesChange)
 			{
 				if (heroStack->type->getId() == change.first)
 				{
@@ -365,7 +365,7 @@ void CRewardableObject::grantRewardAfterLevelup(const CRewardVisitInfo & info, c
 	if(!info.reward.creatures.empty())
 	{
 		CCreatureSet creatures;
-		for (auto & crea : info.reward.creatures)
+		for(const auto & crea : info.reward.creatures)
 			creatures.addToSlot(creatures.getFreeSlot(), new CStackInstance(crea.type, crea.count));
 
 		cb->giveCreatures(this, hero, creatures, false);
@@ -426,7 +426,7 @@ bool CRewardableObject::wasVisited(const CGHeroInstance * h) const
 
 CRewardableObject::EVisitMode CRewardableObject::getVisitMode() const
 {
-	return EVisitMode(visitMode);
+	return static_cast<EVisitMode>(visitMode);
 }
 
 ui16 CRewardableObject::getResetDuration() const
@@ -442,37 +442,36 @@ void CRewardInfo::loadComponents(std::vector<Component> & comps,
 
 	if (heroExperience)
 	{
-		comps.push_back(Component(
-			Component::EXPERIENCE, 0, (si32)h->calculateXp(heroExperience), 0));
+		comps.emplace_back(Component::EXPERIENCE, 0, static_cast<si32>(h->calculateXp(heroExperience)), 0);
 	}
 	if (heroLevel)
-		comps.push_back(Component(Component::EXPERIENCE, 1, heroLevel, 0));
+		comps.emplace_back(Component::EXPERIENCE, 1, heroLevel, 0);
 
 	if (manaDiff || manaPercentage >= 0)
-		comps.push_back(Component(Component::PRIM_SKILL, 5, manaDiff, 0));
+		comps.emplace_back(Component::PRIM_SKILL, 5, manaDiff, 0);
 
 	for (size_t i=0; i<primary.size(); i++)
 	{
 		if (primary[i] != 0)
-			comps.push_back(Component(Component::PRIM_SKILL, (ui16)i, primary[i], 0));
+			comps.emplace_back(Component::PRIM_SKILL, static_cast<ui16>(i), primary[i], 0);
 	}
 
-	for (auto & entry : secondary)
-		comps.push_back(Component(Component::SEC_SKILL, entry.first, entry.second, 0));
+	for(const auto & entry : secondary)
+		comps.emplace_back(Component::SEC_SKILL, entry.first, entry.second, 0);
 
-	for (auto & entry : artifacts)
-		comps.push_back(Component(Component::ARTIFACT, entry, 1, 0));
+	for(const auto & entry : artifacts)
+		comps.emplace_back(Component::ARTIFACT, entry, 1, 0);
 
-	for (auto & entry : spells)
-		comps.push_back(Component(Component::SPELL, entry, 1, 0));
+	for(const auto & entry : spells)
+		comps.emplace_back(Component::SPELL, entry, 1, 0);
 
-	for (auto & entry : creatures)
-		comps.push_back(Component(Component::CREATURE, entry.type->idNumber, entry.count, 0));
+	for(const auto & entry : creatures)
+		comps.emplace_back(Component::CREATURE, entry.type->idNumber, entry.count, 0);
 
 	for (size_t i=0; i<resources.size(); i++)
 	{
 		if (resources[i] !=0)
-			comps.push_back(Component(Component::RESOURCE, (ui16)i, resources[i], 0));
+			comps.emplace_back(Component::RESOURCE, static_cast<ui16>(i), resources[i], 0);
 	}
 }
 

lib/mapObjects/CommonConstructors.cpp

@@ -24,20 +24,11 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-CObstacleConstructor::CObstacleConstructor()
-{
-}
-
 bool CObstacleConstructor::isStaticObject()
 {
 	return true;
 }
 
-CTownInstanceConstructor::CTownInstanceConstructor() :
-	faction(nullptr)
-{
-}
-
 void CTownInstanceConstructor::initTypeData(const JsonNode & input)
 {
 	VLC->modh->identifiers.requestIdentifier("faction", input["faction"], [&](si32 index)
@@ -55,7 +46,7 @@ void CTownInstanceConstructor::initTypeData(const JsonNode & input)
 void CTownInstanceConstructor::afterLoadFinalization()
 {
 	assert(faction);
-	for(auto entry : filtersJson.Struct())
+	for(const auto & entry : filtersJson.Struct())
 	{
 		filters[entry.first] = LogicalExpression<BuildingID>(entry.second, [this](const JsonNode & node)
 		{
@@ -66,7 +57,7 @@ void CTownInstanceConstructor::afterLoadFinalization()
 
 bool CTownInstanceConstructor::objectFilter(const CGObjectInstance * object, std::shared_ptr<const ObjectTemplate> templ) const
 {
-	auto town = dynamic_cast<const CGTownInstance *>(object);
+	const auto * town = dynamic_cast<const CGTownInstance *>(object);
 
 	auto buildTest = [&](const BuildingID & id)
 	{
@@ -86,17 +77,11 @@ CGObjectInstance * CTownInstanceConstructor::create(std::shared_ptr<const Object
 
 void CTownInstanceConstructor::configureObject(CGObjectInstance * object, CRandomGenerator & rng) const
 {
-	auto templ = getOverride(object->cb->getTile(object->pos)->terType->getId(), object);
+	auto templ = getOverride(CGObjectInstance::cb->getTile(object->pos)->terType->getId(), object);
 	if(templ)
 		object->appearance = templ;
 }
 
-CHeroInstanceConstructor::CHeroInstanceConstructor()
-	:heroClass(nullptr)
-{
-
-}
-
 void CHeroInstanceConstructor::initTypeData(const JsonNode & input)
 {
 	VLC->modh->identifiers.requestIdentifier(
@@ -109,7 +94,7 @@ void CHeroInstanceConstructor::initTypeData(const JsonNode & input)
 
 void CHeroInstanceConstructor::afterLoadFinalization()
 {
-	for(auto entry : filtersJson.Struct())
+	for(const auto & entry : filtersJson.Struct())
 	{
 		filters[entry.first] = LogicalExpression<HeroTypeID>(entry.second, [](const JsonNode & node)
 		{
@@ -120,7 +105,7 @@ void CHeroInstanceConstructor::afterLoadFinalization()
 
 bool CHeroInstanceConstructor::objectFilter(const CGObjectInstance * object, std::shared_ptr<const ObjectTemplate> templ) const
 {
-	auto hero = dynamic_cast<const CGHeroInstance *>(object);
+	const auto * hero = dynamic_cast<const CGHeroInstance *>(object);
 
 	auto heroTest = [&](const HeroTypeID & id)
 	{
@@ -146,11 +131,6 @@ void CHeroInstanceConstructor::configureObject(CGObjectInstance * object, CRando
 
 }
 
-CDwellingInstanceConstructor::CDwellingInstanceConstructor()
-{
-
-}
-
 bool CDwellingInstanceConstructor::hasNameTextID() const
 {
 	return true;
@@ -185,7 +165,7 @@ void CDwellingInstanceConstructor::initTypeData(const JsonNode & input)
 	guards = input["guards"];
 }
 
-bool CDwellingInstanceConstructor::objectFilter(const CGObjectInstance *, std::shared_ptr<const ObjectTemplate>) const
+bool CDwellingInstanceConstructor::objectFilter(const CGObjectInstance * obj, std::shared_ptr<const ObjectTemplate> tmpl) const
 {
 	return false;
 }
@@ -195,7 +175,7 @@ CGObjectInstance * CDwellingInstanceConstructor::create(std::shared_ptr<const Ob
 	CGDwelling * obj = createTyped(tmpl);
 
 	obj->creatures.resize(availableCreatures.size());
-	for(auto & entry : availableCreatures)
+	for(const auto & entry : availableCreatures)
 	{
 		for(const CCreature * cre : entry)
 			obj->creatures.back().second.push_back(cre->idNumber);
@@ -205,12 +185,12 @@ CGObjectInstance * CDwellingInstanceConstructor::create(std::shared_ptr<const Ob
 
 void CDwellingInstanceConstructor::configureObject(CGObjectInstance * object, CRandomGenerator &rng) const
 {
-	CGDwelling * dwelling = dynamic_cast<CGDwelling*>(object);
+	auto * dwelling = dynamic_cast<CGDwelling *>(object);
 
 	dwelling->creatures.clear();
 	dwelling->creatures.reserve(availableCreatures.size());
 
-	for(auto & entry : availableCreatures)
+	for(const auto & entry : availableCreatures)
 	{
 		dwelling->creatures.resize(dwelling->creatures.size() + 1);
 		for(const CCreature * cre : entry)
@@ -257,7 +237,7 @@ void CDwellingInstanceConstructor::configureObject(CGObjectInstance * object, CR
 
 bool CDwellingInstanceConstructor::producesCreature(const CCreature * crea) const
 {
-	for(auto & entry : availableCreatures)
+	for(const auto & entry : availableCreatures)
 	{
 		for(const CCreature * cre : entry)
 			if(crea == cre)
@@ -269,7 +249,7 @@ bool CDwellingInstanceConstructor::producesCreature(const CCreature * crea) cons
 std::vector<const CCreature *> CDwellingInstanceConstructor::getProducedCreatures() const
 {
 	std::vector<const CCreature *> creatures; //no idea why it's 2D, to be honest
-	for(auto & entry : availableCreatures)
+	for(const auto & entry : availableCreatures)
 	{
 		for(const CCreature * cre : entry)
 			creatures.push_back(cre);
@@ -277,12 +257,6 @@ std::vector<const CCreature *> CDwellingInstanceConstructor::getProducedCreature
 	return creatures;
 }
 
-CBankInstanceConstructor::CBankInstanceConstructor()
-	: bankResetDuration(0)
-{
-
-}
-
 bool CBankInstanceConstructor::hasNameTextID() const
 {
 	return true;
@@ -330,12 +304,12 @@ BankConfig CBankInstanceConstructor::generateConfig(const JsonNode & level, CRan
 
 void CBankInstanceConstructor::configureObject(CGObjectInstance * object, CRandomGenerator & rng) const
 {
-	auto bank = dynamic_cast<CBank*>(object);
+	auto * bank = dynamic_cast<CBank *>(object);
 
 	bank->resetDuration = bankResetDuration;
 
 	si32 totalChance = 0;
-	for (auto & node : levels)
+	for(const auto & node : levels)
 		totalChance += static_cast<si32>(node["chance"].Float());
 
 	assert(totalChance != 0);
@@ -343,7 +317,7 @@ void CBankInstanceConstructor::configureObject(CGObjectInstance * object, CRando
 	si32 selectedChance = rng.nextInt(totalChance - 1);
 
 	int cumulativeChance = 0;
-	for(auto & node : levels)
+	for(const auto & node : levels)
 	{
 		cumulativeChance += static_cast<int>(node["chance"].Float());
 		if(selectedChance < cumulativeChance)
@@ -454,11 +428,7 @@ std::vector<PossibleReward<TResources>> CBankInfo::getPossibleResourcesReward()
 
 		if(!resourcesInfo.isNull())
 		{
-			result.push_back(
-				PossibleReward<TResources>(
-					configEntry["chance"].Integer(),
-					TResources(resourcesInfo)
-					));
+			result.emplace_back(configEntry["chance"].Integer(), TResources(resourcesInfo));
 		}
 	}
 
@@ -476,12 +446,9 @@ std::vector<PossibleReward<CStackBasicDescriptor>> CBankInfo::getPossibleCreatur
 
 		for(auto stack : stacks)
 		{
-			auto creature = stack.allowedCreatures.front();
+			const auto * creature = stack.allowedCreatures.front();
 
-			aproximateReward.push_back(
-				PossibleReward<CStackBasicDescriptor>(
-					configEntry["chance"].Integer(),
-					CStackBasicDescriptor(creature, (stack.minAmount + stack.maxAmount) / 2)));
+			aproximateReward.emplace_back(configEntry["chance"].Integer(), CStackBasicDescriptor(creature, (stack.minAmount + stack.maxAmount) / 2));
 		}
 	}
 

lib/mapObjects/CommonConstructors.h

@@ -62,7 +62,6 @@ public:
 class CObstacleConstructor : public CDefaultObjectTypeHandler<CGObjectInstance>
 {
 public:
-	CObstacleConstructor();
 	bool isStaticObject() override;
 };
 
@@ -70,14 +69,13 @@ class CTownInstanceConstructor : public CDefaultObjectTypeHandler<CGTownInstance
 {
 	JsonNode filtersJson;
 protected:
-	bool objectFilter(const CGObjectInstance *, std::shared_ptr<const ObjectTemplate>) const override;
+	bool objectFilter(const CGObjectInstance * obj, std::shared_ptr<const ObjectTemplate> tmpl) const override;
 	void initTypeData(const JsonNode & input) override;
 
 public:
-	CFaction * faction;
+	CFaction * faction = nullptr;
 	std::map<std::string, LogicalExpression<BuildingID>> filters;
 
-	CTownInstanceConstructor();
 	CGObjectInstance * create(std::shared_ptr<const ObjectTemplate> tmpl = nullptr) const override;
 	void configureObject(CGObjectInstance * object, CRandomGenerator & rng) const override;
 	void afterLoadFinalization() override;
@@ -95,14 +93,13 @@ class CHeroInstanceConstructor : public CDefaultObjectTypeHandler<CGHeroInstance
 {
 	JsonNode filtersJson;
 protected:
-	bool objectFilter(const CGObjectInstance *, std::shared_ptr<const ObjectTemplate>) const override;
+	bool objectFilter(const CGObjectInstance * obj, std::shared_ptr<const ObjectTemplate> tmpl) const override;
 	void initTypeData(const JsonNode & input) override;
 
 public:
-	CHeroClass * heroClass;
+	CHeroClass * heroClass = nullptr;
 	std::map<std::string, LogicalExpression<HeroTypeID>> filters;
 
-	CHeroInstanceConstructor();
 	CGObjectInstance * create(std::shared_ptr<const ObjectTemplate> tmpl = nullptr) const override;
 	void configureObject(CGObjectInstance * object, CRandomGenerator & rng) const override;
 	void afterLoadFinalization() override;
@@ -123,13 +120,12 @@ class CDwellingInstanceConstructor : public CDefaultObjectTypeHandler<CGDwelling
 	JsonNode guards;
 
 protected:
-	bool objectFilter(const CGObjectInstance *, std::shared_ptr<const ObjectTemplate> tmpl) const override;
+	bool objectFilter(const CGObjectInstance * obj, std::shared_ptr<const ObjectTemplate> tmpl) const override;
 	void initTypeData(const JsonNode & input) override;
 
 public:
 	bool hasNameTextID() const override;
 
-	CDwellingInstanceConstructor();
 	CGObjectInstance * create(std::shared_ptr<const ObjectTemplate> tmpl = nullptr) const override;
 	void configureObject(CGObjectInstance * object, CRandomGenerator & rng) const override;
 
@@ -146,11 +142,10 @@ public:
 
 struct BankConfig
 {
-	BankConfig() { chance = upgradeChance = combatValue = value = 0; };
-	ui32 value; //overall value of given things
-	ui32 chance; //chance for this level being chosen
-	ui32 upgradeChance; //chance for creatures to be in upgraded versions
-	ui32 combatValue; //how hard are guards of this level
+	ui32 value = 0; //overall value of given things
+	ui32 chance = 0; //chance for this level being chosen
+	ui32 upgradeChance = 0; //chance for creatures to be in upgraded versions
+	ui32 combatValue = 0; //how hard are guards of this level
 	std::vector<CStackBasicDescriptor> guards; //creature ID, amount
 	Res::ResourceSet resources; //resources given in case of victory
 	std::vector<CStackBasicDescriptor> creatures; //creatures granted in case of victory (creature ID, amount)
@@ -212,9 +207,7 @@ protected:
 
 public:
 	// all banks of this type will be reset N days after clearing,
-	si32 bankResetDuration;
-
-	CBankInstanceConstructor();
+	si32 bankResetDuration = 0;
 
 	CGObjectInstance * create(std::shared_ptr<const ObjectTemplate> tmpl = nullptr) const override;
 	void configureObject(CGObjectInstance * object, CRandomGenerator & rng) const override;

lib/mapObjects/JsonRandom.cpp

@@ -100,7 +100,7 @@ namespace JsonRandom
 	std::vector<si32> loadPrimary(const JsonNode & value, CRandomGenerator & rng)
 	{
 		std::vector<si32> ret;
-		for (auto & name : PrimarySkill::names)
+		for(const auto & name : PrimarySkill::names)
 		{
 			ret.push_back(loadValue(value[name], rng));
 		}
@@ -110,7 +110,7 @@ namespace JsonRandom
 	std::map<SecondarySkill, si32> loadSecondary(const JsonNode & value, CRandomGenerator & rng)
 	{
 		std::map<SecondarySkill, si32> ret;
-		for (auto & pair : value.Struct())
+		for(const auto & pair : value.Struct())
 		{
 			SecondarySkill id(VLC->modh->identifiers.getIdentifier(pair.second.meta, "skill", pair.first).get());
 			ret[id] = loadValue(pair.second, rng);
@@ -129,35 +129,35 @@ namespace JsonRandom
 		ui32 maxValue = std::numeric_limits<ui32>::max();
 
 		if (value["class"].getType() == JsonNode::JsonType::DATA_STRING)
-			allowedClasses.insert(VLC->arth->stringToClass(value["class"].String()));
+			allowedClasses.insert(CArtHandler::stringToClass(value["class"].String()));
 		else
-			for (auto & entry : value["class"].Vector())
-				allowedClasses.insert(VLC->arth->stringToClass(entry.String()));
+			for(const auto & entry : value["class"].Vector())
+				allowedClasses.insert(CArtHandler::stringToClass(entry.String()));
 
 		if (value["slot"].getType() == JsonNode::JsonType::DATA_STRING)
-			allowedPositions.insert(VLC->arth->stringToSlot(value["class"].String()));
+			allowedPositions.insert(ArtifactPosition(value["class"].String()));
 		else
-			for (auto & entry : value["slot"].Vector())
-				allowedPositions.insert(VLC->arth->stringToSlot(entry.String()));
+			for(const auto & entry : value["slot"].Vector())
+				allowedPositions.insert(ArtifactPosition(entry.String()));
 
 		if (!value["minValue"].isNull()) minValue = static_cast<ui32>(value["minValue"].Float());
 		if (!value["maxValue"].isNull()) maxValue = static_cast<ui32>(value["maxValue"].Float());
 
-		return VLC->arth->pickRandomArtifact(rng, [=](ArtifactID artID) -> bool
+		return VLC->arth->pickRandomArtifact(rng, [=](const ArtifactID & artID) -> bool
 		{
 			CArtifact * art = VLC->arth->objects[artID];
 
-			if (!vstd::iswithin(art->price, minValue, maxValue))
+			if(!vstd::iswithin(art->price, minValue, maxValue))
 				return false;
 
-			if (!allowedClasses.empty() && !allowedClasses.count(art->aClass))
+			if(!allowedClasses.empty() && !allowedClasses.count(art->aClass))
 				return false;
 
-			if (!allowedPositions.empty())
+			if(!allowedPositions.empty())
 			{
-				for (auto pos : art->possibleSlots[ArtBearer::HERO])
+				for(const auto & pos : art->possibleSlots[ArtBearer::HERO])
 				{
-					if (allowedPositions.count(pos))
+					if(allowedPositions.count(pos))
 						return true;
 				}
 				return false;
@@ -181,7 +181,7 @@ namespace JsonRandom
 		if (value.getType() == JsonNode::JsonType::DATA_STRING)
 			return SpellID(VLC->modh->identifiers.getIdentifier("spell", value).get());
 
-		vstd::erase_if(spells, [=](SpellID spell)
+		vstd::erase_if(spells, [=](const SpellID & spell)
 		{
 			return VLC->spellh->objects[spell]->level != si32(value["level"].Float());
 		});
@@ -189,7 +189,7 @@ namespace JsonRandom
 		return SpellID(*RandomGeneratorUtil::nextItem(spells, rng));
 	}
 
-	std::vector<SpellID> loadSpells(const JsonNode & value, CRandomGenerator & rng, std::vector<SpellID> spells)
+	std::vector<SpellID> loadSpells(const JsonNode & value, CRandomGenerator & rng, const std::vector<SpellID> & spells)
 	{
 		// possible extensions: (taken from spell json config)
 		// "type": "adventure",//"adventure", "combat", "ability"
@@ -247,7 +247,7 @@ namespace JsonRandom
 			info.allowedCreatures.push_back(crea);
 			if (node["upgradeChance"].Float() > 0)
 			{
-				for (auto creaID : crea->upgrades)
+				for(const auto & creaID : crea->upgrades)
 					info.allowedCreatures.push_back(VLC->creh->objects[creaID]);
 			}
 			ret.push_back(info);

lib/mapObjects/JsonRandom.h

@@ -42,7 +42,7 @@ namespace JsonRandom
 	DLL_LINKAGE std::vector<ArtifactID> loadArtifacts(const JsonNode & value, CRandomGenerator & rng);
 
 	DLL_LINKAGE SpellID loadSpell(const JsonNode & value, CRandomGenerator & rng, std::vector<SpellID> spells);
-	DLL_LINKAGE std::vector<SpellID> loadSpells(const JsonNode & value, CRandomGenerator & rng, std::vector<SpellID> spells);
+	DLL_LINKAGE std::vector<SpellID> loadSpells(const JsonNode & value, CRandomGenerator & rng, const std::vector<SpellID> & spells);
 
 	DLL_LINKAGE CStackBasicDescriptor loadCreature(const JsonNode & value, CRandomGenerator & rng);
 	DLL_LINKAGE std::vector<CStackBasicDescriptor> loadCreatures(const JsonNode & value, CRandomGenerator & rng);

lib/mapObjects/MiscObjects.cpp

@@ -42,7 +42,7 @@ static void openWindow(const OpenWindow::EWindow type, const int id1, const int
 	IObjectInterface::cb->sendAndApply(&ow);
 }
 
-static void showInfoDialog(const PlayerColor playerID, const ui32 txtID, const ui16 soundID = 0)
+static void showInfoDialog(const PlayerColor & playerID, const ui32 txtID, const ui16 soundID = 0)
 {
 	InfoWindow iw;
 	if(soundID)
@@ -80,9 +80,9 @@ bool CTeamVisited::wasVisited(const CGHeroInstance * h) const
 	return wasVisited(h->tempOwner);
 }
 
-bool CTeamVisited::wasVisited(TeamID team) const
+bool CTeamVisited::wasVisited(const TeamID & team) const
 {
-	for(auto i : players)
+	for(const auto & i : players)
 	{
 		if(cb->getPlayerState(i)->team == team)
 			return true;
@@ -150,7 +150,7 @@ std::string CGCreature::getHoverText(const CGHeroInstance * hero) const
 	hoverName += VLC->generaltexth->translate("vcmi.adventureMap.monsterThreat.title");
 
 	int choice;
-	double ratio = ((double)getArmyStrength() / hero->getTotalStrength());
+	double ratio = (static_cast<double>(getArmyStrength()) / hero->getTotalStrength());
 		 if (ratio < 0.1)  choice = 0;
 	else if (ratio < 0.25) choice = 1;
 	else if (ratio < 0.6)  choice = 2;
@@ -198,8 +198,8 @@ void CGCreature::onHeroVisit( const CGHeroInstance * h ) const
 			BlockingDialog ynd(true,false);
 			ynd.player = h->tempOwner;
 			std::string tmp = VLC->generaltexth->advobtxt[90];
-			boost::algorithm::replace_first(tmp,"%d",boost::lexical_cast<std::string>(getStackCount(SlotID(0))));
-			boost::algorithm::replace_first(tmp,"%d",boost::lexical_cast<std::string>(action));
+			boost::algorithm::replace_first(tmp, "%d", std::to_string(getStackCount(SlotID(0))));
+			boost::algorithm::replace_first(tmp, "%d", std::to_string(action));
 			boost::algorithm::replace_first(tmp,"%s",VLC->creh->objects[subID]->getNamePluralTranslated());
 			ynd.text << tmp;
 			cb->showBlockingDialog(&ynd);
@@ -245,7 +245,7 @@ void CGCreature::initObj(CRandomGenerator & rand)
 		}
 	}
 
-	temppower = stacks[SlotID(0)]->count * (ui64)1000;
+	temppower = stacks[SlotID(0)]->count * static_cast<ui64>(1000);
 	refusedJoining = false;
 }
 
@@ -256,7 +256,7 @@ void CGCreature::newTurn(CRandomGenerator & rand) const
 		if (stacks.begin()->second->count < VLC->modh->settings.CREEP_SIZE && cb->getDate(Date::DAY_OF_WEEK) == 1 && cb->getDate(Date::DAY) > 1)
 		{
 			ui32 power = static_cast<ui32>(temppower * (100 + VLC->modh->settings.WEEKLY_GROWTH) / 100);
-			cb->setObjProperty(id, ObjProperty::MONSTER_COUNT, std::min(power / 1000, (ui32)VLC->modh->settings.CREEP_SIZE)); //set new amount
+			cb->setObjProperty(id, ObjProperty::MONSTER_COUNT, std::min(power / 1000, static_cast<ui32>(VLC->modh->settings.CREEP_SIZE))); //set new amount
 			cb->setObjProperty(id, ObjProperty::MONSTER_POWER, power); //increase temppower
 		}
 	}
@@ -288,14 +288,14 @@ void CGCreature::setPropertyDer(ui8 what, ui32 val)
 int CGCreature::takenAction(const CGHeroInstance *h, bool allowJoin) const
 {
 	//calculate relative strength of hero and creatures armies
-	double relStrength = double(h->getTotalStrength()) / getArmyStrength();
+	double relStrength = static_cast<double>(h->getTotalStrength()) / getArmyStrength();
 
 	int powerFactor;
 	if(relStrength >= 7)
 		powerFactor = 11;
 
 	else if(relStrength >= 1)
-		powerFactor = (int)(2*(relStrength-1));
+		powerFactor = static_cast<int>(2 * (relStrength - 1));
 
 	else if(relStrength >= 0.5)
 		powerFactor = -1;
@@ -316,10 +316,10 @@ int CGCreature::takenAction(const CGHeroInstance *h, bool allowJoin) const
 			myKindCres.insert(crea->idNumber);
 	}
 
-	int count = 0, //how many creatures of similar kind has hero
-		totalCount = 0;
+	int count = 0; //how many creatures of similar kind has hero
+	int totalCount = 0;
 
-	for (auto & elem : h->Slots())
+	for(const auto & elem : h->Slots())
 	{
 		if(vstd::contains(myKindCres,elem.second->type->idNumber))
 			count += elem.second->count;
@@ -439,7 +439,7 @@ void CGCreature::fight( const CGHeroInstance *h ) const
 	{
 		if (containsUpgradedStack()) //upgrade
 		{
-			SlotID slotID = SlotID((si32)(std::floor((float)stacks.size() / 2.0f)));
+			SlotID slotID = SlotID(static_cast<si32>(std::floor(static_cast<float>(stacks.size()) / 2.0f)));
 			const auto & upgrades = getStack(slotID).type->upgrades;
 			if(!upgrades.empty())
 			{
@@ -527,7 +527,7 @@ bool CGCreature::containsUpgradedStack() const
 	float c = 5325.181015f;
 	float d = 32788.727920f;
 
-	int val = (int)std::floor (a*pos.x + b*pos.y + c*pos.z + d);
+	int val = static_cast<int>(std::floor(a * pos.x + b * pos.y + c * pos.z + d));
 	return ((val % 32768) % 100) < 50;
 }
 
@@ -535,7 +535,7 @@ int CGCreature::getNumberOfStacks(const CGHeroInstance *hero) const
 {
 	//source http://heroescommunity.com/viewthread.php3?TID=27539&PID=1266094#focus
 
-	double strengthRatio = (double)hero->getArmyStrength() / getArmyStrength();
+	double strengthRatio = static_cast<double>(hero->getArmyStrength()) / getArmyStrength();
 	int split = 1;
 
 	if (strengthRatio < 0.5f)
@@ -556,7 +556,7 @@ int CGCreature::getNumberOfStacks(const CGHeroInstance *hero) const
 	ui32 c = 1943276003u;
 	ui32 d = 3174620878u;
 
-	ui32 R1 = a * ui32(pos.x) + b * ui32(pos.y) + c * ui32(pos.z) + d;
+	ui32 R1 = a * static_cast<ui32>(pos.x) + b * static_cast<ui32>(pos.y) + c * static_cast<ui32>(pos.z) + d;
 	ui32 R2 = (R1 >> 16) & 0x7fff;
 
 	int R4 = R2 % 100 + 1;
@@ -577,23 +577,23 @@ void CGCreature::giveReward(const CGHeroInstance * h) const
 	InfoWindow iw;
 	iw.player = h->tempOwner;
 
-	if(resources.size())
+	if(!resources.empty())
 	{
 		cb->giveResources(h->tempOwner, resources);
 		for(int i = 0; i < resources.size(); i++)
 		{
 			if(resources[i] > 0)
-				iw.components.push_back(Component(Component::RESOURCE, i, resources[i], 0));
+				iw.components.emplace_back(Component::RESOURCE, i, resources[i], 0);
 		}
 	}
 
 	if(gainedArtifact != ArtifactID::NONE)
 	{
 		cb->giveHeroNewArtifact(h, VLC->arth->objects[gainedArtifact], ArtifactPosition::FIRST_AVAILABLE);
-		iw.components.push_back(Component(Component::ARTIFACT, gainedArtifact, 0, 0));
+		iw.components.emplace_back(Component::ARTIFACT, gainedArtifact, 0, 0);
 	}
 
-	if(iw.components.size())
+	if(!iw.components.empty())
 	{
 		iw.text.addTxt(MetaString::ADVOB_TXT, 183); // % has found treasure
 		iw.text.addReplacement(h->getNameTranslated());
@@ -622,7 +622,7 @@ void CGCreature::serializeJsonOptions(JsonSerializeFormat & handler)
 	{
 		si32 amount = 0;
 		handler.serializeInt("amount", amount);
-		auto  hlp = new CStackInstance();
+		auto * hlp = new CStackInstance();
 		hlp->count = amount;
 		//type will be set during initialization
 		putStack(SlotID(0), hlp);
@@ -680,7 +680,7 @@ void CGMine::initObj(CRandomGenerator & rand)
 	{
 		//set guardians
 		int howManyTroglodytes = rand.nextInt(100, 199);
-		auto troglodytes = new CStackInstance(CreatureID::TROGLODYTES, howManyTroglodytes);
+		auto * troglodytes = new CStackInstance(CreatureID::TROGLODYTES, howManyTroglodytes);
 		putStack(SlotID(0), troglodytes);
 
 		//after map reading tempOwner placeholds bitmask for allowed resources
@@ -730,7 +730,7 @@ std::string CGMine::getHoverText(PlayerColor player) const
 	return hoverName;
 }
 
-void CGMine::flagMine(PlayerColor player) const
+void CGMine::flagMine(const PlayerColor & player) const
 {
 	assert(tempOwner != player);
 	cb->setOwner(this, player); //not ours? flag it!
@@ -739,11 +739,11 @@ void CGMine::flagMine(PlayerColor player) const
 	iw.soundID = soundBase::FLAGMINE;
 	iw.text.addTxt(MetaString::MINE_EVNTS,producedResource); //not use subID, abandoned mines uses default mine texts
 	iw.player = player;
-	iw.components.push_back(Component(Component::RESOURCE,producedResource,producedQuantity,-1));
+	iw.components.emplace_back(Component::RESOURCE, producedResource, producedQuantity, -1);
 	cb->showInfoDialog(&iw);
 }
 
-ui32 CGMine::defaultResProduction()
+ui32 CGMine::defaultResProduction() const
 {
 	switch(producedResource)
 	{
@@ -801,10 +801,10 @@ void CGMine::serializeJsonOptions(JsonSerializeFormat & handler)
 			const JsonNode & node = handler.getCurrent();
 			std::set<int> possibleResources;
 
-			if(node.getType() != JsonNode::JsonType::DATA_VECTOR || node.Vector().size() == 0)
+			if(node.getType() != JsonNode::JsonType::DATA_VECTOR || node.Vector().empty())
 			{
 				//assume all allowed
-				for(int i = (int)Res::WOOD; i < (int) Res::GOLD; i++)
+				for(int i = static_cast<int>(Res::WOOD); i < static_cast<int>(Res::GOLD); i++)
 					possibleResources.insert(i);
 			}
 			else
@@ -839,11 +839,6 @@ std::string CGResource::getHoverText(PlayerColor player) const
 	return VLC->generaltexth->restypes[subID];
 }
 
-CGResource::CGResource()
-{
-	amount = CGResource::RANDOM_AMOUNT;
-}
-
 void CGResource::initObj(CRandomGenerator & rand)
 {
 	blockVisit = true;
@@ -869,7 +864,7 @@ void CGResource::onHeroVisit( const CGHeroInstance * h ) const
 {
 	if(stacksCount())
 	{
-		if(message.size())
+		if(!message.empty())
 		{
 			BlockingDialog ynd(true,false);
 			ynd.player = h->getOwner();
@@ -894,7 +889,7 @@ void CGResource::onHeroVisit( const CGHeroInstance * h ) const
 	}
 }
 
-void CGResource::collectRes( PlayerColor player ) const
+void CGResource::collectRes(const PlayerColor & player) const
 {
 	cb->giveResource(player, static_cast<Res::ERes>(subID), amount);
 	ShowInInfobox sii;
@@ -925,11 +920,6 @@ void CGResource::serializeJsonOptions(JsonSerializeFormat & handler)
 	handler.serializeString("guardMessage", message);
 }
 
-CGTeleport::CGTeleport() :
-	type(UNKNOWN), channel(TeleportChannelID())
-{
-}
-
 bool CGTeleport::isEntrance() const
 {
 	return type == BOTH || type == ENTRANCE;
@@ -940,12 +930,12 @@ bool CGTeleport::isExit() const
 	return type == BOTH || type == EXIT;
 }
 
-bool CGTeleport::isChannelEntrance(ObjectInstanceID id) const
+bool CGTeleport::isChannelEntrance(const ObjectInstanceID & id) const
 {
 	return vstd::contains(getAllEntrances(), id);
 }
 
-bool CGTeleport::isChannelExit(ObjectInstanceID id) const
+bool CGTeleport::isChannelExit(const ObjectInstanceID & id) const
 {
 	return vstd::contains(getAllExits(), id);
 }
@@ -971,7 +961,7 @@ std::vector<ObjectInstanceID> CGTeleport::getAllExits(bool excludeCurrent) const
 ObjectInstanceID CGTeleport::getRandomExit(const CGHeroInstance * h) const
 {
 	auto passableExits = getPassableExits(cb->gameState(), h, getAllExits(true));
-	if(passableExits.size())
+	if(!passableExits.empty())
 		return *RandomGeneratorUtil::nextItem(passableExits, CRandomGenerator::getDefault());
 
 	return ObjectInstanceID();
@@ -989,14 +979,14 @@ bool CGTeleport::isConnected(const CGTeleport * src, const CGTeleport * dst)
 
 bool CGTeleport::isConnected(const CGObjectInstance * src, const CGObjectInstance * dst)
 {
-	auto srcObj = dynamic_cast<const CGTeleport *>(src);
-	auto dstObj = dynamic_cast<const CGTeleport *>(dst);
+	const auto * srcObj = dynamic_cast<const CGTeleport *>(src);
+	const auto * dstObj = dynamic_cast<const CGTeleport *>(dst);
 	return isConnected(srcObj, dstObj);
 }
 
 bool CGTeleport::isExitPassable(CGameState * gs, const CGHeroInstance * h, const CGObjectInstance * obj)
 {
-	auto objTopVisObj = gs->map->getTile(obj->visitablePos()).topVisitableObj();
+	auto * objTopVisObj = gs->map->getTile(obj->visitablePos()).topVisitableObj();
 	if(objTopVisObj->ID == Obj::HERO)
 	{
 		if(h->id == objTopVisObj->id) // Just to be sure it's won't happen.
@@ -1015,7 +1005,7 @@ bool CGTeleport::isExitPassable(CGameState * gs, const CGHeroInstance * h, const
 
 std::vector<ObjectInstanceID> CGTeleport::getPassableExits(CGameState * gs, const CGHeroInstance * h, std::vector<ObjectInstanceID> exits)
 {
-	vstd::erase_if(exits, [&](ObjectInstanceID exit) -> bool
+	vstd::erase_if(exits, [&](const ObjectInstanceID & exit) -> bool 
 	{
 		return !isExitPassable(gs, h, gs->getObj(exit));
 	});
@@ -1039,21 +1029,21 @@ void CGTeleport::addToChannel(std::map<TeleportChannelID, std::shared_ptr<Telepo
 	if(obj->isExit() && !vstd::contains(tc->exits, obj->id))
 		tc->exits.push_back(obj->id);
 
-	if(tc->entrances.size() && tc->exits.size()
+	if(!tc->entrances.empty() && !tc->exits.empty()
 		&& (tc->entrances.size() != 1 || tc->entrances != tc->exits))
 	{
 		tc->passability = TeleportChannel::PASSABLE;
 	}
 }
 
-TeleportChannelID CGMonolith::findMeChannel(std::vector<Obj> IDs, int SubID) const
+TeleportChannelID CGMonolith::findMeChannel(const std::vector<Obj> & IDs, int SubID) const
 {
 	for(auto obj : cb->gameState()->map->objects)
 	{
 		if(!obj)
 			continue;
 
-		auto teleportObj = dynamic_cast<const CGTeleport *>(cb->getObj(obj->id));
+		const auto * teleportObj = dynamic_cast<const CGTeleport *>(cb->getObj(obj->id));
 		if(teleportObj && vstd::contains(IDs, teleportObj->ID) && teleportObj->subID == SubID)
 			return teleportObj->channel;
 	}
@@ -1068,7 +1058,7 @@ void CGMonolith::onHeroVisit( const CGHeroInstance * h ) const
 		if(cb->isTeleportChannelBidirectional(channel) && 1 < cb->getTeleportChannelExits(channel).size())
 		{
 			auto exits = cb->getTeleportChannelExits(channel);
-			for(auto exit : exits)
+			for(const auto & exit : exits)
 			{
 				td.exits.push_back(std::make_pair(exit, h->convertFromVisitablePos(cb->getObj(exit)->visitablePos())));
 			}
@@ -1094,7 +1084,7 @@ void CGMonolith::teleportDialogAnswered(const CGHeroInstance *hero, ui32 answer,
 	auto randomExit = getRandomExit(hero);
 	auto realExits = getAllExits(true);
 	if(!isEntrance() // Do nothing if hero visited exit only object
-		|| (!exits.size() && !realExits.size()) // Do nothing if there no exits on this channel
+		|| (exits.empty() && realExits.empty()) // Do nothing if there no exits on this channel
 		|| ObjectInstanceID() == randomExit) // Do nothing if all exits are blocked by friendly hero and it's not subterranean gate
 	{
 		return;
@@ -1115,11 +1105,11 @@ void CGMonolith::initObj(CRandomGenerator & rand)
 	{
 	case Obj::MONOLITH_ONE_WAY_ENTRANCE:
 		type = ENTRANCE;
-		IDs.push_back(Obj::MONOLITH_ONE_WAY_EXIT);
+		IDs.emplace_back(Obj::MONOLITH_ONE_WAY_EXIT);
 		break;
 	case Obj::MONOLITH_ONE_WAY_EXIT:
 		type = EXIT;
-		IDs.push_back(Obj::MONOLITH_ONE_WAY_ENTRANCE);
+		IDs.emplace_back(Obj::MONOLITH_ONE_WAY_ENTRANCE);
 		break;
 	case Obj::MONOLITH_TWO_WAY:
 	default:
@@ -1129,7 +1119,7 @@ void CGMonolith::initObj(CRandomGenerator & rand)
 
 	channel = findMeChannel(IDs, subID);
 	if(channel == TeleportChannelID())
-		channel = TeleportChannelID((si32)cb->gameState()->map->teleportChannels.size());
+		channel = TeleportChannelID(static_cast<si32>(cb->gameState()->map->teleportChannels.size()));
 
 	addToChannel(cb->gameState()->map->teleportChannels, this);
 }
@@ -1166,7 +1156,7 @@ void CGSubterraneanGate::postInit() //matches subterranean gates into pairs
 		if(!obj) // FIXME: Find out why there are nullptr objects right after initialization
 			continue;
 
-		auto hlp = dynamic_cast<CGSubterraneanGate *>(cb->gameState()->getObjInstance(obj->id));
+		auto * hlp = dynamic_cast<CGSubterraneanGate *>(cb->gameState()->getObjInstance(obj->id));
 		if(hlp)
 			gatesSplit[hlp->pos.z].push_back(hlp);
 	}
@@ -1181,7 +1171,7 @@ void CGSubterraneanGate::postInit() //matches subterranean gates into pairs
 	{
 		if(obj->channel == TeleportChannelID())
 		{ // if object not linked to channel then create new channel
-			obj->channel = TeleportChannelID((si32)cb->gameState()->map->teleportChannels.size());
+			obj->channel = TeleportChannelID(static_cast<si32>(cb->gameState()->map->teleportChannels.size()));
 			addToChannel(cb->gameState()->map->teleportChannels, obj);
 		}
 	};
@@ -1214,8 +1204,8 @@ void CGSubterraneanGate::postInit() //matches subterranean gates into pairs
 	}
 
 	// we should assign empty channels to underground gates if they don't have matching overground gates
-	for(size_t i = 0; i < gatesSplit[1].size(); i++)
-		assignToChannel(gatesSplit[1][i]);
+	for(auto & i : gatesSplit[1])
+		assignToChannel(i);
 }
 
 void CGWhirlpool::onHeroVisit( const CGHeroInstance * h ) const
@@ -1238,23 +1228,23 @@ void CGWhirlpool::onHeroVisit( const CGHeroInstance * h ) const
 				targetstack = (i->first);
 		}
 
-		TQuantity countToTake = static_cast<TQuantity>(h->getStackCount(targetstack) * 0.5);
+		auto countToTake = static_cast<TQuantity>(h->getStackCount(targetstack) * 0.5);
 		vstd::amax(countToTake, 1);
 
 		InfoWindow iw;
 		iw.player = h->tempOwner;
 		iw.text.addTxt(MetaString::ADVOB_TXT, 168);
-		iw.components.push_back(Component(CStackBasicDescriptor(h->getCreature(targetstack), countToTake)));
+		iw.components.emplace_back(CStackBasicDescriptor(h->getCreature(targetstack), countToTake));
 		cb->showInfoDialog(&iw);
 		cb->changeStackCount(StackLocation(h, targetstack), -countToTake);
 	}
 	else
 	{
 		auto exits = getAllExits();
-		for(auto exit : exits)
+		for(const auto & exit : exits)
 		{
 			auto blockedPosList = cb->getObj(exit)->getBlockedPos();
-			for(auto bPos : blockedPosList)
+			for(const auto & bPos : blockedPosList)
 				td.exits.push_back(std::make_pair(exit, h->convertFromVisitablePos(bPos)));
 		}
 	}
@@ -1266,7 +1256,7 @@ void CGWhirlpool::teleportDialogAnswered(const CGHeroInstance *hero, ui32 answer
 {
 	int3 dPos;
 	auto realExits = getAllExits();
-	if(!exits.size() && !realExits.size())
+	if(exits.empty() && realExits.empty())
 		return;
 	else if(vstd::isValidIndex(exits, answer))
 		dPos = exits[answer].second;
@@ -1277,7 +1267,7 @@ void CGWhirlpool::teleportDialogAnswered(const CGHeroInstance *hero, ui32 answer
 		if(exit == ObjectInstanceID())
 			return;
 
-		auto obj = cb->getObj(exit);
+		const auto * obj = cb->getObj(exit);
 		std::set<int3> tiles = obj->getBlockedPos();
 		dPos = hero->convertFromVisitablePos(*RandomGeneratorUtil::nextItem(tiles, CRandomGenerator::getDefault()));
 	}
@@ -1298,7 +1288,7 @@ void CGArtifact::initObj(CRandomGenerator & rand)
 	{
 		if (!storedArtifact)
 		{
-			auto a = new CArtifactInstance();
+			auto * a = new CArtifactInstance();
 			cb->gameState()->map->addNewArtifactInstance(a);
 			storedArtifact = a;
 		}
@@ -1329,7 +1319,7 @@ void CGArtifact::onHeroVisit(const CGHeroInstance * h) const
 		{
 		case Obj::ARTIFACT:
 			{
-				iw.components.push_back(Component(Component::ARTIFACT, subID, 0, 0));
+				iw.components.emplace_back(Component::ARTIFACT, subID, 0, 0);
 				if(message.length())
 					iw.text << message;
 				else
@@ -1339,7 +1329,7 @@ void CGArtifact::onHeroVisit(const CGHeroInstance * h) const
 		case Obj::SPELL_SCROLL:
 			{
 				int spellID = storedArtifact->getGivenSpellID();
-				iw.components.push_back(Component(Component::SPELL, spellID, 0, 0));
+				iw.components.emplace_back(Component::SPELL, spellID, 0, 0);
 				if(message.length())
 					iw.text << message;
 				else
@@ -1466,7 +1456,7 @@ void CGWitchHut::onHeroVisit( const CGHeroInstance * h ) const
 	}
 	else //give sec skill
 	{
-		iw.components.push_back(Component(Component::SEC_SKILL, ability, 1, 0));
+		iw.components.emplace_back(Component::SEC_SKILL, ability, 1, 0);
 		txt_id = 171;
 		cb->changeSecSkill(h, SecondarySkill(ability), 1, true);
 	}
@@ -1591,7 +1581,7 @@ void CGShrine::onHeroVisit( const CGHeroInstance * h ) const
 		spells.insert(spell);
 		cb->changeSpells(h, true, spells);
 
-		iw.components.push_back(Component(Component::SPELL,spell,0,0));
+		iw.components.emplace_back(Component::SPELL, spell, 0, 0);
 	}
 
 	cb->showInfoDialog(&iw);
@@ -1692,18 +1682,18 @@ void CGScholar::onHeroVisit( const CGHeroInstance * h ) const
 	{
 	case PRIM_SKILL:
 		cb->changePrimSkill(h,static_cast<PrimarySkill::PrimarySkill>(bid),+1);
-		iw.components.push_back(Component(Component::PRIM_SKILL,bid,+1,0));
+		iw.components.emplace_back(Component::PRIM_SKILL, bid, +1, 0);
 		break;
 	case SECONDARY_SKILL:
 		cb->changeSecSkill(h,SecondarySkill(bid),+1);
-		iw.components.push_back(Component(Component::SEC_SKILL,bid,ssl+1,0));
+		iw.components.emplace_back(Component::SEC_SKILL, bid, ssl + 1, 0);
 		break;
 	case SPELL:
 		{
 			std::set<SpellID> hlp;
 			hlp.insert(SpellID(bid));
 			cb->changeSpells(h,true,hlp);
-			iw.components.push_back(Component(Component::SPELL,bid,0,0));
+			iw.components.emplace_back(Component::SPELL, bid, 0, 0);
 		}
 		break;
 	default:
@@ -1727,7 +1717,7 @@ void CGScholar::initObj(CRandomGenerator & rand)
 			bonusID = rand.nextInt(GameConstants::PRIMARY_SKILLS -1);
 			break;
 		case SECONDARY_SKILL:
-			bonusID = rand.nextInt((int)VLC->skillh->size() - 1);
+			bonusID = rand.nextInt(static_cast<int>(VLC->skillh->size()) - 1);
 			break;
 		case SPELL:
 			std::vector<SpellID> possibilities;
@@ -1870,7 +1860,7 @@ void CGMagi::onHeroVisit(const CGHeroInstance * h) const
 			fw.mode = 1;
 			fw.waitForDialogs = true;
 
-			for(auto it : eyelist[subID])
+			for(const auto & it : eyelist[subID])
 			{
 				const CGObjectInstance *eye = cb->getObj(it);
 
@@ -1937,9 +1927,9 @@ void CGSirens::onHeroVisit( const CGHeroInstance * h ) const
 
 		if(xp)
 		{
-			xp = h->calculateXp((int)xp);
+			xp = h->calculateXp(static_cast<int>(xp));
 			iw.text.addTxt(MetaString::ADVOB_TXT,132);
-			iw.text.addReplacement((int)xp);
+			iw.text.addReplacement(static_cast<int>(xp));
 			cb->changePrimSkill(h, PrimarySkill::EXPERIENCE, xp, false);
 		}
 		else
@@ -2089,7 +2079,7 @@ void CGObelisk::onHeroVisit( const CGHeroInstance * h ) const
 		openWindow(OpenWindow::PUZZLE_MAP, h->tempOwner.getNum());
 
 		// mark that particular obelisk as visited for all players in the team
-		for (auto & color : ts->players)
+		for(const auto & color : ts->players)
 		{
 			cb->setObjProperty(id, CGObelisk::OBJPROP_VISITED, color.getNum());
 		}
@@ -2170,7 +2160,7 @@ void CGLighthouse::initObj(CRandomGenerator & rand)
 	}
 }
 
-void CGLighthouse::giveBonusTo(PlayerColor player, bool onInit) const
+void CGLighthouse::giveBonusTo(const PlayerColor & player, bool onInit) const
 {
 	GiveBonus gb(GiveBonus::PLAYER);
 	gb.bonus.type = Bonus::SEA_MOVEMENT;

lib/mapObjects/MiscObjects.h

@@ -25,7 +25,7 @@ public:
 
 	bool wasVisited (const CGHeroInstance * h) const override;
 	bool wasVisited(PlayerColor player) const override;
-	bool wasVisited(TeamID team) const;
+	bool wasVisited(const TeamID & team) const;
 	void setPropertyDer(ui8 what, ui32 val) override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
@@ -34,7 +34,7 @@ public:
 		h & players;
 	}
 
-	static const int OBJPROP_VISITED = 10;
+	static constexpr int OBJPROP_VISITED = 10;
 };
 
 class DLL_LINKAGE CGCreature : public CArmedInstance //creatures on map
@@ -219,19 +219,18 @@ protected:
 class DLL_LINKAGE CGResource : public CArmedInstance
 {
 public:
-	static const ui32 RANDOM_AMOUNT = 0;
-	ui32 amount; //0 if random
+	static constexpr ui32 RANDOM_AMOUNT = 0;
+	ui32 amount = RANDOM_AMOUNT; //0 if random
 	
 	std::string message;
 
-	CGResource();
 	void onHeroVisit(const CGHeroInstance * h) const override;
 	void initObj(CRandomGenerator & rand) override;
 	void battleFinished(const CGHeroInstance *hero, const BattleResult &result) const override;
 	void blockingDialogAnswered(const CGHeroInstance *hero, ui32 answer) const override;
 	std::string getHoverText(PlayerColor player) const override;
 
-	void collectRes(PlayerColor player) const;
+	void collectRes(const PlayerColor & player) const;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -272,7 +271,7 @@ private:
 	void battleFinished(const CGHeroInstance *hero, const BattleResult &result) const override;
 	void blockingDialogAnswered(const CGHeroInstance *hero, ui32 answer) const override;
 
-	void flagMine(PlayerColor player) const;
+	void flagMine(const PlayerColor & player) const;
 	void newTurn(CRandomGenerator & rand) const override;
 	void initObj(CRandomGenerator & rand) override;
 
@@ -287,7 +286,8 @@ public:
 		h & producedResource;
 		h & producedQuantity;
 	}
-	ui32 defaultResProduction();
+	ui32 defaultResProduction() const;
+
 protected:
 	void serializeJsonOptions(JsonSerializeFormat & handler) override;
 };
@@ -296,11 +296,9 @@ struct DLL_LINKAGE TeleportChannel
 {
 	enum EPassability {UNKNOWN, IMPASSABLE, PASSABLE};
 
-	TeleportChannel() : passability(UNKNOWN) {}
-
 	std::vector<ObjectInstanceID> entrances;
 	std::vector<ObjectInstanceID> exits;
-	EPassability passability;
+	EPassability passability = EPassability::UNKNOWN;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -312,16 +310,15 @@ struct DLL_LINKAGE TeleportChannel
 
 class DLL_LINKAGE CGTeleport : public CGObjectInstance
 {
-	bool isChannelEntrance(ObjectInstanceID id) const;
-	bool isChannelExit(ObjectInstanceID id) const;
+	bool isChannelEntrance(const ObjectInstanceID & id) const;
+	bool isChannelExit(const ObjectInstanceID & id) const;
 
 	std::vector<ObjectInstanceID> getAllEntrances(bool excludeCurrent = false) const;
 
 protected:
 	enum EType {UNKNOWN, ENTRANCE, EXIT, BOTH};
-	EType type;
+	EType type = EType::UNKNOWN;
 
-	CGTeleport();
 	ObjectInstanceID getRandomExit(const CGHeroInstance * h) const;
 	std::vector<ObjectInstanceID> getAllExits(bool excludeCurrent = false) const;
 
@@ -350,7 +347,7 @@ public:
 
 class DLL_LINKAGE CGMonolith : public CGTeleport
 {
-	TeleportChannelID findMeChannel(std::vector<Obj> IDs, int SubID) const;
+	TeleportChannelID findMeChannel(const std::vector<Obj> & IDs, int SubID) const;
 
 protected:
 	void onHeroVisit(const CGHeroInstance * h) const override;
@@ -489,7 +486,7 @@ class DLL_LINKAGE CGDenOfthieves : public CGObjectInstance
 class DLL_LINKAGE CGObelisk : public CTeamVisited
 {
 public:
-	static const int OBJPROP_INC = 20;
+	static constexpr int OBJPROP_INC = 20;
 	static ui8 obeliskCount; //how many obelisks are on map
 	static std::map<TeamID, ui8> visited; //map: team_id => how many obelisks has been visited
 
@@ -516,7 +513,8 @@ public:
 	{
 		h & static_cast<CGObjectInstance&>(*this);
 	}
-	void giveBonusTo(PlayerColor player, bool onInit = false) const;
+	void giveBonusTo(const PlayerColor & player, bool onInit = false) const;
+
 protected:
 	void serializeJsonOptions(JsonSerializeFormat & handler) override;
 };

lib/mapObjects/ObjectTemplate.cpp

@@ -272,7 +272,7 @@ void ObjectTemplate::readJson(const JsonNode &node, const bool withTerrain)
 
 	if(withTerrain && !node["allowedTerrains"].isNull())
 	{
-		for(auto& entry : node["allowedTerrains"].Vector())
+		for(const auto & entry : node["allowedTerrains"].Vector())
 		{
 			VLC->modh->identifiers.requestIdentifier("terrain", entry, [this](int32_t identifier){
 				allowedTerrains.insert(TerrainId(identifier));
@@ -306,10 +306,10 @@ void ObjectTemplate::readJson(const JsonNode &node, const bool withTerrain)
 
 	size_t height = mask.size();
 	size_t width  = 0;
-	for(auto & line : mask)
+	for(const auto & line : mask)
 		vstd::amax(width, line.String().size());
 
-	setSize((ui32)width, (ui32)height);
+	setSize(static_cast<ui32>(width), static_cast<ui32>(height));
 
 	for(size_t i = 0; i < mask.size(); i++)
 	{
@@ -420,7 +420,7 @@ void ObjectTemplate::calculateWidth()
 	//TODO: Use 2D array
 	for(const auto& row : usedTiles) //copy is expensive
 	{
-		width = std::max<ui32>(width, (ui32)row.size());
+		width = std::max<ui32>(width, static_cast<ui32>(row.size()));
 	}
 }
 
@@ -437,7 +437,7 @@ void ObjectTemplate::setSize(ui32 width, ui32 height)
 		line.resize(width, 0);
 }
 
-void ObjectTemplate::calculateVsitable()
+void ObjectTemplate::calculateVisitable()
 {
 	for(auto& line : usedTiles)
 	{
@@ -457,7 +457,7 @@ bool ObjectTemplate::isWithin(si32 X, si32 Y) const
 {
 	if (X < 0 || Y < 0)
 		return false;
-	return !(X >= (si32)getWidth() || Y >= (si32)getHeight());
+	return X < static_cast<si32>(getWidth()) && Y < static_cast<si32>(getHeight());
 }
 
 bool ObjectTemplate::isVisitableAt(si32 X, si32 Y) const
@@ -478,9 +478,9 @@ bool ObjectTemplate::isBlockedAt(si32 X, si32 Y) const
 void ObjectTemplate::calculateBlockedOffsets()
 {
 	blockedOffsets.clear();
-	for(int w = 0; w < (int)getWidth(); ++w)
+	for(int w = 0; w < static_cast<int>(getWidth()); ++w)
 	{
-		for(int h = 0; h < (int)getHeight(); ++h)
+		for(int h = 0; h < static_cast<int>(getHeight()); ++h)
 		{
 			if (isBlockedAt(w, h))
 				blockedOffsets.insert(int3(-w, -h, 0));
@@ -490,9 +490,9 @@ void ObjectTemplate::calculateBlockedOffsets()
 
 void ObjectTemplate::calculateBlockMapOffset()
 {
-	for(int w = 0; w < (int)getWidth(); ++w)
+	for(int w = 0; w < static_cast<int>(getWidth()); ++w)
 	{
-		for(int h = 0; h < (int)getHeight(); ++h)
+		for(int h = 0; h < static_cast<int>(getHeight()); ++h)
 		{
 			if (isBlockedAt(w, h))
 			{
@@ -526,9 +526,9 @@ bool ObjectTemplate::isVisitableFrom(si8 X, si8 Y) const
 
 void ObjectTemplate::calculateVisitableOffset()
 {
-	for(int y = 0; y < (int)getHeight(); y++)
+	for(int y = 0; y < static_cast<int>(getHeight()); y++)
 	{
-		for(int x = 0; x < (int)getWidth(); x++)
+		for(int x = 0; x < static_cast<int>(getWidth()); x++)
 		{
 			if (isVisitableAt(x, y))
 			{
@@ -544,7 +544,7 @@ bool ObjectTemplate::canBePlacedAt(TerrainId terrainID) const
 {
 	if (anyTerrain)
 	{
-		auto const & terrain = VLC->terrainTypeHandler->getById(terrainID);
+		const auto & terrain = VLC->terrainTypeHandler->getById(terrainID);
 		return terrain->isLand() && terrain->isPassable();
 	}
 	return vstd::contains(allowedTerrains, terrainID);
@@ -554,7 +554,7 @@ void ObjectTemplate::recalculate()
 {
 	calculateWidth();
 	calculateHeight();
-	calculateVsitable();
+	calculateVisitable();
 	//The lines below use width and height
 	calculateBlockedOffsets();
 	calculateBlockMapOffset();

lib/mapObjects/ObjectTemplate.h

@@ -114,8 +114,8 @@ public:
 	void readTxt(CLegacyConfigParser & parser);
 	void readMsk();
 	void readMap(CBinaryReader & reader);
-	void readJson(const JsonNode & node, const bool withTerrain = true);
-	void writeJson(JsonNode & node, const bool withTerrain = true) const;
+	void readJson(const JsonNode & node, bool withTerrain = true);
+	void writeJson(JsonNode & node, bool withTerrain = true) const;
 
 	bool operator==(const ObjectTemplate& ot) const { return (id == ot.id && subid == ot.subid); }
 
@@ -132,7 +132,7 @@ private:
 
 	void calculateWidth();
 	void calculateHeight();
-	void calculateVsitable();
+	void calculateVisitable();
 	void calculateBlockedOffsets();
 	void calculateBlockMapOffset();
 	void calculateVisitableOffset();