From 0729d14787abc84ac4dc5d143e9c8eedf26c8923 Mon Sep 17 00:00:00 2001
From: privatecore <privatecorepp@gmail.com>
Date: Thu, 13 Nov 2025 00:51:24 +0100
Subject: [PATCH] Fix PlayerbotAI constructors' members order and wrong type
 comparison (uint32 -> int32) (#1763)

Fix warnings: -Wsign-compare and -Wsign-compare.

I would also like to mention that there are issues with the following
methods:

`IsMainTank` -- in the last loop, it returns the result of comparing the
first alive tank it finds with the current bot, which seems odd.
`IsBotMainTank` -- it is used in only two places in the code (Yogg-Saron
strategy) and has a completely different logic for checking. In the last
loop, it only checks for a suitable tank-bot with a lower index (if it's
not the bot itself). Additionally, there is a logic issue in the loop:
if none of the conditions are met after the first iteration, the method
returns `false`.

Can someone from the maintainers take a look at this section of the code
for possible errors and logic issues?
---
 src/PlayerbotAI.cpp | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/PlayerbotAI.cpp b/src/PlayerbotAI.cpp
index 4292a48b..44e601bf 100644
--- a/src/PlayerbotAI.cpp
+++ b/src/PlayerbotAI.cpp
@@ -106,14 +106,14 @@ void PacketHandlingHelper::AddPacket(WorldPacket const& packet)
 PlayerbotAI::PlayerbotAI()
     : PlayerbotAIBase(true),
       bot(nullptr),
+      master(nullptr),
+      accountId(0),
       aiObjectContext(nullptr),
       currentEngine(nullptr),
+      currentState(BOT_STATE_NON_COMBAT),
       chatHelper(this),
       chatFilter(this),
-      accountId(0),
-      security(nullptr),
-      master(nullptr),
-      currentState(BOT_STATE_NON_COMBAT)
+      security(nullptr)
 {
     for (uint8 i = 0; i < BOT_STATE_MAX; i++)
         engines[i] = nullptr;
@@ -128,9 +128,9 @@ PlayerbotAI::PlayerbotAI()
 PlayerbotAI::PlayerbotAI(Player* bot)
     : PlayerbotAIBase(true),
       bot(bot),
+      master(nullptr),
       chatHelper(this),
       chatFilter(this),
-      master(nullptr),
       security(bot)  // reorder args - whipowill
 {
     if (!bot->isTaxiCheater() && HasCheat((BotCheatMask::taxi)))
@@ -1801,11 +1801,11 @@ int32 PlayerbotAI::GetAssistTankIndex(Player* player)
     {
         return -1;
     }
+
     int counter = 0;
     for (GroupReference* ref = group->GetFirstMember(); ref; ref = ref->next())
     {
         Player* member = ref->GetSource();
-
         if (!member)
         {
             continue;
@@ -1815,11 +1815,13 @@ int32 PlayerbotAI::GetAssistTankIndex(Player* player)
         {
             return counter;
         }
+
         if (IsTank(member, true) && group->IsAssistant(member->GetGUID()))
         {
             counter++;
         }
     }
+
     return 0;
 }
 
@@ -2132,14 +2134,15 @@ bool PlayerbotAI::IsMainTank(Player* player)
             break;
         }
     }
+
     if (mainTank != ObjectGuid::Empty)
     {
         return player->GetGUID() == mainTank;
     }
+
     for (GroupReference* ref = group->GetFirstMember(); ref; ref = ref->next())
     {
         Player* member = ref->GetSource();
-
         if (!member)
         {
             continue;
@@ -2150,6 +2153,7 @@ bool PlayerbotAI::IsMainTank(Player* player)
             return player->GetGUID() == member->GetGUID();
         }
     }
+
     return false;
 }
 
@@ -2171,8 +2175,7 @@ bool PlayerbotAI::IsBotMainTank(Player* player)
         return true;  // If no group, consider the bot as main tank
     }
 
-    uint32 botAssistTankIndex = GetAssistTankIndex(player);
-
+    int32 botAssistTankIndex = GetAssistTankIndex(player);
     if (botAssistTankIndex == -1)
     {
         return false;
@@ -2186,8 +2189,7 @@ bool PlayerbotAI::IsBotMainTank(Player* player)
             continue;
         }
 
-        uint32 memberAssistTankIndex = GetAssistTankIndex(member);
-
+        int32 memberAssistTankIndex = GetAssistTankIndex(member);
         if (memberAssistTankIndex == -1)
         {
             continue;