Moved creature upgrade logic to CGObjectInstance inheritors

config/objects/generic.json

@@ -842,7 +842,7 @@
 	/// Classes without dedicated object
 	"hillFort" : {
 		"index" :35,
-		"handler": "generic",
+		"handler": "hillFort",
 		"base" : {
 			"sounds" : {
 				"ambient" : ["LOOPSWAR"],

lib/CGameState.cpp

@@ -2018,54 +2018,28 @@ UpgradeInfo CGameState::fillUpgradeInfo(const CStackInstance &stack) const
 	UpgradeInfo ret;
 	const CCreature *base = stack.type;
 
-	const CGHeroInstance * h = stack.armyObj->ID == Obj::HERO ? dynamic_cast<const CGHeroInstance *>(stack.armyObj) : nullptr;
-	const CGTownInstance *t = nullptr;
+	if (stack.armyObj->ID == Obj::HERO)
+	{
+		auto hero = dynamic_cast<const CGHeroInstance *>(stack.armyObj);
+		hero->fillUpgradeInfo(ret, stack);
 
-	if(stack.armyObj->ID == Obj::TOWN)
-		t = dynamic_cast<const CGTownInstance *>(stack.armyObj);
-	else if(h)
-	{	//hero specialty
-		TConstBonusListPtr lista = h->getBonuses(Selector::typeSubtype(BonusType::SPECIAL_UPGRADE, base->getId()));
-		for(const auto & it : *lista)
+		if (hero->visitedTown)
 		{
-			auto nid = CreatureID(it->additionalInfo[0]);
-			if (nid != base->getId()) //in very specific case the upgrade is available by default (?)
-			{
-				ret.newID.push_back(nid);
-				ret.cost.push_back(nid.toCreature()->getFullRecruitCost() - base->getFullRecruitCost());
-			}
+			hero->visitedTown->fillUpgradeInfo(ret, stack);
 		}
-		t = h->visitedTown;
-	}
-	if(t)
-	{
-		for(const CGTownInstance::TCreaturesSet::value_type & dwelling : t->creatures)
+		else
 		{
-			if (vstd::contains(dwelling.second, base->getId())) //Dwelling with our creature
-			{
-				for(const auto & upgrID : dwelling.second)
-				{
-					if(vstd::contains(base->upgrades, upgrID)) //possible upgrade
-					{
-						ret.newID.push_back(upgrID);
-						ret.cost.push_back(upgrID.toCreature()->getFullRecruitCost() - base->getFullRecruitCost());
-					}
-				}
-			}
+			auto object = vstd::frontOrNull(getVisitableObjs(hero->visitablePos()));
+			auto upgradeSource = dynamic_cast<const ICreatureUpgrader*>(object);
+			if (object != hero && upgradeSource != nullptr)
+				upgradeSource->fillUpgradeInfo(ret, stack);
 		}
 	}
 
-	//hero is visiting Hill Fort
-	if(h && map->getTile(h->visitablePos()).visitableObjects.front()->ID == Obj::HILL_FORT)
+	if (stack.armyObj->ID == Obj::TOWN)
 	{
-		static const int costModifiers[] = {0, 25, 50, 75, 100}; //we get cheaper upgrades depending on level
-		const int costModifier = costModifiers[std::min<int>(std::max((int)base->getLevel() - 1, 0), std::size(costModifiers) - 1)];
-
-		for(const auto & nid : base->upgrades)
-		{
-			ret.newID.push_back(nid);
-			ret.cost.push_back((nid.toCreature()->getFullRecruitCost() - base->getFullRecruitCost()) * costModifier / 100);
-		}
+		auto town = dynamic_cast<const CGTownInstance *>(stack.armyObj);
+		town->fillUpgradeInfo(ret, stack);
 	}
 
 	if(!ret.newID.empty())

lib/mapObjectConstructors/CObjectClassesHandler.cpp

@@ -87,6 +87,7 @@ CObjectClassesHandler::CObjectClassesHandler()
 	SET_HANDLER("whirlpool", CGWhirlpool);
 	SET_HANDLER("witch", CGWitchHut);
 	SET_HANDLER("terrain", CGTerrainPatch);
+	SET_HANDLER("hillFort", HillFort);
 
 #undef SET_HANDLER_CLASS
 #undef SET_HANDLER

lib/mapObjects/CGHeroInstance.cpp

@@ -1715,4 +1715,18 @@ bool CGHeroInstance::isMissionCritical() const
 	return false;
 }
 
+void CGHeroInstance::fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack) const
+{
+	TConstBonusListPtr lista = getBonuses(Selector::typeSubtype(BonusType::SPECIAL_UPGRADE, stack.type->getId()));
+	for(const auto & it : *lista)
+	{
+		auto nid = CreatureID(it->additionalInfo[0]);
+		if (nid != stack.type->getId()) //in very specific case the upgrade is available by default (?)
+		{
+			info.newID.push_back(nid);
+			info.cost.push_back(nid.toCreature()->getFullRecruitCost() - stack.type->getFullRecruitCost());
+		}
+	}
+}
+
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/CGHeroInstance.h

@@ -39,7 +39,7 @@ public:
 };
 
 
-class DLL_LINKAGE CGHeroInstance : public CArmedInstance, public IBoatGenerator, public CArtifactSet, public spells::Caster, public AFactionMember
+class DLL_LINKAGE CGHeroInstance : public CArmedInstance, public IBoatGenerator, public CArtifactSet, public spells::Caster, public AFactionMember, public ICreatureUpgrader
 {
 	// We serialize heroes into JSON for crossover
 	friend class CCampaignState;
@@ -230,6 +230,8 @@ public:
 	void recreateSecondarySkillsBonuses();
 	void updateSkillBonus(const SecondarySkill & which, int val);
 
+	void fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack) const override;
+
 	bool hasVisions(const CGObjectInstance * target, const int subtype) const;
 	/// If this hero perishes, the scenario is failed
 	bool isMissionCritical() const;

lib/mapObjects/CGObjectInstance.cpp

@@ -254,11 +254,6 @@ void CGObjectInstance::onHeroVisit( const CGHeroInstance * h ) const
 {
 	switch(ID)
 	{
-	case Obj::HILL_FORT:
-		{
-			openWindow(EOpenWindowMode::HILL_FORT_WINDOW,id.getNum(),h->id.getNum());
-		}
-		break;
 	case Obj::SANCTUARY:
 		{
 			//You enter the sanctuary and immediately feel as if a great weight has been lifted off your shoulders.  You feel safe here.

lib/mapObjects/CGTownInstance.cpp

@@ -1242,5 +1242,22 @@ int GrowthInfo::totalGrowth() const
 	return ret;
 }
 
+void CGTownInstance::fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack) const
+{
+	for(const CGTownInstance::TCreaturesSet::value_type & dwelling : creatures)
+	{
+		if (vstd::contains(dwelling.second, stack.type->getId())) //Dwelling with our creature
+		{
+			for(const auto & upgrID : dwelling.second)
+			{
+				if(vstd::contains(stack.type->upgrades, upgrID)) //possible upgrade
+				{
+					info.newID.push_back(upgrID);
+					info.cost.push_back(upgrID.toCreature()->getFullRecruitCost() - stack.type->getFullRecruitCost());
+				}
+			}
+		}
+	}
+}
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/CGTownInstance.h

@@ -42,7 +42,7 @@ struct DLL_LINKAGE GrowthInfo
 	int totalGrowth() const;
 };
 
-class DLL_LINKAGE CGTownInstance : public CGDwelling, public IShipyard, public IMarket, public INativeTerrainProvider
+class DLL_LINKAGE CGTownInstance : public CGDwelling, public IShipyard, public IMarket, public INativeTerrainProvider, public ICreatureUpgrader
 {
 	std::string name; // name of town
 public:
@@ -197,6 +197,8 @@ public:
 	void battleFinished(const CGHeroInstance * hero, const BattleResult & result) const override;
 	std::string getObjectName() const override;
 
+	void fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack) const override;
+
 	void afterAddToMap(CMap * map) override;
 	void afterRemoveFromMap(CMap * map) override;
 	static void reset();

lib/mapObjects/IObjectInterface.h

@@ -14,6 +14,7 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 struct BattleResult;
+struct UpgradeInfo;
 class CGObjectInstance;
 class CRandomGenerator;
 class IGameCallback;
@@ -66,6 +67,14 @@ public:
 	}
 };
 
+class DLL_LINKAGE ICreatureUpgrader
+{
+public:
+	virtual void fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack) const = 0;
+
+	virtual ~ICreatureUpgrader() = default;
+};
+
 class DLL_LINKAGE IBoatGenerator
 {
 public:

lib/mapObjects/MiscObjects.cpp

@@ -2164,4 +2164,21 @@ void CGLighthouse::serializeJsonOptions(JsonSerializeFormat& handler)
 	serializeJsonOwner(handler);
 }
 
+void HillFort::onHeroVisit(const CGHeroInstance * h) const
+{
+	openWindow(EOpenWindowMode::HILL_FORT_WINDOW,id.getNum(),h->id.getNum());
+}
+
+void HillFort::fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack) const
+{
+	static const int costModifiers[] = {0, 25, 50, 75, 100}; //we get cheaper upgrades depending on level
+	const int costModifier = costModifiers[std::min<int>(std::max((int)stack.type->getLevel() - 1, 0), std::size(costModifiers) - 1)];
+
+	for(const auto & nid : stack.type->upgrades)
+	{
+		info.newID.push_back(nid);
+		info.cost.push_back((nid.toCreature()->getFullRecruitCost() - stack.type->getFullRecruitCost()) * costModifier / 100);
+	}
+}
+
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/MiscObjects.h

