Fix memory problems with BonusList

Bonus * -> std::shared_ptr<Bonus>

This cures the following problems:

1) Memory corruption at exit. Some Bonus-es were deleted twice (mods?).
2) Memory leaks. Some Bonuses were not deleted.
3) Reduce the number of "Orphaned child" messages.

Valgrind reports 0 leaked memory now and no invalid reads/writes.

AI/BattleAI/BattleAI.cpp

@@ -665,8 +665,9 @@ const TBonusListPtr StackWithBonuses::getAllBonuses(const CSelector &selector, c
 	range::copy(*originalList, std::back_inserter(*ret));
 	for(auto &bonus : bonusesToAdd)
 	{
-		if(selector(&bonus)  &&  (!limit || !limit(&bonus)))
+		auto b = std::make_shared<Bonus>(bonus);
-			ret->push_back(&bonus);
+		if(selector(b.get())  &&  (!limit || !limit(b.get())))
+			ret->push_back(b);
 	}
 
 	//TODO limiters?

client/battle/CBattleInterface.cpp

@@ -1579,8 +1579,8 @@ void CBattleInterface::activateStack()
 	redrawBackgroundWithHexes(activeStack);
 
 	//set casting flag to true if creature can use it to not check it every time
-	const Bonus *spellcaster = s->getBonusLocalFirst(Selector::type(Bonus::SPELLCASTER)),
+	const auto spellcaster = s->getBonusLocalFirst(Selector::type(Bonus::SPELLCASTER)),
-		*randomSpellcaster = s->getBonusLocalFirst(Selector::type(Bonus::RANDOM_SPELLCASTER));
+		randomSpellcaster = s->getBonusLocalFirst(Selector::type(Bonus::RANDOM_SPELLCASTER));
 	if (s->casts &&  (spellcaster || randomSpellcaster))
 	{
 		stackCanCastSpell = true;

client/widgets/MiscWidgets.cpp

@@ -394,7 +394,7 @@ void MoraleLuckBox::set(const IBonusBearer *node)
 		text += CGI->generaltexth->arraytxt[noneTxtId];//no modifiers
 	else
 	{
-		for(const Bonus * elem : *modifierList)
+		for(auto& elem : *modifierList)
 		{
 			if(elem->val != 0)
 				//no bonuses with value 0

client/windows/CCreatureWindow.cpp

@@ -447,7 +447,7 @@ CIntObject * CStackWindow::createSkillEntry(int index)
 	{
 		if (index == 0 && skillID >= 100)
 		{
-			const Bonus *bonus = CGI->creh->skillRequirements[skillID-100].first;
+			const auto bonus = CGI->creh->skillRequirements[skillID-100].first;
 			const CStackInstance *stack = info->commander;
 			CClickableObject * icon = new CClickableObject(new CPicture(stack->bonusToGraphics(bonus)), []{});
 			icon->callback = [=]
@@ -752,15 +752,14 @@ void CStackWindow::initBonusesList()
 
 	while (!input.empty())
 	{
-		Bonus * b = input.front();
+		auto b = input.front();
-
+		output.push_back(std::make_shared<Bonus>(*b));
-		output.push_back(new Bonus(*b));
 		output.back()->val = input.valOfBonuses(Selector::typeSubtype(b->type, b->subtype)); //merge multiple bonuses into one
 		input.remove_if (Selector::typeSubtype(b->type, b->subtype)); //remove used bonuses
 	}
 
 	BonusInfo bonusInfo;
-	for(Bonus* b : output)
+	for(auto b : output)
 	{
 		bonusInfo.name = info->stackNode->bonusToString(b, false);
 		bonusInfo.description = info->stackNode->bonusToString(b, true);
@@ -778,12 +777,12 @@ void CStackWindow::initBonusesList()
 	if (magicResistance)
 	{
 		BonusInfo bonusInfo;
-		Bonus b;
+		auto b = std::make_shared<Bonus>();
-		b.type = Bonus::MAGIC_RESISTANCE;
+		b->type = Bonus::MAGIC_RESISTANCE;
 
-		bonusInfo.name = VLC->getBth()->bonusToString(&b, info->stackNode, false);
+		bonusInfo.name = VLC->getBth()->bonusToString(b, info->stackNode, false);
-		bonusInfo.description = VLC->getBth()->bonusToString(&b, info->stackNode, true);
+		bonusInfo.description = VLC->getBth()->bonusToString(b, info->stackNode, true);
-		bonusInfo.imagePath = info->stackNode->bonusToGraphics(&b);
+		bonusInfo.imagePath = info->stackNode->bonusToGraphics(b);
 		activeBonuses.push_back(bonusInfo);
 	}
 }

client/windows/CHeroWindow.cpp

@@ -55,9 +55,9 @@ const TBonusListPtr CHeroWithMaybePickedArtifact::getAllBonuses(const CSelector
 	else
 		bonusesFromPickedUpArtifact = TBonusListPtr(new BonusList);
 
-	for(Bonus *b : *bonusesFromPickedUpArtifact)
+	for(auto b : *bonusesFromPickedUpArtifact)
 		*heroBonuses -= b;
-	for(Bonus *b : *heroBonuses)
+	for(auto b : *heroBonuses)
 		out->push_back(b);
 	return out;
 }

client/windows/CSpellWindow.cpp

@@ -678,7 +678,7 @@ void CSpellWindow::SpellArea::clickLeft(tribool down, bool previousState)
 			case ESpellCastProblem::SPELL_LEVEL_LIMIT_EXCEEDED:
 				{
 					//Recanter's Cloak or similar effect. Try to retrieve bonus
-					const Bonus *b = owner->myHero->getBonusLocalFirst(Selector::type(Bonus::BLOCK_MAGIC_ABOVE));
+					const auto b = owner->myHero->getBonusLocalFirst(Selector::type(Bonus::BLOCK_MAGIC_ABOVE));
 					//TODO what about other values and non-artifact sources?
 					if(b && b->val == 2 && b->source == Bonus::ARTIFACT)
 					{

lib/BattleState.cpp

@@ -539,7 +539,7 @@ BattleInfo * BattleInfo::setupBattle( int3 tile, ETerrainType terrain, BFieldTyp
 	std::stable_sort(stacks.begin(),stacks.end(),cmpst);
 
 	//spell level limiting bonus
-	curB->addNewBonus(new Bonus(Bonus::ONE_BATTLE, Bonus::LEVEL_SPELL_IMMUNITY, Bonus::OTHER,
+	curB->addNewBonus(std::make_shared<Bonus>(Bonus::ONE_BATTLE, Bonus::LEVEL_SPELL_IMMUNITY, Bonus::OTHER,
 		0, -1, -1, Bonus::INDEPENDENT_MAX));
 
 	//giving terrain overalay premies
@@ -568,7 +568,7 @@ BattleInfo * BattleInfo::setupBattle( int3 tile, ETerrainType terrain, BFieldTyp
 		}
 
 		{ //common part for cases 9, 14, 15, 16, 17
-			curB->addNewBonus(new Bonus(Bonus::ONE_BATTLE, Bonus::MAGIC_SCHOOL_SKILL, Bonus::TERRAIN_OVERLAY, 3, -1, "", bonusSubtype));
+			curB->addNewBonus(std::make_shared<Bonus>(Bonus::ONE_BATTLE, Bonus::MAGIC_SCHOOL_SKILL, Bonus::TERRAIN_OVERLAY, 3, -1, "", bonusSubtype));
 			break;
 		}
 
@@ -593,7 +593,7 @@ BattleInfo * BattleInfo::setupBattle( int3 tile, ETerrainType terrain, BFieldTyp
 		{
 			curB->addNewBonus(makeFeature(Bonus::NO_MORALE, Bonus::ONE_BATTLE, 0, 0, Bonus::TERRAIN_OVERLAY));
 			curB->addNewBonus(makeFeature(Bonus::NO_LUCK, Bonus::ONE_BATTLE, 0, 0, Bonus::TERRAIN_OVERLAY));
-			Bonus * b = makeFeature(Bonus::LEVEL_SPELL_IMMUNITY, Bonus::ONE_BATTLE, GameConstants::SPELL_LEVELS, 1, Bonus::TERRAIN_OVERLAY);
+			auto b = makeFeature(Bonus::LEVEL_SPELL_IMMUNITY, Bonus::ONE_BATTLE, GameConstants::SPELL_LEVELS, 1, Bonus::TERRAIN_OVERLAY);
 			b->valType = Bonus::INDEPENDENT_MAX;
 			curB->addNewBonus(b);
 			break;
@@ -635,11 +635,11 @@ BattleInfo * BattleInfo::setupBattle( int3 tile, ETerrainType terrain, BFieldTyp
 		curB->battleGetArmyObject(i)->getRedAncestors(nodes);
 		for(CBonusSystemNode *n : nodes)
 		{
-			for(Bonus *b : n->getExportedBonusList())
+			for(auto b : n->getExportedBonusList())
 			{
 				if(b->effectRange == Bonus::ONLY_ENEMY_ARMY/* && b->propagator && b->propagator->shouldBeAttached(curB)*/)
 				{
-					auto bCopy = new Bonus(*b);
+					auto bCopy = std::make_shared<Bonus>(*b);
 					bCopy->effectRange = Bonus::NO_LIMIT;
 					bCopy->propagator.reset();
 					bCopy->limiter.reset(new StackOwnerLimiter(curB->sides[!i].color));
@@ -979,7 +979,7 @@ std::vector<si32> CStack::activeSpells() const
 	std::vector<si32> ret;
 
 	TBonusListPtr spellEffects = getSpellBonuses();
-	for(const Bonus *it : *spellEffects)
+	for(const std::shared_ptr<Bonus> it : *spellEffects)
 	{
 		if (!vstd::contains(ret, it->sid)) //do not duplicate spells with multiple effects
 			ret.push_back(it->sid);

lib/CArtHandler.cpp

@@ -108,7 +108,7 @@ std::string CArtifact::nodeName() const
 	return "Artifact: " + Name();
 }
 
-void CArtifact::addNewBonus(Bonus *b)
+void CArtifact::addNewBonus(const std::shared_ptr<Bonus>& b)
 {
 	b->source = Bonus::ARTIFACT;
 	b->duration = Bonus::PERMANENT;
@@ -118,24 +118,24 @@ void CArtifact::addNewBonus(Bonus *b)
 
 void CGrowingArtifact::levelUpArtifact (CArtifactInstance * art)
 {
-	Bonus b;
+	auto b = std::make_shared<Bonus>();
-	b.type = Bonus::LEVEL_COUNTER;
+	b->type = Bonus::LEVEL_COUNTER;
-	b.val = 1;
+	b->val = 1;
-	b.duration = Bonus::COMMANDER_KILLED;
+	b->duration = Bonus::COMMANDER_KILLED;
-	art->accumulateBonus (b);
+	art->accumulateBonus(b);
 
 	for (auto bonus : bonusesPerLevel)
 	{
 		if (art->valOfBonuses(Bonus::LEVEL_COUNTER) % bonus.first == 0) //every n levels
 		{
-			art->accumulateBonus (bonus.second);
+			art->accumulateBonus(std::make_shared<Bonus>(bonus.second));
 		}
 	}
 	for (auto bonus : thresholdBonuses)
 	{
 		if (art->valOfBonuses(Bonus::LEVEL_COUNTER) == bonus.first) //every n levels
 		{
-			art->addNewBonus (&bonus.second);
+			art->addNewBonus(std::make_shared<Bonus>(bonus.second));
 		}
 	}
 }
@@ -303,7 +303,7 @@ CArtifact * CArtHandler::loadFromJson(const JsonNode & node, const std::string &
 
 	for (auto b : node["bonuses"].Vector())
 	{
-		auto bonus = JsonUtils::parseBonus (b);
+		auto bonus = JsonUtils::parseBonus(b);
 		art->addNewBonus(bonus);
 	}
 	return art;
@@ -439,11 +439,13 @@ void CArtHandler::loadGrowingArt(CGrowingArtifact * art, const JsonNode & node)
 {
 	for (auto b : node["growing"]["bonusesPerLevel"].Vector())
 	{
-		art->bonusesPerLevel.push_back (std::pair <ui16, Bonus> (b["level"].Float(), *JsonUtils::parseBonus (b["bonus"])));
+		auto bonus = JsonUtils::parseBonus (b["bonus"]);
+		art->bonusesPerLevel.push_back (std::pair <ui16, Bonus> (b["level"].Float(), *bonus));
 	}
 	for (auto b : node["growing"]["thresholdBonuses"].Vector())
 	{
-		art->thresholdBonuses.push_back (std::pair <ui16, Bonus> (b["level"].Float(), *JsonUtils::parseBonus (b["bonus"])));
+		auto bonus = JsonUtils::parseBonus (b["bonus"]);
+		art->thresholdBonuses.push_back (std::pair <ui16, Bonus> (b["level"].Float(), *bonus));
 	}
 }
 
@@ -546,18 +548,18 @@ ArtifactID CArtHandler::pickRandomArtifact(CRandomGenerator & rand, int flags)
 	return pickRandomArtifact(rand, flags, [](ArtifactID){ return true;});
 }
 
-Bonus *createBonus(Bonus::BonusType type, int val, int subtype, Bonus::ValueType valType, std::shared_ptr<ILimiter> limiter = std::shared_ptr<ILimiter>(), int additionalInfo = 0)
+std::shared_ptr<Bonus> createBonus(Bonus::BonusType type, int val, int subtype, Bonus::ValueType valType, std::shared_ptr<ILimiter> limiter = std::shared_ptr<ILimiter>(), int additionalInfo = 0)
 {
-	auto added = new Bonus(Bonus::PERMANENT,type,Bonus::ARTIFACT,val,-1,subtype);
+	auto added = std::make_shared<Bonus>(Bonus::PERMANENT,type,Bonus::ARTIFACT,val,-1,subtype);
 	added->additionalInfo = additionalInfo;
 	added->valType = valType;
 	added->limiter = limiter;
 	return added;
 }
 
-Bonus *createBonus(Bonus::BonusType type, int val, int subtype, std::shared_ptr<IPropagator> propagator = std::shared_ptr<IPropagator>(), int additionalInfo = 0)
+std::shared_ptr<Bonus> createBonus(Bonus::BonusType type, int val, int subtype, std::shared_ptr<IPropagator> propagator = std::shared_ptr<IPropagator>(), int additionalInfo = 0)
 {
-	auto added = new Bonus(Bonus::PERMANENT,type,Bonus::ARTIFACT,val,-1,subtype);
+	auto added = std::make_shared<Bonus>(Bonus::PERMANENT,type,Bonus::ARTIFACT,val,-1,subtype);
 	added->additionalInfo = additionalInfo;
 	added->valType = Bonus::BASE_NUMBER;
 	added->propagator = propagator;
@@ -574,7 +576,7 @@ void CArtHandler::giveArtBonus(ArtifactID aid, Bonus::BonusType type, int val, i
 	giveArtBonus(aid, createBonus(type, val, subtype, propagator, additionalInfo));
 }
 
-void CArtHandler::giveArtBonus(ArtifactID aid, Bonus *bonus)
+void CArtHandler::giveArtBonus(ArtifactID aid, std::shared_ptr<Bonus> bonus)
 {
 	bonus->sid = aid;
 	if(bonus->subtype == Bonus::MORALE || bonus->type == Bonus::LUCK)
@@ -785,7 +787,7 @@ CArtifactInstance * CArtifactInstance::createScroll( const CSpell *s)
 CArtifactInstance *CArtifactInstance::createScroll(SpellID sid)
 {
 	auto ret = new CArtifactInstance(VLC->arth->artifacts[ArtifactID::SPELL_SCROLL]);
-	auto b = new Bonus(Bonus::PERMANENT, Bonus::SPELL, Bonus::ARTIFACT_INSTANCE, -1, ArtifactID::SPELL_SCROLL, sid);
+	auto b = std::make_shared<Bonus>(Bonus::PERMANENT, Bonus::SPELL, Bonus::ARTIFACT_INSTANCE, -1, ArtifactID::SPELL_SCROLL, sid);
 	ret->addNewBonus(b);
 	return ret;
 }
@@ -957,7 +959,7 @@ CArtifactInstance * CArtifactInstance::createNewArtifactInstance(CArtifact *Art)
 		auto  ret = new CArtifactInstance(Art);
 		if (dynamic_cast<CGrowingArtifact *>(Art))
 		{
-			auto  bonus = new Bonus;
+			auto bonus = std::make_shared<Bonus>();
 			bonus->type = Bonus::LEVEL_COUNTER;
 			bonus->val = 0;
 			ret->addNewBonus (bonus);
@@ -1018,7 +1020,7 @@ void CArtifactInstance::deserializationFix()
 
 SpellID CArtifactInstance::getGivenSpellID() const
 {
-	const Bonus * b = getBonusLocalFirst(Selector::type(Bonus::SPELL));
+	const auto b = getBonusLocalFirst(Selector::type(Bonus::SPELL));
 	if(!b)
 	{
 		logGlobal->warnStream() << "Warning: " << nodeName() << " doesn't bear any spell!";

lib/CArtHandler.h

@@ -65,7 +65,7 @@ public:
 
 	int getArtClassSerial() const; //0 - treasure, 1 - minor, 2 - major, 3 - relic, 4 - spell scroll, 5 - other
 	std::string nodeName() const override;
-	void addNewBonus(Bonus *b) override;
+	void addNewBonus(const std::shared_ptr<Bonus>& b) override;
 
 	virtual void levelUpArtifact (CArtifactInstance * art){};
 
@@ -280,7 +280,7 @@ private:
 
 	void giveArtBonus(ArtifactID aid, Bonus::BonusType type, int val, int subtype = -1, Bonus::ValueType valType = Bonus::BASE_NUMBER, std::shared_ptr<ILimiter> limiter = std::shared_ptr<ILimiter>(), int additionalinfo = 0);
 	void giveArtBonus(ArtifactID aid, Bonus::BonusType type, int val, int subtype, std::shared_ptr<IPropagator> propagator, int additionalinfo = 0);
-	void giveArtBonus(ArtifactID aid, Bonus *bonus);
+	void giveArtBonus(ArtifactID aid, std::shared_ptr<Bonus> bonus);
 
 	void erasePickedArt(ArtifactID id);
 };

lib/CBattleCallback.cpp

@@ -908,7 +908,7 @@ TDmgRange CBattleInfoCallback::calculateDmgRange(const BattleAttackInfo &info) c
 	{ //minDmg and maxDmg are multiplied by hero attack + 1
 		auto retreiveHeroPrimSkill = [&](int skill) -> int
 		{
-			const Bonus *b = info.attackerBonuses->getBonus(Selector::sourceTypeSel(Bonus::HERO_BASE_SKILL).And(Selector::typeSubtype(Bonus::PRIMARY_SKILL, skill)));
+			const std::shared_ptr<Bonus> b = info.attackerBonuses->getBonus(Selector::sourceTypeSel(Bonus::HERO_BASE_SKILL).And(Selector::typeSubtype(Bonus::PRIMARY_SKILL, skill)));
 			return b ? b->val : 0; //if there is no hero or no info on his primary skill, return 0
 		};
 
@@ -925,14 +925,14 @@ TDmgRange CBattleInfoCallback::calculateDmgRange(const BattleAttackInfo &info) c
 	double multDefenceReduction = (100 - battleBonusValue (info.attackerBonuses, Selector::type(Bonus::ENEMY_DEFENCE_REDUCTION))) / 100.0;
 	attackDefenceDifference -= info.defenderBonuses->Defense() * multDefenceReduction;
 
-	if(const Bonus *slayerEffect = info.attackerBonuses->getEffect(SpellID::SLAYER)) //slayer handling //TODO: apply only ONLY_MELEE_FIGHT / DISTANCE_FIGHT?
+	if(const std::shared_ptr<Bonus> slayerEffect = info.attackerBonuses->getEffect(SpellID::SLAYER)) //slayer handling //TODO: apply only ONLY_MELEE_FIGHT / DISTANCE_FIGHT?
 	{
 		std::vector<int> affectedIds;
 		int spLevel = slayerEffect->val;
 
 		for(int g = 0; g < VLC->creh->creatures.size(); ++g)
 		{
-			for(const Bonus *b : VLC->creh->creatures[g]->getBonusList())
+			for(const std::shared_ptr<Bonus> b : VLC->creh->creatures[g]->getBonusList())
 			{
 				if ( (b->type == Bonus::KING3 && spLevel >= 3) || //expert
 					(b->type == Bonus::KING2 && spLevel >= 2) || //adv +
@@ -1019,14 +1019,14 @@ TDmgRange CBattleInfoCallback::calculateDmgRange(const BattleAttackInfo &info) c
 	TBonusListPtr curseEffects = info.attackerBonuses->getBonuses(Selector::type(Bonus::ALWAYS_MINIMUM_DAMAGE));
 	TBonusListPtr blessEffects = info.attackerBonuses->getBonuses(Selector::type(Bonus::ALWAYS_MAXIMUM_DAMAGE));
 	int curseBlessAdditiveModifier = blessEffects->totalValue() - curseEffects->totalValue();
-	double curseMultiplicativePenalty = curseEffects->size() ? (*std::max_element(curseEffects->begin(), curseEffects->end(), &Bonus::compareByAdditionalInfo))->additionalInfo : 0;
+	double curseMultiplicativePenalty = curseEffects->size() ? (*std::max_element(curseEffects->begin(), curseEffects->end(), &Bonus::compareByAdditionalInfo<std::shared_ptr<Bonus>>))->additionalInfo : 0;
 
 	if(curseMultiplicativePenalty) //curse handling (partial, the rest is below)
 	{
 		multBonus *= 1.0 - curseMultiplicativePenalty/100;
 	}
 
-	auto isAdvancedAirShield = [](const Bonus *bonus)
+	auto isAdvancedAirShield = [](const Bonus* bonus)
 	{
 		return bonus->source == Bonus::SPELL_EFFECT
 			&& bonus->sid == SpellID::AIR_SHIELD
@@ -1920,12 +1920,12 @@ SpellID CBattleInfoCallback::getRandomCastedSpell(CRandomGenerator & rand,const
 	if (!bl->size())
 		return SpellID::NONE;
 	int totalWeight = 0;
-	for(Bonus * b : *bl)
+	for(auto b : *bl)
 	{
 		totalWeight += std::max(b->additionalInfo, 1); //minimal chance to cast is 1
 	}
 	int randomPos = rand.nextInt(totalWeight - 1);
-	for(Bonus * b : *bl)
+	for(auto b : *bl)
 	{
 		randomPos -= std::max(b->additionalInfo, 1);
 		if(randomPos < 0)

lib/CBonusTypeHandler.cpp

@@ -126,7 +126,7 @@ CBonusTypeHandler::~CBonusTypeHandler()
 	//dtor
 }
 
-std::string CBonusTypeHandler::bonusToString(const Bonus *bonus, const IBonusBearer *bearer, bool description) const
+std::string CBonusTypeHandler::bonusToString(const std::shared_ptr<Bonus>& bonus, const IBonusBearer *bearer, bool description) const
 {	
 	auto getValue = [=](const std::string &name) -> std::string
 	{
@@ -163,7 +163,7 @@ std::string CBonusTypeHandler::bonusToString(const Bonus *bonus, const IBonusBea
 	return macro.build(getValue);	
 }
 
-std::string CBonusTypeHandler::bonusToGraphics(const Bonus* bonus) const
+std::string CBonusTypeHandler::bonusToGraphics(const std::shared_ptr<Bonus>& bonus) const
 {
 	std::string fileName;
 	bool fullPath = false;

lib/CBonusTypeHandler.h

@@ -75,8 +75,8 @@ public:
 	CBonusTypeHandler();
 	virtual ~CBonusTypeHandler();
 
-	std::string bonusToString(const Bonus *bonus, const IBonusBearer *bearer, bool description) const override;
+	std::string bonusToString(const std::shared_ptr<Bonus>& bonus, const IBonusBearer *bearer, bool description) const override;
-	std::string bonusToGraphics(const Bonus *bonus) const override;
+	std::string bonusToGraphics(const std::shared_ptr<Bonus>& bonus) const override;
 
 	void load();
 	void load(const JsonNode& config);

lib/CCreatureHandler.cpp

@@ -107,7 +107,7 @@ CCreature::CCreature()
 }
 void CCreature::addBonus(int val, Bonus::BonusType type, int subtype /*= -1*/)
 {
-	auto added = new Bonus(Bonus::PERMANENT, type, Bonus::CREATURE_ABILITY, val, idNumber, subtype, Bonus::BASE_NUMBER);
+	auto added = std::make_shared<Bonus>(Bonus::PERMANENT, type, Bonus::CREATURE_ABILITY, val, idNumber, subtype, Bonus::BASE_NUMBER);
 	addNewBonus(added);
 }
 
@@ -145,7 +145,7 @@ void CCreature::setId(CreatureID ID)
 
 static void AddAbility(CCreature *cre, const JsonVector &ability_vec)
 {
-	auto nsf = new Bonus();
+	auto nsf = std::make_shared<Bonus>();
 	std::string type = ability_vec[0].String();
 
 	auto it = bonusNameMap.find(type);
@@ -208,7 +208,7 @@ void CCreatureHandler::loadCommanders()
 
 	for (auto bonus : config["bonusPerLevel"].Vector())
 	{
-		commanderLevelPremy.push_back(JsonUtils::parseBonus (bonus.Vector()));
+		commanderLevelPremy.push_back(JsonUtils::parseBonus(bonus.Vector()));
 	}
 
 	int i = 0;
@@ -224,7 +224,7 @@ void CCreatureHandler::loadCommanders()
 
 	for (auto ability : config["abilityRequirements"].Vector())
 	{
-		std::pair <Bonus*, std::pair <ui8, ui8> > a;
+		std::pair <std::shared_ptr<Bonus>, std::pair <ui8, ui8> > a;
 		a.first = JsonUtils::parseBonus (ability["ability"].Vector());
 		a.second.first = ability["skills"].Vector()[0].Float();
 		a.second.second = ability["skills"].Vector()[1].Float();
@@ -470,7 +470,7 @@ void CCreatureHandler::loadCrExpBon()
 
 		parser.readString(); //ignore index
 		loadStackExp(b, bl, parser);
-		for(Bonus * b : bl)
+		for(auto b : bl)
 			addBonusForAllCreatures(b); //health bonus is common for all
 		parser.endLine();
 
@@ -481,7 +481,7 @@ void CCreatureHandler::loadCrExpBon()
 				parser.readString(); //ignore index
 				bl.clear();
 				loadStackExp(b, bl, parser);
-				for(Bonus * b : bl)
+				for(auto b : bl)
 					addBonusForTier(i, b);
 				parser.endLine();
 			}
@@ -491,7 +491,7 @@ void CCreatureHandler::loadCrExpBon()
 			parser.readString(); //ignore index
 			bl.clear();
 			loadStackExp(b, bl, parser);
-			for(Bonus * b : bl)
+			for(auto b : bl)
 			{
 				addBonusForTier(7, b);
 				creaturesOfLevel[0].addNewBonus(b); //bonuses from level 7 are given to high-level creatures
@@ -504,7 +504,7 @@ void CCreatureHandler::loadCrExpBon()
 			b.sid = sid;
 			bl.clear();
 			loadStackExp(b, bl, parser);
-			for(Bonus * b : bl)
+			for(auto b : bl)
 			{
 				creatures[sid]->addNewBonus(b); //add directly to CCreature Node
 			}
@@ -772,7 +772,7 @@ void CCreatureHandler::loadStackExperience(CCreature * creature, const JsonNode
 				if (val.Bool() == true)
 				{
 					bonus->limiter = std::make_shared<RankRangeLimiter>(RankRangeLimiter(lowerLimit));
-					creature->addNewBonus (new Bonus(*bonus)); //bonuses must be unique objects
+					creature->addNewBonus (std::make_shared<Bonus>(*bonus)); //bonuses must be unique objects
 					break; //TODO: allow bonuses to turn off?
 				}
 				++lowerLimit;
@@ -787,13 +787,12 @@ void CCreatureHandler::loadStackExperience(CCreature * creature, const JsonNode
 				{
 					bonus->val = val.Float() - lastVal;
 					bonus->limiter.reset (new RankRangeLimiter(lowerLimit));
-					creature->addNewBonus (new Bonus(*bonus));
+					creature->addNewBonus (std::make_shared<Bonus>(*bonus));
 				}
 				lastVal = val.Float();
 				++lowerLimit;
 			}
 		}
-		delete bonus;
 	}
 }
 
@@ -1073,7 +1072,7 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 			if (curVal == 1)
 			{
 				b.limiter.reset (new RankRangeLimiter(i));
-				bl.push_back(new Bonus(b));
+				bl.push_back(std::make_shared<Bonus>(b));
 				break; //never turned off it seems
 			}
 		}
@@ -1094,7 +1093,7 @@ void CCreatureHandler::loadStackExp(Bonus & b, BonusList & bl, CLegacyConfigPars
 				b.val = curVal - lastVal;
 				lastVal = curVal;
 				b.limiter.reset (new RankRangeLimiter(i));
-				bl.push_back(new Bonus(b));
+				bl.push_back(std::make_shared<Bonus>(b));
 				lastLev = i; //start new range from here, i = previous rank
 			}
 			else if (curVal < lastVal)
@@ -1118,7 +1117,7 @@ CCreatureHandler::~CCreatureHandler()
 		creature.dellNull();
 
 	for(auto & p : skillRequirements)
-		vstd::clear_pointer(p.first);
+		p.first = nullptr;
 }
 
 CreatureID CCreatureHandler::pickRandomMonster(CRandomGenerator & rand, int tier) const
@@ -1155,13 +1154,13 @@ CreatureID CCreatureHandler::pickRandomMonster(CRandomGenerator & rand, int tier
 	return CreatureID(r);
 }
 
-void CCreatureHandler::addBonusForTier(int tier, Bonus *b)
+void CCreatureHandler::addBonusForTier(int tier, std::shared_ptr<Bonus> b)
 {
 	assert(vstd::iswithin(tier, 1, 7));
 	creaturesOfLevel[tier].addNewBonus(b);
 }
 
-void CCreatureHandler::addBonusForAllCreatures(Bonus *b)
+void CCreatureHandler::addBonusForAllCreatures(std::shared_ptr<Bonus> b)
 {
 	allCreatures.addNewBonus(b);
 }

lib/CCreatureHandler.h

@@ -190,14 +190,14 @@ public:
 	//Commanders
 	BonusList commanderLevelPremy; //bonus values added with each level-up
 	std::vector< std::vector <ui8> > skillLevels; //how much of a bonus will be given to commander with every level. SPELL_POWER also gives CASTS and RESISTANCE
-	std::vector <std::pair <Bonus*, std::pair <ui8, ui8> > > skillRequirements; // first - Bonus, second - which two skills are needed to use it
+	std::vector <std::pair <std::shared_ptr<Bonus>, std::pair <ui8, ui8> > > skillRequirements; // first - Bonus, second - which two skills are needed to use it
 
 	const CCreature * getCreature(const std::string & scope, const std::string & identifier) const;
 
 	void deserializationFix();
 	CreatureID pickRandomMonster(CRandomGenerator & rand, int tier = -1) const; //tier <1 - CREATURES_PER_TOWN> or -1 for any
-	void addBonusForTier(int tier, Bonus *b); //tier must be <1-7>
+	void addBonusForTier(int tier, std::shared_ptr<Bonus> b); //tier must be <1-7>
-	void addBonusForAllCreatures(Bonus *b);
+	void addBonusForAllCreatures(std::shared_ptr<Bonus> b);
 
 	CCreatureHandler();
 	~CCreatureHandler();

lib/CCreatureSet.cpp

@@ -628,7 +628,7 @@ void CStackInstance::setType(const CCreature *c)
 	if(type)
 		attachTo(const_cast<CCreature*>(type));
 }
-std::string CStackInstance::bonusToString(const Bonus *bonus, bool description) const
+std::string CStackInstance::bonusToString(const std::shared_ptr<Bonus>& bonus, bool description) const
 {
 	if(Bonus::MAGIC_RESISTANCE == bonus->type)
 	{
@@ -641,7 +641,7 @@ std::string CStackInstance::bonusToString(const Bonus *bonus, bool description)
 
 }
 
-std::string CStackInstance::bonusToGraphics(const Bonus *bonus) const
+std::string CStackInstance::bonusToGraphics(const std::shared_ptr<Bonus>& bonus) const
 {
 	return VLC->getBth()->bonusToGraphics(bonus);
 }
@@ -814,7 +814,7 @@ void CCommanderInstance::levelUp ()
 	level++;
 	for (auto bonus : VLC->creh->commanderLevelPremy)
 	{ //grant all regular level-up bonuses
-		accumulateBonus (*bonus);
+		accumulateBonus(bonus);
 	}
 }
 

lib/CCreatureSet.h

@@ -69,8 +69,8 @@ public:
 	void readJson(const JsonNode & json);
 
 	//overrides CBonusSystemNode
-	std::string bonusToString(const Bonus *bonus, bool description) const override; // how would bonus description look for this particular type of node
+	std::string bonusToString(const std::shared_ptr<Bonus>& bonus, bool description) const override; // how would bonus description look for this particular type of node
-	std::string bonusToGraphics(const Bonus *bonus) const; //file name of graphics from StackSkills , in future possibly others
+	std::string bonusToGraphics(const std::shared_ptr<Bonus>& bonus) const; //file name of graphics from StackSkills , in future possibly others
 
 	virtual ui64 getPower() const;
 	int getQuantityID() const;

lib/CGameState.cpp

@@ -1596,7 +1596,7 @@ void CGameState::giveCampaignBonusToHero(CGHeroInstance * hero)
 					{
 						continue;
 					}
-					auto bb = new Bonus(Bonus::PERMANENT, Bonus::PRIMARY_SKILL, Bonus::CAMPAIGN_BONUS, val, *scenarioOps->campState->currentMap, g);
+					auto bb = std::make_shared<Bonus>(Bonus::PERMANENT, Bonus::PRIMARY_SKILL, Bonus::CAMPAIGN_BONUS, val, *scenarioOps->campState->currentMap, g);
 					hero->addNewBonus(bb);
 				}
 			}
@@ -1992,7 +1992,7 @@ UpgradeInfo CGameState::getUpgradeInfo(const CStackInstance &stack)
 	else if(h)
 	{	//hero specialty
 		TBonusListPtr lista = h->getBonuses(Selector::typeSubtype(Bonus::SPECIAL_UPGRADE, base->idNumber));
-		for(const Bonus *it : *lista)
+		for(const std::shared_ptr<Bonus> it : *lista)
 		{
 			auto nid = CreatureID(it->additionalInfo);
 			if (nid != base->idNumber) //in very specific case the upgrade is available by default (?)

lib/CPathfinder.cpp

@@ -841,13 +841,14 @@ TurnInfo::BonusCache::BonusCache(TBonusListPtr bl)
 	noTerrainPenalty.reserve(ETerrainType::ROCK);
 	for(int i = 0; i < ETerrainType::ROCK; i++)
 	{
-		noTerrainPenalty.push_back(bl->getFirst(Selector::type(Bonus::NO_TERRAIN_PENALTY).And(Selector::subtype(i))));
+		noTerrainPenalty.push_back(static_cast<bool>(
+				bl->getFirst(Selector::type(Bonus::NO_TERRAIN_PENALTY).And(Selector::subtype(i)))));
 	}
 
-	freeShipBoarding = bl->getFirst(Selector::type(Bonus::FREE_SHIP_BOARDING));
+	freeShipBoarding = static_cast<bool>(bl->getFirst(Selector::type(Bonus::FREE_SHIP_BOARDING)));
-	flyingMovement = bl->getFirst(Selector::type(Bonus::FLYING_MOVEMENT));
+	flyingMovement = static_cast<bool>(bl->getFirst(Selector::type(Bonus::FLYING_MOVEMENT)));
 	flyingMovementVal = bl->valOfBonuses(Selector::type(Bonus::FLYING_MOVEMENT));
-	waterWalking = bl->getFirst(Selector::type(Bonus::WATER_WALKING));
+	waterWalking = static_cast<bool>(bl->getFirst(Selector::type(Bonus::WATER_WALKING)));
 	waterWalkingVal = bl->valOfBonuses(Selector::type(Bonus::WATER_WALKING));
 }
 
@@ -896,7 +897,8 @@ bool TurnInfo::hasBonusOfType(Bonus::BonusType type, int subtype) const
 		return bonusCache->noTerrainPenalty[subtype];
 	}
 
-	return bonuses->getFirst(Selector::type(type).And(Selector::subtype(subtype)));
+	return static_cast<bool>(
+			bonuses->getFirst(Selector::type(type).And(Selector::subtype(subtype))));
 }
 
 int TurnInfo::valOfBonuses(Bonus::BonusType type, int subtype) const

lib/HeroBonus.cpp

@@ -128,10 +128,8 @@ int BonusList::totalValue() const
 	int indepMin = 0;
 	bool hasIndepMin = false;
 
-	for (auto & elem : bonuses)
+	for (auto& b : bonuses)
 	{
-		Bonus *b = elem;
-
 		switch(b->valType)
 		{
 		case Bonus::BASE_NUMBER:
@@ -179,7 +177,7 @@ int BonusList::totalValue() const
 	if(hasIndepMin && hasIndepMax)
 		assert(indepMin < indepMax);
 
-	const int notIndepBonuses = boost::count_if(bonuses, [](const Bonus *b)
+	const int notIndepBonuses = boost::count_if(bonuses, [](const std::shared_ptr<Bonus>& b)
 	{
 		return b->valType != Bonus::INDEPENDENT_MAX && b->valType != Bonus::INDEPENDENT_MIN;
 	});
@@ -201,24 +199,23 @@ int BonusList::totalValue() const
 
 	return valFirst;
 }
-const Bonus * BonusList::getFirst(const CSelector &selector) const
+
+std::shared_ptr<Bonus> BonusList::getFirst(const CSelector &select)
 {
-	for (auto & elem : bonuses)
+	for (auto & b : bonuses)
 	{
-		const Bonus *b = elem;
+		if(select(b.get()))
-		if(selector(b))
+			return b;
-			return &*b;
 	}
 	return nullptr;
 }
 
-Bonus * BonusList::getFirst(const CSelector &select)
+const std::shared_ptr<Bonus> BonusList::getFirst(const CSelector &selector) const
 {
-	for (auto & elem : bonuses)
+	for (auto & b : bonuses)
 	{
-		Bonus *b = elem;
+		if(selector(b.get()))
-		if(select(b))
+			return b;
-			return &*b;
 	}
 	return nullptr;
 }
@@ -230,19 +227,17 @@ void BonusList::getBonuses(BonusList & out, const CSelector &selector) const
 
 void BonusList::getBonuses(BonusList & out, const CSelector &selector, const CSelector &limit) const
 {
-	for (auto & elem : bonuses)
+	for (auto & b : bonuses)
 	{
-		Bonus *b = elem;
-
 		//add matching bonuses that matches limit predicate or have NO_LIMIT if no given predicate
-		if(selector(b) && ((!limit && b->effectRange == Bonus::NO_LIMIT) || ((bool)limit && limit(b))))
+		if(selector(b.get()) && ((!limit && b->effectRange == Bonus::NO_LIMIT) || ((bool)limit && limit(b.get()))))
 			out.push_back(b);
 	}
 }
 
 void BonusList::getAllBonuses(BonusList &out) const
 {
-	for(Bonus *b : bonuses)
+	for(auto & b : bonuses)
 		out.push_back(b);
 }
 
@@ -267,13 +262,13 @@ void BonusList::eliminateDuplicates()
 	bonuses.erase( unique( bonuses.begin(), bonuses.end() ), bonuses.end() );
 }
 
-void BonusList::push_back(Bonus* const &x)
+void BonusList::push_back(std::shared_ptr<Bonus> x)
 {
 	bonuses.push_back(x);
 	changed();
 }
 
-std::vector<Bonus*>::iterator BonusList::erase(const int position)
+BonusList::TInternalContainer::iterator BonusList::erase(const int position)
 {
 	changed();
 	return bonuses.erase(bonuses.begin() + position);
@@ -285,7 +280,7 @@ void BonusList::clear()
 	changed();
 }
 
-std::vector<BonusList*>::size_type BonusList::operator-=(Bonus* const &i)
+std::vector<BonusList*>::size_type BonusList::operator-=(std::shared_ptr<Bonus> const &i)
 {
 	auto itr = std::find(bonuses.begin(), bonuses.end(), i);
 	if(itr == bonuses.end())
@@ -295,13 +290,13 @@ std::vector<BonusList*>::size_type BonusList::operator-=(Bonus* const &i)
 	return true;
 }
 
-void BonusList::resize(std::vector<Bonus*>::size_type sz, Bonus* c )
+void BonusList::resize(BonusList::TInternalContainer::size_type sz, std::shared_ptr<Bonus> c )
 {
 	bonuses.resize(sz, c);
 	changed();
 }
 
-void BonusList::insert(std::vector<Bonus*>::iterator position, std::vector<Bonus*>::size_type n, Bonus* const &x)
+void BonusList::insert(BonusList::TInternalContainer::iterator position, BonusList::TInternalContainer::size_type n, std::shared_ptr<Bonus> const &x)
 {
 	bonuses.insert(position, n, x);
 	changed();
@@ -503,23 +498,23 @@ const TBonusListPtr IBonusBearer::getSpellBonuses() const
 	std::stringstream cachingStr;
 	cachingStr << "!type_" << Bonus::NONE << "source_" << Bonus::SPELL_EFFECT;
 	CSelector selector = Selector::sourceType(Bonus::SPELL_EFFECT)
-		.And(CSelector([](const Bonus * b)->bool
+		.And(CSelector([](const Bonus *b)->bool
 		{
 			return b->type != Bonus::NONE;
 		}));
 	return getBonuses(selector, Selector::anyRange(), cachingStr.str());
 }
 
-const Bonus * IBonusBearer::getEffect(ui16 id, int turn /*= 0*/) const
+const std::shared_ptr<Bonus> IBonusBearer::getEffect(ui16 id, int turn /*= 0*/) const
 {
 	//TODO should check only local bonuses?
 	auto bonuses = getAllBonuses();
-	for(const Bonus *it : *bonuses)
+	for(auto & it : *bonuses)
 	{
 		if(it->source == Bonus::SPELL_EFFECT && it->sid == id)
 		{
 			if(!turn || it->turnsRemain > turn)
-				return &(*it);
+				return it;
 		}
 	}
 	return nullptr;
@@ -531,7 +526,7 @@ ui8 IBonusBearer::howManyEffectsSet(ui16 id) const
 	ui8 ret = 0;
 
 	auto bonuses = getAllBonuses();
-	for(const Bonus *it : *bonuses)
+	for(auto& it : *bonuses)
 	{
 		if(it->source == Bonus::SPELL_EFFECT && it->sid == id) //effect found
 		{
@@ -544,20 +539,20 @@ ui8 IBonusBearer::howManyEffectsSet(ui16 id) const
 
 const TBonusListPtr IBonusBearer::getAllBonuses() const
 {
-	auto matchAll= [] (const Bonus *) { return true; };
+	auto matchAll  = [] (const Bonus *) { return true; };
-	auto matchNone= [] (const Bonus *) { return true; };
+	auto matchNone = [] (const Bonus *) { return true; };
 	return getAllBonuses(matchAll, matchNone);
 }
 
-const Bonus * IBonusBearer::getBonus(const CSelector &selector) const
+const std::shared_ptr<Bonus> IBonusBearer::getBonus(const CSelector &selector) const
 {
 	auto bonuses = getAllBonuses();
 	return bonuses->getFirst(selector);
 }
 
-Bonus * CBonusSystemNode::getBonusLocalFirst(const CSelector &selector)
+std::shared_ptr<Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector &selector)
 {
-	Bonus *ret = bonuses.getFirst(selector);
+	auto ret = bonuses.getFirst(selector);
 	if(ret)
 		return ret;
 
@@ -571,7 +566,7 @@ Bonus * CBonusSystemNode::getBonusLocalFirst(const CSelector &selector)
 	return nullptr;
 }
 
-const Bonus * CBonusSystemNode::getBonusLocalFirst( const CSelector &selector ) const
+const std::shared_ptr<Bonus> CBonusSystemNode::getBonusLocalFirst( const CSelector &selector ) const
 {
 	return (const_cast<CBonusSystemNode*>(this))->getBonusLocalFirst(selector);
 }
@@ -688,13 +683,13 @@ const TBonusListPtr CBonusSystemNode::getAllBonusesWithoutCaching(const CSelecto
 		BonusList rootBonuses, limitedRootBonuses;
 		getAllBonusesRec(rootBonuses);
 
-		for(Bonus *b : beforeLimiting)
+		for(auto b : beforeLimiting)
 			rootBonuses.push_back(b);
 
 		rootBonuses.eliminateDuplicates();
 		root->limitBonuses(rootBonuses, limitedRootBonuses);
 
-		for(Bonus *b : beforeLimiting)
+		for(auto b : beforeLimiting)
 			if(vstd::contains(limitedRootBonuses, b))
 				afterLimiting.push_back(b);
 
@@ -746,13 +741,9 @@ CBonusSystemNode::~CBonusSystemNode()
 
 	if(children.size())
 	{
-		logBonus->warnStream() << "Warning: an orphaned child!";
 		while(children.size())
 			children.front()->detachFrom(this);
 	}
-
-	for(Bonus *b : exportedBonuses)
-		delete b;
 }
 
 void CBonusSystemNode::attachTo(CBonusSystemNode *parent)
@@ -787,7 +778,7 @@ void CBonusSystemNode::popBonuses(const CSelector &s)
 {
 	BonusList bl;
 	exportedBonuses.getBonuses(bl, s);
-	for(Bonus *b : bl)
+	for(auto b : bl)
 		removeBonus(b);
 
 	for(CBonusSystemNode *child : children)
@@ -798,7 +789,7 @@ void CBonusSystemNode::updateBonuses(const CSelector &s)
 {
 	BonusList bl;
 	exportedBonuses.getBonuses(bl, s);
-	for(Bonus *b : bl)
+	for(auto b : bl)
 	{
 		b->turnsRemain--;
 		if(b->turnsRemain <= 0)
@@ -809,37 +800,36 @@ void CBonusSystemNode::updateBonuses(const CSelector &s)
 		child->updateBonuses(s);
 }
 
-void CBonusSystemNode::addNewBonus(Bonus *b)
+void CBonusSystemNode::addNewBonus(const std::shared_ptr<Bonus>& b)
 {
 	//turnsRemain shouldn't be zero for following durations
-	if(Bonus::NTurns(b) || Bonus::NDays(b) || Bonus::OneWeek(b))
+	if(Bonus::NTurns(b.get()) || Bonus::NDays(b.get()) || Bonus::OneWeek(b.get()))
 	{
 		assert(b->turnsRemain);
 	}
 
-	assert(!vstd::contains(exportedBonuses,b));
+	assert(!vstd::contains(exportedBonuses, b));
 	exportedBonuses.push_back(b);
 	exportBonus(b);
 	CBonusSystemNode::treeHasChanged();
 }
 
-void CBonusSystemNode::accumulateBonus(Bonus &b)
+void CBonusSystemNode::accumulateBonus(const std::shared_ptr<Bonus>& b)
 {
-	Bonus *bonus = exportedBonuses.getFirst(Selector::typeSubtype(b.type, b.subtype)); //only local bonuses are interesting //TODO: what about value type?
+	auto bonus = exportedBonuses.getFirst(Selector::typeSubtype(b->type, b->subtype)); //only local bonuses are interesting //TODO: what about value type?
 	if(bonus)
-		bonus->val += b.val;
+		bonus->val += b->val;
 	else
-		addNewBonus(new Bonus(b)); //duplicate needed, original may get destroyed
+		addNewBonus(std::make_shared<Bonus>(*b)); //duplicate needed, original may get destroyed
 }
 
-void CBonusSystemNode::removeBonus(Bonus *b)
+void CBonusSystemNode::removeBonus(const std::shared_ptr<Bonus>& b)
 {
 	exportedBonuses -= b;
 	if(b->propagator)
 		unpropagateBonus(b);
 	else
 		bonuses -= b;
-	vstd::clear_pointer(b);
 	CBonusSystemNode::treeHasChanged();
 }
 
@@ -856,7 +846,7 @@ bool CBonusSystemNode::actsAsBonusSourceOnly() const
 	}
 }
 
-void CBonusSystemNode::propagateBonus(Bonus * b)
+void CBonusSystemNode::propagateBonus(std::shared_ptr<Bonus> b)
 {
 	if(b->propagator->shouldBeAttached(this))
 	{
@@ -868,7 +858,7 @@ void CBonusSystemNode::propagateBonus(Bonus * b)
 		child->propagateBonus(b);
 }
 
-void CBonusSystemNode::unpropagateBonus(Bonus * b)
+void CBonusSystemNode::unpropagateBonus(std::shared_ptr<Bonus> b)
 {
 	if(b->propagator->shouldBeAttached(this))
 	{
@@ -968,7 +958,7 @@ void CBonusSystemNode::getRedChildren(TNodes &out)
 
 void CBonusSystemNode::newRedDescendant(CBonusSystemNode *descendant)
 {
-	for(Bonus *b : exportedBonuses)
+	for(auto b : exportedBonuses)
 		if(b->propagator)
 			descendant->propagateBonus(b);
 
@@ -978,7 +968,7 @@ void CBonusSystemNode::newRedDescendant(CBonusSystemNode *descendant)
 
 void CBonusSystemNode::removedRedDescendant(CBonusSystemNode *descendant)
 {
-	for(Bonus *b : exportedBonuses)
+	for(auto b : exportedBonuses)
 		if(b->propagator)
 			descendant->unpropagateBonus(b);
 
@@ -1005,7 +995,7 @@ void CBonusSystemNode::battleTurnPassed()
 	updateBonuses(Bonus::NTurns);
 }
 
-void CBonusSystemNode::exportBonus(Bonus * b)
+void CBonusSystemNode::exportBonus(std::shared_ptr<Bonus> b)
 {
 	if(b->propagator)
 		propagateBonus(b);
@@ -1017,7 +1007,7 @@ void CBonusSystemNode::exportBonus(Bonus * b)
 
 void CBonusSystemNode::exportBonuses()
 {
-	for(Bonus *b : exportedBonuses)
+	for(auto b : exportedBonuses)
 		exportBonus(b);
 }
 
@@ -1073,7 +1063,7 @@ void CBonusSystemNode::limitBonuses(const BonusList &allBonuses, BonusList &out)
 		int undecidedCount = undecided.size();
 		for(int i = 0; i < undecided.size(); i++)
 		{
-			Bonus *b = undecided[i];
+			auto b = undecided[i];
 			BonusLimitationContext context = {b, *this, out};
 			int decision = b->limiter ? b->limiter->limit(context) : ILimiter::ACCEPT; //bonuses without limiters will be accepted by default
 			if(decision == ILimiter::DISCARD)
@@ -1206,10 +1196,10 @@ Bonus::~Bonus()
 {
 }
 
-Bonus * Bonus::addPropagator(TPropagatorPtr Propagator)
+std::shared_ptr<Bonus> Bonus::addPropagator(TPropagatorPtr Propagator)
 {
 	propagator = Propagator;
-	return this;
+	return this->shared_from_this();
 }
 
 namespace Selector
@@ -1314,7 +1304,7 @@ DLL_LINKAGE std::ostream & operator<<(std::ostream &out, const BonusList &bonusL
 {
 	for (ui32 i = 0; i < bonusList.size(); i++)
 	{
-		Bonus *b = bonusList[i];
+		auto b = bonusList[i];
 		out << "Bonus " << i << "\n" << *b << std::endl;
 	}
 	return out;
@@ -1341,7 +1331,7 @@ DLL_LINKAGE std::ostream & operator<<(std::ostream &out, const Bonus &bonus)
 	return out;
 }
 
-Bonus * Bonus::addLimiter(TLimiterPtr Limiter)
+std::shared_ptr<Bonus> Bonus::addLimiter(TLimiterPtr Limiter)
 {
 	if (limiter)
 	{
@@ -1361,7 +1351,7 @@ Bonus * Bonus::addLimiter(TLimiterPtr Limiter)
 	{
 		limiter = Limiter;
 	}
-	return this;
+	return this->shared_from_this();
 }
 
 ILimiter::~ILimiter()

lib/HeroBonus.h

@@ -131,7 +131,7 @@ public:
 	BONUS_NAME(SPELL_AFTER_ATTACK) /* subtype - spell id, value - chance %, additional info % 1000 - level, (additional info)/1000 -> [0 - all attacks, 1 - shot only, 2 - melee only*/ \
 	BONUS_NAME(SPELL_BEFORE_ATTACK) /* subtype - spell id, value - chance %, additional info % 1000 - level, (additional info)/1000 -> [0 - all attacks, 1 - shot only, 2 - melee only*/ \
 	BONUS_NAME(SPELL_RESISTANCE_AURA) /*eg. unicorns, value - resistance bonus in % for adjacent creatures*/ \
-	BONUS_NAME(LEVEL_SPELL_IMMUNITY) /*creature is immune to all spell with level below or equal to value of this bonus*/ \
+	BONUS_NAME(LEVEL_SPELL_IMMUNITY) /*creature is immune to all spell with level below or equal to value of this bonus */ \
 	BONUS_NAME(BLOCK_MAGIC_ABOVE) /*blocks casting spells of the level > value */ \
 	BONUS_NAME(BLOCK_ALL_MAGIC) /*blocks casting spells*/ \
 	BONUS_NAME(TWO_HEX_ATTACK_BREATH) /*eg. dragons*/	\
@@ -245,11 +245,11 @@ public:
 	BONUS_VALUE(BASE_NUMBER)\
 	BONUS_VALUE(PERCENT_TO_ALL)\
 	BONUS_VALUE(PERCENT_TO_BASE)\
-	BONUS_VALUE(INDEPENDENT_MAX) /*used for SPELL bonus*/ \
+	BONUS_VALUE(INDEPENDENT_MAX) /*used for SPELL bonus */\
 	BONUS_VALUE(INDEPENDENT_MIN) //used for SECONDARY_SKILL_PREMY bonus
 
 /// Struct for handling bonuses of several types. Can be transferred to any hero
-struct DLL_LINKAGE Bonus
+struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 {
 	enum { EVERY_TYPE = -1 };
 
@@ -267,7 +267,7 @@ struct DLL_LINKAGE Bonus
 		ONE_WEEK = 8, //at the end of week (bonus lasts till the end of week, thats NOT 7 days
 		N_TURNS = 16, //used during battles, after battle bonus is always removed
 		N_DAYS = 32,
-		UNITL_BEING_ATTACKED = 64,/*removed after attack and counterattacks are performed*/
+		UNITL_BEING_ATTACKED = 64, /*removed after attack and counterattacks are performed*/
 		UNTIL_ATTACK = 128, /*removed after attack and counterattacks are performed*/
 		STACK_GETS_TURN = 256, /*removed when stack gets its turn - used for defensive stance*/
 		COMMANDER_KILLED = 512
@@ -334,7 +334,8 @@ struct DLL_LINKAGE Bonus
 		h & duration & type & subtype & source & val & sid & description & additionalInfo & turnsRemain & valType & effectRange & limiter & propagator;
 	}
 
-	static bool compareByAdditionalInfo(const Bonus *a, const Bonus *b)
+	template <typename Ptr>
+	static bool compareByAdditionalInfo(const Ptr& a, const Ptr& b)
 	{
 		return a->additionalInfo < b->additionalInfo;
 	}
@@ -378,9 +379,9 @@ struct DLL_LINKAGE Bonus
 	{
 		return hb->duration & Bonus::COMMANDER_KILLED;
 	}
-	static bool IsFrom(const Bonus &hb, ui8 source, ui32 id) //if id==0xffffff then id doesn't matter
+	static bool IsFrom(const Bonus *hb, ui8 source, ui32 id) //if id==0xffffff then id doesn't matter
 	{
-		return hb.source==source && (id==0xffffff  ||  hb.sid==id);
+		return hb->source==source && (id==0xffffff  ||  hb->sid==id);
 	}
 	inline bool operator == (const BonusType & cf) const
 	{
@@ -398,8 +399,8 @@ struct DLL_LINKAGE Bonus
 
 	std::string Description() const;
 
-	Bonus *addLimiter(TLimiterPtr Limiter); //returns this for convenient chain-calls
+	std::shared_ptr<Bonus> addLimiter(TLimiterPtr Limiter); //returns this for convenient chain-calls
-	Bonus *addPropagator(TPropagatorPtr Propagator); //returns this for convenient chain-calls
+	std::shared_ptr<Bonus> addPropagator(TPropagatorPtr Propagator); //returns this for convenient chain-calls
 };
 
 DLL_LINKAGE std::ostream & operator<<(std::ostream &out, const Bonus &bonus);
@@ -407,9 +408,10 @@ DLL_LINKAGE std::ostream & operator<<(std::ostream &out, const Bonus &bonus);
 
 class DLL_LINKAGE BonusList
 {
-private:
+public:
-	typedef std::vector<Bonus*> TInternalContainer;
+	typedef std::vector<std::shared_ptr<Bonus>> TInternalContainer;
 
+private:
 	TInternalContainer bonuses;
 	bool belongsToTree;
 	void changed();
@@ -427,24 +429,23 @@ public:
 	BonusList& operator=(const BonusList &bonusList);
 
 	// wrapper functions of the STL vector container
-	std::vector<Bonus*>::size_type size() const { return bonuses.size(); }
+	TInternalContainer::size_type size() const { return bonuses.size(); }
-	void push_back(Bonus* const &x);
+	void push_back(std::shared_ptr<Bonus> x);
-	std::vector<Bonus*>::iterator erase (const int position);
+	TInternalContainer::iterator erase (const int position);
 	void clear();
 	bool empty() const { return bonuses.empty(); }
-	void resize(std::vector<Bonus*>::size_type sz, Bonus* c = nullptr );
+	void resize(TInternalContainer::size_type sz, std::shared_ptr<Bonus> c = nullptr );
-	void insert(std::vector<Bonus*>::iterator position, std::vector<Bonus*>::size_type n, Bonus* const &x);
+	std::shared_ptr<Bonus> &operator[] (TInternalContainer::size_type n) { return bonuses[n]; }
-	Bonus *const &operator[] (std::vector<Bonus*>::size_type n) { return bonuses[n]; }
+	const std::shared_ptr<Bonus> &operator[] (TInternalContainer::size_type n) const { return bonuses[n]; }
-	Bonus *const &operator[] (std::vector<Bonus*>::size_type n) const { return bonuses[n]; }
+	std::shared_ptr<Bonus> &back() { return bonuses.back(); }
-	Bonus *const &back() { return bonuses.back(); }
+	std::shared_ptr<Bonus> &front() { return bonuses.front(); }
-	Bonus *const &front() { return bonuses.front(); }
+	const std::shared_ptr<Bonus> &back() const { return bonuses.back(); }
-	Bonus *const &back() const { return bonuses.back(); }
+	const std::shared_ptr<Bonus> &front() const { return bonuses.front(); }
-	Bonus *const &front() const { return bonuses.front(); }
 
 	// There should be no non-const access to provide solid,robust bonus caching
-	std::vector<Bonus*>::const_iterator begin() const { return bonuses.begin(); }
+	TInternalContainer::const_iterator begin() const { return bonuses.begin(); }
-	std::vector<Bonus*>::const_iterator end() const { return bonuses.end(); }
+	TInternalContainer::const_iterator end() const { return bonuses.end(); }
-	std::vector<Bonus*>::size_type operator-=(Bonus* const &i);
+	TInternalContainer::size_type operator-=(std::shared_ptr<Bonus> const &i);
 
 	// BonusList functions
 	int totalValue() const; //subtype -> subtype of bonus, if -1 then any
@@ -454,8 +455,8 @@ public:
 	void getBonuses(BonusList & out, const CSelector &selector) const;
 
 	//special find functions
-	Bonus *getFirst(const CSelector &select);
+	std::shared_ptr<Bonus> getFirst(const CSelector &select);
-	const Bonus *getFirst(const CSelector &select) const;
+	const std::shared_ptr<Bonus> getFirst(const CSelector &select) const;
 	int valOfBonuses(const CSelector &select) const;
 
 	//void limit(const CBonusSystemNode &node); //erases bonuses using limitor
@@ -468,8 +469,8 @@ public:
 		BonusList newList;
 		for (ui32 i = 0; i < bonuses.size(); i++)
 		{
-			Bonus *b = bonuses[i];
+			auto b = bonuses[i];
-			if (!pred(b))
+			if (!pred(b.get()))
 				newList.push_back(b);
 		}
 		bonuses.clear();
@@ -479,10 +480,12 @@ public:
 
 	template <class InputIterator>
 	void insert(const int position, InputIterator first, InputIterator last);
+	void insert(TInternalContainer::iterator position, TInternalContainer::size_type n, std::shared_ptr<Bonus> const &x);
 
-	template <typename Handler> void serialize(Handler &h, const int version)
+	template <typename Handler>
+	void serialize(Handler &h, const int version)
 	{
-		h & static_cast<std::vector<Bonus*>&>(bonuses);
+		h & static_cast<TInternalContainer&>(bonuses);
 	}
 
 	// C++ for range support
@@ -549,7 +552,7 @@ public:
 
 struct BonusLimitationContext
 {
-	const Bonus *b;
+	const std::shared_ptr<Bonus> b;
 	const CBonusSystemNode &node;
 	const BonusList &alreadyAccepted;
 };
@@ -583,7 +586,7 @@ public:
 	const TBonusListPtr getBonuses(const CSelector &selector, const std::string &cachingStr = "") const;
 
 	const TBonusListPtr getAllBonuses() const;
-	const Bonus *getBonus(const CSelector &selector) const; //returns any bonus visible on node that matches (or nullptr if none matches)
+	const std::shared_ptr<Bonus> getBonus(const CSelector &selector) const; //returns any bonus visible on node that matches (or nullptr if none matches)
 
 	//legacy interface
 	int valOfBonuses(Bonus::BonusType type, const CSelector &selector) const;
@@ -603,7 +606,7 @@ public:
 	bool isLiving() const; //non-undead, non-non living or alive
 	virtual si32 magicResistance() const;
 	ui32 Speed(int turn = 0, bool useBind = false) const; //get speed of creature with all modificators
-	const Bonus * getEffect(ui16 id, int turn = 0) const; //effect id (SP)
+	const std::shared_ptr<Bonus> getEffect(ui16 id, int turn = 0) const; //effect id (SP)
 	ui8 howManyEffectsSet(ui16 id) const; //returns amount of effects with given id set for this stack
 
 	si32 manaLimit() const; //maximum mana value for this hero (basically 10*knowledge)
@@ -652,7 +655,7 @@ public:
 	TBonusListPtr limitBonuses(const BonusList &allBonuses) const; //same as above, returns out by val for convienence
 	const TBonusListPtr getAllBonuses(const CSelector &selector, const CSelector &limit, const CBonusSystemNode *root = nullptr, const std::string &cachingStr = "") const override;
 	void getParents(TCNodes &out) const;  //retrieves list of parent nodes (nodes to inherit bonuses from),
-	const Bonus *getBonusLocalFirst(const CSelector &selector) const;
+	const std::shared_ptr<Bonus> getBonusLocalFirst(const CSelector &selector) const;
 
 	//non-const interface
 	void getParents(TNodes &out);  //retrieves list of parent nodes (nodes to inherit bonuses from)
@@ -660,35 +663,35 @@ public:
 	void getRedAncestors(TNodes &out);
 	void getRedChildren(TNodes &out);
 	void getRedDescendants(TNodes &out);
-	Bonus *getBonusLocalFirst(const CSelector &selector);
+	std::shared_ptr<Bonus> getBonusLocalFirst(const CSelector &selector);
 
 	void attachTo(CBonusSystemNode *parent);
 	void detachFrom(CBonusSystemNode *parent);
 	void detachFromAll();
-	virtual void addNewBonus(Bonus *b); //b will be deleted with destruction of node
+	virtual void addNewBonus(const std::shared_ptr<Bonus>& b);
-	void accumulateBonus(Bonus &b); //add value of bonus with same type/subtype or create new
+	void accumulateBonus(const std::shared_ptr<Bonus>& b); //add value of bonus with same type/subtype or create new
 
 	void newChildAttached(CBonusSystemNode *child);
 	void childDetached(CBonusSystemNode *child);
-	void propagateBonus(Bonus * b);
+	void propagateBonus(std::shared_ptr<Bonus> b);
-	void unpropagateBonus(Bonus * b);
+	void unpropagateBonus(std::shared_ptr<Bonus> b);
 	//void addNewBonus(const Bonus &b); //b will copied
-	void removeBonus(Bonus *b);
+	void removeBonus(const std::shared_ptr<Bonus>& b);
 	void newRedDescendant(CBonusSystemNode *descendant); //propagation needed
 	void removedRedDescendant(CBonusSystemNode *descendant); //de-propagation needed
 	void battleTurnPassed(); //updates count of remaining turns and removed outdated bonuses
 
 	bool isIndependentNode() const; //node is independent when it has no parents nor children
 	bool actsAsBonusSourceOnly() const;
-	//bool isLimitedOnUs(Bonus *b) const; //if bonus should be removed from list acquired from this node
+	//bool isLimitedOnUs(std::shared_ptr<Bonus>b) const; //if bonus should be removed from list acquired from this node
 
 	void popBonuses(const CSelector &s);
 	void updateBonuses(const CSelector &s);
-	virtual std::string bonusToString(const Bonus *bonus, bool description) const {return "";}; //description or bonus name
+	virtual std::string bonusToString(const std::shared_ptr<Bonus>& bonus, bool description) const {return "";}; //description or bonus name
 	virtual std::string nodeName() const;
 
 	void deserializationFix();
-	void exportBonus(Bonus * b);
+	void exportBonus(std::shared_ptr<Bonus> b);
 	void exportBonuses();
 
 	const BonusList &getBonusList() const;
@@ -736,9 +739,9 @@ inline Bonus makeFeatureVal(Bonus::BonusType type, ui8 duration, si16 subtype, s
 }
 
 ///generates HeroBonus from given data
-inline Bonus * makeFeature(Bonus::BonusType type, ui8 duration, si16 subtype, si32 value, Bonus::BonusSource source, ui16 turnsRemain = 0, si32 additionalInfo = 0)
+inline std::shared_ptr<Bonus> makeFeature(Bonus::BonusType type, ui8 duration, si16 subtype, si32 value, Bonus::BonusSource source, ui16 turnsRemain = 0, si32 additionalInfo = 0)
 {
-	return new Bonus(makeFeatureVal(type, duration, subtype, value, source, turnsRemain, additionalInfo));
+	return std::make_shared<Bonus>(makeFeatureVal(type, duration, subtype, value, source, turnsRemain, additionalInfo));
 }
 
 template<typename T>

lib/IBonusTypeHandler.h

@@ -20,6 +20,6 @@ class IBonusTypeHandler
 public:
 	virtual ~IBonusTypeHandler(){};
 
-	virtual std::string bonusToString(const Bonus *bonus, const IBonusBearer *bearer, bool description) const = 0;
+	virtual std::string bonusToString(const std::shared_ptr<Bonus>& bonus, const IBonusBearer *bearer, bool description) const = 0;
-	virtual std::string bonusToGraphics(const Bonus *bonus) const = 0;
+	virtual std::string bonusToGraphics(const std::shared_ptr<Bonus>& bonus) const = 0;
 };

lib/JsonNode.cpp

@@ -331,7 +331,7 @@ JsonNode & JsonNode::resolvePointer(const std::string &jsonPointer)
 
 ///JsonUtils
 
-void JsonUtils::parseTypedBonusShort(const JsonVector& source, Bonus *dest)
+void JsonUtils::parseTypedBonusShort(const JsonVector& source, std::shared_ptr<Bonus> dest)
 {
 	dest->val = source[1].Float();
 	resolveIdentifier(source[2],dest->subtype);
@@ -341,9 +341,9 @@ void JsonUtils::parseTypedBonusShort(const JsonVector& source, Bonus *dest)
 }
 
 
-Bonus * JsonUtils::parseBonus (const JsonVector &ability_vec) //TODO: merge with AddAbility, create universal parser for all bonus properties
+std::shared_ptr<Bonus> JsonUtils::parseBonus (const JsonVector &ability_vec) //TODO: merge with AddAbility, create universal parser for all bonus properties
 {
-	auto  b = new Bonus();
+	auto b = std::make_shared<Bonus>();
 	std::string type = ability_vec[0].String();
 	auto it = bonusNameMap.find(type);
 	if (it == bonusNameMap.end())
@@ -418,10 +418,10 @@ void JsonUtils::resolveIdentifier (const JsonNode &node, si32 &var)
 	}
 }
 
-Bonus * JsonUtils::parseBonus (const JsonNode &ability)
+std::shared_ptr<Bonus> JsonUtils::parseBonus(const JsonNode &ability)
 {
 
-	auto  b = new Bonus();
+	auto b = std::make_shared<Bonus>();
 	const JsonNode *value;
 
 	std::string type = ability["type"].String();
@@ -560,7 +560,7 @@ Key reverseMapFirst(const Val & val, const std::map<Key, Val> & map)
 	return "";
 }
 
-void JsonUtils::unparseBonus( JsonNode &node, const Bonus * bonus )
+void JsonUtils::unparseBonus( JsonNode &node, const std::shared_ptr<Bonus>& bonus )
 {
 	node["type"].String() = reverseMapFirst<std::string, Bonus::BonusType>(bonus->type, bonusNameMap);
 	node["subtype"].Float() = bonus->subtype;

lib/JsonNode.h

@@ -127,12 +127,12 @@ namespace JsonUtils
 	 * @brief parse short bonus format, excluding type
 	 * @note sets duration to Permament
 	 */
-	DLL_LINKAGE void parseTypedBonusShort(const JsonVector &source, Bonus *dest);
+	DLL_LINKAGE void parseTypedBonusShort(const JsonVector &source, std::shared_ptr<Bonus> dest);
 
 	///
-	DLL_LINKAGE Bonus * parseBonus (const JsonVector &ability_vec);
+	DLL_LINKAGE std::shared_ptr<Bonus> parseBonus (const JsonVector &ability_vec);
-	DLL_LINKAGE Bonus * parseBonus (const JsonNode &bonus);
+	DLL_LINKAGE std::shared_ptr<Bonus> parseBonus (const JsonNode &bonus);
-	DLL_LINKAGE void unparseBonus (JsonNode &node, const Bonus * bonus);
+	DLL_LINKAGE void unparseBonus (JsonNode &node, const std::shared_ptr<Bonus>& bonus);
 	DLL_LINKAGE void resolveIdentifier (si32 &var, const JsonNode &node, std::string name);
 	DLL_LINKAGE void resolveIdentifier (const JsonNode &node, si32 &var);
 

lib/NetPacksLib.cpp

@@ -106,10 +106,10 @@ DLL_LINKAGE void SetCommanderProperty::applyGs(CGameState *gs)
 	switch (which)
 	{
 		case BONUS:
-			commander->accumulateBonus (accumulatedBonus);
+			commander->accumulateBonus (std::make_shared<Bonus>(accumulatedBonus));
 			break;
 		case SPECIAL_SKILL:
-			commander->accumulateBonus (accumulatedBonus);
+			commander->accumulateBonus (std::make_shared<Bonus>(accumulatedBonus));
 			commander->specialSKills.insert (additionalInfo);
 			break;
 		case SECONDARY_SKILL:
@@ -272,7 +272,7 @@ DLL_LINKAGE void GiveBonus::applyGs( CGameState *gs )
 	if(Bonus::OneWeek(&bonus))
 		bonus.turnsRemain = 8 - gs->getDate(Date::DAY_OF_WEEK); // set correct number of days before adding bonus
 
-	auto b = new Bonus(bonus);
+	auto b = std::make_shared<Bonus>(bonus);
 	cbsn->addNewBonus(b);
 
 	std::string &descr = b->description;
@@ -350,7 +350,7 @@ DLL_LINKAGE void RemoveBonus::applyGs( CGameState *gs )
 
 	for (int i = 0; i < bonuses.size(); i++)
 	{
-		Bonus *b = bonuses[i];
+		auto b = bonuses[i];
 		if(b->source == source && b->sid == id)
 		{
 			bonus = *b; //backup bonus (to show to interfaces later)
@@ -1273,7 +1273,7 @@ DLL_LINKAGE void BattleTriggerEffect::applyGs( CGameState *gs )
 		}
 		case Bonus::POISON:
 		{
-			Bonus * b = st->getBonusLocalFirst(Selector::source(Bonus::SPELL_EFFECT, SpellID::POISON)
+			auto b = st->getBonusLocalFirst(Selector::source(Bonus::SPELL_EFFECT, SpellID::POISON)
 					.And(Selector::type(Bonus::STACK_HEALTH)));
 			if (b)
 				b->val = val;
@@ -1513,7 +1513,7 @@ DLL_LINKAGE void BattleSpellCast::applyGs( CGameState *gs )
 
 void actualizeEffect(CStack * s, const Bonus & ef)
 {
-	for(Bonus *stackBonus : s->getBonusList()) //TODO: optimize
+	for(auto stackBonus : s->getBonusList()) //TODO: optimize
 	{
 		if(stackBonus->source == Bonus::SPELL_EFFECT && stackBonus->type == ef.type && stackBonus->subtype == ef.subtype)
 		{
@@ -1550,7 +1550,7 @@ DLL_LINKAGE void SetStackEffect::applyGs( CGameState *gs )
 		{
 			//no such effect or cumulative - add new
 			logBonus->traceStream() << sta->nodeName() << " receives a new bonus: " << effect.Description();
-			sta->addNewBonus( new Bonus(effect));
+			sta->addNewBonus(std::make_shared<Bonus>(effect));
 		}
 		else
 			actualizeEffect(sta, effect);
@@ -1633,7 +1633,7 @@ DLL_LINKAGE void StacksHealedOrResurrected::applyGs( CGameState *gs )
 		else if(cure)
 		{
 			//removing all effects from negative spells
-			auto selector = [](const Bonus * b)
+			auto selector = [](const Bonus* b)
 			{
 				const CSpell *s = b->sourceSpell();
 				//Special case: DISRUPTING_RAY is "immune" to dispell

lib/mapObjects/CArmedInstance.cpp

@@ -46,10 +46,10 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 	if(!validTypes(false)) //object not randomized, don't bother
 		return;
 
-	Bonus *b = getExportedBonusList().getFirst(Selector::sourceType(Bonus::ARMY).And(Selector::type(Bonus::MORALE)));
+	auto b = getExportedBonusList().getFirst(Selector::sourceType(Bonus::ARMY).And(Selector::type(Bonus::MORALE)));
  	if(!b)
 	{
-		b = new Bonus(Bonus::PERMANENT, Bonus::MORALE, Bonus::ARMY, 0, -1);
+		b = std::make_shared<Bonus>(Bonus::PERMANENT, Bonus::MORALE, Bonus::ARMY, 0, -1);
 		addNewBonus(b);
 	}
 
@@ -100,12 +100,12 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 
 	//-1 modifier for any Undead unit in army
 	const ui8 UNDEAD_MODIFIER_ID = -2;
-	Bonus *undeadModifier = getExportedBonusList().getFirst(Selector::source(Bonus::ARMY, UNDEAD_MODIFIER_ID));
+	auto undeadModifier = getExportedBonusList().getFirst(Selector::source(Bonus::ARMY, UNDEAD_MODIFIER_ID));
  	if(hasUndead)
 	{
 		if(!undeadModifier)
 		{
-			undeadModifier = new Bonus(Bonus::PERMANENT, Bonus::MORALE, Bonus::ARMY, -1, UNDEAD_MODIFIER_ID, VLC->generaltexth->arraytxt[116]);
+			undeadModifier = std::make_shared<Bonus>(Bonus::PERMANENT, Bonus::MORALE, Bonus::ARMY, -1, UNDEAD_MODIFIER_ID, VLC->generaltexth->arraytxt[116]);
 			undeadModifier->description = undeadModifier->description.substr(0, undeadModifier->description.size()-2);//trim value
 			addNewBonus(undeadModifier);
 		}

lib/mapObjects/CGHeroInstance.cpp

@@ -507,7 +507,7 @@ void CGHeroInstance::initObj(CRandomGenerator & rand)
 
 	for(const auto &spec : type->spec) //TODO: unfity with bonus system
 	{
-		auto bonus = new Bonus();
+		auto bonus = std::make_shared<Bonus>();
 		bonus->val = spec.val;
 		bonus->sid = id.getNum(); //from the hero, specialty has no unique id
 		bonus->duration = Bonus::PERMANENT;
@@ -540,12 +540,12 @@ void CGHeroInstance::initObj(CRandomGenerator & rand)
 					bonus->subtype = PrimarySkill::ATTACK;
 					hs->addNewBonus(bonus);
 
-					bonus = new Bonus(*bonus);
+					bonus = std::make_shared<Bonus>(*bonus);
 					bonus->subtype = PrimarySkill::DEFENSE;
 					hs->addNewBonus(bonus);
 					//values will be calculated later
 
-					bonus = new Bonus(*bonus);
+					bonus = std::make_shared<Bonus>(*bonus);
 					bonus->type = Bonus::STACKS_SPEED;
 					bonus->val = 1; //+1 speed
 					hs->addNewBonus(bonus);
@@ -558,7 +558,7 @@ void CGHeroInstance::initObj(CRandomGenerator & rand)
 				bonus->subtype = spec.subtype; //skill id
 				bonus->val = spec.val; //value per level, in percent
 				hs->addNewBonus(bonus);
-				bonus = new Bonus(*bonus);
+				bonus = std::make_shared<Bonus>(*bonus);
 
 				switch (spec.additionalinfo)
 				{
@@ -636,15 +636,14 @@ void CGHeroInstance::initObj(CRandomGenerator & rand)
 				bonus->subtype = spec.subtype; //base id
 				bonus->additionalInfo = spec.additionalinfo; //target id
 				hs->addNewBonus(bonus);
-				bonus = new Bonus(*bonus);
+				bonus = std::make_shared<Bonus>(*bonus);
 
 				for(auto cre_id : creatures[spec.subtype]->upgrades)
 				{
 					bonus->subtype = cre_id; //propagate for regular upgrades of base creature
 					hs->addNewBonus(bonus);
-					bonus = new Bonus(*bonus);
+					bonus = std::make_shared<Bonus>(*bonus);
 				}
-				vstd::clear_pointer(bonus);
 				break;
 			}
 			case 10://resource generation
@@ -710,7 +709,7 @@ void CGHeroInstance::Updatespecialty() //TODO: calculate special value of bonuse
 		{
 			//const auto &creatures = VLC->creh->creatures;
 
-			for(Bonus * b : hs->getBonusList())
+			for(auto& b : hs->getBonusList())
 			{
 				switch (b->type)
 				{
@@ -775,10 +774,10 @@ void CGHeroInstance::updateSkill(SecondarySkill which, int val)
 		bool luck = which == SecondarySkill::LUCK;
 		Bonus::BonusType type[] = {Bonus::MORALE, Bonus::LUCK};
 
-		Bonus *b = getBonusLocalFirst(Selector::type(type[luck]).And(Selector::sourceType(Bonus::SECONDARY_SKILL)));
+		auto b = getBonusLocalFirst(Selector::type(type[luck]).And(Selector::sourceType(Bonus::SECONDARY_SKILL)));
 		if(!b)
 		{
-			b = new Bonus(Bonus::PERMANENT, type[luck], Bonus::SECONDARY_SKILL, +val, which, which, Bonus::BASE_NUMBER);
+			b = std::make_shared<Bonus>(Bonus::PERMANENT, type[luck], Bonus::SECONDARY_SKILL, +val, which, which, Bonus::BASE_NUMBER);
 			addNewBonus(b);
 		}
 		else
@@ -787,10 +786,10 @@ void CGHeroInstance::updateSkill(SecondarySkill which, int val)
 	else if(which == SecondarySkill::DIPLOMACY) //surrender discount: 20% per level
 	{
 
-		if(Bonus *b = getBonusLocalFirst(Selector::type(Bonus::SURRENDER_DISCOUNT).And(Selector::sourceType(Bonus::SECONDARY_SKILL))))
+		if(auto b = getBonusLocalFirst(Selector::type(Bonus::SURRENDER_DISCOUNT).And(Selector::sourceType(Bonus::SECONDARY_SKILL))))
 			b->val = +val;
 		else
-			addNewBonus(new Bonus(Bonus::PERMANENT, Bonus::SURRENDER_DISCOUNT, Bonus::SECONDARY_SKILL, val * 20, which));
+			addNewBonus(std::make_shared<Bonus>(Bonus::PERMANENT, Bonus::SURRENDER_DISCOUNT, Bonus::SECONDARY_SKILL, val * 20, which));
 	}
 
 	int skillVal = 0;
@@ -837,7 +836,7 @@ void CGHeroInstance::updateSkill(SecondarySkill which, int val)
 
 
 	Bonus::ValueType skillValType = skillVal ? Bonus::BASE_NUMBER : Bonus::INDEPENDENT_MIN;
-	if(Bonus * b = getExportedBonusList().getFirst(Selector::typeSubtype(Bonus::SECONDARY_SKILL_PREMY, which)
+	if(auto b = getExportedBonusList().getFirst(Selector::typeSubtype(Bonus::SECONDARY_SKILL_PREMY, which)
 			.And(Selector::sourceType(Bonus::SECONDARY_SKILL)))) //only local hero bonus
 	{
 		b->val = skillVal;
@@ -845,7 +844,7 @@ void CGHeroInstance::updateSkill(SecondarySkill which, int val)
 	}
 	else
 	{
-		auto bonus = new Bonus(Bonus::PERMANENT, Bonus::SECONDARY_SKILL_PREMY, Bonus::SECONDARY_SKILL, skillVal, id.getNum(), which, skillValType);
+		auto bonus = std::make_shared<Bonus>(Bonus::PERMANENT, Bonus::SECONDARY_SKILL_PREMY, Bonus::SECONDARY_SKILL, skillVal, id.getNum(), which, skillValType);
 		bonus->source = Bonus::SECONDARY_SKILL;
 		addNewBonus(bonus);
 	}
@@ -1174,7 +1173,7 @@ void CGHeroInstance::pushPrimSkill( PrimarySkill::PrimarySkill which, int val )
 {
 	assert(!hasBonus(Selector::typeSubtype(Bonus::PRIMARY_SKILL, which)
 						.And(Selector::sourceType(Bonus::HERO_BASE_SKILL))));
-	addNewBonus(new Bonus(Bonus::PERMANENT, Bonus::PRIMARY_SKILL, Bonus::HERO_BASE_SKILL, val, id.getNum(), which));
+	addNewBonus(std::make_shared<Bonus>(Bonus::PERMANENT, Bonus::PRIMARY_SKILL, Bonus::HERO_BASE_SKILL, val, id.getNum(), which));
 }
 
 EAlignment::EAlignment CGHeroInstance::getAlignment() const
@@ -1411,7 +1410,7 @@ void CGHeroInstance::setPrimarySkill(PrimarySkill::PrimarySkill primarySkill, si
 {
 	if(primarySkill < PrimarySkill::EXPERIENCE)
 	{
-		Bonus * skill = getBonusLocalFirst(Selector::type(Bonus::PRIMARY_SKILL)
+		auto skill = getBonusLocalFirst(Selector::type(Bonus::PRIMARY_SKILL)
 			.And(Selector::subtype(primarySkill))
 			.And(Selector::sourceType(Bonus::HERO_BASE_SKILL)));
 		assert(skill);

lib/mapObjects/CGTownInstance.cpp

@@ -447,12 +447,12 @@ GrowthInfo CGTownInstance::getGrowthInfo(int level) const
 
 	//other *-of-legion-like bonuses (%d to growth cumulative with grail)
 	TBonusListPtr bonuses = getBonuses(Selector::type(Bonus::CREATURE_GROWTH).And(Selector::subtype(level)));
-	for(const Bonus *b : *bonuses)
+	for(const std::shared_ptr<Bonus> b : *bonuses)
 		ret.entries.push_back(GrowthInfo::Entry(b->val, b->Description()));
 
 	//statue-of-legion-like bonus: % to base+castle
 	TBonusListPtr bonuses2 = getBonuses(Selector::type(Bonus::CREATURE_GROWTH_PERCENT));
-	for(const Bonus *b : *bonuses2)
+	for(const std::shared_ptr<Bonus> b : *bonuses2)
 		ret.entries.push_back(GrowthInfo::Entry(b->val * (base + castleBonus) / 100, b->Description()));
 
 	if(hasBuilt(BuildingID::GRAIL)) //grail - +50% to ALL (so far added) growth
@@ -929,10 +929,10 @@ void CGTownInstance::deserializationFix()
 
 void CGTownInstance::updateMoraleBonusFromArmy()
 {
-	Bonus *b = getExportedBonusList().getFirst(Selector::sourceType(Bonus::ARMY).And(Selector::type(Bonus::MORALE)));
+	auto b = getExportedBonusList().getFirst(Selector::sourceType(Bonus::ARMY).And(Selector::type(Bonus::MORALE)));
 	if(!b)
 	{
-		b = new Bonus(Bonus::PERMANENT, Bonus::MORALE, Bonus::ARMY, 0, -1);
+		b = std::make_shared<Bonus>(Bonus::PERMANENT, Bonus::MORALE, Bonus::ARMY, 0, -1);
 		addNewBonus(b);
 	}
 
@@ -951,7 +951,7 @@ void CGTownInstance::recreateBuildingsBonuses()
 
 	BonusList bl;
 	getExportedBonusList().getBonuses(bl, Selector::sourceType(Bonus::TOWN_STRUCTURE));
-	for(Bonus *b : bl)
+	for(auto b : bl)
 		removeBonus(b);
 
 	//tricky! -> checks tavern only if no bratherhood of sword or not a castle
@@ -1021,7 +1021,7 @@ bool CGTownInstance::addBonusIfBuilt(BuildingID building, Bonus::BonusType type,
 			descr << "-";
 		descr << val;
 
-		Bonus *b = new Bonus(Bonus::PERMANENT, type, Bonus::TOWN_STRUCTURE, val, building, descr.str(), subtype);
+		auto b = std::make_shared<Bonus>(Bonus::PERMANENT, type, Bonus::TOWN_STRUCTURE, val, building, descr.str(), subtype);
 		if(prop)
 			b->addPropagator(prop);
 		addNewBonus(b);

lib/mapObjects/JsonRandom.cpp

@@ -219,9 +219,8 @@ namespace JsonRandom
 		std::vector<Bonus> ret;
 		for (const JsonNode & entry : value.Vector())
 		{
-			Bonus * bonus = JsonUtils::parseBonus(entry);
+			auto bonus = JsonUtils::parseBonus(entry);
 			ret.push_back(*bonus);
-			delete bonus;
 		}
 		return ret;
 	}

lib/mapObjects/MiscObjects.cpp

@@ -1433,7 +1433,7 @@ void CGArtifact::serializeJsonOptions(JsonSerializeFormat& handler)
 
 	if(handler.saving && ID == Obj::SPELL_SCROLL)
 	{
-		const Bonus * b = storedArtifact->getBonusLocalFirst(Selector::type(Bonus::SPELL));
+		const std::shared_ptr<Bonus> b = storedArtifact->getBonusLocalFirst(Selector::type(Bonus::SPELL));
 		SpellID spellId(b->subtype);
 
 		std::string spell = SpellID(b->subtype).toSpell()->identifier;

lib/spells/BattleSpellMechanics.cpp

@@ -51,7 +51,7 @@ void AntimagicMechanics::applyBattle(BattleInfo * battle, const BattleSpellCast
 {
 	DefaultSpellMechanics::applyBattle(battle, packet);
 
-	doDispell(battle, packet, [this](const Bonus * b) -> bool
+	doDispell(battle, packet, [this](const Bonus *b) -> bool
 	{
 		if(b->source == Bonus::SPELL_EFFECT)
 		{

lib/spells/CDefaultSpellMechanics.cpp

@@ -499,7 +499,7 @@ void DefaultSpellMechanics::applyBattleEffects(const SpellCastEnvironment * env,
 		{
 			sse.effect.back().additionalInfo =  parameters.casterStack->ID;
 		}
-		const Bonus * bonus = nullptr;
+		std::shared_ptr<Bonus> bonus = nullptr;
 		if(parameters.casterHero)
 			bonus = parameters.casterHero->getBonusLocalFirst(Selector::typeSubtype(Bonus::SPECIAL_PECULIAR_ENCHANT, owner->id));
 		//TODO does hero specialty should affects his stack casting spells?
@@ -721,7 +721,7 @@ ESpellCastProblem::ESpellCastProblem DefaultSpellMechanics::isImmuneByStack(cons
 
 void DefaultSpellMechanics::doDispell(BattleInfo * battle, const BattleSpellCast * packet, const CSelector & selector) const
 {
-	auto localSelector = [](const Bonus * bonus)
+	auto localSelector = [](const Bonus *bonus)
 	{
 		const CSpell * sourceSpell = bonus->sourceSpell();
 		if(sourceSpell != nullptr)

lib/spells/CSpellHandler.cpp

@@ -1027,7 +1027,7 @@ CSpell * CSpellHandler::loadFromJson(const JsonNode & json, const std::string &
 		for(const auto & elem : levelNode["effects"].Struct())
 		{
 			const JsonNode & bonusNode = elem.second;
-			Bonus * b = JsonUtils::parseBonus(bonusNode);
+			auto b = JsonUtils::parseBonus(bonusNode);
 			const bool usePowerAsValue = bonusNode["val"].isNull();
 
 			//TODO: make this work. see CSpellHandler::afterLoadFinalization()
@@ -1053,10 +1053,9 @@ void CSpellHandler::afterLoadFinalization()
 	{
 		for(auto & level: spell->levels)
 		{
-			for(Bonus * bonus : level.effectsTmp)
+			for(auto bonus : level.effectsTmp)
 			{
 				level.effects.push_back(*bonus);
-				delete bonus;
 			}
 			level.effectsTmp.clear();
 

lib/spells/CSpellHandler.h

@@ -130,7 +130,7 @@ public:
 
 		std::vector<Bonus> effects;
 
-		std::vector<Bonus *> effectsTmp; //TODO: this should replace effects
+		std::vector<std::shared_ptr<Bonus>> effectsTmp; //TODO: this should replace effects
 
 		LevelInfo();
 		~LevelInfo();

lib/spells/CreatureSpellMechanics.cpp

@@ -90,7 +90,7 @@ ESpellCastProblem::ESpellCastProblem DispellHelpfulMechanics::isImmuneByStack(co
 {
 	TBonusListPtr spellBon = obj->getSpellBonuses();
 	bool hasPositiveSpell = false;
-	for(const Bonus * b : *spellBon)
+	for(const std::shared_ptr<Bonus> b : *spellBon)
 	{
 		if(SpellID(b->sid).toSpell()->isPositive())
 		{

server/CGameHandler.cpp

@@ -834,7 +834,7 @@ void CGameHandler::prepareAttack(BattleAttack &bat, const CStack *att, const CSt
 		}
 	}
 
-	const Bonus * bonus = att->getBonusLocalFirst(Selector::type(Bonus::SPELL_LIKE_ATTACK));
+	const std::shared_ptr<Bonus> bonus = att->getBonusLocalFirst(Selector::type(Bonus::SPELL_LIKE_ATTACK));
 	if (bonus && (bat.shot())) //TODO: make it work in melee?
 	{
 		//this is need for displaying hit animation
@@ -4218,8 +4218,8 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 			SpellID spellID = SpellID(ba.additionalInfo);
 			BattleHex destination(ba.destinationTile);
 
-			const Bonus *randSpellcaster = stack->getBonusLocalFirst(Selector::type(Bonus::RANDOM_SPELLCASTER));
+			const std::shared_ptr<Bonus> randSpellcaster = stack->getBonusLocalFirst(Selector::type(Bonus::RANDOM_SPELLCASTER));
-			const Bonus * spellcaster = stack->getBonusLocalFirst(Selector::typeSubtype(Bonus::SPELLCASTER, spellID));
+			const std::shared_ptr<Bonus> spellcaster = stack->getBonusLocalFirst(Selector::typeSubtype(Bonus::SPELLCASTER, spellID));
 
 			//TODO special bonus for genies ability
 			if(randSpellcaster && battleGetRandomStackSpell(getRandomGenerator(), stack, CBattleInfoCallback::RANDOM_AIMED) < 0)
@@ -4489,7 +4489,7 @@ void CGameHandler::stackTurnTrigger(const CStack * st)
 			BonusList bl = *(st->getBonuses(Selector::type(Bonus::BIND_EFFECT)));
 			std::set<const CStack*> stacks = gs->curB-> batteAdjacentCreatures(st);
 
-			for(Bonus * b : bl)
+			for(auto b : bl)
 			{
 				const CStack * stack = gs->curB->battleGetStackByID(b->additionalInfo); //binding stack must be alive and adjacent
 				if (stack)
@@ -4524,7 +4524,7 @@ void CGameHandler::stackTurnTrigger(const CStack * st)
 
 		if(st->hasBonusOfType(Bonus::POISON))
 		{
-			const Bonus * b = st->getBonusLocalFirst(Selector::source(Bonus::SPELL_EFFECT, SpellID::POISON).And(Selector::type(Bonus::STACK_HEALTH)));
+			const std::shared_ptr<Bonus> b = st->getBonusLocalFirst(Selector::source(Bonus::SPELL_EFFECT, SpellID::POISON).And(Selector::type(Bonus::STACK_HEALTH)));
 			if (b) //TODO: what if not?...
 			{
 				bte.val = std::max (b->val - 10, -(st->valOfBonuses(Bonus::POISON)));
@@ -4582,7 +4582,7 @@ void CGameHandler::stackTurnTrigger(const CStack * st)
 				auto bonus = *RandomGeneratorUtil::nextItem(bl, getRandomGenerator());
 				auto spellID = SpellID(bonus->subtype);
 				const CSpell * spell = SpellID(spellID).toSpell();
-				bl.remove_if([&bonus](Bonus * b){return b==bonus;});
+				bl.remove_if([&bonus](const Bonus* b){return b==bonus.get();});
 
 				if (gs->curB->battleCanCastThisSpell(st, spell, ECastingMode::ENCHANTER_CASTING) == ESpellCastProblem::OK)
 				{
@@ -5255,7 +5255,7 @@ void CGameHandler::attackCasting(const BattleAttack & bat, Bonus::BonusType atta
 	{
 		std::set<SpellID> spellsToCast;
 		TBonusListPtr spells = attacker->getBonuses(Selector::type(attackMode));
-		for(const Bonus *sf : *spells)
+		for(const std::shared_ptr<Bonus> sf : *spells)
 		{
 			spellsToCast.insert (SpellID(sf->subtype));
 		}
@@ -5275,7 +5275,7 @@ void CGameHandler::attackCasting(const BattleAttack & bat, Bonus::BonusType atta
 				return;
 			int spellLevel = 0;
 			TBonusListPtr spellsByType = attacker->getBonuses(Selector::typeSubtype(attackMode, spellID));
-			for(const Bonus *sf : *spellsByType)
+			for(const std::shared_ptr<Bonus> sf : *spellsByType)
 			{
 				vstd::amax(spellLevel, sf->additionalInfo % 1000); //pick highest level
 				int meleeRanged = sf->additionalInfo / 1000;
@@ -5369,7 +5369,7 @@ void CGameHandler::handleAfterAttackCasting( const BattleAttack & bat )
 
 	int acidDamage = 0;
 	TBonusListPtr acidBreath = attacker->getBonuses(Selector::type(Bonus::ACID_BREATH));
-	for(const Bonus *b : *acidBreath)
+	for(const std::shared_ptr<Bonus> b : *acidBreath)
 	{
 		if (b->additionalInfo > getRandomGenerator().nextInt(99))
 			acidDamage += b->val;
@@ -5627,7 +5627,7 @@ void CGameHandler::runBattle()
 		{
 			TBonusListPtr bl = h->getBonuses(Selector::type(Bonus::OPENING_BATTLE_SPELL));
 
-			for (Bonus *b : *bl)
+			for (auto b : *bl)
 			{
 				const CSpell * spell = SpellID(b->subtype).toSpell();