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

Container items are no longer tracked reliably after the latest patch (?) #500

Closed
rdw-software opened this issue Nov 12, 2022 · 22 comments · Fixed by #620
Closed

Container items are no longer tracked reliably after the latest patch (?) #500

rdw-software opened this issue Nov 12, 2022 · 22 comments · Fixed by #620

Comments

@rdw-software
Copy link
Member

rdw-software commented Nov 12, 2022

There have been multiple reports of this problem:

Why Rarity addon stopped printing attempt numbers for [Knockoff Blingtron]? It is still in the list of not found pets.
Source: Curse

Since prepatch, it hasn't been counting Strange Humming Crystal from Kirin Tor Chests nor Thistleleaf Adventurer from Dreamweaver Provisions.
Source: WowAce

And more on Discord (I suspect they're all referring to the same problem):


Looking into this, it seems something wonky is going on:

  • Usually, BAG_UPDATE triggers (twice) and Rarity should detect the attempt after opening a container
  • Sometimes it now seems to not trigger at all, in which case SHOW_LOOT_TOAST was the only event I saw (may depend on the item?)
  • Or they might trigger "too early" (before the container loot has been obtained), if I didn't miss it...
  • In such a case, Rarity will fail to pick up the new items since they aren't present when the inventory is scanned
  • However, in theory it should pick up the attempt when the bags are scanned again later (but it doesn't?)

I guess this needs more investigation, but it's tricky because you need to find container items to open (which are not ubiquitous) AND you can only test it once per item, but also each container may have different rules (loot toast vs bag update...) so it's difficult to generalize.

@rdw-software
Copy link
Member Author

Another theory I had was that the cursor event changes (#493 ) somehow affected the detection for container items, but I can see no way that these two different detection methods should interact. They're two separate event handlers and the inventory scanning logic should not interact with the "node detection" code that is based on tooltip scanning...

@Furydins
Copy link

Debug output from opening a Cracked Fel spotted Egg:

Rarity: LOOT_READY with target: NO TARGET
Rarity: Locking session for 1 second(s) to prevent duplicate attempts
Rarity: LOOT_READY with target: NO TARGET
Rarity: Session is locked ignorning this LOOT READY event
Rarity: Bag Update
Rarity: Setting last count to nil
Rarity: Setting last count to nil

I still have two of the rays to collect - neither had their attempt counts updated.

@Furydins
Copy link

From what I could see from opening the Cracked Fel Spotted Eggs - there's no BAG_UPDATE event firing only the LOOT_READY event.

@Furydins
Copy link

Not sure how to capture ETRAC output But here's what I got from itL

ITEM_LOCK_CHANGED
ITEM_LOCKED
LOOT_READT true
LOOT_OPENED
LOOT_READY
LOOT_SLOT_CLEARED
ITEM_LOCK_CHANGED
ITEM_UNLOCKED
LOOT_CLOSED
LOOT_CLOSED
ITEM_PUSH 0,1570762
SOCIAL_ITEM_RECIEVED
CHAT_MSG_LOOT
BAG_NEW_ITEMS_UPDATED
UNIT_INVENTORY_CHANGED "Player"
BAG_UPDATE 0
UNIT_INVENTORY_CHANGED @player:
BAG_UPDATE 0
BAG_UPDATE 4
BAG_UPDATE_DELAYED
ITEM_COUNT_CHANGED
ITEM_COUNT_CHANGED 153054

@Furydins
Copy link

Annoyingly I forgot to put debug mode on so I don't know if the addon missed the BAG_UPDATE (as it did with the debug ouput I posted. I've another egg in a few days so I'll check then.

@rdw-software
Copy link
Member Author

From what I could see from opening the Cracked Fel Spotted Eggs - there's no BAG_UPDATE event firing only the LOOT_READY event.

Yes, I noticed that this event was now used by some items. This alone already causes a huge problem, as I've no idea which items use it and it's not feasible to go through all the "container" items Rarity tracks to test if they're still using the old method or LOOT_READY after the patch.

But there's more: Since many of the items that have been reported to be tracked unreliably are actually using the old method still, and "sometimes" the tracking is working while at other times it is not. This is presumably because either the standard BAG_READY event isn't fired, is fired too early/late, the timeout Rarity uses to prevent duplicate attempts is no longer appropriate, or there might be some other reason Rarity doesn't pick up the attempt "sometimes". Or it could just be yet another Blizzard bug, who knows?

Needs more testing, since "sometimes" isn't enough to determine what's going on and devise of a fix, if there is any.

@rdw-software rdw-software removed this from the Maintenance milestone Jan 9, 2023
@Urtgard
Copy link

Urtgard commented Jan 31, 2023

Reminder for debugging:
After you collected said container on the live server, you can test this as often as you like on the ptr.
Just copy your character to the ptr and open the container.

@vttale
Copy link

vttale commented Feb 10, 2023

This is probably the same issue underlying why Love Rocket attempts aren't being counted, yes?

@rdw-software
Copy link
Member Author

I'm not positive on that, but it seems likely that the container issue would also affect the "heart-shaped box" containers, yes.

@Furydins
Copy link

Furydins commented Feb 10, 2023 via email

@TreizeEU
Copy link

I'll add that it works for me, but only sometimes?

First few days of Love is in the Air was more untracked attempts than tracked attempts. And I can't tell if it's certain alts that's not tracked correctly, or just randomly bugs out.

@lfstyles
Copy link

Still not updating Heart shaped box attempts for me. It did it once but that was it.

@Furydins
Copy link

Furydins commented Feb 24, 2023

I had a Tribute of the Ambitious chest (Maldraxxus calling - has a chance to drop the necroray egg), and rarity did (according to the debug log) register that I looted it but did not register the attempt.

Looking at the logs rarity only got a BAG_UPDATE event and did process the items I recieved from looting the chest but didn't register it needed to increment the counter:

rarity: BAG_UPDATE
rarity: Processing inventory item 194820 (currentInventoryAmount: 10)
Rarity: processing Inventory Amount 184307 (currentInventory Amount 5)
.....
Rarity: Processing Inventory Amount 191320 (currentInventoryAmount 10)
Rarity: Setting lastNode to nil

I haven't included the entire list of items it processed but the item ID for the chest did not appear in it.

@Furydins
Copy link

Furydins commented Feb 26, 2023

Quick update I did a Necro calling today and got a Tribute of the duty bound (the other chest) The addon got a BAG_UPDATE when I recieved the chest and did process that I had the item:

rarity: processing Inventory item 181733 (currentInventoryAmount: 1).

When I opened the chest rarity got a BAG_UPDATE event but there was no mention at all of item 181733 - so it kinda looks like it's not registering that the inventory amount from the chest changed from 1->0.

I:m still farming the egg so I'll be looting these chests from time to time. If there's anything else I can check or capture let me know.

@Furydins
Copy link

Looking at the code I'd expect to see either an "Processed item %s is something we're tracking" or "FOUND ITEM %d!" and I saw neither in the debug output when I got the container even tho the "rarity: processing Inventory item 181733 (currentInventoryAmount: 1)" was there.

@Furydins
Copy link

Furydins commented Mar 13, 2023

Ok I added some extra debugging to function R:ProcessContainerItems() and my finding is that when I open the container it is detecting that the container is something that rarity is tracking and that it decreased (from 1->0).

It appears that the loop inside R:ProcessContainerItems() that scans for items that care about the container isn't finding anything whose attempt count should be increased. I'm fairly certain at this stage that it's a bug in Rarity and not a Blizzard issue.

I did a scan through the changes to EventHandler but don't see anything that could explain what broke. My current suspicion is that there may be an issue with Rarity's DB.

I'm going to see if I can come up with some more intrusive debug output in my local build to get a deeper view into why/how it's missing that the container has things it should track.

@Furydins
Copy link

Having checked the Db - it seems the Necroray Egg isn't flagged as repeatable so having collected two in the past it was no longer set to tracked.

Sadly this makes these a bit of red herring for the overall issue.

@rdw-software
Copy link
Member Author

Do we know of any containers that frequently (if not reliably) cause this problem? I tried a few, that are easy to get like Blingtron gifts etc, but I was unable to get even a single failed attempt this time around. That is to say, everything seemed to be working.

If there was a timing issue or similar bug in Rarity, I would expect the bug to manifest if I just open enough containers. But even several hundred of the MOP food thingies (which can easily be purchased, hence I used them for testing) did not exhibit the behavior. It's obviously not practical to farm emissary caches or similar items just in case the bug occurs, or repeat PTR copies.

As for some containers now using LOOT_READY, I don't have any solution other than basically duplicating the container item detection and checking both methods "just in case" an item now uses a different method. But it'd need testing with different item types and I don't know if it'll cause problems further down the line. There haven't been many reports on this so maybe it's not affecting as many items as I originally suspected? Needs more info I guess.

@Furydins
Copy link

Furydins commented Aug 5, 2023

From memory I was consistently having issues with the chests (Invaders forgotten treasures) from the Garrison invasion. I got the mounts eventually and stopped doing them so don't know if it remains an issue.

The issue with the fel spotted egg I had a look at upstream I eventually traced to it not being set up in rarity as repeatable (you need 3 of them to drop to collect all the mounts) and Rarity had stopped tracking after I got my 1st. The LOOT_READY I was tyring to debug with those was just a red herring I think.

The other one I noticed more recently is the Chests from the Island Expedition Dubloon's vendor - I haven't checked but I'd hazard a guess Rarity just wasn't set up to track those as they were added midway though BFA's final patch?

@Rubio9
Copy link

Rubio9 commented Aug 6, 2023

I originally noticed this issue with Hallows End containers as it was not reliably counting attempts at the mount. I imagine (if the issue remains) it'd be applicable to any of the similar holiday events with loot containers. Hallows End is coming up again, is definitively a case where the issue showed up last year, and is repeatable daily times however many max-level characters a player has to try it on. One way or another, I'll be, eh, "testing" it quite a bit this year again.

@rdw-software
Copy link
Member Author

The other one I noticed more recently is the Chests from the Island Expedition Dubloon's vendor - I haven't checked but I'd hazard a guess Rarity just wasn't set up to track those as they were added midway though BFA's final patch?

I've just pushed an untested fix, which should be available in the latest alpha. No way I can test expeditions, but it "should work".

Unfortunately, garrison invasions are also difficult to test so I'll pass on that. Not sure about the rest, might revisit this later...

@rdw-software
Copy link
Member Author

Reviewing all these comments, it's probably helpful to treat this as multiple (potentially unrelated) issues:

  1. Legion emissary chests not counting: Cannot reproduce, might be a stale report or caused by a script error
  2. Blingtron gift package: More difficult to test (daily lockout) - it worked for me, however
  3. Necroray egg: I'll assume this is resolved as per the last comments related to it
  4. Container items using LOOT_READY - I think this might be how they've always worked and I just got confused (?)
  5. Love rocket: Uses the same method as legion emissaries, but no idea if it triggers the same events - can't test now
  6. Garrison invasion: I'll test this separately if I can find the time, but if it doesn't fail reliably then it's not worthwhile to debug

To summarize the results from my recent testing:

  • The existing BAG_UPDATE handling uses bucketed events and seems to be working just fine
  • The LOOT_READY handling is questionable, I'll rework this shortly (using bucketed events)
  • Rarity might sometimes miss attempts if opening multiple containers too fast (I'll ignore that for now as it's easily avoided)
  • The cursor API changes seem to not have caused any issues from what I can tell
  • No one has reported any item that "frequently" isn't working and that I can easily test (i.e., isn't the love rocket)

Lastly, if the detection was indeed broken, I'd expect TONS of bug reports - but there weren't really any, at least recently. So I'll improve the LOOT_READY handling via AceBucket and then this can probably be considered resolved. If the problem resurfaces with Hallow's End, feel free to comment here or open a new issue. Same if there's any other items that are clearly affected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants