Show (mostly) correct messages in Shrines

config/objects/rewardableShrine.json

@@ -19,6 +19,8 @@
 				"compatibilityIdentifiers" : [ "object" ],
 				
 				"visitMode" : "limiter",
+				"visitedTooltip" : 354,
+				"description" : "(Learn 1st level spell)",
 
 				"variables" : {
 					"spell" : {
@@ -42,10 +44,11 @@
 						"spells" : [
 							"@gainedSpell"
 						],
-						"message" : 127 // You learn new spell
+						"description" : 355,
+						"message" : [ 127, "%s." ] // You learn new spell
 					}
 				],
-				"onVisitedMessage" : 174, // You already known this spell
+				"onVisitedMessage" : [ 127, "%s.", 174 ], // You already known this spell
 				"onEmpty" : [
 					{
 						"limiter" : {
@@ -55,10 +58,10 @@
 								}
 							]
 						},
-						"message" : 130 // No Wisdom
+						"message" : [ 127, "%s.", 130 ] // No Wisdom
 					},
 					{
-						"message" : 131 // No spellbook
+						"message" : [ 127, "%s.", 131 ] // No spellbook
 					}
 				]
 			}
@@ -84,6 +87,8 @@
 				"compatibilityIdentifiers" : [ "object" ],
 				
 				"visitMode" : "limiter",
+				"visitedTooltip" : 354,
+				"description" : "(Learn 2nd level spell)",
 
 				"variables" : {
 					"spell" : {
@@ -107,10 +112,11 @@
 						"spells" : [
 							"@gainedSpell"
 						],
-						"message" : 128 // You learn new spell
+						"description" : 355,
+						"message" : [ 128, "%s." ] // You learn new spell
 					}
 				],
-				"onVisitedMessage" : 174, // You already known this spell
+				"onVisitedMessage" : [ 128, "%s.", 174 ], // You already known this spell
 				"onEmpty" : [
 					{
 						"limiter" : {
@@ -120,10 +126,10 @@
 								}
 							]
 						},
-						"message" : 130 // No Wisdom
+						"message" : [ 128, "%s.", 130 ] // No Wisdom
 					},
 					{
-						"message" : 131 // No spellbook
+						"message" : [ 128, "%s.", 131 ] // No spellbook
 					}
 				]
 			}
@@ -149,6 +155,8 @@
 				"compatibilityIdentifiers" : [ "object" ],
 				
 				"visitMode" : "limiter",
+				"visitedTooltip" : 354,
+				"description" : "(Learn 3rd level spell)",
 
 				"variables" : {
 					"spell" : {
@@ -172,10 +180,11 @@
 						"spells" : [
 							"@gainedSpell"
 						],
-						"message" : 129 // You learn new spell
+						"description" : 355,
+						"message" : [ 129, "%s." ] // You learn new spell
 					}
 				],
-				"onVisitedMessage" : 174, // You already known this spell
+				"onVisitedMessage" : [ 129, "%s.", 174 ], // You already known this spell
 				"onEmpty" : [
 					{
 						"limiter" : {
@@ -185,10 +194,10 @@
 								}
 							]
 						},
-						"message" : 130 // No Wisdom
+						"message" : [ 129, "%s.", 130 ] // No Wisdom
 					},
 					{
-						"message" : 131 // No spellbook
+						"message" : [ 129, "%s.", 131 ] // No spellbook
 					}
 				]
 			}

config/objects/rewardableWitchHut.json

@@ -19,6 +19,7 @@
 				"compatibilityIdentifiers" : [ "object" ],
 				
 				"visitMode" : "limiter",
