04-12-06, 10:28 AM | #1 |
Code optimization for novices
I've been working on an addon (which atm is only available to my guildies) during the past weeks - and the more it grows, the more I feel the need for optimizing my code.
I never really learned programming, besides writing extensive JavaScript functions for DHTML a few years ago. So I'm not fluent with object orientation, and my lua code is based on the code I see in so many other addons I analyze. On the other side if I try to grasp the programming guide at lua.org/pil I mostly fail. Partly because I'd need real world exampled, partly because it seems as if it's written for professional programmers. So if anyone could help me solve some probably pretty basic problems, I'd appreciate it. 1) the 'local function' problem. I try not to pollute the namespace with global functions and variables - but as soon as my addon has more than one .lua file (and I prefer to have 5-6 smaller files than one with 1500 lines of code) I don't know how to solve that problem. So how do I define functions that should only be available to my addon but be available in all my .lua files? 2) the 'local variable' problem. Somewhere I've seen the concept to store all variables in an array, and to set a short alias name to it to have smaller code. E.g.: Code:
myAddonData = {} local m = myAddonData m.whatever = "one" m.somethingelse = true ... 3) problem with loops pt1: How do I skip to the next data in my loop? I'd like to replace something like: Code:
for k in whatever do if k["one"] then if k["one"]["really"] == true then doSomething(k) end end end Code:
for k in whatever do if not k["one"] or not k["one"]["really"] then GOTONEXT end doSomething(k) end 4) problem with loops pt2: The following just doesn't feel right: Code:
function getBestRank(spell,damage) local rank = 1 for i = 1, ranks[spell] do if healamount[spell][rank] < damage then rank = rank + 1 else break end end rank = rank - 1 if rank < 1 then rank = 1 return rank end 5) problem with loops pt3: Do I really need to declare a local here and then use it? Isn't there a more elegant way? Code:
local really = true for k in something do if not areYouSure(k) then really = false end end if really then ... else ... end I'm using an array that stores various info about each raid member which I update on PARTY_MEMBERS_CHANGED: Code:
roster = { } for i = 1,GetNumRaidMembers() do local name = UnitName("raid"..i) roster[name] = {} roster[name]["raidid"] = 'raid'..i _,roster[name]["class"] = UnitClass('raid'..i) _,_,roster[name]["group"] = GetRaidRosterInfo(i) end Code:
health = { } for name in roster do local unit = roster[name].id tinsert(health, { name, UnitHealth(unit)/UnitHealthMax(unit) }) end table.sort( health, function(a,b) return a[2] < b[2] end ) needy = { } local i = 1 for k,v in health do needy[i] = {} needy[i].name = v[1] needy[i].hp = v[2] end I know. Lots of questions. Some may sound very noobish to you. But if you could help optimizing the one or other code snippet, I'd really appreciate it. I'm trying to learn something new from the lua.org guide day for day, but I havent found an answer to my question yet - other than "yeah, use metatables and __.index" - "uum?". Thanks in advance. PS: I just found out about the existance of pastbin.com, so I uploaded the sample code snippets there. So if you have a solution, you can update the code online: http://maia.pastebin.com/656331 Last edited by Maia : 04-12-06 at 02:08 PM. |
|
04-13-06, 04:44 AM | #2 |
Hello
I am quite the LUA beginner as well, but I've learned alot lately so I'll try to help if I can....but no guarantees here 1) the 'local function' problem. I really don't know on this one, having never tried using >1 .lua file, I was just under the impression that all the files listed in your .toc would be considered as a whole. For instance, I know I can call functions in my .xml that are defined in my .lua, however I haven't tried .lua -> .lua. I'm eager to hear what the solution to this is myself. 2) the 'local variable' problem. See above. 3) problem with loops pt1: I made all possible-but-untested-and-probably-grossly-wrong changes here: http://maia.pastebin.com/657356 4) problem with loops pt2: See pastebin. 5) problem with loops pt3: See pastebin. 6) sorting non-indexed (?) tables/arrays I have no freaking clue I'm almost certain that what I gave is certaintly not the best method to do things, but I figured I'd at least give my input and hopefully at least 1 part of it will be useful for you. Good luck! |
|
04-13-06, 05:11 AM | #3 |
Okay let me give a few pointers yust on point one.
1 variables in lua are nothing but references... a function is nothing more then a reference to a function with a name. a table can contain any datatype..... Therefor you can store Functions in a table. Like this. Myaddon = {} Function Myaddon:Myfunction(text) return text end If all your function are placed in the table you keep the global namespace clean. The addon object does not have to be local so it is available to new .lua files. You can ofcourse store other data types in here perfectly fine too. Myaddon.shouldidosomething = true
__________________
|
|
04-13-06, 05:56 AM | #4 |
I have to confess I've been away from WoW for a long time but such eagerness to learn is a rarity so had to reply. My answers may be off a little though with the absence (Lua is a weird creature).
1) the 'local function' problem. MoonWolf answered this well. 2) the 'local variable' problem. I did something like this. I think your problem may be that the 'local m =' assignments are happening before myAddonData is set in some files. Make sure you create myAddonData in the first file listed in your toc and hopefully the 'local m = myAddonData' will work in all files. 3) problem with loops pt1: Though the reply above is quite creative, the 'continue' keyword is the direct answer. 4) problem with loops pt2: Is this what you're after? Code:
function getBestRank(spell,damage) local i; // could arrange the data better here but that's really being anal for rank = 1, ranks[spell] do if healamount[spell][rank] > damage then break end if i > rank then rank-- return rank end 5) problem with loops pt3: Code:
local really = true for k in something do if not areYouSure(k) then really = false end end if really then ... else ... end 6) sorting non-indexed (?) tables/arrays Personally I'd advise against your table of everyone in the raid - the WoW client already has this information and duplicating it in your own table is really a waste of space. If you need this table (and plenty of reasons for that) I suggest ditching the name/class from it - unless you need to look up by name/class really often - as in several times per second. Here's some pretty simple (and untested) emergency monitor update code - though you can modify it to use your table which really should be indexed by raid id. Note that you shouldn't run this code every frame (as in every update()) - people don't react that fast anyway and it'll contribute to choppiness. I think I see a bug in your code by the way - in theory it'll only work if there aren't 'spaces' in the raid, for example you move one person to the last slot in last group but noone in the previous groups. Though if GetRaidNumMembers() returns the max 'slot' used that's untrue. I haven't worked much with raid code though so don't take my word for this. But if I'm right, just scan for 40 as the max number. That and this code snippet can easily be improved for smaller raids though. Code:
function updateEmergencyMonitor() local temp = {} for i = 1, 40 do temp[i].id = i; if UnitHealth('raid') then temp[i].pct = UnitHealth('raid' .. i) / UnitHealthMax('raid' .. i) else temp[i].pct = 1; end // Make sure they don't get sorted low if not there end table.sort(temp, function(a,b) return a.pct < b.pct; end) // Now temp[1] will be the lowest % member, etc end Last edited by svip : 04-13-06 at 05:58 AM. |
|
04-13-06, 12:03 PM | #5 |
Elviso, MoonWolf, Svip, thanks a lot (plus Mark and Tyrone who submitted suggestions directly in the pastebin).
As for the local variable problem: Svip, I did declare myAddon={} in the first lua file listed in the .xml and then declare m=myAddon in all lua files (and in the main file obviously after the myAddon={}) - it doesn't work though. Guess using such a short alias won't work as I hoped. As for "problems with loops, pt1": My goal is to have code that is easy to read. I prefer short code, short lines, and not a ton of nested if statements. The hint regarding 'continue' seems interesting, but I can't find any reference to it on lua.org - sure it does exist in lua and not only C? As I understand it, a continue will jump to the next iteration of a loop, right? (this is what I'd be looking for) problem with loops, pt2 (the underhealing function): Svip, I don't understand your code: you're declaring "local i;" and then comparing rank to i - but i never is a number, is it? But Elviso's suggestion seems to be an easy way: Code:
function getBestRank(spell,damage) local rank = 1 for i = 1, ranks[spell] do if not healamount[spell][i] < damage then break end rank = i -- use i to assign the rank after the break to keep from over-ranking end return rank end sorting tables: Hmm. I've been indexing by id and not name for a while, until Cladhaire mentioned on the wowace forums that it's better to index by name (one reason was that it's then easily possible to use the same functions for party members, pets,... but I think another reason was that a raidid can change at any time, whereas the name always stays the same) - which is why I changed it. And I guess I do need the table, as I do need the class info several times I second (I also need info if someone has aggro or not, see above, or if someone is being healed by others). I guess I'll have to explain my goal: I'm trying to write a small emergency monitor which won't only sort by health, but add various modifiers. So e.g. a tank who has aggro, is not being healed by others, who is in your subgroup and has 60%hp has priority over a rogue in a different subgroup, being healed by another healer and has 50%hp. Which is why I'd want to gather all that info every 0.2sec and then display a list of the 5 people who you should be healing most. (oh, and yes, I could live without that addon - but at the moment I'm having more fun writing it than spending even more time raiding or in a battleground, and learning programming is pretty nice as well). So I could use the following code: Code:
function updateEmergencyMonitor() local temp = {} for i = 1,40 do temp[i].id = i local hp = getHealthPct(i) --0-100 local aggro = getAggro(i) --returns -20 if aggro, otherwise 0 local healedbyothers = getHealData(i) --returns +10 if healed by others, else 0 local classmodifier = getClassModifier(roster[i].class) --returns -20 for a warrior, +10 for a rogue,... temp[i].priority = hp + aggro + healedbyothers + classmodifier end table.sort(temp, function(a,b) return a.priority < b.priority end) end Thanks again. |
|
04-13-06, 02:00 PM | #6 | ||||
Lua is, shall we say, heavily influenced by C but not in this matter I guess. But preferring short code is something to be proud of - stick with that. So is there any way you can rewrite this? For example putting the for loop in another function, or the fuctionality if the if check is true.
Code:
function getBestRank(spell,damage) for rank = 1, ranks[spell] do if healamount[spell][rank] > damage then break end if rank > 1 then rank-- return rank end
A minor change to make it simpler if you stick with this would be Code:
roster[name]["aggro"] = aggro
Now, I think the array is in an arbitrary order when there's no index in it. If I'm wrong about that there should be some clever way to do it with the original array but I'm at a loss here. So instead I'll have a look at making your code simpler. First of all, table.sort. table.sort only works to ordered an indexed table, as you've found, so we will need the second array. Code:
local emerg = {} for r in pairs(roster) do // For everyone in the roster // Put the priority code in another function. Pretty! // Also stores it directly in the roster. If it's going // to be used so often we may as well.. r.priority = getHealPriority(r); table.insert(emerg, r) // Store a reference to the roster entry end // Sort it table.sort(emerg, function(a, b) return a.priority < b.priority); // Theoretical code from a warped mind - but this should discard // the superfluous tablebits while emerg[5] do table.remove(emerg, 5) end // I assume you stored the id in the roster so the following should // get the name of the first person on the list local firstPerson = UnitName('raid' .. emerg[1].id) // And of course - // roster[UnitName('raid' .. emerg[1].id)] == emerg[1] |
|||||
04-13-06, 05:34 PM | #7 | ||
Code:
function hasAggro(unit) if UnitExists(unit.."target") and UnitCanAttack("player",unit.."target") and UnitExists(unit.."targettarget") and UnitInRaid(unit.."targettarget") and UnitAffectingCombat(unit.."targettarget") then return true else return false end end for name in roster do if hasAggro("raid"..roster[name].id) then roster[name].aggro = true end Code:
for name in roster do local unit = "raid"..roster[name].id if not UnitExists(unit) then continue end if not UnitCanAttack("player",unit.."target") then continue end if not UnitExists(unit.."targettarget") then continue end if not UnitInRaid(unit.."targettarget") then continue end if not UnitAffectingCombat(unit.."targettarget") then continue end roster[UnitName(unit.."targettarget")].aggro = true end
So the following should work? Code:
local emerg = {} for name in pairs(roster) do name.priority = getHealPriority(name) -- so this is the same as roster[name].priority = getHealPriority(name) ? table.insert(emerg, name) end table.sort(emerg, function(a, b) return a.priority < b.priority) while emerg[5] do table.remove(emerg, 5) end -- now you're saying: -- roster[UnitName('raid' .. emerg[1].id)] == emerg[1] -- so emerg[1] is the same object as roster[emerg[1]]? -- if yes, then: for i in emerg do frame[i].name:SetText(emerg[i]) -- UnitName frame[i].name:SetColor(getRaidColor(emerg[i].class)) -- color depending on class frame[i].SetValue(emerg[1].hppcct) -- set bar, min/max values 0,100 end Next step: find out how to dynamically create frames. I'd like to have an addon without xml file Last edited by Maia : 04-13-06 at 05:48 PM. |
|||
04-13-06, 06:09 PM | #8 | |||
Code:
for name in roster do local unit = "raid"..roster[name].id if UnitExists(unit) and UnitCanAttack("player",unit.."target") and UnitExists(unit.."targettarget") and UnitInRaid(unit.."targettarget") and UnitAffectingCombat(unit.."targettarget") then roster[UnitName(unit.."targettarget")].aggro = true end end
For example when I picked out an array element to do lots of work with, I'd do somehthing like: Code:
local r = roster[UnitName('targettarget')]; if r.class = 'Rogue' then do stuff with r;
|
||||
04-13-06, 09:33 PM | #9 |
I think I saw it posted above in some form, but here's the standard function I use when I need to iterate a table that's sorted by it's keys. You can use it in place of your normal pairs() function if you need it. Note that it does create another table to sort. So it's probably not something you'd want to run every .1 seconds on a large table.
Code:
function PairsByKeys(t, f) -- t = table -- f = optional sort function local a = {} for n in pairs(t) do table.insert(a, n) end table.sort(a, f) local i = 0 -- iterator variable local iter = function () -- iterator function i = i + 1 if a[i] == nil then return nil else return a[i], t[a[i]] end end return iter end |
|
04-18-06, 06:37 PM | #10 |
Thanks everyone for your help. I really appreciate it!
|
|
04-18-06, 07:34 PM | #11 |
Local is handled at load time, the second it's executed it takes effect and is availble to the file/function/if/for/etc scope.
file1.lua -- Create a local table local m = {} -- A variable m.debug = true -- A function m.foo = function() if m.debug then print debug end end -- Make a global referance MyAddon = m |
|
WoWInterface » Developer Discussions » Lua/XML Help » Code optimization for novices |
«
Previous Thread
|
Next Thread
»
|
Display Modes |
Linear Mode |
Switch to Hybrid Mode |
Switch to Threaded Mode |
|
|