Quantcast Which is Better? - WoWInterface
Thread Tools Display Modes
12-28-14, 12:23 PM   #1
cokedrivers
A Frostmaul Preserver
 
cokedrivers's Avatar
AddOn Author - Click to view addons
Join Date: Aug 2009
Posts: 296
Which is Better?

Which code is written in a more correct way or are they both the same?

Style A:
Lua Code:
  1. local cBuffs = CreateFrame("Frame")
  2. cBuffs:RegisterEvent("ADDON_LOADED")
  3. cBuffs:SetScript("OnEvent", function(self, event, arg1)
  4.     if event == "ADDON_LOADED" and arg1 == "cBuffs" then
  5.         BuffFrame:SetScale(1.193)
  6.        
  7.        
  8.         local origSecondsToTimeAbbrev = _G.SecondsToTimeAbbrev
  9.         local function SecondsToTimeAbbrevHook(seconds)
  10.             origSecondsToTimeAbbrev(seconds)
  11.  
  12.             local tempTime
  13.             if (seconds >= 86400) then
  14.                 tempTime = ceil(seconds / 86400)
  15.                 return '|cffffffff%dd|r', tempTime
  16.             end
  17.  
  18.             if (seconds >= 3600) then
  19.                 tempTime = ceil(seconds / 3600)
  20.                 return '|cffffffff%dh|r', tempTime
  21.             end
  22.  
  23.             if (seconds >= 60) then
  24.                 tempTime = ceil(seconds / 60)
  25.                 return '|cffffffff%dm|r', tempTime
  26.             end
  27.  
  28.             return '|cffffffff%d|r', seconds
  29.         end
  30.         SecondsToTimeAbbrev = SecondsToTimeAbbrevHook  
  31.  
  32.        
  33.         hooksecurefunc('AuraButton_Update', function(self, index)
  34.             local font = [[Fonts\ARIALN.ttf]]
  35.             local button = _G[self..index]
  36.             if (button) then
  37.                 local duration = _G[self..index..'Duration']
  38.                 if (duration) then
  39.                     duration:ClearAllPoints()
  40.                     duration:SetPoint('BOTTOM', button, 'BOTTOM', 0, -2)
  41.                     if (self:match('Debuff')) then
  42.                         duration:SetFont(font, 12, 'THINOUTLINE')
  43.                     else
  44.                         duration:SetFont(font, 12, 'THINOUTLINE')
  45.                     end
  46.                     duration:SetShadowOffset(0, 0)
  47.                     duration:SetDrawLayer('OVERLAY')
  48.                 end
  49.  
  50.                 local count = _G[self..index..'Count']
  51.                 if (count) then
  52.                     count:ClearAllPoints()
  53.                     count:SetPoint('TOPRIGHT', button)
  54.                     if (self:match('Debuff')) then
  55.                         count:SetFont(font, 12, 'THINOUTLINE')
  56.                     else
  57.                         count:SetFont(font, 12, 'THINOUTLINE')
  58.                     end
  59.                     count:SetShadowOffset(0, 0)
  60.                     count:SetDrawLayer('OVERLAY')
  61.                 end    
  62.             end
  63.         end)
  64.     end
  65.  
  66.     SlashCmdList['RELOADUI'] = function()
  67.         ReloadUI()
  68.     end
  69.     SLASH_RELOADUI1 = '/rl'
  70.    
  71.     self:UnregisterEvent("ADDON_LOADED")
  72. end)

