ClassicWoWCommunity / sod-bugs

This is a place for Season of Discovery bug reports
1 stars 0 forks source link

[BUG] Ring rune spell IDs not available via engraving API #8

Closed kayla-glick closed 1 month ago

kayla-glick commented 3 months ago

1. Current Behavior

1.1. Description

The WoWSims Exporter addon has been using the following script to access rune IDs for equipped gear:

---Get rune spell from an item in a slot, if item has a rune engraved.
---@param slotId integer
---@param bagId integer|nil If not nil check bag items instead of equipped items.
---@return integer|nil abilitySpellId The first spell id granted by the rune, or nil if no rune engraved.
function Env.GetEngravedRuneSpell(slotId, bagId)
    -- After first login the whole engraving stuff may not be loaded yet!
    -- GetNumRunesKnown will return 0 for maximum runes available in that case.
    if select(2, C_Engraving.GetNumRunesKnown()) == 0 then
        LoadAddOn("Blizzard_EngravingUI")
        C_Engraving.RefreshRunesList()
    end

    local runeData
    if bagId == nil then
        runeData = C_Engraving.GetRuneForEquipmentSlot(slotId)
    else
        runeData = C_Engraving.GetRuneForInventorySlot(bagId, slotId)
    end

    if runeData then
        local firstSpellId = runeData.learnedAbilitySpellIDs[1]
        if runeSpellRemap[firstSpellId] then
            return runeSpellRemap[firstSpellId]
        end
        return firstSpellId
    end
end

For every rune slot except rings this works, where runeData.learnedAbilitySpellIDs[1] returns the expected Spell ID. For slots 11 and 12 though the list of Spell IDs seems to be empty and our addon is unable to get ring rune data.

1.2. How to Reproduce

      1. Equip a rune on equipped boots
      2. Run `/run local runeData = C_Engraving.GetRuneForEquipmentSlot(8) print(runeData.learnedAbilitySpellIDs[1])` and see that the rune's Spell ID is printed out
       3. Equip a rune on either ring slot
       4. Run `/run local runeData = C_Engraving.GetRuneForEquipmentSlot(11) print(runeData.learnedAbilitySpellIDs[1])` and see that the it returns `nil`. The same is true for slot 12
       5. I tried several different indexes in the array but they're all `nil`

2. Expected Behavior

2.1. Description

The rune Spell ID should be available for rings through the C_Engraving API. If there's a different/better way to access that information, please let me know!

raethkcj commented 1 month ago

NAB, got worked around using enchantment IDs: https://github.com/wowsims/exporter/pull/21