View Single Post
06-13-16, 01:55 PM   #19
Rainrider
A Firelord
AddOn Author - Click to view addons
Join Date: Nov 2008
Posts: 454
The way you uploaded it, it won't load at all (no ToC file).

You need a way to tell users when data changed. You could use CallbackHandler for this.

Your RefreshEquipped() and RefreshBags() are double dipping as SocketInventoryItem and SocketContainerItem will issue ARTIFACT_UPDATE and your event handler will call RefreshPowers() again.

As long as you call RefreshEquipped() and RefreshBags() only at PLAYER_ENTERING_WORLD (PEW) you could spare the checks for ArtifactFrame. Your first call to SocketInventoryItem will load the ArtifactUI and from then on you actually open and close ArtifactFrame for every artifact in the player's inventory. This is probably not a problem as the user won't have any open UI panels during PEW. If you change this (by adding a forced update), you will have to consider that the ArtifactFrame listens for ARTIFACT_UPDATE as well.

GetPowerPurchased actually returns the number of purchased traits. Maybe GetNumTraitsPurchased would be a better name. Same goes for GetPowers and the like. I know you follow Blizzard's own naming here, but they probably coded that before the UI wording was set in place and didn't care to update the API to reflect that. Also prepending Lib to your addon name would be nice, but this is just my personal taste.

As for features, you could also provide the knowledge level and multiplier as those are only available when the ArtifactFrame is shown. They are shared between artifacts. The API for them is C_ArtifactUI.GetArtifactKnowledgeLevel() and C_ArtifactUI.GetArtifactKnowledgeMultiplier().