vcmi: remove code duplication for native terrain

include/vcmi/Entity.h

@@ -28,7 +28,16 @@ class DLL_LINKAGE INativeTerrainProvider
 public:
 	virtual Identifier<ETerrainId> getNativeTerrain() const = 0;
 	virtual FactionID getFaction() const = 0;
-	virtual bool isItNativeTerrain(Identifier<ETerrainId> terrain) const;
+	virtual bool isNativeTerrain(Identifier<ETerrainId> terrain) const;
+};
+
+class DLL_LINKAGE IConstBonusNativeTerrainProvider: public IConstBonusProvider, public INativeTerrainProvider
+{
+public:
+	/**
+	Returns native terrain considering some terrain bonuses.
+	*/
+	virtual Identifier<ETerrainId> getNativeTerrain() const;
 };
 
 class DLL_LINKAGE Entity
@@ -60,7 +69,7 @@ class DLL_LINKAGE EntityWithBonuses : public EntityT<IdType>, public IConstBonus
 };
 
 template <typename IdType>
-class DLL_LINKAGE EntityWithNativeTerrain : public EntityWithBonuses<IdType>, public INativeTerrainProvider
+class DLL_LINKAGE EntityWithNativeTerrain : public EntityT<IdType>, public IConstBonusNativeTerrainProvider
 {
 };
 

lib/BasicTypes.cpp

@@ -9,12 +9,33 @@
  */
 
 #include "StdInc.h"
+
+#include "VCMI_Lib.h"
 #include "GameConstants.h"
+#include "HeroBonus.h"
 
 #include <vcmi/Entity.h>
+#include <vcmi/Faction.h>
+#include <vcmi/FactionService.h>
 
-bool INativeTerrainProvider::isItNativeTerrain(TerrainId terrain) const
+VCMI_LIB_NAMESPACE_BEGIN
+
+bool INativeTerrainProvider::isNativeTerrain(TerrainId terrain) const
 {
 	auto native = getNativeTerrain();
 	return native == terrain || native == ETerrainId::ANY_TERRAIN;
-}
+}
+
+TerrainId IConstBonusNativeTerrainProvider::getNativeTerrain() const
+{
+	constexpr auto any = TerrainId(ETerrainId::ANY_TERRAIN);
+	const std::string cachingStringNoTerrainPenalty = "type_NO_TERRAIN_PENALTY_sANY";
+	static const auto selectorNoTerrainPenalty = Selector::typeSubtype(Bonus::NO_TERRAIN_PENALTY, any);
+
+	//this code is used in the CreatureTerrainLimiter::limit to setup battle bonuses
+	//and in the CGHeroInstance::getNativeTerrain() to setup movement bonuses or/and penalties.
+	return getBonusBearer()->hasBonus(selectorNoTerrainPenalty, cachingStringNoTerrainPenalty)
+		? any : VLC->factions()->getById(getFaction())->getNativeTerrain();
+}
+
+VCMI_LIB_NAMESPACE_END

lib/CCreatureHandler.cpp

@@ -330,18 +330,6 @@ std::string CCreature::nodeName() const
 	return "\"" + getNamePluralTextID() + "\"";
 }
 
-TerrainId CCreature::getNativeTerrain() const
-{
-	constexpr auto any = TerrainId(ETerrainId::ANY_TERRAIN);
-	const std::string cachingStringNoTerrainPenalty = "type_NO_TERRAIN_PENALTY_sANY";
-	static const auto selectorNoTerrainPenalty = Selector::typeSubtype(Bonus::NO_TERRAIN_PENALTY, any);
-
-	//this code is used in the CreatureTerrainLimiter::limit to setup battle bonuses
-	//and in the CGHeroInstance::getNativeTerrain() to setup movement bonuses or/and penalties.
-	return getBonusBearer()->hasBonus(selectorNoTerrainPenalty, cachingStringNoTerrainPenalty)
-		? any : VLC->factions()->getById(getFaction())->getNativeTerrain();
-}
-
 void CCreature::updateFrom(const JsonNode & data)
 {
 	JsonUpdater handler(nullptr, data);

lib/CCreatureHandler.h

@@ -163,10 +163,6 @@ public:
 	std::string getNameSingularTextID() const override;
 
 	FactionID getFaction() const override;
-	/**
-	Returns creature native terrain considering some terrain bonuses.
-	*/
-	TerrainId getNativeTerrain() const override;
 	int32_t getIndex() const override;
 	int32_t getIconIndex() const override;
 	std::string getJsonKey() const override;

lib/CCreatureSet.cpp

@@ -928,18 +928,6 @@ const IBonusBearer* CStackInstance::getBonusBearer() const
 	return this;
 }
 