+				"visitedTooltip" : 354,
 
 				"variables" : {
 					"secondarySkill" : {

lib/JsonNode.cpp

@@ -822,10 +822,7 @@ std::shared_ptr<Bonus> JsonUtils::parseBonus(const JsonNode &ability)
 	{
 		// caller code can not handle this case and presumes that returned bonus is always valid
 		logGlobal->error("Failed to parse bonus! Json config was %S ", ability.toJson());
-
 		b->type = BonusType::NONE;
-		assert(0); // or throw? Game *should* work with dummy bonus
-
 		return b;
 	}
 	return b;

lib/mapObjects/CRewardableObject.cpp

@@ -21,13 +21,6 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-// FIXME: copy-pasted from CObjectHandler
-static std::string visitedTxt(const bool visited)
-{
-	int id = visited ? 352 : 353;
-	return VLC->generaltexth->allTexts[id];
-}
-
 void CRewardableObject::grantRewardWithMessage(const CGHeroInstance * contextHero, int index, bool markAsVisit) const
 {
 	auto vi = configuration.info.at(index);
@@ -124,6 +117,12 @@ void CRewardableObject::onHeroVisit(const CGHeroInstance *h) const
 	{
 		logGlobal->debug("Revisiting already visited object");
 
+		if (!wasVisited(h->getOwner()))
+		{
+			ChangeObjectVisitors cov(ChangeObjectVisitors::VISITOR_ADD_TEAM, id, h->id);
+			cb->sendAndApply(&cov);
+		}
+
 		auto visitedRewards = getAvailableRewards(h, Rewardable::EEventType::EVENT_ALREADY_VISITED);
 		if (!visitedRewards.empty())
 			grantRewardWithMessage(h, visitedRewards[0], false);
@@ -212,6 +211,11 @@ bool CRewardableObject::wasVisited(PlayerColor player) const
 	}
 }
 
+bool CRewardableObject::wasScouted(PlayerColor player) const
+{
+	return vstd::contains(cb->getPlayerState(player)->visitedObjects, ObjectInstanceID(id));
+}
+
 bool CRewardableObject::wasVisited(const CGHeroInstance * h) const
 {
 	switch (configuration.visitMode)
@@ -221,24 +225,34 @@ bool CRewardableObject::wasVisited(const CGHeroInstance * h) const
 		case Rewardable::VISIT_HERO:
 			return h->visitedObjects.count(ObjectInstanceID(id));
 		case Rewardable::VISIT_LIMITER:
-			return configuration.visitLimiter.heroAllowed(h);
+			return wasScouted(h->getOwner()) && configuration.visitLimiter.heroAllowed(h);
 		default:
-			return wasVisited(h->tempOwner);
+			return wasVisited(h->getOwner());
 	}
 }
 
 std::string CRewardableObject::getHoverText(PlayerColor player) const
 {
 	if(configuration.visitMode == Rewardable::VISIT_PLAYER || configuration.visitMode == Rewardable::VISIT_ONCE)
-		return getObjectName() + " " + visitedTxt(wasVisited(player));
-	return getObjectName();
+	{
+		if (wasVisited(player))
+			return getObjectName() + "\n" + configuration.visitedTooltip.toString() + "\n\n" + configuration.description.toString();
+		else
+			return getObjectName() + "\n" + configuration.notVisitedTooltip.toString() + "\n\n" + configuration.description.toString();
+	}
+	return getObjectName() + "\n\n" + configuration.description.toString();
 }
 
 std::string CRewardableObject::getHoverText(const CGHeroInstance * hero) const
 {
 	if(configuration.visitMode != Rewardable::VISIT_UNLIMITED)
-		return getObjectName() + " " + visitedTxt(wasVisited(hero));
-	return getObjectName();
+	{
+		if (wasVisited(hero))
+			return getObjectName() + "\n" + configuration.visitedTooltip.toString() + "\n\n" + configuration.description.toString();
+		else
+			return getObjectName() + "\n" + configuration.notVisitedTooltip.toString() + "\n\n" + configuration.description.toString();
+	}
+	return getObjectName() + "\n\n" + configuration.description.toString();
 }
 
 void CRewardableObject::setPropertyDer(ui8 what, ui32 val)

lib/mapObjects/CRewardableObject.h

@@ -19,7 +19,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 class DLL_LINKAGE CRewardableObject : public CArmedInstance, public Rewardable::Interface
 {
 protected:
-	
+
 	bool onceVisitableObjectCleared = false;
 	
 	/// reward selected by player, no serialize
@@ -41,6 +41,9 @@ public:
 	/// Visitability checks. Note that hero check includes check for hero owner (returns true if object was visited by player)
 	bool wasVisited(PlayerColor player) const override;
 	bool wasVisited(const CGHeroInstance * h) const override;
+
+	/// Returns true if object was scouted by player and he is aware of its internal state
+	bool wasScouted(PlayerColor player) const;
 	
 	/// gives reward to player or ask for choice in case of multiple rewards
 	void onHeroVisit(const CGHeroInstance *h) const override;
@@ -74,10 +77,6 @@ public:
 //TODO:
 
 // MAX
-// class DLL_LINKAGE CGPandoraBox : public CArmedInstance
-// class DLL_LINKAGE CGEvent : public CGPandoraBox  //event objects
-// class DLL_LINKAGE CGSeerHut : public CArmedInstance, public IQuestObject //army is used when giving reward
-// class DLL_LINKAGE CGQuestGuard : public CGSeerHut
 // class DLL_LINKAGE CBank : public CArmedInstance
 // class DLL_LINKAGE CGPyramid : public CBank
 

lib/rewardable/Configuration.h

@@ -84,6 +84,10 @@ struct DLL_LINKAGE VisitInfo
 	/// Message that will be displayed on granting of this reward, if not empty
 	MetaString message;
 
+	/// Object description that will be shown on right-click, after object name
+	/// Used only after player have "scouted" object and knows internal state of an object
+	MetaString description;
+
 	/// Event to which this reward is assigned
 	EEventType visitType;
 
@@ -94,6 +98,7 @@ struct DLL_LINKAGE VisitInfo
 		h & limiter;
 		h & reward;
 		h & message;
+		h & description;
 		h & visitType;
 	}
 };
