Improved approach to reasoning about the attacked and followed Creature

src/creature.cpp

@@ -27,7 +27,7 @@ Creature::Creature() { onIdleStatus(); }
 Creature::~Creature()
 {
 	for (Creature* summon : summons) {
-		summon->setAttackedCreature(nullptr);
+		summon->removeAttackedCreature();
 		summon->removeMaster();
 	}
 
@@ -339,12 +339,12 @@ void Creature::onRemoveCreature(Creature* creature, bool) { onCreatureDisappear(
 void Creature::onCreatureDisappear(const Creature* creature, bool isLogout)
 {
 	if (attackedCreature == creature) {
-		setAttackedCreature(nullptr);
+		removeAttackedCreature();
 		onAttackedCreatureDisappear(isLogout);
 	}
 
 	if (followCreature == creature) {
-		setFollowCreature(nullptr);
+		removeFollowCreature();
 		onFollowCreatureDisappear(isLogout);
 	}
 }
@@ -722,26 +722,42 @@ BlockType_t Creature::blockHit(Creature* attacker, CombatType_t combatType, int3
 	return blockType;
 }
 
-bool Creature::setAttackedCreature(Creature* creature)
+void Creature::setAttackedCreature(Creature* creature)
 {
-	if (creature) {
-		const Position& creaturePos = creature->getPosition();
-		if (creaturePos.z != getPosition().z || !canSee(creaturePos)) {
-			attackedCreature = nullptr;
-			return false;
-		}
+	if (isAttackingCreature(creature)) {
+		return;
+	}
 
-		attackedCreature = creature;
-		onAttackedCreature(attackedCreature);
-		attackedCreature->onAttacked();
-	} else {
-		attackedCreature = nullptr;
+	if (!canAttackCreature(creature)) {
+		removeAttackedCreature();
+		return;
 	}
 
+	attackedCreature = creature;
+	onAttackedCreature(attackedCreature);
+	attackedCreature->onAttacked();
+
 	for (Creature* summon : summons) {
 		summon->setAttackedCreature(creature);
 	}
-	return true;
+}
+
+void Creature::removeAttackedCreature()
+{
+	attackedCreature = nullptr;
+
+	for (Creature* summon : summons) {
+		summon->removeAttackedCreature();
+	}
+}
+
+bool Creature::canAttackCreature(Creature* creature)
+{
+	const auto& creaturePos = creature->getPosition();
+	if (creaturePos.z != getPosition().z) {
+		return false;
+	}
+	return canSee(creaturePos);
 }
 
 void Creature::getPathSearchParams(const Creature*, FindPathParams& fpp) const
@@ -753,37 +769,50 @@ void Creature::getPathSearchParams(const Creature*, FindPathParams& fpp) const
 	fpp.maxTargetDist = 1;
 }
 
-bool Creature::setFollowCreature(Creature* creature)
+void Creature::setFollowCreature(Creature* creature)
 {
-	if (creature) {
-		if (followCreature == creature) {
-			return true;
-		}
+	if (isFollowingCreature(creature)) {
+		return;
+	}
 
-		const Position& creaturePos = creature->getPosition();
-		if (creaturePos.z != getPosition().z || !canSee(creaturePos)) {
-			followCreature = nullptr;
-			return false;
-		}
+	if (!canFollowCreature(creature)) {
+		removeFollowCreature();
+		return;
+	}
 
-		if (!listWalkDir.empty()) {
-			listWalkDir.clear();
-			onWalkAborted();
-		}
+	followCreature = creature;
+	onFollowCreature(creature);
+}
 
-		hasFollowPath = false;
-		forceUpdateFollowPath = false;
-		followCreature = creature;
-		isUpdatingPath = true;
-	} else {
-		isUpdatingPath = false;
-		followCreature = nullptr;
+void Creature::removeFollowCreature()
+{
+	followCreature = nullptr;
+	onUnfollowCreature();
+}
+
+bool Creature::canFollowCreature(Creature* creature)
+{
+	const auto& creaturePos = creature->getPosition();
+	if (creaturePos.z != getPosition().z) {
+		return false;
 	}
+	return canSee(creaturePos);
+}
 
-	onFollowCreature(creature);
-	return true;
+void Creature::onFollowCreature(const Creature*)
+{
+	if (!listWalkDir.empty()) {
+		listWalkDir.clear();
+		onWalkAborted();
+	}
+
+	hasFollowPath = false;
+	forceUpdateFollowPath = false;
+	isUpdatingPath = true;
 }
 
+void Creature::onUnfollowCreature() { isUpdatingPath = false; }
+
 double Creature::getDamageRatio(Creature* attacker) const
 {
 	uint32_t totalDamage = 0;

src/creature.h

@@ -190,14 +190,22 @@ class Creature : virtual public Thing
 
 	// follow functions
 	Creature* getFollowCreature() const { return followCreature; }
-	virtual bool setFollowCreature(Creature* creature);
+	virtual void setFollowCreature(Creature* creature);
+	virtual void removeFollowCreature();
+	virtual bool canFollowCreature(Creature* creature);
+	virtual bool isFollowingCreature(Creature* creature) { return followCreature == creature; }
 
 	// follow events
-	virtual void onFollowCreature(const Creature*) {}
+	virtual void onFollowCreature(const Creature*);
+	virtual void onUnfollowCreature();
 
 	// combat functions
 	Creature* getAttackedCreature() { return attackedCreature; }
-	virtual bool setAttackedCreature(Creature* creature);
+	virtual void setAttackedCreature(Creature* creature);
+	virtual void removeAttackedCreature();
+	virtual bool canAttackCreature(Creature* creature);
+	virtual bool isAttackingCreature(Creature* creature) { return attackedCreature == creature; }
+
 	virtual BlockType_t blockHit(Creature* attacker, CombatType_t combatType, int32_t& damage,
 	                             bool checkDefense = false, bool checkArmor = false, bool field = false,
 	                             bool ignoreResistances = false);

src/game.cpp

@@ -3232,14 +3232,14 @@ void Game::playerSetAttackedCreature(uint32_t playerId, uint32_t creatureId)
 	}
 
 	if (player->getAttackedCreature() && creatureId == 0) {
-		player->setAttackedCreature(nullptr);
+		player->removeAttackedCreature();
 		player->sendCancelTarget();
 		return;
 	}
 
 	Creature* attackCreature = getCreatureByID(creatureId);
 	if (!attackCreature) {
-		player->setAttackedCreature(nullptr);
+		player->removeAttackedCreature();
 		player->sendCancelTarget();
 		return;
 	}
@@ -3248,11 +3248,12 @@ void Game::playerSetAttackedCreature(uint32_t playerId, uint32_t creatureId)
 	if (ret != RETURNVALUE_NOERROR) {
 		player->sendCancelMessage(ret);
 		player->sendCancelTarget();
-		player->setAttackedCreature(nullptr);
+		player->removeAttackedCreature();
 		return;
 	}
 
 	player->setAttackedCreature(attackCreature);
+
 	g_dispatcher.addTask([this, id = player->getID()]() { updateCreatureWalk(id); });
 }
 
@@ -3263,9 +3264,15 @@ void Game::playerFollowCreature(uint32_t playerId, uint32_t creatureId)
 		return;
 	}
 
-	player->setAttackedCreature(nullptr);
+	player->removeAttackedCreature();
+
+	if (Creature* followCreature = getCreatureByID(creatureId)) {
+		player->setFollowCreature(followCreature);
+	} else {
+		player->removeFollowCreature();
+	}
+
 	g_dispatcher.addTask([this, id = player->getID()]() { updateCreatureWalk(id); });
-	player->setFollowCreature(getCreatureByID(creatureId));
 }
 
 void Game::playerSetFightModes(uint32_t playerId, fightMode_t fightMode, bool chaseMode, bool secureMode)

src/luascript.cpp

@@ -8116,11 +8116,20 @@ int LuaScriptInterface::luaCreatureGetTarget(lua_State* L)
 int LuaScriptInterface::luaCreatureSetTarget(lua_State* L)
 {
 	// creature:setTarget(target)
-	Creature* creature = tfs::lua::getUserdata<Creature>(L, 1);
-	if (creature) {
-		tfs::lua::pushBoolean(L, creature->setAttackedCreature(tfs::lua::getCreature(L, 2)));
-	} else {
+	auto creature = tfs::lua::getUserdata<Creature>(L, 1);
+	if (!creature) {
 		lua_pushnil(L);
+
+		return 1;
+	}
+
+	auto target = tfs::lua::getCreature(L, 2);
+	if (target) {
+		creature->setAttackedCreature(target);
+		tfs::lua::pushBoolean(L, creature->canAttackCreature(target));
+	} else {
+		creature->removeAttackedCreature();
+		tfs::lua::pushBoolean(L, true);
 	}
 	return 1;
 }
@@ -8147,11 +8156,19 @@ int LuaScriptInterface::luaCreatureGetFollowCreature(lua_State* L)
 int LuaScriptInterface::luaCreatureSetFollowCreature(lua_State* L)
 {
 	// creature:setFollowCreature(followedCreature)
-	Creature* creature = tfs::lua::getUserdata<Creature>(L, 1);
-	if (creature) {
-		tfs::lua::pushBoolean(L, creature->setFollowCreature(tfs::lua::getCreature(L, 2)));
-	} else {
+	auto creature = tfs::lua::getUserdata<Creature>(L, 1);
+	if (!creature) {
 		lua_pushnil(L);
+		return 1;
+	}
+
+	auto followedCreature = tfs::lua::getCreature(L, 2);
+	if (followedCreature) {
+		creature->setFollowCreature(followedCreature);
+		tfs::lua::pushBoolean(L, creature->canFollowCreature(followedCreature));
+	} else {
+		creature->removeFollowCreature();
+		tfs::lua::pushBoolean(L, true);
 	}
 	return 1;
 }

src/monster.cpp

@@ -680,11 +680,22 @@ bool Monster::selectTarget(Creature* creature)
 	}
 
 	if (isHostile() || isSummon()) {
-		if (setAttackedCreature(creature) && !isSummon()) {
-			g_dispatcher.addTask([id = getID()]() { g_game.checkCreatureAttack(id); });
+		if (canAttackCreature(creature)) {
+			setAttackedCreature(creature);
+
+			if (isHostile()) {
+				g_dispatcher.addTask([id = getID()]() { g_game.checkCreatureAttack(id); });
+			}
+		} else {
+			removeAttackedCreature();
 		}
 	}
-	return setFollowCreature(creature);
+
+	if (isFollowingCreature(creature) || canFollowCreature(creature)) {
+		setFollowCreature(creature);
+		return true;
+	}
+	return false;
 }
 
 void Monster::setIdle(bool idle)
@@ -788,7 +799,7 @@ void Monster::onThink(uint32_t interval)
 						setFollowCreature(getMaster());
 					}
 				} else if (attackedCreature == this) {
-					setFollowCreature(nullptr);
+					removeFollowCreature();
 				} else if (followCreature != attackedCreature) {
 					// This happens just after a master orders an attack, so lets follow it as well.
 					setFollowCreature(attackedCreature);
@@ -1853,7 +1864,7 @@ bool Monster::canWalkTo(Position pos, Direction direction) const
 
 void Monster::death(Creature*)
 {
-	setAttackedCreature(nullptr);
+	removeAttackedCreature();
 
 	for (Creature* summon : summons) {
 		summon->changeHealth(-summon->getHealth());

src/npc.cpp

@@ -897,7 +897,14 @@ int NpcScriptInterface::luaActionFollow(lua_State* L)
 		return 1;
 	}
 
-	tfs::lua::pushBoolean(L, npc->setFollowCreature(tfs::lua::getPlayer(L, 1)));
+	auto followedPlayer = tfs::lua::getPlayer(L, 1);
+	if (followedPlayer) {
+		npc->setFollowCreature(followedPlayer);
+		tfs::lua::pushBoolean(L, npc->canFollowCreature(followedPlayer));
+	} else {
+		npc->removeFollowCreature();
+		tfs::lua::pushBoolean(L, true);
+	}
 	return 1;
 }