Fixed crash when MotionMaster's Clean or ExpireMovement called from MovementGenerator update. Original patch by ApoC. Backport from 308.

--HG--
branch : trunk

src/game/MotionMaster.cpp

@@ -1,7 +1,7 @@
 /*
- * Copyright (C) 2005-2008 MaNGOS <http://www.mangosproject.org/>
+ * Copyright (C) 2005-2009 MaNGOS <http://getmangos.com/>
  *
- * Copyright (C) 2008 Trinity <http://www.trinitycore.org/>
+ * Copyright (C) 2008-2009 Trinity <http://www.trinitycore.org/>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -47,11 +47,7 @@ MotionMaster::Initialize()
     {
         MovementGenerator *curr = top();
         pop();
-        if(!curr)
+        if(curr) DirectDelete(curr);
-            continue;
-        curr->Finalize(*i_owner);
-        if( !isStatic( curr ) )
-            delete curr;
     }
 
     // set new default movement generator
@@ -72,45 +68,113 @@ MotionMaster::~MotionMaster()
     {
         MovementGenerator *curr = top();
         pop();
-        if(!curr)
+        if(curr) DirectDelete(curr);
-            continue;
-        curr->Finalize(*i_owner);
-        if( !isStatic( curr ) )
-            delete curr;
     }
 }
 
 void
-MotionMaster::UpdateMotion(const uint32 &diff)
+MotionMaster::UpdateMotion(uint32 diff)
 {
     if( i_owner->hasUnitState(UNIT_STAT_ROOT | UNIT_STAT_STUNNED) )
         return;
     assert( !empty() );
+    m_cleanFlag |= MMCF_UPDATE;
     if (!top()->Update(*i_owner, diff))
+    {
+        m_cleanFlag &= ~MMCF_UPDATE;
         MovementExpired();
+    }
+    else
+        m_cleanFlag &= ~MMCF_UPDATE;
+
+    if (m_expList)
+    {
+        for (int i = 0; i < m_expList->size(); ++i)
+        {
+            MovementGenerator* mg = (*m_expList)[i];
+            DirectDelete(mg);
+        }
+
+        delete m_expList;
+        m_expList = NULL;
+
+        if (empty())
+            Initialize();
+
+        if (m_cleanFlag & MMCF_RESET)
+        {
+            top()->Reset(*i_owner);
+            m_cleanFlag &= ~MMCF_RESET;
+        }
+    }
 }
 
 void
-MotionMaster::Clear(bool reset)
+MotionMaster::DirectClean(bool reset)
 {
     while( !empty() && size() > 1 )
     {
         MovementGenerator *curr = top();
         pop();
-        if(!curr)
+        if(curr) DirectDelete(curr);
-            continue;
-        curr->Finalize(*i_owner);
-        if( !isStatic( curr ) )
-            delete curr;
     }
 
-    if (reset)
+    if(reset)
-    {
-        assert( !empty() );
         top()->Reset(*i_owner);
+}
+
+void
+MotionMaster::DelayedClean()
+{
+    while( !empty() && size() > 1 )
+    {
+        MovementGenerator *curr = top();
+        pop();
+        if(curr) DelayedDelete(curr);
     }
 }
 
+void
+MotionMaster::DirectExpire(bool reset)
+{
+    if( empty() || size() == 1 )
+        return;
+
+    MovementGenerator *curr = top();
+    pop();
+    DirectDelete(curr);
+
+    while(!top())
+        --i_top;
+
+    if(empty())
+        Initialize();
+    if(reset)
+        top()->Reset(*i_owner);
+}
+
+void
+MotionMaster::DelayedExpire()
+{
+    if( empty() || size() == 1 )
+        return;
+
+    MovementGenerator *curr = top();
+    pop();
+    DelayedDelete(curr);
+
+    while(!top())
+        --i_top;
+}
+
+void MotionMaster::MoveIdle(MovementSlot slot)
+{
+    //if( empty() || !isStatic( top() ) )
+    //    push( &si_idleMovement );
+    if(!isStatic(Impl[slot]))
+        Mutate(&si_idleMovement, slot);
+}
+
 void
 MotionMaster::MoveRandom(float spawndist)
 {
@@ -121,48 +185,11 @@ MotionMaster::MoveRandom(float spawndist)
     }
 }
 
-void
-MotionMaster::MovementExpired(bool reset)
-{
-    if( empty() || size() == 1 )
-        return;
-
-    MovementGenerator *curr = top();
-    curr->Finalize(*i_owner);
-    pop();
-
-    if( !isStatic(curr) )
-        delete curr;
-
-    assert( !empty() );
-    while(!top())
-        --i_top;
-    /*while( !empty() && top()->GetMovementGeneratorType() == TARGETED_MOTION_TYPE )
-    {
-        // Should check if target is still valid? If not valid it will crash.
-        curr = top();
-        curr->Finalize(*i_owner);
-        pop();
-        delete curr;
-    }*/
-    if( empty() )
-        Initialize();
-    if (reset) top()->Reset(*i_owner);
-}
-
-void MotionMaster::MoveIdle(MovementSlot slot)
-{
-    //if( empty() || !isStatic( top() ) )
-    //    push( &si_idleMovement );
-    if(!isStatic(Impl[slot]))
-        Mutate(&si_idleMovement, slot);
-}
-
 void
 MotionMaster::MoveTargetedHome()
 {
-    if(i_owner->hasUnitState(UNIT_STAT_FLEEING))
+    //if(i_owner->hasUnitState(UNIT_STAT_FLEEING))
-        return;
+    //    return;
 
     Clear(false);
 
@@ -360,9 +387,10 @@ void MotionMaster::Mutate(MovementGenerator *m, MovementSlot slot)
 {
     if(MovementGenerator *curr = Impl[slot])
     {
-        curr->Finalize(*i_owner);
+        if(i_top == slot && (m_cleanFlag & MMCF_UPDATE))
-        if( !isStatic( curr ) )
+            DelayedDelete(curr);
-            delete curr;
+        else
+            DirectDelete(curr);
     }
     else if(i_top < slot)
     {
@@ -370,20 +398,6 @@ void MotionMaster::Mutate(MovementGenerator *m, MovementSlot slot)
     }
     m->Initialize(*i_owner);
     Impl[slot] = m;
-
-    /*if (!empty())
-    {
-        switch(top()->GetMovementGeneratorType())
-        {
-            // HomeMovement is not that important, delete it if meanwhile a new comes
-            case HOME_MOTION_TYPE:
-            // DistractMovement interrupted by any other movement
-            case DISTRACT_MOTION_TYPE:
-                MovementExpired(false);
-        }
-    }
-    m->Initialize(*i_owner);
-    push(m);*/
 }
 
 void MotionMaster::MovePath(uint32 path_id, bool repeatable)
@@ -432,6 +446,24 @@ MovementGeneratorType MotionMaster::GetCurrentMovementGeneratorType() const
    return top()->GetMovementGeneratorType();
 }
 
+void MotionMaster::DirectDelete(_Ty curr)
+{
+    if(isStatic(curr))
+        return;
+    curr->Finalize(*i_owner);
+    delete curr;
+}
+
+void MotionMaster::DelayedDelete(_Ty curr)
+{
+    sLog.outError("CRASH ALARM! Unit (Entry %u) is trying to delete its updating MG (Type %u)!", i_owner->GetEntry(), curr->GetMovementGeneratorType());
+    if(isStatic(curr))
+        return;
+    if(!m_expList)
+        m_expList = new ExpireList();
+    m_expList->push_back(curr);
+}
+
 bool MotionMaster::GetDestination(float &x, float &y, float &z)
 {
    if(empty())

src/game/MotionMaster.h

@@ -1,7 +1,7 @@
 /*
- * Copyright (C) 2005-2008 MaNGOS <http://www.mangosproject.org/>
+ * Copyright (C) 2005-2009 MaNGOS <http://getmangos.com/>
  *
- * Copyright (C) 2008 Trinity <http://www.trinitycore.org/>
+ * Copyright (C) 2008-2009 Trinity <http://www.trinitycore.org/>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -22,7 +22,7 @@
 #define TRINITY_MOTIONMASTER_H
 
 #include "Common.h"
-#include <stack>
+#include <vector>
 
 class MovementGenerator;
 class Unit;
@@ -55,12 +55,20 @@ enum MovementSlot
     MAX_MOTION_SLOT,
 };
 
+enum MMCleanFlag
+{
+    MMCF_NONE   = 0,
+    MMCF_UPDATE = 1, // Clear or Expire called from update
+    MMCF_RESET  = 2  // Flag if need top()->Reset()
+};
+
 class TRINITY_DLL_SPEC MotionMaster //: private std::stack<MovementGenerator *>
 {
     private:
         //typedef std::stack<MovementGenerator *> Impl;
         typedef MovementGenerator* _Ty;
         _Ty Impl[MAX_MOTION_SLOT];
+        typedef std::vector<_Ty> ExpireList;
         int i_top;
 
         bool empty() const { return i_top < 0; }
@@ -68,7 +76,7 @@ class TRINITY_DLL_SPEC MotionMaster //: private std::stack<MovementGenerator *>
         void push(_Ty _Val) { ++i_top; Impl[i_top] = _Val; }
     public:
 
-        explicit MotionMaster(Unit *unit) : i_owner(unit), i_top(-1)
+        explicit MotionMaster(Unit *unit) : i_owner(unit), m_expList(NULL), m_cleanFlag(MMCF_NONE), i_top(-1)
         {
             for(int i = 0; i < MAX_MOTION_SLOT; ++i)
                 Impl[i] = NULL;
@@ -77,21 +85,40 @@ class TRINITY_DLL_SPEC MotionMaster //: private std::stack<MovementGenerator *>
 
         void Initialize();
 
-        //MovementGenerator* operator->(void) { return top(); }
-
         int size() const { return i_top + 1; }
         _Ty top() const { return Impl[i_top]; }
         _Ty GetMotionSlot(int slot) { return Impl[slot]; }
-        /*using Impl::top;
-        using Impl::empty;
 
-        typedef Impl::container_type::const_iterator const_iterator;
+        void DirectDelete(_Ty curr);
-        const_iterator begin() const { return Impl::c.begin(); }
+        void DelayedDelete(_Ty curr);
-        const_iterator end() const { return Impl::c.end(); }*/
 
-        void UpdateMotion(const uint32 &diff);
+        void UpdateMotion(uint32 diff);
-        void Clear(bool reset = true);
+        void Clear(bool reset = true)
-        void MovementExpired(bool reset = true);
+        {
+            if (m_cleanFlag & MMCF_UPDATE)
+            {
+                if(reset)
+                    m_cleanFlag |= MMCF_RESET;
+                else
+                    m_cleanFlag &= ~MMCF_RESET;
+                DelayedClean();
+            }
+            else
+                DirectClean(reset);
+        }
+        void MovementExpired(bool reset = true)
+        {
+            if (m_cleanFlag & MMCF_UPDATE)
+            {
+                if(reset)
+                    m_cleanFlag |= MMCF_RESET;
+                else
+                    m_cleanFlag &= ~MMCF_RESET;
+                DelayedExpire();
+            }
+            else
+                DirectExpire(reset);
+        }
 
         void MoveIdle(MovementSlot slot = MOTION_SLOT_ACTIVE);
         void MoveTargetedHome();
@@ -114,6 +141,14 @@ class TRINITY_DLL_SPEC MotionMaster //: private std::stack<MovementGenerator *>
     private:
         void Mutate(MovementGenerator *m, MovementSlot slot);                  // use Move* functions instead
 
-        Unit *i_owner;
+        void DirectClean(bool reset);
+        void DelayedClean();
+
+        void DirectExpire(bool reset);
+        void DelayedExpire();
+
+        Unit       *i_owner;
+        ExpireList *m_expList;
+        uint8       m_cleanFlag;
 };
 #endif