Core/Transports: Fixed crash happening during teleporting transports

Closes #11793

9 files changed, 59 insertions, 72 deletions

diff --git a/src/server/game/Entities/DynamicObject/DynamicObject.cpp b/src/server/game/Entities/DynamicObject/DynamicObject.cpp
index 7cc94d992a1..e008146bb85 100644
--- a/src/server/game/Entities/DynamicObject/DynamicObject.cpp
+++ b/src/server/game/Entities/DynamicObject/DynamicObject.cpp
@@ -48,18 +48,6 @@ DynamicObject::~DynamicObject()
     delete _removedAura;
 }
 
-void DynamicObject::CleanupsBeforeDelete(bool finalCleanup /* = true */)
-{
-    WorldObject::CleanupsBeforeDelete(finalCleanup);
-
-    if (Transport* transport = GetTransport())
-    {
-        transport->RemovePassenger(this);
-        SetTransport(NULL);
-        m_movementInfo.transport.Reset();
-    }
-}
-
 void DynamicObject::AddToWorld()
 {
     ///- Register the dynamicObject for guid lookup and for caster
@@ -124,14 +112,11 @@ bool DynamicObject::CreateDynamicObject(uint32 guidlow, Unit* caster, uint32 spe
     Transport* transport = caster->GetTransport();
     if (transport)
     {
-        m_movementInfo.transport.guid = GetGUID();
-
         float x, y, z, o;
         pos.GetPosition(x, y, z, o);
         transport->CalculatePassengerOffset(x, y, z, &o);
         m_movementInfo.transport.pos.Relocate(x, y, z, o);
 
-        SetTransport(transport);
         // This object must be added to transport before adding to map for the client to properly display it
         transport->AddPassenger(this);
     }
diff --git a/src/server/game/Entities/DynamicObject/DynamicObject.h b/src/server/game/Entities/DynamicObject/DynamicObject.h
index f52c86afdee..c9fd1d29f8b 100644
--- a/src/server/game/Entities/DynamicObject/DynamicObject.h
+++ b/src/server/game/Entities/DynamicObject/DynamicObject.h
@@ -41,8 +41,6 @@ class DynamicObject : public WorldObject, public GridObject<DynamicObject>, publ
         void AddToWorld();
         void RemoveFromWorld();
 
-        void CleanupsBeforeDelete(bool finalCleanup = true) override;
-
         bool CreateDynamicObject(uint32 guidlow, Unit* caster, uint32 spellId, Position const& pos, float radius, DynamicObjectType type);
         void Update(uint32 p_time);
         void Remove();
diff --git a/src/server/game/Entities/GameObject/GameObject.cpp b/src/server/game/Entities/GameObject/GameObject.cpp
index c6b4039621a..69b15c2b56b 100644
--- a/src/server/game/Entities/GameObject/GameObject.cpp
+++ b/src/server/game/Entities/GameObject/GameObject.cpp
@@ -98,20 +98,12 @@ std::string GameObject::GetAIName() const
     return "";
 }
 
-void GameObject::CleanupsBeforeDelete(bool /*finalCleanup*/)
+void GameObject::CleanupsBeforeDelete(bool finalCleanup)
 {
-    if (IsInWorld())
-        RemoveFromWorld();
+    WorldObject::CleanupsBeforeDelete(finalCleanup);
 
     if (m_uint32Values)                                      // field array can be not exist if GameOBject not loaded
         RemoveFromOwner();
-
-    if (GetTransport() && !ToTransport())
-    {
-        GetTransport()->RemovePassenger(this);
-        SetTransport(NULL);
-        m_movementInfo.transport.Reset();
-    }
 }
 
 void GameObject::RemoveFromOwner()
@@ -705,7 +697,7 @@ void GameObject::getFishLoot(Loot* fishloot, Player* loot_owner)
     {
         //subzone no result,use zone loot
         fishloot->FillLoot(zone, LootTemplates_Fishing, loot_owner, true, true);
-        //use zone 1 as default, somewhere fishing got nothing,becase subzone and zone not set, like Off the coast of Storm Peaks. 
+        //use zone 1 as default, somewhere fishing got nothing,becase subzone and zone not set, like Off the coast of Storm Peaks.
         if (fishloot->empty())
             fishloot->FillLoot(defaultzone, LootTemplates_Fishing, loot_owner, true, true);
     }
diff --git a/src/server/game/Entities/Object/Object.cpp b/src/server/game/Entities/Object/Object.cpp
index f2215fa2d6d..4ff0153dea8 100644
--- a/src/server/game/Entities/Object/Object.cpp
+++ b/src/server/game/Entities/Object/Object.cpp
@@ -1163,6 +1163,9 @@ void WorldObject::CleanupsBeforeDelete(bool /*finalCleanup*/)
 {
     if (IsInWorld())
         RemoveFromWorld();
+
+    if (Transport* transport = GetTransport())
+        transport->RemovePassenger(this);
 }
 
 void WorldObject::_Create(uint32 guidlow, HighGuid guidhigh, uint32 phaseMask)
@@ -1215,7 +1218,7 @@ bool WorldObject::_IsWithinDist(WorldObject const* obj, float dist2compare, bool
     float sizefactor = GetObjectSize() + obj->GetObjectSize();
     float maxdist = dist2compare + sizefactor;
 
-    if (m_transport && obj->GetTransport() &&  obj->GetTransport()->GetGUIDLow() == m_transport->GetGUIDLow())
+    if (GetTransport() && obj->GetTransport() && obj->GetTransport()->GetGUIDLow() == GetTransport()->GetGUIDLow())
     {
         float dtx = m_movementInfo.transport.pos.m_positionX - obj->m_movementInfo.transport.pos.m_positionX;
         float dty = m_movementInfo.transport.pos.m_positionY - obj->m_movementInfo.transport.pos.m_positionY;
diff --git a/src/server/game/Entities/Player/Player.cpp b/src/server/game/Entities/Player/Player.cpp
index 977142c8b21..8ceb6c00808 100644
--- a/src/server/game/Entities/Player/Player.cpp
+++ b/src/server/game/Entities/Player/Player.cpp
@@ -2131,11 +2131,9 @@ bool Player::TeleportTo(uint32 mapid, float x, float y, float z, float orientati
     {
         TC_LOG_DEBUG("maps", "Player %s using client without required expansion tried teleport to non accessible map %u", GetName().c_str(), mapid);
 
-        if (GetTransport())
+        if (Transport* transport = GetTransport())
         {
-            m_transport->RemovePassenger(this);
-            m_transport = NULL;
-            m_movementInfo.transport.Reset();
+            transport->RemovePassenger(this);
             RepopAtGraveyard();                             // teleport to near graveyard if on transport, looks blizz like :)
         }
 
@@ -2153,16 +2151,12 @@ bool Player::TeleportTo(uint32 mapid, float x, float y, float z, float orientati
     SetUnitMovementFlags(GetUnitMovementFlags() & MOVEMENTFLAG_MASK_HAS_PLAYER_STATUS_OPCODE);
     DisableSpline();
 
-    if (m_transport)
+    if (Transport* transport = GetTransport())
     {
         if (options & TELE_TO_NOT_LEAVE_TRANSPORT)
             AddUnitMovementFlag(MOVEMENTFLAG_ONTRANSPORT);
         else
-        {
-            m_transport->RemovePassenger(this);
-            m_transport = NULL;
-            m_movementInfo.transport.Reset();
-        }
+            transport->RemovePassenger(this);
     }
 
     // The player was ported to another map and loses the duel immediately.
@@ -2297,8 +2291,8 @@ bool Player::TeleportTo(uint32 mapid, float x, float y, float z, float orientati
                 // send transfer packets
                 WorldPacket data(SMSG_TRANSFER_PENDING, 4 + 4 + 4);
                 data << uint32(mapid);
-                if (m_transport)
-                    data << m_transport->GetEntry() << GetMapId();
+                if (Transport* transport = GetTransport())
+                    data << transport->GetEntry() << GetMapId();
 
                 GetSession()->SendPacket(&data);
             }
@@ -2316,7 +2310,7 @@ bool Player::TeleportTo(uint32 mapid, float x, float y, float z, float orientati
             {
                 WorldPacket data(SMSG_NEW_WORLD, 4 + 4 + 4 + 4 + 4);
                 data << uint32(mapid);
-                if (m_transport)
+                if (GetTransport())
                     data << m_movementInfo.transport.pos.PositionXYZOStream();
                 else
                     data << m_teleport_dest.PositionXYZOStream();
@@ -17401,15 +17395,15 @@ bool Player::LoadFromDB(uint32 guid, SQLQueryHolder *holder)
     {
         uint64 transGUID = MAKE_NEW_GUID(transLowGUID, 0, HIGHGUID_MO_TRANSPORT);
 
+        Transport* transport = NULL;
         if (GameObject* go = HashMapHolder<GameObject>::Find(transGUID))
-            m_transport = go->ToTransport();
+            transport = go->ToTransport();
 
-        if (m_transport)
+        if (transport)
         {
-            m_movementInfo.transport.guid = transGUID;
             float x = fields[26].GetFloat(), y = fields[27].GetFloat(), z = fields[28].GetFloat(), o = fields[29].GetFloat();
             m_movementInfo.transport.pos.Relocate(x, y, z, o);
-            m_transport->CalculatePassengerPosition(x, y, z, &o);
+            transport->CalculatePassengerPosition(x, y, z, &o);
 
             if (!Trinity::IsValidMapCoord(x, y, z, o) ||
                 // transport size limited
@@ -17420,7 +17414,6 @@ bool Player::LoadFromDB(uint32 guid, SQLQueryHolder *holder)
                 TC_LOG_ERROR("entities.player", "Player (guidlow %d) have invalid transport coordinates (X: %f Y: %f Z: %f O: %f). Teleport to bind location.",
                     guid, x, y, z, o);
 
-                m_transport = NULL;
                 m_movementInfo.transport.Reset();
 
                 RelocateToHomebind();
@@ -17428,10 +17421,9 @@ bool Player::LoadFromDB(uint32 guid, SQLQueryHolder *holder)
             else
             {
                 Relocate(x, y, z, o);
-                mapId = m_transport->GetMapId();
+                mapId = transport->GetMapId();
 
-                m_transport->AddPassenger(this);
-                AddUnitMovementFlag(MOVEMENTFLAG_ONTRANSPORT);
+                transport->AddPassenger(this);
             }
         }
         else
diff --git a/src/server/game/Entities/Transport/Transport.cpp b/src/server/game/Entities/Transport/Transport.cpp
index 8bc07732bb6..d907274f8d1 100644
--- a/src/server/game/Entities/Transport/Transport.cpp
+++ b/src/server/game/Entities/Transport/Transport.cpp
@@ -35,7 +35,7 @@
 
 Transport::Transport() : GameObject(),
     _transportInfo(NULL), _isMoving(true), _pendingStop(false),
-    _triggeredArrivalEvent(false), _triggeredDepartureEvent(false)
+    _triggeredArrivalEvent(false), _triggeredDepartureEvent(false), _passengerTeleportItr(_passengers.begin())
 {
     m_updateFlag = UPDATEFLAG_TRANSPORT | UPDATEFLAG_LOWGUID | UPDATEFLAG_STATIONARY_POSITION | UPDATEFLAG_ROTATION;
 }
@@ -107,9 +107,6 @@ void Transport::CleanupsBeforeDelete(bool finalCleanup /*= true*/)
     while (!_passengers.empty())
     {
         WorldObject* obj = *_passengers.begin();
-        obj->m_movementInfo.RemoveMovementFlag(MOVEMENTFLAG_ONTRANSPORT);
-        obj->m_movementInfo.transport.Reset();
-        obj->SetTransport(NULL);
         RemovePassenger(obj);
     }
 
@@ -231,6 +228,9 @@ void Transport::AddPassenger(WorldObject* passenger)
 
     if (_passengers.insert(passenger).second)
     {
+        passenger->SetTransport(this);
+        passenger->m_movementInfo.AddMovementFlag(MOVEMENTFLAG_ONTRANSPORT);
+        passenger->m_movementInfo.transport.guid = GetGUID();
         TC_LOG_DEBUG("entities.transport", "Object %s boarded transport %s.", passenger->GetName().c_str(), GetName().c_str());
 
         if (Player* plr = passenger->ToPlayer())
@@ -240,8 +240,27 @@ void Transport::AddPassenger(WorldObject* passenger)
 
 void Transport::RemovePassenger(WorldObject* passenger)
 {
-    if (_passengers.erase(passenger) || _staticPassengers.erase(passenger)) // static passenger can remove itself in case of grid unload
+    bool erased = false;
+    if (_passengerTeleportItr != _passengers.end())
     {
+        PassengerSet::iterator itr = _passengers.find(passenger);
+        if (itr != _passengers.end())
+        {
+            if (itr == _passengerTeleportItr)
+                ++_passengerTeleportItr;
+
+            _passengers.erase(itr);
+            erased = true;
+        }
+    }
+    else
+        erased = _passengers.erase(passenger) > 0;
+
+    if (erased || _staticPassengers.erase(passenger)) // static passenger can remove itself in case of grid unload
+    {
+        passenger->SetTransport(NULL);
+        passenger->m_movementInfo.RemoveMovementFlag(MOVEMENTFLAG_ONTRANSPORT);
+        passenger->m_movementInfo.transport.Reset();
         TC_LOG_DEBUG("entities.transport", "Object %s removed from transport %s.", passenger->GetName().c_str(), GetName().c_str());
 
         if (Player* plr = passenger->ToPlayer())
@@ -570,9 +589,9 @@ bool Transport::TeleportTransport(uint32 newMapid, float x, float y, float z, fl
         GetMap()->RemoveFromMap<Transport>(this, false);
         SetMap(newMap);
 
-        for (std::set<WorldObject*>::iterator itr = _passengers.begin(); itr != _passengers.end();)
+        for (_passengerTeleportItr = _passengers.begin(); _passengerTeleportItr != _passengers.end();)
         {
-            WorldObject* obj = (*itr++);
+            WorldObject* obj = (*_passengerTeleportItr++);
 
             float destX, destY, destZ, destO;
             obj->m_movementInfo.transport.pos.GetPosition(destX, destY, destZ, destO);
@@ -595,7 +614,7 @@ bool Transport::TeleportTransport(uint32 newMapid, float x, float y, float z, fl
                 }
                 case TYPEID_PLAYER:
                     if (!obj->ToPlayer()->TeleportTo(newMapid, destX, destY, destZ, destO, TELE_TO_NOT_LEAVE_TRANSPORT))
-                        _passengers.erase(obj);
+                        RemovePassenger(obj);
                     break;
                 case TYPEID_DYNAMICOBJECT:
                     obj->AddObjectToRemoveList();
@@ -613,7 +632,7 @@ bool Transport::TeleportTransport(uint32 newMapid, float x, float y, float z, fl
     else
     {
         // Teleport players, they need to know it
-        for (std::set<WorldObject*>::iterator itr = _passengers.begin(); itr != _passengers.end(); ++itr)
+        for (PassengerSet::iterator itr = _passengers.begin(); itr != _passengers.end(); ++itr)
         {
             if ((*itr)->GetTypeId() == TYPEID_PLAYER)
             {
@@ -630,9 +649,9 @@ bool Transport::TeleportTransport(uint32 newMapid, float x, float y, float z, fl
     }
 }
 
-void Transport::UpdatePassengerPositions(std::set<WorldObject*>& passengers)
+void Transport::UpdatePassengerPositions(PassengerSet& passengers)
 {
-    for (std::set<WorldObject*>::iterator itr = passengers.begin(); itr != passengers.end(); ++itr)
+    for (PassengerSet::iterator itr = passengers.begin(); itr != passengers.end(); ++itr)
     {
         WorldObject* passenger = *itr;
         // transport teleported but passenger not yet (can happen for players)
diff --git a/src/server/game/Entities/Transport/Transport.h b/src/server/game/Entities/Transport/Transport.h
index 293d4334a2e..e644417f1ac 100644
--- a/src/server/game/Entities/Transport/Transport.h
+++ b/src/server/game/Entities/Transport/Transport.h
@@ -31,6 +31,8 @@ class Transport : public GameObject, public TransportBase
 
         Transport();
     public:
+        typedef std::set<WorldObject*> PassengerSet;
+
         ~Transport();
 
         bool Create(uint32 guidlow, uint32 entry, uint32 mapid, float x, float y, float z, float ang, uint32 animprogress);
@@ -42,7 +44,7 @@ class Transport : public GameObject, public TransportBase
 
         void AddPassenger(WorldObject* passenger);
         void RemovePassenger(WorldObject* passenger);
-        std::set<WorldObject*> const& GetPassengers() const { return _passengers; }
+        PassengerSet const& GetPassengers() const { return _passengers; }
 
         Creature* CreateNPCPassenger(uint32 guid, CreatureData const* data);
         GameObject* CreateGOPassenger(uint32 guid, GameObjectData const* data);
@@ -99,7 +101,7 @@ class Transport : public GameObject, public TransportBase
         void MoveToNextWaypoint();
         float CalculateSegmentPos(float perc);
         bool TeleportTransport(uint32 newMapid, float x, float y, float z, float o);
-        void UpdatePassengerPositions(std::set<WorldObject*>& passengers);
+        void UpdatePassengerPositions(PassengerSet& passengers);
         void DoEventIfAny(KeyFrame const& node, bool departure);
 
         //! Helpers to know if stop frame was reached
@@ -118,8 +120,9 @@ class Transport : public GameObject, public TransportBase
         bool _triggeredArrivalEvent;
         bool _triggeredDepartureEvent;
 
-        std::set<WorldObject*> _passengers;
-        std::set<WorldObject*> _staticPassengers;
+        PassengerSet _passengers;
+        PassengerSet::iterator _passengerTeleportItr;
+        PassengerSet _staticPassengers;
 };
 
 #endif
diff --git a/src/server/game/Entities/Unit/Unit.cpp b/src/server/game/Entities/Unit/Unit.cpp
index e82790df46a..c0f179eb504 100644
--- a/src/server/game/Entities/Unit/Unit.cpp
+++ b/src/server/game/Entities/Unit/Unit.cpp
@@ -13555,12 +13555,7 @@ void Unit::CleanupsBeforeDelete(bool finalCleanup)
 {
     CleanupBeforeRemoveFromMap(finalCleanup);
 
-    if (GetTransport())
-    {
-        GetTransport()->RemovePassenger(this);
-        SetTransport(NULL);
-        m_movementInfo.transport.Reset();
-    }
+    WorldObject::CleanupsBeforeDelete(finalCleanup);
 }
 
 void Unit::UpdateCharmAI()
diff --git a/src/server/game/Entities/Unit/Unit.h b/src/server/game/Entities/Unit/Unit.h
index 20c987c5105..103f6b596d9 100644
--- a/src/server/game/Entities/Unit/Unit.h
+++ b/src/server/game/Entities/Unit/Unit.h
@@ -1289,7 +1289,7 @@ class Unit : public WorldObject
         void RemoveFromWorld();
 
         void CleanupBeforeRemoveFromMap(bool finalCleanup);
-        void CleanupsBeforeDelete(bool finalCleanup = true);                        // used in ~Creature/~Player (or before mass creature delete to remove cross-references to already deleted units)
+        void CleanupsBeforeDelete(bool finalCleanup = true) override;                        // used in ~Creature/~Player (or before mass creature delete to remove cross-references to already deleted units)
 
         DiminishingLevels GetDiminishing(DiminishingGroup  group);
         void IncrDiminishing(DiminishingGroup group);