Use bonus only as shared_ptr to avoid memory corruption

lib/entities/artifact/CArtHandler.cpp

@@ -142,15 +142,10 @@ std::shared_ptr<CArtifact> CArtHandler::loadFromJson(const std::string & scope,
 	if(!node["growing"].isNull())
 	{
 		for(auto bonus : node["growing"]["bonusesPerLevel"].Vector())
-		{
-			art->bonusesPerLevel.emplace_back(static_cast<ui16>(bonus["level"].Float()), Bonus());
-			JsonUtils::parseBonus(bonus["bonus"], &art->bonusesPerLevel.back().second);
-		}
+			art->bonusesPerLevel.emplace_back(static_cast<ui16>(bonus["level"].Float()), JsonUtils::parseBonus(bonus["bonus"]));
+
 		for(auto bonus : node["growing"]["thresholdBonuses"].Vector())
-		{
-			art->thresholdBonuses.emplace_back(static_cast<ui16>(bonus["level"].Float()), Bonus());
-			JsonUtils::parseBonus(bonus["bonus"], &art->thresholdBonuses.back().second);
-		}
+			art->thresholdBonuses.emplace_back(static_cast<ui16>(bonus["level"].Float()), JsonUtils::parseBonus(bonus["bonus"]));
 	}
 
 	art->id = ArtifactID(index);

lib/entities/artifact/CArtifact.cpp

@@ -59,22 +59,22 @@ bool CGrowingArtifact::isGrowing() const
 	return !bonusesPerLevel.empty() || !thresholdBonuses.empty();
 }
 
-std::vector <std::pair<ui16, Bonus>> & CGrowingArtifact::getBonusesPerLevel()
+std::vector <std::pair<ui16, std::shared_ptr<Bonus>>> & CGrowingArtifact::getBonusesPerLevel()
 {
 	return bonusesPerLevel;
 }
 
-const std::vector <std::pair<ui16, Bonus>> & CGrowingArtifact::getBonusesPerLevel() const
+const std::vector <std::pair<ui16, std::shared_ptr<Bonus>>> & CGrowingArtifact::getBonusesPerLevel() const
 {
 	return bonusesPerLevel;
 }
 
-std::vector <std::pair<ui16, Bonus>> & CGrowingArtifact::getThresholdBonuses()
+std::vector <std::pair<ui16, std::shared_ptr<Bonus>>> & CGrowingArtifact::getThresholdBonuses()
 {
 	return thresholdBonuses;
 }
 
-const std::vector <std::pair<ui16, Bonus>> & CGrowingArtifact::getThresholdBonuses() const
+const std::vector <std::pair<ui16, std::shared_ptr<Bonus>>> & CGrowingArtifact::getThresholdBonuses() const
 {
 	return thresholdBonuses;
 }

lib/entities/artifact/CArtifact.h

@@ -54,15 +54,15 @@ class DLL_LINKAGE CGrowingArtifact
 protected:
 	CGrowingArtifact() = default;
 
-	std::vector<std::pair<ui16, Bonus>> bonusesPerLevel; // Bonus given each n levels
-	std::vector<std::pair<ui16, Bonus>> thresholdBonuses; // After certain level they will be added once
+	std::vector<std::pair<ui16, std::shared_ptr<Bonus>>> bonusesPerLevel; // Bonus given each n levels
+	std::vector<std::pair<ui16, std::shared_ptr<Bonus>>> thresholdBonuses; // After certain level they will be added once
 public:
 	bool isGrowing() const;
 
-	std::vector<std::pair<ui16, Bonus>> & getBonusesPerLevel();
-	const std::vector<std::pair<ui16, Bonus>> & getBonusesPerLevel() const;
-	std::vector<std::pair<ui16, Bonus>> & getThresholdBonuses();
-	const std::vector<std::pair<ui16, Bonus>> & getThresholdBonuses() const;
+	std::vector<std::pair<ui16, std::shared_ptr<Bonus>>> & getBonusesPerLevel();
+	const std::vector<std::pair<ui16, std::shared_ptr<Bonus>>> & getBonusesPerLevel() const;
+	std::vector<std::pair<ui16, std::shared_ptr<Bonus>>> & getThresholdBonuses();
+	const std::vector<std::pair<ui16, std::shared_ptr<Bonus>>> & getThresholdBonuses() const;
 };
 
 class DLL_LINKAGE CChargedArtifact

