Skip to content

Conversation

@ToxicKevinFerm
Copy link

No description provided.

if (debuffs.criticalMass || debuffs.shadowAndFlame) {
debuffStats = debuffStats.addPseudoStat(PseudoStat.PseudoStatSpellCritPercent, 5);
}
// const debuffs = this.player.sim.raid.getDebuffs();
Copy link

Choose a reason for hiding this comment

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

These comments can just be removed

Choose a reason for hiding this comment

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

If there are no more debuffs that boost character stats against that target in MoP, then we should remove this method entirely and also remove the "Debuffs" part of the character stats breakdown.

Copy link

@1337LutZ 1337LutZ left a comment

Choose a reason for hiding this comment

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

Found a bug in the stacking Armor reduction

Copy link

@1337LutZ 1337LutZ left a comment

Choose a reason for hiding this comment

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

Fixed the Armor bug & some UI input state bugs

Copy link

@NerdEgghead NerdEgghead left a comment

Choose a reason for hiding this comment

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

Looks like there are also test failures to resolve.

Comment on lines 104 to 106
func MortalWoundsArua(target *Unit) *Aura {
return majorHealingReductionAura(target, "Mortal Wounds", 115804, 0.25)
}

Choose a reason for hiding this comment

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

Typo, should be MortalWoundsAura()

Copy link
Author

Choose a reason for hiding this comment

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

True, fixed

Comment on lines 167 to 172
func ScheduledMajorArmorAura(aura *Aura, options PeriodicActionOptions, raid *proto.Raid) {
aura.OnReset = func(aura *Aura, sim *Simulation) {
aura.Duration = NeverExpires
StartPeriodicAction(sim, options)
}
}

Choose a reason for hiding this comment

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

I don't think this is needed anymore?

Copy link
Author

Choose a reason for hiding this comment

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

Deleted

Comment on lines 20 to 21
// Trauma in this case should *never* be overwritten
// as its duration from 'MakePermanent' should make it non overwritable by 1 min duration mangles

Choose a reason for hiding this comment

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

Update comment to reflect the new test cases being used.

Copy link
Author

Choose a reason for hiding this comment

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

Updated

Comment on lines 50 to 51

// In this case mangle should overwrite trauma as mangle will give a greater duration

Choose a reason for hiding this comment

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

Update comment here as well.

Copy link
Author

Choose a reason for hiding this comment

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

Updated

if (debuffs.criticalMass || debuffs.shadowAndFlame) {
debuffStats = debuffStats.addPseudoStat(PseudoStat.PseudoStatSpellCritPercent, 5);
}
// const debuffs = this.player.sim.raid.getDebuffs();

Choose a reason for hiding this comment

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

If there are no more debuffs that boost character stats against that target in MoP, then we should remove this method entirely and also remove the "Debuffs" part of the character stats breakdown.

Comment on lines 156 to 157
private updateRow(rowElem: Element, pickers: (IconPicker<Player<any>, any> | IconEnumPicker<Player<any>, any>)[]) {
console.log('Updating consumes row', pickers);

Choose a reason for hiding this comment

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

Put this behind an isDevMode() check.

Copy link
Author

Choose a reason for hiding this comment

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

Removed it since its not needed

@ToxicKevinFerm ToxicKevinFerm changed the title [Feature] Raid debuffs [Feature] Raid buffs & debuffs May 5, 2025
Copy link

@NerdEgghead NerdEgghead left a comment

Choose a reason for hiding this comment

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

Looks good overall, just needs some cleanup.

Comment on lines 490 to 491
int32 mana_tide_totem_count = 35;
}

Choose a reason for hiding this comment

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

Need to add count fields for Skull Banner and Stormlash Totem as well.

Comment on lines 337 to 345
func CommandingShoutAura(unit *Unit, asExternal bool, withGlyph bool) *Aura {
baseAura := makeExclusiveBuff(unit, BuffConfig{
"Commanding Shout",
ActionID{SpellID: 469},
[]StatConfig{
{stats.AttackPower, 1.2, true},
{stats.AttackPower, 1.1, true},
{stats.RangedAttackPower, 1.1, true},
{stats.MP5, 326, false},
{stats.Stamina, 1.1, true},
}})

Choose a reason for hiding this comment

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

Does Commanding Shout buff both AP and Stam in MoP?

Comment on lines 390 to 392
func UnleashedRageAura(u *Unit) *Aura {
return makeExclusiveBuff(u, BuffConfig{"Unleashed Rage", ActionID{SpellID: 30809}, []StatConfig{{stats.AttackPower, 1.2, true}, {stats.RangedAttackPower, 1.1, true}}})
}

Choose a reason for hiding this comment

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

Is different multipliers for melee vs. ranged AP here intended?

Comment on lines 1806 to 1812
var ManaTideTotemActionID = ActionID{SpellID: 16190}
var ManaTideTotemAuraTag = "ManaTideTotem"

const ManaTideTotemDuration = time.Second * 12
const ManaTideTotemCD = time.Minute * 5

func registerManaTideTotemCD(agent Agent, numManaTideTotems int32) {

Choose a reason for hiding this comment

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

Did you intend to delete this? The proto field is still present.

Choose a reason for hiding this comment

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

Mana Tide works differently in MoP, I've decided to leave it to be implemented later since it's more complex. It gives 200% of the casters Spirit, so we'd need an input for that etc.

Copy link
Author

Choose a reason for hiding this comment

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

I added it back since it looks like it worked the same in cata (wrongly?)

Comment on lines -47 to -52
healingModel := options.HealingModel
if healingModel != nil {
if healingModel.InspirationUptime > 0.0 {
core.ApplyInspiration(&bdk.Unit, healingModel.InspirationUptime)
}
}

Choose a reason for hiding this comment

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

If Inspiration / Ancestral Fortitude or equivalents no longer exist in MoP, then the inspiration_uptime field should also be deleted from the HealingModel proto.

Comment on lines 135 to 137
individualBuffs: IndividualBuffs.create({
communion: true,
}),

Choose a reason for hiding this comment

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

IndividualBuffs also needs to be cleaned out if Replenishment is no longer a thing in MoP.

Comment on lines 149 to 152
export const DefaultIndividualBuffs = IndividualBuffs.create({
vampiricTouch: true,
darkIntent: true,
});

Choose a reason for hiding this comment

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

darkIntent also needs to be removed from IndividualBuffs in MoP

Comment on lines 459 to 461
label: 'Terrifying Roar',
actionId: ActionId.fromSpellId(90309),
playerData: playerClass(Class.ClassDruid),

Choose a reason for hiding this comment

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

I think you meant Hunter here.

Comment on lines 479 to 481
label: 'Roar of Courage',
actionId: ActionId.fromSpellId(93435),
playerData: playerClass(Class.ClassWarrior),

Choose a reason for hiding this comment

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

Should be Hunter as well

Comment on lines 504 to 506
label: 'Qiraji Fortitude',
actionId: ActionId.fromSpellId(90364),
playerData: playerClass(Class.ClassDruid),

Choose a reason for hiding this comment

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

Should be Hunter as well

@1337LutZ 1337LutZ mentioned this pull request May 6, 2025
@ToxicKevinFerm ToxicKevinFerm requested a review from NerdEgghead May 6, 2025 23:21
Comment on lines 441 to 442
bool commanding_shout = 4; // Warriors

Choose a reason for hiding this comment

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

Group this with the other Stamina buffs.

Comment on lines 488 to 489

// Major Mana Replenishment

Choose a reason for hiding this comment

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

Name this section "Raid Cooldowns" for generality.

Choose a reason for hiding this comment

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

Also need to delete vampiric_touch, enduring_winter, soul_leach, revitalize, communion, power_infusion_count, focus_magic, and dark_intent from IndividualBuffs.

Comment on lines 530 to 283
CommandingShoutAura(unit, true, false)
CommandingShoutAura(u, false)

Choose a reason for hiding this comment

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

Should be true rather than false.

Comment on lines 316 to 317
func HornOfWinterAura(unit *Unit, asExternal bool, withGlyph bool) *Aura {
baseAura := makeExclusiveBuff(unit, BuffConfig{

Choose a reason for hiding this comment

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

Remove the withGlyph input if it's no longer used.

Comment on lines 334 to 335
func BattleShoutAura(unit *Unit, asExternal bool, withGlyph bool) *Aura {
baseAura := makeExclusiveBuff(unit, BuffConfig{

Choose a reason for hiding this comment

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

Remove the withGlyph input if it's no longer used.

Comment on lines 387 to 389
func UnleashedRageAura(u *Unit) *Aura {
return makeExclusiveBuff(u, BuffConfig{"Unleashed Rage", ActionID{SpellID: 30809}, []StatConfig{{stats.AttackPower, 1.1, true}, {stats.RangedAttackPower, 1.1, true}}})
}

Choose a reason for hiding this comment

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

This got changed from an AP buff to a Haste buff in MoP I believe.

Comment on lines 363 to 367
{
label: 'Commanding Shout',
actionId: ActionId.fromSpellId(469),
playerData: playerClass(Class.ClassWarrior),
},

Choose a reason for hiding this comment

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

This should be in the 'Stamina' section.

@ToxicKevinFerm ToxicKevinFerm merged commit fb60f5b into master May 7, 2025
2 checks passed
@InDebt InDebt mentioned this pull request May 11, 2025
57 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants