[CF 1635] Doesn't work check for pet spells in range
tmw-issue-import opened this issue ยท 14 comments
I want to check DK's pet Jump spell for in range condition. I tried "Icon" tab with Spell Cooldown type and "Conditions" tab with spell in range, nothing don't work. I searched some info and found that IsSpellInRange function has stop work since 7.2 patch, seems like truth coz I can't receive through /dump true statement but I found that action bar slot still works for that. Is any way to make check pet spell for in range?
Posted by CurseForge user cordankos | Imported from CurseForge issue #1635 | Raw
First: from your Spellbook's Pet tab, drag "Leap" (from here on out I'm assuming that's what you mean by Jump spell) to one of your action bars.
Import the following custom lua snippet:
^1^T^SName^SLeapInRance ^SCode^Sfunction~`GetActionSlot(id)~J ~`~`~`~`local~`id~`=~`id~J ~`~`~`~`for~`i=1,~`72~`do~J ~`~`~`~`~`~`~`~`if~`select(2,~`GetActionInfo(i))~`==~`id~`then~J ~`~`~`~`~`~`~`~`~`~`~`~`return~`i~J ~`~`~`~`~`~`~`~`end~J ~`~`~`~`end~J end~J ~J leapslot~`=~`GetActionSlot(47482)~J if~`not~`leapslot~`then~J ~`~`~`~`leapslot~`=~`GetActionSlot(91809)~J end~J ~J ~J function~`IsLeapInRange(unit)~J ~`~`~`~`if~`not~`leapslot~`then~J ~`~`~`~`~`~`~`~`return~`false~J ~`~`~`~`end~J ~`~`~`~`if~`not~`unit~`then~J ~`~`~`~`~`~`~`~`unit~`=~`"target"~J ~`~`~`~`end~J ~`~`~`~`return~`UnitExists(unit)~`and~`IsActionInRange(leapslot,unit)~J end~J ~J ~J ~J ~J ^t^N85702^S~`~| ^Scodesnippet^^
For your icon's condition, create a Miscellaneous > "Lua (advanced)" condition and put IsLeapInRange() in the textbox.
Here's an example group with such an icon you can import and look at:
^1^T^SGUID^STMW:group:1S63h0liB99r ^SScale^F6380114325536771 ^f-51^SPoint ^T^Sy^F8469066696097792 ^f-47^Spoint ^SBOTTOM^SrelativePoint ^SBOTTOM^Sx ^F-5339729237517724^f-46 ^t^SColumns^N1 ^SIcons^T ^N1^T ^SType^Sconditionicon ^SConditions^T ^N1^T ^SType^SLUA ^SName^SIsLeapInRange() ^t^Sn^N1 ^t^SSettingsPerView^T ^Sicon^T ^STextLayout^Sicon1 ^STexts^T ^N1^SLeap ^N2^S ^t^t^t^SCustomTex^S47482 ^SStates^T ^N1^T ^t^N3^T ^t^N4^T ^t^t^SEnabled^B ^t^N2^T ^SStates^T ^N1^T ^t^N3^T ^t^N4^T ^t^t^t^N3^T ^SStates^T ^N1^T ^t^N3^T ^t^N4^T ^t^t^t^N4^T ^SStates^T ^N1^T ^t^N3^T ^t^N4^T ^t^t^t^t^t^N85702^S~`~| ^Sgroup^N1 ^^
Edited Dec 17, 2018
Posted by CurseForge user canofbutter
Thanks for reply, actually its heavy load cpu with huge fps drop to query each slot so I did it by cache through events UNIT_PET and ACTIONBAR_SLOT_CHANGED, idea is same, so sadly that I saw this too late.
Posted by CurseForge user cordankos
It doesn't check every cycle. It only has to check every slot once when the snippet first loads. (the downside to having done that means if you move Leap to a different slot, you'll have to rerun the snippet or /reload) Read the code again - you'll see that's how it works.
Edited Dec 18, 2018
Posted by CurseForge user canofbutter
Here you go - I spent a little more time on this for you so you can freely move Leap around on your action bars without reloading:
^1^T^SName^SLeapInRange ^SCode^Sfunction~`GetActionSlot(id)~J ~`~`~`~`local~`id~`=~`id~J ~`~`~`~`for~`i=1,~`72~`do~J ~`~`~`~`~`~`~`~`if~`select(2,~`GetActionInfo(i))~`==~`id~`then~J ~`~`~`~`~`~`~`~`~`~`~`~`return~`i~J ~`~`~`~`~`~`~`~`end~J ~`~`~`~`end~J end~J ~J local~`leapslot~`=~`GetActionSlot(47482)~J if~`not~`leapslot~`then~J ~`~`~`~`leapslot~`=~`GetActionSlot(91809)~J end~J ~J ~J local~`leapf~`=~`CreateFrame("FRAME");~J ~J leapf:RegisterEvent("ACTIONBAR_SLOT_CHANGED");~J ~J function~`LeapButtonHandler(self,~`event,~`...)~J ~`~`~`~`leapslot~`=~`GetActionSlot(47482)~J ~`~`~`~`if~`not~`leapslot~`then~J ~`~`~`~`~`~`~`~`leapslot~`=~`GetActionSlot(91809)~J ~`~`~`~`end~J end~J ~J leapf:SetScript("OnEvent",~`LeapButtonHandler);~J ~J ~J ~J function~`IsLeapInRange(unit)~J ~`~`~`~`if~`not~`leapslot~`then~J ~`~`~`~`~`~`~`~`return~`false~J ~`~`~`~`end~J ~`~`~`~`if~`not~`unit~`then~J ~`~`~`~`~`~`~`~`unit~`=~`"target"~J ~`~`~`~`end~J ~`~`~`~`return~`UnitExists(unit)~`and~`IsActionInRange(leapslot,unit)~J end~J ~J ~J ~J ~J ~J ~J ^t^N85702^S~`~| ^Scodesnippet^^
For convenience of reviewing it, here's the full code of the snippet without the import string encoding:
function GetActionSlot(id)
local id = id
for i=1, 72 do
if select(2, GetActionInfo(i)) == id then
return i
end
end
end
local leapslot = GetActionSlot(47482)
if not leapslot then
leapslot = GetActionSlot(91809)
end
local leapf = CreateFrame("FRAME");
leapf:RegisterEvent("ACTIONBAR_SLOT_CHANGED");
function LeapButtonHandler(self, event, ...)
leapslot = GetActionSlot(47482)
if not leapslot then
leapslot = GetActionSlot(91809)
end
end
leapf:SetScript("OnEvent", LeapButtonHandler);
function IsLeapInRange(unit)
if not leapslot then
return false
end
if not unit then
unit = "target"
end
return UnitExists(unit) and IsActionInRange(leapslot,unit)
end
As you can see, the function that scans all action bar slots is only called on load and on the action bar event - not every update. IsLeapInRange uses the already scanned for "leapslot" variable in calling IsActionInRange; it does not continuously call GetActionSlot.
Posted by CurseForge user canofbutter
I have around same code. In your code you still have weak in cpu performance because ACTIONBAR_SLOT_CHANGED trigers on anything, if you have macro with modifiers then it will triger event and again call GetActionSlot several times while you playing since slot changing by macro. Also more better will be add time stamp to stop triger event several times at same time rate. And even if you triger ACTIONBAR_SLOT_CHANGED then in arg1 it return slot which has been changed, so you can compare it with leapslot variable as well which make you more performance. UnitExists(unit) in IsLeapInRange can be removed since IsActionInRange always return nil if target is not valid, better "return IsActionInRange(leapslot,unit) or false". Wow has 120 max buttons to check, I don't here a lot why need use 72 if possible make all 120. True ID always is 47482 for Jump, no need use 91809, or it can be checked by locale name compare through GetSpellInfo.
Posted by CurseForge user cordankos
https://wow.gamepedia.com/Action_slot
Really only makes sense to check 72 at most.
The operation to scan them is very fast and not heavy on the CPU. Such a small, short, simple loop is likely not slower than any cache lookups - also I wasn't aware of any action bar caching; what do you mean by this?
No real reason for time stamping or the like - even with heavy macro usage it won't fire all that frequently (no more so than maybe once or twice per modifier keypress at most). I messed around with this a bit and in the worst case got it to fire maybe 3 times per second if I was absolutely mad about smashing modifiers.
UnitExists is a leftover from other checks I was doing (that I removed for this) in addition to IsActionInRange that actually needed it - feel free to remove it.
If you feel you have a better answer to your own question, I don't see why you're not sharing it. Other people might benefit that are trying to do the same/similar things...
Posted by CurseForge user canofbutter
function GetActionSlot(id)
local id = id
for i=1, 72 do
actionType, actionid, subType = GetActionInfo(i)
if subType == "pet" and actionid == id then
return i
end
end
message("Leap is not found")
end
local leapslot, timestamp = GetActionSlot(47482), TMW.time
local leapf = CreateFrame("FRAME")
leapf:RegisterEvent("ACTIONBAR_SLOT_CHANGED")
function LeapButtonHandler(self, event, ...)
if PetHasActionBar() and TMW.time ~= timestamp and (not leapslot or leapslot == ...) then
timestamp = TMW.time
leapslot = GetActionSlot(47482)
end
end
leapf:SetScript("OnEvent", LeapButtonHandler)
function IsLeapInRange()
return (leapslot and IsActionInRange(leapslot)) or false
end
I don't have a lot of powerfull pc and I see game spikes when query slots through ACTIONBAR_SLOT_CHANGED. Function IsActionInRange has only one argument to use, its slot without unit, you can check slot in range for "focus" if there exist macro with @Focus.
Edited Dec 18, 2018
Posted by CurseForge user cordankos
Default TMW must have this feature. Would be interest in theory somehow create hidden button to use it for query action.
Posted by CurseForge user cordankos
Still not seeing anything one would call "caching" related to the action bar. Also, I don't think you intended to assign the slot location to "timestamp", that wouldn't really mean anything.
If you're using a ton of macro modifiers it would fire a lot, I suppose. For a test I filled all 6 of my action bars with macros that had modifiers. Even using my code without the timestamp check, it scanned them all repeatedly like that in around 1/8000th of a second. Try as I might, I just can't see your frame drops coming from that, but there's no reason not to do the timestamp check as you did, so that's fine.
As for your change in GetActionSlot - you don't need to check the subtype for pet - spell ids are unique
IsActionInRange can indeed take a unit - API documentation you're finding must be out of date. I rely on this a lot when checking units by nameplate.
Regardless, if you're really trying to squeeze out everything you can performance wise, skip all the event handling entirely and use the original I posted - it'll only ever scan when you load in or /reload - can't get better than that and it's what I use for all pet range checking - here's that code block:
function GetActionSlot(id)
for i=1, 72 do
if select(2, GetActionInfo(i)) == id then
return i
end
end
end
-- this only runs once on load/reload, so do a /reload if you move the button
local leapslot = GetActionSlot(47482)
function IsLeapInRange(unit)
if not leapslot then
return false
end
if not unit then
unit = "target"
end
return IsActionInRange(leapslot,unit) or false
end
As for any sort of hidden buttons, no not really: IsActionInRange accepts action bar slots from the base game only. Any new, hidden action buttons you create could not be passed to it.
Edited Dec 18, 2018
Posted by CurseForge user canofbutter
I remade your code which you sent first time because as you noticed in last message
If you're using a ton of macro modifiers it would fire a lot, I suppose
function LeapButtonHandler(self, event, ...)
leapslot = GetActionSlot(47482) -- here is each Alt Ctrl Shift will fire this event and query slots which will make for me game spikes
if not leapslot then
leapslot = GetActionSlot(91809)
end
end
So better use:
if PetHasActionBar() and TMW.time ~= timestamp and (not leapslot or leapslot == ...) then
Cache also means that, don't query slots if we know that Leap already here or saved LeapSlot has been changed in current fired slot.
OMG IsActionInRange second argument for me is big suprise, because I checked wowpedia and wowwiki, they are both telling only 1 argument. Where you looking actual API for that?
Edited Dec 18, 2018
Posted by CurseForge user cordankos
https://wow.gamepedia.com/Patch_2.0.1/API_changes#API_Changes:_Information
Been available since 2.0.1 - works 100% - use it all the time
Also, no, the code that I sent the first time (the import string) didn't have any event handling at all - it was a single run on load. We're over complicating this to handle what is really just an edge case: moving the button around without relying on a /reload. That's just unnecessary and the original snippet import string (not the readable code block - my very first reply had import strings) is more then sufficient. I never should have tried adding that bit, as it just seems to be leading to confusion and possible performance issues.
Posted by CurseForge user canofbutter
Oh, and the cache thing you're mentioning: that's EXACTLY what the first reply's import snippet did. You just with apparent misunderstanding about how TMW's snippets worked (they don't run constantly - they're run once). Using a LUA condition and actually calling IsLeapInRange() is the only thing that will run repeatedly, and that's using the action bar location of your pet's ability that was found on the snippets initial first run and never gets rescanned by that function.
Posted by CurseForge user canofbutter
Oh yea, it was 3rd msg. I guess for people who will search any solution will fair enough everything here. Well this thing is very usefully for range class with pet though nameplates. Thanks canofbutter
Posted by CurseForge user cordankos
It does seem that IsSpellInRange does indeed not work for pet spells anymore. I don't plan on implementing any workarounds in TMW to do actionbar scanning - Blizzards needs to get their stuff together and fix their API. Unforunately, as a general rule of thumb, Blizzard couldn't care less about any of their API as long as it isn't broken in a way that affects the default UI.
Posted by CurseForge user Cybeloras