Changing target frame based on unit classification

[Joker119]

So I'm trying to change the target's border frame based on the target's UnitClassification

Basically, I want bosses to have a different border than elites and I want non-elites to have a different border than either of the other 2.
Here's the code I'm using:

local createArtwork = function(self)
    local t = self:CreateTexture(nil,"BACKGROUND",nil,-8)
        t:SetPoint("TOP",0,25)
    t:SetPoint("LEFT",-62,0)
    t:SetPoint("RIGHT",60,0)
    t:SetPoint("BOTTOM",0,-15)
      
        if UnitClassification(unit) == "worldboss" then
            t:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target_boss")
    elseif UnitClassification(unit) == "rare" or UnitClassification(unit) == "rareelite" or UnitClassification(unit) == "elite" then
         t:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target_elite")
    else 
         t:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target")
    end

Expected result:Bosses should get the frame named target_boss, elites get target_elite and everyone else gets target.
Actual result: Everyone including elites and bosses get the 'target' frame border.

[Resike]

Your unit is nil then?

[Joker119]

Originally Posted by Resike Your unit is nil then?

It's only looking for UnitClassification, if it is worldboss, rare, rareelite or elite it's suppose to change to a different target frame, otherwise, use the default.

[MunkDev]

Where's the unit variable coming from? There's no argument fed to that function for the actual unit. Also, how are you updating this to reflect new targets? It seems to me you're creating the artwork either once or over and over again on an event, such as PLAYER_TARGET_CHANGED.

local default = "Interface\\AddOns\\Roth_UI\\media\\target"
local unitClassTextures = {
    worldboss = "Interface\\AddOns\\Roth_UI\\media\\target_boss",
    elite = "Interface\\AddOns\\Roth_UI\\media\\target_elite",
    rare = "Interface\\AddOns\\Roth_UI\\media\\target_elite",
    rareelite = "Interface\\AddOns\\Roth_UI\\media\\target_elite",
}
 
local function OnEvent(self, event, ...)
    if event == "PLAYER_TARGET_CHANGED" and UnitExists("target") then
        self.RothBorder:SetTexture( unitClassTextures[UnitClassification("target")] or default )
    end
end
 
local function CreateArtwork(self)
    local t = self.RothBorder or self:CreateTexture(nil, "BACKGROUND", nil, -8)
    self.RothBorder = t -- recycle the same texture if you call this again for some reason
 
    t:SetPoint("TOP", 0, 25)
    t:SetPoint("LEFT", -62, 0)
    t:SetPoint("RIGHT", 60, 0)
    t:SetPoint("BOTTOM", 0, -15)
 
    self:RegisterEvent("PLAYER_TARGET_CHANGED")
    self:HookScript("OnEvent", OnEvent)
end

[Joker119]

Originally Posted by MunkDev Where's the unit variable coming from? There's no argument fed to that function for the actual unit. Also, how are you updating this to reflect new targets? It seems to me you're creating the artwork either once or over and over again on an event, such as PLAYER_TARGET_CHANGED.

https://github.com/galaxy119/Roth_UI
There's the source for the entire addon.

Spawning of the target frames is handled by the oUF framework. The addon ties into this here:

---------------------------------------------
  -- SPAWN TARGET UNIT
  ---------------------------------------------
 
  if cfg.units.target.show then
    oUF:RegisterStyle("diablo:target", createStyle)
    oUF:SetActiveStyle("diablo:target")
    oUF:Spawn("target", "Roth_UITargetFrame")
  end

oUF is in Roth_UI/embeds/oUF
target.lua is in Roth_UI/units

[Phanx]

Originally Posted by Galaxy119 Spawning of the target frames is handled by the oUF framework.

Right, but oUF doesn't magically pass your functions a hidden variable named "unit". You have to either accept an argument passed to your function and name it "unit", call a function and assign its return value to a variable named "unit", or manually define a variable named "unit" with some value.

Let's look at your target.lua file.

