Trinitycore: [3.3.5] - Crash MotionMaster::Update

[Jildor]

Description:

Boss: http://www.wowhead.com/npc=33350
Crashlog: https://gist.github.com/Jildor/feb71ecf5ab491ae47a27ce5e004c68a
crash here: https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/game/Movement/MotionMaster.cpp#L295
@ccrs

Steps to reproduce the problem:

1. Go to instance Ulduar, boss Mimiron
2. Kill all parts and go to phase 4
3. when parts of boss try to join, crash.

Branch(es):

3.3.5

TC rev. hash/commit:

TrinityCore rev. 982643cd9679 2018-06-03 10:06:57 -0700 (3.3.5 branch) (Unix, Release, Static)
Using SSL version: OpenSSL 1.0.2l 25 May 2017 (library: OpenSSL 1.0.2l 25 May 2017)
Using Boost version: 1.62.0
Using MySQL version: 10.1.26-MariaDB
Using CMake version: 3.7.2
Compiled on: Linux 4.9.0-4-amd64
Automatic database updates are enabled for the following databases: Auth, Characters, World
Worldserver listening connections on port 8085
Realmlist (Realm Id: 1) configured in port 8085
VMAPs status: Enabled. LineOfSight: 1, getHeight: 1, indoorCheck: 1
MMAPs status: Enabled
maps directory located in /home/trinity/data/335/data/maps. Total size: 252191207 bytes
vmaps directory located in /home/trinity/data/335/data/vmaps. Total size: 588247501 bytes
mmaps directory located in /home/trinity/data/335/data/mmaps. Total size: 2152621156 bytes
Using esES DBC Locale as default. All available DBC locales: esES
Using World DB: TDB 335.64

[ccrs]

Really busy, will work on it.
ETA Sunday.

[ccrs]

Found the cause:
a GenericMovementGenerator which was Deactivated and never Initialized (and therefore never launched)

[ccrs]

Most likely triggered by this

aerial->GetMotionMaster()->MoveLand(0, (aerial->GetPositionX(), aerial->GetPositionY(), aerial->GetPositionZ()));
aerial->SetByteValue(UNIT_FIELD_BYTES_1, UNIT_BYTES_1_OFFSET_ANIM_TIER, 0);
aerial->CastSpell(vx001, SPELL_MOUNT_VX_001);
aerial->CastSpell(aerial, SPELL_HALF_HEAL);

MoveLand and SPELL_MOUNT_VX_001 both create GenericMovementGenerator.
The first is created when the call is done, the second when the spell is handled (next update tick).

MoveLand definition is:

Movement::MoveSplineInit init(_owner);
init.MoveTo(PositionToVector3(pos));
init.SetAnimation(Movement::ToGround);
Add(new GenericMovementGenerator(std::move(init), EFFECT_MOTION_TYPE, id));

SPELL_MOUNT_VX_001 is handled into:

Movement::MoveSplineInit init(Passenger);
init.DisableTransportPathTransformations();
init.MoveTo(veSeat->m_attachmentOffsetX, veSeat->m_attachmentOffsetY, veSeat->m_attachmentOffsetZ, false, true);
init.SetFacing(0.0f);
init.SetTransportEnter();
Passenger->GetMotionMaster()->LaunchMoveSpline(std::move(init), EVENT_VEHICLE_BOARD, MOTION_PRIORITY_HIGHEST);

So, after MoveLand creates a GenericMovementGenerator prepared to Initialize on the next tick, SPELL_MOUNT_VX_001 creates its own GenericMovementGenerator with higher priority, that deactivates the previous one and launches when MotionMaster::Update is called (last call within Unit::Update).

So far so good, right? Well no, because the GenericMovementGenerator created by MoveLand is still there, waiting to be Reset and Initialized, and thats something I had not foreseen, and thats why the ASSERT is triggered.

[ccrs]

TODO:

• Check Reset and Initialize call order. Is it the correct one?
• Redefine GenericMovementGenerator Reset, Initialize and Deactivate logic

[ccrs]

Workaround:

//aerial->GetMotionMaster()->MoveLand(0, (aerial->GetPositionX(), aerial->GetPositionY(), aerial->GetPositionZ()));
aerial->SetByteValue(UNIT_FIELD_BYTES_1, UNIT_BYTES_1_OFFSET_ANIM_TIER, 0);
aerial->CastSpell(vx001, SPELL_MOUNT_VX_001);
aerial->CastSpell(aerial, SPELL_HALF_HEAL);

[ccrs]

btw, kudos to @Jildor for the report