mod-gold-rush/docs/herb-ore-category-fix-spec.md
Claude 5d292b3d9c 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.
2026-09-08 22:55:58 +00:00

6.2 KiB

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:

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<uint32, bool> 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.