Merge pull request #4677 from IvanSavenko/rewardable_fixes

Unify rewardable map object and town building code

lib/mapObjects/CRewardableObject.cpp

@@ -12,99 +12,33 @@
 #include "CRewardableObject.h"
 
 #include "../CPlayerState.h"
-#include "../GameSettings.h"
 #include "../IGameCallback.h"
+#include "../IGameSettings.h"
 #include "../battle/BattleLayout.h"
 #include "../gameState/CGameState.h"
 #include "../mapObjectConstructors/AObjectTypeHandler.h"
-#include "../mapObjectConstructors/CObjectClassesHandler.h"
 #include "../mapObjectConstructors/CRewardableConstructor.h"
 #include "../mapObjects/CGHeroInstance.h"
 #include "../networkPacks/PacksForClient.h"
 #include "../networkPacks/PacksForClientBattle.h"
 #include "../serializer/JsonSerializeFormat.h"
-#include "../texts/CGeneralTextHandler.h"
 
 #include <vstd/RNG.h>
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-void CRewardableObject::grantRewardWithMessage(const CGHeroInstance * contextHero, int index, bool markAsVisit) const
+const IObjectInterface * CRewardableObject::getObject() const
 {
-	auto vi = configuration.info.at(index);
-	logGlobal->debug("Granting reward %d. Message says: %s", index, vi.message.toString());
-	// show message only if it is not empty or in infobox
-	if (configuration.infoWindowType != EInfoWindowMode::MODAL || !vi.message.toString().empty())
-	{
-		InfoWindow iw;
-		iw.player = contextHero->tempOwner;
-		iw.text = vi.message;
-		vi.reward.loadComponents(iw.components, contextHero);
-		iw.type = configuration.infoWindowType;
-		if(!iw.components.empty() || !iw.text.toString().empty())
-			cb->showInfoDialog(&iw);
-	}
-	// grant reward afterwards. Note that it may remove object
-	if(markAsVisit)
-		markAsVisited(contextHero);
-	grantReward(index, contextHero);
+	return this;
 }
 
-void CRewardableObject::selectRewardWithMessage(const CGHeroInstance * contextHero, const std::vector<ui32> & rewardIndices, const MetaString & dialog) const
+void CRewardableObject::markAsScouted(const CGHeroInstance * hero) const
 {
-	BlockingDialog sd(configuration.canRefuse, rewardIndices.size() > 1);
-	sd.player = contextHero->tempOwner;
-	sd.text = dialog;
-	sd.components = loadComponents(contextHero, rewardIndices);
-	cb->showBlockingDialog(this, &sd);
-
+	ChangeObjectVisitors cov(ChangeObjectVisitors::VISITOR_ADD_PLAYER, id, hero->id);
+	cb->sendAndApply(&cov);
 }
 
-void CRewardableObject::grantAllRewardsWithMessage(const CGHeroInstance * contextHero, const std::vector<ui32> & rewardIndices, bool markAsVisit) const
-{
-	if (rewardIndices.empty())
-		return;
-		
-	for (auto index : rewardIndices)
-	{
-		// TODO: Merge all rewards of same type, with single message?
-		grantRewardWithMessage(contextHero, index, false);
-	}
-	// Mark visited only after all rewards were processed
-	if(markAsVisit)
-		markAsVisited(contextHero);
-}
-
-std::vector<Component> CRewardableObject::loadComponents(const CGHeroInstance * contextHero, const std::vector<ui32> & rewardIndices) const
-{
-	std::vector<Component> result;
-
-	if (rewardIndices.empty())
-		return result;
-
-	if (configuration.selectMode != Rewardable::SELECT_FIRST && rewardIndices.size() > 1)
-	{
-		for (auto index : rewardIndices)
-			result.push_back(configuration.info.at(index).reward.getDisplayedComponent(contextHero));
-	}
-	else
-	{
-		configuration.info.at(rewardIndices.front()).reward.loadComponents(result, contextHero);
-	}
-
-	return result;
-}
-
-bool CRewardableObject::guardedPotentially() const
-{
-	for (auto const & visitInfo : configuration.info)
-		if (!visitInfo.reward.guards.empty())
-			return true;
-
-	return false;
-}
-
-bool CRewardableObject::guardedPresently() const
+bool CRewardableObject::isGuarded() const
 {
 	return stacksCount() > 0;
 }
@@ -117,7 +51,7 @@ void CRewardableObject::onHeroVisit(const CGHeroInstance *hero) const
 		cb->sendAndApply(&cov);
 	}
 
-	if (guardedPresently())
+	if (isGuarded())
 	{
 		auto guardedIndexes = getAvailableRewards(hero, Rewardable::EEventType::EVENT_GUARDED);
 		auto guardedReward = configuration.info.at(guardedIndexes.at(0));
@@ -136,94 +70,9 @@ void CRewardableObject::onHeroVisit(const CGHeroInstance *hero) const
 	}
 }
 
-void CRewardableObject::doHeroVisit(const CGHeroInstance *h) const
-{
-	if(!wasVisitedBefore(h))
-	{
-		auto rewards = getAvailableRewards(h, Rewardable::EEventType::EVENT_FIRST_VISIT);
-		bool objectRemovalPossible = false;
-		for(auto index : rewards)
-		{
-			if(configuration.info.at(index).reward.removeObject)
-				objectRemovalPossible = true;
-		}
-
-		logGlobal->debug("Visiting object with %d possible rewards", rewards.size());
-		switch (rewards.size())
-		{
-			case 0: // no available rewards, e.g. visiting School of War without gold
-			{
-				auto emptyRewards = getAvailableRewards(h, Rewardable::EEventType::EVENT_NOT_AVAILABLE);
-				if (!emptyRewards.empty())
-					grantRewardWithMessage(h, emptyRewards[0], false);
-				else
-					logMod->warn("No applicable message for visiting empty object!");
-				break;
-			}
-			case 1: // one reward. Just give it with message
-			{
-				if (configuration.canRefuse)
-					selectRewardWithMessage(h, rewards, configuration.info.at(rewards.front()).message);
-				else
-					grantRewardWithMessage(h, rewards.front(), true);
-				break;
-			}
-			default: // multiple rewards. Act according to select mode
-			{
-				switch (configuration.selectMode) {
-					case Rewardable::SELECT_PLAYER: // player must select
-						selectRewardWithMessage(h, rewards, configuration.onSelect);
-						break;
-					case Rewardable::SELECT_FIRST: // give first available
-						if (configuration.canRefuse)
-							selectRewardWithMessage(h, { rewards.front() }, configuration.info.at(rewards.front()).message);
-						else
-							grantRewardWithMessage(h, rewards.front(), true);
-						break;
-					case Rewardable::SELECT_RANDOM: // give random
-					{
-						ui32 rewardIndex = *RandomGeneratorUtil::nextItem(rewards, cb->gameState()->getRandomGenerator());
-						if (configuration.canRefuse)
-							selectRewardWithMessage(h, { rewardIndex }, configuration.info.at(rewardIndex).message);
-						else
-							grantRewardWithMessage(h, rewardIndex, true);
-						break;
-					}
-					case Rewardable::SELECT_ALL: // grant all possible
-						grantAllRewardsWithMessage(h, rewards, true);
-						break;
-				}
-				break;
-			}
-		}
-
-		if(!objectRemovalPossible && getAvailableRewards(h, Rewardable::EEventType::EVENT_FIRST_VISIT).empty())
-		{
-			ChangeObjectVisitors cov(ChangeObjectVisitors::VISITOR_ADD_PLAYER, id, h->id);
-			cb->sendAndApply(&cov);
-		}
-	}
-	else
-	{
-		logGlobal->debug("Revisiting already visited object");
-
-		if (!wasVisited(h->getOwner()))
-		{
-			ChangeObjectVisitors cov(ChangeObjectVisitors::VISITOR_ADD_PLAYER, id, h->id);
-			cb->sendAndApply(&cov);
-		}
-
-		auto visitedRewards = getAvailableRewards(h, Rewardable::EEventType::EVENT_ALREADY_VISITED);
-		if (!visitedRewards.empty())
-			grantRewardWithMessage(h, visitedRewards[0], false);
-		else
-			logMod->warn("No applicable message for visiting already visited object!");
-	}
-}
-
 void CRewardableObject::heroLevelUpDone(const CGHeroInstance *hero) const
 {
-	grantRewardAfterLevelup(cb, configuration.info.at(selectedReward), this, hero);
+	grantRewardAfterLevelup(configuration.info.at(selectedReward), this, hero);
 }
 
 void CRewardableObject::battleFinished(const CGHeroInstance *hero, const BattleResult &result) const
