FactionMember: move code around

client/windows/CHeroWindow.cpp

@@ -69,6 +69,16 @@ si32 CHeroWithMaybePickedArtifact::manaLimit() const
 	return si32(getPrimSkillLevel(PrimarySkill::KNOWLEDGE) * (valOfBonuses(Bonus::MANA_PER_KNOWLEDGE)));
 }
 
+const IBonusBearer * CHeroWithMaybePickedArtifact::getBonusBearer() const 
+{
+	return this;
+}
+
+FactionID CHeroWithMaybePickedArtifact::getFaction() const
+{
+	return hero->getFaction();
+} 
+
 CHeroWithMaybePickedArtifact::CHeroWithMaybePickedArtifact(CWindowWithArtifacts * Cww, const CGHeroInstance * Hero)
 	: hero(Hero), cww(Cww)
 {

client/windows/CHeroWindow.h

@@ -9,6 +9,8 @@
  */
 #pragma once
 
+#include <vcmi/FactionMember.h>
+
 #include "../../lib/HeroBonus.h"
 #include "../widgets/CWindowWithArtifacts.h"
 #include "../widgets/CGarrisonInt.h"
@@ -45,7 +47,7 @@ public:
 };
 
 //helper class for calculating values of hero bonuses without bonuses from picked up artifact
