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?
This commit is contained in:
privatecore
2025-11-13 00:51:24 +01:00
committed by GitHub
parent a37dd2b9ae
commit 0729d14787

View File

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