@@ -236,7 +85,7 @@ void CRewardableObject::battleFinished(const CGHeroInstance *hero, const BattleR
 
 void CRewardableObject::blockingDialogAnswered(const CGHeroInstance * hero, int32_t answer) const
 {
-	if(guardedPresently())
+	if(isGuarded())
 	{
 		if (answer)
 		{
@@ -273,12 +122,12 @@ void CRewardableObject::markAsVisited(const CGHeroInstance * hero) const
 void CRewardableObject::grantReward(ui32 rewardID, const CGHeroInstance * hero) const
 {
 	cb->setObjPropertyValue(id, ObjProperty::REWARD_SELECT, rewardID);
-	grantRewardBeforeLevelup(cb, configuration.info.at(rewardID), hero);
+	grantRewardBeforeLevelup(configuration.info.at(rewardID), hero);
 	
 	// hero is not blocked by levelup dialog - grant remainder immediately
 	if(!cb->isVisitCoveredByAnotherQuery(this, hero))
 	{
-		grantRewardAfterLevelup(cb, configuration.info.at(rewardID), this, hero);
+		grantRewardAfterLevelup(configuration.info.at(rewardID), this, hero);
 	}
 }
 
@@ -410,7 +259,7 @@ std::vector<Component> CRewardableObject::getPopupComponentsImpl(PlayerColor pla
 	if (!wasScouted(player))
 		return {};
 
-	if (guardedPresently())
+	if (isGuarded())
 	{
 		if (!cb->getSettings().getBoolean(EGameSettings::BANKS_SHOW_GUARDS_COMPOSITION))
 			return {};

lib/mapObjects/CRewardableObject.h

@@ -25,32 +25,24 @@ protected:
 	/// reward selected by player, no serialize
 	ui16 selectedReward = 0;
 	
-	void grantReward(ui32 rewardID, const CGHeroInstance * hero) const;
-	void markAsVisited(const CGHeroInstance * hero) const;
+	void grantReward(ui32 rewardID, const CGHeroInstance * hero) const override;
+	void markAsVisited(const CGHeroInstance * hero) const override;
+
+	const IObjectInterface * getObject() const override;
+	void markAsScouted(const CGHeroInstance * hero) const override;
 	
 	/// return true if this object was "cleared" before and no longer has rewards applicable to selected hero
 	/// unlike wasVisited, this method uses information not available to player owner, for example, if object was cleared by another player before
-	bool wasVisitedBefore(const CGHeroInstance * contextHero) const;
+	bool wasVisitedBefore(const CGHeroInstance * contextHero) const override;
 	
 	void serializeJsonOptions(JsonSerializeFormat & handler) override;
 	
-	virtual void grantRewardWithMessage(const CGHeroInstance * contextHero, int rewardIndex, bool markAsVisit) const;
-	virtual void selectRewardWithMessage(const CGHeroInstance * contextHero, const std::vector<ui32> & rewardIndices, const MetaString & dialog) const;
-
-	virtual void grantAllRewardsWithMessage(const CGHeroInstance * contextHero, const std::vector<ui32>& rewardIndices, bool markAsVisit) const;
-
-	std::vector<Component> loadComponents(const CGHeroInstance * contextHero, const std::vector<ui32> & rewardIndices) const;
-
 	std::string getDisplayTextImpl(PlayerColor player, const CGHeroInstance * hero, bool includeDescription) const;
 	std::string getDescriptionMessage(PlayerColor player, const CGHeroInstance * hero) const;
 	std::vector<Component> getPopupComponentsImpl(PlayerColor player, const CGHeroInstance * hero) const;
 
-	void doHeroVisit(const CGHeroInstance *h) const;
-
-	/// Returns true if this object might have guards present, whether they were cleared or not
-	bool guardedPotentially() const;
 	/// Returns true if this object is currently guarded
-	bool guardedPresently() const;
+	bool isGuarded() const;
 public:
 
 	/// Visitability checks. Note that hero check includes check for hero owner (returns true if object was visited by player)

lib/mapObjects/TownBuildingInstance.cpp

@@ -12,14 +12,10 @@
 #include "TownBuildingInstance.h"
 
 #include "CGTownInstance.h"
-#include "../texts/CGeneralTextHandler.h"
 #include "../IGameCallback.h"
-#include "../gameState/CGameState.h"
 #include "../mapObjects/CGHeroInstance.h"
-#include "../networkPacks/PacksForClient.h"
 #include "../entities/building/CBuilding.h"
 
-
 #include <vstd/RNG.h>
 
 VCMI_LIB_NAMESPACE_BEGIN
@@ -130,7 +126,7 @@ void TownRewardableBuildingInstance::setProperty(ObjProperty what, ObjPropertyID
 
 void TownRewardableBuildingInstance::heroLevelUpDone(const CGHeroInstance *hero) const
 {
-	grantRewardAfterLevelup(cb, configuration.info.at(selectedReward), town, hero);
+	grantRewardAfterLevelup(configuration.info.at(selectedReward), town, hero);
 }
 
 void TownRewardableBuildingInstance::blockingDialogAnswered(const CGHeroInstance *hero, int32_t answer) const
@@ -154,14 +150,12 @@ void TownRewardableBuildingInstance::blockingDialogAnswered(const CGHeroInstance
 
 void TownRewardableBuildingInstance::grantReward(ui32 rewardID, const CGHeroInstance * hero) const
 {
-	town->addHeroToStructureVisitors(hero, getBuildingType());
-	
-	grantRewardBeforeLevelup(cb, configuration.info.at(rewardID), hero);
+	grantRewardBeforeLevelup(configuration.info.at(rewardID), hero);
 	
 	// hero is not blocked by levelup dialog - grant remainder immediately
 	if(!cb->isVisitCoveredByAnotherQuery(town, hero))
 	{
-		grantRewardAfterLevelup(cb, configuration.info.at(rewardID), town, hero);
+		grantRewardAfterLevelup(configuration.info.at(rewardID), town, hero);
 	}
 }
 
@@ -196,93 +190,42 @@ bool TownRewardableBuildingInstance::wasVisitedBefore(const CGHeroInstance * con
 
 void TownRewardableBuildingInstance::onHeroVisit(const CGHeroInstance *h) const
 {
-	auto grantRewardWithMessage = [&](int index) -> void
+	assert(town->hasBuilt(getBuildingType()));
+
+	if(town->hasBuilt(getBuildingType()))
+		doHeroVisit(h);
+}
+
+const IObjectInterface * TownRewardableBuildingInstance::getObject() const
+{
+	return this;
+}
+
+bool TownRewardableBuildingInstance::wasVisited(PlayerColor player) const
+{
+	switch (configuration.visitMode)
 	{
-		auto vi = configuration.info.at(index);
-		logGlobal->debug("Granting reward %d. Message says: %s", index, vi.message.toString());
-		
-		town->addHeroToStructureVisitors(h, getBuildingType()); //adding to visitors
-
-		InfoWindow iw;
-		iw.player = h->tempOwner;
-		iw.text = vi.message;
-		vi.reward.loadComponents(iw.components, h);
-		iw.type = EInfoWindowMode::MODAL;
-		if(!iw.components.empty() || !iw.text.toString().empty())
-			cb->showInfoDialog(&iw);
-		
-		grantReward(index, h);
-	};
-	auto selectRewardsMessage = [&](const std::vector<ui32> & rewards, const MetaString & dialog) -> void
-	{
-		BlockingDialog sd(configuration.canRefuse, rewards.size() > 1);
-		sd.player = h->tempOwner;
-		sd.text = dialog;
-
-		if (rewards.size() > 1)
-			for (auto index : rewards)
-				sd.components.push_back(configuration.info.at(index).reward.getDisplayedComponent(h));
-
-		if (rewards.size() == 1)
-			configuration.info.at(rewards.front()).reward.loadComponents(sd.components, h);
-
-		cb->showBlockingDialog(this, &sd);
-	};
-	
-	if(!town->hasBuilt(getBuildingType()))
-		return;
-
-	if(!wasVisitedBefore(h))
-	{
-		auto rewards = getAvailableRewards(h, Rewardable::EEventType::EVENT_FIRST_VISIT);
-
-		logGlobal->debug("Visiting object with %d possible rewards", rewards.size());
-		switch (rewards.size())
-		{
-			case 0: // no available rewards, e.g. visiting School of War without gold
-			{
-				auto emptyRewards = getAvailableRewards(h, Rewardable::EEventType::EVENT_NOT_AVAILABLE);
-				if (!emptyRewards.empty())
-					grantRewardWithMessage(emptyRewards[0]);
-				else
-					logMod->warn("No applicable message for visiting empty object!");
-				break;
-			}
-			case 1: // one reward. Just give it with message
-			{
-				if (configuration.canRefuse)
-					selectRewardsMessage(rewards, configuration.info.at(rewards.front()).message);
-				else
-					grantRewardWithMessage(rewards.front());
-				break;
-			}
-			default: // multiple rewards. Act according to select mode
-			{
-				switch (configuration.selectMode) {
-					case Rewardable::SELECT_PLAYER: // player must select
-						selectRewardsMessage(rewards, configuration.onSelect);
-						break;
-					case Rewardable::SELECT_FIRST: // give first available
-						grantRewardWithMessage(rewards.front());
-						break;
-					case Rewardable::SELECT_RANDOM: // give random
-						grantRewardWithMessage(*RandomGeneratorUtil::nextItem(rewards, cb->gameState()->getRandomGenerator()));
-						break;
-				}
-				break;
-			}
-		}
+		case Rewardable::VISIT_UNLIMITED:
+		case Rewardable::VISIT_BONUS:
+		case Rewardable::VISIT_HERO:
+		case Rewardable::VISIT_LIMITER:
+			return false;
+		case Rewardable::VISIT_ONCE:
+		case Rewardable::VISIT_PLAYER:
+			return !visitors.empty();
+		default:
+			return false;
 	}
-	else
-	{
-		logGlobal->debug("Revisiting already visited object");
+}
 
-		auto visitedRewards = getAvailableRewards(h, Rewardable::EEventType::EVENT_ALREADY_VISITED);
-		if (!visitedRewards.empty())
-			grantRewardWithMessage(visitedRewards[0]);
-		else
-			logMod->debug("No applicable message for visiting already visited object!");
-	}
+void TownRewardableBuildingInstance::markAsVisited(const CGHeroInstance * hero) const
+{
+	town->addHeroToStructureVisitors(hero, getBuildingType());
+}
+
+void TownRewardableBuildingInstance::markAsScouted(const CGHeroInstance * hero) const
+{
+	// no-op - town building is always 'scouted' by owner
 }
 
 

lib/mapObjects/TownBuildingInstance.h

@@ -63,10 +63,14 @@ class DLL_LINKAGE TownRewardableBuildingInstance : public TownBuildingInstance,
 	ui16 selectedReward = 0;
 	std::set<ObjectInstanceID> visitors;
 
-	bool wasVisitedBefore(const CGHeroInstance * contextHero) const;
-	void grantReward(ui32 rewardID, const CGHeroInstance * hero) const;
+	bool wasVisitedBefore(const CGHeroInstance * contextHero) const override;
+	void grantReward(ui32 rewardID, const CGHeroInstance * hero) const override;
 	Rewardable::Configuration generateConfiguration(vstd::RNG & rand) const;
 
+	const IObjectInterface * getObject() const override;
+	bool wasVisited(PlayerColor player) const override;
+	void markAsVisited(const CGHeroInstance * hero) const override;
+	void markAsScouted(const CGHeroInstance * hero) const override;
 public:
 	void setProperty(ObjProperty what, ObjPropertyID identifier) override;
 	void onHeroVisit(const CGHeroInstance * h) const override;

lib/rewardable/Interface.cpp

@@ -25,6 +25,8 @@
 #include "../networkPacks/PacksForClient.h"
 #include "../IGameCallback.h"
 
+#include <vstd/RNG.h>
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 std::vector<ui32> Rewardable::Interface::getAvailableRewards(const CGHeroInstance * hero, Rewardable::EEventType event) const
@@ -44,8 +46,10 @@ std::vector<ui32> Rewardable::Interface::getAvailableRewards(const CGHeroInstanc
 	return ret;
 }
 
-void Rewardable::Interface::grantRewardBeforeLevelup(IGameCallback * cb, const Rewardable::VisitInfo & info, const CGHeroInstance * hero) const
+void Rewardable::Interface::grantRewardBeforeLevelup(const Rewardable::VisitInfo & info, const CGHeroInstance * hero) const
 {
+	auto cb = getObject()->cb;
+
 	assert(hero);
 	assert(hero->tempOwner.isValidPlayer());
 	assert(info.reward.creatures.size() <= GameConstants::ARMY_SIZE);
@@ -129,8 +133,10 @@ void Rewardable::Interface::grantRewardBeforeLevelup(IGameCallback * cb, const R
 		cb->giveExperience(hero, expToGive);
 }
 
-void Rewardable::Interface::grantRewardAfterLevelup(IGameCallback * cb, const Rewardable::VisitInfo & info, const CArmedInstance * army, const CGHeroInstance * hero) const
+void Rewardable::Interface::grantRewardAfterLevelup(const Rewardable::VisitInfo & info, const CArmedInstance * army, const CGHeroInstance * hero) const
 {
+	auto cb = getObject()->cb;
+
 	if(info.reward.manaDiff || info.reward.manaPercentage >= 0)
 		cb->setManaPoints(hero->id, info.reward.calculateManaPoints(hero));
 
@@ -216,4 +222,148 @@ void Rewardable::Interface::serializeJson(JsonSerializeFormat & handler)
 	configuration.serializeJson(handler);
 }
 
+void Rewardable::Interface::grantRewardWithMessage(const CGHeroInstance * contextHero, int index, bool markAsVisit) const
+{
+	auto vi = configuration.info.at(index);
+	logGlobal->debug("Granting reward %d. Message says: %s", index, vi.message.toString());
+	// show message only if it is not empty or in infobox
+	if (configuration.infoWindowType != EInfoWindowMode::MODAL || !vi.message.toString().empty())
+	{
+		InfoWindow iw;
+		iw.player = contextHero->tempOwner;
+		iw.text = vi.message;
+		vi.reward.loadComponents(iw.components, contextHero);
+		iw.type = configuration.infoWindowType;
+		if(!iw.components.empty() || !iw.text.toString().empty())
+			getObject()->cb->showInfoDialog(&iw);
+	}
+	// grant reward afterwards. Note that it may remove object
+	if(markAsVisit)
+		markAsVisited(contextHero);
+	grantReward(index, contextHero);
+}
+
+void Rewardable::Interface::selectRewardWithMessage(const CGHeroInstance * contextHero, const std::vector<ui32> & rewardIndices, const MetaString & dialog) const
+{
+	BlockingDialog sd(configuration.canRefuse, rewardIndices.size() > 1);
+	sd.player = contextHero->tempOwner;
+	sd.text = dialog;
+	sd.components = loadComponents(contextHero, rewardIndices);
+	getObject()->cb->showBlockingDialog(getObject(), &sd);
+}
+
+std::vector<Component> Rewardable::Interface::loadComponents(const CGHeroInstance * contextHero, const std::vector<ui32> & rewardIndices) const
+{
+	std::vector<Component> result;
+
+	if (rewardIndices.empty())
+		return result;
+
+	if (configuration.selectMode != Rewardable::SELECT_FIRST && rewardIndices.size() > 1)
+	{
+		for (auto index : rewardIndices)
+			result.push_back(configuration.info.at(index).reward.getDisplayedComponent(contextHero));
+	}
+	else
+	{
+		configuration.info.at(rewardIndices.front()).reward.loadComponents(result, contextHero);
+	}
+
+	return result;
+}
+
+void Rewardable::Interface::grantAllRewardsWithMessage(const CGHeroInstance * contextHero, const std::vector<ui32> & rewardIndices, bool markAsVisit) const
+{
+	if (rewardIndices.empty())
+		return;
+
+	for (auto index : rewardIndices)
+	{
+		// TODO: Merge all rewards of same type, with single message?
+		grantRewardWithMessage(contextHero, index, false);
+	}
+	// Mark visited only after all rewards were processed
+	if(markAsVisit)
+		markAsVisited(contextHero);
+}
+
+void Rewardable::Interface::doHeroVisit(const CGHeroInstance *h) const
+{
+	if(!wasVisitedBefore(h))
+	{
+		auto rewards = getAvailableRewards(h, Rewardable::EEventType::EVENT_FIRST_VISIT);
+		bool objectRemovalPossible = false;
+		for(auto index : rewards)
+		{
+			if(configuration.info.at(index).reward.removeObject)
+				objectRemovalPossible = true;
+		}
+
+		logGlobal->debug("Visiting object with %d possible rewards", rewards.size());
+		switch (rewards.size())
+		{
+			case 0: // no available rewards, e.g. visiting School of War without gold
+			{
+				auto emptyRewards = getAvailableRewards(h, Rewardable::EEventType::EVENT_NOT_AVAILABLE);
+				if (!emptyRewards.empty())
+					grantRewardWithMessage(h, emptyRewards[0], false);
+				else
+					logMod->warn("No applicable message for visiting empty object!");
+				break;
+			}
+			case 1: // one reward. Just give it with message
+			{
+				if (configuration.canRefuse)
+					selectRewardWithMessage(h, rewards, configuration.info.at(rewards.front()).message);
+				else
+					grantRewardWithMessage(h, rewards.front(), true);
+				break;
+			}
+			default: // multiple rewards. Act according to select mode
+			{
+				switch (configuration.selectMode) {
+					case Rewardable::SELECT_PLAYER: // player must select
+						selectRewardWithMessage(h, rewards, configuration.onSelect);
+						break;
+					case Rewardable::SELECT_FIRST: // give first available
+						if (configuration.canRefuse)
+							selectRewardWithMessage(h, { rewards.front() }, configuration.info.at(rewards.front()).message);
+						else
+							grantRewardWithMessage(h, rewards.front(), true);
+						break;
+					case Rewardable::SELECT_RANDOM: // give random
+					{
+						ui32 rewardIndex = *RandomGeneratorUtil::nextItem(rewards, getObject()->cb->getRandomGenerator());
+						if (configuration.canRefuse)
+							selectRewardWithMessage(h, { rewardIndex }, configuration.info.at(rewardIndex).message);
+						else
+							grantRewardWithMessage(h, rewardIndex, true);
+						break;
+					}
+					case Rewardable::SELECT_ALL: // grant all possible
+						grantAllRewardsWithMessage(h, rewards, true);
+						break;
+				}
+				break;
+			}
+		}
+
+		if(!objectRemovalPossible && getAvailableRewards(h, Rewardable::EEventType::EVENT_FIRST_VISIT).empty())
+			markAsScouted(h);
+	}
+	else
+	{
+		logGlobal->debug("Revisiting already visited object");
+
+		if (!wasVisited(h->getOwner()))
+			markAsScouted(h);
+
+		auto visitedRewards = getAvailableRewards(h, Rewardable::EEventType::EVENT_ALREADY_VISITED);
+		if (!visitedRewards.empty())
+			grantRewardWithMessage(h, visitedRewards[0], false);
+		else
+			logMod->warn("No applicable message for visiting already visited object!");
+	}
+}
+
 VCMI_LIB_NAMESPACE_END

lib/rewardable/Interface.h

@@ -15,7 +15,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-class IGameCallback;
+class IObjectInterface;
 
 namespace Rewardable
 {
@@ -30,11 +30,24 @@ private:
 protected:
 	
 	/// function that must be called if hero got level-up during grantReward call
-	virtual void grantRewardAfterLevelup(IGameCallback * cb, const Rewardable::VisitInfo & reward, const CArmedInstance * army, const CGHeroInstance * hero) const;
+	void grantRewardAfterLevelup(const Rewardable::VisitInfo & reward, const CArmedInstance * army, const CGHeroInstance * hero) const;
 
 	/// grants reward to hero
-	virtual void grantRewardBeforeLevelup(IGameCallback * cb, const Rewardable::VisitInfo & reward, const CGHeroInstance * hero) const;
+	void grantRewardBeforeLevelup(const Rewardable::VisitInfo & reward, const CGHeroInstance * hero) const;
 	
+	virtual void grantRewardWithMessage(const CGHeroInstance * contextHero, int rewardIndex, bool markAsVisit) const;
+	void selectRewardWithMessage(const CGHeroInstance * contextHero, const std::vector<ui32> & rewardIndices, const MetaString & dialog) const;
+	void grantAllRewardsWithMessage(const CGHeroInstance * contextHero, const std::vector<ui32>& rewardIndices, bool markAsVisit) const;
+	std::vector<Component> loadComponents(const CGHeroInstance * contextHero, const std::vector<ui32> & rewardIndices) const;
+
+	void doHeroVisit(const CGHeroInstance *h) const;
+
+	virtual const IObjectInterface * getObject() const = 0;
+	virtual bool wasVisitedBefore(const CGHeroInstance * hero) const = 0;
+	virtual bool wasVisited(PlayerColor player) const = 0;
+	virtual void markAsVisited(const CGHeroInstance * hero) const = 0;
+	virtual void markAsScouted(const CGHeroInstance * hero) const = 0;
+	virtual void grantReward(ui32 rewardID, const CGHeroInstance * hero) const = 0;
 public:
 
 	/// filters list of visit info and returns rewards that can be granted to current hero

server/CGameHandler.cpp

@@ -1194,7 +1194,7 @@ void CGameHandler::visitCastleObjects(const CGTownInstance * t, std::vector<cons
 
 	for (auto & building : t->rewardableBuildings)
 	{
-		if (!t->town->buildings.at(building.first)->manualHeroVisit)
+		if (!t->town->buildings.at(building.first)->manualHeroVisit && t->hasBuilt(building.first))
 			buildingsToVisit.push_back(building.first);
 	}