mirror of
https://github.com/liyunfan1223/mod-playerbots.git
synced 2026-06-20 15:39:25 +02:00
Fix: Logout refactor and RNDbot logout error handling (#2131)
# Pull Request * Improper crash fix by 297f11d3e59f52a29f68188245e6b786c9fa838e. That fix worked not because setting `logout = true` stops the server from crashing, but because it stopped the following `if (!logout)` block from executing at all. In that block `delete target;` was the actual cause of the crashing and this was recreated in testing. `botWorldSessionPtr->LogoutPlayer(true);` already deleted target internally, then comes `delete target;` to delete already freed memory and the whole thing crashes. * Players had the ability to logout anyone's alt/addClass bots. Now there's a check to make sure command is from master. * When commanded to logout, RNDbots in player's party used to reply with "I'm logging out!", but they don't, because they shouldn't, because they are not alt/adClass bots. Now they say "You can't command me to logout!" * Added early exits for the "logout cancel" block, then I remembered bots were made to always instantly logout because of past issues with timed logout. Need to review whether or not we should re-implement timed logout, or if it's not worth it and its dead code removed with instant logout remaining the only option. --- ## Design Philosophy We prioritize **stability, performance, and predictability** over behavioral realism. Complex player-mimicking logic is intentionally limited due to its negative impact on scalability, maintainability, and long-term robustness. Excessive processing overhead can lead to server hiccups, increased CPU usage, and degraded performance for all participants. Because every action and decision tree is executed **per bot and per trigger**, even small increases in logic complexity can scale poorly and negatively affect both players and world (random) bots. Bots are not expected to behave perfectly, and perfect simulation of human decision-making is not a project goal. Increased behavioral realism often introduces disproportionate cost, reduced predictability, and significantly higher maintenance overhead. Every additional branch of logic increases long-term responsibility. All decision paths must be tested, validated, and maintained continuously as the system evolves. If advanced or AI-intensive behavior is introduced, the **default configuration must remain the lightweight decision model**. More complex behavior should only be available as an **explicit opt-in option**, clearly documented as having a measurable performance cost. Principles: - **Stability before intelligence** A stable system is always preferred over a smarter one. - **Performance is a shared resource** Any increase in bot cost affects all players and all bots. - **Simple logic scales better than smart logic** Predictable behavior under load is more valuable than perfect decisions. - **Complexity must justify itself** If a feature cannot clearly explain its cost, it should not exist. - **Defaults must be cheap** Expensive behavior must always be optional and clearly communicated. - **Bots should look reasonable, not perfect** The goal is believable behavior, not human simulation. Before submitting, confirm that this change aligns with those principles. --- ## Feature Evaluation Please answer the following: - Describe the **minimum logic** required to achieve the intended behavior? - Describe the **cheapest implementation** that produces an acceptable result? - Describe the **runtime cost** when this logic executes across many bots? This PR removes more code than it adds, and makes sure that exits happen as early as possible. It has no effect on processing power and makes the code slightly more maintainable. --- ## How to Test the Changes 1. Whisper `logout` to any bot whose master is not you. The bot can be RND/alt/addClass. The bot may have someone else as a master or may not have a master at all. The bot may be part of a party or not. Regardless, you are not its master. It should tell you "You are not my master!". Two players or two instances of the client from two different accounts are needed for this test, in order for Player A to command a bot to logout, when the bot's master is Player B. 2. Invite an RND bot to your party. As along as it's in your party, you are it's master, but RND bots cannot be logged out through chat commands. If you whisper `logout` to it, it should say "You can't command me to logout!", and not logout. 3. Whisper to an alt/addClass bot `logout`. The bot can be in your party or could've been uninvited. All that matters is that you are its master. It should reply "I'm logging out!" ## Complexity & Impact Does this change add new decision branches? - - [x] No - - [ ] Yes (**explain below**) Does this change increase per-bot or per-tick processing? - - [x] No - - [ ] Yes (**describe and justify impact**) Could this logic scale poorly under load? - - [x] No - - [ ] Yes (**explain why**) --- ## Defaults & Configuration Does this change modify default bot behavior? - - [ ] No - - [x] Yes (**explain why**) In that it fixes wrong behavior. If this introduces more advanced or AI-heavy logic: - - [x] Lightweight mode remains the default - - [x] More complex behavior is optional and thereby configurable --- ## AI Assistance Was AI assistance (e.g. ChatGPT or similar tools) used while working on this change? - - [ ] No - - [x] Yes (**explain below**) Used Claude for code review, and translations. If yes, please specify: - AI tool or model used (e.g. ChatGPT, GPT-4, Claude, etc.) - Purpose of usage (e.g. brainstorming, refactoring, documentation, code generation) - Which parts of the change were influenced or generated - Whether the result was manually reviewed and adapted AI assistance is allowed, but all submitted code must be fully understood, reviewed, and owned by the contributor. Any AI-influenced changes must be verified against existing CORE and PB logic. We expect contributors to be honest about what they do and do not understand. --- ## Lines to Translate These are keys and defaults of lines that were added/edited, and to be translated at a later SQL update. | Key | Default line | | --- | --- | | bot_not_your_master | You are not my master! | | bot_rndbot_no_logout | You can't command me to logout! | --- ## Final Checklist - - [x] Stability is not compromised - - [x] Performance impact is understood, tested, and acceptable - - [x] Added logic complexity is justified and explained - - [x] Documentation updated if needed --- ## Notes for Reviewers Anything that significantly improves realism at the cost of stability or performance should be carefully discussed before merging.
This commit is contained in:
parent
e27b429908
commit
c7ac849fbe
@ -0,0 +1,59 @@
|
|||||||
|
-- Translations for additional logout related messages
|
||||||
|
DELETE FROM ai_playerbot_texts WHERE name IN ('bot_not_your_master', 'bot_rndbot_no_logout');
|
||||||
|
DELETE FROM ai_playerbot_texts_chance WHERE name IN ('bot_not_your_master', 'bot_rndbot_no_logout');
|
||||||
|
|
||||||
|
INSERT INTO `ai_playerbot_texts`
|
||||||
|
(`id`, `name`, `text`, `say_type`, `reply_type`,
|
||||||
|
`text_loc1`, `text_loc2`, `text_loc3`, `text_loc4`,
|
||||||
|
`text_loc5`, `text_loc6`, `text_loc7`, `text_loc8`)
|
||||||
|
VALUES (
|
||||||
|
1740,
|
||||||
|
'bot_not_your_master',
|
||||||
|
"You are not my master!",
|
||||||
|
0, 0,
|
||||||
|
-- koKR
|
||||||
|
"당신은 내 주인이 아닙니다!",
|
||||||
|
-- frFR
|
||||||
|
"Tu n'es pas mon maître !",
|
||||||
|
-- deDE
|
||||||
|
"Du bist nicht mein Meister!",
|
||||||
|
-- zhCN
|
||||||
|
"你不是我的主人!",
|
||||||
|
-- zhTW
|
||||||
|
"你不是我的主人!",
|
||||||
|
-- esES
|
||||||
|
"¡No eres mi amo!",
|
||||||
|
-- esMX
|
||||||
|
"¡No eres mi amo!",
|
||||||
|
-- ruRU
|
||||||
|
"Ты не мой хозяин!");
|
||||||
|
|
||||||
|
INSERT INTO ai_playerbot_texts_chance (name, probability) VALUES ('bot_not_your_master', 100);
|
||||||
|
|
||||||
|
INSERT INTO `ai_playerbot_texts`
|
||||||
|
(`id`, `name`, `text`, `say_type`, `reply_type`,
|
||||||
|
`text_loc1`, `text_loc2`, `text_loc3`, `text_loc4`,
|
||||||
|
`text_loc5`, `text_loc6`, `text_loc7`, `text_loc8`)
|
||||||
|
VALUES (
|
||||||
|
1741,
|
||||||
|
'bot_rndbot_no_logout',
|
||||||
|
"You can't command me to logout!",
|
||||||
|
0, 0,
|
||||||
|
-- koKR
|
||||||
|
"당신은 나에게 로그아웃을 명령할 수 없습니다!",
|
||||||
|
-- frFR
|
||||||
|
"Tu ne peux pas m'ordonner de me déconnecter !",
|
||||||
|
-- deDE
|
||||||
|
"Du kannst mir nicht befehlen, mich auszuloggen!",
|
||||||
|
-- zhCN
|
||||||
|
"你不能命令我下线!",
|
||||||
|
-- zhTW
|
||||||
|
"你不能命令我登出!",
|
||||||
|
-- esES
|
||||||
|
"¡No puedes ordenarme que cierre sesión!",
|
||||||
|
-- esMX
|
||||||
|
"¡No puedes ordenarme que cierre sesión!",
|
||||||
|
-- ruRU
|
||||||
|
"Ты не можешь приказать мне выйти из игры!");
|
||||||
|
|
||||||
|
INSERT INTO ai_playerbot_texts_chance (name, probability) VALUES ('bot_rndbot_no_logout', 100);
|
||||||
@ -1015,29 +1015,59 @@ void PlayerbotAI::HandleCommand(uint32 type, std::string const text, Player* fro
|
|||||||
}
|
}
|
||||||
else if (filtered == "logout")
|
else if (filtered == "logout")
|
||||||
{
|
{
|
||||||
if (!bot->GetSession()->isLogingOut())
|
if (bot->GetSession()->isLogingOut())
|
||||||
|
return;
|
||||||
|
|
||||||
|
// Verify the command came from this bot's master. Also handles nullptr
|
||||||
|
if (fromPlayer != master)
|
||||||
{
|
{
|
||||||
if (type == CHAT_MSG_WHISPER)
|
if (type == CHAT_MSG_WHISPER)
|
||||||
TellMaster("I'm logging out!");
|
{
|
||||||
|
std::string message = PlayerbotTextMgr::instance().GetBotTextOrDefault(
|
||||||
|
"bot_not_your_master", "You are not my master!", {});
|
||||||
|
bot->Whisper(message, LANG_UNIVERSAL, fromPlayer);
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
PlayerbotMgr* masterBotMgr = GET_PLAYERBOT_MGR(master);
|
||||||
|
if (!masterBotMgr)
|
||||||
|
return;
|
||||||
|
|
||||||
|
// Only respond if this bot is in master's collection (alt/addclass)
|
||||||
|
if (masterBotMgr->GetPlayerBot(bot->GetGUID()))
|
||||||
|
{
|
||||||
|
if (type == CHAT_MSG_WHISPER)
|
||||||
|
{
|
||||||
|
std::string message = PlayerbotTextMgr::instance().GetBotTextOrDefault(
|
||||||
|
"logout_start", "I'm logging out!", {});
|
||||||
|
TellMaster(message);
|
||||||
|
}
|
||||||
|
|
||||||
PlayerbotMgr* masterBotMgr = nullptr;
|
|
||||||
if (master)
|
|
||||||
masterBotMgr = GET_PLAYERBOT_MGR(master);
|
|
||||||
if (masterBotMgr)
|
|
||||||
masterBotMgr->LogoutPlayerBot(bot->GetGUID());
|
masterBotMgr->LogoutPlayerBot(bot->GetGUID());
|
||||||
}
|
}
|
||||||
|
else if (type == CHAT_MSG_WHISPER)
|
||||||
|
{
|
||||||
|
std::string message = PlayerbotTextMgr::instance().GetBotTextOrDefault(
|
||||||
|
"bot_rndbot_no_logout", "You can't command me to logout!", {});
|
||||||
|
TellMaster(message);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
else if (filtered == "logout cancel")
|
else if (filtered == "logout cancel")
|
||||||
{
|
{
|
||||||
if (bot->GetSession()->isLogingOut())
|
if (!bot->GetSession()->isLogingOut())
|
||||||
{
|
return;
|
||||||
|
|
||||||
if (type == CHAT_MSG_WHISPER)
|
if (type == CHAT_MSG_WHISPER)
|
||||||
TellMaster("Logout cancelled!");
|
{
|
||||||
|
std::string message = PlayerbotTextMgr::instance().GetBotTextOrDefault(
|
||||||
|
"logout_cancel", "Logout cancelled!", {});
|
||||||
|
TellMaster(message);
|
||||||
|
}
|
||||||
|
|
||||||
WorldPackets::Character::LogoutCancel data = WorldPacket(CMSG_LOGOUT_CANCEL);
|
WorldPackets::Character::LogoutCancel data = WorldPacket(CMSG_LOGOUT_CANCEL);
|
||||||
bot->GetSession()->HandleLogoutCancelOpcode(data);
|
bot->GetSession()->HandleLogoutCancelOpcode(data);
|
||||||
}
|
}
|
||||||
}
|
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
chatCommands.push_back(ChatCommandHolder(filtered, fromPlayer, type));
|
chatCommands.push_back(ChatCommandHolder(filtered, fromPlayer, type));
|
||||||
|
|||||||
@ -362,6 +362,9 @@ void PlayerbotHolder::LogoutPlayerBot(ObjectGuid guid)
|
|||||||
if (master)
|
if (master)
|
||||||
masterWorldSessionPtr = master->GetSession();
|
masterWorldSessionPtr = master->GetSession();
|
||||||
|
|
||||||
|
// TODO: Review whether or not to implement timed logout.
|
||||||
|
// Unused block. Useful only for timed logout.
|
||||||
|
/*
|
||||||
// check for instant logout
|
// check for instant logout
|
||||||
bool logout = botWorldSessionPtr->ShouldLogOut(time(nullptr));
|
bool logout = botWorldSessionPtr->ShouldLogOut(time(nullptr));
|
||||||
|
|
||||||
@ -373,58 +376,19 @@ void PlayerbotHolder::LogoutPlayerBot(ObjectGuid guid)
|
|||||||
|
|
||||||
if (bot->HasFlag(PLAYER_FLAGS, PLAYER_FLAGS_RESTING) || bot->HasUnitState(UNIT_STATE_IN_FLIGHT) ||
|
if (bot->HasFlag(PLAYER_FLAGS, PLAYER_FLAGS_RESTING) || bot->HasUnitState(UNIT_STATE_IN_FLIGHT) ||
|
||||||
botWorldSessionPtr->GetSecurity() >= (AccountTypes)sWorld->getIntConfig(CONFIG_INSTANT_LOGOUT))
|
botWorldSessionPtr->GetSecurity() >= (AccountTypes)sWorld->getIntConfig(CONFIG_INSTANT_LOGOUT))
|
||||||
{
|
|
||||||
logout = true;
|
logout = true;
|
||||||
}
|
|
||||||
|
|
||||||
if (master &&
|
if (master &&
|
||||||
(master->HasFlag(PLAYER_FLAGS, PLAYER_FLAGS_RESTING) || master->HasUnitState(UNIT_STATE_IN_FLIGHT) ||
|
(master->HasFlag(PLAYER_FLAGS, PLAYER_FLAGS_RESTING) || master->HasUnitState(UNIT_STATE_IN_FLIGHT) ||
|
||||||
(masterWorldSessionPtr &&
|
(masterWorldSessionPtr &&
|
||||||
masterWorldSessionPtr->GetSecurity() >= (AccountTypes)sWorld->getIntConfig(CONFIG_INSTANT_LOGOUT))))
|
masterWorldSessionPtr->GetSecurity() >= (AccountTypes)sWorld->getIntConfig(CONFIG_INSTANT_LOGOUT))))
|
||||||
{
|
|
||||||
logout = true;
|
logout = true;
|
||||||
}
|
*/
|
||||||
|
// Instant logout (the only option right now)
|
||||||
TravelTarget* target = nullptr;
|
|
||||||
if (botAI->GetAiObjectContext()) // Maybe some day re-write to delate all pointer values.
|
|
||||||
{
|
{
|
||||||
target = botAI->GetAiObjectContext()->GetValue<TravelTarget*>("travel target")->Get();
|
std::string message = PlayerbotTextMgr::instance().GetBotTextOrDefault(
|
||||||
}
|
"goodbye", "Goodbye!", {});
|
||||||
|
botAI->TellMaster(message);
|
||||||
// Peiru: Allow bots to always instant logout to see if this resolves logout crashes
|
|
||||||
logout = true;
|
|
||||||
|
|
||||||
// if no instant logout, request normal logout
|
|
||||||
if (!logout)
|
|
||||||
{
|
|
||||||
if (bot->GetSession()->isLogingOut())
|
|
||||||
return;
|
|
||||||
else if (bot)
|
|
||||||
{
|
|
||||||
botAI->TellMaster("I'm logging out!");
|
|
||||||
WorldPackets::Character::LogoutRequest data = WorldPacket(CMSG_LOGOUT_REQUEST);
|
|
||||||
botWorldSessionPtr->HandleLogoutRequestOpcode(data);
|
|
||||||
if (!bot)
|
|
||||||
{
|
|
||||||
RemoveFromPlayerbotsMap(guid);
|
|
||||||
delete botWorldSessionPtr;
|
|
||||||
if (target)
|
|
||||||
delete target;
|
|
||||||
}
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
else
|
|
||||||
{
|
|
||||||
RemoveFromPlayerbotsMap(guid); // deletes bot player ptr inside this WorldSession PlayerBotMap
|
|
||||||
delete botWorldSessionPtr; // finally delete the bot's WorldSession
|
|
||||||
if (target)
|
|
||||||
delete target;
|
|
||||||
}
|
|
||||||
return;
|
|
||||||
} // if instant logout possible, do it
|
|
||||||
else if (bot && (logout || !botWorldSessionPtr->isLogingOut()))
|
|
||||||
{
|
|
||||||
botAI->TellMaster("Goodbye!");
|
|
||||||
RemoveFromPlayerbotsMap(guid); // deletes bot player ptr inside this WorldSession PlayerBotMap
|
RemoveFromPlayerbotsMap(guid); // deletes bot player ptr inside this WorldSession PlayerBotMap
|
||||||
botWorldSessionPtr->LogoutPlayer(true); // this will delete the bot Player object and PlayerbotAI object
|
botWorldSessionPtr->LogoutPlayer(true); // this will delete the bot Player object and PlayerbotAI object
|
||||||
delete botWorldSessionPtr; // finally delete the bot's WorldSession
|
delete botWorldSessionPtr; // finally delete the bot's WorldSession
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user