Playerbot: defer quest push for not-in-world bots; tighten follow formation; grant bound quest items

- OnAcceptQuest parks class-equivalent quests for companions still mid-login
  (connected but not in-world); OnPlayerLogin drains them once the bot lands.
- Tighten AltFollow to match mod-playerbots (formation slots 1.5y, slot
  tolerance 2y, min recall radius 2y); default formation Spread (ring 2.5y).
- Fix O(n²) formation-slot reassignment in the companion tick (was re-iterating
  every alt for every alt every 250ms).
- QUEST_OBJECTIVE_FLAG_2_QUEST_BOUND_ITEM objectives are never stored in
  inventory and the core rejects AddItem for them ('inventory full or
  unplaceable' despite free space). Grant by ticking the objective counter via
  SetQuestObjectiveData instead, unblocking chains like Westfall 112 -> 114.

src/modules/PlayerbotV2/Bot/Formation.cpp

@@ -29,9 +29,11 @@ FormationOffset ComputeFormationOffset(
 
         case FormationType::Spread:
         {
-            // Ring 8y around leader; slots evenly spaced. With 8 slots
+            // Ring around leader; slots evenly spaced. With 8 slots
             // each gets 45°. Modulo 16 so >16 bots wrap (rare).
-            out.distance = 5.0f;
+            // Radius 2.5y (tightened from 5.0y to match mod-playerbots'
+            // surround feel — the default formation for companion squads).
+            out.distance = 2.5f;
             out.angle_radians = kPi * 2.0f * float(slot % 16) / 16.0f;
             return out;
         }

src/modules/PlayerbotV2/Bot/Formation.h

@@ -21,7 +21,7 @@ enum class FormationType : uint8_t
 {
     Free    = 0,   // Legacy — MotionMaster decides; no slot offset.
     Tight   = 1,   // All bots stack on top of leader.
-    Spread  = 2,   // Ring 8y around leader, slot * 45° each.
+    Spread  = 2,   // Ring 2.5y around leader, slot * 45° each.
     Line    = 3,   // Side-by-side behind leader, slots fan laterally.
     Column  = 4,   // Single-file behind leader.
     Wedge   = 5,   // V-shape behind leader; alternating flanks.

src/modules/PlayerbotV2/PlayerbotV2.cpp

@@ -213,6 +213,12 @@ bool BotIsHealerPlayer(Player* bot)
 
 namespace Playerbot::V2 {
 
+// Push an owner quest (class/race variant) onto a single in-world bot.
+// Defined near Module::OnAcceptQuest; forward-declared here because
+// Module::OnPlayerLogin (the deferred-quest drain) calls it earlier in
+// the file.
+static bool TryPushQuestToBot(Player* bot, Player* owner, uint32 quest_id);
+
 namespace {
 
 // Coarse spatial bucket key for the per-tick real-player proximity set.
@@ -485,6 +491,16 @@ constexpr uint32 kRescueRepeatWindowMs = 10u * 60u * 1000u;     // 10 min
 std::unordered_map<uint64, GlobalStuckState> g_global_stuck;
 std::unordered_map<uint64, uint32> g_lastLevelSyncMs;  // companion level-sync throttle
 
+// Deferred quest pushes: owner accepted a quest while a companion bot was
+// still logging in (connected but not yet in-world). OnAcceptQuest can't
+// push to a bot whose Player isn't in-world, so the quest is parked here
+// keyed by bot guid-low and drained in OnPlayerLogin once the bot lands.
+// Erased on bot logout; capped per bot so a quest-accept storm can't
+// grow the list without bound. (Regression since the module split — see
+// "[OnAcceptQuest] defer guid=… (not in world)".)
+std::unordered_map<uint64, std::vector<uint32>> g_pending_quests;
+constexpr size_t kPendingQuestCap = 20;
+
 constexpr uint32 kStuckThreshold      = 40;        // blocks in window (raised from 15 — favour path retries over rescue)
 constexpr uint32 kStuckWindowMs       = 60u * 1000u; // 60 s (raised from 30 s)
 constexpr uint32 kStuckRescueCooldown = 300u * 1000u; // 5 min (raised from 120 s)
@@ -1220,6 +1236,28 @@ void Module::OnWorldUpdate(std::chrono::milliseconds diff)
                 uint32 const account = sess->GetAccountId();
                 auto alts = Services::Altbots().AltsOfAccount(account);
 
+                // Formation slot assignment — ONCE per owner, before the
+                // per-bot loop. The previous placement sat INSIDE the
+                // `for (botId : alts)` loop and re-iterated every alt for
+                // every alt (O(n²)) every 250ms tick; with a growing squad
+                // that quadratically eats the world-thread budget. Slots
+                // are owner-relative and order-stable, so recomputing once
+                // per owner per tick is equivalent and O(n).
+                {
+                    uint8 formationIdx = 0;
+                    for (BotId b_id : alts)
+                    {
+                        if (!Services::Registry().has(b_id))
+                            continue;
+                        if (BotAI* b_ai = Services::Registry().ai(b_id))
+                        {
+                            if (b_ai->formation_type() == FormationType::Free)
+                                b_ai->set_formation_type(FormationType::Spread);
+                            b_ai->set_formation_slot(formationIdx++);
+                        }
+                    }
+                }
+
                 for (BotId const botId : alts)
                 {
                     if (!Services::Registry().has(botId))
@@ -1262,20 +1300,6 @@ void Module::OnWorldUpdate(std::chrono::milliseconds diff)
                         }
                     }
                     */
-                    {
-                        uint8 formationIdx = 0;
-                        for (BotId b_id : Services::Altbots().AltsOfAccount(account))
-                        {
-                            if (!Services::Registry().has(b_id))
-                                continue;
-                            if (BotAI* b_ai = Services::Registry().ai(b_id))
-                            {
-                                if (b_ai->formation_type() == FormationType::Free)
-                                    b_ai->set_formation_type(FormationType::Line);
-                                b_ai->set_formation_slot(formationIdx++);
-                            }
-                        }
-                    }
 
                     Player* bot = ObjectAccessor::FindConnectedPlayer(
                         ObjectGuid::Create<HighGuid::Player>(botId));
@@ -1424,9 +1448,13 @@ void Module::OnWorldUpdate(std::chrono::milliseconds diff)
                         continue;
                     }
 
-                    constexpr float kFormDist      = 2.0f;
+                    // Tighten to match mod-playerbots (followDistance=1.5y):
-                    constexpr float kSlotOkSq      = 4.0f * 4.0f;
+                    // formation slots sit 1.5y out, "in slot" within 2y,
-                    constexpr float kSlotCorrectSq = 14.0f * 14.0f;
+                    // and the minimum recall radius around the owner is 2y
+                    // (was 4y — bots trailed too loose).
+                    constexpr float kFormDist      = 1.5f;
+                    constexpr float kSlotOkSq      = 2.0f * 2.0f;
+                    constexpr float kSlotCorrectSq = 12.0f * 12.0f;
                     constexpr uint32 kSnapCD       = 1500;
 
                     float followDist  = kFormDist;
@@ -1478,7 +1506,7 @@ void Module::OnWorldUpdate(std::chrono::milliseconds diff)
                         float const odSq = ox * ox + oy * oy + oz * oz;
                         float const rad = (ft == FormationType::Free)
                             ? followDist
-                            : std::max(followDist, 4.0f);
+                            : std::max(followDist, 2.0f);
                         bool const inSlot =
                             (ft == FormationType::Free) || (slotDistSq <= kSlotOkSq);
                         if (!(inSlot && odSq < rad * rad))
@@ -2491,12 +2519,14 @@ void Module::OnPlayerLogin(Player* p)
 
                 // Assign formation slots so bots spread out when following
                 // instead of stacking on each other. Slot order matches the
-                // order bots were created. Defaults to Line (behind leader
+                // order bots were created. Defaults to Spread — a tight ring
-                // with lateral fan).
+                // around the leader (2.5y) matching mod-playerbots' surround
+                // feel; Line (behind-leader fan) remains available via
+                // /formation.
                 if (BotAI* ai = Services::Registry().ai(bot_id))
                 {
                     if (ai->formation_type() == FormationType::Free)
-                        ai->set_formation_type(FormationType::Line);
+                        ai->set_formation_type(FormationType::Spread);
                     ai->set_formation_slot(formationIdx++);
                 }
 
@@ -2604,6 +2634,36 @@ void Module::OnPlayerLogin(Player* p)
         : DefaultPersonality();
     Services::Registry().register_bot(id, personality, BotRng{SeedForBot(id)});
     Services::Scheduler().register_bot(id, ActivityTier::Idle);
+
+    // Deferred quest push: the owner accepted a quest while this bot was
+    // still logging in (connected, not yet in-world) and OnAcceptQuest
+    // parked it in g_pending_quests. Apply the class/race variant now that
+    // the bot is in-world so freshly-added companions don't miss the quest
+    // (regression since the module split — see "[OnAcceptQuest] defer…").
+    {
+        auto const pq = g_pending_quests.find(id);
+        if (pq != g_pending_quests.end())
+        {
+            OwnerBinding const ob = Services::Owners().GetOwner(id);
+            Player* owner = nullptr;
+            if (ob.player_guid)
+            {
+                owner = ObjectAccessor::FindConnectedPlayer(
+                    ObjectGuid::Create<HighGuid::Player>(ob.player_guid));
+                if (owner && !owner->IsInWorld())
+                    owner = nullptr;
+            }
+            uint32 drained = 0;
+            for (uint32 const qid : pq->second)
+                if (TryPushQuestToBot(p, owner, qid))
+                    ++drained;
+            TC_LOG_INFO("playerbot.v2",
+                "[OnAcceptQuest] drained {} deferred quest(s) for {}",
+                drained, p->GetName());
+            g_pending_quests.erase(pq);
+        }
+    }
+
     // Re-apply persisted squad state (formation type/slot, follow
     // distance, verbose flag) so a relog comes back with the same
     // owner-tunable preferences that were active before logout.
@@ -2713,6 +2773,9 @@ void Module::OnPlayerLogout(Player* p)
     Services::Scheduler().unregister_bot(id);
     Services::Snapshots().remove(id);
     Services::Registry().unregister_bot(id);
+    // Drop any quests deferred by OnAcceptQuest while this bot was offline
+    // — it's gone now, so the pending entry would otherwise leak.
+    g_pending_quests.erase(id);
     // Clean per-bot entries in file-scope maps. These keys are uint64
     // player guid_low; without erase, entries accumulate forever on
     // populations that rotate bots in/out (BotPopulationManager
@@ -2785,6 +2848,25 @@ static bool EnsureBotQuestItems(Player* bot, Quest const* quest)
 
         uint32 const itemId = uint32(obj.ObjectID);
         uint32 const need   = uint32(obj.Amount);
+
+        // QUEST_OBJECTIVE_FLAG_2_QUEST_BOUND_ITEM objectives are never
+        // stored in inventory — the counter is the source of truth and the
+        // core refuses StoreNewItem/AddItem for them (surfaced as
+        // "inventory full or unplaceable", permanently wedging chains like
+        // Westfall 112 → 114). Tick the objective instead of granting an item.
+        if (obj.Flags2 & QUEST_OBJECTIVE_FLAG_2_QUEST_BOUND_ITEM)
+        {
+            int32 const cur = bot->GetQuestObjectiveData(obj);
+            if (cur < int32(need))
+            {
+                bot->SetQuestObjectiveData(obj, need);
+                TC_LOG_ERROR("playerbot.v2",
+                    "[EnsureBotQuestItems] {} +{}x bound item={} quest={} ({}/{})",
+                    bot->GetName(), need - uint32(cur), itemId, quest->GetQuestId(), need, need);
+            }
+            continue;
+        }
+
         uint32 const have   = bot->GetItemCount(itemId, /*inBank*/ false);
 
         if (have >= need)
@@ -2866,6 +2948,7 @@ static bool GiveQuestItem(Player* bot, uint32 itemId, uint32 count)
 
     uint32 have = bot->GetItemCount(itemId, false);
     uint32 need = 0;
+    QuestObjective const* boundObj = nullptr;
 
     for (uint8 qs = 0; qs < MAX_QUEST_LOG_SIZE; ++qs)
     {
@@ -2887,6 +2970,8 @@ static bool GiveQuestItem(Player* bot, uint32 itemId, uint32 count)
                 continue;
             if (obj.Flags & QUEST_OBJECTIVE_FLAG_OPTIONAL)
                 continue;
+            if (obj.Flags2 & QUEST_OBJECTIVE_FLAG_2_QUEST_BOUND_ITEM)
+                boundObj = &obj;
             need = std::max(need, uint32(obj.Amount));
         }
     }
@@ -2906,6 +2991,27 @@ static bool GiveQuestItem(Player* bot, uint32 itemId, uint32 count)
         return false;
     }
 
+    // Quest-bound objectives (QUEST_OBJECTIVE_FLAG_2_QUEST_BOUND_ITEM) are
+    // not stored in inventory; the counter is the source of truth and the
+    // core rejects AddItem for them. Tick the counter directly.
+    if (boundObj)
+    {
+        int32 const cur = bot->GetQuestObjectiveData(*boundObj);
+        if (cur >= int32(need))
+        {
+            TC_LOG_ERROR("playerbot.v2",
+                "[QuestItemCopy] FULL {} bound item={} have={}/{}",
+                bot->GetName(), itemId, cur, need);
+            return false;
+        }
+        int32 const grant = std::min<int32>(int32(count), need - cur);
+        bot->SetQuestObjectiveData(*boundObj, cur + grant);
+        TC_LOG_ERROR("playerbot.v2",
+            "[QuestItemCopy] OK {} +{}x bound item={} ({}/{})",
+            bot->GetName(), grant, itemId, cur + grant, need);
+        return true;
+    }
+
     // GiveQuestItem uses count as-is — OnLootItem provides
     // the actual number taken by the owner. OnLootUnit is not
     // used for quest items (its li->count is unreliable).
@@ -3112,6 +3218,96 @@ void Module::OnPlayerAttack(Player* player, Unit* victim)
     }
 }
 
+// Push the owner's quest (class/race variant) onto a single bot. Returns
+// true when the bot ends the call holding the quest INCOMPLETE/COMPLETE.
+// `owner` may be null (deferred drain from OnPlayerLogin, owner offline) —
+// the group re-attach step is skipped in that case. Shared by the immediate
+// OnAcceptQuest path and the pending-quest drain so both give identical
+// class-equivalent/status checks.
+static bool TryPushQuestToBot(Player* bot, Player* owner, uint32 quest_id)
+{
+    if (!bot || !bot->IsInWorld())
+        return false;
+
+    // Class/race variant of owner's quest (e.g. 28767 → 28762 for another class)
+    uint32 const botQuestId = ResolveBotQuestId(bot, quest_id);
+    if (!botQuestId)
+    {
+        TC_LOG_ERROR("playerbot.v2",
+            "[OnAcceptQuest] skip {} (no class-equivalent for quest {})",
+            bot->GetName(), quest_id);
+        return false;
+    }
+
+    Quest const* botQuest = sObjectMgr->GetQuestTemplate(botQuestId);
+    if (!botQuest)
+        return false;
+
+    QuestStatus const st = bot->GetQuestStatus(botQuestId);
+    if (st == QUEST_STATUS_INCOMPLETE ||
+        st == QUEST_STATUS_COMPLETE ||
+        st == QUEST_STATUS_REWARDED)
+    {
+        TC_LOG_ERROR("playerbot.v2",
+            "[OnAcceptQuest] {} already status={} on quest {}",
+            bot->GetName(), uint32(st), botQuestId);
+        return false;
+    }
+
+    if (!bot->CanAddQuest(botQuest, false))
+    {
+        TC_LOG_ERROR("playerbot.v2",
+            "[OnAcceptQuest] skip {} quest {} (CanAddQuest)",
+            bot->GetName(), botQuestId);
+        return false;
+    }
+
+    if (!bot->SatisfyQuestClass(botQuest, false) ||
+        !bot->SatisfyQuestRace(botQuest, false))
+    {
+        TC_LOG_ERROR("playerbot.v2",
+            "[OnAcceptQuest] {} class/race soft-mismatch quest {} — pushing anyway",
+            bot->GetName(), botQuestId);
+    }
+
+    bot->AddQuestAndCheckCompletion(botQuest, nullptr);
+
+    if (botQuest->GetSrcSpell() > 0)
+        bot->CastSpell(bot, botQuest->GetSrcSpell(), true);
+
+    QuestStatus const after = bot->GetQuestStatus(botQuestId);
+    TC_LOG_ERROR("playerbot.v2",
+        "[OnAcceptQuest] {} ownerQ={} botQ={} status_after={}",
+        bot->GetName(), quest_id, botQuestId, uint32(after));
+
+    bool const pushed = (after == QUEST_STATUS_INCOMPLETE || after == QUEST_STATUS_COMPLETE);
+
+    // Keep bot in owner's group (skip when owner absent — deferred drain)
+    if (owner && owner->GetGroup())
+    {
+        Group* og = owner->GetGroup();
+        if (!bot->GetGroup())
+        {
+            if (!og->IsFull())
+            {
+                og->AddMember(bot);
+                og->SendUpdate();
+            }
+        }
+        else if (bot->GetGroup() != og)
+        {
+            bot->RemoveFromGroup();
+            if (!og->IsFull())
+            {
+                og->AddMember(bot);
+                og->SendUpdate();
+            }
+        }
+    }
+
+    return pushed;
+}
+
 void Module::OnAcceptQuest(Player* player, uint32 quest_id)
 {
     if (!initialized_ || !player)
@@ -3155,104 +3351,34 @@ void Module::OnAcceptQuest(Player* player, uint32 quest_id)
     }
 
     uint32 pushed = 0;
