feat(ahbot): whisper restock orders via separate clerk bot - #164
Conversation
Allow a configured player account to whisper item-link + quantity to an online clerk character; listings still use the offline AH seller GUID so random restock/bidding keep running. Fix Alliance/Horde house selection using character-cache race.
📝 WalkthroughWalkthroughWhisper orders can now be configured, routed to an online clerk, parsed from private messages, and fulfilled through auction listings. Seller-player lifecycle handling is centralized for both whisper orders and scheduled auction-bot updates. ChangesWhisper Order Restocking
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WhisperSender
participant AHBot_PlayerScript
participant AuctionHouseBot
participant TemporarySellerPlayer
participant AuctionHouse
WhisperSender->>AHBot_PlayerScript: Send private item and quantity whisper
AHBot_PlayerScript->>AHBot_PlayerScript: Validate receiver, account, and order syntax
AHBot_PlayerScript->>AuctionHouseBot: Call SellOrderedItem(itemId, quantity)
AuctionHouseBot->>TemporarySellerPlayer: Acquire seller player
AuctionHouseBot->>AuctionHouse: Create and persist auction listings
AuctionHouse-->>AuctionHouseBot: Return listing totals and errors
AuctionHouseBot-->>AHBot_PlayerScript: Return OrderResult
AHBot_PlayerScript-->>WhisperSender: Send localized result message
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
conf/mod_ahbot.conf.dist (1)
123-125: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider defaulting
WhisperOrdersto 0.Every other AHBot toggle in this file defaults to 0.
WhisperOrdersenables an item-creation path, so an opt-in default matches the module convention and the safer posture. The in-code global also initializes tofalseinsrc/AuctionHouseBotCommon.cpp, whilesConfigMgr->GetOptionusestrueas fallback; aligning all three removes the inconsistency.♻️ Proposed change
-AuctionHouseBot.WhisperOrders = 1 +AuctionHouseBot.WhisperOrders = 0Also update the doc block at Line 41 and the fallback in
AHBot_WorldScript::OnBeforeConfigLoad.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@conf/mod_ahbot.conf.dist` around lines 123 - 125, Default AuctionHouseBot.WhisperOrders to 0 in the distributed configuration and update its documentation entry to match. In AHBot_WorldScript::OnBeforeConfigLoad, change the GetOption fallback for WhisperOrders from true to false so it aligns with the false-initialized global and opt-in module defaults.src/AuctionHouseBot.cpp (1)
1136-1193: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftReduce duplication with
Sell.Lines 1141-1191 repeat the item-creation, pricing, deposit, and auction-persistence sequence of
Sellat Lines 801-929. The two copies already differ in small ways, for example the price fallback. Extract a shared private helper that takesconfig,prototype,stackCount, andAHBplayer, and returns the createdAuctionEntry. Both call sites then stay consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/AuctionHouseBot.cpp` around lines 1136 - 1193, Extract the duplicated item creation, pricing, deposit, and auction persistence flow from Sell and the shown listing loop into a shared private helper that accepts config, prototype, stackCount, and AHBplayer and returns the created AuctionEntry. Update both Sell and the loop to use this helper, preserving each operation’s transaction and result-accounting behavior while centralizing price fallback logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/AHBotWhisperOrderParse.h`:
- Around line 18-53: Add uint32_t overflow guards to the digit-accumulation
loops for itemId and quantity in the whisper parsing logic, rejecting the input
before multiplication and addition would wrap. Preserve the existing zero-value
and invalid-input checks, and apply the same validation consistently to both
loops.
In `@src/AuctionHouseBot.cpp`:
- Around line 1044-1134: The order flow in AuctionHouseBot.cpp currently stores
user-facing Chinese text directly in OrderResult::error; replace each
result.error literal in the shown validation and setup paths with a stable
OrderResult error enum. In AuctionHouseBotPlayerScript.cpp lines 62-89, add a
single catalog that maps those enum values and success cases to localized text
selected by GetSessionDbLocaleIndex(), and use it for all whisper responses
across both affected files.
- Around line 1062-1066: Update the auction price resolution near the logic
selecting BuyPrice or SellPrice based on config->UseBuyPriceForSeller: if the
selected field is zero, fall back to the other price field, then reject the
order when the resolved buyoutPrice remains zero. Ensure bidPrice is derived
only after this validation so auctions cannot be created with both startbid and
buyout set to zero.
In `@src/AuctionHouseBotPlayerScript.cpp`:
- Around line 47-57: Update the order flow around SellOrderedItem to select the
seller bot deterministically instead of using gBots.begin(), whose pointer
ordering is arbitrary. Resolve the bot by its configured GUID, or pass the
requested faction into the order so SellOrderedItem always uses the intended
auction house.
- Line 43: Update the non-matching clerk-whisper logging in
AuctionHouseBotPlayerScript to stop including the full message body. Keep the
log at INFO level, but report only the sender GUID and the parse-failure
context.
In `@src/AuctionHouseBotWorldScript.cpp`:
- Around line 165-193: Update the missing-character LOG_ERROR in the whisper
clerk login flow to state that gWhisperOrdersReceiver does not correspond to an
existing character GUID. Rework the one-shot _whisperLoginAttempted guard so the
clerk login check can run again after AddPlayerBot fails or the clerk later
disconnects, preserving the requirement that the clerk remains online.
- Around line 5-10: Guard the mod-playerbots-specific include and the
sRandomPlayerbotMgr.AddPlayerBot(...) call in AuctionHouseBotWorldScript using
the project’s build macro, and update README.md and module metadata to declare
mod-playerbots as a dependency so builds without it remain supported.
---
Nitpick comments:
In `@conf/mod_ahbot.conf.dist`:
- Around line 123-125: Default AuctionHouseBot.WhisperOrders to 0 in the
distributed configuration and update its documentation entry to match. In
AHBot_WorldScript::OnBeforeConfigLoad, change the GetOption fallback for
WhisperOrders from true to false so it aligns with the false-initialized global
and opt-in module defaults.
In `@src/AuctionHouseBot.cpp`:
- Around line 1136-1193: Extract the duplicated item creation, pricing, deposit,
and auction persistence flow from Sell and the shown listing loop into a shared
private helper that accepts config, prototype, stackCount, and AHBplayer and
returns the created AuctionEntry. Update both Sell and the loop to use this
helper, preserving each operation’s transaction and result-accounting behavior
while centralizing price fallback logic.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fb92444c-4f45-4567-a4bb-3e4e8ebbe8cf
📒 Files selected for processing (10)
conf/mod_ahbot.conf.distsrc/AHBotWhisperOrderParse.hsrc/AuctionHouseBot.cppsrc/AuctionHouseBot.hsrc/AuctionHouseBotCommon.cppsrc/AuctionHouseBotCommon.hsrc/AuctionHouseBotPlayerScript.cppsrc/AuctionHouseBotScript.cppsrc/AuctionHouseBotWorldScript.cppsrc/AuctionHouseBotWorldScript.h
| uint32_t itemId = 0; | ||
| while (pos < msg.size() && std::isdigit(static_cast<unsigned char>(msg[pos]))) | ||
| { | ||
| itemId = itemId * 10u + static_cast<uint32_t>(msg[pos] - '0'); | ||
| ++pos; | ||
| } | ||
| if (itemId == 0) | ||
| return false; | ||
|
|
||
| // Link form: |Hitem:...|h[Name]|h | ||
| std::size_t firstH = msg.find("|h", pos); | ||
| if (firstH == std::string::npos) | ||
| return false; | ||
| std::size_t secondH = msg.find("|h", firstH + 2); | ||
| if (secondH == std::string::npos) | ||
| return false; | ||
|
|
||
| pos = secondH + 2; | ||
| // Client links often end with |r before the quantity. | ||
| if (pos + 1 < msg.size() && msg[pos] == '|' && msg[pos + 1] == 'r') | ||
| pos += 2; | ||
|
|
||
| while (pos < msg.size() && std::isspace(static_cast<unsigned char>(msg[pos]))) | ||
| ++pos; | ||
|
|
||
| if (pos >= msg.size() || !std::isdigit(static_cast<unsigned char>(msg[pos]))) | ||
| return false; | ||
|
|
||
| uint32_t quantity = 0; | ||
| while (pos < msg.size() && std::isdigit(static_cast<unsigned char>(msg[pos]))) | ||
| { | ||
| quantity = quantity * 10u + static_cast<uint32_t>(msg[pos] - '0'); | ||
| ++pos; | ||
| } | ||
| if (quantity == 0) | ||
| return false; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard the digit accumulation against unsigned wraparound.
Both loops accumulate into uint32_t without a bound. A whisper that contains a long digit run wraps the value silently. Example: a quantity of 4294967297 becomes 1, so the caller lists a different amount than the sender requested. The same wrap applies to the item id.
Add an overflow check in each loop.
🛡️ Proposed fix
uint32_t itemId = 0;
while (pos < msg.size() && std::isdigit(static_cast<unsigned char>(msg[pos])))
{
+ if (itemId > (UINT32_MAX - static_cast<uint32_t>(msg[pos] - '0')) / 10u)
+ return false;
itemId = itemId * 10u + static_cast<uint32_t>(msg[pos] - '0');
++pos;
}Apply the same guard to the quantity loop. Include <cstdint> is already present.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/AHBotWhisperOrderParse.h` around lines 18 - 53, Add uint32_t overflow
guards to the digit-accumulation loops for itemId and quantity in the whisper
parsing logic, rejecting the input before multiplication and addition would
wrap. Preserve the existing zero-value and invalid-input checks, and apply the
same validation consistently to both loops.
| ItemTemplate const* prototype = sObjectMgr->GetItemTemplate(itemId); | ||
| if (!prototype) | ||
| { | ||
| result.error = "物品不存在"; | ||
| return result; | ||
| } | ||
| result.itemName = prototype->Name1; | ||
|
|
||
| if (prototype->Bonding == BIND_WHEN_PICKED_UP) | ||
| { | ||
| result.error = "拾取绑定物品不能上架"; | ||
| return result; | ||
| } | ||
| if (prototype->Bonding == BIND_QUEST_ITEM) | ||
| { | ||
| result.error = "任务物品不能上架"; | ||
| return result; | ||
| } | ||
| if (prototype->BuyPrice == 0 && prototype->SellPrice == 0) | ||
| { | ||
| result.error = "物品没有价格，无法上架"; | ||
| return result; | ||
| } | ||
| if (prototype->Quality > AHB_MAX_QUALITY) | ||
| { | ||
| result.error = "物品品质不受支持"; | ||
| return result; | ||
| } | ||
|
|
||
| uint32 maxStack = prototype->GetMaxStackSize(); | ||
| if (maxStack == 0) | ||
| maxStack = 1; | ||
|
|
||
| if (quantity == 0) | ||
| { | ||
| result.error = "数量无效"; | ||
| return result; | ||
| } | ||
| if (quantity > maxStack * 20u) | ||
| { | ||
| result.error = Acore::StringFormat("数量过大（上限 {}）", maxStack * 20u); | ||
| return result; | ||
| } | ||
|
|
||
| // Pick AH config: neutral-only when two-side AH is on; otherwise seller race. | ||
| // Do NOT use a temp Player::GetTeamId() after Initialize-only — that does not | ||
| // load race and was listing Alliance sellers onto the Horde AH (house 6). | ||
| AHBConfig* config = gNeutralConfig; | ||
| if (!sWorld->getBoolConfig(CONFIG_ALLOW_TWO_SIDE_INTERACTION_AUCTION)) | ||
| { | ||
| TeamId team = TEAM_NEUTRAL; | ||
| if (CharacterCacheEntry const* info = sCharacterCache->GetCharacterCacheByGuid( | ||
| ObjectGuid::Create<HighGuid::Player>(_id))) | ||
| team = Player::TeamIdForRace(info->Race); | ||
|
|
||
| if (team == TEAM_ALLIANCE) | ||
| config = gAllianceConfig; | ||
| else if (team == TEAM_HORDE) | ||
| config = gHordeConfig; | ||
| else | ||
| { | ||
| result.error = "无法确定拍卖行阵营"; | ||
| return result; | ||
| } | ||
| } | ||
|
|
||
| if (!config) | ||
| { | ||
| result.error = "拍卖行未就绪"; | ||
| return result; | ||
| } | ||
|
|
||
| AuctionHouseEntry const* ahEntry = sAuctionMgr->GetAuctionHouseEntryFromFactionTemplate(config->GetAHFID()); | ||
| AuctionHouseObject* auctionHouse = sAuctionMgr->GetAuctionsMap(config->GetAHFID()); | ||
| if (!ahEntry || !auctionHouse) | ||
| { | ||
| result.error = "拍卖行未就绪"; | ||
| return result; | ||
| } | ||
|
|
||
| std::string accountName = "AuctionHouseBot" + std::to_string(_account); | ||
| WorldSession session(_account, std::move(accountName), 0, nullptr, SEC_PLAYER, | ||
| sWorld->getIntConfig(CONFIG_EXPANSION), 0, LOCALE_enUS, 0, false, false, 0); | ||
| Player tempPlayer(&session); | ||
| bool addedToAccessor = false; | ||
| Player* AHBplayer = AcquireAHBplayer(tempPlayer, addedToAccessor); | ||
| if (!AHBplayer) | ||
| { | ||
| result.error = "拍卖机器人卖家在线冲突，无法上架"; | ||
| return result; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Centralize the user-facing whisper-order messages. Both files embed Chinese response text directly in code, while the module configuration and logs are English. The shared root cause is the absence of a message catalog and a locale mapping layer.
src/AuctionHouseBot.cpp#L1044-L1134: replace theresult.errorstring literals with a stable error enum onOrderResult.src/AuctionHouseBotPlayerScript.cpp#L62-L89: map the error enum and the success cases to text through a single catalog, keyed byGetSessionDbLocaleIndex().
📍 Affects 2 files
src/AuctionHouseBot.cpp#L1044-L1134(this comment)src/AuctionHouseBotPlayerScript.cpp#L62-L89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/AuctionHouseBot.cpp` around lines 1044 - 1134, The order flow in
AuctionHouseBot.cpp currently stores user-facing Chinese text directly in
OrderResult::error; replace each result.error literal in the shown validation
and setup paths with a stable OrderResult error enum. In
AuctionHouseBotPlayerScript.cpp lines 62-89, add a single catalog that maps
those enum values and success cases to localized text selected by
GetSessionDbLocaleIndex(), and use it for all whisper responses across both
affected files.
| if (prototype->BuyPrice == 0 && prototype->SellPrice == 0) | ||
| { | ||
| result.error = "物品没有价格，无法上架"; | ||
| return result; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent auctions with a zero start bid and zero buyout.
The validation at Lines 1062-1066 passes when only one of BuyPrice and SellPrice is non-zero. At Line 1156 the code selects a single field based on config->UseBuyPriceForSeller. If that selected field is 0 and the other one is non-zero, buyoutPrice stays 0. bidPrice then also becomes 0, and the auction is listed with startbid = 0 and buyout = 0. A player can take the stack for free.
Add a fallback to the other price field, and reject the order when the resolved price is 0.
🐛 Proposed fix
uint64 buyoutPrice = config->GetItemPrice(itemId);
if (buyoutPrice == 0)
buyoutPrice = config->UseBuyPriceForSeller ? prototype->BuyPrice : prototype->SellPrice;
+ if (buyoutPrice == 0)
+ buyoutPrice = config->UseBuyPriceForSeller ? prototype->SellPrice : prototype->BuyPrice;
buyoutPrice = buyoutPrice * urand(config->GetMinPrice(prototype->Quality), config->GetMaxPrice(prototype->Quality));
buyoutPrice = buyoutPrice / 100;
uint64 bidPrice = buyoutPrice * urand(config->GetMinBidPrice(prototype->Quality), config->GetMaxBidPrice(prototype->Quality));
bidPrice = bidPrice / 100;
+ if (bidPrice == 0)
+ bidPrice = 1;Also applies to: 1154-1161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/AuctionHouseBot.cpp` around lines 1062 - 1066, Update the auction price
resolution near the logic selecting BuyPrice or SellPrice based on
config->UseBuyPriceForSeller: if the selected field is zero, fall back to the
other price field, then reject the order when the resolved buyoutPrice remains
zero. Ensure bidPrice is derived only after this validation so auctions cannot
be created with both startbid and buyout set to zero.
| uint32_t quantity = 0; | ||
| if (!AHBotParseWhisperOrder(msg, itemId, quantity)) | ||
| { | ||
| LOG_INFO("module", "AHBot: whisper to clerk not an order (need item-link + quantity). msg='{}'", msg); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not log the whisper body.
This line writes the full private message text to the server log at INFO level for every non-matching whisper sent to the clerk. Private chat content is user data. Log only the sender GUID and the parse failure.
🔒 Proposed fix
- LOG_INFO("module", "AHBot: whisper to clerk not an order (need item-link + quantity). msg='{}'", msg);
+ LOG_DEBUG("module", "AHBot: whisper to clerk from account {} is not an order (need item-link + quantity)",
+ player->GetSession()->GetAccountId());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LOG_INFO("module", "AHBot: whisper to clerk not an order (need item-link + quantity). msg='{}'", msg); | |
| LOG_DEBUG("module", "AHBot: whisper to clerk from account {} is not an order (need item-link + quantity)", | |
| player->GetSession()->GetAccountId()); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/AuctionHouseBotPlayerScript.cpp` at line 43, Update the non-matching
clerk-whisper logging in AuctionHouseBotPlayerScript to stop including the full
message body. Keep the log at INFO level, but report only the sender GUID and
the parse-failure context.
| if (gBots.empty()) | ||
| { | ||
| ChatHandler handler(player->GetSession()); | ||
| handler.PSendSysMessage("上架失败：拍卖机器人未就绪"); | ||
| return false; | ||
| } | ||
|
|
||
| LOG_INFO("module", "AHBot: whisper order from {} item {} qty {}", player->GetName(), itemId, quantity); | ||
|
|
||
| // Listing is always performed by the AH seller bot(s), never by the clerk. | ||
| AuctionHouseBot::OrderResult result = (*gBots.begin())->SellOrderedItem(itemId, quantity); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Select the seller bot deterministically.
gBots is a std::set<AuctionHouseBot*>, so it is ordered by pointer value. *gBots.begin() therefore returns an arbitrary bot. SellOrderedItem derives the auction house faction from that bot's own character race. With more than one configured bot, the same order lists on the Alliance house in one run and on the Horde house in another. This contradicts the stated faction-selection fix.
Select the bot by a configured GUID, or accept a faction argument in the order.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/AuctionHouseBotPlayerScript.cpp` around lines 47 - 57, Update the order
flow around SellOrderedItem to select the seller bot deterministically instead
of using gBots.begin(), whose pointer ordering is arbitrary. Resolve the bot by
its configured GUID, or pass the requested faction into the order so
SellOrderedItem always uses the intended auction house.
| #include "CharacterCache.h" | ||
| #include "Config.h" | ||
| #include "Log.h" | ||
| #include "ObjectAccessor.h" | ||
| #include "ObjectGuid.h" | ||
| #include "RandomPlayerbotMgr.h" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the repository declares a playerbots dependency and how it is guarded.
fd -H -t f 'CMakeLists.txt|README.md|module.json|\.gitmodules' | while IFS= read -r f; do
echo "=== $f ==="; cat -n "$f"
done
rg -n 'RandomPlayerbotMgr|MOD_PLAYERBOTS|playerbot' -g '!*.md' .Repository: azerothcore/mod-ah-bot
Length of output: 3854
🏁 Script executed:
#!/bin/bash
set -u
echo "=== tracked module metadata/files ==="
git ls-files | rg '(^|/)(CMakeLists\.txt|README\.md|module\.json|\.gitmodules|.*\.(cmake|dist)$|.*\.h$|.*\.cpp$)' | sed -n '1,200p'
echo
echo "=== source outline ==="
ast-grep outline src/AuctionHouseBotWorldScript.cpp --view expanded || true
echo
echo "=== relevant source slices ==="
cat -n src/AuctionHouseBotWorldScript.cpp | sed -n '1,35p;140,210p'
echo
echo "=== build config references ==="
for f in $(git ls-files | rg '(^|/)(CMakeLists\.txt|.*\.cmake)$'); do
echo "--- $f ---"
rg -n 'playerbot|Playerbot|PLAYERBOT|modules|MOD_|add_library|add_subdirectory|OptionalModule|mod_ahbot|mod-ahbot' "$f" || true
doneRepository: azerothcore/mod-ah-bot
Length of output: 4915
🌐 Web query:
RandomPlayerbotMgr.h sRandomPlayerbotMgr AddPlayerBot ObjectGuid uint32 mod-playerbots
💡 Result:
In the mod-playerbots project, AddPlayerBot is a method inherited from the PlayerbotHolder class, which serves as the base for both PlayerbotMgr and RandomPlayerbotMgr [1][2]. The signature for the method is [1]: void AddPlayerBot(ObjectGuid guid, uint32 masterAccountId); Usage and Functionality: 1. Scope: AddPlayerBot is defined in PlayerbotHolder.h and implemented in PlayerbotMgr.cpp [1][3]. It is used to initiate the login process for a bot [2]. 2. Parameters: - ObjectGuid guid: The unique identifier for the character to be added as a bot [1][3]. - uint32 masterAccountId: The account ID of the master player controlling the bot [1][3]. If this is 0, the system treats it as a "Random Bot" (Type 1), managed by the server [2]. 3. Security Checks: Before a bot is added, the method performs several validations to ensure the master player is permitted to control the specific character [3][2]. These checks include verifying guild membership, account linking, or whether the character is configured as a permitted bot class [3][2]. 4. RandomPlayerbotMgr Implementation: In RandomPlayerbotMgr.cpp, this method is called within the ProcessBot function to log in random bots that are not currently in the world [4][5]. The macro sRandomPlayerbotMgr provides global access to the RandomPlayerbotMgr instance, which manages these server-controlled random bots [6][2].
Citations:
- 1: https://github.com/liyunfan1223/mod-playerbots/blob/c1222da8/src/PlayerbotMgr.h
- 2: https://deepwiki.com/liyunfan1223/mod-playerbots/4.2-bot-management-systems
- 3: https://github.com/liyunfan1223/mod-playerbots/blob/c1222da8/src/PlayerbotMgr.cpp
- 4: https://github.com/liyunfan1223/mod-playerbots/blob/c1222da8/src/RandomPlayerbotMgr.cpp
- 5: https://github.com/liyunfan1223/mod-playerbots/blob/299e4398/src/Bot/RandomPlayerbotMgr.cpp
- 6: https://github.com/liyunfan1223/mod-playerbots/blob/c1222da8/src/RandomPlayerbotMgr.h
Document and guard the mod-playerbots dependency.
include "RandomPlayerbotMgr.h" and sRandomPlayerbotMgr.AddPlayerBot(...) require mod-playerbots, but README.md and the module metadata do not declare it. Add the dependency to the installation/config, or surround the include and call with a build macro so mod-ahbot does not fail on builds without RandomPlayerbotMgr.h.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/AuctionHouseBotWorldScript.cpp` around lines 5 - 10, Guard the
mod-playerbots-specific include and the sRandomPlayerbotMgr.AddPlayerBot(...)
call in AuctionHouseBotWorldScript using the project’s build macro, and update
README.md and module metadata to declare mod-playerbots as a dependency so
builds without it remain supported.
| _whisperLoginAttempted = true; | ||
|
|
||
| // Never log in AuctionHouseBot.GUID (seller). That character must stay offline | ||
| // so classic temp-Player Update()/Sell()/Buy() keep running. Login a separate | ||
| // clerk character that only receives whisper orders. | ||
| if (!gWhisperOrdersReceiver) | ||
| { | ||
| LOG_ERROR("module", "AHBot: WhisperOrders enabled but WhisperOrdersReceiverGUID is 0"); | ||
| return; | ||
| } | ||
|
|
||
| if (gBotsId.find(gWhisperOrdersReceiver) != gBotsId.end()) | ||
| { | ||
| LOG_ERROR("module", "AHBot: WhisperOrdersReceiverGUID {} must not be an AH seller GUID", gWhisperOrdersReceiver); | ||
| return; | ||
| } | ||
|
|
||
| ObjectGuid guid = ObjectGuid::Create<HighGuid::Player>(gWhisperOrdersReceiver); | ||
| if (ObjectAccessor::FindConnectedPlayer(guid)) | ||
| return; | ||
|
|
||
| if (!sCharacterCache->GetCharacterAccountIdByGuid(guid)) | ||
| { | ||
| LOG_ERROR("module", "AHBot: failed to keep clerk character {} online for WhisperOrders; whispers will not work until it is logged in", gWhisperOrdersReceiver); | ||
| return; | ||
| } | ||
|
|
||
| LOG_INFO("module", "AHBot: WhisperOrders login clerk character {}", gWhisperOrdersReceiver); | ||
| sRandomPlayerbotMgr.AddPlayerBot(guid, 0); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Correct the error text and reconsider the one-shot login attempt.
Two points in this block:
- Line 186 checks character existence through
GetCharacterAccountIdByGuid. The message at Line 188 states that the module "failed to keep clerk character online". The real cause is a missing character for that GUID. Change the text to state that the GUID does not exist. _whisperLoginAttemptedis set at Line 165 before the login call. IfAddPlayerBotfails, or if the clerk logs out later, the module never retries. The comment at Lines 167-169 promises a clerk that stays online. If a periodic re-check is intended, reset the flag or re-run the check on an interval instead of once.
🔧 Proposed change for the message
if (!sCharacterCache->GetCharacterAccountIdByGuid(guid))
{
- LOG_ERROR("module", "AHBot: failed to keep clerk character {} online for WhisperOrders; whispers will not work until it is logged in", gWhisperOrdersReceiver);
+ LOG_ERROR("module", "AHBot: WhisperOrdersReceiverGUID {} does not exist; whisper orders are disabled", gWhisperOrdersReceiver);
return;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/AuctionHouseBotWorldScript.cpp` around lines 165 - 193, Update the
missing-character LOG_ERROR in the whisper clerk login flow to state that
gWhisperOrdersReceiver does not correspond to an existing character GUID. Rework
the one-shot _whisperLoginAttempted guard so the clerk login check can run again
after AddPlayerBot fails or the clerk later disconnects, preserving the
requirement that the clerk remains online.
1 participant
Summary
WhisperOrders,WhisperOrdersAccount,WhisperOrdersReceiverGUIDTest plan
Summary by CodeRabbit