-class CHeroWithMaybePickedArtifact : public virtual IBonusBearer
+class CHeroWithMaybePickedArtifact : public IBonusBearer, public IFactionMember
 {
 public:
 	const CGHeroInstance * hero;
@@ -54,6 +56,9 @@ public:
 	CHeroWithMaybePickedArtifact(CWindowWithArtifacts * Cww, const CGHeroInstance * Hero);
 	TConstBonusListPtr getAllBonuses(const CSelector & selector, const CSelector & limit, const CBonusSystemNode * root = nullptr, const std::string & cachingStr = "") const override;
 
+	const IBonusBearer * getBonusBearer() const override;
+	FactionID getFaction() const override; 
+
 	int64_t getTreeVersion() const override;
 
 	si32 manaLimit() const;

include/vcmi/Creature.h

@@ -10,7 +10,7 @@
 
 #pragma once
 
-#include "Entity.h"
+#include "FactionMember.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -18,6 +18,20 @@ class CreatureID;
 class ResourceSet;
 enum class EGameResID : int8_t;
 
+/// Base class for creatures and battle stacks
+class DLL_LINKAGE ICreature: public IFactionMember
+{
+public:
+	bool isLiving() const; //non-undead, non-non living or alive
+	ui32 Speed(int turn = 0, bool useBind = false) const; //get speed (in moving tiles) of creature with all modificators
+	ui32 MaxHealth() const; //get max HP of stack with all modifiers
+};
+
+template <typename IdType>
+class DLL_LINKAGE CreatureEntity : public EntityT<IdType>, public ICreature
+{
+};
+
 class DLL_LINKAGE Creature : public CreatureEntity<CreatureID>
 {
 protected:

include/vcmi/Entity.h

@@ -31,43 +31,6 @@ public:
 	virtual bool isNativeTerrain(Identifier<ETerrainId> terrain) const;
 };
 
-class DLL_LINKAGE IFactionMember: public IConstBonusProvider, public INativeTerrainProvider
-{
-public:
-	/**
-	 Returns native terrain considering some terrain bonuses.
-	*/
-	virtual Identifier<ETerrainId> getNativeTerrain() const;
-	/**
-	 Returns magic resistance considering some bonuses.
-	*/
-	virtual int32_t magicResistance() const;
-	/**
-	 Returns minimal damage of creature or (when implemented) hero.
-	*/
-	virtual int getMinDamage(bool ranged) const;
-	/**
-	 Returns maximal damage of creature or (when implemented) hero.
-	*/
-	virtual int getMaxDamage(bool ranged) const;
-	/**
-	 Returns attack of creature or hero.
-	*/
-	virtual int getAttack(bool ranged) const;
-	/**
-	 Returns defence of creature or hero.
-	*/
-	virtual int getDefense(bool ranged) const;
-};
-
-/// Base class for creatures and battle stacks
-class DLL_LINKAGE ICreature: public IFactionMember
-{
-public:
-	ui32 Speed(int turn = 0, bool useBind = false) const; //get speed (in moving tiles) of creature with all modificators
-	ui32 MaxHealth() const; //get max HP of stack with all modifiers
-};
-
 class DLL_LINKAGE Entity
 {
 public:
@@ -96,9 +59,4 @@ class DLL_LINKAGE EntityWithBonuses : public EntityT<IdType>, public IConstBonus
 {
 };
 
-template <typename IdType>
-class DLL_LINKAGE CreatureEntity : public EntityT<IdType>, public ICreature
-{
-};
-
 VCMI_LIB_NAMESPACE_END

include/vcmi/FactionMember.h

@@ -0,0 +1,55 @@
+/*
+ * FactionMember.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#pragma once
+
+#include "Entity.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+namespace PrimarySkill
+{
+    enum PrimarySkill : int8_t;
+}
+
+class DLL_LINKAGE IFactionMember: public IConstBonusProvider, public INativeTerrainProvider
+{
+public:
+	/**
+	 Returns native terrain considering some terrain bonuses.
+	*/
+	virtual Identifier<ETerrainId> getNativeTerrain() const;
+	/**
+	 Returns magic resistance considering some bonuses.
+	*/
+	virtual int32_t magicResistance() const;
+	/**
+	 Returns minimal damage of creature or (when implemented) hero.
+	*/
+	virtual int getMinDamage(bool ranged) const;
+	/**
+	 Returns maximal damage of creature or (when implemented) hero.
+	*/
+	virtual int getMaxDamage(bool ranged) const;
+	/**
+	 Returns attack of creature or hero.
+	*/
+	virtual int getAttack(bool ranged) const;
+	/**
+	 Returns defence of creature or hero.
+	*/
+	virtual int getDefense(bool ranged) const;
+	/**
+	 Returns primskill of creature or hero.
+	*/
+	int getPrimSkillLevel(PrimarySkill::PrimarySkill id) const;
+};
+
+VCMI_LIB_NAMESPACE_END

lib/BasicTypes.cpp

@@ -14,8 +14,9 @@
 #include "GameConstants.h"
 #include "HeroBonus.h"
 
-#include <vcmi/Entity.h>
+#include <vcmi/Creature.h>
 #include <vcmi/Faction.h>
+#include <vcmi/FactionMember.h>
 #include <vcmi/FactionService.h>
 
 VCMI_LIB_NAMESPACE_BEGIN
@@ -77,6 +78,16 @@ int IFactionMember::getMaxDamage(bool ranged) const
 	return getBonusBearer()->valOfBonuses(selector, cachingStr);
 }
 
+int IFactionMember::getPrimSkillLevel(PrimarySkill::PrimarySkill id) const
+{
+	static const CSelector selectorAllSkills = Selector::type()(Bonus::PRIMARY_SKILL);
+	static const std::string keyAllSkills = "type_PRIMARY_SKILL";
+	auto allSkills = getBonusBearer()->getBonuses(selectorAllSkills, keyAllSkills);
+	auto ret = allSkills->valOfBonuses(Selector::subtype()(id));
+	auto minSkillValue = (id == PrimarySkill::SPELL_POWER || id == PrimarySkill::KNOWLEDGE) ? 1 : 0;
+	return std::max(ret, minSkillValue); //otherwise, some artifacts may cause negative skill value effect, sp=0 works in old saves
+}
+
 ui32 ICreature::MaxHealth() const
 {
 	const std::string cachingStr = "type_STACK_HEALTH";
@@ -100,4 +111,17 @@ ui32 ICreature::Speed(int turn, bool useBind) const
 
 	return getBonusBearer()->valOfBonuses(Selector::type()(Bonus::STACKS_SPEED).And(Selector::turns(turn)));
 }
+
+bool ICreature::isLiving() const //TODO: theoreticaly there exists "LIVING" bonus in stack experience documentation
+{
+	static const std::string cachingStr = "IBonusBearer::isLiving";
+	static const CSelector selector = Selector::type()(Bonus::UNDEAD)
+		.Or(Selector::type()(Bonus::NON_LIVING))
+		.Or(Selector::type()(Bonus::GARGOYLE))
+		.Or(Selector::type()(Bonus::SIEGE_WEAPON));
+
+	return !getBonusBearer()->hasBonus(selector, cachingStr);
+}
+
+
 VCMI_LIB_NAMESPACE_END

lib/GameConstants.h

@@ -398,7 +398,7 @@ class TeleportChannelID : public BaseForID<TeleportChannelID, si32>
 // Enum declarations
 namespace PrimarySkill
 {
-	enum PrimarySkill { NONE = -1, ATTACK, DEFENSE, SPELL_POWER, KNOWLEDGE,
+	enum PrimarySkill : int8_t { NONE = -1, ATTACK, DEFENSE, SPELL_POWER, KNOWLEDGE,
 				EXPERIENCE = 4}; //for some reason changePrimSkill uses it
 }
 

lib/HeroBonus.cpp

@@ -744,28 +744,6 @@ int IBonusBearer::LuckValAndBonusList(TConstBonusListPtr & bonusList) const
 	return std::clamp(luckValue.getValueAndList(bonusList), -3, +3);
 }
 
-int IBonusBearer::getPrimSkillLevel(PrimarySkill::PrimarySkill id) const
-{
-	static const CSelector selectorAllSkills = Selector::type()(Bonus::PRIMARY_SKILL);
-	static const std::string keyAllSkills = "type_PRIMARY_SKILL";
-	auto allSkills = getBonuses(selectorAllSkills, keyAllSkills);
-	auto ret = allSkills->valOfBonuses(Selector::subtype()(id));
-	auto minSkillValue = (id == PrimarySkill::SPELL_POWER || id == PrimarySkill::KNOWLEDGE) ? 1 : 0;
-	vstd::amax(ret, minSkillValue); //otherwise, some artifacts may cause negative skill value effect
-	return ret; //sp=0 works in old saves
-}
-
-bool IBonusBearer::isLiving() const //TODO: theoreticaly there exists "LIVING" bonus in stack experience documentation
-{
-	static const std::string cachingStr = "IBonusBearer::isLiving";
-	static const CSelector selector = Selector::type()(Bonus::UNDEAD)
-		.Or(Selector::type()(Bonus::NON_LIVING))
-		.Or(Selector::type()(Bonus::GARGOYLE))
-		.Or(Selector::type()(Bonus::SIEGE_WEAPON));
-
-	return !hasBonus(selector, cachingStr);
-}
-
 std::shared_ptr<const Bonus> IBonusBearer::getBonus(const CSelector &selector) const
 {
 	auto bonuses = getAllBonuses(selector, Selector::all);

lib/HeroBonus.h

@@ -753,10 +753,6 @@ public:
 	int MoraleValAndBonusList(TConstBonusListPtr & bonusList) const;
 	int LuckValAndBonusList(TConstBonusListPtr & bonusList) const;
 
-	bool isLiving() const; //non-undead, non-non living or alive
-
-	int getPrimSkillLevel(PrimarySkill::PrimarySkill id) const;
-
 	virtual int64_t getTreeVersion() const = 0;
 };
 

lib/battle/Unit.h

@@ -10,7 +10,7 @@
 
 #pragma once
 
-#include <vcmi/Entity.h>
+#include <vcmi/Creature.h>
 #include <vcmi/spells/Caster.h>
 
 #include "../HeroBonus.h"