[Fix issue #1527] : startup crash in tank target selection — add TOCTOU & null-safety guards (#1532)

* 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

@@ -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<Unit*>("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);
 }

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<GuidVector>("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<GuidVector>("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();
 }