Bug Reports / Suggestions!

[Greymalkin]

Bug: Typo in checkChanneling of the nested Interrupt code
Build: 6.0.3r11

Details:
The recent inconsistent issues with interruptAt and interruptsAt appear to boil down to a typo on line 709 of Probably/system/conditions/core.lua The "name" variable has an extra underscore behind it which is not letting the next line query its value. Since "name" on line 710 is getting a value returned from somewhere (I ended up tracking the "name" variable down to an old add-on bleeding variables to global which is most likely why some PE users had the issue and some didn't if they had old add-ons running) it is cascading incorrect returns, i.e. checkChanneling line 710 to checkCasting line 717 to casting.delta line 732 (where startTime, endTime, notInterruptible are being populated as nil) to casting.delta line 733 evaluating as true to casting.delta line 734 trying to evaluate castLength as (nil - nil) / 1000. This is where all of the reported "attempt to perform arithmetic on local 'endTime' (a nil value)" errors are originating from.

Fix:
Just remove the trailing underscore from the "name" variable in the checkChanneling function.

[Greymalkin]

Bug: Generic focus logic for parsing "tank" ?
Build: 6.0.3r11

Details:
I'm not sure if this is actually a bug or if I'm just completely missing the reason for logic here. (I suspect this is left over from the quick build of the healing framework from way back whenever, but I could be wrong.) I cannot see the use of "tank" as the unit for any target other than a healable/friendly one. The first step of the ProbablyEngine.raid.tank() function will check for a healable focus and return it as the result if found. However, the code in both the dsl.lua and parser.lua files checks for the existence of a focus and returns it regardless of if the focus is a friend or enemy before the ProbablyEngine.raid.tank() function is called. This becomes an issue if the player is a healer assigned to chain CC an enemy mob (or player) or if the focus becomes mind controlled so that it is no longer able to receive healing or beneficial spells.

Is a reason why the focus check is explicit in the parsing:
If yes, then add a check for canHeal('focus') to the sections in the dsl.lua and parser.lua files so that they will not return enemy players, mobs, MCs, etc. as the tank.
If no, then remove the focus check from the sections in the dsl.lua and parser.lua files and let the ProbablyEngine.raid.tank() call that follows them handle the tank value return.
Thanks my take on it anyway. Thanks!

Related code sections:
Probably/system/core/dsl.lua
Lines 144-165
Lines 166-188 (Same concern may also apply here as well.)

Probably/system/core/parser.lua
Lines 250-254

Probably/system/core/raid.lua
Lines 190-220

[Mackdaddy2887]

Bug: Generic focus logic for parsing "tank" ?
Build: 6.0.3r11

Details:
I'm not sure if this is actually a bug or if I'm just completely missing the reason for logic here. (I suspect this is left over from the quick build of the healing framework from way back whenever, but I could be wrong.) I cannot see the use of "tank" as the unit for any target other than a healable/friendly one. The first step of the ProbablyEngine.raid.tank() function will check for a healable focus and return it as the result if found. However, the code in both the dsl.lua and parser.lua files checks for the existence of a focus and returns it regardless of if the focus is a friend or enemy before the ProbablyEngine.raid.tank() function is called. This becomes an issue if the player is a healer assigned to chain CC an enemy mob (or player) or if the focus becomes mind controlled so that it is no longer able to receive healing or beneficial spells.

Is a reason why the focus check is explicit in the parsing:
If yes, then add a check for canHeal('focus') to the sections in the dsl.lua and parser.lua files so that they will not return enemy players, mobs, MCs, etc. as the tank.
If no, then remove the focus check from the sections in the dsl.lua and parser.lua files and let the ProbablyEngine.raid.tank() call that follows them handle the tank value return.
Thanks my take on it anyway. Thanks!

Related code sections:
Probably/system/core/dsl.lua
Lines 144-165
Lines 166-188 (Same concern may also apply here as well.)

Probably/system/core/parser.lua
Lines 250-254

Probably/system/core/raid.lua
Lines 190-220

As someone who predominantly heals and writes routines fit healing, if I want to target the focus, ill make 'focus' my target. If I use tank, I want it NOT use the focus, but do the rest, check for people set to that role, etc etc.

In short, do what he mentioned above and remove focus from the tank value

[Greymalkin]

As someone who predominantly heals and writes routines fit healing, if I want to target the focus, ill make 'focus' my target. If I use tank, I want it NOT use the focus, but do the rest, check for people set to that role, etc etc.

In short, do what he mentioned above and remove focus from the tank value

Hey Mack,
What I was recommending is to remove the heavy-handed focus checks from the parsing and let the ProbablyEngine.raid.tank() function handle the focus check, rather than removing the focus checks from all points of the tank evaluation completely. The focus check in the ProbablyEngine.raid.tank() function was most likely used as a safe guard against the function returning different tank values (too rapidly or at all) and causing buffing conflicts. In prior expansions some buffs would overwrite the same buffs from other players, i.e. a Shaman's Earth Shield, where if you didn't set focus on the tank you were assigned to the Earth Shield would jump among the available tanks in the raid overwriting other Shamans' Earth Shield on the tanks they were focusing. (I cringe just thinking about all those wasted casts and GCDs on both sides!) I believe this is less of a concern now that Earth Shield and the like are no longer exclusive on a target. However, if a healer is assigned to primarily keep a specific tank up, having that tank set as focus and returned when it is healable is still preferred as far as I can see. If that tank dies or gets out of range, the canHeal('focus') check will fail and the ProbablyEngine.raid.tank() function will continue through the other evaluation methods to find a new viable tank until the focus tank is back in range or revived. As a whole, I personally do not want to see the healable focus check removed from the ProbablyEngine.raid.tank() function.

Side note: Just a thought, but we could ask the devs with a feature suggestion to add a "tanks" unit that acts similar to "tank" without the focus safe guard. That way if heals / buffs need to specifically be spread around on all the tanks, the lowest health unit of all tanks at the moment would be returned. ...OR maybe make "tanks" a little more robust so we could use conditions like tanks.lowest, !tanks.buff(Riptide), etc., but that is getting more into building a full healing engine which is a story for another day!

[thefrobel]

Not sure how close PE's core is to Simulationcraft, but the verbiage looks REALLY close... Also, not sure if you would be able to, but making PE able to copy and paste the 'priority' section of individual spec dps rotations might make making optimal rotation LUAs for PE quite easy...

From Simulationcraft.org's website for a shadowpriest running COP

actions.cop_dotweave
# 	count 	action,conditions
c 	7.61 	devouring_plague,if=target.dot.vampiric_touch.ticking&target.dot.shadow_word_pain.ticking&shadow_orb=5&cooldown_react
d 	0.00 	devouring_plague,if=(buff.mental_instinct.remains<gcd&buff.mental_instinct.remains)
e 	7.38 	devouring_plague,if=(target.dot.vampiric_touch.ticking&target.dot.shadow_word_pain.ticking&!buff.shadow_word_insanity.remains&cooldown.mind_blast.remains>0.4*gcd)
f 	0.00 	mind_blast,if=glyph.mind_harvest.enabled&mind_harvest=0&shadow_orb<=2,cycle_targets=1
g 	47.87 	mind_blast,if=shadow_orb<=4&cooldown_react
h 	2.00 	shadowfiend,if=!talent.mindbender.enabled&!buff.shadow_word_insanity.remains
i 	0.00 	mindbender,if=talent.mindbender.enabled&!buff.shadow_word_insanity.remains
j 	0.00 	shadow_word_pain,if=shadow_orb=4&set_bonus.tier17_2pc&!target.dot.shadow_word_pain.ticking&!target.dot.devouring_plague.ticking&cooldown.mind_blast.remains<1.2*gcd&cooldown.mind_blast.remains>0.2*gcd
k 	7.68 	shadow_word_pain,if=shadow_orb=5&!target.dot.devouring_plague.ticking&!target.dot.shadow_word_pain.ticking
l 	7.64 	vampiric_touch,if=shadow_orb=5&!target.dot.devouring_plague.ticking&!target.dot.vampiric_touch.ticking
m 	15.34 	insanity,if=buff.shadow_word_insanity.remains,chain=1,interrupt_if=cooldown.mind_blast.remains<=0.1
n 	0.08 	shadow_word_pain,if=shadow_orb>=2&target.dot.shadow_word_pain.remains>=6&cooldown.mind_blast.remains>0.5*gcd&target.dot.vampiric_touch.remains&buff.bloodlust.up&!set_bonus.tier17_2pc
o 	0.00 	vampiric_touch,if=shadow_orb>=2&target.dot.vampiric_touch.remains>=5&cooldown.mind_blast.remains>0.5*gcd&buff.bloodlust.up&!set_bonus.tier17_2pc
p 	5.42 	halo,if=cooldown.mind_blast.remains>0.5*gcd&talent.halo.enabled&target.distance<=30&target.distance>=17
q 	0.00 	divine_star,if=cooldown.mind_blast.remains>0.5&gcd&talent.divine_star.enabled&(active_enemies>1|target.distance<=24)
r 	0.00 	cascade,if=cooldown.mind_blast.remains>0.5*gcd&talent.cascade.enabled&((active_enemies>1|target.distance>=28)&target.distance<=40&target.distance>=11)
s 	0.00 	shadow_word_pain,if=primary_target=0&(!ticking|remains<=18*0.3),cycle_targets=1,max_cycle_targets=5
t 	0.00 	vampiric_touch,if=primary_target=0&(!ticking|remains<=15*0.3),cycle_targets=1,max_cycle_targets=5
u 	16.93 	mind_spike,if=buff.shadow_word_insanity.remains<=gcd&buff.bloodlust.up&!target.dot.shadow_word_pain.remains&!target.dot.vampiric_touch.remains
v 	1.00 	mind_spike,if=((target.dot.shadow_word_pain.remains&!target.dot.vampiric_touch.remains)|(!target.dot.shadow_word_pain.remains&target.dot.vampiric_touch.remains))&shadow_orb<=2&cooldown.mind_blast.remains>0.5*gcd
w 	0.00 	mind_flay,if=set_bonus.tier17_2pc&target.dot.shadow_word_pain.remains&target.dot.vampiric_touch.remains&cooldown.mind_blast.remains>0.9*gcd,interrupt_if=(cooldown.mind_blast.remains<=0.1|cooldown.shadow_word_death.remains<=0.1)
x 	50.13 	mind_spike
y 	0.00 	shadow_word_death,moving=1
z 	0.00 	halo,if=talent.halo.enabled&target.distance<=30,moving=1
{ 	0.00 	divine_star,if=talent.divine_star.enabled&target.distance<=28,moving=1
| 	0.00 	cascade,if=talent.cascade.enabled&target.distance<=40,moving=1
} 	0.00 	shadow_word_pain,moving=1

Just a thought.

[Mackdaddy2887]

Hey Mack,
What I was recommending is to remove the heavy-handed focus checks from the parsing and let the ProbablyEngine.raid.tank() function handle the focus check, rather than removing the focus checks from all points of the tank evaluation completely. The focus check in the ProbablyEngine.raid.tank() function was most likely used as a safe guard against the function returning different tank values (too rapidly or at all) and causing buffing conflicts. In prior expansions some buffs would overwrite the same buffs from other players, i.e. a Shaman's Earth Shield, where if you didn't set focus on the tank you were assigned to the Earth Shield would jump among the available tanks in the raid overwriting other Shamans' Earth Shield on the tanks they were focusing. (I cringe just thinking about all those wasted casts and GCDs on both sides!) I believe this is less of a concern now that Earth Shield and the like are no longer exclusive on a target. However, if a healer is assigned to primarily keep a specific tank up, having that tank set as focus and returned when it is healable is still preferred as far as I can see. If that tank dies or gets out of range, the canHeal('focus') check will fail and the ProbablyEngine.raid.tank() function will continue through the other evaluation methods to find a new viable tank until the focus tank is back in range or revived. As a whole, I personally do not want to see the healable focus check removed from the ProbablyEngine.raid.tank() function.

Side note: Just a thought, but we could ask the devs with a feature suggestion to add a "tanks" unit that acts similar to "tank" without the focus safe guard. That way if heals / buffs need to specifically be spread around on all the tanks, the lowest health unit of all tanks at the moment would be returned. ...OR maybe make "tanks" a little more robust so we could use conditions like tanks.lowest, !tanks.buff(Riptide), etc., but that is getting more into building a full healing engine which is a story for another day!

Yes. But if you were assigned to heal just a certain tank, then set him as focus, and in the profile put:
{"Earth shield", {"!focus.buff", "focus.exists"}, "focus"},
{"Earth shield", {"!tank.buff", "!focus.exists"}, "tank"},

That way you can cast earth shield on your designated target. OR if you don't have one, it'll assign it to the currently active tank (however the function works).

The reason I request this is simple, I want to have a tank healing section, as "lowest" doesn't always meet my needs (for example I'd rather let the 5% hp DPS die than the 10% tank, but your focus is the 80% tank). Now what you can tell me is to make a targeting and focus macro to auto focus the currently active tank, but that defeats the purpose of why you wanted the focus apart of the tank function to begin with...

