cmangos / issues

This repository is used as a centralized point for all issues regarding CMaNGOS.
179 stars 47 forks source link

Dummy Auras are not reapplied if stacking with themselves #1808

Closed cala closed 5 years ago

cala commented 5 years ago

Current behavior:

Four Horsemen encounter uses a mechanics where a stacking dummy aura is applied to players. Every time the aura is applied, players should take damages based on the amount of stacks. The relevant code lives in void Aura::HandleAuraDummy(bool apply, bool Real). (see https://github.com/cmangos/mangos-classic/commit/7051c539fa1533d2bf92d744c539165f192053e2)

However, this code is only executed once: when the aura is first applied. All others stacks do not run this code and instead execute bool SpellAuraHolder::ModStackAmount(int32 num, Unit* newCaster) and then void SpellAuraHolder::SetStackAmount(uint32 stackAmount, Unit* newCaster)

The code handling this two differents cases lives in bool Unit::AddSpellAuraHolder(SpellAuraHolder* holder).

Poking @killerwife as I think he is the man for this question. I see two main options to fix this:

What do you think?

Expected behavior:

Four Horsemen encounter uses a mechanics where a stacking dummy aura is applied to players. Every time the aura is applied, players do take damages based on the amount of stacks.

Steps to reproduce:

  1. .gm on
  2. .go creature blaumeux
  3. Engage combat and notice that none of the four marks do damage when applied whatever the number of stacks (they are not supposed to do damage on first appliance, so no worries if no damage is applied even when going through Aura::HandleAuraDummy for the first and only time

Client version: Classic 1.12.1 (not tested with TBC and WotLK but should also be relevant there)

Commit hash: https://github.com/cmangos/mangos-classic/commit/7051c539fa1533d2bf92d744c539165f192053e2

Database version:

Classic DB 1.9

Operating system: mac OS / Win 7

killerwife commented 5 years ago

Give me 10 minutes.

killerwife commented 5 years ago

@cala My proposed change: void SpellAuraHolder::SetStackAmount(uint32 stackAmount, Unit* newCaster)

            int32 baseAmount = aur->GetModifier()->m_baseAmount;
            int32 amount = m_stackAmount * baseAmount;
            // Reapply if amount change
            if (!baseAmount || amount != aur->GetModifier()->m_amount)
            {
                aur->SetRemoveMode(AURA_REMOVE_BY_GAINED_STACK);
                if (IsAuraRemoveOnStacking(this->GetSpellProto(), aur->GetEffIndex()))
                    aur->ApplyModifier(false, true);
                aur->GetModifier()->m_amount = amount;
                aur->GetModifier()->m_recentAmount = baseAmount * (stackAmount - oldStackAmount);
                aur->ApplyModifier(true, true);
            }
killerwife commented 5 years ago

The dummy has 0 in base amount and amount. There are 29 such spells in tbc. select Id,SpellName from spell_template where StackAmount>0 AND Effect1 = 6 AND EffectBasePoints1=0 AND EffectRealPointsPerLevel1=0 AND EffectBaseDice1=0 AND EffectBasePoints1=0;

cala commented 5 years ago

Well spotted, I did not investigate the logic in the (base) amount values! Your fix works like a charm. 👍

There are 8 spells in Classic that match the query above. Four of them are the Four Horsemen mark spells, three more are related to an AQ20 mechanics that should also gain from this fix. There is only one spell 27673 (Five Fat Finger Exploding Heart Technique) that I don't know of. The spell is commented out in the core and does nothing.

So, if you are fine with it, I'll push your fix to all three branches. Thanks for looking at it this quickly.

killerwife commented 5 years ago

Five Fat Finger Exploding Heart Technique is from BRD https://www.wowhead.com/npc=16049/lefty and its a kill bill reference.

cala commented 5 years ago

Thanks. One more Classic NPC I don't know of (even though I did BRD numerous times for librams or forging MC gear).

From what I understand from the description of the NPC, this spell should have the same mechanics than Horsemen marks, so I think we can push your fix if you have no objection.

killerwife commented 5 years ago

Yes, you can go ahead and push it.