lib/entities/artifact/CArtifactInstance.cpp

@@ -109,7 +109,7 @@ void CGrowingArtifactInstance::growingUp()
 			// Every n levels
 			if(artInst->valOfBonuses(BonusType::ARTIFACT_GROWING) % bonus.first == 0)
 			{
-				artInst->accumulateBonus(std::make_shared<Bonus>(bonus.second));
+				artInst->accumulateBonus(std::make_shared<Bonus>(*bonus.second));
 			}
 		}
 		for(const auto & bonus : artType->getThresholdBonuses())
@@ -117,7 +117,7 @@ void CGrowingArtifactInstance::growingUp()
 			// At n level
 			if(artInst->valOfBonuses(BonusType::ARTIFACT_GROWING) == bonus.first)
 			{
-				artInst->addNewBonus(std::make_shared<Bonus>(bonus.second));
+				artInst->addNewBonus(std::make_shared<Bonus>(*bonus.second));
 			}
 		}
 

lib/json/JsonRandom.cpp

@@ -580,13 +580,13 @@ JsonRandom::JsonRandom(IGameInfoCallback * cb, IGameRandomizer & gameRandomizer)
 		return ret;
 	}
 
-	std::vector<Bonus> JsonRandom::loadBonuses(const JsonNode & value)
+	std::vector<std::shared_ptr<Bonus>> JsonRandom::loadBonuses(const JsonNode & value)
 	{
-		std::vector<Bonus> ret;
+		std::vector<std::shared_ptr<Bonus>> ret;
 		for (const JsonNode & entry : value.Vector())
 		{
 			if(auto bonus = JsonUtils::parseBonus(entry))
-				ret.push_back(*bonus);
+				ret.push_back(bonus);
 		}
 		return ret;
 	}

lib/json/JsonRandom.h

@@ -91,7 +91,7 @@ public:
 	std::vector<HeroTypeID> loadHeroes(const JsonNode & value);
 	std::vector<HeroClassID> loadHeroClasses(const JsonNode & value);
 
-	static std::vector<Bonus> loadBonuses(const JsonNode & value);
+	static std::vector<std::shared_ptr<Bonus>> loadBonuses(const JsonNode & value);
 };
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjectConstructors/CRewardableConstructor.cpp

@@ -46,12 +46,12 @@ std::shared_ptr<CGObjectInstance> CRewardableConstructor::create(IGameInfoCallba
 	return ret;
 }
 
-void CRewardableConstructor::assignBonuses(std::vector<Bonus> & bonuses, MapObjectID objectID) const
+void CRewardableConstructor::assignBonuses(std::vector<std::shared_ptr<Bonus>> & bonuses, MapObjectID objectID) const
 {
 	for (auto & bonus : bonuses)
 	{
-		bonus.source = BonusSource::OBJECT_TYPE;
-		bonus.sid = BonusSourceID(objectID);
+		bonus->source = BonusSource::OBJECT_TYPE;
+		bonus->sid = BonusSourceID(objectID);
 	}
 }
 

lib/mapObjectConstructors/CRewardableConstructor.h

@@ -20,7 +20,7 @@ class DLL_LINKAGE CRewardableConstructor : public AObjectTypeHandler
 {
 	Rewardable::Info objectInfo;
 
-	void assignBonuses(std::vector<Bonus> & bonuses, MapObjectID objectID) const;
+	void assignBonuses(std::vector<std::shared_ptr<Bonus>> & bonuses, MapObjectID objectID) const;
 	void initTypeData(const JsonNode & config) override;
 	
 	bool blockVisit = false;

lib/mapObjectConstructors/CommonConstructors.cpp

@@ -269,7 +269,7 @@ void BoatInstanceConstructor::initializeObject(CGBoat * boat) const
 	boat->onboardAssaultAllowed = onboardAssaultAllowed;
 	boat->onboardVisitAllowed = onboardVisitAllowed;
 	for(auto & b : bonuses)
-		boat->addNewBonus(std::make_shared<Bonus>(b));
+		boat->addNewBonus(b);
 }
 
 AnimationPath BoatInstanceConstructor::getBoatAnimationName() const

lib/mapObjectConstructors/CommonConstructors.h

@@ -110,7 +110,7 @@ class DLL_LINKAGE BoatInstanceConstructor : public CDefaultObjectTypeHandler<CGB
 protected:
 	void initTypeData(const JsonNode & config) override;
 	
-	std::vector<Bonus> bonuses;
+	std::vector<std::shared_ptr<Bonus>> bonuses;
 	EPathfindingLayer layer;
 	bool onboardAssaultAllowed; //if true, hero can attack units from transport
 	bool onboardVisitAllowed; //if true, hero can visit objects from transport

lib/mapObjects/CGPandoraBox.cpp

@@ -109,10 +109,10 @@ void CGPandoraBox::grantRewardWithMessage(IGameEventCallback & gameEvents, const
 	
 	for(auto b : vi.reward.heroBonuses)
 	{
-		if(b.val && b.type == BonusType::MORALE)
-			txt = setText(b.val > 0, 179, 178, h);
-		if(b.val && b.type == BonusType::LUCK)
-			txt = setText(b.val > 0, 181, 180, h);
+		if(b->val && b->type == BonusType::MORALE)
+			txt = setText(b->val > 0, 179, 178, h);
+		if(b->val && b->type == BonusType::LUCK)
+			txt = setText(b->val > 0, 181, 180, h);
 	}
 	sendInfoWindow(txt, temp);
 	
@@ -229,11 +229,11 @@ void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 		int val = 0;
 		handler.serializeInt("morale", val, 0);
 		if(val)
-			vinfo.reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
+			vinfo.reward.heroBonuses.push_back(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id)));
 		
 		handler.serializeInt("luck", val, 0);
 		if(val)
-			vinfo.reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
+			vinfo.reward.heroBonuses.push_back(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id)));
 		
 		vinfo.reward.resources.serializeJson(handler, "resources");
 		{

lib/mapObjects/CQuest.cpp

@@ -703,9 +703,9 @@ void CGSeerHut::serializeJsonOptions(JsonSerializeFormat & handler)
 		if(metaTypeName == "mana")
 			reward.manaDiff = val;
 		if(metaTypeName == "morale")
-			reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
+			reward.heroBonuses.push_back(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id)));
 		if(metaTypeName == "luck")
-			reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id));
+			reward.heroBonuses.push_back(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(id)));
 		if(metaTypeName == "resource")
 		{
 			auto rawId = *LIBRARY->identifiers()->getIdentifier(ModScope::scopeMap(), fullIdentifier, false);

lib/mapObjects/TownBuildingInstance.cpp

@@ -73,7 +73,7 @@ TownRewardableBuildingInstance::TownRewardableBuildingInstance(CGTownInstance *
 	configuration = generateConfiguration(gameRandomizer);
 }
 
-void TownRewardableBuildingInstance::assignBonuses(std::vector<Bonus> & bonuses) const
+void TownRewardableBuildingInstance::assignBonuses(std::vector<std::shared_ptr<Bonus>> & bonuses) const
 {
 	const auto & building = town->getTown()->buildings.at(getBuildingType());
 
@@ -81,13 +81,13 @@ void TownRewardableBuildingInstance::assignBonuses(std::vector<Bonus> & bonuses)
 	{
 		if (building->mapObjectLikeBonuses.hasValue())
 		{
-			bonus.source = BonusSource::OBJECT_TYPE;
-			bonus.sid = BonusSourceID(building->mapObjectLikeBonuses);
+			bonus->source = BonusSource::OBJECT_TYPE;
+			bonus->sid = BonusSourceID(building->mapObjectLikeBonuses);
 		}
 		else
 		{
-			bonus.source = BonusSource::TOWN_STRUCTURE;
-			bonus.sid = BonusSourceID(building->getUniqueTypeID());
+			bonus->source = BonusSource::TOWN_STRUCTURE;
+			bonus->sid = BonusSourceID(building->getUniqueTypeID());
 		}
 	}
 }

lib/mapObjects/TownBuildingInstance.h

@@ -58,7 +58,7 @@ class DLL_LINKAGE TownRewardableBuildingInstance : public TownBuildingInstance,
 	bool wasVisitedBefore(const CGHeroInstance * contextHero) const override;
 	void grantReward(IGameEventCallback & gameEvents, ui32 rewardID, const CGHeroInstance * hero) const override;
 	Rewardable::Configuration generateConfiguration(IGameRandomizer & gameRandomizer) const;
-	void assignBonuses(std::vector<Bonus> & bonuses) const;
+	void assignBonuses(std::vector<std::shared_ptr<Bonus>> & bonuses) const;
 
 	const IObjectInterface * getObject() const override;
 	bool wasVisited(PlayerColor player) const override;

lib/mapping/MapFormatH3M.cpp

@@ -1083,9 +1083,9 @@ void CMapLoaderH3M::readBoxContent(CGPandoraBox * object, const int3 & mapPositi
 	reward.heroExperience = reader->readUInt32();
 	reward.manaDiff = reader->readInt32();
 	if(auto val = reader->readInt8Checked(-3, 3))
-		reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven));
+		reward.heroBonuses.push_back(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven)));
 	if(auto val = reader->readInt8Checked(-3, 3))
