12-11-14, 02:07 AM | #1 |
oUF_Layout optimization
Hey all,
I've finally gotten around to writing my own personal oUF. I am nearly complete; completed modules work as desired. Party, pet, boss, and raid still to go, but wanted some feedback on better ways to optimize my layout. I've written this in a rather verbose manner, a lot of similar looking code for the sake of lots of customization on a unit to unit basis. The pastebin can be found here: http://pastebin.com/bB3MFkeM Thanks! |
|
12-11-14, 10:07 AM | #2 |
Just took a glance at your code. I noticed that you can easily optimize by reusing similar code.
For instance, create health and power bar code just once and insert it for each unit. You could do this with using a "Shared" function, that contains code used for every unit and/or by creating single functions. Somewhat like this: Code:
local createPower = function(self) -- Power bar ....... end Code:
UnitSpecific.player = function(self, ...) Shared(self, ...) -- only if you use a shared function, obviously createPower(self) end €: Just to add to the confusion, let's say you create a "createPower" function; you don't want to put the SetPoint stuff in there. Unless every unit you use is exactly the same, ofc. Only put the CreateFrame, background, PostUpdate, color and stuff like that in there. Set your Points for each unit, after you inserted the createPower(self) function. like Code:
UnitSpecific.player = function(self, ...) createPower(self) self.Power:SetPoint("BOTTOMLEFT", self, "BOTTOMLEFT", 38, 20) createRaidIcon(self) self.RaidIcon:SetPoint("TOPLEFT", self, "TOPLEFT", 4, 0) end UnitSpecific.target = function(self, ...) createPower(self) self.Power:SetPoint("BOTTOMRIGHT", self, "BOTTOMRIGHT", -38, 20) createRaidIcon(self) self.RaidIcon:SetPoint("TOPRIGHT", self, "TOPRIGHT", -4, 0) end
__________________
Rock: "We're sub-standard DPS. Nerf Paper, Scissors are fine." Paper: "OMG, WTF, Scissors!" Scissors: "Rock is OP and Paper are QQers. We need PvP buffs." "neeh the game wont be remembered as the game who made blizz the most money, it will be remembered as the game who had the most QQ'ers that just couldnt quit the game for some reason..." Last edited by Dawn : 12-11-14 at 10:14 AM. |
|
12-11-14, 11:56 AM | #3 |
Personally I've never seen the point in splitting each part of the spawn function out into a "make a health bar" function, a "make a power bar" function, a "do stuff for the player unit" and "do stuff for the target unit" functions, etc. You're only running that code once per frame created, so just have it all in the same place.
Code:
local health = CreateFrame("StatusBar", nil, self) self.Health = health if unit == "player" then -- do stuff with the health bar for the player frame elseif unit == "target" then -- do stuff with the health bar for the target frame end ---------- This is also pretty inefficient: Code:
BuffBars.customFilter = function(_, _, _, _, _, _, _, _, _, _, _, _, spellID, _, _, _, _, _) if spellID == 97341 or --Guild Champion spellID == 93339 or --Champion spellID == 93828 or --Silvermoon Champion spellID == 93341 or --Hyjal Champion spellID == 93347 or --Therazane Champion spellID == 97340 or --Guild Champion spellID == 93811 or --Exodar Champion spellID == 93816 or --Gilnean Champion spellID == 72968 or --Precious's Ribbon spellID == 93806 or --Darnassus Champion spellID == 93368 or --Wildhammer Champion spellID == 93337 or --Ramkahen Champion spellID == 126434 or --Tushui Champion spellID == 57819 or --Argent Chamption spellID == 57822 or --Wyrmrest Champion spellID == 93821 or --Gnomergan Champion spellID == 142234 or --Brawling Champion (toot toot) spellID == 142239 or --Brawling Champion (gorgeous) spellID == 142242 or --Brawling Champion (south seas) spellID == 142243 or --Brawling Champion (splat) spellID == 142244 or --Brawling Champion (bruce) spellID == 142246 or --Brawling Champion (rockpaperscissors) spellID == 142247 or --Brawling Champion (blingtrong) spellID == 143625 or --Brawling Champion (rank 10) spellID == 93805 or --Ironforge Champion spellID == 25780 or --Righteous Fury spellID == 17619 or --Alchemist Stone spellID == 93795 --Stormwind Champion then return false else return true end end Code:
local blacklist = { [93795] = true, -- Stormwind Champion -- more spells here } Code:
return not blacklist[spellID] Code:
BuffBars.customFilter = function(_, _, _, _, _, _, _, _, _, _, _, _, spellID, _, _, _, _, _) Code:
BuffBars.customFilter = function(_, _, _, _, _, _, _, _, _, _, _, _, spellID) Your focus frame is ... I don't even. Why are you creating a bunch of extra frames and listening for events yourself, instead of just using oUF elements and/or tags? ---------- Other than that, your #1 priority should be removing all that duplication.
__________________
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. Last edited by Phanx : 12-11-14 at 12:01 PM. |
|
12-11-14, 12:21 PM | #4 |
Thank you both for the replies. My initial thought was to have all that repeat code because as far as I know, and correct me if I'm wrong, no less CPU/men efficient than making a shared function that goes through all the frames and does the same code anyway. It's certainly more unwieldy to look at that, and for that it will get changed. I just wanted to easily be able to go to a certain line of code for a certain frame and make tweaks before I became satisfied.
In regards to the aura if checks, I'll change that as well as the unnecessary _ variables. As for the reason I didn't make my own tags in ouf. That code has to be put somewhere in the ouf folder, be it a tags file in my layout folder, or editing the ouf tags. while it would look cleaner if I just had tags in my layout, rather than functions and frame creations, it would ultimately be messy somewhere. Tl;Dr is what I've done, minus the buff checking, any less CPU/men efficient, or just unsightly? Last edited by sirann : 12-11-14 at 01:52 PM. |
|
12-11-14, 04:31 PM | #5 | |
The biggest problem with duplication is that it makes things harder to debug and maintain in the long run. If you decide to change anything about your health bar, for example, you have to make the same change 4 times in 4 different places, which is boring, a waste of time, and easy to screw up. If you only have to make the change in one place, it's faster and less error-prone.
Also just noticed this: Code:
self.HealthBG = HealthBG
Code:
oUF.Tags.Events.healthtext = 'UNIT_HEALTH' oUF.Tags.Methods.healthtext = function(unit) if UnitIsGhost(unit) then return '|cffb40000Ghost' elseif UnitIsDead(unit) then return '|cffb40000Dead' end local r, g = 0, 180 local v, vmax = UnitHealth(unit), UnitHealthMax(unit) if v < vmax then r, g = g, r end if v > 1000000 then return format('|cff%02x%02x00%.1fm', r, g, v / 1000000) elseif v > 1000 then return format('|cff%02x%02x00%.0fk', r, g, v / 1000) else return format('|cff%02x%02x00%d', r, g, v) end end oUF.Tags.Events.healthpercent = 'UNIT_HEALTH' oUF.Tags.Methods.healthpercent = function(unit) if UnitIsDead(unit) then return end local r, g = 0, 180 local v, vmax = UnitHealth(unit), UnitHealthMax(unit) if v < vmax then r, g = g, r end return format('|cff%02x%02x00%.0f%%', r, g, 100 * (v / vmax)) end Code:
local HealthText = Health:CreateFontString(nil, 'OVERLAY') HealthText:SetPoint('RIGHT', 5, -10) HealthText:SetFont(FONT, 10, 'OUTLINE') HealthText:SetJustifyH('LEFT') Health.Text = HealthText self:Tag(HealthText, '[healthvalue]') local HealthPercent = playerhealthtextframe:CreateFontString(nil, 'OVERLAY') HealthPercent:SetPoint('LEFT', -5, -10) HealthPercent:SetFont(FONT, 10, 'OUTLINE') HealthPercent:SetJustifyH('RIGHT') Health.Percent = HealthPercent self:Tag(HealthPercent, '[healthpercent]')
__________________
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. |
||
12-11-14, 06:50 PM | #6 |
As an update I did this: http://pastebin.com/FcVTCRA4
Obviously it's only the player, I have to raid or I'd finish the other units. Relevant tags I made can be found here: http://pastebin.com/KGfZFXEA I appreciate both of your initial, and hopefully ongoing, feedback. |
|
12-11-14, 10:39 PM | #7 |
Tags:
Code:
return format('|cff%02x%02x%02x%s', r, g, 0, healthcurrent) Code:
return format('|cff%02x%02x00%s', r, g, healthcurrent) Code:
local color if healthcurrent < healthmax then color = '|cffb40000' else color = '|cff00b400' end if healthcurrent >= 1000000 then healthcurrent = format('%.1fm', healthcurrent/1000000) elseif healthcurrent >= 1000 then healthcurrent = format('%.0fk', healthcurrent/1000) end return color .. healthcurrent Code:
powertype = select(2,UnitPowerType(u)) Code:
local powercurrent, powertype, pr, pg, pb, _ _, powertype = UnitPowerType(u) Code:
powertype, powertype = UnitPowerType(u) Code:
elseif powertype == 'FOCUS' then pr, pg, pb = 255, 128, 65 elseif powertype == 'ENERGY' then pr, pg, pb = 255, 255, 0 elseif powertype == 'RUNIC_POWER' then pr, pg, pb = 0, 209, 255 else pr, pg, pb = 255, 0, 0 end Code:
else local c = PowerBarColor[powertype] pr, pg, pb = c.r * 255, c*g * 255, c*b * 255 end
__________________
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. |
|
12-11-14, 10:53 PM | #8 |
Other:
De-duplication looking much better overall. Code:
self.Health:SetHeight(29) The best way to avoid this would be to use relative positioning. Instead of anchoring the health bar by the TOPLEFT and TOPRIGHT corners and giving it an explicit height, add a third anchor to the BOTTOM point with a vertical offset to leave space for the power bar -- and then anchor the TOP of the power bar to the BOTTOM of the health bar. That way the only value you have to change is the frame height, and the health bar will automatically get bigger or smaller accordingly. ======================== Several problems in here: Code:
local leaderassistframe = CreateFrame('Button', 'PlayerLeaderAssistFrame', self) leaderassistframe:SetSize(3,8) leaderassistframe:SetPoint('TOPLEFT', self, 'TOPLEFT', 2, 0) leaderassistframe:RegisterEvent('GROUP_ROSTER_UPDATE') leaderassistframe:RegisterEvent('PARTY_LEADER_CHANGED') leaderassistframe:RegisterEvent('PLAYER_ENTERING_WORLD') local leaderassisttexture = leaderassistframe:CreateTexture('PlayerLeaderAssistTexture', 'OVERLAY') --leaderassisttexture:SetTexture('Interface\AddOns\oUF_Terenna\Status_Texture', 'OVERLAY') leaderassisttexture = leaderassistframe:CreateTexture('Interface\AddOns\oUF_P3lim\F1_StatusBox_Bar', 'OVERLAY') leaderassisttexture:SetTexture(1, 1, 0) leaderassisttexture:SetAllPoints(true) leaderassistframe:SetScript('OnEvent', function(self, event, unit) if UnitIsGroupLeader('player') or UnitIsRaidOfficer('player') then leaderassisttexture:Show() else leaderassisttexture:Hide() end end) 2. These objects don't need to exist at all. Get rid of them and use oUF's native Assistant and Leader elements; you can set them both to use the same texture if you really don't want to know the difference. 3. Those file paths are invalid; you need to either escape your backslashes by doubling them, or use [[ literal string notation ]] instead of "double quotes" around the path. 4. This appears to be a bad copypasta: Code:
leaderassisttexture = leaderassistframe:CreateTexture('Interface\AddOns\oUF_P3lim\F1_StatusBox_Bar', 'OVERLAY') b. This usage of CreateTexture will actually produce a new texture object whose global name is that (malformed) file path. You should delete this line and uncomment the SetTexture line directly above it. ======================== oUF also already provides a combat indicator element and a resting indicator element; you don't need to create your own frames and manage them yourself with event handlers. Just use the elements and give them the textures, dimensions, etc. that you want. ======================== However, oUF does not provide any elements named "BuffBars" or "DebuffBars" so if you want those objects to do anything, you either need to use the proper names -- "Buffs" and "Debuffs" -- or install/write a third-party element/plugin to handle them. ======================== Code:
oUF:RegisterStyle('Terenna', Shared) oUF:RegisterStyle('oUF_Terenna_Player', UnitSpecific.player) oUF:SetActiveStyle('oUF_Terenna_Player')
__________________
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. |
|
12-12-14, 12:53 PM | #9 |
1.
Code:
self.HealthBG = HealthBG ----------------------------------------------------------------------------- 2. Code:
self.Health:SetHeight(29) ----------------------------------------------------------------------------- 3. Several problems in here: A. I'm assuming global name is PlayerLeaderAssistFrame and PlayerLeaderAssistTexture, will remove B. Will use oUF elements and change texture path C. The texture paths do work. If they're not supposed to, IDK why, but I can assure you they do. D. That is bad copypasta. But it did work, so I didn't catch that. ----------------------------------------------------------------------------- 4. oUF also already provides a combat indicator element and a resting indicator element Noted. Will use those and change textures again if needed like assist frame. ----------------------------------------------------------------------------- 5. However, oUF does not provide any elements named "BuffBars" or "DebuffBars" This is from an element you rewrote for me, oUF_AuraBars. I still use it, although I think I'm literally the only person to do so. I just couldn't deal with elkbuffbars another god damn day. ----------------------------------------------------------------------------- 6. Code:
oUF:RegisterStyle('Terenna', Shared) oUF:RegisterStyle('oUF_Terenna_Player', UnitSpecific.player) oUF:SetActiveStyle('oUF_Terenna_Player') oUF:RegisterStyle('oUF_Terenna_Player', UnitSpecific.player) as well as other units with the UnitSpecific.insertunitnamehere ? And then just have oUF:SetActiveStyle('oUF_Terenna') at the bottom of my code outside of all functions? Last edited by sirann : 12-12-14 at 01:10 PM. |
|
12-13-14, 02:57 AM | #10 | |
Code:
local func = UnitSpecific[unit] if func then func() end Yes.
__________________
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. |
||
WoWInterface » Featured Projects » oUF (Otravi Unit Frames) » oUF_Layout optimization |
«
Previous Thread
|
Next Thread
»
|
Display Modes |
Linear Mode |
Switch to Hybrid Mode |
Switch to Threaded Mode |
|
|