-TerrainId CStackInstance::getNativeTerrain() const
-{
-	const std::string cachingStringNoTerrainPenalty = "type_NO_TERRAIN_PENALTY_sANY";
-	static const auto selectorNoTerrainPenalty = Selector::typeSubtype(Bonus::NO_TERRAIN_PENALTY, static_cast<int>(ETerrainId::ANY_TERRAIN));
-
-	//this code is used in the CreatureTerrainLimiter::limit to setup battle bonuses
-	//and in the CGHeroInstance::getNativeTerrain() to setup movement bonuses or/and penalties.
-	return getBonusBearer()->hasBonus(selectorNoTerrainPenalty, cachingStringNoTerrainPenalty)
-		? TerrainId(ETerrainId::ANY_TERRAIN)
-		: VLC->factions()->getById(getFaction())->getNativeTerrain();
-}
-
 CCommanderInstance::CCommanderInstance()
 {
 	init();

lib/CCreatureSet.h

@@ -63,7 +63,7 @@ public:
 	void serializeJson(JsonSerializeFormat & handler);
 };
 
-class DLL_LINKAGE CStackInstance : public CBonusSystemNode, public CStackBasicDescriptor, public CArtifactSet, public IConstBonusProvider, public INativeTerrainProvider
+class DLL_LINKAGE CStackInstance : public CBonusSystemNode, public CStackBasicDescriptor, public CArtifactSet, public IConstBonusNativeTerrainProvider
 {
 protected:
 	const CArmedInstance *_armyObj; //stack must be part of some army, army must be part of some object
@@ -98,7 +98,6 @@ public:
 	const IBonusBearer* getBonusBearer() const override;
 	//INativeTerrainProvider
 	FactionID getFaction() const override;
-	TerrainId getNativeTerrain() const override;
 
 	virtual ui64 getPower() const;
 	CCreature::CreatureQuantityId getQuantityID() const;

lib/battle/CBattleInfoEssentials.cpp

@@ -87,7 +87,7 @@ bool CBattleInfoEssentials::battleHasNativeStack(ui8 side) const
 
 	for(const auto * s : battleGetAllStacks())
 	{
-		if(s->side == side && s->isItNativeTerrain(getBattle()->getTerrainType()))
+		if(s->side == side && s->isNativeTerrain(getBattle()->getTerrainType()))
 			return true;
 	}
 

lib/battle/Unit.cpp

@@ -52,18 +52,6 @@ const IBonusBearer* Unit::getBonusBearer() const
 	return this;
 }
 
