LFG: make the dungeon finder work end to end, and fix what it uncovered

src/game/ChatCommands/DebugCommands.cpp

@@ -35,6 +35,7 @@
  */
 
 #include "Common.h"
+#include "LFGMgr.h"
 #include "Database/DatabaseEnv.h"
 #include "WorldPacket.h"
 #include "Player.h"
@@ -1057,6 +1058,80 @@ bool ChatHandler::HandleDebugGetItemStateCommand(char* args)
  * @param args Command arguments.
  * @returns True if the command executed successfully, false otherwise.
  */
+/**
+ * @brief Handler for the `.debug dungeon` command.
+ *
+ * Mirrors `.debug bg`, which lets a battleground start 1v0 so it can be tested without
+ * finding nineteen other people. The dungeon finder has the same problem and worse: a
+ * normal five-man will not form until 1 tank, 1 healer and 3 damage are all present, so
+ * the proposal, group-creation and teleport paths are unreachable on a test realm.
+ *
+ *   .debug dungeon         a game master's queue entry completes on its own
+ *   .debug dungeon group   as above, and it also absorbs whoever else is waiting,
+ *                          whatever roles they picked
+ *   .debug dungeon off     back to normal matchmaking
+ *
+ * While any mode is active a game master leads the resulting dungeon group regardless of
+ * who holds the leader bit.
+ *
+ * The relaxations are scoped to entries that actually contain a game master, so ordinary
+ * players continue to match each other by the normal rules while this is on.
+ *
+ * @param args "group", "off", or empty for solo mode.
+ * @returns True if the command executed successfully, false otherwise.
+ */
+bool ChatHandler::HandleDebugDungeonCommand(char* args)
+{
+    char* mode = ExtractLiteralArg(&args);
+
+    LFGDebugMode newMode = LFG_DEBUG_SOLO;
+    if (mode)
+    {
+        if (!stricmp(mode, "group"))
+        {
+            newMode = LFG_DEBUG_GROUP;
+        }
+        else if (!stricmp(mode, "off"))
+        {
+            newMode = LFG_DEBUG_OFF;
+        }
+        else
+        {
+            SendSysMessage("Usage: .debug dungeon [group|off]");
+            SetSentErrorMessage(true);
+            return false;
+        }
+    }
+    else if (sLFGMgr.GetDebugMode() != LFG_DEBUG_OFF)
+    {
+        // Bare `.debug dungeon` toggles off when something is already on, so the command
+        // behaves like `.debug bg` when used without arguments.
+        newMode = LFG_DEBUG_OFF;
+    }
+
+    sLFGMgr.SetDebugMode(newMode);
+
+    switch (newMode)
+    {
+        case LFG_DEBUG_SOLO:
+            SendSysMessage("Dungeon finder debug ON: a game master's queue entry now forms a group on its own.");
+            break;
+        case LFG_DEBUG_GROUP:
+            SendSysMessage("Dungeon finder debug ON (group): a game master's entry now also takes whoever else is queued, whatever roles they picked.");
+            break;
+        default:
+            SendSysMessage("Dungeon finder debug OFF: normal matchmaking.");
+            break;
+    }
+
+    if (newMode != LFG_DEBUG_OFF)
+    {
+        SendSysMessage("Ordinary players still match by the normal rules; only entries containing a game master are affected.");
+    }
+
+    return true;
+}
+
 bool ChatHandler::HandleDebugBattlegroundCommand(char* /*args*/)
 {
     sBattleGroundMgr.ToggleTesting();

src/game/Object/LootMgr.cpp

@@ -1137,6 +1137,17 @@ bool BuildMopLootResponse(WorldPacket& out, LootView const& view,
     response.lootType = uint8(lootType);
     response.success = true;
 
+    // The trailing optional byte. It is NOT a failure reason despite the field name:
+    // the client reads it into msg+40 and only consults it on the success == 0 branch
+    // (0x936FDE), where it selects an error string. On success it is dead.
+    //
+    // Retail nevertheless always ships it -- the gating bit is 0 on all 32 successful
+    // responses decoded from the corpus, and set in only 3 of 9437 packets, all of them
+    // failures. The value is 17 on success and 18 on failure. Emitting it costs one byte
+    // and removes the last shape difference between our response and retail's.
+    response.hasFailureReason = true;
+    response.failureReason = 17;
+
     if (view.permission != NONE_PERMISSION)
     {
         response.money = loot.gold;
@@ -1160,7 +1171,14 @@ bool BuildMopLootResponse(WorldPacket& out, LootView const& view,
         wireItem.situ.assign(4, 0); // Client-compatible empty item-modifier block.
         wireItem.randomPropertyId = item.randomPropertyId;
         wireItem.lootListId = lootListId;
-        wireItem.slotType = uint8(slotType);
+        wireItem.slotType = ToWireLootSlotType(slotType);
+
+        // Retail sets this 2-bit field to 3 on every one of the 67 item records
+        // decoded from the corpus, and 0/1/2 never appear. The 18414 client parses
+        // it into msgItem+24 and never reads it back -- the only consumer of the
+        // item array, sub_9D5F3D, touches +0,+4,+8,+12,+16,+28,+32,+36 only -- so
+        // this is byte fidelity rather than behaviour.
+        wireItem.unknown = 3;
         if (ItemPrototype const* prototype =
                 ObjectMgr::GetItemPrototype(item.itemid))
         {
@@ -1268,7 +1286,11 @@ bool BuildMopLootResponse(WorldPacket& out, LootView const& view,
                     currency.amount = item.count;
                     currency.currencyId = item.itemid;
                     currency.lootListId = itr->index;
-                    currency.slotType = uint8(personalSlotType);
+                    // Same translation as the item rows: a raw LOOT_SLOT_NORMAL
+                    // puts 0 on the wire, which the auto-loot pass skips and which
+                    // raises a bind confirmation. Missed when the item paths were
+                    // converted; caught in review.
+                    currency.slotType = ToWireLootSlotType(personalSlotType);
                     response.currencies.push_back(currency);
                 }
             }

src/game/Object/LootMgr.h

@@ -91,6 +91,46 @@ enum LootSlotType
     MAX_LOOT_SLOT_TYPE                                      // custom, use for mark skipped from show items
 };
 
+/// Translate the server's pre-MoP LootSlotType onto the value the 18414 client reads.
+///
+/// The internal enum below is inherited and predates this build. Casting it straight onto
+/// the wire ships values the client never receives from a retail server: a corpus sweep of
+/// 9437 SMSG_LOOT_RESPONSE packets at build 18414 finds the 3-bit field takes exactly
+/// {3, 4, 7} -- 4 on 1809 ordinary drops, 3 on 542, 7 on 34 -- and 0, 1, 2, 5 and 6 never
+/// appear at all. Our LOOT_SLOT_NORMAL is 0.
+///
+/// It matters because the client branches on this value (record offset +24, built by
+/// sub_9D5F3D):
+///   * the auto-loot pass (sub_9387D6, 0x938824) takes a slot ONLY when it is 3 or 4, so a
+///     0 is silently skipped and never auto-looted;
+///   * only 4 suppresses the bind-on-pickup confirmation (sub_937CB8, 0x937DCB), so a 0
+///     raises a BoP prompt where retail shows none;
+///   * 2 opens the master-loot list instead of looting, 5 reports the slot locked, and 7
+///     refuses the click outright.
+///
+/// It only misfires in a GROUP. A solo kill already resolves OWNER_PERMISSION to
+/// LOOT_SLOT_OWNER, which is 4 and happens to be correct; shared loot resolves
+/// LOOT_SLOT_NORMAL, which is 0 and is not.
+///
+/// 2 is deliberately reachable only from LOOT_SLOT_MASTER. It has no corpus support, but a
+/// master-loot row has to say so somehow and every other value would misrepresent it.
+inline uint8 ToWireLootSlotType(LootSlotType slotType)
+{
+    switch (slotType)
+    {
+        case LOOT_SLOT_OWNER:  return 4;    // takeable now, no bind confirmation
+        case LOOT_SLOT_NORMAL: return 3;    // takeable, ordinary shared-loot rules
+        case LOOT_SLOT_MASTER: return 2;    // opens the master looter list
+        case LOOT_SLOT_VIEW:                // visible but not takeable by this player
+        case LOOT_SLOT_REQS:   return 7;    // refused: requirements not met
+        // Only reachable from MAX_LOOT_SLOT_TYPE -- the sentinel meaning "skip this
+        // row" -- or from a corrupt value. Refuse rather than offer: 3 would make a
+        // row the server already decided not to show look takeable and auto-lootable,
+        // leaving the server to reject the click it invited. 7 fails closed.
+        default:               return 7;
+    }
+}
+
 namespace MopLootPackets
 {
     constexpr size_t MAX_TAKE_ENTRIES = 50;

src/game/Object/ObjectMgrInstanceData.cpp

@@ -38,6 +38,7 @@
 #include "SQLStorages.h"
 #include "DBCStores.h"
 #include "Group.h"
+#include "LFGMgr.h"
 
 /**
  * @brief Gets instance template data by map id.
@@ -145,7 +146,24 @@ void ObjectMgr::LoadGroups()
     // TODO: maybe delete from the DB before loading in this case
     for (GroupMap::iterator itr = mGroupMap.begin(); itr != mGroupMap.end();)
     {
-        if (itr->second->GetMembersCount() < 2)
+        // Mirror RemoveMember's own survival threshold rather than assuming two.
+        //
+        // This loop predates the branch and was correct while a one-member group could
+        // never reach startup: the logout path dissolved it. It no longer does -- an LFG
+        // group survives logout deliberately, so a run that bled down to a single member
+        // persists to the database and loads here.
+        //
+        // Disbanding it at this point defeats the whole restart-survival feature, and
+        // silently: it runs BEFORE the instance-bind loop, so RestoreDungeonGroup is
+        // never called for the group, and before the demotion sweep, which then cannot
+        // see it either. The player logs back in inside the instance with no group, no
+        // LFG block, no eye and no teleport out -- exactly the stranding this branch
+        // exists to prevent.
+        //
+        // 1 < 1 is false, so a single-member LFG or battleground group now survives to
+        // the bind loop and is either restored or demoted there.
+        uint32 const minMembers = (itr->second->isBGGroup() || itr->second->isLFGGroup()) ? 1u : 2u;
+        if (itr->second->GetMembersCount() < minMembers)
         {
             itr->second->Disband();
             delete itr->second;
@@ -216,6 +234,13 @@ void ObjectMgr::LoadGroups()
 
             DungeonPersistentState* state = (DungeonPersistentState*)sMapPersistentStateMgr.AddPersistentState(mapEntry, fields[2].GetUInt32(), Difficulty(diff), (time_t)fields[5].GetUInt64(), (fields[6].GetUInt32() == 0), true, true, fields[8].GetUInt32());
             group->BindToInstance(state, fields[3].GetBool(), true);
+
+            // Nothing in LFGMgr is persisted, so a dungeon-finder party that was inside its
+            // instance when the world went down comes back with the group and the bind but
+            // no LFG status -- which empties SMSG_GROUP_LIST's LFG block and takes the eye,
+            // both teleport options and the Vote Kick gate away from the client. Rebuild it
+            // here, where the bind's map, difficulty and encounter mask are all in hand.
+            sLFGMgr.RestoreDungeonGroup(group, mapId, uint32(diff), fields[8].GetUInt32());
         }
         while (result->NextRow());
         delete result;
@@ -224,6 +249,51 @@ void ObjectMgr::LoadGroups()
     sLog.outString(">> Loaded %u group-instance binds total", count);
     sLog.outString();
 
+    // Any group still flagged as a finder run but WITHOUT a restored status has outlived
+    // its instance, and must be demoted rather than left in between.
+    //
+    // RestoreDungeonGroup above rebuilds a run's LFG status from its bind. A group whose
+    // bind is gone -- an ordinary dungeon instance expires two hours after it is created,
+    // so this is the normal outcome of leaving a party assembled overnight -- never
+    // reaches that call at all, because the loop iterates binds. It comes back with
+    // GROUPTYPE_LFD set and no status behind it.
+    //
+    // Left alone the client gets neither behaviour: SMSG_GROUP_LIST omits the LFG block,
+    // so the eye and the teleport options disappear, while every server-side
+    // isLFGGroup() test still says finder group -- which is why TeleportPlayer refuses
+    // with "has no LFG status" instead of moving anyone. Demote here, once, where the
+    // binds have all been processed and the answer is finally knowable.
+    {
+        uint32 demoted = 0;
+        for (GroupMap::const_iterator itr = mGroupMap.begin(); itr != mGroupMap.end(); ++itr)
+        {
+            Group* group = itr->second;
+            if (!group || !group->isLFGGroup())
+            {
+                continue;
+            }
+
+            // Exactly the predicate Group::SendUpdate uses to decide whether to emit an
+            // LFG block, so a group is demoted precisely when the client would otherwise
+            // have been sent nothing and left in the half-state.
+            if (sLFGMgr.GetGroupDungeonEntry(group->GetObjectGuid()) != 0)
+            {
+                continue;                                   // a live run, restored above
+            }
+
+            sLog.outString("Group %u was a dungeon finder group whose instance no longer "
+                           "exists; converting it to an ordinary party.", group->GetId());
+            group->ClearLfgGroup();
+            ++demoted;
+        }
+
+        if (demoted)
+        {
+            sLog.outString(">> Demoted %u finder group(s) with no surviving instance", demoted);
+            sLog.outString();
+        }
+    }
+
     sLog.outString(">> Loaded %u group members total", count);
     sLog.outString();
 }

src/game/Object/ObjectUpdate.cpp

@@ -527,6 +527,14 @@ void Object::BuildCreateUpdateBlockForPlayer(UpdateData* data, Player* target) c
         movement.z = unit->GetPositionZ();
         movement.o = unit->GetOrientation();
         movement.moveTime = GameTime::GetGameTimeMS();
+
+        // The nine speeds are sanitised inside AppendSimpleLivingMovement rather
+        // than here. The client's create validator rejects any object whose speed
+        // is approximately zero, and a rejected create discards the whole rest of
+        // the packet -- but this is NOT the only writer that fills a create block:
+        // Map::SendInitSelf builds the player's own create through the same
+        // emitter. Clamping at this call site left that path uncovered, and a
+        // rejected SELF create is worse than a rejected observer create.
         movement.speedWalk = unit->GetSpeed(MOVE_WALK);
         movement.speedRun = unit->GetSpeed(MOVE_RUN);
         movement.speedRunBack = unit->GetSpeed(MOVE_RUN_BACK);

src/game/Object/Player.cpp

@@ -4225,7 +4225,38 @@ void Player::HandleStealthedUnitsDetection()
             if (!hasAtClient)
             {
                 ObjectGuid i_guid = (*i)->GetObjectGuid();
-                (*i)->SendCreateUpdateToPlayer(this);
+
+                // Only record the object as known once the create block has ACTUALLY gone
+                // out. SendCreateUpdateToPlayer returns false when no block could be built
+                // -- BuildCreateUpdateBlockForPlayer bails on !CanBuildMopCreateUpdate(),
+                // which rejects vehicles, boarded units, units on a transport and units
+                // carrying optional movement extras -- and this site used to discard that
+                // result and insert the guid regardless.
+                //
+                // The consequence is not a missing object, it is a POISONED one. With the
+                // guid in m_clientGUIDs, HaveAtClient() reports true, so
+                // WorldObjectChangeAccumulator builds VALUES update blocks for an object the
+                // client was never given. The 18414 client resolves the guid in that block
+                // (ObjectMgrClient.cpp, block type 0 -> sub_79DE32), finds nothing, and
+                // replies CMSG_OBJECT_UPDATE_FAILED naming the guid. It then skips the block
+                // (sub_79BC10 walks the update mask, discards the fields and returns 1, so
+                // the caller does not break) and carries on with the packet -- the damage is
+                // confined to that one object, but it is permanent: nothing ever removes the
+                // guid from m_clientGUIDs, so no create is ever sent again and the player
+                // stays invisible to that client until they zone.
+                //
+                // Observed live 2026-08-06: four of five clients in one instance each sent
+                // CMSG_OBJECT_UPDATE_FAILED 17 times, naming player guids 1 and 6 and a pet
+                // (0xF140000400000001) -- exactly the characters reported invisible while
+                // still showing on the minimap. It surfaces as the party frame's "phasing"
+                // icon because UnitInPhase is not a phase test at all: it returns false when
+                // the client simply has no object for that member.
+                //
+                // The other two insert sites already guard this way; this one did not.
+                if (!(*i)->SendCreateUpdateToPlayer(this))
+                {
+                    continue;
+                }
                 m_clientGUIDs.insert(i_guid);
 
                 DEBUG_FILTER_LOG(LOG_FILTER_VISIBILITY_CHANGES, "%s is detected in stealth by player %u. Distance = %f", i_guid.GetString().c_str(), GetGUIDLow(), GetDistance(*i));

src/game/Object/Player.h

@@ -5919,6 +5919,14 @@ class Player : public Unit
             m_pendingEmoteRefresh[guid] = EMOTE_REFRESH_DELAY_MS;
         }
 
+        /// Drop every queued refresh. Used when the client discards its object
+        /// manager wholesale (far teleport), which makes every pending target
+        /// unknown to it.
+        void ClearPendingEmoteRefresh()
+        {
+            m_pendingEmoteRefresh.clear();
+        }
+
         /// Client reported its loading screen appearing or disappearing.
         void SetAwaitingLoadScreen(bool loading)
         {

src/game/Object/PlayerBattleGround.cpp

@@ -88,6 +88,29 @@
  */
 bool Player::TeleportToBGEntryPoint()
 {
+    // Refuse to teleport to an entry point that was never recorded.
+    //
+    // m_bgData.joinPos default-constructs to map 0 at (0,0,0) -- the origin of Eastern
+    // Kingdoms, which is inside the terrain. Teleporting there drops the player under the
+    // world and kills them. This is reachable whenever the return teleport runs without a
+    // matching SetBattleGroundEntryPoint(): observed live on 2026-08-06, where a player who
+    // was already standing inside an LFG dungeon when the world restarted used "Teleport out
+    // of dungeon" and was sent to SMSG_NEW_WORLD map=0 (0.00, 0.00, 0.00), fell and died.
+    //
+    // Homebind is the correct fallback -- it is where every other "we do not know where this
+    // player belongs" path in the core sends them.
+    if (!MaNGOS::IsValidMapCoord(m_bgData.joinPos.coord_x, m_bgData.joinPos.coord_y,
+                                 m_bgData.joinPos.coord_z) ||
+        (m_bgData.joinPos.coord_x == 0.0f && m_bgData.joinPos.coord_y == 0.0f &&
+         m_bgData.joinPos.coord_z == 0.0f))
+    {
+        sLog.outError("Player::TeleportToBGEntryPoint: %s has no valid entry point "
+                      "(map %u, %.2f %.2f %.2f) -- sending to homebind instead.",
+                      GetName(), m_bgData.joinPos.mapid, m_bgData.joinPos.coord_x,
+                      m_bgData.joinPos.coord_y, m_bgData.joinPos.coord_z);
+        return TeleportToHomebind();
+    }
+
     ScheduleDelayedOperation(DELAYED_BG_MOUNT_RESTORE);
     ScheduleDelayedOperation(DELAYED_BG_TAXI_RESTORE);
     return TeleportTo(m_bgData.joinPos);