Fixed https://bugs.vcmi.eu/view.php?id=2904

lib/battle/CUnitState.cpp

@@ -554,6 +554,11 @@ int32_t CUnitState::creatureIconIndex() const
 	return unitType()->iconIndex;
 }
 
+int32_t CUnitState::getCasterUnitId() const
+{
+	return static_cast<int32_t>(unitId());
+}
+
 ui8 CUnitState::getSpellSchoolLevel(const spells::Spell * spell, int * outSelectedSchool) const
 {
 	int skill = valOfBonuses(Selector::typeSubtype(Bonus::SPELLCASTER, spell->getIndex()));

lib/battle/CUnitState.h

@@ -214,6 +214,8 @@ public:
 	int32_t creatureCost() const override;
 	int32_t creatureIconIndex() const override;
 
+	int32_t getCasterUnitId() const override;
+
 	ui8 getSpellSchoolLevel(const spells::Spell * spell, int * outSelectedSchool = nullptr) const override;
 	int getEffectLevel(const spells::Spell * spell) const override;
 

lib/mapObjects/CGHeroInstance.cpp

@@ -612,6 +612,11 @@ TExpType CGHeroInstance::calculateXp(TExpType exp) const
 	return exp * (100 + valOfBonuses(Bonus::SECONDARY_SKILL_PREMY, SecondarySkill::LEARNING))/100.0;
 }
 
