View Single Post
10-07-15, 10:58 AM   #2
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
There's no need for separate frames for display and handling events. Being hidden or shown doesn't affect whether a frame receives events.

Declaring all your variables at the file scope instead of the scope where they're actually used isn't a good practice. Your "total" and "equipped" variables are only used in the PLAYER_AVG_ITEM_LEVEL_UPDATE event handler, so you should just stick a "local" in front of the line that assigns them the values returned by GetAverageItemLevel().

Similarly, when you need to run a function to find a value (eg. your CheckBags function) rather than declaring a variable at the top of the file, running the function to update the variable's value, then reading the variable, it's cleaner to just have the function return the desired value. That way you just assign it like any other function regurn, eg. "local count = CheckBags()" -- but in this case, you don't need that CheckBags function at all. Since all you want to know is "how many of item 109217 do I have in my bags?" you can just use the API function that answers that question:

Code:
local count = GetItemCount(109217)
You could also, in theory, display the three boxes side by side, so in case you're failing twice (or thrice!) as much you can see.
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.
  Reply With Quote