-		reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven));
+		reward.heroBonuses.push_back(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven)));
 
 	reader->readResources(reward.resources);
 	for(int x = 0; x < GameConstants::PRIMARY_SKILLS; ++x)
@@ -2275,12 +2275,12 @@ void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, con
 			}
 			case ESeerHutRewardType::MORALE:
 			{
-				reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven));
+				reward.heroBonuses.push_back(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven)));
 				break;
 			}
 			case ESeerHutRewardType::LUCK:
 			{
-				reward.heroBonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven));
+				reward.heroBonuses.push_back(std::make_shared<Bonus>(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven)));
 				break;
 			}
 			case ESeerHutRewardType::RESOURCES:

lib/rewardable/Interface.cpp

@@ -152,24 +152,24 @@ void Rewardable::Interface::grantRewardAfterLevelup(IGameEventCallback & gameEve
 		gameEvents.setMovePoints(&smp);
 	}
 
-	for(const Bonus & bonus : info.reward.heroBonuses)
+	for(const auto & bonus : info.reward.heroBonuses)
 	{
-		GiveBonus gb(GiveBonus::ETarget::OBJECT, hero->id, bonus);
+		GiveBonus gb(GiveBonus::ETarget::OBJECT, hero->id, *bonus);
 		gameEvents.giveHeroBonus(&gb);
 	}
 
 	if (hero->getCommander())
 	{
-		for(const Bonus & bonus : info.reward.commanderBonuses)
+		for(const auto & bonus : info.reward.commanderBonuses)
 		{
-			GiveBonus gb(GiveBonus::ETarget::HERO_COMMANDER, hero->id, bonus);
+			GiveBonus gb(GiveBonus::ETarget::HERO_COMMANDER, hero->id, *bonus);
 			gameEvents.giveHeroBonus(&gb);
 		}
 	}
 
-	for(const Bonus & bonus : info.reward.playerBonuses)
+	for(const auto & bonus : info.reward.playerBonuses)
 	{
-		GiveBonus gb(GiveBonus::ETarget::PLAYER, hero->getOwner(), bonus);
+		GiveBonus gb(GiveBonus::ETarget::PLAYER, hero->getOwner(), *bonus);
 		gameEvents.giveHeroBonus(&gb);
 	}
 

lib/rewardable/Reward.cpp