-TerrainId Unit::getNativeTerrain() const
-{
-	const std::string cachingStringNoTerrainPenalty = "type_NO_TERRAIN_PENALTY_sANY";
-	static const auto selectorNoTerrainPenalty = Selector::typeSubtype(Bonus::NO_TERRAIN_PENALTY, static_cast<int>(ETerrainId::ANY_TERRAIN));
-
-	//this code is used in the CreatureTerrainLimiter::limit to setup battle bonuses
-	//and in the CGHeroInstance::getNativeTerrain() to setup movement bonuses or/and penalties.
-	return getBonusBearer()->hasBonus(selectorNoTerrainPenalty, cachingStringNoTerrainPenalty)
-		? TerrainId(ETerrainId::ANY_TERRAIN)
-		: VLC->factions()->getById(getFaction())->getNativeTerrain();
-}
-
 std::vector<BattleHex> Unit::getSurroundingHexes(BattleHex assumedPosition) const
 {
 	BattleHex hex = (assumedPosition != BattleHex::INVALID) ? assumedPosition : getPosition(); //use hypothetical position

lib/battle/Unit.h

@@ -41,7 +41,7 @@ namespace BattlePhases
 
 class CUnitState;
 
-class DLL_LINKAGE Unit : public IUnitInfo, public spells::Caster, public virtual IBonusBearer, public IConstBonusProvider, public INativeTerrainProvider
+class DLL_LINKAGE Unit : public IUnitInfo, public spells::Caster, public virtual IBonusBearer, public IConstBonusNativeTerrainProvider
 {
 public:
 	virtual ~Unit();
@@ -129,8 +129,6 @@ public:
 
 	//IConstBonusProvider
 	const IBonusBearer* getBonusBearer() const override;
-	//INativeTerrainProvider
-	TerrainId getNativeTerrain() const override;
 
 	//NOTE: save could possibly be const, but this requires heavy changes to Json serialization,
 	//also this method should be called only after modifying object

lib/mapObjects/CGHeroInstance.h

@@ -40,7 +40,7 @@ public:
 };
 
 
-class DLL_LINKAGE CGHeroInstance : public CArmedInstance, public IBoatGenerator, public CArtifactSet, public spells::Caster, public IConstBonusProvider, public INativeTerrainProvider
+class DLL_LINKAGE CGHeroInstance : public CArmedInstance, public IBoatGenerator, public CArtifactSet, public spells::Caster, public IConstBonusNativeTerrainProvider
 {
 	// We serialize heroes into JSON for crossover
 	friend class CCampaignState;

scripting/lua/api/Artifact.cpp

@@ -34,7 +34,7 @@ const std::vector<ArtifactProxy::CustomRegType> ArtifactProxy::REGISTER_CUSTOM =
 	{"getName", LuaMethodWrapper<Artifact, decltype(&Entity::getNameTranslated), &Entity::getNameTranslated>::invoke, false},
 
 	{"getId", LuaMethodWrapper<Artifact, decltype(&EntityT<ArtifactID>::getId), &EntityT<ArtifactID>::getId>::invoke, false},
-	{"getBonusBearer", LuaMethodWrapper<Artifact, decltype(&EntityWithBonuses<ArtifactID>::getBonusBearer), &EntityWithBonuses<ArtifactID>::getBonusBearer>::invoke, false},
+	{"getBonusBearer", LuaMethodWrapper<Artifact, decltype(&IConstBonusProvider::getBonusBearer), &IConstBonusProvider::getBonusBearer>::invoke, false},
 
 	{"getDescription", LuaMethodWrapper<Artifact, decltype(&Artifact::getDescriptionTranslated), &Artifact::getDescriptionTranslated>::invoke, false},
 	{"getEventText", LuaMethodWrapper<Artifact, decltype(&Artifact::getEventTranslated), &Artifact::getEventTranslated>::invoke, false},

scripting/lua/api/Creature.cpp

@@ -32,7 +32,7 @@ const std::vector<CreatureProxy::CustomRegType> CreatureProxy::REGISTER_CUSTOM =
 	{"getIndex", LuaMethodWrapper<Creature, decltype(&Entity::getIndex), &Entity::getIndex>::invoke, false},
 	{"getJsonKey", LuaMethodWrapper<Creature, decltype(&Entity::getJsonKey), &Entity::getJsonKey>::invoke, false},
 	{"getName", LuaMethodWrapper<Creature, decltype(&Entity::getNameTranslated), &Entity::getNameTranslated>::invoke, false},
-	{"getBonusBearer", LuaMethodWrapper<Creature, decltype(&EntityWithBonuses<CreatureID>::getBonusBearer), &EntityWithBonuses<CreatureID>::getBonusBearer>::invoke, false},
+	{"getBonusBearer", LuaMethodWrapper<Creature, decltype(&IConstBonusProvider::getBonusBearer), &IConstBonusProvider::getBonusBearer>::invoke, false},
 
 	{"getMaxHealth", LuaMethodWrapper<Creature,decltype(&Creature::getMaxHealth), &Creature::getMaxHealth>::invoke, false},
 	{"getPluralName", LuaMethodWrapper<Creature, decltype(&Creature::getNamePluralTranslated), &Creature::getNamePluralTranslated>::invoke, false},