Download
(96 Kb)
Download
Updated: 06-21-14 04:11 PM
Compatibility:
Siege of Orgrimmar (5.4)
Updated:06-21-14 04:11 PM
Created:06-17-14 01:54 AM
Downloads:88
Favorites:1
MD5:
5.4
Juke
Version: 0.3
by: Ketho [More]
Shows Jukes and Interrupts
  • Shows Jukes and Interrupts in the chat frame
  • Custom message formatting
  • Supports announce to chat (disabled in battlegrounds)
Slash Command
/juke



v0.3 [2014.06.22]
  • Added esES and esMX locales
  • Optimized code as suggested by Phanx
v0.2 [2014.06.19]
  • Added "Solo Mode" option for only showing your own interrupts/jukes
  • Fixed some bugs
v0.1 [2014.06.17]
  • Initial release
Optional Files (0)


Archived Files (2)
File Name
Version
Size
Author
Date
0.2
96kB
Ketho
06-19-14 08:17 AM
0.1
95kB
Ketho
06-17-14 01:54 AM


Post A Reply Comment Options
Old 06-21-14, 10:57 AM  
Phanx
A Pyroguard Emberseer
 
Phanx's Avatar
AddOn Author - Click to view AddOns

Forum posts: 3964
File comments: 1913
Uploads: 38
Originally Posted by Ketho
areyouawizard.jpg
I put on my robe and wizard hat...

Actually you can avoid all those awkward-looking table lookups and just add this:

Code:
      if delay > 0 then
         timers[func] = delay
         running = running + 1
      else
No need to do all those table lookups, or to set the value to <= 0 when you're just going to delete it immediately. :P

Also, I deliberately avoided using next since calling functions is the Slowest Thing Ever. Unless you're going to be running thousands of timers concurrently, it's faster to keep your own count (the running variable) while you're already looping over the table. Actually, looking at that again you don't even need a count; a boolean is fine...

Code:
   local stop = true
   for func, delay in pairs(timers) do
      delay = delay - elapsed
      if delay > 0 then
         timers[func] = delay
         stop = nil
      else
         timers[func] = nil
         func()
      end
   end
   if stop then
      self:Hide()
   end
Also, I filled in the translations for Spanish... both of them... not really sure why Blizz even bothered to make esMX a separate locale, when 99.9999999% of it is identical to esES, and the main difference is which word they used for "dungeon".
__________________
Author/maintainer of Grid, PhanxChat, ShieldsUp, 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!
Last edited by Phanx : 06-21-14 at 10:58 AM.
Phanx is offline Report comment to moderator  
Reply With Quote
Old 06-21-14, 09:26 AM  
Ketho
A Molten Giant
 
Ketho's Avatar
AddOn Author - Click to view AddOns

Forum posts: 559
File comments: 110
Uploads: 21
areyouawizard.jpg
No, seriously.

There was a little error, but it worked nicely afterwards.
Code:
for func, delay in pairs(timers) do
	delay = delay - elapsed

I changed it a bit, I'm happy with this
Code:
local timers = {}
local Timer = CreateFrame("Frame")
Timer:Hide()

function Timer:New(func, delay)
	timers[func] = delay -- add timer
	self:Show()
end

Timer:SetScript("OnUpdate", function(self, elapsed)
	for f in pairs(timers) do
		timers[f] = timers[f] - elapsed
		if timers[f] < 0 then
			timers[f] = nil -- remove timer
			f()
		end
	end
	if not next(timers) then -- all timers finished
		self:Hide()
	end
end)
Ketho is offline Report comment to moderator  
Reply With Quote
Old 06-20-14, 07:03 PM  
Phanx
A Pyroguard Emberseer
 
Phanx's Avatar
AddOn Author - Click to view AddOns

Forum posts: 3964
File comments: 1913
Uploads: 38
In your GetTimer function, you're creating a new frame for every additional concurrent timer... while you can only have one OnUpdate script per frame, you don't need a dedicated OnUpdate for every timer. Just keep a list of running timers and loop over them in your OnUpdate, call any that are up, and hide the frame once they're all done:

Code:
local Timer = CreateFrame("Frame")
local timers = {}

function Timer:New(func, delay)
   timers[func] = delay
   self:Show()
end

Timer:Hide()
Timer:SetScript("OnUpdate", function(self, elapsed)
   local running = 0
   for func, delay in pairs(timers) do
      delay = delay - elapsed
      if delay > 0 then
         running = running + 1
      else
         timers[func] = nil
         func()
      end
   end
   if running == 0 then
      self:Hide()
   end
end)
On another note, while the examples in the Lua-users wiki, and in the official Lua documentation, are quite helfpul in terms of showing you how the language works, it's been my experience that more often than not, they're actually pretty bad examples of how to write efficient WoW addon code. While creating a table, stuffing it with the contents of a vararg, and then unpacking it to get the values may be okay in a standalone script that runs once and quits, when you start doing it frequently in a program that can potentially keep running indefinitely, you end up wasting a lot of memory and CPU cycles. Better to use the method that's more efficient and makes for more readable code.
__________________
Author/maintainer of Grid, PhanxChat, ShieldsUp, 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!
Last edited by Phanx : 06-20-14 at 07:14 PM.
Phanx is offline Report comment to moderator  
Reply With Quote
Old 06-20-14, 05:24 PM  
Ketho
A Molten Giant
 
Ketho's Avatar
AddOn Author - Click to view AddOns

Forum posts: 559
File comments: 110
Uploads: 21
Sorry for the late reply, Phanx.
I guess I'm not doing this right
  • I'm going to just use those individual variables (from line 307) then. It sure is a lot easier.
    No idea why I wanted to pack and unpack the vararg in the first place :S it just looked so neat~
    Actually ... I shamelessly stole the code from http://lua-users.org/wiki/VarargTheSecondClassCitizen under "a variant noted by Shirik".

    • I didn't use the # length operator, because there might be holes in the table (e.g. destName). But in this case there won't be holes in the table I think, it's just that I sloppily ripped the code from my other addon and that snippet ><

  • I will continue passing anonymous functions to the timer because I personally think it looks more readable vs passing the CLEU args to a separate function. It also feels very good to be lazy! (sorry for that pretext)

  • My timer implementation could idd be better. The OnUpdate script part should be better now:
    Code:
    local timers = {}
    
    local function GetTimer() -- allocate timers
    	local i = 1
    	while timers[i] and timers[i].running do
    		i = i + 1
    	end
    	-- if a timer isnt running return that, otherwise return a new one
    	timers[i] = timers[i] or CreateFrame("Frame")
    	return timers[i]
    end
    
    local function UpdateTimer(s, e)
    	s.sum = s.sum + e
    	if s.sum > s.delay then
    		s:SetScript("OnUpdate", nil)
    		s.running = false
    		s.func()
    	end
    end
    
    function S.ScheduleTimer(func, delay)
    	local t = GetTimer()
    	t.running = true
    	t.sum = 0
    	t.delay = delay
    	t.func = func
    	t:SetScript("OnUpdate", UpdateTimer)
    end

  • Point taken, I moved the destType and destPlayer where they rightfully belong now
    Code:
    function Juke:COMBAT_LOG_EVENT_UNFILTERED(event, ...)
    	local timestamp, subevent, hideCaster, sourceGUID, sourceName, sourceFlags, sourceRaidFlags, destGUID, destName, destFlags, destRaidFlags, spellID, spellName, spellSchool, SuffixParam1, SuffixParam2, SuffixParam3 = ...
    	
    	-- when the unit doesnt exist, guid is an empty string and name is nil
    	if #sourceGUID == 0 then return end
    	
    	local sourcePlayer = (bit_band(strsub(sourceGUID, 5, 5), 0x7) == 0)
    	if not sourcePlayer then return end -- only show jukes/interrupts done by players

Didn't update it yet, my bedtime now ..
__________________
Last edited by Ketho : 06-20-14 at 06:24 PM.
Ketho is offline Report comment to moderator  
Reply With Quote
Old 06-20-14, 04:21 AM  
Phanx
A Pyroguard Emberseer
 
Phanx's Avatar
AddOn Author - Click to view AddOns

Forum posts: 3964
File comments: 1913
Uploads: 38
Code:
			-- trying to save a vararg; am I doing this right? :x
			local varg, n = {...}, select("#", ...)
Noooooooo!

This creates a new table on every handled combat log event, which is basically a memory leak. You've already assigned all the values from the vararg to individual variables (on line 307) so just use those variables, or reuse a predefined table like you're doing with your args table. Your timer implementation is also very inefficient, as you're creating two new functions every time you start a timer (the one you pass to your Timer function, and the one you set as an OnUpdate script). Also, since you're returning out if sourcePlayer is false, you can save a bit of CPU time by refraining from defining the destType and destPlayer variables (with all their attendant function calls) until after that point.

Oh, and also if you kept your current implementation, you don't need to use select to get the length of the vararg; you can just use the much faster # operator on the table you created.
__________________
Author/maintainer of Grid, PhanxChat, ShieldsUp, 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!
Last edited by Phanx : 06-20-14 at 04:23 AM.
Phanx is offline Report comment to moderator  
Reply With Quote
Post A Reply



Category Jump: