diff --git a/CHANGELOG.md b/CHANGELOG.md index c22e22c..dc8e312 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,33 @@ All notable changes to `mod-gold-rush` will be documented in this file. +## [Unreleased — hallsworth fork] + +### Fixed +- Bot routing is no longer a silent no-op. `Engine::ChangeStrategy` dispatches on the + first character of the strategy name and ignores anything without a `+`/`-`/`~`/`?` + prefix, so the bare `"new rpg"` calls did nothing. Now sends `"+new rpg"`, and only + to the non-combat engine, which is the only engine `AiFactory` ever adds it to. +- Temporary node lifetime is no longer 1000x too long. The `respawnTime` argument to + `Map::SummonGameObject` reaches `GameObject::SetRespawnTime(int32)`, which is in + seconds; the module was passing milliseconds, turning a 20-minute event into a + ~14-day one. Nodes never expired on their own. +- A config reload during a live event no longer orphans its gameobjects. `LoadConfig` + cleared the tracked node list without despawning, leaving nodes in the world with + nothing left to remove them. +- Nodes are placed on real ground. Spawns previously reused the anchor's exact Z for + every scattered X/Y, so on sloped terrain they floated or sank. Placement now probes + ground height, rejects cliffs and lower floors, requires line of sight to the anchor, + and retries a few times before giving up on a node. + +### Changed +- Node scatter uses a square-root radius so points spread evenly across the disc + instead of bunching near the anchor. +- `DespawnHotspotNodes` resolves the map once instead of per node, and warns if the + map cannot be resolved. +- Dropped the redundant `SetRespawnTime` / `SetSpawnedByDefault` calls after + `Map::SummonGameObject`, which already does both. + ## [Unreleased] ### Added diff --git a/src/GoldRush.cpp b/src/GoldRush.cpp index 481bb75..3349957 100644 --- a/src/GoldRush.cpp +++ b/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("GoldRush.Enable", true); _config.VerboseLogging = sConfigMgr->GetOption("GoldRush.VerboseLogging", false); _config.MinIntervalMs = sConfigMgr->GetOption("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(1, _config.DurationMs / IN_MILLISECONDS); + constexpr float twoPi = 6.28318530717958647692f; + float const anchorZ = anchor->GetPositionZ(); + for (uint32 i = 0; i < nodeCount; ++i) { std::vector 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();