From c6b0424c29b6a1bf5b3574135128d30d19838411 Mon Sep 17 00:00:00 2001 From: Alex Dcnh <140754794+Wishmaster117@users.noreply.github.com> Date: Mon, 11 Aug 2025 17:00:31 +0200 Subject: [PATCH] =?UTF-8?q?[Fix=20issue=20#1527]=20:=20startup=20crash=20i?= =?UTF-8?q?n=20tank=20target=20selection=20=E2=80=94=20add=20TOCTOU=20&=20?= =?UTF-8?q?null-safety=20guards=20(#1532)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Harden playerbot logout & packet dispatch; add null-safety in chat hooks and RPG checks * Fix Issue 1528 * Fix Issue #1527 --- src/strategy/values/TankTargetValue.cpp | 126 +++++++++++++++++++++++- src/strategy/values/TargetValue.cpp | 24 ++++- 2 files changed, 144 insertions(+), 6 deletions(-) diff --git a/src/strategy/values/TankTargetValue.cpp b/src/strategy/values/TankTargetValue.cpp index bcef1a3a..048ef8a5 100644 --- a/src/strategy/values/TankTargetValue.cpp +++ b/src/strategy/values/TankTargetValue.cpp @@ -14,7 +14,7 @@ class FindTargetForTankStrategy : public FindNonCcTargetStrategy public: FindTargetForTankStrategy(PlayerbotAI* botAI) : FindNonCcTargetStrategy(botAI), minThreat(0) {} - void CheckAttacker(Unit* creature, ThreatMgr* threatMgr) override + /*void CheckAttacker(Unit* creature, ThreatMgr* threatMgr) override { if (!creature || !creature->IsAlive()) { @@ -37,6 +37,43 @@ public: return; } } + if (minThreat >= threat) + { + minThreat = threat; + result = creature; + } + }*/ + + void CheckAttacker(Unit* creature, ThreatMgr* threatMgr) override + { + // [Crash fix] Filter out anything that is not ready/valid + if (!creature || !creature->IsAlive() || !creature->IsInWorld() || creature->IsDuringRemoveFromWorld()) + return; + + if (!threatMgr) + return; + + Player* bot = botAI->GetBot(); + if (!bot) + return; + + float threat = threatMgr->GetThreat(bot); + + if (!result || !result->IsAlive() || !result->IsInWorld() || result->IsDuringRemoveFromWorld()) + { + // [Crash fix] If the previous target has become invalid, restart cleanly + minThreat = threat; + result = creature; + } + + // Neglect si la victime actuelle est le MT (ou s'il n'y a pas de victime) + if (HostileReference* cv = threatMgr->getCurrentVictim()) + { + Unit* victim = cv->getTarget(); + if (victim && victim->ToPlayer() && botAI->IsMainTank(victim->ToPlayer())) + return; + } + if (minThreat >= threat) { minThreat = threat; @@ -53,7 +90,7 @@ class FindTankTargetSmartStrategy : public FindTargetStrategy public: FindTankTargetSmartStrategy(PlayerbotAI* botAI) : FindTargetStrategy(botAI) {} - void CheckAttacker(Unit* attacker, ThreatMgr* threatMgr) override + /*void CheckAttacker(Unit* attacker, ThreatMgr* threatMgr) override { if (Group* group = botAI->GetBot()->GetGroup()) { @@ -69,8 +106,32 @@ public: { result = attacker; } + }*/ + void CheckAttacker(Unit* attacker, ThreatMgr* /*threatMgr*/) override + { + // [Crash fix] Protect against null/out-of-world/being-removed units + if (!attacker || !attacker->IsAlive() || !attacker->IsInWorld() || attacker->IsDuringRemoveFromWorld()) + return; + + if (Player* me = botAI->GetBot()) + { + if (Group* group = me->GetGroup()) + { + ObjectGuid guid = group->GetTargetIcon(4); + if (guid && attacker->GetGUID() == guid) + return; + } + } + + // [Crash fix] If 'result' has become invalid, forget it + if (result && (!result->IsAlive() || !result->IsInWorld() || result->IsDuringRemoveFromWorld())) + result = nullptr; + + if (!result || IsBetter(attacker, result)) + result = attacker; } - bool IsBetter(Unit* new_unit, Unit* old_unit) + + /*bool IsBetter(Unit* new_unit, Unit* old_unit) { Player* bot = botAI->GetBot(); // if group has multiple tanks, main tank just focus on the current target @@ -97,8 +158,47 @@ public: return new_dis < old_dis; } return new_threat < old_threat; + }*/ + bool IsBetter(Unit* new_unit, Unit* old_unit) + { + // [Crash fix] If either one is invalid, decide straight away + if (!new_unit || !new_unit->IsAlive() || !new_unit->IsInWorld() || new_unit->IsDuringRemoveFromWorld()) + return false; + if (!old_unit || !old_unit->IsAlive() || !old_unit->IsInWorld() || old_unit->IsDuringRemoveFromWorld()) + return true; + + Player* bot = botAI->GetBot(); + if (!bot) + return false; + + // if multiple tanks, logically focus on the current target + Unit* currentTarget = botAI->GetAiObjectContext()->GetValue("current target")->Get(); + if (currentTarget && botAI->IsMainTank(bot) && botAI->GetGroupTankNum(bot) > 1) + { + if (old_unit == currentTarget) + return false; + if (new_unit == currentTarget) + return true; + } + + float new_threat = new_unit->GetThreatMgr().GetThreat(bot); + float old_threat = old_unit->GetThreatMgr().GetThreat(bot); + float new_dis = bot->GetDistance(new_unit); + float old_dis = bot->GetDistance(old_unit); + + // hasAggro? -> withinMelee? -> threat + int nl = GetIntervalLevel(new_unit); + int ol = GetIntervalLevel(old_unit); + if (nl != ol) + return nl > ol; + + if (nl == 2) + return new_dis < old_dis; + + return new_threat < old_threat; } - int32_t GetIntervalLevel(Unit* unit) + + /*int32_t GetIntervalLevel(Unit* unit) { if (!botAI->HasAggro(unit)) { @@ -109,12 +209,28 @@ public: return 1; } return 0; + }*/ + int32_t GetIntervalLevel(Unit* unit) + { + // [Crash fix] Basic guards + if (!unit || !unit->IsAlive() || !unit->IsInWorld() || unit->IsDuringRemoveFromWorld()) + return 0; + + if (!botAI->HasAggro(unit)) + return 2; + + if (Player* bot = botAI->GetBot()) + { + if (bot->IsWithinMeleeRange(unit)) + return 1; + } + return 0; } }; Unit* TankTargetValue::Calculate() { - // FindTargetForTankStrategy strategy(botAI); + // [Note] Using the "smart" strategy below. Guards have been added in CheckAttacker/IsBetter. FindTankTargetSmartStrategy strategy(botAI); return FindTarget(&strategy); } diff --git a/src/strategy/values/TargetValue.cpp b/src/strategy/values/TargetValue.cpp index ebffd328..9b613681 100644 --- a/src/strategy/values/TargetValue.cpp +++ b/src/strategy/values/TargetValue.cpp @@ -14,7 +14,7 @@ Unit* FindTargetStrategy::GetResult() { return result; } -Unit* TargetValue::FindTarget(FindTargetStrategy* strategy) +/*Unit* TargetValue::FindTarget(FindTargetStrategy* strategy) { GuidVector attackers = botAI->GetAiObjectContext()->GetValue("attackers")->Get(); for (ObjectGuid const guid : attackers) @@ -27,6 +27,28 @@ Unit* TargetValue::FindTarget(FindTargetStrategy* strategy) strategy->CheckAttacker(unit, &ThreatMgr); } + return strategy->GetResult(); +}*/ + +Unit* TargetValue::FindTarget(FindTargetStrategy* strategy) +{ + // [Crash fix] The very first AI tick can occur before everything is "in world". + // Filter out units that are non-living / being removed / out of world. + AiObjectContext* ctx = botAI->GetAiObjectContext(); + if (!ctx) + return strategy->GetResult(); + + GuidVector attackers = ctx->GetValue("attackers")->Get(); + for (ObjectGuid const& guid : attackers) + { + Unit* unit = botAI->GetUnit(guid); + if (!unit || !unit->IsAlive() || !unit->IsInWorld() || unit->IsDuringRemoveFromWorld()) + continue; + + ThreatMgr& threatMgrRef = unit->GetThreatMgr(); + strategy->CheckAttacker(unit, &threatMgrRef); + } + return strategy->GetResult(); }