+int32_t CGHeroInstance::getCasterUnitId() const
+{
+	return -1; //TODO: special value for attacker/defender hero
+}
+
 ui8 CGHeroInstance::getSpellSchoolLevel(const spells::Spell * spell, int * outSelectedSchool) const
 {
 	si16 skill = -1; //skill level

lib/mapObjects/CGHeroInstance.h

@@ -232,6 +232,7 @@ public:
 	std::string nodeName() const override;
 
 	///spells::Caster
+	int32_t getCasterUnitId() const override;
 	ui8 getSpellSchoolLevel(const spells::Spell * spell, int * 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

@@ -210,7 +210,7 @@ void BattleSpellMechanics::cast(const PacketSender * server, vstd::RNG & rng, co
 	sc.tile = target.at(0).hexValue;
 
 	sc.castByHero = mode == Mode::HERO;
-	sc.casterStack = (casterUnit ? casterUnit->unitId() : -1);
+	sc.casterStack = caster->getCasterUnitId();
 	sc.manaGained = 0;
 
 	sc.activeCast = false;
@@ -350,9 +350,9 @@ void BattleSpellMechanics::beforeCast(BattleSpellCast & sc, vstd::RNG & rng, con
 
 	if(mode == Mode::MAGIC_MIRROR)
 	{
-		if(casterUnit)
+		if(caster->getCasterUnitId() >= 0)
 		{
-			addCustomEffect(sc, casterUnit, 3);
+			addCustomEffect(sc, caster->getCasterUnitId(), 3);
 		}
 	}
 
@@ -393,10 +393,15 @@ void BattleSpellMechanics::cast(IBattleState * battleState, vstd::RNG & rng, con
 }
 
 void BattleSpellMechanics::addCustomEffect(BattleSpellCast & sc, const battle::Unit * target, ui32 effect)
+{
+	addCustomEffect(sc, target->unitId(), effect);
+}
+
+void BattleSpellMechanics::addCustomEffect(BattleSpellCast & sc, ui32 targetId, ui32 effect)
 {
 	CustomEffectInfo customEffect;
 	customEffect.effect = effect;
-	customEffect.stack = target->unitId();
+	customEffect.stack = targetId;
 	sc.customEffects.push_back(customEffect);
 }
 

lib/spells/BattleSpellMechanics.h

@@ -54,6 +54,7 @@ private:
 	void beforeCast(BattleSpellCast & sc, vstd::RNG & rng, const Target & target);
 
 	void addCustomEffect(BattleSpellCast & sc, const battle::Unit * target, ui32 effect);
+	void addCustomEffect(BattleSpellCast & sc, ui32 targetId, ui32 effect);
 
 	std::set<const battle::Unit *> collectTargets() const;
 

lib/spells/ISpellMechanics.cpp

@@ -435,7 +435,6 @@ std::unique_ptr<ISpellMechanicsFactory> ISpellMechanicsFactory::get(const CSpell
 Mechanics::Mechanics()
 	: cb(nullptr),
 	caster(nullptr),
-	casterUnit(nullptr),
 	casterSide(0)
 {
 
@@ -453,12 +452,8 @@ BaseMechanics::BaseMechanics(const IBattleCast * event)
 	cb = event->getBattle();
 	caster = event->getCaster();
 
-	casterUnit = dynamic_cast<const battle::Unit *>(caster);
-
-	//FIXME: ensure caster and check for valid player and side
-	casterSide = 0;
-	if(caster)
-		casterSide = cb->playerToSide(caster->getOwner()).get();
+	//FIXME: do not crash on invalid side
+	casterSide = cb->playerToSide(caster->getOwner()).get();
 
 	{
 		auto value = event->getSpellLevel();

lib/spells/ISpellMechanics.h

@@ -256,8 +256,6 @@ public:
 	const CBattleInfoCallback * cb;
 	const Caster * caster;
 
-	const battle::Unit * casterUnit; //deprecated
-
 	ui8 casterSide;
 };
 

lib/spells/Magic.h

@@ -100,6 +100,8 @@ class DLL_LINKAGE Caster
 public:
 	virtual ~Caster() = default;
 
+	virtual int32_t getCasterUnitId() 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
 	/// optionally returns number of selected school by arg - 0 - air magic, 1 - fire magic, 2 - water magic, 3 - earth magic

lib/spells/ProxyCaster.cpp

@@ -24,6 +24,11 @@ ProxyCaster::ProxyCaster(const Caster * actualCaster_)
 
 ProxyCaster::~ProxyCaster() = default;
 
+int32_t ProxyCaster::getCasterUnitId() const
+{
+	return actualCaster->getCasterUnitId();
+}
+
 ui8 ProxyCaster::getSpellSchoolLevel(const Spell * spell, int * outSelectedSchool) const
 {
 	return actualCaster->getSpellSchoolLevel(spell, outSelectedSchool);

lib/spells/ProxyCaster.h

@@ -21,6 +21,7 @@ public:
 	ProxyCaster(const Caster * actualCaster_);
 	virtual ~ProxyCaster();
 
+	int32_t getCasterUnitId() const override;
 	ui8 getSpellSchoolLevel(const Spell * spell, int * outSelectedSchool = nullptr) const override;
 	int getEffectLevel(const Spell * spell) const override;
 	int64_t getSpellBonus(const Spell * spell, int64_t base, const battle::Unit * affectedStack) const override;

lib/spells/effects/Effects.cpp

@@ -112,7 +112,7 @@ Effects::EffectsToApply Effects::prepare(const Mechanics * m, const Target & aim
 		//todo: find a better way to handle such special cases
 
 		if(m->getSpellIndex() == SpellID::RESURRECTION && e->name == "cure")
-			applyThis = (m->casterUnit != nullptr);
+			applyThis = (m->caster->getCasterUnitId() >= 0);
 		else
 			applyThis = !e->indirect;
 

lib/spells/effects/Timed.cpp

@@ -65,8 +65,8 @@ void Timed::convertBonus(const Mechanics * m, int32_t & duration, std::vector<Bo
 		if((nb.sid == SpellID::SHIELD || nb.sid == SpellID::AIR_SHIELD) && (nb.type == Bonus::GENERAL_DAMAGE_REDUCTION))
 			nb.val = 100 - nb.val;
 		//we need to know who cast Bind
-		else if(nb.sid == SpellID::BIND && nb.type == Bonus::BIND_EFFECT && m->casterUnit)
-			nb.additionalInfo = m->casterUnit->unitId();
+		else if(nb.sid == SpellID::BIND && nb.type == Bonus::BIND_EFFECT && m->caster->getCasterUnitId() >= 0)
+			nb.additionalInfo = m->caster->getCasterUnitId();
 
 		converted.push_back(nb);
 	}

server/CGameHandler.cpp

@@ -89,6 +89,14 @@ public:
 
 	~ObstacleCasterProxy() = default;
 
+	int32_t getCasterUnitId() const override
+	{
+		if(hero)
+			return hero->getCasterUnitId();
+		else
+			return -1;
+	}
+
 	ui8 getSpellSchoolLevel(const Spell * spell, int * outSelectedSchool = nullptr) const override
 	{
 		return obs->spellLevel;
@@ -4715,11 +4723,18 @@ void CGameHandler::stackTurnTrigger(const CStack *st)
 
 			for (auto b : bl)
 			{
-				const CStack * stack = gs->curB->battleGetStackByID(b->additionalInfo); //binding stack must be alive and adjacent
-				if(stack)
+				if(b->additionalInfo >= 0)
+				{
+					const CStack * stack = gs->curB->battleGetStackByID(b->additionalInfo); //binding stack must be alive and adjacent
+					if(stack)
+					{
+						if(vstd::contains(adjacent, stack)) //binding stack is still present
+							unbind = false;
+					}
+				}
+				else
 				{
-					if(vstd::contains(adjacent, stack)) //binding stack is still present
-						unbind = false;
+					unbind = false;
 				}
 			}
 			if (unbind)

test/mock/mock_battle_Unit.h

@@ -18,6 +18,7 @@ public:
 	MOCK_CONST_METHOD4(getAllBonuses, const TBonusListPtr(const CSelector &, const CSelector &, const CBonusSystemNode *, const std::string &));
 	MOCK_CONST_METHOD0(getTreeVersion, int64_t());
 
+	MOCK_CONST_METHOD0(getCasterUnitId, int32_t());
 	MOCK_CONST_METHOD2(getSpellSchoolLevel, ui8(const spells::Spell *, int *));
 	MOCK_CONST_METHOD1(getEffectLevel, int(const spells::Spell *));
 	MOCK_CONST_METHOD3(getSpellBonus, int64_t(const spells::Spell *, int64_t, const battle::Unit *));