fix: herb/ore categorization broken for most v2 zones #7
3 changed files with 182 additions and 3 deletions
|
|
@ -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
|
||||
|
|
|
|||
114
docs/herb-ore-category-fix-spec.md
Normal file
114
docs/herb-ore-category-fix-spec.md
Normal file
|
|
@ -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<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.
|
||||
|
|
@ -24,6 +24,7 @@
|
|||
#include <sstream>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
#include <unordered_map>
|
||||
#include <unordered_set>
|
||||
#include <vector>
|
||||
|
||||
|
|
@ -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<uint32, bool> 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);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue