vcmi: modernize lib/spells/effects

lib/spells/effects/Catapult.cpp

@@ -32,17 +32,9 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(Catapult, EFFECT_NAME);
 
-Catapult::Catapult()
-	: LocationEffect(),
-	targetsToAttack(0)
-{
-}
-
-Catapult::~Catapult() = default;
-
 bool Catapult::applicable(Problem & problem, const Mechanics * m) const
 {
-	auto town = m->battle()->battleGetDefendedTown();
+	const auto *town = m->battle()->battleGetDefendedTown();
 
 	if(nullptr == town)
 	{
@@ -113,7 +105,7 @@ void Catapult::apply(ServerCallback * server, const Mechanics * m, const EffectT
 
 		if (attackInfo == ca.attackedParts.end()) // new part
 		{
-			CatapultAttack::AttackInfo newInfo;
+			CatapultAttack::AttackInfo newInfo{};
 			newInfo.damageDealt = 1;
 			newInfo.attackedPart = target;
 			newInfo.destinationTile = m->battle()->wallPartToBattleHex(target);

lib/spells/effects/Catapult.h

@@ -22,16 +22,13 @@ namespace effects
 class Catapult : public LocationEffect
 {
 public:
-	Catapult();
-	virtual ~Catapult();
-
 	bool applicable(Problem & problem, const Mechanics * m) const override;
 	void apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const override;
 
 protected:
 	void serializeJsonEffect(JsonSerializeFormat & handler) override;
 private:
-	int targetsToAttack;
+	int targetsToAttack = 0;
 };
 
 }

lib/spells/effects/Clone.cpp

@@ -28,14 +28,6 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(Clone, EFFECT_NAME);
 
-Clone::Clone()
-	: UnitEffect(),
-	maxTier(0)
-{
-}
-
-Clone::~Clone() = default;
-
 void Clone::apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const
 {
 	for(const Destination & dest : target)
@@ -80,7 +72,7 @@ void Clone::apply(ServerCallback * server, const Mechanics * m, const EffectTarg
 
 		BattleUnitsChanged cloneFlags;
 
-		auto cloneUnit = m->battle()->battleGetUnitByID(unitId);
+		const auto *cloneUnit = m->battle()->battleGetUnitByID(unitId);
 
 		if(!cloneUnit)
 		{
@@ -105,7 +97,7 @@ void Clone::apply(ServerCallback * server, const Mechanics * m, const EffectTarg
 		lifeTimeMarker.turnsRemain = m->getEffectDuration();
 		std::vector<Bonus> buffer;
 		buffer.push_back(lifeTimeMarker);
-		sse.toAdd.push_back(std::make_pair(unitId, buffer));
+		sse.toAdd.emplace_back(unitId, buffer);
 		server->apply(&sse);
 	}
 }

lib/spells/effects/Clone.h

@@ -22,9 +22,6 @@ namespace effects
 class Clone : public UnitEffect
 {
 public:
-	Clone();
-	virtual ~Clone();
-
 	void apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const override;
 protected:
 	bool isReceptive(const Mechanics * m, const battle::Unit * s) const override;
@@ -32,7 +29,7 @@ protected:
 
 	void serializeJsonUnitEffect(JsonSerializeFormat & handler) override final;
 private:
-	int maxTier;
+	int maxTier = 0;
 };
 
 }

lib/spells/effects/Damage.cpp

@@ -31,15 +31,6 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(Damage, EFFECT_NAME);
 
-Damage::Damage()
-	: UnitEffect(),
-	killByPercentage(false),
-	killByCount(false)
-{
-}
-
-Damage::~Damage() = default;
-
 void Damage::apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const
 {
 	StacksInjured stacksInjured;
@@ -52,7 +43,7 @@ void Damage::apply(ServerCallback * server, const Mechanics * m, const EffectTar
 	uint32_t killed = 0;
 	bool multiple = false;
 
-	for(auto & t : target)
+	for(const auto & t : target)
 	{
 		const battle::Unit * unit = t.unitValue;
 		if(unit && unit->alive())

lib/spells/effects/Damage.h

@@ -24,9 +24,6 @@ namespace effects
 class Damage : public UnitEffect
 {
 public:
-	Damage();
-	virtual ~Damage();
-
 	void apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const override;
 
 protected:
@@ -39,8 +36,8 @@ protected:
 	virtual void describeEffect(std::vector<MetaString> & log, const Mechanics * m, const battle::Unit * firstTarget, uint32_t kills, int64_t damage, bool multiple) const;
 
 private:
-	bool killByPercentage;
-	bool killByCount;
+	bool killByPercentage = false;
+	bool killByCount = false;
 };
 
 }

lib/spells/effects/DemonSummon.cpp

@@ -28,15 +28,6 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(DemonSummon, EFFECT_NAME);
 
-DemonSummon::DemonSummon()
-	: UnitEffect()
-	, creature(0)
-	, permanent(false)
-{
-}
-
-DemonSummon::~DemonSummon() = default;
-
 void DemonSummon::apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const
 {
 	BattleUnitsChanged pack;
@@ -60,7 +51,7 @@ void DemonSummon::apply(ServerCallback * server, const Mechanics * m, const Effe
 			break;
 		}
 
-		auto creatureType = creature.toCreature(m->creatures());
+		const auto *creatureType = creature.toCreature(m->creatures());
 
 		int32_t deadCount         = targetStack->unitBaseAmount();
 		int32_t deadTotalHealth   = targetStack->getTotalHealth();
@@ -110,7 +101,7 @@ bool DemonSummon::isValidTarget(const Mechanics * m, const battle::Unit * s) con
 	if (s->isGhost())
 		return false;
 
-	auto creatureType = creature.toCreature(m->creatures());
+	const auto *creatureType = creature.toCreature(m->creatures());
 
 	if (s->getTotalHealth() < creatureType->getMaxHealth())
 		return false;

lib/spells/effects/DemonSummon.h

@@ -23,9 +23,6 @@ namespace effects
 class DemonSummon : public UnitEffect
 {
 public:
-	DemonSummon();
-	virtual ~DemonSummon();
-
 	void apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const override;
 protected:
 	bool isValidTarget(const Mechanics * m, const battle::Unit * s) const override;
@@ -33,9 +30,9 @@ protected:
 	void serializeJsonUnitEffect(JsonSerializeFormat & handler) override final;
 
 private:
-	CreatureID creature;
+	CreatureID creature = CreatureID(0);
 
-	bool permanent;
+	bool permanent = false;
 };
 
 }

lib/spells/effects/Dispel.cpp

@@ -32,21 +32,13 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(Dispel, EFFECT_NAME);
 
-Dispel::Dispel()
-	: UnitEffect()
-{
-
-}
-
-Dispel::~Dispel() = default;
-
 void Dispel::apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const
 {
 	const bool describe = server->describeChanges();
 	SetStackEffect sse;
 	BattleLogMessage blm;
 
-	for(auto & t : target)
+	for(const auto & t : target)
 	{
 		const battle::Unit * unit = t.unitValue;
 		if(unit)
@@ -63,11 +55,11 @@ void Dispel::apply(ServerCallback * server, const Mechanics * m, const EffectTar
 			std::vector<Bonus> buffer;
 			auto bl = getBonuses(m, unit);
 
-			for(auto item : *bl)
+			for(const auto& item : *bl)
 				buffer.emplace_back(*item);
 
 			if(!buffer.empty())
-				sse.toRemove.push_back(std::make_pair(unit->unitId(), buffer));
+				sse.toRemove.emplace_back(unit->unitId(), buffer);
 		}
 	}
 

lib/spells/effects/Dispel.h

@@ -27,9 +27,6 @@ namespace effects
 class Dispel : public UnitEffect
 {
 public:
-	Dispel();
-	virtual ~Dispel();
-
 	void apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const override;
 
 protected:

lib/spells/effects/Effect.cpp

@@ -21,15 +21,6 @@ namespace spells
 namespace effects
 {
 
-Effect::Effect()
-	: indirect(false),
-	optional(false)
-{
-
-}
-
-Effect::~Effect() = default;
-
 bool Effect::applicable(Problem & problem, const Mechanics * m) const
 {
 	return true;
@@ -49,7 +40,7 @@ void Effect::serializeJson(JsonSerializeFormat & handler)
 
 std::shared_ptr<Effect> Effect::create(const Registry * registry, const std::string & type)
 {
-	auto factory = registry->find(type);
+	const auto *factory = registry->find(type);
 
 	if(factory)
 	{

lib/spells/effects/Effect.h

@@ -43,13 +43,12 @@ using TargetType = spells::AimType;
 class DLL_LINKAGE Effect
 {
 public:
-	bool indirect;
-	bool optional;
+	bool indirect = false;
+	bool optional = false;
 
 	std::string name;
 
-	Effect();
-	virtual ~Effect();
+	virtual ~Effect() = default; //Required for child classes
 
 	// TODO: document me
 	virtual void adjustTargetTypes(std::vector<TargetType> & types) const = 0;

lib/spells/effects/Effects.cpp

@@ -29,7 +29,7 @@ Effects::Effects() = default;
 
 Effects::~Effects() = default;
 
-void Effects::add(const std::string & name, std::shared_ptr<Effect> effect, const int level)
+void Effects::add(const std::string & name, const std::shared_ptr<Effect>& effect, const int level)
 {
 	effect->name = name;
 	data.at(level)[name] = effect;
@@ -97,7 +97,7 @@ bool Effects::applicable(Problem & problem, const Mechanics * m, const Target &
 void Effects::forEachEffect(const int level, const std::function<void(const Effect *, bool &)> & callback) const
 {
 	bool stop = false;
-	for(auto one : data.at(level))
+	for(const auto& one : data.at(level))
 	{
 		callback(one.second.get(), stop);
 		if(stop)
@@ -138,7 +138,7 @@ void Effects::serializeJson(const Registry * registry, JsonSerializeFormat & han
 
 	const JsonNode & effectMap = handler.getCurrent();
 
-	for(auto & p : effectMap.Struct())
+	for(const auto & p : effectMap.Struct())
 	{
 		const std::string & name = p.first;
 

lib/spells/effects/Effects.h

@@ -33,7 +33,7 @@ public:
 	Effects();
 	virtual ~Effects();
 
-	void add(const std::string & name, std::shared_ptr<Effect> effect, const int level);
+	void add(const std::string & name, const std::shared_ptr<Effect>& effect, const int level);
 
 	bool applicable(Problem & problem, const Mechanics * m) const;
 	bool applicable(Problem & problem, const Mechanics * m, const Target & aimPoint, const Target & spellTarget) const;

lib/spells/effects/Heal.cpp

@@ -31,17 +31,6 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(Heal, EFFECT_NAME);
 
-Heal::Heal()
-	: UnitEffect(),
-	healLevel(EHealLevel::HEAL),
-	healPower(EHealPower::PERMANENT),
-	minFullUnits(0)
-{
-
-}
-
-Heal::~Heal() = default;
-
 void Heal::apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const
 {
 	apply(m->getEffectValue(), server, m, target);
@@ -117,7 +106,7 @@ void Heal::serializeJsonUnitEffect(JsonSerializeFormat & handler)
 
 void Heal::prepareHealEffect(int64_t value, BattleUnitsChanged & pack, BattleLogMessage & logMessage, RNG & rng, const Mechanics * m, const EffectTarget & target) const
 {
-	for(auto & oneTarget : target)
+	for(const auto & oneTarget : target)
 	{
 		const battle::Unit * unit = oneTarget.unitValue;
 

lib/spells/effects/Heal.h

@@ -26,9 +26,6 @@ namespace effects
 class Heal : public UnitEffect
 {
 public:
-	Heal();
-	virtual ~Heal();
-
 	void apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const override;
 
 protected:
@@ -38,10 +35,10 @@ protected:
 	void serializeJsonUnitEffect(JsonSerializeFormat & handler) override final;
 
 private:
-    EHealLevel healLevel;
-	EHealPower healPower;
+	EHealLevel healLevel = EHealLevel::HEAL;
+	EHealPower healPower = EHealPower::PERMANENT;
 
-	int32_t minFullUnits;
+	int32_t minFullUnits = 0;
 
 	void prepareHealEffect(int64_t value, BattleUnitsChanged & pack, BattleLogMessage & logMessage, RNG & rng, const Mechanics * m, const EffectTarget & target) const;
 };

lib/spells/effects/LocationEffect.cpp

@@ -19,13 +19,6 @@ namespace spells
 namespace effects
 {
 
-LocationEffect::LocationEffect()
-	: Effect()
-{
-}
-
-LocationEffect::~LocationEffect() = default;
-
 void LocationEffect::adjustTargetTypes(std::vector<TargetType> & types) const
 {
 
@@ -33,7 +26,7 @@ void LocationEffect::adjustTargetTypes(std::vector<TargetType> & types) const
 
 void LocationEffect::adjustAffectedHexes(std::set<BattleHex> & hexes, const Mechanics * m, const Target & spellTarget) const
 {
-	for(auto & destnation : spellTarget)
+	for(const auto & destnation : spellTarget)
 		hexes.insert(destnation.hexValue);
 }
 

lib/spells/effects/LocationEffect.h

@@ -23,9 +23,6 @@ namespace effects
 class LocationEffect : public Effect
 {
 public:
-	LocationEffect();
-	virtual ~LocationEffect();
-
 	void adjustTargetTypes(std::vector<TargetType> & types) const override;
 
 	void adjustAffectedHexes(std::set<BattleHex> & hexes, const Mechanics * m, const Target & spellTarget) const override;

lib/spells/effects/Obstacle.cpp

@@ -30,27 +30,9 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(Obstacle, EFFECT_NAME);
 
-ObstacleSideOptions::ObstacleSideOptions()
-	: shape(),
-	range()
-{
-}
-
-void ObstacleSideOptions::serializeJson(JsonSerializeFormat & handler)
-{
-	serializeRelativeShape(handler, "shape", shape);
-	serializeRelativeShape(handler, "range", range);
-
-	handler.serializeString("appearSound", appearSound);
-	handler.serializeString("appearAnimation", appearAnimation);
-	handler.serializeString("triggerSound", triggerSound);
-	handler.serializeString("triggerAnimation", triggerAnimation);
-	handler.serializeString("animation", animation);
-
-	handler.serializeInt("offsetY", offsetY);
-}
+using RelativeShape = std::vector<std::vector<BattleHex::EDir>>;
 
-void ObstacleSideOptions::serializeRelativeShape(JsonSerializeFormat & handler, const std::string & fieldName, RelativeShape & value)
+static void serializeRelativeShape(JsonSerializeFormat & handler, const std::string & fieldName, RelativeShape & value)
 {
 	static const std::vector<std::string> EDirMap =
 	{
@@ -102,19 +84,19 @@ void ObstacleSideOptions::serializeRelativeShape(JsonSerializeFormat & handler,
 	}
 }
 
-Obstacle::Obstacle()
-	: LocationEffect(),
-	hidden(false),
-	passable(false),
-	trigger(false),
-	trap(false),
-	removeOnTrigger(false),
-	patchCount(1),
-	turnsRemaining(-1)
+void ObstacleSideOptions::serializeJson(JsonSerializeFormat & handler)
 {
-}
+	serializeRelativeShape(handler, "shape", shape);
+	serializeRelativeShape(handler, "range", range);
+
+	handler.serializeString("appearSound", appearSound);
+	handler.serializeString("appearAnimation", appearAnimation);
+	handler.serializeString("triggerSound", triggerSound);
+	handler.serializeString("triggerAnimation", triggerAnimation);
+	handler.serializeString("animation", animation);
 
-Obstacle::~Obstacle() = default;
+	handler.serializeInt("offsetY", offsetY);
+}
 
 void Obstacle::adjustAffectedHexes(std::set<BattleHex> & hexes, const Mechanics * m, const Target & spellTarget) const
 {
@@ -124,7 +106,7 @@ void Obstacle::adjustAffectedHexes(std::set<BattleHex> & hexes, const Mechanics
 
 	for(auto & destination : effectTarget)
 	{
-		for(auto & trasformation : options.shape)
+		for(const auto & trasformation : options.shape)
 		{
 			BattleHex hex = destination.hexValue;
 
@@ -154,7 +136,7 @@ bool Obstacle::applicable(Problem & problem, const Mechanics * m, const EffectTa
 
 		for(const auto & destination : target)
 		{
-			for(auto & trasformation : options.shape)
+			for(const auto & trasformation : options.shape)
 			{
 				BattleHex hex = destination.hexValue;
 				for(auto direction : trasformation)
@@ -177,9 +159,9 @@ EffectTarget Obstacle::transformTarget(const Mechanics * m, const Target & aimPo
 
 	if(!m->isMassive())
 	{
-		for(auto & spellDestination : spellTarget)
+		for(const auto & spellDestination : spellTarget)
 		{
-			for(auto & rangeShape : options.range)
+			for(const auto & rangeShape : options.range)
 			{
 				BattleHex hex = spellDestination.hexValue;
 
@@ -330,7 +312,7 @@ void Obstacle::placeObstacles(ServerCallback * server, const Mechanics * m, cons
 		obstacle.customSize.clear();
 		obstacle.customSize.reserve(options.shape.size());
 
-		for(auto & shape : options.shape)
+		for(const auto & shape : options.shape)
 		{
 			BattleHex hex = destination.hexValue;
 

lib/spells/effects/Obstacle.h

@@ -35,22 +35,14 @@ public:
 	std::string triggerAnimation;
 	std::string animation;
 
-	int offsetY;
-
-	ObstacleSideOptions();
+	int offsetY = 0;
 
 	void serializeJson(JsonSerializeFormat & handler);
-
-private:
-	void serializeRelativeShape(JsonSerializeFormat & handler, const std::string & fieldName, RelativeShape & value);
 };
 
 class Obstacle : public LocationEffect
 {
 public:
-	Obstacle();
-	virtual ~Obstacle();
-
 	void adjustAffectedHexes(std::set<BattleHex> & hexes, const Mechanics * m, const Target & spellTarget) const override;
 
 	bool applicable(Problem & problem, const Mechanics * m) const override;
@@ -64,13 +56,13 @@ protected:
 	void serializeJsonEffect(JsonSerializeFormat & handler) override;
 
 private:
-	bool hidden;
-	bool passable;
-	bool trigger;
-	bool trap;
-	bool removeOnTrigger;
-	int32_t patchCount;//random patches to place, only for massive spells
-	int32_t turnsRemaining;
+	bool hidden = false;
+	bool passable = false;
+	bool trigger = false;
+	bool trap = false;
+	bool removeOnTrigger = false;
+	int32_t patchCount = 1;//random patches to place, only for massive spells
+	int32_t turnsRemaining = -1;
 
 	std::array<ObstacleSideOptions, 2> sideOptions;
 

lib/spells/effects/Registry.cpp

@@ -23,9 +23,6 @@ namespace detail
 class RegistryImpl : public Registry
 {
 public:
-	RegistryImpl() = default;
-	~RegistryImpl() = default;
-
 	const IEffectFactory * find(const std::string & name) const override
 	{
 		auto iter = data.find(name);
@@ -46,10 +43,6 @@ private:
 
 }
 
-Registry::Registry() = default;
-
-Registry::~Registry() = default;
-
 Registry * GlobalRegistry::get()
 {
 	static std::unique_ptr<Registry> Instance = std::make_unique<detail::RegistryImpl>();

lib/spells/effects/Registry.h

@@ -40,8 +40,7 @@ class DLL_LINKAGE Registry
 public:
 	using FactoryPtr = std::shared_ptr<IEffectFactory>;
 
-	Registry();
-	virtual ~Registry();
+	virtual ~Registry() = default; //Required for child classes
 	virtual const IEffectFactory * find(const std::string & name) const = 0;
 	virtual void add(const std::string & name, FactoryPtr item) = 0;
 };

lib/spells/effects/RemoveObstacle.cpp

@@ -31,16 +31,6 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(RemoveObstacle, EFFECT_NAME);
 
-RemoveObstacle::RemoveObstacle()
-	: LocationEffect(),
-	removeAbsolute(false),
-	removeUsual(false),
-	removeAllSpells(false)
-{
-}
-
-RemoveObstacle::~RemoveObstacle() = default;
-
 bool RemoveObstacle::applicable(Problem & problem, const Mechanics * m) const
 {
 	if (getTargets(m, EffectTarget(), true).empty())
@@ -80,7 +70,7 @@ bool RemoveObstacle::canRemove(const CObstacleInstance * obstacle) const
 		return true;
 	if(removeUsual && obstacle->obstacleType == CObstacleInstance::USUAL)
 		return true;
-	auto spellObstacle = dynamic_cast<const SpellCreatedObstacle *>(obstacle);
+	const auto *spellObstacle = dynamic_cast<const SpellCreatedObstacle *>(obstacle);
 
 	if(removeAllSpells && spellObstacle)
 		return true;

lib/spells/effects/RemoveObstacle.h

@@ -27,9 +27,6 @@ namespace effects
 class RemoveObstacle : public LocationEffect
 {
 public:
-	RemoveObstacle();
-	virtual ~RemoveObstacle();
-
 	bool applicable(Problem & problem, const Mechanics * m) const override;
 	bool applicable(Problem & problem, const Mechanics * m, const EffectTarget & target) const override;
 
@@ -39,9 +36,9 @@ protected:
 	void serializeJsonEffect(JsonSerializeFormat & handler) override;
 
 private:
-    bool removeAbsolute;
-    bool removeUsual;
-    bool removeAllSpells;
+    bool removeAbsolute = false;
+    bool removeUsual = false;
+    bool removeAllSpells = false;
 
     std::set<SpellID> removeSpells;
 

lib/spells/effects/Sacrifice.cpp

@@ -31,14 +31,6 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(Sacrifice, EFFECT_NAME);
 
-Sacrifice::Sacrifice()
-	: Heal()
-{
-
-}
-
-Sacrifice::~Sacrifice() = default;
-
 void Sacrifice::adjustTargetTypes(std::vector<TargetType> & types) const
 {
 	if(!types.empty())
@@ -109,7 +101,7 @@ bool Sacrifice::applicable(Problem & problem, const Mechanics * m, const EffectT
 
 	if(target.size() == 2)
 	{
-		auto victim = target.at(1).unitValue;
+		const auto *victim = target.at(1).unitValue;
 		if(!victim)
 			return false;
 		
@@ -160,7 +152,7 @@ EffectTarget Sacrifice::transformTarget(const Mechanics * m, const Target & aimP
 	//add victim
 	if(aimPoint.size() >= 2)
 	{
-		auto victim = aimPoint.at(1).unitValue;
+		const auto *victim = aimPoint.at(1).unitValue;
 		if(victim && getStackFilter(m, false, victim) && isReceptive(m, victim))
 			res.emplace_back(victim);
 	}
@@ -168,7 +160,7 @@ EffectTarget Sacrifice::transformTarget(const Mechanics * m, const Target & aimP
 	return res;
 }
 
-int64_t Sacrifice::calculateHealEffectValue(const Mechanics * m, const battle::Unit * victim) const
+int64_t Sacrifice::calculateHealEffectValue(const Mechanics * m, const battle::Unit * victim) 
 {
 	return (m->getEffectPower() + victim->MaxHealth() + m->calculateRawEffectValue(0, 1)) * victim->getCount();
 }

lib/spells/effects/Sacrifice.h

@@ -22,9 +22,6 @@ namespace effects
 class Sacrifice : public Heal
 {
 public:
-	Sacrifice();
-	virtual ~Sacrifice();
-
 	void adjustTargetTypes(std::vector<TargetType> & types) const override;
 
 	bool applicable(Problem & problem, const Mechanics * m) const override;
@@ -38,7 +35,7 @@ protected:
 	bool isValidTarget(const Mechanics * m, const battle::Unit * unit) const override;
 
 private:
-	int64_t calculateHealEffectValue(const Mechanics * m, const battle::Unit * victim ) const;
+	static int64_t calculateHealEffectValue(const Mechanics * m, const battle::Unit * victim);
 };
 
 }

lib/spells/effects/Summon.cpp

@@ -34,18 +34,6 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(Summon, EFFECT_NAME);
 
-Summon::Summon()
-	: Effect(),
-	creature(),
-	permanent(false),
-	exclusive(true),
-	summonByHealth(false),
-	summonSameUnit(false)
-{
-}
-
-Summon::~Summon() = default;
-
 void Summon::adjustAffectedHexes(std::set<BattleHex> & hexes, const Mechanics * m, const Target & spellTarget) const
 {
 	//no hexes affected
@@ -72,12 +60,12 @@ bool Summon::applicable(Problem & problem, const Mechanics * m) const
 
 		if(!otherSummoned.empty())
 		{
-			auto elemental = otherSummoned.front();
+			const auto *elemental = otherSummoned.front();
 
 			MetaString text;
 			text.addTxt(MetaString::GENERAL_TXT, 538);
 
-			auto caster = dynamic_cast<const CGHeroInstance *>(m->caster);
+			const auto *caster = dynamic_cast<const CGHeroInstance *>(m->caster);
 			if(caster)
 			{
 				text.addReplacement(caster->getNameTranslated());
@@ -105,7 +93,7 @@ void Summon::apply(ServerCallback * server, const Mechanics * m, const EffectTar
 
 	BattleUnitsChanged pack;
 
-	for(auto & dest : target)
+	for(const auto & dest : target)
 	{
 		if(dest.unitValue)
 		{
@@ -122,7 +110,7 @@ void Summon::apply(ServerCallback * server, const Mechanics * m, const EffectTar
 
 			if(summonByHealth)
 			{
-				auto creatureType = creature.toCreature(m->creatures());
+				const auto *creatureType = creature.toCreature(m->creatures());
 				auto creatureMaxHealth = creatureType->getMaxHealth();
 				amount = static_cast<int32_t>(valueWithBonus / creatureMaxHealth);
 			}

lib/spells/effects/Summon.h

@@ -23,9 +23,6 @@ namespace effects
 class Summon : public Effect
 {
 public:
-	Summon();
-	virtual ~Summon();
-
 	void adjustAffectedHexes(std::set<BattleHex> & hexes, const Mechanics * m, const Target & spellTarget) const override;
 	void adjustTargetTypes(std::vector<TargetType> & types) const override;
 
@@ -43,10 +40,10 @@ protected:
 private:
 	CreatureID creature;
 
-	bool permanent;
-	bool exclusive;
-	bool summonByHealth;
-	bool summonSameUnit;
+	bool permanent = false;
+	bool exclusive = true;
+	bool summonByHealth = false;
+	bool summonSameUnit = false;
 };
 
 }

lib/spells/effects/Teleport.cpp

@@ -11,7 +11,6 @@
 
 #include "Teleport.h"
 #include "Registry.h"
-#include "Registry.h"
 #include "../ISpellMechanics.h"
 #include "../../NetPacks.h"
 #include "../../battle/CBattleInfoCallback.h"
@@ -29,12 +28,6 @@ namespace effects
 {
 VCMI_REGISTER_SPELL_EFFECT(Teleport, EFFECT_NAME);
 
-Teleport::Teleport()
-	: UnitEffect()
-{
-}
-
-Teleport::~Teleport() = default;
 
 void Teleport::adjustTargetTypes(std::vector<TargetType> & types) const
 {
@@ -71,7 +64,7 @@ void Teleport::apply(ServerCallback * server, const Mechanics * m, const EffectT
 		return;
 	}
 
-	auto targetUnit = target[0].unitValue;
+	const auto *targetUnit = target[0].unitValue;
 	if(nullptr == targetUnit)
 	{
 		server->complain("No unit to teleport");

lib/spells/effects/Teleport.h

@@ -24,9 +24,6 @@ namespace effects
 class Teleport : public UnitEffect
 {
 public:
-	Teleport();
-	virtual ~Teleport();
-
 	void adjustTargetTypes(std::vector<TargetType> & types) const override;
 
 	bool applicable(Problem & problem, const Mechanics * m) const override;

lib/spells/effects/Timed.cpp

@@ -29,14 +29,73 @@ namespace effects
 
 VCMI_REGISTER_SPELL_EFFECT(Timed, EFFECT_NAME);
 
-Timed::Timed()
-	: UnitEffect(),
-	cumulative(false),
-	bonus()
+static void describeEffect(std::vector<MetaString> & log, const Mechanics * m, const std::vector<Bonus> & bonuses, const battle::Unit * target) 
 {
-}
+	auto addLogLine = [&](const int32_t baseTextID, const boost::logic::tribool & plural)
+	{
+		MetaString line;
+		target->addText(line, MetaString::GENERAL_TXT, baseTextID, plural);
+		target->addNameReplacement(line, plural);
+		log.push_back(std::move(line));
+	};
+
+	if(m->getSpellIndex() == SpellID::DISEASE)
+	{
+		addLogLine(553, boost::logic::indeterminate);
+		return;
+	}
+
+	for(const auto & bonus : bonuses)
+	{
+		switch(bonus.type)
+		{
+		case Bonus::NOT_ACTIVE:
+			{
+				switch(bonus.subtype)
+				{
+				case SpellID::STONE_GAZE:
+					addLogLine(558, boost::logic::indeterminate);
+					return;
+				case SpellID::PARALYZE:
+					addLogLine(563, boost::logic::indeterminate);
+					return;
+				default:
+					break;
+				}
+			}
+			break;
+		case Bonus::POISON:
+			addLogLine(561, boost::logic::indeterminate);
+			return;
+		case Bonus::BIND_EFFECT:
+			addLogLine(-560, true);
+			return;
+		case Bonus::STACK_HEALTH:
+			{
+				if(bonus.val < 0)
+				{
+					BonusList unitHealth = *target->getBonuses(Selector::type()(Bonus::STACK_HEALTH));
 
-Timed::~Timed() = default;
+					auto oldHealth = unitHealth.totalValue();
+					unitHealth.push_back(std::make_shared<Bonus>(bonus));
+					auto newHealth = unitHealth.totalValue();
+
+					//"The %s shrivel with age, and lose %d hit points."
+					MetaString line;
+					target->addText(line, MetaString::GENERAL_TXT, 551);
+					target->addNameReplacement(line);
+
+					line.addReplacement(oldHealth - newHealth);
+					log.push_back(std::move(line));
+					return;
+				}
+			}
+			break;
+		default:
+			break;
+		}
+	}
+}
 
 void Timed::apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const
 {
@@ -50,7 +109,7 @@ void Timed::apply(ServerCallback * server, const Mechanics * m, const EffectTarg
 	std::shared_ptr<const Bonus> addedValueBonus = nullptr;
 	std::shared_ptr<const Bonus> fixedValueBonus = nullptr; 
 
-	auto casterHero = dynamic_cast<const CGHeroInstance *>(m->caster);
+	const auto *casterHero = dynamic_cast<const CGHeroInstance *>(m->caster);
 	if(casterHero)
 	{ 
 		peculiarBonus = casterHero->getBonusLocalFirst(Selector::typeSubtype(Bonus::SPECIAL_PECULIAR_ENCHANT, m->getSpellIndex()));
@@ -62,7 +121,7 @@ void Timed::apply(ServerCallback * server, const Mechanics * m, const EffectTarg
 	SetStackEffect sse;
 	BattleLogMessage blm;
 
-	for(auto & t : target)
+	for(const auto & t : target)
 	{
 		std::vector<Bonus> buffer;
 		std::copy(converted.begin(), converted.end(), std::back_inserter(buffer));
@@ -147,9 +206,9 @@ void Timed::apply(ServerCallback * server, const Mechanics * m, const EffectTarg
 		}
 
 		if(cumulative)
-			sse.toAdd.push_back(std::make_pair(affected->unitId(), buffer));
+			sse.toAdd.emplace_back(affected->unitId(), buffer);
 		else
-			sse.toUpdate.push_back(std::make_pair(affected->unitId(), buffer));
+			sse.toUpdate.emplace_back(affected->unitId(), buffer);
 	}
 
 	if(!(sse.toAdd.empty() && sse.toUpdate.empty()))
@@ -163,7 +222,7 @@ void Timed::convertBonus(const Mechanics * m, int32_t & duration, std::vector<Bo
 {
 	int32_t maxDuration = 0;
 
-	for(const std::shared_ptr<Bonus> & b : bonus)
+	for(const auto & b : bonus)
 	{
 		Bonus nb(*b);
 
@@ -189,74 +248,6 @@ void Timed::convertBonus(const Mechanics * m, int32_t & duration, std::vector<Bo
 	duration = maxDuration;
 }
 
-void Timed::describeEffect(std::vector<MetaString> & log, const Mechanics * m, const std::vector<Bonus> & bonuses, const battle::Unit * target) const
-{
-	auto addLogLine = [&](const int32_t baseTextID, const boost::logic::tribool & plural)
-	{
-		MetaString line;
-		target->addText(line, MetaString::GENERAL_TXT, baseTextID, plural);
-		target->addNameReplacement(line, plural);
-		log.push_back(std::move(line));
-	};
-
-	if(m->getSpellIndex() == SpellID::DISEASE)
-	{
-		addLogLine(553, boost::logic::indeterminate);
-		return;
-	}
-
-	for(const auto & bonus : bonuses)
-	{
-		switch(bonus.type)
-		{
-		case Bonus::NOT_ACTIVE:
-			{
-				switch(bonus.subtype)
-				{
-				case SpellID::STONE_GAZE:
-					addLogLine(558, boost::logic::indeterminate);
-					return;
-				case SpellID::PARALYZE:
-					addLogLine(563, boost::logic::indeterminate);
-					return;
-				default:
-					break;
-				}
-			}
-			break;
-		case Bonus::POISON:
-			addLogLine(561, boost::logic::indeterminate);
-			return;
-		case Bonus::BIND_EFFECT:
-			addLogLine(-560, true);
-			return;
-		case Bonus::STACK_HEALTH:
-			{
-				if(bonus.val < 0)
-				{
-					BonusList unitHealth = *target->getBonuses(Selector::type()(Bonus::STACK_HEALTH));
-
-					auto oldHealth = unitHealth.totalValue();
-					unitHealth.push_back(std::make_shared<Bonus>(bonus));
-					auto newHealth = unitHealth.totalValue();
-
-					//"The %s shrivel with age, and lose %d hit points."
-					MetaString line;
-					target->addText(line, MetaString::GENERAL_TXT, 551);
-					target->addNameReplacement(line);
-
-					line.addReplacement(oldHealth - newHealth);
-					log.push_back(std::move(line));
-					return;
-				}
-			}
-			break;
-		default:
-			break;
-		}
-	}
-}
-
 void Timed::serializeJsonUnitEffect(JsonSerializeFormat & handler)
 {
 	assert(!handler.saving);

lib/spells/effects/Timed.h

@@ -26,12 +26,9 @@ namespace effects
 class Timed : public UnitEffect
 {
 public:
-	bool cumulative;
+	bool cumulative = false;
 	std::vector<std::shared_ptr<Bonus>> bonus;
 
-	Timed();
-	virtual ~Timed();
-
 	void apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const override;
 
 protected:
@@ -39,8 +36,6 @@ protected:
 
 private:
 	void convertBonus(const Mechanics * m, int32_t & duration, std::vector<Bonus> & converted) const;
-	void describeEffect(std::vector<MetaString> & log, const Mechanics * m, const std::vector<Bonus> & bonuses, const battle::Unit * target) const;
-
 };
 
 }

lib/spells/effects/UnitEffect.cpp

@@ -25,16 +25,6 @@ namespace spells
 namespace effects
 {
 
-UnitEffect::UnitEffect()
-	: Effect(),
-	chainLength(0),
-	chainFactor(0.0),
-	ignoreImmunity(false)
-{
-}
-
-UnitEffect::~UnitEffect() = default;
-
 void UnitEffect::adjustTargetTypes(std::vector<TargetType> & types) const
 {
 
@@ -42,7 +32,7 @@ void UnitEffect::adjustTargetTypes(std::vector<TargetType> & types) const
 
 void UnitEffect::adjustAffectedHexes(std::set<BattleHex> & hexes, const Mechanics * m, const Target & spellTarget) const
 {
-	for(auto & destnation : spellTarget)
+	for(const auto & destnation : spellTarget)
 		hexes.insert(destnation.hexValue);
 }
 
@@ -72,7 +62,7 @@ bool UnitEffect::applicable(Problem & problem, const Mechanics * m, const Effect
 	//stack effect is applicable if it affects at least one smart target
 	//assume target correctly transformed, just reapply smart filter
 
-	for(auto & item : target)
+	for(const auto & item : target)
 		if(item.unitValue)
 			if(getStackFilter(m, true, item.unitValue))
 				return true;
@@ -127,7 +117,7 @@ EffectTarget UnitEffect::transformTargetByRange(const Mechanics * m, const Targe
 	{
 		//ignore spellTarget and add all stacks
 		auto units = m->battle()->battleGetUnitsIf(mainFilter);
-		for(auto unit : units)
+		for(const auto *unit : units)
 			targets.insert(unit);
 	}
 	else
@@ -152,7 +142,7 @@ EffectTarget UnitEffect::transformTargetByRange(const Mechanics * m, const Targe
 
 				auto units = m->battle()->battleGetUnitsIf(predicate);
 
-				for(auto unit : units)
+				for(const auto *unit : units)
 				{
 					if(unit->alive())
 					{
@@ -188,7 +178,7 @@ EffectTarget UnitEffect::transformTargetByRange(const Mechanics * m, const Targe
 
 	EffectTarget effectTarget;
 
-	for(auto s : targets)
+	for(const auto *s : targets)
 		effectTarget.push_back(Destination(s));
 
 	return effectTarget;
@@ -217,7 +207,7 @@ EffectTarget UnitEffect::transformTargetByChain(const Mechanics * m, const Targe
 		return isValidTarget(m, unit);
 	});
 
-	for(auto unit : possibleTargets)
+	for(const auto *unit : possibleTargets)
 	{
 		for(auto hex : battle::Unit::getHexes(unit->getPosition(), unit->doubleWide(), unit->unitSide()))
 			possibleHexes.insert(hex);
@@ -228,7 +218,7 @@ EffectTarget UnitEffect::transformTargetByChain(const Mechanics * m, const Targe
 
 	for(int32_t targetIndex = 0; targetIndex < chainLength; ++targetIndex)
 	{
-		auto unit = m->battle()->battleGetUnitByPos(destHex, true);
+		const auto *unit = m->battle()->battleGetUnitByPos(destHex, true);
 
 		if(!unit)
 			break;

lib/spells/effects/UnitEffect.h

@@ -22,9 +22,6 @@ namespace effects
 class UnitEffect : public Effect
 {
 public:
-	UnitEffect();
-	virtual ~UnitEffect();
-
 	void adjustTargetTypes(std::vector<TargetType> & types) const override;
 
 	void adjustAffectedHexes(std::set<BattleHex> & hexes, const Mechanics * m, const Target & spellTarget) const override;
@@ -41,8 +38,8 @@ public:
     virtual bool eraseByImmunityFilter(const Mechanics * m, const battle::Unit * s) const;
 
 protected:
-	int32_t chainLength;
-	double chainFactor;
+	int32_t chainLength = 0;
+	double chainFactor = 0.0;
 
 	virtual bool isReceptive(const Mechanics * m, const battle::Unit * unit) const;
 	virtual bool isSmartTarget(const Mechanics * m, const battle::Unit * unit, bool alwaysSmart) const;
@@ -52,7 +49,7 @@ protected:
 	virtual void serializeJsonUnitEffect(JsonSerializeFormat & handler) = 0;
 
 private:
-	bool ignoreImmunity;
+	bool ignoreImmunity = false;
 
 	EffectTarget transformTargetByRange(const Mechanics * m, const Target & aimPoint, const Target & spellTarget) const;
 	EffectTarget transformTargetByChain(const Mechanics * m, const Target & aimPoint, const Target & spellTarget) const;