Implementing the bear sim

[emsimpson92]

No description provided.

[kayla-glick]

Were these removed by mistake?

[kayla-glick]

This changed since you put up the PR but we should modify the new CD FlatModifier field for effects like this now. CDs store the original Duration, a cumulative flat modifier like this one, and then a cumulative multiplier, then the sim calculates the real duration from those like the game does

Suggested change

	druid.Enrage.CD.Duration -= time.Second * 30
	druid.Enrage.CD.FlatModifier -= time.Second * 30

[kayla-glick]

Oh also since this is also a rune, we'll want to follow the pattern of creating a function that does the aura check / creates an aura to handle the set bonus. Following the other example, create the aura, then in the OnInit you can make the change to Enrage's CD FlatModifier

[kayla-glick]

This will double dip without the aura code here. What you can do for this bonus is in the OnInit, grab the old OnGain and OnExpire for bear form, then override them, calling the old ones and then also adding the threat mod here:

OnInit: func(aura *core.Aura, sim *core.Simulation) {
  oldOnGain := druid.BearFormAura.OnGain
  druid.BearFormAura.OnGain = func(aura *core.Aura, sim *core.Simulation) {
    oldOnGain(aura, sim)
    druid.Pseudostats.ThreatMultiplier += 0.20
  }

 oldOnExpire := druid.BearFormAura.OnExpire
 druid.BearFormAura.OnExpire = func(aura *core.Aura, sim *core.Simulation) {
   oldOnExpire(aura, sim)
   druid.Pseudostats.ThreatMultiplier -= 0.20
 }
}

[kayla-glick]

This also needs to have the aura stuff here because of the tier bonus + shoulder runes, so you could move this bonus crit rating into the OnInit here and that should be fine.

Another way we could do this is to override the ApplyEffects of the affected spells so that before calling the old one we check if lacerate is active then add 5% bonus crit to the spell, call the original ApplyEffects, then remove the bonus crit. Basically what you did in the spell code but all in one place.

You would also want to add a check in this function that returns if the druid doesn't have the Lacerate rune

// This is all inside of the aura's OnInit

for _, spell := range []*DruidSpell{druid.MangleBear, druid.SwipeBear, druid.Maul, druid.Lacerate} {
  if spell == nil {
    continue
  }

  oldApplyEffects := spell.ApplyEffects
  spell.ApplyEffects = func(sim *core.Simulation, target *core.Unit, spell *core.Spell) {
    bonusCrit := 0.0
    if druid.LacerateBleed.Dot(target).GetStacks() > 0 {
      bonusCrit = 5 * core.CritRatingPerCritChance
    }

    spell.BonusCritRating += bonusCrit
    oldApplyEffects(sim, target, spell)
    spell.BonusCritRating -= bonusCrit
  }
}

[kayla-glick]

If you implemented this here then you would want to remove the similar code from the mangle, swipe, lacerate, and maul spells

[kayla-glick]

This stuff also needs to come back but bumping this set bonus as unfinished

[kayla-glick]

My suggestion for this would be to bake a target count into lacerate's code and default it to. Maybe you can use an attribute on the druid to track the current target count that lacerate should hit. Sounds like there's two different effects that make lacerate affect multiple targets?

[kayla-glick]

Same comment as above with lacerate ^ bake a target count in by default, start with the current target then N times use the next target unit to apply the effects too