First, you define a file-level variable named "unit" in the main chunk on line 15:
https://github.com/galaxy119/Roth_UI...target.lua#L15

...that points to the unnamed, unadorned frame created in this other file for reasons unknown:
https://github.com/galaxy119/Roth_UI...core/units.lua

GitHub won't let me search for "ns.unit" so I can't tell if you're doing anything else with that frame in other files.

Then (back in target.lua) you define the "createArtwork" function in the main chunk, starting at line 36, here:
https://github.com/galaxy119/Roth_UI...target.lua#L36

Inside this function, you're referencing a "unit" variable, but you're handling it like it contains a unit token string. While normally that would be what a variable named "unit" contains, in this scope it is defined as a pointer to a frame. You can't pass a frame to a unit API function. At best, functions like UnitClassification will just silently ignore bad input and return nil; at worst, you'll get an error about passing a table where a string is expected.

Then you call that function on line 310, here:
https://github.com/galaxy119/Roth_UI...arget.lua#L310

...which is inside the "createStyle" function, which is defined in the main chunk starting at line 297:
https://github.com/galaxy119/Roth_UI...arget.lua#L297

...at the end of which, on line 370, you add the newly styled frame to that confusingly named "unit" object:
https://github.com/galaxy119/Roth_UI...arget.lua#L370

Based on that last line, I guess you're using that "unit" frame to keep track of all the frames created by your layout, but you really don't need to do that. oUF already keeps a list of all the objects it creates, in "oUF.objects". If you really think you need a table mapping unit-->frame (but you should think about it, because you probably don't actually need that -- in the rare case you need to find, for instance, the focus frame specifically, you can just iterate over oUF.objects and find the frame whose unit is "focus") you should do it a little less confusingly. Use a plain table (you don't need a frame to store key/value pairs) and don't refer to it as "unit" when that name is basically a reserved keyword in WoW addon programming.

Finally, re-read MunkDev's post. Your function (even if you fix the variable naming/scoping issues) only runs one time when your frame is first created. Even if we assume you already have a target at this point (which you can't, because that frame is created at login, and even if you just /reloaded, that still clears your target, so you don't have one anymore when oUF loads up again) you'd only be setting the texture one time, based on the classification of that specific target. Later, when you targetted something else, the texture would not update.

You can just register for events and handle them directly as in MunkDev's post, but if you're using oUF, you should do it properly and make it an element. The default QuestIcon element is a good starting point, as it already handles the right event (UNIT_CLASSIFICATION_CHANGED) and shows/hides a texture. All you'd need to do is make it look at UnitClassification instead of UnitIsQuestBoss, and have it change the texture in addition to showing/hiding it.

[Joker119]

