target_ready is not being evaluated on a singlet target sim

seanpeters86 commented 4 years ago

Describe the bug

In the priest module we use target_ready to add a minimum range for the spell which works perfectly if you move a target or add adds into the fight. However strangely this does not work on a pure single target default sim and it just casts Shadow Crash as normal, even though the actor is for sure on top of the boss. Adding in an add makes it work correctly.

Expected behavior

target_ready should be respected in a sim even without adds

To Reproduce

Profile:

debug=1

priest="Base"
level=60
role=spell
position=back
spec=shadow
covenant=necrolord
talents=2121321
spec=shadow
default_actions=0

actions=shadow_crash
actions+=/mind_flay

head=confidants_favored_cap,id=183021,bonus_id=6807
neck=charm_of_eternal_winter,id=183040,bonus_id=6807
shoulder=shawl_of_the_penitent,id=183020,bonus_id=6807
back=crest_of_the_legionnaire_general,id=183032,bonus_id=6807
chest=robes_of_the_cursed_commando,id=182998,bonus_id=6807
wrists=acolytes_velvet_bindings,id=183017,bonus_id=6807
hands=impossibly_oversized_mitts,id=183022,bonus_id=6807
waist=shadewarped_sash,id=183004,bonus_id=6807
legs=leggings_of_lethal_reverberations,id=182981,bonus_id=6807
feet=slippers_of_the_forgotten_heretic,id=182979,bonus_id=6807
finger1=hyperlight_band,id=183038,bonus_id=6807
finger1=ritualists_treasured_ring,id=183037,bonus_id=6807
trinket1=,id=174500,gem_id=168639,bonus_id=4824/1517/4786/6514
trinket2=,id=175722,bonus_id=6707,drop_level=54
main_hand=sinful_gladiators_blade,id=176000
off_hand=sinful_gladiators_focus,id=176008

single_actor_batch=1
iterations=10000

This actor will still cast Shadow Crash and it will hit the target with shadow_crash_damage even though the actor is set at 0yds away from the target right now: https://github.com/simulationcraft/simc/blob/shadowlands/engine/class_modules/priest/sc_priest_shadow.cpp#L2166

If you add in raid_events+=/adds,count=3,first=45,cooldown=45,duration=10,distance=5 to the sim it no longer casts shadow crash at all (expected)

Additional information

latest shadowlands branch

navv1234 commented 4 years ago

Target_ready() will be called as part of the normal APL scanning process. You can trivially see this in action_t:.action_ready() which is called by the APL scanning function in player_t (select_action).

The problem is that your priest is 27 yards away from the target. This is because you set the distance in a weird place (init_spells_shadow()), and if you look at priest_t::init_spells(), shadow one gets called first. Neither init_spells_discipline() or init_spells_holy() guard against spec in any way, so they simply overwrite your shadow setting.

seanpeters86 commented 4 years ago

I don't think that's true. I can clearly see the actor at 0, 0 in my sims.

navv1234 commented 4 years ago

I added a debug print to shadow crash outputting current.distance. It says 27. In addition, you can see this in the debug log:

0.000 Player 'Base' generic base stats: ... distance=27.0 ...

seanpeters86 commented 4 years ago

Okay I can't reproduce that line i saw earlier, but nonetheless why is adding adds to the sim breaking the logic?

If what you are saying is true having adds in the sim shouldnt change the fact that it is able to cast shadow crash but it clearly stops it from doing so.

seanpeters86 commented 4 years ago

Okay I've added a check for the spec here: https://github.com/simulationcraft/simc/commit/485dae1367186010844efbed53e48d097b28a8a2

But I'm still getting the same results as I posted above, giving the sim adds in this simple sim no longer makes it able to cast Shadow Crash. I would expect adds being given to the sim not changing the behavior of distance.

With the recent commit it should be able to cast shadow crash because the actor should be at 8yds as shadow now

vituscze commented 4 years ago

Using raid_events with distance= turns on distance targeting, which is why you're seeing the sim behave differently.

simulationcraft / simc