fix: correct bot routing, node lifetime, reload cleanup and spawn placement

Four defects found reviewing this against AzerothCore efe123fab and
mod-playerbots 8d9f6aa6 (both 2026-08-14).

1. Bot routing did nothing. Engine::ChangeStrategy switches on name[0] and
   only handles '+', '-', '~' and '?'; a bare "new rpg" fell through every
   case. Bots still drifted toward the hotspot, but only because
   rpgInfo.ChangeToGoGrind() works and AiFactory already grants the strategy
   when AiPlayerbot.EnableNewRpgStrategy is on. Now sends "+new rpg", and
   only to BOT_STATE_NON_COMBAT -- the combat engine never carries it.

2. Node lifetime was off by 1000x. The respawnTime argument to
   Map::SummonGameObject lands in GameObject::SetRespawnTime(int32), which is
   seconds. Passing DurationMs made a 20-minute event's nodes live ~13.9 days,
   so they only ever vanished via the explicit despawn at event end.

3. LoadConfig cleared _spawnedNodes and _active without despawning, so a
   `.reload config` mid-event orphaned every node permanently (compounded by
   defect 2). It now tears the running event down first.

4. Spawns reused the anchor's exact Z for every scattered X/Y with no ground
   probe and no LOS check, leaving nodes floating or buried on the hilly zones
   in the default pool. Placement now probes ground height, rejects drift
   beyond 20y, requires LOS from the anchor, and retries up to 8 times.

Verified with a clang -fsyntax-only pass using the live build's own compile
flags plus the mod-playerbots include tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019vTNrJGsprcjdjjjDJD5ZM

src/GoldRush.cpp

@@ -32,6 +32,14 @@ namespace
 {
 static char const* const GoldRushLogFilter = "module.goldrush";
 
+// Node placement tuning.
+static constexpr uint8 MaxPlacementAttempts = 8;      // candidate spots tried per node
+static constexpr float HeightProbeUp        = 5.0f;   // start the ground probe above the anchor
+static constexpr float HeightSearchDist     = 50.0f;  // how far down the probe may search
+static constexpr float MaxVerticalDrift     = 20.0f;  // reject ground this far off the anchor's
+// Mirrors INVALID_HEIGHT from GridTerrainData.h without taking a dependency on that header.
+static constexpr float InvalidHeightSentinel = -99999.0f;
+
 struct GoldRushSite
 {
     std::string ZoneLabel;
@@ -144,6 +152,18 @@ class GoldRushManager
 public:
     void LoadConfig()
     {
+        // A config reload (or `.reload config`) used to wipe the tracked node list while an
+        // event was live, orphaning every temporary gameobject in the world with no owner
+        // left to despawn it. Tear the running event down first, then rebuild state.
+        if (_active)
+        {
+            LOG_INFO(GoldRushLogFilter, "Gold Rush config reload during an active event; despawning {} temporary node(s) and releasing routed bots first.",
+                _spawnedNodes.size());
+
+            DespawnHotspotNodes();
+            ClearBotRouting();
+        }
+
         _config.Enabled = sConfigMgr->GetOption<bool>("GoldRush.Enable", true);
         _config.VerboseLogging = sConfigMgr->GetOption<bool>("GoldRush.VerboseLogging", false);
         _config.MinIntervalMs = sConfigMgr->GetOption<uint32>("GoldRush.MinIntervalMinutes", 120) * MINUTE * IN_MILLISECONDS;
@@ -844,7 +864,14 @@ private:
                 nodeCount, oreEntries.size(), herbEntries.size(), fallbackEntries.size());
         }
 
+        // Map::SummonGameObject feeds this straight into GameObject::SetRespawnTime(int32),
+        // which is measured in SECONDS. Passing milliseconds made every node live ~14 days
+        // instead of the configured duration, so nothing ever expired on its own.
+        uint32 const nodeLifetimeSeconds = std::max<uint32>(1, _config.DurationMs / IN_MILLISECONDS);
+
         constexpr float twoPi = 6.28318530717958647692f;
+        float const anchorZ = anchor->GetPositionZ();
+
         for (uint32 i = 0; i < nodeCount; ++i)
         {
             std::vector<uint32> const* pool = nullptr;
@@ -861,44 +888,76 @@ private:
                 continue;
 
             uint32 entry = Acore::Containers::SelectRandomContainerElement(*pool);
-            float angle = RandomFloat(0.0f, twoPi);
-            float distance = RandomFloat(0.0f, _config.SpawnRadius);
-            float x = anchor->GetPositionX() + (std::cos(angle) * distance);
-            float y = anchor->GetPositionY() + (std::sin(angle) * distance);
-            float z = anchor->GetPositionZ();
 
-            if (GameObject* go = map->SummonGameObject(entry, x, y, z, anchor->GetOrientation(), 0.0f, 0.0f, 0.0f, 0.0f, _config.DurationMs, true))
+            // Try a handful of candidate spots; a node that cannot be placed on real ground
+            // within line of sight of the anchor is skipped rather than left floating.
+            for (uint8 attempt = 0; attempt < MaxPlacementAttempts; ++attempt)
             {
-                go->SetRespawnTime(_config.DurationMs);
-                go->SetSpawnedByDefault(false);
+                float angle = RandomFloat(0.0f, twoPi);
+
+                // sqrt() spreads points evenly across the disc; a plain uniform radius piles
+                // most of them near the anchor.
+                float distance = _config.SpawnRadius * std::sqrt(RandomFloat(0.0f, 1.0f));
+                float x = anchor->GetPositionX() + (std::cos(angle) * distance);
+                float y = anchor->GetPositionY() + (std::sin(angle) * distance);
+
+                float z = map->GetHeight(x, y, anchorZ + HeightProbeUp, true, HeightSearchDist);
+                if (z <= InvalidHeightSentinel)
+                    continue;
+
+                // Reject cliff faces, rooftops and lower floors picked up by the probe.
+                if (std::fabs(z - anchorZ) > MaxVerticalDrift)
+                    continue;
+
+                if (!anchor->IsWithinLOS(x, y, z + 1.0f))
+                    continue;
+
+                GameObject* go = map->SummonGameObject(entry, x, y, z, anchor->GetOrientation(), 0.0f, 0.0f, 0.0f, 0.0f, nodeLifetimeSeconds, true);
+                if (!go)
+                    continue;
+
+                // Map::SummonGameObject already applied the respawn time and cleared
+                // spawned-by-default; re-doing it here is redundant.
                 _spawnedNodes.push_back({ go->GetGUID(), entry });
                 ++_telemetry.NodesSpawned;
 
                 if (_config.VerboseLogging)
                 {
-                    LOG_INFO(GoldRushLogFilter, "Gold Rush node spawned: entry {} at ({:.2f}, {:.2f}, {:.2f}) on map {}.",
-                        entry, x, y, z, map->GetId());
+                    LOG_INFO(GoldRushLogFilter, "Gold Rush node spawned: entry {} at ({:.2f}, {:.2f}, {:.2f}) on map {} after {} attempt(s).",
+                        entry, x, y, z, map->GetId(), attempt + 1);
                 }
+
+                break;
             }
         }
 
+        if (_config.VerboseLogging && _spawnedNodes.size() < nodeCount)
+        {
+            LOG_INFO(GoldRushLogFilter, "Gold Rush placed {} of {} requested node(s); the rest found no valid ground near the anchor.",
+                _spawnedNodes.size(), nodeCount);
+        }
+
         return !_spawnedNodes.empty();
     }
 
     void DespawnHotspotNodes()
     {
+        Map* map = sMapMgr->FindMap(_currentSite.MapId, 0);
+        if (!map && !_spawnedNodes.empty())
+        {
+            LOG_WARN(GoldRushLogFilter, "Gold Rush could not resolve map {} to despawn {} temporary node(s); they will expire on their own respawn timer.",
+                _currentSite.MapId, _spawnedNodes.size());
+        }
+
         for (SpawnedNode const& node : _spawnedNodes)
         {
-            if (!node.Guid)
+            if (!node.Guid || !map)
                 continue;
 
-            if (Map* map = sMapMgr->FindMap(_currentSite.MapId, 0))
+            if (GameObject* go = map->GetGameObject(node.Guid))
             {
-                if (GameObject* go = map->GetGameObject(node.Guid))
-                {
-                    go->DespawnOrUnsummon();
-                    ++_telemetry.NodesRemoved;
-                }
+                go->DespawnOrUnsummon();
+                ++_telemetry.NodesRemoved;
             }
         }
 
@@ -957,8 +1016,12 @@ private:
             {
                 WorldPosition pos(_hotspot.GetMapId(), _hotspot.GetPositionX(), _hotspot.GetPositionY(), _hotspot.GetPositionZ());
                 botAI->rpgInfo.ChangeToGoGrind(pos);
-                botAI->ChangeStrategy("new rpg", BOT_STATE_NON_COMBAT);
-                botAI->ChangeStrategy("new rpg", BOT_STATE_COMBAT);
+
+                // Engine::ChangeStrategy dispatches on the first character and ignores any
+                // name without a +/-/~/? prefix, so the old bare "new rpg" calls were silent
+                // no-ops. "new rpg" is a non-combat strategy (AiFactory only ever adds it to
+                // the non-combat engine), so the old BOT_STATE_COMBAT call was wrong as well.
+                botAI->ChangeStrategy("+new rpg", BOT_STATE_NON_COMBAT);
                 _routedBotIds.insert(bot->GetGUID().GetCounter());
                 ++routedCount;
 
@@ -1015,6 +1078,9 @@ private:
             if (!bot)
                 continue;
 
+            // Only the rpg target is cleared. "new rpg" is part of the bots' default
+            // non-combat loadout when AiPlayerbot.EnableNewRpgStrategy is on, so removing
+            // it here would strip behaviour the module never granted.
             if (PlayerbotAI* botAI = GET_PLAYERBOT_AI(bot))
                 botAI->rpgInfo.ChangeToIdle();