Originally Posted by Phanx Right, but oUF doesn't magically pass your functions a hidden variable named "unit". You have to either accept an argument passed to your function and name it "unit", call a function and assign its return value to a variable named "unit", or manually define a variable named "unit" with some value. Let's look at your target.lua file. First, you define a file-level variable named "unit" in the main chunk on line 15: https://github.com/galaxy119/Roth_UI...target.lua#L15 ...that points to the unnamed, unadorned frame created in this other file for reasons unknown: https://github.com/galaxy119/Roth_UI...core/units.lua GitHub won't let me search for "ns.unit" so I can't tell if you're doing anything else with that frame in other files. Then (back in target.lua) you define the "createArtwork" function in the main chunk, starting at line 36, here: https://github.com/galaxy119/Roth_UI...target.lua#L36 Inside this function, you're referencing a "unit" variable, but you're handling it like it contains a unit token string. While normally that would be what a variable named "unit" contains, in this scope it is defined as a pointer to a frame. You can't pass a frame to a unit API function. At best, functions like UnitClassification will just silently ignore bad input and return nil; at worst, you'll get an error about passing a table where a string is expected. Then you call that function on line 310, here: https://github.com/galaxy119/Roth_UI...arget.lua#L310 ...which is inside the "createStyle" function, which is defined in the main chunk starting at line 297: https://github.com/galaxy119/Roth_UI...arget.lua#L297 ...at the end of which, on line 370, you add the newly styled frame to that confusingly named "unit" object: https://github.com/galaxy119/Roth_UI...arget.lua#L370 Based on that last line, I guess you're using that "unit" frame to keep track of all the frames created by your layout, but you really don't need to do that. oUF already keeps a list of all the objects it creates, in "oUF.objects". If you really think you need a table mapping unit-->frame (but you should think about it, because you probably don't actually need that -- in the rare case you need to find, for instance, the focus frame specifically, you can just iterate over oUF.objects and find the frame whose unit is "focus") you should do it a little less confusingly. Use a plain table (you don't need a frame to store key/value pairs) and don't refer to it as "unit" when that name is basically a reserved keyword in WoW addon programming. Finally, re-read MunkDev's post. Your function (even if you fix the variable naming/scoping issues) only runs one time when your frame is first created. Even if we assume you already have a target at this point (which you can't, because that frame is created at login, and even if you just /reloaded, that still clears your target, so you don't have one anymore when oUF loads up again) you'd only be setting the texture one time, based on the classification of that specific target. Later, when you targetted something else, the texture would not update. You can just register for events and handle them directly as in MunkDev's post, but if you're using oUF, you should do it properly and make it an element. The default QuestIcon element is a good starting point, as it already handles the right event (UNIT_CLASSIFICATION_CHANGED) and shows/hides a texture. All you'd need to do is make it look at UnitClassification instead of UnitIsQuestBoss, and have it change the texture in addition to showing/hiding it.

I will see what I can do to work that in in it's place.

This addon is a takeover project from zork, since he plans to abandon it in legion he let me have it and maintain it. I'm still trying to decipher bits and pieces of his code aswell.
What confuses me, is if we look later in the target.lua file, https://github.com/galaxy119/Roth_UI...arget.lua#L262
we can see UnitClassification(unit) being stored as a variable, then the variable called to in the proceeding if, then, else function to change the classtext based on what the targets UnitClassification is, and this particular function DOES work, but I see no where that it updates based on PLAYER_TARGET_CHANGED or anything similar.
If this works here, why does it not work in the previous function creating the unit artwork?

[Joker119]

I would still like to know why the previous code wasn't working, as I'm still learning LUA and would like to know what I missed.

I did however, fix the issue by adding

artwork = t

to the end of the createArtwork function and moving the SetTexture for it into the if then else function that I knew to be working. This solved my issue, making the frames change when elites/bosses are targeted.

[Phanx]

Originally Posted by Galaxy119 What confuses me, is if we look later in the target.lua file, https://github.com/galaxy119/Roth_UI...arget.lua#L262 we can see UnitClassification(unit) being stored as a variable, then the variable called to in the proceeding if, then, else function to change the classtext based on what the targets UnitClassification is, and this particular function DOES work, but I see no where that it updates based on PLAYER_TARGET_CHANGED or anything similar. If this works here, why does it not work in the previous function creating the unit artwork?

It works there because that function is registered with oUF as the update function for the "diablo:classtext" tag. (The target frame uses that tag on this font string.) oUF automatically updates tags (and all other registered elements) when you select a new target.

[Joker119]

Originally Posted by Phanx It works there because that function is registered with oUF as the update function for the "diablo:classtext" tag. (The target frame uses that tag on this font string.) oUF automatically updates tags (and all other registered elements) when you select a new target.

Ohhhh I see. Ok that makes sense. Theoretically it would work inside any function that updates when a new target is selected, then?

[Phanx]

Yes, but it's not a good idea to stick unrelated code in random places. You'll forget where you put it, and anyone else looking at your code will be very confused about why there is code updating a texture based on the unit's classification in a function that otherwise handles updating a mana bar. Just do it right and make it an element.

[Joker119]

Originally Posted by Phanx Yes, but it's not a good idea to stick unrelated code in random places. You'll forget where you put it, and anyone else looking at your code will be very confused about why there is code updating a texture based on the unit's classification in a function that otherwise handles updating a mana bar. Just do it right and make it an element.

Agreed, random code in stupid places is a bad idea. I tried making my own function for it before reading your reply, this is what i have..

--Classification Artwork
local classification = function(self,event)
 local uclass = UnitClassification(self.unit)
    if event == "PLAYER_TARGET_CHANGED" then
        if uclass == "worldboss" or UnitLevel(self.unit) == -1 then
        artwork:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target_boss")
    elseif uclass == "rare" or uclass == "rareelite" or uclass == "elite" then
        artwork:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target_elite")
    else
        artwork:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target")
    end
    end
end

and it is called at the end of the lua file with:

--Classification
    self:RegisterEvent("PLAYER_TARGET_CHANGED", classification)
    self.Health:SetScript("OnShow",function(s)
        classification(self,"PLAYER_TARGET_CHANGED")
    end)

However, using the PLAYER_TARGET_CHANGED even for this is very buggy, and I tried using UNIT_CLASSIFICATION_CHANGED but that doesn't appear to update the artwork at all (it just leaves it with none)


I'm still new to oUF and lua in general, i'm unsure how to go about making it an element :s

[Phanx]

1. Copy the QuestIcon element file (oUF/elements/qicon.lua)

2. Change all instances of "QuestIcon" inside the file to "RareIcon" (or whatever you want to call it)

3. Change this part of the "Update" function:

	local isQuestBoss = UnitIsQuestBoss(unit)
	if(isQuestBoss) then
		qicon:Show()
	else
		qicon:Hide()
	end

to query UnitClassification instead, and use SetTexture with the desired texture path instead of Show/Hide.

4. Remove this part of the "Enable" function:

		if(qicon:IsObjectType'Texture' and not qicon:GetTexture()) then
			qicon:SetTexture[[Interface\TargetingFrame\PortraitQuestBadge]]
	end

5. Congratulations, you've made an element. Don't forget to add it to your addon's TOC. It should be listed before any files that will spawn frames.

6. Now back in your frame creation, remove the SetTexture parts of the "createArtwork" function, and assign the created texture object to "self.RareIcon" (or whatever you named the element in step 2).

---------------

On a side note, you really need to pay more attention to your indentation. Look at the code from your last post:

    --Classification Artwork
    local classification = function(self,event)
     local uclass = UnitClassification(self.unit)
        if event == "PLAYER_TARGET_CHANGED" then
            if uclass == "worldboss" or UnitLevel(self.unit) == -1 then
            artwork:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target_boss")
        elseif uclass == "rare" or uclass == "rareelite" or uclass == "elite" then
            artwork:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target_elite")
        else
            artwork:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target")
        end
        end
    end

The line in magenta opens a new block level, but the lines following it aren't indented by another level. This makes it look like the cyan "elseif" belongs to the same conditional chain as the cyan "if" -- but it doesn't. It also makes it look like you have an extra end statement (highlighted in orange) near the end of the function. Finally first 3 lines should have the same level of indentation, but lines 1-2 are indented with 4 spaces, and line 3 with 5 spaces.

If you are manually indenting with spaces by pressing the space key 4 times, stop that right now. Use tabs instead of spaces (one press of the key = 1 level of indentation) or switch to an editor that handles space-based indentation properly (one press of the tab key inserts 4 spaces, and pressing backspace deletes 4 spaces at a time).

Right now you're making things unnecessarily difficult for yourself, and anyone trying to help with your code, by making your code look far more broken than it is.

[Joker119]