Style B:
Lua Code:
  1. local cBuffs = CreateFrame("Frame")
  2. cBuffs:RegisterEvent("ADDON_LOADED")
  3. cBuffs:SetScript("OnEvent", function(self, event, arg1)
  4.     if event == "ADDON_LOADED" and arg1 == "cBuffs" then
  5.         self:SetScale()
  6.         self:TimeDisplayAdjust()
  7.         self:Duration_Count_Placement()
  8.     end
  9.  
  10.     SlashCmdList['RELOADUI'] = function()
  11.         ReloadUI()
  12.     end
  13.     SLASH_RELOADUI1 = '/rl'
  14.    
  15.     self:UnregisterEvent("ADDON_LOADED")
  16. end)
  17.  
  18. function cBuffs:SetScale()
  19.     BuffFrame:SetScale(1.193)
  20. end
  21.  
  22. function cBuffs:TimeDisplayAdjust()
  23.     local origSecondsToTimeAbbrev = _G.SecondsToTimeAbbrev
  24.     local function SecondsToTimeAbbrevHook(seconds)
  25.         origSecondsToTimeAbbrev(seconds)
  26.  
  27.         local tempTime
  28.         if (seconds >= 86400) then
  29.             tempTime = ceil(seconds / 86400)
  30.             return '|cffffffff%dd|r', tempTime
  31.         end
  32.  
  33.         if (seconds >= 3600) then
  34.             tempTime = ceil(seconds / 3600)
  35.             return '|cffffffff%dh|r', tempTime
  36.         end
  37.  
  38.         if (seconds >= 60) then
  39.             tempTime = ceil(seconds / 60)
  40.             return '|cffffffff%dm|r', tempTime
  41.         end
  42.  
  43.         return '|cffffffff%d|r', seconds
  44.     end
  45.     SecondsToTimeAbbrev = SecondsToTimeAbbrevHook
  46. end
  47.  
  48. function cBuffs:Duration_Count_Placement()
  49.     hooksecurefunc('AuraButton_Update', function(self, index)
  50.         local font = [[Fonts\ARIALN.ttf]]
  51.         local button = _G[self..index]
  52.         if (button) then
  53.             local duration = _G[self..index..'Duration']
  54.             if (duration) then
  55.                 duration:ClearAllPoints()
  56.                 duration:SetPoint('BOTTOM', button, 'BOTTOM', 0, -2)
  57.                 if (self:match('Debuff')) then
  58.                     duration:SetFont(font, 12, 'THINOUTLINE')
  59.                 else
  60.                     duration:SetFont(font, 12, 'THINOUTLINE')
  61.                 end
  62.                 duration:SetShadowOffset(0, 0)
  63.                 duration:SetDrawLayer('OVERLAY')
  64.             end
  65.  
  66.             local count = _G[self..index..'Count']
  67.             if (count) then
  68.                 count:ClearAllPoints()
  69.                 count:SetPoint('TOPRIGHT', button)
  70.                 if (self:match('Debuff')) then
  71.                     count:SetFont(font, 12, 'THINOUTLINE')
  72.                 else
  73.                     count:SetFont(font, 12, 'THINOUTLINE')
  74.                 end
  75.                 count:SetShadowOffset(0, 0)
  76.                 count:SetDrawLayer('OVERLAY')
  77.             end    
  78.         end
  79.     end)
  80. end

Thanks for any opinion on this.

Coke
  Reply With Quote
12-28-14, 01:31 PM   #2
Duugu
Premium Member
 
Duugu's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 851
My guess: as you're unregistering ADDON_LOADED without checking the addons name both Versions will only work if there are no other addons enabled or if there are no other addons loaded before your addon.

[e] Apart from such bugs imho the second one is just an awkward overcomplicated/redundant way to do things.

Last edited by Duugu : 12-28-14 at 01:33 PM.
  Reply With Quote
12-28-14, 04:37 PM   #3
Resike
A Pyroguard Emberseer
AddOn Author - Click to view addons
Join Date: Mar 2010
Posts: 1,286
Originally Posted by Duugu View Post
My guess: as you're unregistering ADDON_LOADED without checking the addons name both Versions will only work if there are no other addons enabled or if there are no other addons loaded before your addon.
Nah, unregistering ADDON_LOADED after your addon have been found and loaded, will save some resources. Because if you don't unregister it even after your addon is loaded it will run over the other addons which comes after yours alphabetically, and gonna try to compare their name to your addon, aka run the for cycle which loops over addons further. Unregistering is pretty much breaking from that cycle, after you found what you looking for.

It's pointless tho if your addon's name is at the end of the alphabet.

I personally like the second one, since it's gonna help you to keep organized with more complex projects, specially if you want to call functions from different files. But for such a small ones it's pretty much up to you which one you prefer, since the resourse usage difference is negligible even if you duplicate thoose functions.

Last edited by Resike : 12-28-14 at 04:45 PM.
  Reply With Quote
12-28-14, 04:47 PM   #4
Duugu
Premium Member
 
Duugu's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 851
Originally Posted by Resike View Post
Nah, unregistering ADDON_LOADED after your addon have been found and loaded, will save some resources.
Sure. But the code does not check if the event is for addon cBuffs before unregistering it. It simply unregisters the event if it is triggered for the first time.
  Reply With Quote
