From 5d292b3d9cc12b7276f2f782ccfda95355f0b605 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 22:55:58 +0000 Subject: [PATCH 1/2] docs: fix spec for broken herb/ore categorization 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. --- docs/herb-ore-category-fix-spec.md | 114 +++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 docs/herb-ore-category-fix-spec.md diff --git a/docs/herb-ore-category-fix-spec.md b/docs/herb-ore-category-fix-spec.md new file mode 100644 index 0000000..8f44440 --- /dev/null +++ b/docs/herb-ore-category-fix-spec.md @@ -0,0 +1,114 @@ +# Fix: herb/ore categorization breaks for almost every zone in the v2 pool + +## The bug, confirmed by reading the code + +`SplitByCategory()` (`GoldRush.cpp:648`) decides whether a resource entry +is a herb or ore purely by matching keywords in its display name: + +```cpp +bool isHerb = ContainsWord(name, "lotus") || ContainsWord(name, "clover") || ContainsWord(name, "thorn") || ContainsWord(name, "bloom"); +bool isOre = ContainsWord(name, "vein"); +``` + +This was written against the *original 5-entry Northrend default pool* +(Goldclover→"clover", Icethorn→"thorn", Frost Lotus→"lotus", Titanium +**Vein**→"vein") and never updated when the v2 zone-resources feature (PR +#5) introduced ~70 new resource names across 57 zones. + +**Confirmed failure case**: tested live in Hellfire Peninsula. Its +configured pool is Felweed, Dreaming Glory, Fel Iron **Deposit**, +Adamantite **Deposit**. None of these match any herb keyword, and neither +ore entry contains "vein" — most Outland/Classic ore uses "Deposit," not +"Vein." So both `herbEntries` and `oreEntries` come back empty, and +`SpawnHotspot()` (`GoldRush.cpp:998-1017`) falls back to +`fallbackEntries` (the whole site pool, unsplit) for every node. The +intended herb/ore alternation silently never happens for the large +majority of the 57 configured zones — only the handful whose names happen +to contain one of the four hardcoded herb words or "vein" categorize +correctly at all. + +**Observed symptom**: the test run in Hellfire produced overwhelmingly +Felweed with no observed ore, despite the fallback pool containing ore +entries. Whether that's pure sampling variance or a second, separate +issue (see "Worth checking after this fix" below) isn't confirmed from +reading the code alone — don't assume the placement-bias theory without +testing after this fix ships. + +## Fix: stop guessing category from the name, use a known lookup table + +Every entry ID used across the 57-zone pool was already individually +verified against `acore_world.gameobject_template` when the pool was +built (`docs/zone-specific-resources-spec.md`). Rather than continue +patching an ever-growing, ever-fragile keyword list — the next zone added +will just as easily introduce another unmatched name — replace the +name-based guess with a static ID→category lookup built from data that's +already verified correct. + +**Add a lookup table** (e.g. `static const std::unordered_map IsHerbById` or similar, `true` = herb, `false` = ore), populated +from every entry in the tables below. Change `SplitByCategory` to check +this table first; **keep the existing name-keyword check as a fallback +only** for any entry not found in the table (defensive — covers the +original global-pool default entries and anything added by hand later +without an explicit table update). + +### Herb entries (mark `true`) +``` +1618, 3724, 1617, 3725, 1619, 3726, // Peacebloom, Silverleaf, Earthroot +1620, 3727, 1621, 3729, 2045, 1622, 3730, // Mageroyal, Briarthorn, Stranglekelp, Bruiseweed +1623, 1624, 2041, 2042, 2046, 2043, 2866, // Wild Steelbloom, Kingsblood, Liferoot, Fadeleaf, Goldthorn, Khadgar's Whisker, Firebloom +142140, 180165, 142141, 176642, // Purple Lotus, Arthas' Tears +142142, 176636, 180164, 142143, 183046, // Sungrass, Blindweed +142144, 142145, 176637, 176587, 176641, // Ghost Mushroom, Gromsblood, Plaguebloom +176583, 176638, 180167, 176584, 176639, // Golden Sansam, Dreamfoil +180168, 176586, 176640, 180166, 176588, // Mountain Silversage, Icecap +191303, 176589, // Firethorn, Black Lotus +181270, 183044, 181271, 183045, 181275, // Felweed, Dreaming Glory, Ragveil +183043, 181277, 181276, 181279, 181280, 181281, // Terocone, Flame Cap, Netherbloom, Nightmare Vine, Mana Thistle +189973, 190171, 190172, 190176, 190170, // Goldclover, Lichbloom, Icethorn, Frost Lotus, Talandra's Rose +191019, 190169 // Adder's Tongue, Tiger Lily +``` + +### Ore entries (mark `false`) +``` +1731, 2055, 3763, 103713, 181248, // Copper Vein +1732, 2054, 3764, 103711, 181249, // Tin Vein +1733, 105569, // Silver Vein +1735, // Iron Deposit +1734, 150080, 181109, // Gold Vein +2040, 150079, 176645, // Mithril Deposit +2047, 150081, 181108, // Truesilver Deposit +324, 150082, 176643, 175404, // Small/Rich Thorium Vein +165658, // Dark Iron Deposit +181555, 181556, 181569, 181570, 181557, // Fel Iron, Adamantite (+ Rich), Khorium +189978, 189979, 189980, 189981, 191133 // Cobalt/Saronite Deposit (+ Rich), Titanium Vein +``` + +(This is every entry that appears anywhere in `gold_rush.conf.dist`'s +`GoldRush.ZonePool` plus the module's own default global +`GoldRush.NodeEntries` — cross-check against both files while +implementing, in case an entry was missed here.) + +## Worth checking after this fix — don't build blind for this part + +Once real ID-based categorization is in, re-test Hellfire Peninsula (or +another Outland zone) with `.goldrush teststart` a few times. If ore +still rarely/never appears despite `oreEntries` now correctly populated, +look at the placement-validation loop (`GoldRush.cpp:1019` onward) — +specifically the `MaxVerticalDrift` cliff/rock-face rejection. Ore +deposits are commonly placed on steep terrain in the actual game world; +if placement attempts for ore entries are failing validation far more +often than herb entries, that's a second, separate bug worth its own fix +— but confirm it's actually happening before changing that logic, rather +than assuming. + +## Testing + +- Hellfire Peninsula (the confirmed failure case) should now show a mix + of Felweed/Dreaming Glory *and* Fel Iron/Adamantite, not all-herb. +- Spot-check one or two zones whose original names *did* match the old + keyword list (e.g. Un'Goro Crater — Firethorn contains "thorn", Black + Lotus contains "lotus") to confirm the table-based path didn't regress + anything that used to work by keyword-coincidence. +- Confirm entries not in either table (if any exist) still fall through + to the old keyword check rather than being silently dropped. From 24b0815fc632f9469ab7f6ed5900676f7c4967ba Mon Sep 17 00:00:00 2001 From: "Troll (Hermes Agent)" Date: Tue, 8 Sep 2026 23:04:04 +0000 Subject: [PATCH 2/2] fix: herb/ore categorization via static ID lookup table 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. --- CHANGELOG.md | 9 +++++++ src/GoldRush.cpp | 62 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de30557..7421500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,15 @@ All notable changes to `mod-gold-rush` will be documented in this file. global `GoldRush.NodeEntries` pool exactly as before. ### Fixed +- Herb/ore categorization no longer guesses from display-name keywords. The old + `SplitByCategory()` matched "lotus"/"clover"/"thorn"/"bloom" for herbs and + "vein" for ore, which only worked for the original Northrend default pool and + silently fell back to the unsplit pool for almost every v2 zone (Outland/Classic + ore uses "Deposit", not "Vein"; most herb names match no keyword). Replaced with + a static ID→category lookup table built from the DB-verified entry IDs in + `docs/zone-specific-resources-spec.md`, with the old keyword check retained only + as a fallback for entries not in the table. See + `docs/herb-ore-category-fix-spec.md`. - Event start/end announcements no longer crash the server. `StartEvent()` and `EndEvent()` both used `ChatHandler(nullptr).SendWorldText(...)` to broadcast server-wide -- `ChatHandler::SendWorldText` assumes a real session and calls diff --git a/src/GoldRush.cpp b/src/GoldRush.cpp index 9e7e873..99f94ce 100644 --- a/src/GoldRush.cpp +++ b/src/GoldRush.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -41,6 +42,47 @@ static constexpr float MaxVerticalDrift = 20.0f; // reject ground this far // Mirrors INVALID_HEIGHT from GridTerrainData.h without taking a dependency on that header. static constexpr float InvalidHeightSentinel = -99999.0f; +// Static ID -> category lookup for herb/ore classification. Every entry ID used +// across the 57-zone GoldRush.ZonePool (plus the module's own default global +// GoldRush.NodeEntries) was individually verified against +// acore_world.gameobject_template when the pool was built +// (docs/zone-specific-resources-spec.md). Guessing the category from the display +// name (the old keyword-list approach) broke for almost every zone: Outland/Classic +// ore uses "Deposit" rather than "Vein", and most herb names don't contain any of +// the four hardcoded herb keywords. true = herb, false = ore. +static const std::unordered_map IsHerbById = { + // Classic herbs + { 1618, true }, { 3724, true }, { 1617, true }, { 3725, true }, { 1619, true }, { 3726, true }, // Peacebloom, Silverleaf, Earthroot + { 1620, true }, { 3727, true }, { 1621, true }, { 3729, true }, { 2045, true }, { 1622, true }, { 3730, true }, // Mageroyal, Briarthorn, Stranglekelp, Bruiseweed + { 1623, true }, { 1624, true }, { 2041, true }, { 2042, true }, { 2046, true }, { 2043, true }, { 2866, true }, // Wild Steelbloom, Kingsblood, Liferoot, Fadeleaf, Goldthorn, Khadgar's Whisker, Firebloom + { 142140, true }, { 180165, true }, { 142141, true }, { 176642, true }, // Purple Lotus, Arthas' Tears + { 142142, true }, { 176636, true }, { 180164, true }, { 142143, true }, { 183046, true }, // Sungrass, Blindweed + { 142144, true }, { 142145, true }, { 176637, true }, { 176587, true }, { 176641, true }, // Ghost Mushroom, Gromsblood, Plaguebloom + { 176583, true }, { 176638, true }, { 180167, true }, { 176584, true }, { 176639, true }, // Golden Sansam, Dreamfoil + { 180168, true }, { 176586, true }, { 176640, true }, { 180166, true }, { 176588, true }, // Mountain Silversage, Icecap + { 191303, true }, { 176589, true }, // Firethorn, Black Lotus + // Outland herbs + { 181270, true }, { 183044, true }, { 181271, true }, { 183045, true }, { 181275, true }, // Felweed, Dreaming Glory, Ragveil + { 183043, true }, { 181277, true }, { 181276, true }, { 181279, true }, { 181280, true }, { 181281, true }, // Terocone, Flame Cap, Netherbloom, Nightmare Vine, Mana Thistle + // Northrend herbs + { 189973, true }, { 190171, true }, { 190172, true }, { 190176, true }, { 190170, true }, // Goldclover, Lichbloom, Icethorn, Frost Lotus, Talandra's Rose + { 191019, true }, { 190169, true }, // Adder's Tongue, Tiger Lily + // Classic ore + { 1731, false }, { 2055, false }, { 3763, false }, { 103713, false }, { 181248, false }, // Copper Vein + { 1732, false }, { 2054, false }, { 3764, false }, { 103711, false }, { 181249, false }, // Tin Vein + { 1733, false }, { 105569, false }, // Silver Vein + { 1735, false }, // Iron Deposit + { 1734, false }, { 150080, false }, { 181109, false }, // Gold Vein + { 2040, false }, { 150079, false }, { 176645, false }, // Mithril Deposit + { 2047, false }, { 150081, false }, { 181108, false }, // Truesilver Deposit + { 324, false }, { 150082, false }, { 176643, false }, { 175404, false }, // Small/Rich Thorium Vein + { 165658, false }, // Dark Iron Deposit + // Outland ore + { 181555, false }, { 181556, false }, { 181569, false }, { 181570, false }, { 181557, false }, // Fel Iron, Adamantite (+ Rich), Khorium + // Northrend ore + { 189978, false }, { 189979, false }, { 189980, false }, { 189981, false }, { 191133, false }, // Cobalt/Saronite Deposit (+ Rich), Titanium Vein +}; + struct GoldRushSite { std::string ZoneLabel; @@ -657,9 +699,23 @@ private: if (!goinfo) continue; - std::string name = Normalize(goinfo->name); - bool isHerb = ContainsWord(name, "lotus") || ContainsWord(name, "clover") || ContainsWord(name, "thorn") || ContainsWord(name, "bloom"); - bool isOre = ContainsWord(name, "vein"); + // Prefer the verified ID -> category lookup table. The old name-keyword + // guess is kept only as a defensive fallback for entries not in the table + // (e.g. the original global-pool defaults or anything added by hand later + // without an explicit table update). + bool isHerb; + auto const it = IsHerbById.find(entry); + if (it != IsHerbById.end()) + { + isHerb = it->second; + } + else + { + std::string name = Normalize(goinfo->name); + isHerb = ContainsWord(name, "lotus") || ContainsWord(name, "clover") || ContainsWord(name, "thorn") || ContainsWord(name, "bloom"); + } + + bool isOre = !isHerb; if (herbs && isHerb) result.push_back(entry);