Trinitycore: Core/Phasing: spell_aura_phase not on vehicles ($20 bounty)

[Wylath]

Hello,

There is a problem with spell_aura_phase and vehicles. When the player has aura from the phase and is on a vehicle, when the area changes, the player is dismounted.

Example:
https://youtu.be/6eqHnbg_1JM

http://www.wowhead.com/spell=68482

[gonzo1247]

https://gitlab.com/trinitycore/TrinityCore_434/commit/95b6332bef6e266badcbcaddc852750885cb5591
that was a fix for this problem, but that createt crashes.

[Wylath]

Replace

RemoveNotOwnSingleTargetAuras(0, true);

by

RemoveNotOwnSingleTargetAuras(id, true);

if (update)
    UpdateObjectVisibility();

in bool Unit::SetInPhase
It solves the problem.

[gonzo1247]

With the fix from 434 repo? Or only this change from you?

[Wylath]

Just with my change

[gonzo1247]

Mhm maybe work this only with phase aura on player? when i use a vehicle without phase auras on player and the area is changes, the player is dismounting from Vehicle.

[Wylath]

Hm I'll see if I made other changes

[gonzo1247]

Mhm maybe work this only with phase aura on player? So i mean your fix. When i use a vehicle without phase auras on player and the area is changes, the player is dismounting from Vehicle.
In the Issue on 434 repo Kilyana means that using conditions for vehicle, so that they not despawn. But when i read the wiki for condition : CONDITION_TYPE_CREATURE_TEMPLATE_VEHICLE is this info:
Note: creature entry must be a vehicle. Example: If this is used with CONDITION_AREA, the player will be dismounted of the vehicle if the player leaves that area.

so i think without condition the vehicle is not despawn on area change, but can it is that here is a problem with the core?

[gonzo1247]

Some news about this?

[Wylath]

hm I have not made other changes. Now if the phaseid does not remain the same during the change of area I dismount too.

[gonzo1247]

Ok.
The question is, why is the vehicle ride aura removed on area change. in some cases also happend on change the ground from earth to a web and below them is water
I think on the area change is this the core check the auras and the permission for the auras and decides that the player character have no permission to have this aura in the new area and then they remove the aura. But why he decides so? That is the point that i not understand

[Lopfest]

Maybe it is not related, but if it is, it might help someone to find a fix.
When I tried to hackfix this bug with SAI:

(55649, 0, 7, 0, 28, 1, 100, 0, 0, 0, 0, 0, 85, 46598, 2, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 'Shang Xi''s Hot Air Balloon - On Passenger Removed - Invoker Cast Ride Vehicle Hardcoded'), -- temp

when the area changed and I got dismounted, SAI was called and the server crashed with error:

Unit.cpp:14837 in Unit::_EnterVehicle ASSERTION FAILED:
    !m_vehicle

[gonzo1247]

this is so a bad issue^^ any news about this?

[gonzo1247]

no more?

[Flameshot]

Anything on this?

[giridhar123]

Wylat

Replace

RemoveNotOwnSingleTargetAuras(0, true);

by

RemoveNotOwnSingleTargetAuras(id, true);

if (update)
UpdateObjectVisibility();

in bool Unit::SetInPhase
It solves the problem.

This doesn't solve the problem.

[gonzo1247]

This is from https://gitlab.com/trinitycore/TrinityCore_434/issues/42 from giridhar

New info about this: In unit.cpp: void Unit::RemoveNotOwnSingleTargetAuras If y add comment from: /* AuraList& scAuras = GetSingleCastAuras(); until:

else ++iter; }*/

Y solve the problem about the vehicle, (they are not removed on area changes), so the problem is in that lines. But there also the same problem with other aura like this one: 49417, to solve the problem with aura like this you must add comment in Player.cpp on void Player::UpdateAreaDependentAuras Adding comment from: /* for (AuraMap::iterator iter = m_ownedAuras.begin(); iter != m_ownedAuras.end();) until:

else ++iter; }*/ So the problem is in that lines, but now I don't know how to solve it...

[gonzo1247]

$20 bounty added
https://www.bountysource.com/issues/29186386-6-x-core-phasing-spell_aura_phase-not-on-vehicles