@@ -553,4 +553,11 @@ public:
 	}
 };
 
+class DLL_LINKAGE HillFort : public CGObjectInstance, public ICreatureUpgrader
+{
+protected:
+	void onHeroVisit(const CGHeroInstance * h) const override;
+	void fillUpgradeInfo(UpgradeInfo & info, const CStackInstance &stack) const override;
+};
+
 VCMI_LIB_NAMESPACE_END

lib/registerTypes/RegisterTypes.h

@@ -64,6 +64,7 @@ void registerTypesMapObjects1(Serializer &s)
 	s.template registerType<CGObjectInstance, CGDenOfthieves>();
 	s.template registerType<CGObjectInstance, CGLighthouse>();
 	s.template registerType<CGObjectInstance, CGTerrainPatch>();
+	s.template registerType<CGObjectInstance, HillFort>();
 	s.template registerType<CGObjectInstance, CGMarket>();
 		s.template registerType<CGMarket, CGBlackMarket>();
 		s.template registerType<CGMarket, CGUniversity>();
@@ -138,6 +139,7 @@ void registerTypesMapObjectTypes(Serializer &s)
 	REGISTER_GENERIC_HANDLER(CGTownInstance);
 	REGISTER_GENERIC_HANDLER(CGUniversity);
 	REGISTER_GENERIC_HANDLER(CGWitchHut);
+	REGISTER_GENERIC_HANDLER(HillFort);
 
 #undef REGISTER_GENERIC_HANDLER
 

test/mock/mock_spells_Spell.h

@@ -45,6 +45,7 @@ public:
 	MOCK_CONST_METHOD0(isOffensive, bool());
 	MOCK_CONST_METHOD0(isSpecial, bool());
 	MOCK_CONST_METHOD0(isMagical, bool());
+	MOCK_CONST_METHOD1(hasSchool, bool(ESpellSchool));
 	MOCK_CONST_METHOD1(forEachSchool, void(const SchoolCallback &));
 	MOCK_CONST_METHOD0(getCastSound, const std::string &());
 	MOCK_CONST_METHOD1(registerIcons, void(const IconRegistar &));