Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Item Fields use wrong item of same name. #5710

Open
1 task done
nullKomplex opened this issue Mar 1, 2025 · 7 comments
Open
1 task done

Item Fields use wrong item of same name. #5710

nullKomplex opened this issue Mar 1, 2025 · 7 comments
Labels
⏱ Awaiting Response This ticket hasn't been triaged yet. 🐛 Bug This is a problem with WeakAuras.

Comments

@nullKomplex
Copy link
Contributor

nullKomplex commented Mar 1, 2025

Is there an existing issue for this?

  • I have searched the existing open and closed issues.

Description

This is almost entirely exclusively relevant to items across multiple difficulties, but this should be relevant for any item with the same name.

For example, I had an aura for tracking Apparatus of Khaz'goroth, a previously BiS trinket. Upon upgrading it to Heroic I noticed the aura was no longer enabled, because the Load Options was set to load on Item Equipped: Apparatus of Khaz'goroth. This setting is only checking for the normal version of Apparatus of Khaz'goroth, not the heroic.

Similarly I am currently testing it with an LFR and normal version of Ring of the Riven and the bug also exists with an Item Equipped trigger.

Normally I'd fix this myself, and while I know what the issue stems from, I don't know what the preferred solution would be: C_Item.GetItemInfo and C_Item.GetItemInfoInstant

Oddly enough I expected to be able to link the item into the item field and have that work instead (as it works for C_Item.GetItemInfo) but it appears to sanitize it to simply text.

WeakAuras Version

0ea549a

World of Warcraft Flavor

Cataclysm Classic

World of Warcraft Region

US/NA

Tested with only WeakAuras

I got this issue with only WeakAuras enabled

Lua Error


Reproduction Steps

  1. Have item on multiple difficulties.
  2. Create trigger with Item > Item Equipped and put in the name of the item.
  3. Equip one version of the item.
  4. Replace the item with the other version.
    Observe as aura will trigger for one version but not the other.

Last Good Version

No response

Screenshots

No response

Export String


Bisector Report


@nullKomplex nullKomplex added the 🐛 Bug This is a problem with WeakAuras. label Mar 1, 2025
@github-actions github-actions bot added the ⏱ Awaiting Response This ticket hasn't been triaged yet. label Mar 1, 2025
@InfusOnWoW
Copy link
Contributor

In the text you are talking about the "Load Option", but your reproduction steps are for a trigger. The trigger has an option for "Exact match". (Which iirc, I couldn't actually test, because this is a classic only problem.)

The intended behaviour with that options is:
a) If not enabled, the matching is for the name, so both all versions of the item should work
b) If enabled, the matching is for the item id, thus only the item for the displayed item id should work.

The load option is missing that option, thus the fix should be to add it there too.

@nullKomplex
Copy link
Contributor Author

nullKomplex commented Mar 3, 2025

In the text you are talking about the "Load Option", but your reproduction steps are for a trigger.

My intent was to cover all of the equipped item fields in WA, that's why. I likely did a poor job of that. Both the trigger and the load option are, in my opinion, bugged, and in the same way.

a) If not enabled, the matching is for the name, so both all versions of the item should work

This is not behaving as you have stated it should. Without exact match only the LFR version of my ring works. With exact match it's whichever ID is input into the field. I'll be honest though, I completely missed the existence of this option when I was originally writing this ticket. Regardless I believe it is bugged when not enabled.

The load option is missing that option, thus the fix should be to add it there too.

Correct, and as a result, the behavior described above without exact match is what occurs in-game.

Also I'd have to figure out how to get an item to test this but I don't think this is a Classic exclusive bug. I'm sure this could be replicated with retail items, it's just a lot rarer for items to share names and not be the same ID already. And you likely wouldn't be making auras for them.

@InfusOnWoW
Copy link
Contributor

Well, so let's debug the trigger. Without the "use_exact_itemName" option enabled, the trigger code should run in line 8711, so let's add a print there:

          local itemName = C_Item.GetItemInfo(triggerItemName)
          print("item:", itemName, itemSlot)
          local equipped = WeakAuras.CheckForItemEquipped(itemName, itemSlot)

I'm pretty sure that will output the name of the item.

And then WeakAuras.CheckForItemEquipped (when not checking a specific slot) is a very simple wrapper around C_Item.IsEquippedItem, but let's add a print there too:

print("C_Item.IsEquippedItem(", itemId, ") => ", C_Item.IsEquippedItem(itemId or ''))

And from testing on retail the output of all those calls look correct, which leads me to suspect that C_Item.IsEquippedItem might be bugged, that is if you give it a non-unique name it might internally convert it to an item id, and then check the wrong one.

@nullKomplex
Copy link
Contributor Author

nullKomplex commented Mar 3, 2025

[12:55:04] Equipping normal
[12:55:07] item: Ring of the Riven nil
[12:55:07] C_Item.IsEquippedItem( Ring of the Riven ) => false
[12:55:07] item: Ring of the Riven nil
[12:55:07] C_Item.IsEquippedItem( Ring of the Riven ) => false
[12:55:11] Equipping LFR
[12:55:13] item: Ring of the Riven nil
[12:55:13] C_Item.IsEquippedItem( Ring of the Riven ) => true
[12:55:13] item: Ring of the Riven nil
[12:55:13] C_Item.IsEquippedItem( Ring of the Riven ) => true

"Equipping" messages are manual prints by me.

@InfusOnWoW
Copy link
Contributor

InfusOnWoW commented Mar 3, 2025

So that's proves that C_Item.IsEquipped is simply broken. Great.

That means, we need something like

WeakAuras.CheckForItemEquipped = function(itemId, specificSlot)
  if not specificSlot then
    print("C_Item.IsEquippedItem(", itemId, ") => ", C_Item.IsEquippedItem(itemId or ''))
    if type(itemId) == "number" then
      return C_Item.IsEquippedItem(itemId or '')
    else
      for slot in pairs(Private.item_slot_types) do
        if WeakAuras.CheckForItemEquipped(itemId, slot) then
          return
        end
      end
    end
  else
    local item = Item:CreateFromEquipmentSlot(specificSlot)
    if item and not item:IsItemEmpty() then
      if type(itemId) == "number" then
        return itemId == item:GetItemID()
      else
        return itemId == item:GetItemName()
      end
    end
  end
end

@nullKomplex
Copy link
Contributor Author

The new loop doesn't return the result of whether or not it's equipped, but once fixed it works as expected.

@InfusOnWoW
Copy link
Contributor

Ah yeah needs to be return true.

Now, that fixes the trigger. I'll change the load option in the coming days and will upload a PR to test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏱ Awaiting Response This ticket hasn't been triaged yet. 🐛 Bug This is a problem with WeakAuras.
Projects
None yet
Development

No branches or pull requests

2 participants