+    uint32 deferred = 0;
     for (BotId id : bots)
     {
         Player* bot = ObjectAccessor::FindConnectedPlayer(
             ObjectGuid::Create<HighGuid::Player>(id));
         if (!bot || !bot->IsInWorld())
         {
+            // Bot is mid-login (connected but not yet in-world). Park the
+            // quest so OnPlayerLogin drains it once the bot lands — a plain
+            // skip here permanently loses the quest for freshly-added bots
+            // (regression since the module split).
             TC_LOG_ERROR("playerbot.v2",
-                "[OnAcceptQuest] skip guid={} (not in world)", id);
+                "[OnAcceptQuest] defer guid={} (not in world)", id);
+            auto& q = g_pending_quests[id];
+            if (q.size() < kPendingQuestCap &&
+                std::find(q.begin(), q.end(), quest_id) == q.end())
+                q.push_back(quest_id);
+            ++deferred;
             continue;
         }
 
-        // Class/race variant of owner's quest (e.g. 28767 → 28762 for another class)
+        if (TryPushQuestToBot(bot, player, quest_id))
-        uint32 const botQuestId = ResolveBotQuestId(bot, quest_id);
-        if (!botQuestId)
-        {
-            TC_LOG_ERROR("playerbot.v2",
-                "[OnAcceptQuest] skip {} (no class-equivalent for quest {})",
-                bot->GetName(), quest_id);
-            continue;
-        }
-
-        Quest const* botQuest = sObjectMgr->GetQuestTemplate(botQuestId);
-        if (!botQuest)
-            continue;
-
-        QuestStatus const st = bot->GetQuestStatus(botQuestId);
-        if (st == QUEST_STATUS_INCOMPLETE ||
-            st == QUEST_STATUS_COMPLETE ||
-            st == QUEST_STATUS_REWARDED)
-        {
-            TC_LOG_ERROR("playerbot.v2",
-                "[OnAcceptQuest] {} already status={} on quest {}",
-                bot->GetName(), uint32(st), botQuestId);
-            continue;
-        }
-/*
-        if (bot->GetLevel() < player->GetLevel())
-        {
-            bot->GiveLevel(player->GetLevel());
-            bot->InitTalentForLevel();
-            // do not SetXP(0)
-        }
-*/
-        if (!bot->CanAddQuest(botQuest, false))
-        {
-            TC_LOG_ERROR("playerbot.v2",
-                "[OnAcceptQuest] skip {} quest {} (CanAddQuest)",
-                bot->GetName(), botQuestId);
-            continue;
-        }
-
-        if (!bot->SatisfyQuestClass(botQuest, false) ||
-            !bot->SatisfyQuestRace(botQuest, false))
-        {
-            TC_LOG_ERROR("playerbot.v2",
-                "[OnAcceptQuest] {} class/race soft-mismatch quest {} — pushing anyway",
-                bot->GetName(), botQuestId);
-        }
-
-        bot->AddQuestAndCheckCompletion(botQuest, nullptr);
-
-        if (botQuest->GetSrcSpell() > 0)
-            bot->CastSpell(bot, botQuest->GetSrcSpell(), true);
-
-        QuestStatus const after = bot->GetQuestStatus(botQuestId);
-        TC_LOG_ERROR("playerbot.v2",
-            "[OnAcceptQuest] {} ownerQ={} botQ={} status_after={}",
-            bot->GetName(), quest_id, botQuestId, uint32(after));
-
-        if (after == QUEST_STATUS_INCOMPLETE || after == QUEST_STATUS_COMPLETE)
             ++pushed;