[Greymalkin]

Yes. But if you were assigned to heal just a certain tank, then set him as focus, and in the profile put:
{"Earth shield", {"!focus.buff", "focus.exists"}, "focus"},
{"Earth shield", {"!tank.buff", "!focus.exists"}, "tank"},

That way you can cast earth shield on your designated target. OR if you don't have one, it'll assign it to the currently active tank (however the function works).

The reason I request this is simple, I want to have a tank healing section, as "lowest" doesn't always meet my needs (for example I'd rather let the 5% hp DPS die than the 10% tank, but your focus is the 80% tank). Now what you can tell me is to make a targeting and focus macro to auto focus the currently active tank, but that defeats the purpose of why you wanted the focus apart of the tank function to begin with...

Oh, I see your logic and it appears completely valid. I guess I just like having that healable focus fail safe in there because it always has been, because I like being able to specify the main recipient of my heals, or because I haven't set focus specific lines in the CRs to account for it not using focus. >.> ...or maybe all three? =D

As for a tank healing section, PE could use a bit of work on the healing front as a whole. I would love to see a prioritization of tanks > heals > dps when evaluating "lowest" rather than absolute heath in raid or a method for checking/targeting units/roles with/without specific buffs for spell preparation/synergy (Wow, that was an over use of slashes!), but that goes back to the devs having time to build a full healing engine framework in PE.

I'm not sure what you mean by "make a targeting and focus macro" for the tanks, but that just sounds dirty ...and not in the fun kind of way!

[Mackdaddy2887]

Oh, I see your logic and it appears completely valid. I guess I just like having that healable focus fail safe in there because it always has been, because I like being able to specify the main recipient of my heals, or because I haven't set focus specific lines in the CRs to account for it not using focus. >.> ...or maybe all three? =D

As for a tank healing section, PE could use a bit of work on the healing front as a whole. I would love to see a prioritization of tanks > heals > dps when evaluating "lowest" rather than absolute heath in raid or a method for checking/targeting units/roles with/without specific buffs for spell preparation/synergy (Wow, that was an over use of slashes!), but that goes back to the devs having time to build a full healing engine framework in PE.

I'm not sure what you mean by "make a targeting and focus macro" for the tanks, but that just sounds dirty ...and not in the fun kind of way!

This may not be exactly right as I'm reciting from the top of my head, but (going to cheat and not use structure):
/target enemy dead noexists, "target.dead" "! Target.exists"},
{"/focs targettarget", "target.enemy"},

Targets enemy if you don't have a target etc, focuses at the targetstargeg if the target is an enemy. (So if you target an ally or npc or something it won't focus it's target, you want to focus the bosses target, basically the currently active tank.
Put these lines at thy very very last of your routine.
---------------

