code review

include/vcmi/spells/Caster.h

@@ -38,7 +38,6 @@ public:
 	virtual ~Caster() = default;
 
 	virtual int32_t getCasterUnitId() const = 0;
-	virtual BattleHex getCasterPosition() const = 0;
 
 	/// returns level on which given spell would be cast by this(0 - none, 1 - basic etc);
 	/// caster may not know this spell at all

lib/battle/CUnitState.cpp

@@ -407,11 +407,6 @@ int32_t CUnitState::getCasterUnitId() const
 	return static_cast<int32_t>(unitId());
 }
 
-BattleHex CUnitState::getCasterPosition() const
-{
-	return getPosition();
-}
-
 const CGHeroInstance * CUnitState::getHeroCaster() const
 {
 	return nullptr;

lib/battle/CUnitState.h

@@ -170,7 +170,6 @@ public:
 	int32_t creatureIconIndex() const override;
 
 	int32_t getCasterUnitId() const override;
-	BattleHex getCasterPosition() const override;
 
 	int32_t getSpellSchoolLevel(const spells::Spell * spell, SpellSchool * outSelectedSchool = nullptr) const override;
 	int32_t getEffectLevel(const spells::Spell * spell) const override;

lib/mapObjects/CGHeroInstance.cpp

@@ -782,11 +782,6 @@ int32_t CGHeroInstance::getCasterUnitId() const
 	return id.getNum();
 }
 
-BattleHex CGHeroInstance::getCasterPosition() const
-{
-	return BattleHex::INVALID;
-}
-
 int32_t CGHeroInstance::getSpellSchoolLevel(const spells::Spell * spell, SpellSchool * outSelectedSchool) const
 {
 	int32_t skill = -1; //skill level

lib/mapObjects/CGHeroInstance.h

@@ -279,7 +279,6 @@ public:
 
 	///spells::Caster
 	int32_t getCasterUnitId() const override;
-	BattleHex getCasterPosition() const override;
 	int32_t getSpellSchoolLevel(const spells::Spell * spell, SpellSchool * outSelectedSchool = nullptr) const override;
 	int64_t getSpellBonus(const spells::Spell * spell, int64_t base, const battle::Unit * affectedStack) const override;
 	int64_t getSpecificSpellBonus(const spells::Spell * spell, int64_t base) const override;

lib/spells/BattleSpellMechanics.cpp

@@ -223,8 +223,9 @@ bool BattleSpellMechanics::canCastAtTarget(const battle::Unit * target) const
 	if(range <= 0)
 		return true;
 
-	std::vector<BattleHex> casterPos = { caster->getCasterPosition() };
-	BattleHex casterWidePos = battle()->battleGetStackByID(caster->getCasterUnitId(), false)->occupiedHex();
+	auto casterStack = battle()->battleGetStackByID(caster->getCasterUnitId(), false);
+	std::vector<BattleHex> casterPos = { casterStack->getPosition() };
+	BattleHex casterWidePos = casterStack->occupiedHex();
 	if(casterWidePos != BattleHex::INVALID)
 		casterPos.push_back(casterWidePos);
 

lib/spells/ProxyCaster.cpp

@@ -36,14 +36,6 @@ int32_t ProxyCaster::getCasterUnitId() const
 	return -1;
 }
 
-BattleHex ProxyCaster::getCasterPosition() const
-{
-	if(actualCaster)
-		return actualCaster->getCasterPosition();
-
-	return BattleHex::INVALID;
-}
-
 int32_t ProxyCaster::getSpellSchoolLevel(const Spell * spell, SpellSchool * outSelectedSchool) const
 {
 	if(actualCaster)

lib/spells/ProxyCaster.h

@@ -24,7 +24,6 @@ public:
 	virtual ~ProxyCaster();
 
 	int32_t getCasterUnitId() const override;
-	BattleHex getCasterPosition() const override;
 	int32_t getSpellSchoolLevel(const Spell * spell, SpellSchool * outSelectedSchool = nullptr) const override;
 	int32_t getEffectLevel(const Spell * spell) const override;
 	int64_t getSpellBonus(const Spell * spell, int64_t base, const battle::Unit * affectedStack) const override;

test/mock/mock_battle_Unit.h

@@ -19,7 +19,6 @@ public:
 	MOCK_CONST_METHOD0(getTreeVersion, int32_t());
 
 	MOCK_CONST_METHOD0(getCasterUnitId, int32_t());
-	MOCK_CONST_METHOD0(getCasterPosition, BattleHex());
 	MOCK_CONST_METHOD2(getSpellSchoolLevel, int32_t(const spells::Spell *, SpellSchool *));
 	MOCK_CONST_METHOD1(getEffectLevel, int32_t(const spells::Spell *));
 	MOCK_CONST_METHOD3(getSpellBonus, int64_t(const spells::Spell *, int64_t, const battle::Unit *));