Go to Page... |
Thread Tools | Display Modes |
07-21-13, 09:33 PM | #1 |
Checking if raid group is in combat with a specific boss...
Hi. I'm new to LUA and AddOn development, so this might be a pretty simple question but I'm having problems finding an efficient way to check if a raid group is in combat with a boss.
I currently have it so that when it registers that I enter combat it checks each group member's target (cycles through based on "raid1" "raid2" etc.) and gets its NPC ID then compares that to values in a table. This sometimes work but it's very inefficient and unreliable for several reasons, but mainly being that if people don't have the boss targeted right away when their turn comes it won't register it and thus if no one does it may miss a correct ID. I tried looking over DBM's codes but since I've only started learning this this past week, it's way too advanced for me to get an idea of what they did. Any help is greatly appreciated! |
|
07-22-13, 12:11 AM | #2 |
This is not a simple question, and the answer depends on what bosses you're trying to track.
Typically, a fight against a raid boss from newer expansions (I think starting with wrath?) create a unit frame for the boss, and therefor have corresponding events which make it relatively easy to detect when you're in combat with them. If the boss in question does not spawn a unit frame when engaged it becomes more complicated, as some fights don't even involve any sort of combat between you and the boss itself. DBM and BigWigs treat each boss independently and have modules that describe how each fight is detected. Most bosses (or other mobs) can be detected through general combat log events though, so unless the fight you're trying to track is really unusual like Valithria in Icecrown where the objective is to actually heal the boss then it probably won't be too hard to figure out. Odds are you'll be tracking it through either the boss frame event (INSTANCE_ENCOUNTER_ENGAGE_UNIT), the combat log, or some emote that gets sent to the chat when the fight starts. |
|
07-22-13, 01:39 AM | #3 |
It really depends a lot on what you're actually trying to do.
In my add-on, for example, I have a list of unit names that correspond to boss encounters, and as soon as one of them appears in the log I know this encounter is in progress. For example, Sul the Sand Crawler is from the (Elder Council, was it?) encounter, and Lu'lin is from the Twin Consorts encounter. This way I don't depend on any frames or events that may or may not work on specific encounters, but on the other hand I have to do extra work for specific bosses.
__________________
SanityCheck - If you've ever said the words "Sorry, I forgot" then you need this add-on. Remember, every time you post a comment on an add-on, a kitten gets its wings! |
|
07-22-13, 02:49 AM | #4 |
Using NPC names is a really bad way to do it, as they are not locale-independent, and require hardcoded translations for every language. Instead, you should use mobIDs, and check them against the unit's GUID. To find a mobID, look it up on Wowhead and copy the number out of the URL. For example, the mobID for Horridon is 68476. And to get the mobID from a GUID inside your CLEU handler:
Code:
local mobID = tonumber(strsub(sourceGUID, 6, 10), 16) Code:
-- outside CLEU handler: local mobIDs = setmetatable({}, { __index = function(t, guid) local mobID = tonumber(strsub(guid, 6, 10), 16) t[guid] = mobID return mobID end }) -- inside CLEU handler: local mobID = mobIDs[sourceGUID]
__________________
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. |
|
07-22-13, 03:20 AM | #5 |
If you are checking for a specific boss, or a list of bosses, could you use INSTANCE_ENCOUNTER_ENGAGE_UNIT? AFAIK, it only works during encounters where you would see one or more boss frames. Having checked both wowprogramming.com and wowpedia.org, it doesn't appear to pass any arguments, but the documentation might not be current.
|
|
07-22-13, 04:29 AM | #6 | |
I don't think it's necessary to call UnitExists first since you need the GUID anyway and UnitGUID returns nil for an invalid unit ID. |
||
07-22-13, 05:45 AM | #7 |
I think I've seen the GUID method that you mention used in DBM, and while the advantages are clear, I felt that getting my version 1.00 up and running was a higher priority than making sure the add-on would work for users that may or may not exist. It's just a quick and dirty solution. Which is why I said it depends on what you're trying to do at the moment
__________________
SanityCheck - If you've ever said the words "Sorry, I forgot" then you need this add-on. Remember, every time you post a comment on an add-on, a kitten gets its wings! |
|
07-22-13, 01:55 PM | #8 | |
Code:
local BossesToCheck = { [68476] = true, -- Horridon [69017] = true, -- Primordius } local MyAddon = CreateFrame("Frame") MyAddon:RegisterEvent("INSTANCE_ENCOUNTER_ENGAGE_UNIT") MyAddon:SetScript("OnEvent", function(self, event, ...) for i = 1, MAX_BOSS_FRAMES do local guid = UnitGUID("boss"..i) if guid then local mobid = tonumber(strsub(guid, 6, 10), 16) if BossesToCheck[mobid] then -- You are engaged with one of the bosses on your list. return end end end -- If you didn't return out by now, you're not engaged with any of the listed bosses. 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. |
||
07-22-13, 04:23 PM | #9 |
Just an update what I came up with so far is this:
Code:
local timestamp, eventType, sourceGUID, sourceName, sourceFlags, destGUID, destName = ... enemy = NPCID(sourceName) if event == "COMBAT_LOG_EVENT_UNFILTERED" then for k,v in pairs(ml_bosses) do if v == enemy and methodtrigger ~= 1 then method, threshold = GetLootMethod(), GetLootThreshold() print("|cff00ff00MountLooter: Master Looter set! Will revert back to \""..method.."\" loot method and threshold "..threshold.." when boss is killed.") SetLootMethod("master",UnitName("player")) methodtrigger = 1 -- prevents spam break end end end ml_bosses is the table where I have the NPCIDs of the bosses I want stored. The NPCID() function is just a function I made with the conversion from GUID to NPCID. methodtrigger prevents spam since it will set the value to 1 the first time a correct NPC is found in the combat log and I have the code if statement dependent of this being ~= 1. Further on in the "LOOT_OPENED" event I'll set methodtrigger = nil so it can rinse and repeat for other bosses. I have only been able to test it so far on Onyxia and some mobs outside because I'm trying to finish up the rest of the coding before I check others. For now I just need a place to stay and test. So far it's working. Thanks for the pointers and further tips are still appreciated, I will be checking back in (especially because I haven't been able to test everything yet, so I might still have problems!). Also because I don't want to spam a bunch of posts in this forum, I do have another question. I created two frames that I was using to handle my events. One I made as a sort of welcome frame that defines appropriate variables when entering the world and prints a message depending on whether or not the player is in an instance and raid leader. I set it so that if these criterion were met if would register events to the second frame to enable the functionality of the addon (so that it wouldn't register the events frame two was watching if, for example, you weren't even raid leader, etc). I registered the COMBAT_LOG_EVENT_UNFILTERED to frame two and it works just fine. However when I try to register "LOOT_OPENED" to the same frame it doesn't work. The event fires but it won't execute the if statement I set. For debugging purposes I registered it to my welcome frame instead and it worked. So, does COMBAT_LOG_EVENT_UNFILTERED have to have its own frame or is there another issue going on here? Last edited by Niketa : 07-22-13 at 04:45 PM. |
|
07-22-13, 08:53 PM | #10 |
You don't need a separate frame for each event, but you do need to handle your events properly. In the code you posted, you are defining CLEU-specific variables before checking that you're actually handling the CLEU event. Check the event first, and then assign the provided arguments to variables:
Code:
MyFrame:SetScript("OnEvent", function(self, event, ...) if event == "COMBAT_LOG_EVENT_UNFILTERED" then local timestamp, eventType, sourceGUID, sourceName, sourceFlags, destGUID, destName = ... -- do stuff here elseif event == "LOOT_OPENED" then local autoLoot = ... -- do stuff here end end) Finally, there is no reason to define your event-handler-specific variables as locals in any scope other than the scope of your event handler function. Always limit your variables to the narrowest scope necessary. Defining, for instance, local enemy outside of your CLEU handler just makes your code look badly written (eg. leaked globals) to anyone else who glances at it, and makes it harder for anyone (yourself included) to determine the variable's scope, but doesn't actually confer any performance benefits.
__________________
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. |
|
07-22-13, 09:55 PM | #11 |
@Phanx
The reason why I didn't use INSTANCE_ENCOUNTER_ENGAGE_UNIT is because from what I understand it only works on more recent bosses that have the boss frames. For the purpose of the addon I'm working on, I will need to be able to check bosses from older raids as well. Also as for the local variables being defined out of the handler functions, I've only defined those outside when I need to use that variable in a different handler. So I set it as a local variable with a nil value and then when an event fires the value is changed appropriately. I'm then able to use that same variable in other events without defining it as global, which I've been trying to avoid as much as possible so I don't have any conflicts with other addons using the same variables and without having to use ridiculously long variable names. What would be a more efficient method of passing variables around? With the CLEU variables defined in the wrong space, that just came from a lot of confusion. All of my programming work is self taught through trial and error and as much google/forum research as I can. Definitely still at a beginner phase. At the time of writing that I didn't quite understand the arguments and I still only have a hazy idea of how they work (but I understand how I messed up because these values would be different for each CLEU). |
|
07-23-13, 07:50 AM | #12 |
@Semlar: I was making the suggestion to use the event.
@Phanx: And that is how I would have used the suggestion. @Niketa: Well, now I see my suggestion does not help. Darn older content! |
|
07-23-13, 11:09 AM | #13 |
Oh, I was confused because I had already brought it up in my first post.
@Niketa If you're trying to automatically set the master looter when you get into combat with specific bosses then I would make a table like Phanx had with the npc IDs as the keys so you don't need to loop over the entire table just to see if the boss is in there. If you're going to use the combat log, make sure to only monitor events that actually put you in combat with the unit, for example SPELL_AURA_APPLIED fires for a lot of spells that don't (like hunter's mark) and should probably be ignored. You also want to make sure you don't accidentally set the master looter again after the boss has died due to residual effects that could still be going off like void zones or dots. |
|
07-23-13, 11:21 AM | #14 |
I was thinking to reduce some of the variables could I register the LOOT_OPENED event to a new frame if the CLEU finds a match like I do to enable the functionality for only raid leaders anyway? Or is having too many different event handler frames bad practice?
Sorry if that's confusing, on my phone and it's hard to type. I can explain what I mean more later when I can type if necessary. Semlar I have my CLEU if statement dependent on UnitAffectingCombat("player") too. I had something else to say but... I should wait til I'm home. Lol Last edited by Niketa : 07-23-13 at 11:27 AM. |
|
07-23-13, 02:52 PM | #15 | ||
That said, checking UnitAffectingCombat every time CLEU fires is really wasteful. If you only want to respond to CLEU events while you're in combat, then you should only register for the CLEU event while you're in combat. Instead of registering for CLEU right away, register for PLAYER_REGEN_DISABLED and PLAYER_REGEN_ENABLED instead. When PLD fires, that means you entered combat, so register CLEU. When PRE fires, that means you left combat, so unregister CLEU. This way, you frame will simply not receive CLEU events when you don't want them.
__________________
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. |
|||
07-23-13, 07:04 PM | #16 |
The code I currently have is all over the place and will give you a headache. It's currently functional in my test environment (have not been able to test all bosses yet since I don't lead my raids until the weekend) but I'm starting over from scratch to try do it right.
This is what I have so far. Obviously it's not done, but this is the new direction I'm heading based on the feedback you guys have given. Lua Code:
Is this a step in the right direction or am I still way off? (* I just realized I forgot to add in your suggestion about caching the NPCID so I don't keep calling local enemyNPCID. I don't really understand your code for that [as far as metatables go]; would you mind explaining so I actually understand what I'm writing [we don't want another issue like my local variables for CLEU, lol]? *) Last edited by Niketa : 07-23-13 at 08:12 PM. |
|
07-23-13, 11:13 PM | #17 |
This is a considerable improvement.
One change you should make is use 'elseif' for your event checks so that it doesn't have to evaluate the other conditions after one succeeds. The other thing is you didn't localize nmlmethod or nmlthreshold so they become global variables which you generally want to avoid on the off-chance you overwrite something. You can also consolidate your conditions a little bit, for example.. Lua Code:
Lua Code:
Last edited by semlar : 07-23-13 at 11:22 PM. |
|
07-23-13, 11:21 PM | #18 | |
That's looking a lot better. Just a couple suggestions:
1. Your event handler should be an if-else chain, not a bunch of separate if blocks. For example, right now, when PLAYER_ENTERING_WORLD fires, the first if block in your function runs -- but then all the other blocks have to be checked, too, even though it's obvious to you as a human that they'll never match that event. Alternatively, add a return inside each if block, so that if the block is run, it will exit the function afterwards instead of going through and checking all the other blocks. Code:
if event == "PLAYER_ENTERING_WORLD" or event == "PARTY_LEADER_CHANGED" then -- do something -- no "end" here elseif event == "PLAYER_REGEN_DISABLED" then -- do something for entering combat ... Don't: Code:
if event == "COMBAT_LOG_EVENT_UNFILTERED" and eventType ~= "SPELL_AURA_APPLIED" then -- do something end Code:
if event == "COMBAT_LOG_EVENT_UNFILTERED" then if eventType == "SPELL_AURA_APPLIED" then -- don't want this, quit. return end -- do something end Code:
events:RegisterEvent("PLAYER_ENTERING_WORLD") events:RegisterEvent("PARTY_LEADER_CHANGED") events:SetScript("OnEvent", function(self, event, ...) -- automatically call the method for this event, if it exists return self[event] and self[event](self, event, ...) end) function events:PLAYER_ENTERING_WORLD(event) -- this function gets called when PEW fires end function events:PARTY_LEADER_CHANGED(event) -- this function gets called when PLC fires end Code:
function events:PLAYER_ENTERING_WORLD(event) -- this function gets called when PEW *or* PLC fires end events.PARTY_LEADER_CHANGED = events.PLAYER_ENTERING_WORLD
Code:
local mobIDFromGUID = setmetatable({}, { __index = function(t, guid) local mobID = tonumber(strsub(guid, 6, 10), 16) t[guid] = mobID return mobID end }) Code:
local enemyNPCIDa = tonumber(strsub(sourceGUID, 6, 10), 16) local enemyNPCIDb = tonumber(strsub(destGUID, 6, 10), 16) Code:
local enemyNPCIDa = mobIDFromGUID[sourceGUID] local enemyNPCIDb = mobIDFromGUID[destGUID] Code:
local mobIDFromGUID = setmetatable({}, { __index = function(t, guid) local mobID = tonumber(strsub(guid, 6, 10), 16) t[guid] = mobID return mobID end }) Code:
local mobIDFromGUID = {} Code:
setmetatable(mobIDFromGUID, {}) But, we want the table to do something special when we look up a value that isn't actually contained in it, so we'll add an __index key to the metatable, whose value is the function to run when this happens: Code:
setmetatable(mobIDFromGUID, { __index = function(t, k) print("GUID", k, "isn't in the table yet!") end }) Code:
setmetatable(mobIDFromGUID, { __index = function(t, guid) -- Extract the mob ID from the looked-up GUID: local mobID = tonumber(strsub(guid, 6, 10), 16) -- Store the guid->mobID pair in the table so it can be accessed faster -- the next time the same GUID gets looked up: t[guid] = mobID -- Finally, return the mobID to whatever did the lookup. -- As far as the lookup code (eg. mobIDFromGUID(sourceGUID)) is concerned, -- the only thing that happened is a regular table lookup. return mobID 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. |
||
07-24-13, 12:20 AM | #19 | |||
I'm still reworking the code with your suggestions so I won't post quite yet, but I will either before I go to bed or after work tomorrow. Last edited by Niketa : 07-24-13 at 01:21 AM. |
||||
07-24-13, 01:37 AM | #20 | |||
Code:
events:SetScript("OnEvent", function(self, event, ...) end) Typically, the first two arguments are named "self" and "event" but variable names do not actually affect the variables, so you can could name them "panda" and "polkadot" if you really wanted, though I'd recommend you stick with the established conventions to keep your code readable, both for yourself and for anyone else who looks at it. Code:
events:SetScript("OnEvent", function(self, event, ...) local method = self[event] end) Code:
function events:PLAYER_LOGIN(event) print("I am now logged in!") end Code:
events:SetScript("OnEvent", function(self, event, ...) local method = self[event] if method then method(self, event, ...) end end) Finally, if the function exists, we call it, passing along all the arguments received by the OnEvent script. Code:
events:SetScript("OnEvent", function(self, event, ...) return self[event] and self[event](self, event, ...) end)
Code:
function events:PLAYER_ENTERING_WORLD(event) local isInInstance, instanceType = IsisInInstance("player") if not isInInstance or instanceType ~= "raid" then -- Quit: return end if not UnitIsGroupLeader("player") then -- Stop listening to events: self:UnregisterEvent("PLAYER_REGEN_DISABLED") self:UnregisterEvent("PLAYER_REGEN_ENABLED") self:UnregisterEvent("PARTY_LOOT_METHOD_CHANGED") -- Tell the player: print("|cff00ff00Niketa's Mount Looter disabled: You need to be raid leader to use this addon.") -- Quit: return end -- If you've gotten to this point, you are in a raid instance, -- and you are the group leader. -- Start listening to events: self:RegisterEvent("PLAYER_REGEN_DISABLED") self:RegisterEvent("PLAYER_REGEN_ENABLED") self:RegisterEvent("PARTY_LOOT_METHOD_CHANGED") -- Tell the player: print("|cff00ff00Niketa's Mount Looter enabled!") end events.PARTY_LEADER_CHANGED = events.PLAYER_ENTERING_WORLD
The only time you are required to use a specific variable name is when you are using method notation, in which case the variable "self" is automatically created and assigned. This is method notation: Code:
function MyAddon:DoSomething() print(type(self)) end Code:
function MyAddon.DoSomething(self) print(type(self)) end Code:
MyAddon:DoSomething() Code:
MyAddon.DoSomething(MyAddon) This is also why inside your OnEvent function, you can just refer to "self" instead of "events", though both variables mean the same thing in that particular scope.
__________________
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 : 07-24-13 at 01:44 AM. |
||||
WoWInterface » Developer Discussions » Lua/XML Help » Checking if raid group is in combat with a specific boss... |
«
Previous Thread
|
Next Thread
»
|
Thread Tools | |
Display Modes | |
|
|