WowRarity / Rarity

Collectibles and statistics tracking addon for World of Warcraft
https://www.curseforge.com/wow/addons/rarity
GNU General Public License v2.0
55 stars 35 forks source link

COLLECTION items aren't tracked when they define a single item ID as a number and not a table (?) #413

Open rdw-software opened 3 years ago

rdw-software commented 3 years ago

Not sure on the exact details here, but if I understood this correctly then the collectedItemId property isn't working when you define a single itemID, like so: collectedItemId = 12345, i.e. you need to define it as collectedItemId = { 12345 }, which is the syntax usually reserved for multiple items.

If so, this is inconsistent with how any other property is used. I'd say it's probably a bug and rooted in the OnBagUpdate code that I recently reviewed.

@godejord Can you confirm this is the issue you encountered (or correct me if I'm wrong)?

godejord commented 3 years ago

Yeah, this was my conclusion while adding the new 9.1 collection mounts. Both mounts are obtained after gathering X amount of one type of item each. I added them without curly braces because there was only a singular id per mount, and was not able to get any recorded attempts. While debugging, I saw that the function that scans all items in the inventory (ProcessInventoryItems) was not even looking for the ID I had added. After adding curly braces around the id's, they began tracking attempts just perfectly.

rdw-software commented 3 years ago

Possibly related: https://www.wowace.com/projects/rarity?comment=2660

Getting this bug since the latest version 22x Rarity\Core\EventHandlers.lua:1112: bad argument #1 to 'pairs' (table expected, got number)

This is trying to access the collectedItemId as a table. Not sure why the table check has vanished.

I think the problem is in line 1047 where HasMultipleCollectionItems tests against "item" instead of "collectionItem" as changing it does stop the errors.

Maybe an oversight while fixing the detection?

rdw-software commented 3 years ago

Nevermind, I think I just made a mistake when refactoring that part. It was checking the wrong item, though weirdly I didn't get any errors myself.