-
-        // Keep bot in owner's group
-        if (Group* og = player->GetGroup())
-        {
-            if (!bot->GetGroup())
-            {
-                if (!og->IsFull())
-                {
-                    og->AddMember(bot);
-                    og->SendUpdate();
-                }
-            }
-            else if (bot->GetGroup() != og)
-            {
-                bot->RemoveFromGroup();
-                if (!og->IsFull())
-                {
-                    og->AddMember(bot);
-                    og->SendUpdate();
-                }
-            }
-        }
     }
 
     TC_LOG_ERROR("playerbot.v2",
-        "[OnAcceptQuest] owner={} quest={} pushed={}",
+        "[OnAcceptQuest] owner={} quest={} pushed={} deferred={}",
-        player->GetName(), quest_id, pushed);
+        player->GetName(), quest_id, pushed, deferred);
 }
 
 void Module::OnAreaExplored(Player* player, uint32 quest_id)
@@ -3460,7 +3586,7 @@ void Module::DrainCompanionFinalizes(uint32 now_ms)
         if (BotAI* ai = Services::Registry().ai(id))
         {
             if (ai->formation_type() == FormationType::Free)
-                ai->set_formation_type(FormationType::Line);
+                ai->set_formation_type(FormationType::Spread);
         }
     }
     pending_companion_finalize_.swap(still);
@@ -3674,7 +3800,7 @@ void Module::DrainAltFinalizes(uint32 /*now_ms*/)
             ai->set_owned(true);
             ai->set_role(BotRole::Altbot);
             if (ai->formation_type() == FormationType::Free)
-                ai->set_formation_type(FormationType::Line);
+                ai->set_formation_type(FormationType::Spread);
         }
 
         ApplyAltGear(bot, owner, false);