12-28-14, 04:48 PM   #5
sirann
A Flamescale Wyrmkin
Join Date: Mar 2007
Posts: 142
Originally Posted by cokedrivers View Post
if event == "ADDON_LOADED" and arg1 == "cBuffs" then
you only have the frame registered to addon_loaded so the only event that can cause the script to fire is addon_loaded, thus making the initial part of this two part if check, redundant and unncessary.
  Reply With Quote
12-28-14, 04:50 PM   #6
Duugu
Premium Member
 
Duugu's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 851
Originally Posted by sirann View Post
you only have the frame registered to addon_loaded so the only event that can cause the script to fire is addon_loaded, thus making the initial part of this two part if check, redundant and unncessary.
If I remember correctly the event fires for every addon that is loaded. I thought that's why arg1 provides the addon name.
  Reply With Quote
12-28-14, 05:23 PM   #7
sirann
A Flamescale Wyrmkin
Join Date: Mar 2007
Posts: 142
correct, all he needs is the arg1 == check, not the event check.

if arg1 == "cBuffs" then

instead of

if event == "ADDON_LOADED" and arg1 == "cBuffs" then
  Reply With Quote
12-28-14, 06:18 PM   #8
Duugu
Premium Member
 
Duugu's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 851
Originally Posted by sirann View Post
correct, all he needs is the arg1 == check, not the event check.

if arg1 == "cBuffs" then

instead of

if event == "ADDON_LOADED" and arg1 == "cBuffs" then
But, but ... he is unregistering the event outside of the if statement. Or not??
  Reply With Quote
12-28-14, 06:56 PM   #9
sirann
A Flamescale Wyrmkin
Join Date: Mar 2007
Posts: 142
I'm starting to think we are speaking about two different things. I was merely adding (regardless of the code below it) that there is no need for the event check, there's only one registered, it can only fire for addon_loaded.

I believe what you are getting at, is the fact that, as currently written, this script would fire as soon as the first addon is loaded, which I'm confident is not cbuffs. Since the initial if check requirements would not be met, the script would skip to the slashcommand creation, and would then unregister the only event it's listening for. This would essentially ensure the script never fires when he wants it to.

If that's what you're saying, I agree, and in no way intended to imply or convey otherwise. I just noticed a relatively common, and small, unoptimization.

If that's not what you're saying, please expand, because I don't see where I am incorrect in my suggestion.
  Reply With Quote
12-28-14, 07:26 PM   #10
Duugu
Premium Member
 
Duugu's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 851
Originally Posted by sirann View Post
If that's what you're saying, I agree, and in no way intended to imply or convey otherwise. I just noticed a relatively common, and small, unoptimization.
Thats it.
  Reply With Quote
12-28-14, 07:28 PM   #11
Resike
A Pyroguard Emberseer
AddOn Author - Click to view addons
Join Date: Mar 2010
Posts: 1,286
Originally Posted by Duugu View Post
Sure. But the code does not check if the event is for addon cBuffs before unregistering it. It simply unregisters the event if it is triggered for the first time.
Woops i missed that, you gotta put that unregisterevent inside the ADDON_LOADED block.
  Reply With Quote
12-29-14, 08:59 PM   #12
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
There is no value in delaying any of this stuff. You can just do all your function hooking in the main chunk, since all of the functions you're hooking are defined in FrameXML and not in LoD portions of the UI. You can also just call SetScale on the BuffFrame in the main chunk, since the default UI never calls that method anywhere. Just get rid of the whole frame and event handler.

However, for future reference if you do have things that need to be delayed, I'd just use the first style. Factoring out each item into its own function is wasteful. As a general rule, if a function will only ever be called from one place (either one time or 1000 times) you should just take the code from that function, and move it to the place where you call the function instead. Otherwise you're just wasting CPU cycles by adding another function call.

The only reason to factor setup things out into separate functions would be if you were going to make each item toggle-able on the fly with in-game options, but hooksecurefunc isn't togglable -- once it's done, it can't be undone -- so you would need to check the setting inside the hook function, and there still wouldn't be any point in having a separate function that does nothing but install the hook. Overwriting SecondsToTimeAbbrev and setting scales, however, are undoable, so you could toggle those on the fly, so if you went that way, it would be sensible to factor those out into a function that either did or undid those things, eg.

Code:
local old_SecondsToTimeAbbrev = SecondsToTimeAbbrev
local new_SecondsToTimeAbbrev = function(seconds)
    -- your code here
end
function MyAddon:EnableTimeHook(enable)
    if enable then
        SecondsToTimeAbbrev = new_SecondsToTimeAbbrev
    else
        SecondsToTimeAbbrev = old_SecondsToTimeAbbrev
    end
end
A couple other issues with the code in your first post:

Code:
local origSecondsToTimeAbbrev = _G.SecondsToTimeAbbrev
local function SecondsToTimeAbbrevHook(seconds)
	origSecondsToTimeAbbrev(seconds)

	local tempTime
	if (seconds >= 86400) then
		tempTime = ceil(seconds / 86400)
		return '|cffffffff%dd|r', tempTime
	end

	if (seconds >= 3600) then
		tempTime = ceil(seconds / 3600)
		return '|cffffffff%dh|r', tempTime
	end

	if (seconds >= 60) then
		tempTime = ceil(seconds / 60)
		return '|cffffffff%dm|r', tempTime
	end

	return '|cffffffff%d|r', seconds
end
SecondsToTimeAbbrev = SecondsToTimeAbbrevHook
1. There is no reason to call the original SecondsToTimeAbbrev function inside your replacement. The only thing this function does is return a formatted string. Since you do not use the string the original function returns, calling it is just a waste of CPU time.

2. There is no reason to assign a value to your tempTime value when you just return that value on the very next line. Just return the value directly and skip creating a variable:

Code:
	if (seconds >= 86400) then
		return '|cffffffff%dd|r', ceil(seconds / 86400)
	end
3. Unless you plan to toggle the function replacement on the fly, there is no reason to assign the replacement to a different name, and then assign the global to that replacement. Just assign the replacement function directly to the global:

Code:
function SecondsToTimeAbbrev(seconds)
	-- your code here
end
4. This function is used in many places in the UI. Since you are not actually using colors to represent anything (ie. everything is just colored white) you should remove the color codes, so the time strings will inherit the color being used by the font string they are displayed in.

