simulationcraft / simc

Simulationcraft engine/GUI
GNU General Public License v3.0
1.42k stars 704 forks source link

[Druid] Guardian Sim Defects #1723

Closed navv1234 closed 9 years ago

navv1234 commented 9 years ago

Originally reported on Google Code with ID 1724

I did a first-pass code review of the Druid module and found the following defects:

buffs_t:
- How is Lacerate a "buff"?  Is this the Berserk proc (Mangle CD Reset)?  If it is
it should be removed.
- Remove Frenzied Regeneration.
- Add tier15_2pc_guardian (http://www.wowhead.com/spell=138217)
- Add might_of_ursoc (http://www.wowhead.com/spell=106922)
- Add tooth_and_claw (http://www.wowhead.com/spell=135286)
- Add cenarion_ward (http://www.wowhead.com/spell=102351)

glyphs_t:
- Add Survival Instincts (http://www.wowhead.com/item=45601)
- Add Might of Ursoc (http://www.wowhead.com/item=45603)

procs_t:
- Add mangle_bear_cd_reset - or whatever you would like to call it.
- Add tooth_and_claw

rngs_t:
- Based on what I see I think tooth_and_claw and mangle_bear_cd_reset should be in
here as well.

Frenzied Regeneration:
- Is being treated as an attack instead of a heal in the sim.
- Actual formula: 2.2*(MAX((AP - 2*Agi), 2.5*Sta))
- Variable Rage consumption not implemented HealAmt=MIN((RageConsumed/60),1)*(FRValue)

Lacerate:
- Remove the Mangle CD reset on bleed tick
- Add 25% chance to reset Mangle CD when it hits.

Swipe:
- Does 246 + 0.22487*AP in damage.  Does not use Weapon Damage.
- Buff vs bleeding targets remains.

Thrash_Bear:
- Does 1232 + 0.191*AP in damage on impact, which ignores armor.  Does not use Weapon
Damage.
- Each tick does 686 + 0.141*AP in damage.
- Add 25% chance to reset Mangle CD when it hits.

Bear Melee:
- Rage gain is only 10.85 per swing.

Faerie Fire:
- Add 25% chance to reset Mangle CD when it hits.

Enrage:
- Add 4t15 Guardian Set Bonus.

Reported by arrieelle on 2013-05-12 16:01:44

navv1234 commented 9 years ago
Just an fyi, the lacerate "buff" is used to keep track of the number of stacks of the
dot.

Reported by swbusche on 2013-05-12 21:40:09

navv1234 commented 9 years ago
Ok, cool.  I wasn't sure so I thought I'd ask :)

Reported by arrieelle on 2013-05-13 18:37:45

navv1234 commented 9 years ago
buff.frenzied_regeneration might as well stay and be repurposed for the glyphed version,
frenzied_regeneration being added to glyphs_t as well of course.

I've added lacerate stacks being tracked via state data and mangle CD being affected
by  Berserk in r16436.

Reported by jorgster247123 on 2013-05-14 23:29:34

navv1234 commented 9 years ago
Should be able to remove the buff all together an expose the stacks with an expression.
Otherwise at least make it a debuff, as it's on the target no? 

Reported by swbusche on 2013-05-15 00:01:00

navv1234 commented 9 years ago
It is a debuff already. I had removed the buff entirely but realized I don't have the
know how to make an expression that returns the stack count.

Adding a generic dot.foo.stack expression that checks for and returns the presence
of foo_stack in the target data seems like a good all around solution to me.

Reported by jorgster247123 on 2013-05-15 00:56:52

navv1234 commented 9 years ago
Also, I've added Thick Hide's damage reduction in r16438.

Reported by jorgster247123 on 2013-05-15 01:02:25

navv1234 commented 9 years ago
Added 2pT15 bonus in r16439. Also it seems Frenzied Regeneration glyph is already fully
implemented so nothing about that needs to be changed.

Reported by jorgster247123 on 2013-05-15 02:16:38

navv1234 commented 9 years ago
yay. more tanks! 

just for reference:
https://docs.google.com/spreadsheet/ccc?key=0AuwcTdFIcJZydE02WXNTd3pCVmd1eExuNzFRLWc5OEE#gid=0
it includes the dimishing return values and agi to dodge ratios. I'd assume that they
are not up to date right now.

Reported by dr.max.moellers on 2013-05-15 06:58:48

navv1234 commented 9 years ago
Well the FR glyph is completely useless.  So...yay?

The Coeffs are all correct in Tang's (now defunct) spreadsheet.  Ignore it for anything
else.  For reference they are also all listed in: http://theincbear.com/forums/viewtopic.php?f=9&t=697

Reported by arrieelle on 2013-05-15 07:24:19

navv1234 commented 9 years ago
So if the values in the spreadsheet are correct I take it diminished_kfactor should
equal 1.222 and diminished_dodge_cap should equal 1.502? Could very well be interpreting
wrong.

Reported by jorgster247123 on 2013-05-15 08:26:41

navv1234 commented 9 years ago
looks good to me. I'd recommend to test it with a high dodge/agi profile and compare
ingame values with our estimates.
Also. you should set the base miss/parry/dodge values (take a look at the sc_warrior.cpp::init_Base_stats
for comparison)

Reported by dr.max.moellers on 2013-05-15 08:38:20

navv1234 commented 9 years ago
Well it's perfectly accurate to my character sheet in-game with feral gems (pretty heavy
agi) after adding the base dodge so I would assume it's good!

Reported by jorgster247123 on 2013-05-15 08:52:11

navv1234 commented 9 years ago
I've updated Frenzied Regeneration in r16440. It should do the correct amount of healing
(MAX(2.2*(AP - 2*Agi), 2.5*Sta)) and is no longer an attack nor does it (for some reason)
deal damage.

Variable rage consumption was already and still is implemented.

It's still implemented as a "health gain" instead of a heal so perhaps that could be
changed.

Reported by jorgster247123 on 2013-05-15 08:59:33

navv1234 commented 9 years ago
I've added Tooth and Claw and correct the rage generation from autoattacks in r16457.

Reported by jorgster247123 on 2013-05-16 08:23:35

navv1234 commented 9 years ago
In the most recent Simc script for guardian, you need to add the talent.enabled check
to Nature's vigil. If you try to sim a HoTW specced bear at the moment it keeps trying
to cast a spell that you don't have.

The check for actions+=/thrash_bear,if=debuff.weakened_blows.remains<3 clearly doesn't
work as during incarnation the sim tries to spam thrash and in fact, we don't really
use weakened blows as a check to see if we should cast thrash, instead just keeping
thrash up is what we should look at.  (thrash = 16 sec, weakened blows=30, so keeping
thrash up keeps weakened blows up)

The line which states to perform an FR every time you are below 40% health is not constructive
as you will always be below 40% health in this simulation as you are not healed, it
triggers almost 250 times and savage defence only once. A similar thing goes for the
other frenzied regeneration line as it consumes way too much rage and basically activates
all the time and prevents any tooth and claw usage by the sim. Until there is some
way of simulating healers healing, I would leave frenzied regeneration out of the simulation.(a
similar sort of logic would apply to NS Healing Touch, which is pretty much used on
cd in the sim, something that is completely unrealistic)

the default flask should be spring blossoms, not earth (or alternatively elixirs of
armor+crit)

There were a couple of other fixes I made vis-a-vis lacerate uptime and flask, I have
attached the proposed action list for your consideration.  

Reported by crlac.aquaman on 2013-06-17 14:45:27


navv1234 commented 9 years ago
Thanks for the detailed analysis.  Please let me know if you would like commit access.

Reported by natehieter on 2013-06-17 14:47:50

navv1234 commented 9 years ago
Left some stuff in the previous one that I didn't want left in (i.e. the NS stuff) its
fixed in this version.

Reported by crlac.aquaman on 2013-06-17 15:09:09


navv1234 commented 9 years ago
The point of the thrash_bear line is for it to prioritize it above all else in any situation
where the debuff isn't up. In practice this means it will only have effect once per
fight (on pull) but it still servers a purpose.

I see your point on FR (I was aware of this when I implemented it, just went with it
anyway) and I will disable it by default. As far as Nature's Swiftness, I've made it
so it only triggers when the druid takes 70% or more of their health in damage over
5 seconds, this causes it to trigger somewhat close to cooldown (on average 10-30 seconds
depending on gear).

The same thing could potentially be done with FR but there would be some issues with
having multiple lines using this sort of conditional (they would tend to all trigger
at the same time).

Reported by jorgster247123 on 2013-07-27 20:59:23

navv1234 commented 9 years ago
Fixed mangle CD reset in r17049, added tier15_4pc_tank in r17050.

Still have to add the t16 bonuses, but otherwise I believe everything in the originally
ticket as been taken care of except for implementing Cenarion Ward and possibly Swipe
damage.

Reported by jorgster247123 on 2013-07-28 17:10:39

navv1234 commented 9 years ago
I've fixed all the remaining problems in this issue (as well as a handful of other things)
and as such I'll be closing this issue now. If you have any more feedback on what needs
to tweaked or implemented feel free to submit a new issue.

Reported by jorgster247123 on 2013-09-24 11:52:56