@@ -81,10 +81,10 @@ void Rewardable::Reward::loadComponents(std::vector<Component> & comps, const CG
 	
 	for (auto & bonus : heroBonuses)
 	{
-		if (bonus.type == BonusType::MORALE)
-			comps.emplace_back(ComponentType::MORALE, bonus.val);
-		if (bonus.type == BonusType::LUCK)
-			comps.emplace_back(ComponentType::LUCK, bonus.val);
+		if (bonus->type == BonusType::MORALE)
+			comps.emplace_back(ComponentType::MORALE, bonus->val);
+		if (bonus->type == BonusType::LUCK)
+			comps.emplace_back(ComponentType::LUCK, bonus->val);
 	}
 	
 	if (heroExperience)

lib/rewardable/Reward.h

@@ -86,9 +86,9 @@ struct DLL_LINKAGE Reward final
 	std::vector<CStackBasicDescriptor> guards;
 
 	/// list of bonuses, e.g. morale/luck
-	std::vector<Bonus> heroBonuses;
-	std::vector<Bonus> commanderBonuses;
-	std::vector<Bonus> playerBonuses;
+	std::vector<std::shared_ptr<Bonus>> heroBonuses;
+	std::vector<std::shared_ptr<Bonus>> commanderBonuses;
+	std::vector<std::shared_ptr<Bonus>> playerBonuses;
 
 	/// skills that hero may receive or lose
 	std::vector<si32> primary;

mapeditor/inspector/rewardswidget.cpp

@@ -345,7 +345,7 @@ void RewardsWidget::saveCurrentVisitInfo(int index)
 		auto dur = bonusDurationMap.at(ui->bonuses->item(i, 0)->text().toStdString());
 		auto typ = static_cast<BonusType>(*LIBRARY->identifiers()->getIdentifier(ModScope::scopeBuiltin(), "bonus", ui->bonuses->item(i, 1)->text().toStdString()));
 		auto val = ui->bonuses->item(i, 2)->data(Qt::UserRole).toInt();
-		vinfo.reward.heroBonuses.emplace_back(dur, typ, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(object.id));
+		vinfo.reward.heroBonuses.push_back(std::make_shared<Bonus>(dur, typ, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(object.id)));
 	}
 	
 	vinfo.limiter.dayOfWeek = ui->lDayOfWeek->currentIndex();
@@ -483,7 +483,7 @@ void RewardsWidget::loadCurrentVisitInfo(int index)
 	
 	for(auto & i : vinfo.reward.heroBonuses)
 	{
-		auto dur = vstd::findKey(bonusDurationMap, i.duration);
+		auto dur = vstd::findKey(bonusDurationMap, i->duration);
 		for(int i = 0; i < ui->bonusDuration->count(); ++i)
 		{
 			if(ui->bonusDuration->itemText(i) == QString::fromStdString(dur))
@@ -493,7 +493,7 @@ void RewardsWidget::loadCurrentVisitInfo(int index)
 			}
 		}
 		
-		std::string typ = LIBRARY->bth->bonusToString(i.type);
+		std::string typ = LIBRARY->bth->bonusToString(i->type);
 		for(int i = 0; i < ui->bonusType->count(); ++i)
 		{
 			if(ui->bonusType->itemText(i) == QString::fromStdString(typ))
@@ -503,7 +503,7 @@ void RewardsWidget::loadCurrentVisitInfo(int index)
 			}
 		}
 		
-		ui->bonusValue->setValue(i.val);
+		ui->bonusValue->setValue(i->val);
 		on_bonusAdd_clicked();
 	}
 	
@@ -819,8 +819,8 @@ void RewardsDelegate::updateModelData(QAbstractItemModel * model, const QModelIn
 		QStringList bonusesList;
 		for (auto & bonus : vinfo.reward.heroBonuses)
 		{
-			std::string bonusName = LIBRARY->bth->bonusToString(bonus.type);
-			bonusesList += QString("%1 %2 (%3)").arg(QString::fromStdString(vstd::findKey(bonusDurationMap, bonus.duration))).arg(QString::fromStdString(bonusName)).arg(bonus.val);
+			std::string bonusName = LIBRARY->bth->bonusToString(bonus->type);
+			bonusesList += QString("%1 %2 (%3)").arg(QString::fromStdString(vstd::findKey(bonusDurationMap, bonus->duration))).arg(QString::fromStdString(bonusName)).arg(bonus->val);
 		}
 		textList += QObject::tr("Bonuses: %1").arg(bonusesList.join(", "));
 	}