View Single Post
06-14-16, 01:21 AM   #20
kokomala
A Fallenroot Satyr
AddOn Author - Click to view addons
Join Date: Oct 2008
Posts: 29
Originally Posted by Rainrider View Post
*snip*
Nice feedback. First library I've written, so a small learning curve - but everything helps

The library will work fine if it's embedded and your addon is reacting to events to pull data. I can make it standalone enabled and implement CallBackHandler to fire a cache update event, which will make this a little more friendly for UI display updates.

Feature wise, this was more intended towards caching just the traits. However thinking on it, it might be better to simply expand it to cache more of the data provided by C_ArtifactUI API (saves future requests). I'll need to look at the API listing again and see how best to manage what options there are. I don't want the library to take up too much UI time if I can help it. A fair bit of the functions will likely not be needed.

Naming wise was to keep it the somewhat similar to Blizzard's, as this partially turns the library into a cached alias for C_ArtifactUI, which is why I've opted to return information in the same format. I did totally do a goof on PowersPurchased (should have used GetTotalPurchasedRanks) :P

Your RefreshEquipped() and RefreshBags() are double dipping as SocketInventoryItem and SocketContainerItem will issue ARTIFACT_UPDATE and your event handler will call RefreshPowers() again.
This was the case on the first release and should be fixed in the 1.1 version. I basically shifted f:RegisterEvent('ARTIFACT_UPDATE') into the PEW event code, and then unregistered the PEW event to avoid rescanning whenever you hit a load screen.

--
One thing I've been debating on is if I should add bank inventory scanning - I'm leaning towards yes, regardless of how unlikely it is for an unscanned artifact to be equipped and used prior to caching that specific artifact. However this is entirely dependant if I can code it such that it doesn't intrude on the user - part of the issue is you can't 'socket' an item whilst it's in the bank.

Thanks again. Will keep the update discussion in this thread as I progress

edit:
As long as you call RefreshEquipped() and RefreshBags() only at PLAYER_ENTERING_WORLD (PEW) you could spare the checks for ArtifactFrame.
Initially I was going to keep this in, but after playing around with bag scanning after closing the bank it's far simpler to simply register/unregister both the UI and my controller for ARTIFACT_UPDATE during the scan. This avoids multiple triggers and it's fast enough.

The only issue is that if you have the artifact frame open, then close the bank - the artifact frame will close due to the data caching. I'd rather not have the library do this, but there's no other way to cache a new artifact. Something to muse over.

Last edited by kokomala : 06-14-16 at 09:09 AM.