[zgn1988]

any news?

[gonzo1247]

Almost a Month past, some one have news?

[Flameshot]

I am curious what are the files that must be checked cuz I want to give an eye on it

[malcrom]

This shitty code was allowed to stay so long without a fix? Some hacks seem to be acceptable.

[malcrom]

I just added a check here for SPELL_AURA_CONTROL_VEHICLE. Don't care if its the best way I need to test paths.

void Unit::RemoveNotOwnSingleTargetAuras(uint32 newPhase, bool phaseid)
{
    // single target auras from other casters
    // Iterate m_ownedAuras - aura is marked as single target in Unit::AddAura (and pushed to m_ownedAuras).
    // m_appliedAuras will NOT contain the aura before first Unit::Update after adding it to m_ownedAuras.
    // Quickly removing such an aura will lead to it not being unregistered from caster's single cast auras container
    // leading to assertion failures if the aura was cast on a player that can
    // (and is changing map at the point where this function is called).
    // Such situation occurs when player is logging in inside an instance and fails the entry check for any reason.
    // The aura that was loaded from db (indirectly, via linked casts) gets removed before it has a chance
    // to register in m_appliedAuras
    for (AuraMap::iterator iter = m_ownedAuras.begin(); iter != m_ownedAuras.end();)
    {
        Aura const* aura = iter->second;

        if (aura->GetCasterGUID() != GetGUID() && aura->IsSingleTarget())
        {
            if (!newPhase && !phaseid)
                RemoveOwnedAura(iter);
            else
            {
                Unit* caster = aura->GetCaster();
                if (!caster || (newPhase && !caster->IsInPhase(newPhase)) || (!newPhase && !caster->IsInPhase(this)))
                    RemoveOwnedAura(iter);
                else
                    ++iter;
            }
        }
        else
            ++iter;
    }

    // single target auras at other targets
    AuraList& scAuras = GetSingleCastAuras();
    for (AuraList::iterator iter = scAuras.begin(); iter != scAuras.end();)
    {
        Aura* aura = *iter;
        if ((aura->GetUnitOwner() != this && !aura->GetUnitOwner()->IsInPhase(newPhase)) && aura->GetEffect(0)->GetAuraType() != SPELL_AURA_CONTROL_VEHICLE && aura->GetEffect(1)->GetAuraType() != SPELL_AURA_CONTROL_VEHICLE)
        {
            aura->Remove();
            iter = scAuras.begin();
        }
        else
            ++iter;
    }
}

[Shauren]

And it will horribly crash if the aura does not have effect_0 or effect_1

[gonzo1247]

i test it and the vehicle aura is still removed by area change.

[malcrom]

It is working for me gonzo1247.

[malcrom]

All spells have effect_0, Shauren. effect_1 can be removed and only most, not all SPELL_AURA_CONTROL_VEHICLE spells will be ignored.

[Shauren]

No, spell effect ordering is not guaranteed at all

[malcrom]

Yes some use effect_1 for SPELL_AURA_CONTROL_VEHICLE

[giridhar123]

I found a spell that has area restrinction and the id is 67435.
If you use SpellWork to check that spell you can see that tehre's written:

Allowed areas:
4768 - Gallywix's Villa (Map: 648)

You can look in the spellwork source code to see from when that value is taken, so you can fix the code to remove properly aura that has value in that field.
What do you think?

[giridhar123]

@malcrom or @Shauren Nothing to say about that?

[Lopfest]

@giridhar123 Reported bug is not related to "Allowed areas" field. You get dismounted with general ride vehicle spell 46598 which has no restricted areas.

[giridhar123]

Ok. Btw the malcrom's fix doesn't works for me.

[gonzo1247]

I try this:

    if (aura->GetCasterGUID() != GetGUID() && aura->IsSingleTarget())
    {
        if (!newPhase && !phaseid)
        {
            TC_LOG_DEBUG("scripts", "RemoveNotOwnSingleTargetAuras: first if remove Aura: [%u]", aura->GetId());
            RemoveOwnedAura(iter);
        }
        else
        {
            Unit* caster = aura->GetCaster();
            if (!caster || ((newPhase && !caster->IsInPhase(newPhase)) || (!newPhase && !caster->IsInPhase(this))) && aura->GetEffect(0)->GetAuraType() != SPELL_AURA_CONTROL_VEHICLE && aura->GetEffect(1)->GetAuraType() != SPELL_AURA_CONTROL_VEHICLE)
            {
                TC_LOG_DEBUG("scripts", "RemoveNotOwnSingleTargetAuras: second if in else remove aura: [%u], newPhase is: [%u]", aura->GetId(), newPhase);
                if (!caster->IsInPhase(newPhase))
                    TC_LOG_DEBUG("scripts", "RemoveNotOwnSingleTargetAuras: caster is not in newPhase");
                else
                    TC_LOG_DEBUG("scripts", "RemoveNotOwnSingleTargetAuras: caster is in newPhase");
                RemoveOwnedAura(iter);
            }
            else
                ++iter;
        }
    }
    else
        ++iter;

and now its working for me...
with the Log Message, i found that they remove the "46598 Ride Vehicle Hardcoded" Aura in this if part:
if (!caster || (newPhase && !caster->IsInPhase(newPhase)) || (!newPhase && !caster->IsInPhase(this))

and than i try to add the same code from below...and now it is working. but i don't know if this create crashes or other problems...

[gonzo1247]

some time ago, about the last post here.
i have tested the "fix" above and this create some crashes with other auras, they dind't have a SPELL_AURA_CONTROL_VEHICLE aura.
so i removed this - but the problem is also here.

[Flameshot]

Anything new on this?.. Any chance for a hackfix or something like that?

[Caydan]

I use this as temporary solution:

void Unit::RemoveNotOwnSingleTargetAuras(uint32 newPhase, bool phaseid)
{
    for (AuraMap::iterator iter = m_ownedAuras.begin(); iter != m_ownedAuras.end();)
    {
        Aura const* aura = iter->second;

        if (aura->GetCasterGUID() != GetGUID() && aura->IsSingleTarget())
        {
            if (!newPhase && !phaseid)
                RemoveOwnedAura(iter);
            else
            {
                Unit* caster = aura->GetCaster();
                if (!caster || (newPhase && !caster->IsInPhase(newPhase)) || (!newPhase && !caster->IsInPhase(this)))
                    RemoveOwnedAura(iter);
                else
                    ++iter;
            }
        }
        else
            ++iter;
    }

    // single target auras at other targets
    AuraList& scAuras = GetSingleCastAuras();
    for (AuraList::iterator iter = scAuras.begin(); iter != scAuras.end();)
    {
        Aura* aura = *iter;
        if (aura->GetUnitOwner() && aura->GetUnitOwner() != this && !aura->GetUnitOwner()->IsInPhase(newPhase))
        {
            if ((newPhase == 0x0 && !phaseid) ||
                (!aura->GetEffect(0) || aura->GetEffect(0)->GetAuraType() != SPELL_AURA_CONTROL_VEHICLE) &&
                (!aura->GetEffect(1) || aura->GetEffect(1)->GetAuraType() != SPELL_AURA_CONTROL_VEHICLE))
            {
                aura->Remove();
                iter = scAuras.begin();
            }
            else ++iter;
        }
        else
            ++iter;
    }
}

I've made the changes from malcrom and added two small checks:

(!aura->GetEffect(0) || aura->GetEffect(0)->GetAuraType() != SPELL_AURA_CONTROL_VEHICLE)
this checks the existance of the effect

(newPhase == 0x0 && !phaseid)
this condition lets remove the aura when RemoveFromWorld is called

[gonzo1247]

I will this test a lot of time to show is they make crashes or not.
But actually looks nice.

[Killyana]

How fixing an issue related to gameobjects fix this issue about phasing.

[malcrom]

Wow. I guess the important things to questing never get fixed lol.

[Shauren]

This was supposed to have been fixed with phasing rewrite already, I just closed the issue because I wasn't tracking open issues about it and got reminded about it in that unrelated commit

[Killyana]

Actually the issue is fixed in all branchs.