Fix text formatting, fix todo's

lib/bonuses/Bonus.cpp

@@ -90,14 +90,14 @@ JsonNode CAddInfo::toJsonNode() const
 }
 std::string Bonus::Description(std::optional<si32> customValue) const
 {
-	MetaString descriptionHelper;
+	MetaString descriptionHelper = description;
+	auto valueToShow = customValue.value_or(val);
 
-	if(description.empty())
+	if(descriptionHelper.empty())
 	{
-		if(stacking.empty() || stacking == "ALWAYS")
+		// no custom description - try to generate one based on bonus source
+		switch(source)
 		{
-			switch(source)
-			{
 			case BonusSource::ARTIFACT:
 				descriptionHelper.appendName(sid.as<ArtifactID>());
 				break;
@@ -113,24 +113,41 @@ std::string Bonus::Description(std::optional<si32> customValue) const
 			case BonusSource::HERO_SPECIAL:
 				descriptionHelper.appendTextID(sid.as<HeroTypeID>().toEntity(VLC)->getNameTextID());
 				break;
-			default:
-				//todo: handle all possible sources
-				descriptionHelper.appendRawString("Unknown");
-				break;
-			}
 		}
-		else
-			descriptionHelper = MetaString::createFromRawString(stacking);
 	}
-	else
+
+	if(descriptionHelper.empty())
 	{
-		descriptionHelper = description;
+		// still no description - try to generate one based on duration
+		if ((duration & BonusDuration::ONE_BATTLE).any())
+		{
+			if (val > 0)
+				descriptionHelper.appendTextID("core.arraytxt.110"); //+%d Temporary until next battle"
+			else
+				descriptionHelper.appendTextID("core.arraytxt.109"); //-%d Temporary until next battle"
+
+			// erase sign - already present in description string
+			valueToShow = std::abs(valueToShow);
+		}
 	}
 
-	auto valueToShow = customValue.value_or(val);
+	if(descriptionHelper.empty())
+	{
+		// still no description - generate placeholder one
+		descriptionHelper.appendRawString("Unknown");
+	}
 
 	if(valueToShow != 0)
+	{
 		descriptionHelper.replacePositiveNumber(valueToShow);
+		// there is one known string that uses '%s' placeholder for bonus value:
+		// "core.arraytxt.69" : "\nFountain of Fortune Visited %s",
+		// So also add string replacement to handle this case
+		if (valueToShow > 0)
+			descriptionHelper.replaceRawString(std::to_string(valueToShow));
+		else
+			descriptionHelper.replaceRawString("-" + std::to_string(valueToShow));
+	}
 
 	return descriptionHelper.toString();
 }

lib/bonuses/Bonus.h

@@ -85,8 +85,6 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 	Bonus(BonusDuration::Type Duration, BonusType Type, BonusSource Src, si32 Val, BonusSourceID sourceID, BonusSubtypeID subtype, BonusValueType ValType);
 	Bonus() = default;
 
-public:
-
 	template <typename Handler> void serialize(Handler &h)
 	{
 		h & duration;

lib/mapObjects/CArmedInstance.cpp

@@ -113,7 +113,6 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 		b->val = 2 - static_cast<si32>(factionsInArmy);
 		bonusDescription.appendTextID("core.arraytxt.114"); //Troops of %d alignments %d
 		bonusDescription.replaceNumber(factionsInArmy);
-		bonusDescription.replaceNumber(b->val);
 	}
 	
 	b->description = bonusDescription;

lib/mapObjects/CBank.cpp

@@ -225,15 +225,15 @@ void CBank::doVisit(const CGHeroInstance * hero) const
 			{
 			case Obj::SHIPWRECK:
 				textID = 123;
-				gbonus.bonus.description.appendRawString(VLC->generaltexth->arraytxt[99]);
+				gbonus.bonus.description = MetaString::createFromTextID("core.arraytxt.99");
 				break;
 			case Obj::DERELICT_SHIP:
 				textID = 42;
-				gbonus.bonus.description.appendRawString(VLC->generaltexth->arraytxt[101]);
+				gbonus.bonus.description = MetaString::createFromTextID("core.arraytxt.101");
 				break;
 			case Obj::CRYPT:
 				textID = 120;
-				gbonus.bonus.description.appendRawString(VLC->generaltexth->arraytxt[98]);
+				gbonus.bonus.description = MetaString::createFromTextID("core.arraytxt.98");
 				break;
 			}
 			cb->giveHeroBonus(&gbonus);