Anyways, if focus is removed, (or lowered in priority) from the tank function, you can have a specific heal section for the tanks with out specifying which.

Which then, if you still want focus specific stuff, just have lines in your CR that specifies focus as thy target, like the example above. You can use both.
Tank for general tank healing, and focus for your specific focus needs.
---------------------

Also, lowest needs some work. Example:

The only issue with "lowest" is, well example:
Boss: Ko'ragh
Your assigned to raid healing, lets say shaman is assigned to focus the hunter whos soaking in the center

Now lets say the raid size is 10man:
raid1=hunter
raid2-10=everyone else

Now imagine:
Raid1 hp=25%
raid2-10 hps are..50-70%

your job, blanket the raid with rejuvs/germination, raid heal, as per your assignment.

Except... the lowest is, and will probably stay, the hunter. so the Combat routine casts rejuv/germination on the hunter, then does nothing because the "lowest" i.e. the hunter, already has the HoTs.


To account for this, im my resto profile I set up a raid and party mode. Where is force blankets the raid. (obviously it will focus the lowest HP first and foremost always)


(Check out the druid cr of mine if you have time,

[Greymalkin]

Bug: unit.name errors when unit doesn't exist
Build: 6.0.3r11

Details:
The registered condition "name" tries to return an evaluation on the target arg whether the unit of that target arg exists or nor. This causes PE to stream errors when using conditions like "target.name(sapper)" when nothing is currently targeted.

Fix:
Possibly use a default false return and wrap the evaluation return with an UnitExists(target) check like some of the other registered conditions have.

if UnitExists(target) then
return UnitName(target):lower():find(expectedName:lower()) ~= nil
end
return false

[Hackinte]

haha, well, three things that would be super...

Feature: Allow for unit.area().friendly
Details:
Let us check for friendly units in an area.

Feature: lowest.withbuff(x)
Details:
For better healing, a way to cast hots like riptide on the lowest unit that doesn't already have it.

and

Feature: Dispel / Interrupt whitelists
Details:
Create custom tables for things to interrupt, and only interrupt those things.

Added unit.area().friendly

The other two I see the value in but are quite a ways back in the list of things to do.

[Hackinte]

Would be nice if

Balance.moon
Balance.Sun

We're fixed

Fixed! Added unit.eclipse and fixed unit.solar and unit.lunar

unit.eclipse Will be a value from 0 to 100 in the current eclipse direction.
unit.lunar Will return true if the unit is in the lunar phase.
unit.solar Will return true if the unit is in the solar phase.

[Hackinte]

player.behind doesn't seem to work,

Fixed

10chars

[Hackinte]

copy/past from my previous post.

Build: CURRENT (as of 12/27)

Also..

TO whom it may concern:

modifier.last(insert spell here)

Doesnt work per se, the (insert spell here) part doesnt work right. for instance:
using this test code:

A. { "!Soothing Mist", { "lowest.health <= 100", "!player.moving", "!modifier.last" }, "lowest" }, -- Soothing Mist
B. { "!Soothing Mist", { "lowest.health <= 100", "!player.moving", "!modifier.last(Soothing Mist)" }, "lowest" }, -- Soothing Mist

you would think it funcitons the same. But code B SPAMS soothing mist, even though it was indeed the last spell cast. Whereas Code A only casts Soothing Mist if it WASNT cast last.



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

ALSO:

test code2:

{ "Surging Mist", { "player.casting","lowest.health <= 100" }, "lowest" }, -- Surging Mist
{ "Surging Mist", { "player.casting(Soothing Mist)","lowest.health <= 100" }, "lowest" }, -- Surging Mist
{ "Surging Mist", { "player.casting(I Love Boobs)","lowest.health <= 100" }, "lowest" }, -- Surging Mist

all work the EXACT same. Doesnt Matter what you put in (), it only checks to see if the player is casting. I tried "player.spell(Soothing Mist).casting, but that always returns false and the spell wont cast. Some clarity on how the conditions work would be nice.

Fixed

modifier.last IS DEPRECATED, YOU WILL GET A WARNING.

Use lastcast instead!
Examples:

{ "Spell", "lastcast(Other Spell)" }
{ "Spell", "!lastcast" }

[Hackinte]

Bug:

Crash PE totally on Orgron Twins using Nocarriers monk profile - seems to be when the Boss is doing its whirlwind routine.

Interface\AddOns\Probably\system\conditions\core.lua:734: attempt to perform arithmetic on local 'endTime' (a nil value)
Count: 99

Call Stack:
[C]: ?
Interface\AddOns\Probably\system\conditions\core.lua:734: in function `casting.delta'
Interface\AddOns\Probably\system\conditions\core.lua:771: in function <Interface\AddOns\Probably\system\conditions\core.lua:767>
Interface\AddOns\Probably\system\core\dsl.lua:90: in function <Interface\AddOns\Probably\system\core\dsl.lua:20>
(tail call): ?
Interface\AddOns\Probably\system\core\parser.lua:271: in function `table'
Interface\AddOns\Probably\system\timers\rotation.lua:71: in function `cycle'
Interface\AddOns\Probably\system\timers\rotation.lua:130: in function `event'
Interface\AddOns\Probably\system\core\timer.lua:16: in function <Interface\AddOns\Probably\system\core\timer.lua:11>

I'm not sure why this happened, but I've added sanity checks to make sure it doesn't.

[Hackinte]

Bug: unit.name errors when unit doesn't exist
Build: 6.0.3r11

Details:
The registered condition "name" tries to return an evaluation on the target arg whether the unit of that target arg exists or nor. This causes PE to stream errors when using conditions like "target.name(sapper)" when nothing is currently targeted.

Fix:
Possibly use a default false return and wrap the evaluation return with an UnitExists(target) check like some of the other registered conditions have.

if UnitExists(target) then
return UnitName(target):lower():find(expectedName:lower()) ~= nil
end
return false

Fixed

10chars