Code:
hooksecurefunc('AuraButton_Update', function(self, index)
For clarity I would recommend not naming the first argument for this function self. That name is typically used for frame references, but the first argument here is not a frame reference -- it's a string containing the global name of a frame. Blizzard code names this argument buttonName, and I'd personally stick with that, but names like name or selfName would also be less confusing.

Code:
	if (self:match('Debuff')) then
		duration:SetFont(font, 12, 'THINOUTLINE')
	else
		duration:SetFont(font, 12, 'THINOUTLINE')
	end
1. You're not actually doing anything differently if the icon is for a debuff, so there's no reason for this check at all, unless this is a copy-paste error and you did mean to do something differently, in which case...

2. This is a good example of why naming a string value self is confusing. My first thought when I saw this part was that your code should be raising an "attempt to call match (a nil value)" error, and this will probably be the first throught of most experienced Lua programmers who look at this code. It will also be confusing to you if don't look at this code for a few weeks, months or even years, and then come back to it to make a change in the future.

3. String operations (match, find, format, etc.) are fairly expensive and should be avoided when possible. In this case, your goal is to distinguish between buff icons and debuff icons, so you can achieve that more efficiently by checking for to see if the buffon has a symbol member -- debuff icons have it, but buff icons don't.

Code:
	if button.symbol then
		-- it's a debuff icon
	else
		-- it's a buff icon
	end
And finally, as others have been trying to communicate in a rather roundabout and confusing fashion, there is a problem with your second code:

Lua Code:
  1. local cBuffs = CreateFrame("Frame")
  2. cBuffs:RegisterEvent("ADDON_LOADED")
  3. cBuffs:SetScript("OnEvent", function(self, event, arg1)
  4.     if event == "ADDON_LOADED" and arg1 == "cBuffs" then
  5.         -- some stuff was here
  6.     end
  7.  
  8.     -- some other stuff was here
  9.    
  10.     self:UnregisterEvent("ADDON_LOADED")
  11. end)

First, not really a problem, but you don't need to check if event == "ADDON_LOADED". Your frame is not registered for any other events, so it will never receive any other events. Every event it receives will be an ADDON_LOADED event.

Second, and more importantly, the "some other stuff" and the UnregisterEvent lines are called on any ADDON_LOADED event, not just the one where arg1 == "cBuffs", so you're adding a slash command and then unregistering your event on the first event received. That's probably your addon's event, but it's not guaranteed to be, and if it's not, your frame will never receive your addon's event because you already told it to stop listening. You need to either (a) wrap the entire contents of the event handler in the check:

Code:
	if arg1 == "cBuffs" then
		-- some stuff was here

		-- some other stuff was here
	
		self:UnregisterEvent("ADDON_LOADED")
	end
or (b) save yourself some tab key pressing and just use a return statement at the top of the function:

Code:
	if arg1 ~= "cBuffs" then return end

	-- some stuff was here

	-- some other stuff was here
	
	self:UnregisterEvent("ADDON_LOADED")
__________________
Author/maintainer of Grid, PhanxChat, oUF_Phanx, and many more.
Troubleshoot an addonTurn any code into an addonMore addon resources
Need help with your code? Post all of your actual code! Attach or paste your files.
Please don’t PM me about addon bugs or code questions. Post a comment or forum thread instead!
  Reply With Quote
12-30-14, 06:03 AM   #13
Banknorris
A Chromatic Dragonspawn
 
Banknorris's Avatar
AddOn Author - Click to view addons
Join Date: Oct 2014
Posts: 153
Just my 2 cents: I think it is a good practice to always check the event even if you just have one registered, always doing this will make maintaining your code easier. You will just need to add lines, same about event handler, sometimes you don't need to use the parameters and could just
f:SetScript("OnEvent", function()
but if you always declare
f:SetScript("OnEvent", function(self, event, ...)
you made your code more generic and easier to maintain.
Scripted language main point is to descriptive, being a performance freak in scripted languages does you no good when that makes your code harder to read and maintain. Of course that is no excuse to poor coding though.

Last edited by Banknorris : 12-30-14 at 01:42 PM.
  Reply With Quote
12-30-14, 06:29 AM   #14
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
If I'm writing a simple "delay for ADDON_LOADED or PLAYER_LOGIN" frame that only handles that one event by design, I would never add an event check, since it's blatantly obvious that the frame is only handling one event and will only ever handle one event. Adding an event check in this type of code just wastes (imaginary) CPU time and adds useless clutter. I'd argue that the clutter aspect actually makes your code harder to maintain, since when you come back and look at it and see an event check, you assume it must be able to handle other events, and then are confused as to why the hell you included an event check on a frame that only handles one event.
__________________
Author/maintainer of Grid, PhanxChat, oUF_Phanx, and many more.
Troubleshoot an addonTurn any code into an addonMore addon resources
Need help with your code? Post all of your actual code! Attach or paste your files.
Please don’t PM me about addon bugs or code questions. Post a comment or forum thread instead!
  Reply With Quote
12-30-14, 11:50 AM   #15
ravagernl
Proceritate Corporis
Premium Member
AddOn Author - Click to view addons
Join Date: Feb 2006
Posts: 1,176
Originally Posted by Phanx View Post
If I'm writing a simple "delay for ADDON_LOADED or PLAYER_LOGIN" frame that only handles that one event by design, I would never add an event check, since it's blatantly obvious that the frame is only handling one event and will only ever handle one event. Adding an event check in this type of code just wastes (imaginary) CPU time and adds useless clutter. I'd argue that the clutter aspect actually makes your code harder to maintain, since when you come back and look at it and see an event check, you assume it must be able to handle other events, and then are confused as to why the hell you included an event check on a frame that only handles one event.
^This The following has somewhat been my personal boilerplate for several years (inspired by several genious people, AFAIK that includes tekkub, haste and p3lim, among others...)

lua Code:
  1. -- events.lua
  2. local addon_name, addon = ...
  3. local event_frame = CreateFrame("Frame")
  4. event_frame:SetScript("onevent", function(self, event, ...)
  5.     if addon[event] then
  6.         addon[event](addon, ...)
  7.     else
  8.         error(addon_name, "unhandled event:", event, ...)
  9.     end
  10. end)
  11. function addon:RegisterEvent(event, function)
  12.     event_frame:RegisterEvent(event)
  13.     if function and not addon[event] then
  14.         addon[event] = function
  15.     -- elseif function then
  16.         -- error(addon_name, "attempt to register handler: handler already exists for event ", event)
  17.     end
  18. end
  19. function addon:RegisterEvents(function, event, ...)
  20.     if event then
  21.         self:RegisterEvent(event, function)
  22.         self:RegisterEvents(function, ...)
  23.     end
  24. end
  25.  
  26. function addon:UnregisterEvent(event, ...)
  27.     if event then
  28.         event_frame:UnregisterEvent(event)
  29.         self:UnregisterEvent(...)
  30.     end
  31. end
  32. addon.UnregisterEvents = addon.UnregisterEvent
  33.  
  34. function addon:KillEvents(event, ...)
  35.      self:UnregisterEvents(event, ...)
  36.      if event then
  37.           self[event] = nil
  38.           self:KillEvents(...)
  39.      end
  40. end
  41. addon.KillEvent = addon.KillEvent
lua Code:
  1. -- core.lua
  2.  
  3. local addon_name, addon = ...
  4.  
  5. function addon:PLAYER_LOGIN()
  6.     -- on login (saved variables loaded + spellbook ready, etc...)
  7.  
  8.     self:KillEvent("PLAYER_LOGIN")
  9. end
  10.  
  11. addon:RegisterEvent("ADDON_LOADED", function(loaded_addon)
  12.     if addon_name ~= loaded_addon then return end
  13.     self:KillEvent("ADDON_LOADED")
  14.     -- on initialize (saved variables loaded)
  15.  
  16.     if IsLoggedIn() then
  17.         self:PLAYER_LOGIN()
  18.     else
  19.         self:RegisterEvent("PAYER_LOGIN")
  20.     end
  21. end)
I'm not saying you should or should always use this method, as it probably contains errors and it has both disadvantages and advantages, if you have a lot of events to register for, then for readability, you could use this method.

Last edited by ravagernl : 12-30-14 at 12:36 PM. Reason: Oh Brain you... (bonk bonk bonk)
  Reply With Quote
12-30-14, 11:59 AM   #16
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Well, that's not quite relevant to what I was saying; I was talking about a frame that's created for the sole purpose of handling a single hardcoded event, typically just one time, such as delaying something for after saved variables are loaded or after spell information is available. Obviously if your frame is handling more than one event, or if you dynamically register and unregister events, you probably have to check the event, and you are doing that in the code you posted.

Originally Posted by ravagernl View Post
Code:
function addon.PLAYER_LOGIN()
    -- on login (saved variables loaded + spellbook ready, etc...)

    self:KillEvent("PLAYER_LOGIN")
end
I hope this was the result of "I'm too lazy to go find my real code and copy/paste it so here's some badly written fake code instead" and not your actual code, or you should be getting an error since "self" is not defined in that scope since you used a period instead of a colon.

Also, why go with "KillEvent" instead of just sticking to convention and naming that "UnregisterEvent" ?
__________________
Author/maintainer of Grid, PhanxChat, oUF_Phanx, and many more.
Troubleshoot an addonTurn any code into an addonMore addon resources
Need help with your code? Post all of your actual code! Attach or paste your files.
Please don’t PM me about addon bugs or code questions. Post a comment or forum thread instead!
  Reply With Quote
12-30-14, 12:21 PM   #17
ravagernl
Proceritate Corporis
Premium Member
AddOn Author - Click to view addons
Join Date: Feb 2006
Posts: 1,176
Originally Posted by Phanx View Post
Well, that's not quite relevant to what I was saying; I was talking about a frame that's created for the sole purpose of handling a single hardcoded event, typically just one time, such as delaying something for after saved variables are loaded or after spell information is available. Obviously if your frame is handling more than one event, or if you dynamically register and unregister events, you probably have to check the event, and you are doing that in the code you posted.
If ADDON_LOADED is triggered in response to the AddOn being loaded on demand (or by a loadmanager) then PLAYER_LOGIN doesn't trigger.

I hope this was the result of "I'm too lazy to go find my real code and copy/paste it so here's some badly written fake code instead" and not your actual code, or you should be getting an error since "self" is not defined in that scope since you used a period instead of a colon.
Must..Resist... Well ok.

It indeed is, wrote that from memory and didn't test it, I'm a bit tired and chilling on my laptop

Also, why go with "KillEvent" instead of just sticking to convention and naming that "UnregisterEvent" ?
Because I'm lazy UnregisterEvent is in there too, KillEvent just also kicks the handler function away from the table scope to reduce lookups Could have named it UnregisterEventAndNillifyHandler, but it's a bitch to type.

Last edited by ravagernl : 12-30-14 at 12:34 PM. Reason: damn you bbcode!
  Reply With Quote
12-30-14, 07:30 PM   #18
cokedrivers
A Frostmaul Preserver
 
cokedrivers's Avatar
AddOn Author - Click to view addons
Join Date: Aug 2009
Posts: 296
Thank you everyone for the advice and help.

So here is what I have came up withy for the cBuffs.lua
Lua Code:
  1. BuffFrame:SetScale(1.193)
  2.  
  3. local origSecondsToTimeAbbrev = _G.SecondsToTimeAbbrev
  4. local function SecondsToTimeAbbrevHook(seconds)
  5.  
  6.     if (seconds >= 86400) then
  7.         return '%dd', ceil(seconds / 86400)
  8.     end
  9.  
  10.     if (seconds >= 3600) then
  11.         return '%dh', ceil(seconds / 3600)
  12.     end
  13.  
  14.     if (seconds >= 60) then
  15.         return '%dm', ceil(seconds / 60)
  16.     end
  17.  
  18.     return '%d', seconds
  19. end
  20. SecondsToTimeAbbrev = SecondsToTimeAbbrevHook
  21.  
  22.  
  23. hooksecurefunc('AuraButton_Update', function(buttonName, index)
  24.     local font = [[Fonts\ARIALN.ttf]]
  25.     local button = _G[buttonName..index]
  26.     if (button) then
  27.         local duration = _G[buttonName..index..'Duration']
  28.         if (duration) then
  29.             duration:ClearAllPoints()
  30.             duration:SetPoint('BOTTOM', button, 'BOTTOM', 0, -2)
  31.             if button.symbol then
  32.                 duration:SetFont(font, 12, 'THINOUTLINE')
  33.             else
  34.                 duration:SetFont(font, 12, 'THINOUTLINE')
  35.             end
  36.             duration:SetShadowOffset(0, 0)
  37.             duration:SetDrawLayer('OVERLAY')
  38.         end
  39.  
  40.         local count = _G[buttonName..index..'Count']
  41.         if (count) then
  42.             count:ClearAllPoints()
  43.             count:SetPoint('TOPRIGHT', button)
  44.             if button.symbol then
  45.                 count:SetFont(font, 12, 'THINOUTLINE')
  46.             else
  47.                 count:SetFont(font, 12, 'THINOUTLINE')
  48.             end
  49.             count:SetShadowOffset(0, 0)
  50.             count:SetDrawLayer('OVERLAY')
  51.         end    
  52.     end
  53. end)
  54.  
  55. SlashCmdList['RELOADUI'] = function()
  56. ReloadUI()
  57. end
  58. SLASH_RELOADUI1 = '/rl'

Again thank you for the help.
Coke
  Reply With Quote
12-30-14, 07:51 PM   #19
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
Much better, though you shouldn't need the "if (button) then ... end" wrapper around the contents of your AuraButton_Update hook, since the default UI should never be calling update functions for buttons that don't exist. Leaving it won't really affect performance, it just adds an extra level of indentation for no particular reason. Smaller amounts are usually better when it comes to code.
__________________
Author/maintainer of Grid, PhanxChat, oUF_Phanx, and many more.
Troubleshoot an addonTurn any code into an addonMore addon resources
Need help with your code? Post all of your actual code! Attach or paste your files.
Please don’t PM me about addon bugs or code questions. Post a comment or forum thread instead!
  Reply With Quote
12-31-14, 08:56 PM   #20
cokedrivers
A Frostmaul Preserver
 
cokedrivers's Avatar
AddOn Author - Click to view addons
Join Date: Aug 2009
Posts: 296
Thank You Phanx I removed the "if button ... end"

Finally almost complete with my personal UI

Still to do:
A) Find/work out a slim downed (with only the option I want) chat mod like PhanXChat or nChat from NeavUI.

B) Get nData to work better with WoD and use less resources.


Thanks Everyone for your advice and help.
Coke

[EDIT] for those that are wondering what this buff mod does it pretty basic It scales the buffs bigger and mover the text inside the buff icon (see image below)

Last edited by cokedrivers : 12-31-14 at 09:12 PM.
  Reply With Quote

WoWInterface » Developer Discussions » Lua/XML Help » Which is Better?

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off