@@ -245,7 +245,7 @@ void CBank::doVisit(const CGHeroInstance * hero) const
 		{
 			GiveBonus gb;
 			gb.bonus = Bonus(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, -2, BonusSourceID(id));
-			gb.bonus.description.appendTextID("core.arraytxt.70");
+			gb.bonus.description = MetaString::createFromTextID("core.arraytxt.70");
 			gb.id = hero->id;
 			cb->giveHeroBonus(&gb);
 			textID = 107;

lib/mapObjects/CGTownBuilding.cpp

@@ -115,7 +115,7 @@ std::string CGTownBuilding::getCustomBonusGreeting(const Bonus & bonus) const
 	if(bonus.type == BonusType::LUCK)
 		paramTextID = "core.genrltxt.385"; // Luck
 
-//	greeting.replaceTextID(bonus.descriptionTextID);
+	greeting.replaceTextID(town->getTown()->getSpecialBuilding(bType)->getNameTextID());
 	greeting.replaceNumber(bonus.val);
 	greeting.replaceTextID(paramTextID);
 

lib/networkPacks/NetPacksLib.cpp

@@ -1004,27 +1004,6 @@ void GiveBonus::applyGs(CGameState *gs)
 
 	auto b = std::make_shared<Bonus>(bonus);
 	cbsn->addNewBonus(b);
-
-
-	if(b->description.empty())
-	{
-		if((bonus.type == BonusType::LUCK || bonus.type == BonusType::MORALE)
-		  && (bonus.source == BonusSource::OBJECT_TYPE || bonus.source == BonusSource::OBJECT_INSTANCE))
-		{
-			//?could use allways when Type == BonusDuration::Type::ONE_BATTLE
-			if (bonus.val > 0)
-				b->description.appendTextID("core.arraytxt.110"); //+%d Temporary until next battle"
-			else
-				b->description.appendTextID("core.arraytxt.109"); //-%d Temporary until next battle"
-		}
-		else
-		{
-			logGlobal->debug("Empty bonus decription. Type=%d", (int) bonus.type);
-		}
-	}
-	// Some of(?) versions of H3 use " %s" here instead of %d. Try to replace both of them
-	//boost::replace_first(descr, "%d", std::to_string(std::abs(bonus.val))); // " +/-%d Temporary until next battle
-	//boost::replace_first(descr, " %s", boost::str(boost::format(" %+d") % bonus.val));  // " %s" in arraytxt.69, fountian of fortune
 }
 
 void ChangeObjPos::applyGs(CGameState *gs)

lib/spells/BonusCaster.cpp

@@ -16,6 +16,9 @@
 #include "../MetaString.h"
 #include "../battle/Unit.h"
 #include "../bonuses/Bonus.h"
+#include "../VCMI_Lib.h"
+#include "../CSkillHandler.h"
+#include "../CHeroHandler.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -32,10 +35,27 @@ BonusCaster::~BonusCaster() = default;
 
 void BonusCaster::getCasterName(MetaString & text) const
 {
-//	if(!bonus->descriptionTextID.empty())
-//		text.replaceTextID(bonus->descriptionTextID);
-//	else
-		actualCaster->getCasterName(text);
+	switch(bonus->source)
+	{
+		case BonusSource::ARTIFACT:
+			text.replaceName(bonus->sid.as<ArtifactID>());
+			break;
+		case BonusSource::SPELL_EFFECT:
+			text.replaceName(bonus->sid.as<SpellID>());
+			break;
+		case BonusSource::CREATURE_ABILITY:
+			text.replaceNamePlural(bonus->sid.as<CreatureID>());
+			break;
+		case BonusSource::SECONDARY_SKILL:
+			text.replaceTextID(bonus->sid.as<SecondarySkill>().toEntity(VLC)->getNameTextID());
+			break;
+		case BonusSource::HERO_SPECIAL:
+			text.replaceTextID(bonus->sid.as<HeroTypeID>().toEntity(VLC)->getNameTextID());
+			break;
+		default:
+			actualCaster->getCasterName(text);
+			break;
+	}
 }
 
 void BonusCaster::getCastDescription(const Spell * spell, const std::vector<const battle::Unit*> & attacked, MetaString & text) const

server/battles/BattleActionProcessor.cpp

@@ -1541,7 +1541,7 @@ void BattleActionProcessor::addGenericKilledLog(BattleLogMessage & blm, const CS
 				line.replaceTextID("core.genrltxt.42"); // creature
 		}
 		else
-			line.replaceName(CreatureID(defender->unitId()), killed);
+			line.replaceName(defender->unitType()->getId(), killed);
 
 		blm.lines.push_back(line);
 	}