@@ -121,6 +126,16 @@ struct DLL_LINKAGE Configuration
 	/// Message that will be shown if player needs to select one of multiple rewards
 	MetaString onSelect;
 
+	/// Object description that will be shown on right-click, after object name
+	/// Used only if player is not aware of object internal state, e.g. have never visited it
+	MetaString description;
+
+	/// Text that will be shown if hero has not visited this object
+	MetaString notVisitedTooltip;
+
+	/// Text that will be shown after hero has visited this object
+	MetaString visitedTooltip;
+
 	/// Rewards that can be applied by an object
 	std::vector<Rewardable::VisitInfo> info;
 
@@ -161,6 +176,9 @@ struct DLL_LINKAGE Configuration
 		h & canRefuse;
 		h & resetParameters;
 		h & onSelect;
+		h & description;
+		h & notVisitedTooltip;
+		h & visitedTooltip;
 		h & visitMode;
 		h & selectMode;
 		h & infoWindowType;

lib/rewardable/Info.cpp

@@ -24,12 +24,25 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 namespace {
-	MetaString loadMessage(const JsonNode & value, const TextIdentifier & textIdentifier )
+	MetaString loadMessage(const JsonNode & value, const TextIdentifier & textIdentifier, EMetaText textSource = EMetaText::ADVOB_TXT )
 	{
 		MetaString ret;
+
+		if (value.isVector())
+		{
+			for(const auto & entry : value.Vector())
+			{
+				if (entry.isNumber())
+					ret.appendLocalString(textSource, static_cast<ui32>(entry.Float()));
+				if (entry.isString())
+					ret.appendRawString(entry.String());
+			}
+			return ret;
+		}
+
 		if (value.isNumber())
 		{
-			ret.appendLocalString(EMetaText::ADVOB_TXT, static_cast<ui32>(value.Float()));
+			ret.appendLocalString(textSource, static_cast<ui32>(value.Float()));
 			return ret;
 		}
 
@@ -81,6 +94,9 @@ void Rewardable::Info::init(const JsonNode & objectConfig, const std::string & o
 	}
 
 	loadString(parameters["onSelectMessage"], TextIdentifier(objectName, "onSelect"));
+	loadString(parameters["description"], TextIdentifier(objectName, "description"));
+	loadString(parameters["notVisitedTooltip"], TextIdentifier(objectName, "notVisitedText"));
+	loadString(parameters["visitedTooltip"], TextIdentifier(objectName, "visitedTooltip"));
 	loadString(parameters["onVisitedMessage"], TextIdentifier(objectName, "onVisited"));
 	loadString(parameters["onEmptyMessage"], TextIdentifier(objectName, "onEmpty"));
 }
@@ -277,6 +293,7 @@ void Rewardable::Info::configureRewards(
 
 		info.visitType = event;
 		info.message = loadMessage(reward["message"], TextIdentifier(objectTextID, modeName, i));
+		info.description = loadMessage(reward["description"], TextIdentifier(objectTextID, "description", modeName, i));
 
 		for (const auto & artifact : info.reward.artifacts )
 			info.message.replaceLocalString(EMetaText::ART_NAMES, artifact.getNum());
@@ -298,7 +315,16 @@ void Rewardable::Info::configureObject(Rewardable::Configuration & object, CRand
 	configureRewards(object, rng, parameters["onVisited"], Rewardable::EEventType::EVENT_ALREADY_VISITED, "onVisited");
 	configureRewards(object, rng, parameters["onEmpty"], Rewardable::EEventType::EVENT_NOT_AVAILABLE, "onEmpty");
 
-	object.onSelect   = loadMessage(parameters["onSelectMessage"], TextIdentifier(objectTextID, "onSelect"));
+	object.onSelect = loadMessage(parameters["onSelectMessage"], TextIdentifier(objectTextID, "onSelect"));
+	object.description = loadMessage(parameters["description"], TextIdentifier(objectTextID, "description"));
+	object.notVisitedTooltip = loadMessage(parameters["notVisitedTooltip"], TextIdentifier(objectTextID, "notVisitedTooltip"), EMetaText::GENERAL_TXT);
+	object.visitedTooltip = loadMessage(parameters["visitedTooltip"], TextIdentifier(objectTextID, "visitedTooltip"), EMetaText::GENERAL_TXT);
+
+	if (object.notVisitedTooltip.empty())
+		object.notVisitedTooltip.appendTextID("core.genrltxt.353");
+
+	if (object.visitedTooltip.empty())
+		object.visitedTooltip.appendTextID("core.genrltxt.352");
 
 	if (!parameters["onVisitedMessage"].isNull())
 	{