WoWInterface - View Single Post

cokedrivers · 05-05-14, 10:53 AM

Originally Posted by Phanx

That means you did not make all of the other related changes, and you are still passing a string like "P3" instead of the numeric index from an ordered table.

Yes, you need to change it to what I posted.

Instead of a string like "P3" you need to pass a number like 3.

Also, please stop doing this:

Code:

## Title: |cffCC3333 n|rData

Putting colors at the beginning of your TOC title breaks alphabetical sorting. If your addon is the only one that does it, you can just say "it's at the end of the list" which is only mildly annoying (though still annoying) but when every addon author does it, and uses a different color, addons end up being listed in basically random order, which is extremely obnoxious and totally defeats the purpose of alphabetizing the list in the first place.

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.

Originally Posted by Phanx

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)

1 - Aside from needing to update this for the ordering changes, you should be passing the plugin button/frame object to PlacePlugin, not the object's font string. Otherwise you will end up with the same problems you had before, with invisible clickable regions, etc.

2 - You should not be placing the plugin inside its constructor function anyway. Just delete this line entirely.

Code:

Text:SetFormattedText(hexa.."Professions"..hexb)

There's no reason to use SetFormattedText if you're not actually formatting anything. If you're passing a plain string that doesn't require formatting, use SetText instead.

Code:

if v ~= nil then
	local name, texture, rank, maxRank = GetProfessionInfo(v)
	Text:SetFormattedText(hexa.."Professions"..hexb)
end

If you're not using any of the info returned by GetProfessionInfo, save yourself the function call and don't call it at all.

Code:

local plugin = CreateFrame('Button', nil, Datapanel)

I was going to tell you this is pointless because "Datapanel" isn't defined in this scope... and then I looked at your other file and noticed you've reverted to using this generic name as the global name of your frame. Don't do that. Global object names, like all globals, should be unique, and should clearly identify the addon creating them. "Datapanel" doesn't satisfy either of those criteria. "nData_Datapanel" would be better -- it clearly identifies the frame as belonging to the nData addon, and it's extremely unlikely to conflict with any global created by any other addon or the default UI.

However, specifying a parent here is still pointless, because you're going to re-parent the plugin frame later anyway.

Code:

self:SetAllPoints(Text)

Instead of that, do this:

Code:

self:SetWidth(self:GetStringWidth())

Otherwise you will mess up parenting again.

Let me start with Thank You for Helping again, now that that is said I'm missing something or just not understanding your codeing.

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.

Also, please stop doing this:

The reason this is done is to keep this nData inline with the NeavUI addons (see image below).

Below is the addon zipped up with your changes (I think).

Coke