Skip to content

ravagane fix #1467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hillerstorm merged 3 commits into from
May 29, 2025
Merged

ravagane fix #1467

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c2bff6a
ravagane fix
May 4, 2025
da79ab7
clean up ravagane code
kayla-glick May 11, 2025
506f029
Merge pull request #7 from wowsims/ravagane-tweaks
Adamrch May 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 52 additions & 25 deletions sim/common/sod/item_effects/phase_8.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,13 +748,17 @@ func init() {
// https://www.wowhead.com/classic/item=240919/ravagane
// Chance on hit: You attack all nearby enemies for 9 sec causing weapon damage plus an additional 200 every 1.5 sec.
// Confirmed PPM 0.8
itemhelpers.CreateWeaponProcAura(Ravagane, "Ravagane", 0.8, func(character *core.Character) *core.Aura {

core.NewItemEffect(Ravagane, func(agent core.Agent) {
character := agent.GetCharacter()
tickPeriod := time.Millisecond * 1500

tickSpell := character.RegisterSpell(core.SpellConfig{
ActionID: core.ActionID{SpellID: 1231546},
SpellSchool: core.SpellSchoolPhysical,
DefenseType: core.DefenseTypeMelee,
ProcMask: core.ProcMaskMeleeMHSpecial,
Flags: core.SpellFlagNoOnCastComplete | core.SpellFlagPassiveSpell,
Flags: core.SpellFlagNoOnCastComplete | core.SpellFlagPassiveSpell | core.SpellFlagSuppressEquipProcs,

DamageMultiplier: 1,
ThreatMultiplier: 1,
Expand All @@ -769,7 +773,6 @@ func init() {
})

whirlwindSpell := character.RegisterSpell(core.SpellConfig{
ActionID: core.ActionID{SpellID: 1231547},
SpellSchool: core.SpellSchoolPhysical,
ProcMask: core.ProcMaskMeleeMHSpecial,
Dot: core.DotConfig{
Expand All @@ -785,36 +788,60 @@ func init() {
},
})

return character.RegisterAura(core.Aura{
Label: "Ravagane Bladestorm",
Duration: time.Second * 9,
Icd: &core.Cooldown{
Timer: character.NewTimer(),
Duration: time.Second * 8,
},
whirlwindAura := character.RegisterAura(core.Aura{
Label: "Ravagane Bladestorm Disable Autos",
ActionID: core.ActionID{SpellID: 1231547},
Duration: time.Second * 9 + core.SpellBatchWindow, // Allows 6th and final tick
Copy link
Collaborator

@kayla-glick kayla-glick May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually thinking that we should not expose this aura to the APL and should instead expose the AOEDoT's aura. Then have that aura control the disabling/re-enabling of autos and cancelling the aura in the APL both removes the DoT so that it won't hit, plus it will then go ahead and re-enable autos. I don't really see a need for this aura to exist when we can and should be using that one?

Copy link
Contributor Author

@Adamrch Adamrch May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't get much of anything to work right without using this aura. A ton of issue popped up for me when try to avoid it. If you can get the original set of auras to work better and fix the issues I pointed out I would gladly go for it. (I can't really explain why I need this but I struggled to fix the issues without it)

MaxStacks: 6,
OnGain: func(aura *core.Aura, sim *core.Simulation) {
if !aura.Icd.IsReady(sim) {
return
}
aura.Icd.Use(sim)

whirlwindSpell.AOEDot().Apply(sim)
character.AutoAttacks.CancelAutoSwing(sim)
character.AutoAttacks.AllowAutoSwing(sim, false)
},
OnRefresh: func(aura *core.Aura, sim *core.Simulation) {
if !aura.Icd.IsReady(sim) {
return
}
aura.Icd.Use(sim)
// Delay Disabling of Auto to allow Extra attacks to go out
core.StartDelayedAction(sim, core.DelayedActionOptions{
DoAt: sim.CurrentTime + time.Millisecond * 50, // Exact amount of time unknown
OnAction: func(s *core.Simulation) {
if aura.IsActive() {
character.AutoAttacks.AllowAutoSwing(sim, false)
}
},
})
Copy link
Collaborator

@kayla-glick kayla-glick May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a problem if a swing timer happens to fall within this window right?

Copy link
Contributor Author

@Adamrch Adamrch May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, we don't know exactly at what point between 0 and 1.5 the auto attack gets disabled, but it's not disabled immediately after the first hit. Suggestions?


whirlwindSpell.AOEDot().ApplyOrReset(sim)
character.AutoAttacks.CancelAutoSwing(sim)
core.StartPeriodicAction(sim, core.PeriodicActionOptions{
Period: tickPeriod,
NumTicks: 6,
Priority: core.ActionPriorityDOT,
TickImmediately: false,
OnAction: func(sim *core.Simulation) {
if aura.IsActive() && aura.TimeActive(sim) >= tickPeriod { // Prevent previous proc from casting on Refresh
aura.RemoveStack(sim)
whirlwindSpell.AOEDot().TickOnce(sim)
}
},
})
Copy link
Collaborator

@kayla-glick kayla-glick May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to use a separate periodic action here versus just using the AOEDot's aura?

Copy link
Contributor Author

@Adamrch Adamrch May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try to implement again using the AOEDot Aura but dot damage was not going away at the right time when bladestorm aura was canceled in my branch. (could you help take a look and replace periodic action with the AoEDot apply() ?) I dont mind changing that if it works but it seemed to have issues in my branch.

},
OnExpire: func(aura *core.Aura, sim *core.Simulation) {
whirlwindSpell.AOEDot().Cancel(sim)
character.AutoAttacks.EnableAutoSwing(sim)
if aura.GetStacks() == 0 { // This Check is needed for some cases where On Expire triggers right after On Refresh
character.AutoAttacks.AllowAutoSwing(sim, true)
}
},
})

triggerAura := core.MakeProcTriggerAura(&character.Unit, core.ProcTrigger{
Name: "Ravagane Bladestorm Trigger",
Callback: core.CallbackOnSpellHitDealt,
Outcome: core.OutcomeLanded,
ProcMask: core.ProcMaskMelee,
SpellFlagsExclude: core.SpellFlagSuppressWeaponProcs, // Trigger is Chance on Hit so suppress if triggering spell does not proc Chance on Hit
PPM: 0.8,
ICD: 8 * time.Second,
Handler: func(sim *core.Simulation, _ *core.Spell, result *core.SpellResult) {
whirlwindAura.Activate(sim)
whirlwindAura.SetStacks(sim, 6)
},
})

character.ItemSwap.RegisterProc(Ravagane, triggerAura)
})

// https://www.wowhead.com/classic/item=241002/remnants-of-the-red
Expand Down
20 changes: 19 additions & 1 deletion sim/core/attack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ type WeaponAttack struct {
curSwingSpeed float64
curSwingDuration time.Duration
enabled bool
allowed bool
Copy link
Collaborator

@kayla-glick kayla-glick May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Adamrch can you explain why we need the #AllowAutoSwing method and the allowed state versus what exists now?

Copy link
Contributor Author

@Adamrch Adamrch May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exists now is the player controlled startattack / stopattack. Allowed is a just a flag that causes the swing timer to reset and no auto to go out as seen in game. (the delay at the end of the whirlwind for the first auto is consistant with the swingtimer filling and reseting during the whirlwind)

}

func (wa *WeaponAttack) getWeapon() *Weapon {
Expand Down Expand Up @@ -330,6 +331,10 @@ func (wa *WeaponAttack) setWeapon(sim *Simulation, weapon Weapon) {
func (wa *WeaponAttack) trySwing(sim *Simulation) time.Duration {
if sim.CurrentTime < wa.swingAt {
return wa.swingAt
} else if !wa.allowed {
wa.swingAt = sim.CurrentTime + wa.curSwingDuration // Auto Attack Fails due to being disabled by an item or effect (e.g. Ravagane)
sim.rescheduleWeaponAttack(wa.swingAt)
return wa.swingAt
}
return wa.swing(sim)
}
Expand Down Expand Up @@ -467,6 +472,8 @@ type AutoAttacks struct {
AutoSwingMelee bool
AutoSwingRanged bool

AutoSwingDisabled bool

IsDualWielding bool

character *Character
Expand Down Expand Up @@ -676,6 +683,10 @@ func (aa *AutoAttacks) reset(sim *Simulation) {
aa.oh.enabled = false
aa.ranged.enabled = false

aa.mh.allowed = true
aa.oh.allowed = true
aa.ranged.allowed = true

aa.mh.swingAt = NeverExpires
aa.oh.swingAt = NeverExpires

Expand Down Expand Up @@ -798,6 +809,13 @@ func (aa *AutoAttacks) EnableAutoSwing(sim *Simulation) {
aa.EnableRangedSwing(sim)
}

// Allow the auto swing action for the iteration
func (aa *AutoAttacks) AllowAutoSwing(sim *Simulation, isAllowed bool) {
aa.mh.allowed = isAllowed
aa.oh.allowed = isAllowed
aa.ranged.allowed = isAllowed
}

func (aa *AutoAttacks) EnableMeleeSwing(sim *Simulation) {
if !aa.AutoSwingMelee {
return
Expand All @@ -821,7 +839,7 @@ func (aa *AutoAttacks) EnableMeleeSwing(sim *Simulation) {
}
}

if !aa.IsDualWielding && aa.oh.enabled {
if (!aa.IsDualWielding && aa.oh.enabled) {
sim.removeWeaponAttack(&aa.oh)
aa.oh.enabled = false
}
Expand Down
Loading