05-04-14, 09:47 AM | #1 |
Walking before I Run....
Ok so instead of trying to figure out a "complete" UI I think I'm going to step back and try to just handle one addon.
So (for now) BasicUI will be put on the back shelf. I will upload the current version I have and let people us it if the like. The single addon I will be trying to keep up is nData (WoWInterface and GitHub). Can someone take a look at the zip file below to see if I have everything working correctly. Thanks for everyone's help. Coke |
|
05-04-14, 07:12 PM | #2 |
First and foremost: LibDataBroker. Why are you embedding it when your addon does not use it? Though, really, your coding life would be much simpler if you did use it, and simpler still if you just used one of the many many existing Broker bars out there, that not only have as few (CargoShip, NinjaPanel) or as many (Bazooka, DockingStation) features as you want (StatBlocks and ChocolateBar fall closer to the middle of the feature spectrum, I think) but also support any number of plugins instead of a fixed 9, support an infinitely larger variety of existing plugins, and have compatibility already built into thousands of other addons.
There are plenty of other areas of the UI you could be spending your time on that aren't 100% wheel reinvention, and most of those other areas would be more immediately rewarding, in that you would be spending more time customizing appearances and behaviors with results you could enjoy right away, instead of spending a lot of time on a largely invisible backend framework before actually doing anything interesting. However, if you really want to keep on reinventing the wheel with a data panel: There's no need to nil out OnInitialize, as AceAddon will only ever call it once. Don't forget to re-set your "db" upvalue in your Refresh function if you plan to support profiles: Code:
db = self.db.profile Code:
self:SetEnabledState(db.enable) You're embedding LibSharedMedia-3.0 and the very large AceGUI-3.0-SharedMediaWidgets but not using them. If you don't plan to use them, get rid of these libraries. On a related note, don't put unrelated libraries into the same folder. The LibStub folder should ONLY contain LibStub. Put LibBasicAdjust-1.0 and tekShiner in their own folders, or just in the top-level Libs folder. And, don't include additional copies of LibStub and CallbackHandler in other libraries, as you're doing with LibDataBroker, and do embed the deepest path you can, so for LibDataBroker, you should be embedding the inner LibDataBroker-1.1 folder that only includes the library's Lua file and a README, not the outer folder that also includes a TOC and other clutter. You should either (a) do the panel creation -- self:CreatePanel() -- from OnInitialize insead of OnEnable, or (b) remove the "enable" option, since CreatePanel is only designed to be called once, but if you let users toggle the addon in-game (which, honestly, seems pointless in a self-contained addon they can just disable at the addon screen, or uninstall, if they want to stop using) then OnEnable can be called over and over. Also, if you keep the "enable" option, then you should add an OnDisable method that hides all the panels and does anything else you need to do to "turn off" the addon. AceAddon/AceEvent will automatically unregister any events registered on your addon object, but you should manually unregister events registered on other frames (eg. you can't use RegisterUnitEvent on an AceAddon object because AceEvent doesn't support it, and your individual data plugins are not AceAddon objects anyway). Unconditionally showing your frame on PLAYER_ENTERING_WORLD when you intend for it to be hidden in vehicles and pet battles may yield unexpected results if the user reloads their UI while in a vehicle or pet battle. I'd suggest changing that block as follows: Code:
if event == "UNIT_ENTERING_VEHICLE" or event == "PET_BATTLE_OPENING_START" then self:Hide() elseif event == "UNIT_EXITED_VEHICLE" or event == "PET_BATTLE_CLOSE" then self:Show() else self:SetShown(not C_PetBattles.IsInBattle() and not UnitHasVehicleUI("player")) end Rather than storing your plugin order using strings, eg. "P3", and then using a giant if-else chain on each plugin as it's created, I'd suggest using a simple indexed table to store the order: Code:
-- Stat Locations enabledPlugins = { "stat1", "recount", "stat2", "guild", "spec", "friends", "pro", "dur", "bags", }, disabledPlugins = { "calltoarms", "coords", "system", }, Code:
self.plugins = {} for i = 1, #db.enabledPlugins do local name = db.enabledPlugins[i] local func = self.pluginConstructors[name] if func then local plugin = func() self.plugins[name] = plugin self:PlacePlugin(plugin, i) end end Code:
function nData:PlacePlugin(plugin, i) local area = math.ceil(i / 3) local parent if area == 1 then parent = StatPanelLeft elseif area == 2 then parent = StatPanelCenter elseif area == 3 then parent = StatPanelRight end if not parent then return -- somehow you passed a value > 9 end plugin:SetParent(parent) plugin:SetPoint("TOP") plugin:SetPoint("BOTTOM") if i == (area * 3 - 2) then plugin:SetPoint("LEFT", 30, 0) else plugin:SetPoint("LEFT", parent[i-1], "RIGHT", 10, 0) -- adjust for padding between plugins end parent[i] = plugin end Your options table would be easier to extend and maintain if you wrote it in the desired order, instead of grouping options by type. Putting all the separators first, for example, requires reading through the order properties of the entire options table to figure out where those separators will actually appear in-game. You also have a lot of duplicate orders, which would be much easier to spot if your table was better organized. Finally, you don't need to specify orders for every single option. Use low numbers to force important stuff like the enable toggle to the top, and negative numbers to force low-priority items like reset buttons to the bottom, and let AceGUI arrange the rest alphabetically in between.
__________________
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. |
|
05-04-14, 09:54 PM | #3 |
Thank You Phanx for all this tips, I know it like reinventing the wheel but I guess people like what they like. I have 3k downloads for nData sence I set it up. I know not a lot but there are people that use it.
This part of the new PlacePlugin throws a error: Code:
local area = math.ceil(i / 3) Error: Code:
2x nData\nData-5.4.7.lua:225: attempt to perform arithmetic on local "i" (a table value) nData\nData-5.4.7.lua:225: in function "PlacePlugin" nData-5.4.7\Plugins\Stat1.lua:34: in function "func" nData\nData-5.4.7.lua:301: in function <nData\nData.lua:280> (tail call): ? <in C code> <string>:"safecall Dispatcher[1]":9: in function <string>:"safecall Dispatcher[1]":5 (tail call): ? AckisRecipeList-2.5.5\libs\AceAddon-3.0\AceAddon-3.0-12.lua:558: in function "EnableAddon" AckisRecipeList-2.5.5\libs\AceAddon-3.0\AceAddon-3.0-12.lua:651: in function <AckisRecipeList\libs\AceAddon-3.0\AceAddon-3.0.lua:636> <in C code> FrameXML\UIParent.lua:306: in function "UIParentLoadAddOn" FrameXML\UIParent.lua:329: in function "CombatLog_LoadUI" FrameXML\UIParent.lua:742: in function <FrameXML\UIParent.lua:705> Locals: nil So when I got the plugin it self it uses nData:PlacePlugin(db.bags, Text) how will this new way work? Coke Edit: All the other adjustments are working great. Thank You |
|
05-04-14, 10:30 PM | #4 | |||
Also, please stop doing this: Code:
## Title: |cffCC3333 n|rData If you want to display your addon's name with colors in-game (eg. the title on your config panel) go for it, but please do not include color codes in any strings that are used in alphabetized lists.
__________________
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. |
||||
05-04-14, 11:08 PM | #5 |
You'll also need to change your options to accommodate this... though after looking at them, your current setup doesn't really work anyway, because it doesn't do anything to stop users from setting every single plugin to position 5, for example, which would result in all of them being shown on top of each other.
Hardcoding all of your plugins into your options table is not ideal, as it rather defeats the point of setting up the name/constructor table, and requires a lot of writing the same thing over and over. Here's a better way; also, I made some additional changes to the other stuff to make it easier to manage In your defaults: Code:
-- Stat Locations plugins = { bags = 9, -- show space used in bags on panel. system = 0, -- show total memory and others systems info (FPS/MS) on panel. guild = 4, -- show number on guildmate connected on panel. dur = 8, -- show your equipment durability on panel. friends = 6, -- show number of friends connected. spec = 5, -- show your current spec on panel. coords = 0, -- show your current coords on panel. pro = 7, -- shows your professions and tradeskills stat1 = 1, -- Stat Based on your Role (Avoidance-Tank, AP-Melee, SP/HP-Caster) stat2 = 3, -- Stat Based on your Role (Armor-Tank, Crit-Melee, Crit-Caster) recount = 2, -- Stat Based on Recount"s DPS calltoarms = 0, -- Show Current Call to Arms. } Code:
local classColor, currentFightDPS local pluginOrder = {} -- add this Code:
local function sortPlugins(a, b) if not a then return true end if not b then return false end local orderA, orderB = db.plugins[a] or 100, db.plugins[b] or 100 if orderA == orderB then return strupper(a) < strupper(b) else return orderA < orderB end end Code:
self.plugins = {} for name, state in pairs(db.plugins) do if state > 0 then local func = self.pluginConstructors[name] self.plugins[name] = func and func() or nil tinsert(pluginOrder, name) end end Code:
sort(pluginOrder, sortPlugins) for i = 1, #pluginOrder do local name = pluginOrder[i] db.plugins[name] = i -- fix any weirdness self:PlacePlugin(self.plugins[name], i) end Code:
local statposition = { ["0"] = L["Not Shown"], ["1"] = L["Position #1"], ["2"] = L["Position #2"], ["3"] = L["Position #3"], ["4"] = L["Position #4"], ["5"] = L["Position #5"], ["6"] = L["Position #6"], ["7"] = L["Position #7"], ["8"] = L["Position #8"], ["9"] = L["Position #9"], } Code:
DataGroup = { type = "group", order = 6, name = L["Text Positions"], guiInline = true, get = function(info) return tostring(db.plugins[ info[#info] ]) end, set = function(info, value) value = tonumber(value) local name = info[#info] -- Remove this plugin from the order: for i = 1, #pluginOrder do if pluginOrder[i] == name then tremove(pluginOrder, i) end end -- Re-sort the remaining plugins: sort(pluginOrder, sortPlugins) if value > 0 then -- Put this plugin back in the desired place, -- but only if it's enabled: tinsert(pluginOrder, name, value) else -- Mark it disabled in the db: db.plugins[name] = 0 end -- Now re-place all the plugins, and update -- the db values according to the new order: for i = 1, #pluginOrder do local name = pluginOrder[i] db.plugins[name] = i self:PlacePlugin(self.plugins[name], i) end end args = {}, }, Code:
local t = options.args.datatext.args.DataGroup.args for name in pairs(self.pluginConstructors) do t[name] = { type = "select", name = name, values = statposition, } end return options
__________________
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. |
|
05-04-14, 11:19 PM | #6 |
Also I peeked at one of your modules (Professions.lua), and you're leaking globals again; I assume the others have the same issue. For example, near the top you're leaking "db", "hexa" and "hexb" as globals.
Also you're doing some things wrong again: Code:
nData:PlacePlugin(db.pro, Text) 2 - You should not be placing the plugin inside its constructor function anyway. Just delete this line entirely. Code:
Text:SetFormattedText(hexa.."Professions"..hexb) Code:
if v ~= nil then local name, texture, rank, maxRank = GetProfessionInfo(v) Text:SetFormattedText(hexa.."Professions"..hexb) end Code:
local plugin = CreateFrame('Button', nil, Datapanel) However, specifying a parent here is still pointless, because you're going to re-parent the plugin frame later anyway. Code:
self:SetAllPoints(Text) Code:
self:SetWidth(self:GetStringWidth())
__________________
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. |
|
05-05-14, 10:53 AM | #7 | |||
I went threw and change everything you stated above but apperantly I got lost and something aint correct cause as you stated earlier I get the tooltip not the text ont he bar.
Below is the addon zipped up with your changes (I think). Coke |
||||
WoWInterface » Developer Discussions » Lua/XML Help » Walking before I Run.... |
«
Previous Thread
|
Next Thread
»
|
Display Modes |
Linear Mode |
Switch to Hybrid Mode |
Switch to Threaded Mode |
|
|