SplitByCategory() guessed herb/ore from display-name keywords written
for the original 5-entry Northrend pool. Outland/Classic ore uses
'Deposit' not 'Vein', and most herb names match no keyword, so both
buckets came back empty for almost every v2 zone and the code silently
fell back to the unsplit pool (confirmed live in Hellfire Peninsula).
Replace the keyword guess with a static ID->category lookup table built
from the DB-verified entry IDs in docs/zone-specific-resources-spec.md.
The old keyword check is retained only as a fallback for entries not in
the table.
SplitByCategory() guesses herb/ore purely from keywords in the
display name (lotus/clover/thorn/bloom for herb, vein for ore) --
written for the original 5-entry Northrend default pool, never
updated for the ~70 entries the v2 zone-resources feature (PR #5)
introduced. Confirmed live in Hellfire Peninsula: none of its 4
resources (Felweed, Dreaming Glory, Fel Iron Deposit, Adamantite
Deposit) match any keyword, so both herb and ore buckets come back
empty and every zone silently falls back to the unsplit pool --
breaking the intended herb/ore alternation for the large majority of
the 57 configured zones.
Spec provides the full herb/ore classification for every entry ID
already verified against the live DB when the zone pool was built,
so this can be implemented as a static ID lookup instead of
continuing to patch an ever-fragile keyword list.
SelectSiteForPlayer() tries to anchor an event on a live player's
current position FIRST (BuildEligibleSiteFromPlayer ->
BuildSiteFromAnchor), only falling back to a random pick from _sites
when no eligible player is available. Since there is almost always
an eligible player online, this is the common path in practice --
but BuildSiteFromAnchor only reads the player's live zone/area IDs
from the world state; it has no access to the NodeEntries/
BonusEntries that BuildSites() parsed from GoldRush.ZonePool into
_sites. Every player-anchored event therefore got an empty
NodeEntries/BonusEntries regardless of ZonePool configuration, and
SpawnHotspot()'s (correct, intentional) fallback-to-global-pool logic
silently kicked in every time.
This is why testing in Un'Goro Crater after the v2 zone-resources
feature shipped still produced Titanium Vein and Northrend herbs --
the module correctly identified the player's zone as Un'Goro
(ZoneId/ZoneLabel were right), it just never looked up Un'Goro's
configured resource pool to attach it to that site.
Fix: after building the site from the player's live position, look
up a matching entry in _sites by ZoneId and inherit its NodeEntries/
BonusEntries. A zone that's an eligible anchor but has no configured
entry in _sites still correctly falls through to the global pool --
that fallback path was never the problem, only the missing lookup
for zones that ARE configured.
The hardcoded fallback default for GoldRush.ZonePool still used the
old ZoneName|AreaLabel format after the parser switched to
ZoneName|NodeEntries|BonusEntries. Not a crash risk (BuildNodeEntries
safely no-ops on non-numeric input) but stale and misleading. Cleared
to an empty default -- the real default lives in gold_rush.conf.dist.
Also added CHANGELOG entries for both this PR's zone-specific
resources feature and the earlier null-session broadcast crash fix
(PR #3), neither of which had been logged despite being merged.
- GoldRushSite gains NodeEntries and BonusEntries vectors.
- GoldRush.ZonePool now uses the 2-segment v2 format:
ZoneName|NodeEntries|BonusEntries.
- AreaLabel segment removed; all zone names resolve directly via AreaTable.dbc.
- Per-node bonus chance controlled by GoldRush.BonusChancePercent (default 15).
- Backward compatibility: zones without entries fall back to global NodeEntries.
- conf/gold_rush.conf.dist updated with full 57-zone pool and BonusChancePercent.
- CHANGELOG.md updated.
Implements docs/zone-specific-resources-spec.md section 3.
Relates to PR #4
Extends the spec from 4 hardcoded hotspots to essentially every valid
open-world zone in WotLK 3.3.5a -- ~50 zones total.
Every zone name was validated against AzerothCore's own AreaTable.dbc
reference for 3.3.5a before being included. The original request list
contained several Cataclysm-only zones (Twilight Highlands, Abyssal
Depths, Shimmering Expanse, Kelp'thar Forest, Ruins of Gilneas, the
Northern/Southern Barrens split) that don't exist in this game
version -- those were dropped rather than guessed at. Two zones that
do exist (Plaguelands: The Scarlet Enclave, Mount Hyjal) were
deliberately excluded as impractical for a shared open-world event
(phased/instanced content, not open zones in 3.3.5a).
All ~70 resource entry IDs are individually verified against live
acore_world.gameobject_template, organized by the real Vanilla/TBC/
WotLK herbalism and mining tier progression -- not guessed from
memory. Confirmed, notably, that no 'Rich' ore variant exists for
Copper/Tin/Silver/Iron/Mithril in this database (only Thorium+ tiers
got them), matching actual game design rather than assuming symmetry
across ore tiers.
Same underlying code mechanism as v1 (GoldRushSite gains NodeEntries/
BonusEntries, SpawnHotspot prefers per-site pool with fallback to
global) -- this commit only expands the data, not the approach.
Makes node selection zone-aware instead of drawing from one global
pool for every hotspot. Includes real herb/ore entries verified
against the live acore_world database for all four configured
hotspots (Un'Goro Crater, Winterspring, Eastern Plaguelands, Sholazar
Basin), plus a rare 'bonus' tier per zone (Black Lotus for the three
Classic zones -- a real, accurate spawn-zone match, not an arbitrary
pick).
Extends the existing pipe-delimited ZonePool config format with two
optional segments rather than introducing a new config key, and
explicitly calls out backward compatibility for hotspots that don't
specify zone-specific resources.
StartEvent() and EndEvent() both used ChatHandler(nullptr).SendWorldText()
to announce Gold Rush events server-wide. ChatHandler::SendWorldText()
(core Chat.cpp) unconditionally calls GetSession()->SendPacket() on the
handler's session -- with a null session, that's an immediate null-pointer
dereference. This crashed the live server twice within ~24 hours
(confirmed via a symbolized gdb backtrace against a captured coredump):
WorldSession::GetPlayer (this=0x0)
WorldSession::SendPacket (this=0x0, ...)
ChatHandler::SendWorldText (...)
GoldRushManager::StartEvent (...) at GoldRush.cpp:880
ChatHandler is meant to wrap a real player/GM session; it was only being
constructed here to reuse SendWorldText's string formatting, with no
actual session behind it.
Fix: use sWorldSessionMgr->SendServerMessage(SERVER_MSG_STRING, text)
instead -- the core-provided, session-free broadcast path (the same one
the built-in .announce GM command uses in cs_message.cpp). No session
required, so no crash regardless of whether the event is triggered
automatically or by a player.
Fixed all three call sites using this broken pattern: the event-start
announcement (two locations -- StartEvent has what looks like an earlier
duplicate/preview announcement plus the main one) and the event-end
announcement in EndEvent(). The EndEvent() site had not crashed yet but
is the exact same bug and would have crashed the first time any event
completed naturally.
AreaTable.dbc names the Sholazar Basin subzone "River's Heart", not "The
River's Heart". ResolveAreaEntry does an exact normalised match, so the
default ZonePool entry never resolved and BuildSites silently dropped it --
the module logged "loaded with 3 hotspot(s)" for a four-entry pool.
Verified against the extracted AreaTable.dbc on the live server: the other
seven zone and area names in the default pool all match exactly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019vTNrJGsprcjdjjjDJD5ZM
Every failed scheduled start overwrote the rolled Min/MaxInterval with a
hard-coded 5 minutes and never restored it, so on a server that sits empty
the scheduler degenerated into a permanent 5-minute poll. The moment the
first player logged in, an event fired within one retry window and anchored
on their exact position, with a server-wide announcement -- login ambush
rather than a scheduled world event. It also wrote a retry line to
gold-rush.log every 5 minutes forever (Debug defaults to 1).
StartEvent now counts eligible anchors when the schedule expires, holds
without consuming the interval while below MinPlayersOnline, and inserts
GraceMinutes once the world repopulates. The count runs only on schedule
expiry, never per world tick.
New config: GoldRush.RetryMinutes (5), GoldRush.GraceMinutes (15),
GoldRush.MinPlayersOnline (1). GraceMinutes = 0 rolls a fresh interval
instead. GM-forced starts pass scheduled=false and bypass both gates.
Verified with clang -fsyntax-only against the live build's compile flags.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019vTNrJGsprcjdjjjDJD5ZM
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
