View Single Post
01-27-14, 02:39 PM   #2
Choonstertwo
A Chromatic Dragonspawn
 
Choonstertwo's Avatar
AddOn Author - Click to view addons
Join Date: Jan 2011
Posts: 194
I've gone through your code and written up recommendations as I encounter issues:
  • Give your visible frame a name (prefix it with your actual AddOn name, not MyAddon) and store the reference returned by CreateFrame in a local variable instead of a global one (when you give a frame a name, it's automatically set to the global variable of that name).
  • Make MyAddon_isGearEqSet a local function instead of a global one.
  • Don't copy the IDs returned by GetEquipmentSetItemIDs into a new table, just use the one you're given.
  • Declare itemArrayPlayerEq once outside of the function and reuse it instead of creating a new table each time.
  • Do the same for itemArray, passing it as the second argument to GetEquipmentSetItemIDs
  • Don't loop all the way to 19 if you're always going to ignore slots 18 and 19.
  • When building the array of the player's current equipment, set the keys directly instead of using table.insert (itemArrayPlayerEq[i] = GetInventoryItemID("player", i))
  • Don't use the length operator (#) on arrays with holes in them (the arrays will have nils for any equipment slot without an item)
  • Make MyAddon_getGearEqSet a local function.
  • Use the return value of GetNumEquipmentSets directly in the for loop counter (this is a very minor issue; what you have now isn't wrong, it's just longer than it needs to be)
  • I'm pretty sure you can get rid of your MyAddon_isGearEqSet function by checking the fourth return value of GetEquipmentSetInfo in the MyAddon_getGearEqSet function.
  • Another very minor issue: You can use the return value of GetZonePVPInfo directly in the if condition of your OnEvent handler without the extra variable unless you plan to use the value somewhere else in the function.

I've written quite a lot, but many of these are very simple to fix.

This probably belongs in General Authoring Discussion or Lua/XML Help; it'll probably be moved to the appropriate forum.

Last edited by Choonstertwo : 01-27-14 at 02:48 PM.
  Reply With Quote