Feature/dbc extend exploration - #1516
Conversation
… feature/dbc-extend-exploration # Conflicts: # assets/database/db.bin # assets/database/leftover_db.bin # sim/common/mop/enchants_auto_gen.go # sim/common/mop/stat_bonus_procs_auto_gen.go # sim/mage/arcane/TestArcane.results # sim/mage/fire/TestFire.results # sim/mage/frost/TestFrost.results # sim/priest/shadow/TestShadow.results # sim/rogue/assassination/TestAssassination.results # sim/shaman/elemental/TestElemental.results # sim/warrior/protection/TestProtectionWarrior.results # tools/database/gen_effects.go
hillerstorm
left a comment
There was a problem hiding this comment.
Automated deep review (8 finder angles + per-finding verification against master and the PR head). 9 findings below as inline comments — the top two are live, user-visible regressions from the Skeer's manual-to-generated migration; the rest are latent landmines or robustness issues in the new generator/factory infrastructure.
Also checked and cleared, for the record: enchant SQL dedupe-by-name provably picks the right rows (PR ships the identical 255 enchant effect IDs as master); Renataki's generated values/IDs/ICD match the deleted manual impl exactly; RequireDamageDealt: false on Elemental Force/River's Song, Phase Fingers' shared-CD category (1141), and River's Song stacking to 2 all match DBC data and are taken as intended fixes.
DBC parsing: item/enchant → effect resolution
Audit of how items and enchants reach their spell effects, plus the fixes it turned up.
Wrong data in the shipped database
LEFT JOIN SpellEffectmultiplied ajson_group_arrayunder a bareGROUP BY, duplicating mods on 30,305 spells; 4 legendarycloaks shipped every mod twice. Mods are now aggregated in their own subquery.
EffectId.GROUP BY namecollapsed distinct enchantments sharing adisplay name (3289 Riding Crop / Skybreaker Whip). Replaced with two
ROW_NUMBER()dedupes,which also separates the 5.2/5.4 down-scaled enchant copies from the real enchant.
A_MOD_RATING/A_MOD_RESISTANCEstopped after the firststat in a mask, and
A_MOD_INCREASE_ENERGYturned every power type into mana.ON_EQUIPstats only reached the base scaling state. Now folded per state at that state'sown item level; 98 items gained their PvP Power on the challenge-mode state.
A_MOD_INCREASE_HEALTH_2ignored scaling coefficients. 50 PvP trinkets understated on-usehealth by 1.9–2.4× (Malevolent Gladiator's Emblem: 21608 → 51364). The pre-MoP items where
base points already equal the formula are unchanged, which is what confirms the formula.
order, and generated Go is gofmt-formatted, so regeneration is idempotent.
Relationships that now resolve
when the buff itself is server-scripted.
since no
SpellEffectedge exists. River's Song, Phase Fingers and Elemental Force moved fromhand-written to generated.
NewItemEffectregistrations instead of a per-character call,so they are no longer reported as unimplemented.
Reporting
// Not simulated:comments naming the rule,instead of vanishing silently. Kept out of the frontend TypeScript.
MISSING_ITEM_EFFECTS367 → 250 entries, all real gaps.BuildSpellProcInforequired an emptyOutcome, which it setsunconditionally, so nothing was ever refused and a callback-less trigger shipped as a live
registration that silently did nothing. Nazgrim's Burnished Insignia now works.
EffectBasePoints, a stale leftover("grant 1 Intellect" where the proc gives 11761). The provider now resolves them against the
item's level. Tooltip numbers agree with the stat the sim registers on 120/120 procs
(was 14) and 91/91 on-use effects (was 28).
Pipeline hygiene
SpellItemEnchantment → statsfor enchants, gems, random suffixes andsocket bonuses, and one aura → stats mapping shared with item effects.
ItemStatEffectrenamed toSocketBonusand scoped to the rows items reference (2642 → 282).resolveStatsSpell.Spell.ScalesWithItemLevel()style helpersrather than
HasAttributeAt(11, 0x4).Fixtures to regenerate
AllItems-Nazgrim'sBurnishedInsignia-105549— new case, previously inert.