Originally Posted by Phanx 1. Copy the QuestIcon element file (oUF/elements/qicon.lua) 2. Change all instances of "QuestIcon" inside the file to "RareIcon" (or whatever you want to call it) 3. Change this part of the "Update" function: local isQuestBoss = UnitIsQuestBoss(unit) if(isQuestBoss) then qicon:Show() else qicon:Hide() end to query UnitClassification instead, and use SetTexture with the desired texture path instead of Show/Hide. 4. Remove this part of the "Enable" function: if(qicon:IsObjectType'Texture' and not qicon:GetTexture()) then qicon:SetTexture[[Interface\TargetingFrame\PortraitQuestBadge]] end 5. Congratulations, you've made an element. Don't forget to add it to your addon's TOC. It should be listed before any files that will spawn frames. 6. Now back in your frame creation, remove the SetTexture parts of the "createArtwork" function, and assign the created texture object to "self.RareIcon" (or whatever you named the element in step 2). --------------- On a side note, you really need to pay more attention to your indentation. Look at the code from your last post: --Classification Artwork local classification = function(self,event) local uclass = UnitClassification(self.unit) if event == "PLAYER_TARGET_CHANGED" then if uclass == "worldboss" or UnitLevel(self.unit) == -1 then artwork:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target_boss") elseif uclass == "rare" or uclass == "rareelite" or uclass == "elite" then artwork:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target_elite") else artwork:SetTexture("Interface\\AddOns\\Roth_UI\\media\\target") end end end The line in magenta opens a new block level, but the lines following it aren't indented by another level. This makes it look like the cyan "elseif" belongs to the same conditional chain as the cyan "if" -- but it doesn't. It also makes it look like you have an extra end statement (highlighted in orange) near the end of the function. Finally first 3 lines should have the same level of indentation, but lines 1-2 are indented with 4 spaces, and line 3 with 5 spaces. If you are manually indenting with spaces by pressing the space key 4 times, stop that right now. Use tabs instead of spaces (one press of the key = 1 level of indentation) or switch to an editor that handles space-based indentation properly (one press of the tab key inserts 4 spaces, and pressing backspace deletes 4 spaces at a time). Right now you're making things unnecessarily difficult for yourself, and anyone trying to help with your code, by making your code look far more broken than it is.

THANK YOU so much. I will do this ASAP and let you know the outcome. This is exactly what I needed to be taught xD

And for the indenting, I'm not sure why but sometimes when I copy/paste from the lua file into the reply box it doesn't seem to transfer my indenting properly.
To me (and others that have edited the code) it is layed out properly, I think this may be due to the text editor I use (I am on Linux, btw, not Windows) automatically setting the block indents when I make a new line, and it doesn't always transfer over to forums well.

I'll change it so I do the indents manually, it's been bugging me to seeing bad indents on my lua highlights.

I'm not a noob to coding in general, just noob to Lua and addons. I usually work with web script and BASH.

[Joker119]

Beautiful! Brilliant!

Element is made, tied in and everything functions as expected.


I do have one question about an issue I've noticed..
For some reason UnitClassification(unit) is reporting world bosses as 'normal'

(Boss) target dummies, Supreme Lord Kazzak, etc, are all reported as 'normal' unitclassifications, in order to get the boss portion of the new artwork styles to detect bosses I instead have to resort to

 if UnitLevel(unit) == -1 then

instead of

 if UnitClassification(unit) == "worldboss" then

Is this a bug in the WoW api? Or did they change the classification for bosses in Legion?


Either way, going by unitlevel works pretty well and I don't forsee it causing any issues. Thanks MUCH MUCH for the help.

[Phanx]

Originally Posted by Galaxy119 I'm not sure why but sometimes when I copy/paste from the lua file into the reply box it doesn't seem to transfer my indenting properly.

Most likely, your editor's configuration for tabs vs. spaces doesn't match what was used in the original file before you started working on it, so when you press tab, you're inserting 4 spaces where the rest of the file has actual tabs, or the other way around. If your editor has a function for converting indentation, use it; otherwise do a manual search-and-replace to convert all the existing indentation to your preferred style.

The reason it looks correct in your editor, but not when you paste it into a forum post, is that your editor is using a different width for tab characters than the web browser.