Core/Vehicles: Refactor some parts of vehicle system

- Fix a few crashes:
Closes #9235
- Simplify and get rid of lots of redundant and silly code
- Optimisations in Unit::ChangeSeat
- More documentation

src/server/game/Entities/Creature/Creature.cpp

@@ -1270,14 +1270,14 @@ bool Creature::CreateFromProto(uint32 guidlow, uint32 Entry, uint32 vehId, uint3
     if (!vehId)
         vehId = cinfo->VehicleId;
 
-    if (vehId && !CreateVehicleKit(vehId, Entry))
-        vehId = 0;
-
     Object::_Create(guidlow, Entry, vehId ? HIGHGUID_VEHICLE : HIGHGUID_UNIT);
 
     if (!UpdateEntry(Entry, team, data))
         return false;
 
+    if (vehId && !CreateVehicleKit(vehId, Entry))
+        vehId = 0;
+
     return true;
 }
 

src/server/game/Entities/Unit/Unit.cpp

@@ -11388,8 +11388,6 @@ void Unit::Mount(uint32 mount, uint32 VehicleId, uint32 creatureEntry)
         {
             if (CreateVehicleKit(VehicleId, creatureEntry))
             {
-                GetVehicleKit()->Reset();
-
                 // Send others that we now have a vehicle
                 WorldPacket data(SMSG_PLAYER_VEHICLE_DATA, GetPackGUID().size()+4);
                 data.appendPackGUID(GetGUID());
@@ -15458,7 +15456,10 @@ bool Unit::SetCharmedBy(Unit* charmer, CharmType type, AuraApplication const* au
 
     // dismount players when charmed
     if (GetTypeId() == TYPEID_PLAYER)
-        Dismount();
+        RemoveAurasByType(SPELL_AURA_MOUNTED);
+
+    if (charmer->GetTypeId() == TYPEID_PLAYER)
+        charmer->RemoveAurasByType(SPELL_AURA_MOUNTED);
 
     ASSERT(type != CHARM_TYPE_POSSESS || charmer->GetTypeId() == TYPEID_PLAYER);
     ASSERT((type == CHARM_TYPE_VEHICLE) == IsVehicle());
@@ -16581,26 +16582,9 @@ void Unit::_EnterVehicle(Vehicle* vehicle, int8 seatId, AuraApplication const* a
     {
         if (vehicle->GetBase()->GetTypeId() == TYPEID_PLAYER && player->isInCombat())
             return;
-
-        InterruptNonMeleeSpells(false);
-        player->StopCastingCharm();
-        player->StopCastingBindSight();
-        Dismount();
-        RemoveAurasByType(SPELL_AURA_MOUNTED);
-
-        // drop flag at invisible in bg
-        if (Battleground* bg = player->GetBattleground())
-            bg->EventPlayerDroppedFlag(player);
-
-        WorldPacket data(SMSG_ON_CANCEL_EXPECTED_RIDE_VEHICLE_AURA, 0);
-        player->GetSession()->SendPacket(&data);
-
-        player->UnsummonPetTemporaryIfAny();
     }
 
     ASSERT(!m_vehicle);
-
-
     (void)vehicle->AddPassenger(this, seatId);
 }
 
@@ -16609,15 +16593,17 @@ void Unit::ChangeSeat(int8 seatId, bool next)
     if (!m_vehicle)
         return;
 
-    if (seatId < 0)
-    {
-        seatId = m_vehicle->GetNextEmptySeat(GetTransSeat(), next);
-        if (seatId < 0)
-            return;
-    }
-    else if (seatId == GetTransSeat() || !m_vehicle->HasEmptySeat(seatId))
+    // Don't change if current and new seat are identical
+    if (seatId == GetTransSeat())
         return;
 
+    SeatMap::const_iterator seat = (seatId < 0 ? m_vehicle->GetNextEmptySeat(GetTransSeat(), next) : m_vehicle->Seats.find(seatId));
+    // The second part of the check will only return true if seatId >= 0. @Vehicle::GetNextEmptySeat makes sure of that.
+    if (seat == m_vehicle->Seats.end() || seat->second.Passenger)
+        return;
+    
+    // Todo: the functions below could be consolidated and refactored to take
+    // SeatMap::const_iterator as parameter, to save redundant map lookups.
     m_vehicle->RemovePassenger(this);
     if (!m_vehicle->AddPassenger(this, seatId))
         ASSERT(false);
@@ -16644,6 +16630,9 @@ void Unit::ExitVehicle(Position const* /*exitPosition*/)
 
 void Unit::_ExitVehicle(Position const* exitPosition)
 {
+    /// It's possible m_vehicle is NULL, when this function is called indirectly from @VehicleJoinEvent::Abort.
+    /// In that case it was not possible to add the passenger to the vehicle. The vehicle aura has already been removed
+    /// from the target in the aforementioned function and we don't need to do anything else at this point.
     if (!m_vehicle)
         return;
 

src/server/game/Entities/Vehicle/Vehicle.cpp

@@ -31,6 +31,8 @@
 #include "MoveSplineInit.h"
 #include "TemporarySummon.h"
 #include "EventProcessor.h"
+#include "Player.h"
+#include "Battleground.h"
 
 Vehicle::Vehicle(Unit* unit, VehicleEntry const* vehInfo, uint32 creatureEntry) :
 _me(unit), _vehicleInfo(vehInfo), UsableSeatNum(0), _creatureEntry(creatureEntry), _status(STATUS_NONE)
@@ -46,6 +48,12 @@ _me(unit), _vehicleInfo(vehInfo), UsableSeatNum(0), _creatureEntry(creatureEntry
             }
     }
 
+    // Set or remove correct flags based on available seats. Will overwrite db data (if wrong).
+    if (UsableSeatNum)
+        _me->SetFlag(UNIT_NPC_FLAGS, (_me->GetTypeId() == TYPEID_PLAYER ? UNIT_NPC_FLAG_PLAYER_VEHICLE : UNIT_NPC_FLAG_SPELLCLICK));
+    else
+        _me->RemoveFlag(UNIT_NPC_FLAGS, (_me->GetTypeId() == TYPEID_PLAYER ? UNIT_NPC_FLAG_PLAYER_VEHICLE : UNIT_NPC_FLAG_SPELLCLICK));
+
     InitMovementInfoForBase();
 }
 
@@ -57,6 +65,15 @@ Vehicle::~Vehicle()
         ASSERT(!itr->second.Passenger);
 }
 
+/**
+ * @fn void Vehicle::Install()
+ *
+ * @brief Initializes power type for vehicle. Nothing more.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ */
+
 void Vehicle::Install()
 {
     if (Creature* creature = _me->ToCreature())
@@ -115,10 +132,20 @@ void Vehicle::InstallAllAccessories(bool evading)
             InstallAccessory(itr->AccessoryEntry, itr->SeatId, itr->IsMinion, itr->SummonedType, itr->SummonTime);
 }
 
+/**
+ * @fn void Vehicle::Uninstall()
+ *
+ * @brief Removes all passengers and sets status to STATUS_UNINSTALLING.
+ * 		  No new passengers can be added to the vehicle after this call.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ */
+
 void Vehicle::Uninstall()
 {
     /// @Prevent recursive uninstall call. (Bad script in OnUninstall/OnRemovePassenger/PassengerBoarded hook.)
-    if (_status == STATUS_UNINSTALLING)
+    if (_status == STATUS_UNINSTALLING && !GetBase()->HasUnitTypeMask(UNIT_MASK_MINION))
     {
         sLog->outError(LOG_FILTER_VEHICLES, "Vehicle GuidLow: %u, Entry: %u attempts to uninstall, but already has STATUS_UNINSTALLING! "
             "Check Uninstall/PassengerBoarded script hooks for errors.", _me->GetGUIDLow(), _me->GetEntry());
@@ -129,31 +156,43 @@ void Vehicle::Uninstall()
     sLog->outDebug(LOG_FILTER_VEHICLES, "Vehicle::Uninstall Entry: %u, GuidLow: %u", _creatureEntry, _me->GetGUIDLow());
     RemoveAllPassengers();
 
-
     if (GetBase()->GetTypeId() == TYPEID_UNIT)
         sScriptMgr->OnUninstall(this);
 }
 
+/**
+ * @fn void Vehicle::Reset(bool evading )
+ *
+ * @brief Reapplies immunities and reinstalls accessories. Only has effect for creatures.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param evading true if called from CreatureAI::EnterEvadeMode
+ */
+
 void Vehicle::Reset(bool evading /*= false*/)
 {
-    sLog->outDebug(LOG_FILTER_VEHICLES, "Vehicle::Reset Entry: %u, GuidLow: %u", _creatureEntry, _me->GetGUIDLow());
-    if (_me->GetTypeId() == TYPEID_PLAYER)
-    {
-        if (UsableSeatNum)
-            _me->SetFlag(UNIT_NPC_FLAGS, UNIT_NPC_FLAG_PLAYER_VEHICLE);
-    }
-    else
-    {
-        ApplyAllImmunities();
-        InstallAllAccessories(evading);
-        if (UsableSeatNum)
-            _me->SetFlag(UNIT_NPC_FLAGS, UNIT_NPC_FLAG_SPELLCLICK);
-    }
+    if (GetBase()->GetTypeId() != TYPEID_UNIT)
+        return;
 
-    if (GetBase()->GetTypeId() == TYPEID_UNIT)
-        sScriptMgr->OnReset(this);
+    sLog->outDebug(LOG_FILTER_VEHICLES, "Vehicle::Reset (Entry: %u, GuidLow: %u, DBGuid: %u)", GetCreatureEntry(), _me->GetGUIDLow(), _me->ToCreature()->GetDBTableGUIDLow());
+    
+    ApplyAllImmunities();
+    InstallAllAccessories(evading);
+
+    sScriptMgr->OnReset(this);
 }
 
+/**
+ * @fn void Vehicle::ApplyAllImmunities()
+ *
+ * @brief Applies specific immunities that cannot be set in DB.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ */
+
 void Vehicle::ApplyAllImmunities()
 {
     // This couldn't be done in DB, because some spells have MECHANIC_NONE
@@ -202,10 +241,28 @@ void Vehicle::ApplyAllImmunities()
     }
 }
 
+/**
+ * @fn void Vehicle::RemoveAllPassengers()
+ *
+ * @brief Removes all current and pending passengers from the vehicle.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ */
+
 void Vehicle::RemoveAllPassengers()
 {
     sLog->outDebug(LOG_FILTER_VEHICLES, "Vehicle::RemoveAllPassengers. Entry: %u, GuidLow: %u", _creatureEntry, _me->GetGUIDLow());
 
+    /// Setting to_Abort to true will cause @VehicleJoinEvent::Abort to be executed on next @Unit::UpdateEvents call
+    /// This will properly "reset" the pending join process for the passenger.
+    while (_pendingJoinEvents.size())
+    {
+        VehicleJoinEvent* e = _pendingJoinEvents.front();
+        e->to_Abort = true;
+        _pendingJoinEvents.pop_front();
+    }
+
     // Passengers always cast an aura with SPELL_AURA_CONTROL_VEHICLE on the vehicle
     // We just remove the aura and the unapply handler will make the target leave the vehicle.
     // We don't need to iterate over Seats
@@ -218,18 +275,18 @@ void Vehicle::RemoveAllPassengers()
     //    ASSERT(!itr->second.passenger);
 }
 
-void Vehicle::RemovePendingPassengers()
-{
-    sLog->outDebug(LOG_FILTER_VEHICLES, "Vehicle::RemovePendingPassengers. Entry: %u, GuidLow: %u", _creatureEntry, _me->GetGUIDLow());
-
-    /// Setting to_Abort to true will cause @VehicleJoinEvent::Abort to be executed on next @Unit::UpdateEvents call
-    /// This will properly "reset" the pending join process for the passenger.
-    while (VehicleJoinEvent* e = _pendingJoinEvents.front())
-    {
-        e->to_Abort = true;
-        _pendingJoinEvents.pop_front();
-    }
-}
+/**
+ * @fn bool Vehicle::HasEmptySeat(int8 seatId) const
+ *
+ * @brief Checks if vehicle's seat specified by 'seatId' is empty.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param seatId Identifier for the seat.
+ *
+ * @return true if empty seat, false if not.
+ */
 
 bool Vehicle::HasEmptySeat(int8 seatId) const
 {
@@ -239,6 +296,19 @@ bool Vehicle::HasEmptySeat(int8 seatId) const
     return !seat->second.Passenger;
 }
 
+/**
+ * @fn Unit* Vehicle::GetPassenger(int8 seatId) const
+ *
+ * @brief Gets a passenger on specified seat.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param seatId Seat to look on.
+ *
+ * @return null if it not found, else pointer to passenger if in world
+ */
+
 Unit* Vehicle::GetPassenger(int8 seatId) const
 {
     SeatMap::const_iterator seat = Seats.find(seatId);
@@ -248,78 +318,105 @@ Unit* Vehicle::GetPassenger(int8 seatId) const
     return ObjectAccessor::GetUnit(*GetBase(), seat->second.Passenger);
 }
 
-int8 Vehicle::GetNextEmptySeat(int8 seatId, bool next) const
+/**
+ * @fn SeatMap::const_iterator Vehicle::GetNextEmptySeat(int8 seatId, bool next) const
+ *
+ * @brief Gets the next empty seat based on current seat.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param seatId Identifier for the current seat.
+ * @param next   true if iterating forward, false means iterating backwards.
+ *
+ * @return The next empty seat.
+ */
+
+SeatMap::const_iterator Vehicle::GetNextEmptySeat(int8 seatId, bool next) const
 {
     SeatMap::const_iterator seat = Seats.find(seatId);
     if (seat == Seats.end())
-        return -1;
+        return seat;
 
     while (seat->second.Passenger || (!seat->second.SeatInfo->CanEnterOrExit() && !seat->second.SeatInfo->IsUsableByOverride()))
     {
         if (next)
         {
-            ++seat;
-            if (seat == Seats.end())
+            if (++seat == Seats.end())
                 seat = Seats.begin();
         }
         else
         {
-            if (seat == Seats.begin())
+            if (seat-- == Seats.begin())
                 seat = Seats.end();
-            --seat;
         }
 
+        // Make sure we don't loop indefinetly
         if (seat->first == seatId)
-            return -1; // no available seat
+            return Seats.end();
     }
 
-    return seat->first;
+    return seat;
 }
 
+/**
+ * @fn void Vehicle::InstallAccessory(uint32 entry, int8 seatId, bool minion, uint8 type,
+ *     uint32 summonTime)
+ *
+ * @brief Installs an accessory.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param entry      The NPC entry of accessory.
+ * @param seatId     Identifier for the seat to add the accessory to.
+ * @param minion     true if accessory considered a 'minion'. Implies that the accessory will despawn when the vehicle despawns. 
+ * 					 Essentially that it has no life without the vehicle. Their fates are bound.
+ * @param type       See enum @SummonType.
+ * @param summonTime Time after which the minion is despawned in case of a timed despawn @type specified.
+ */
+
 void Vehicle::InstallAccessory(uint32 entry, int8 seatId, bool minion, uint8 type, uint32 summonTime)
 {
     /// @Prevent adding accessories when vehicle is uninstalling. (Bad script in OnUninstall/OnRemovePassenger/PassengerBoarded hook.)
     if (_status == STATUS_UNINSTALLING)
     {
-        sLog->outError(LOG_FILTER_VEHICLES, "Vehicle GuidLow: %u, Entry: %u attempts to install accessory Entry: %u on seat %d with STATUS_UNINSTALLING! "
-            "Check Uninstall/PassengerBoarded script hooks for errors.", _me->GetGUIDLow(), _me->GetEntry(), entry, (int32)seatId);
+        sLog->outError(LOG_FILTER_VEHICLES, "Vehicle (GuidLow: %u, DB GUID: %u, Entry: %u) attempts to install accessory (Entry: %u) on seat %i with STATUS_UNINSTALLING! "
+            "Check Uninstall/PassengerBoarded script hooks for errors.", _me->GetGUIDLow(), 
+            (_me->GetTypeId() == TYPEID_UNIT ? _me->ToCreature()->GetDBTableGUIDLow() : _me->GetGUIDLow()), GetCreatureEntry(), entry, (int32)seatId);
         return;
     }
 
-    sLog->outDebug(LOG_FILTER_VEHICLES, "Vehicle: Installing accessory entry %u on vehicle entry %u (seat:%i)", entry, GetCreatureEntry(), seatId);
-    if (Unit* passenger = GetPassenger(seatId))
-    {
-        // already installed
-        if (passenger->GetEntry() == entry)
-        {
-            ASSERT(passenger->GetTypeId() == TYPEID_UNIT);
-            if (_me->GetTypeId() == TYPEID_UNIT)
-            {
-                if (_me->ToCreature()->IsInEvadeMode() && passenger->ToCreature()->IsAIEnabled)
-                    passenger->ToCreature()->AI()->EnterEvadeMode();
-                return;
-            }
-        }
-        else
-            passenger->ExitVehicle(); // this should not happen
-    }
+    sLog->outDebug(LOG_FILTER_VEHICLES, "Vehicle (GuidLow: %u, DB Guid: %u, Entry %u): installing accessory (Entry: %u) on seat: %i",
+        _me->GetGUIDLow(), (_me->GetTypeId() == TYPEID_UNIT ? _me->ToCreature()->GetDBTableGUIDLow() : _me->GetGUIDLow()), GetCreatureEntry(),
+        entry, (int32)seatId);
 
-    if (TempSummon* accessory = _me->SummonCreature(entry, *_me, TempSummonType(type), summonTime))
-    {
-        if (minion)
-            accessory->AddUnitTypeMask(UNIT_MASK_ACCESSORY);
+    TempSummon* accessory = _me->SummonCreature(entry, *_me, TempSummonType(type), summonTime);
+    ASSERT(accessory);
 
-        if (!_me->HandleSpellClick(accessory, seatId))
-        {
-            accessory->UnSummon();
-            return;
-        }
+    if (minion)
+        accessory->AddUnitTypeMask(UNIT_MASK_ACCESSORY);
 
-        if (GetBase()->GetTypeId() == TYPEID_UNIT)
-            sScriptMgr->OnInstallAccessory(this, accessory);
-    }
+    (void)_me->HandleSpellClick(accessory, seatId);
+
+    /// If for some reason adding accessory to vehicle fails it will unsummon in
+    /// @VehicleJoinEvent::Abort
 }
 
+/**
+ * @fn bool Vehicle::AddPassenger(Unit* unit, int8 seatId)
+ *
+ * @brief Attempts to add a passenger to the vehicle on 'seatId'.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param [in,out] The prospective passenger.
+ * @param seatId        Identifier for the seat. Value of -1 indicates the next available seat.
+ *
+ * @return true if it succeeds, false if it fails.
+ */
+
 bool Vehicle::AddPassenger(Unit* unit, int8 seatId)
 {
     /// @Prevent adding passengers when vehicle is uninstalling. (Bad script in OnUninstall/OnRemovePassenger/PassengerBoarded hook.)
@@ -330,16 +427,15 @@ bool Vehicle::AddPassenger(Unit* unit, int8 seatId)
         return false;
     }
 
-    if (unit->GetVehicle() != this)
-        return false;
+    sLog->outDebug(LOG_FILTER_VEHICLES, "Unit %s scheduling enter vehicle (entry: %u, vehicleId: %u, guid: %u (dbguid: %s) on seat %d",
+        unit->GetName().c_str(), _me->GetEntry(), _vehicleInfo->m_ID, _me->GetGUIDLow(), 
+        (_me->GetTypeId() == TYPEID_UNIT ? _me->ToCreature()->GetDBTableGUIDLow() : 0), seatId);
 
     // The seat selection code may kick other passengers off the vehicle.
     // While the validity of the following may be arguable, it is possible that when such a passenger
     // exits the vehicle will dismiss. That's why the actual adding the passenger to the vehicle is scheduled
     // asynchronously, so it can be cancelled easily in case the vehicle is uninstalled meanwhile.
     SeatMap::iterator seat;
-    sLog->outDebug(LOG_FILTER_VEHICLES, "Unit %s scheduling enter vehicle entry %u id %u dbguid %u seat %d",
-        unit->GetName().c_str(), _me->GetEntry(), _vehicleInfo->m_ID, _me->GetGUIDLow(), seatId);
     VehicleJoinEvent* e = new VehicleJoinEvent(this, unit);
     unit->m_Events.AddEvent(e, unit->m_Events.CalculateTime(0));
     _pendingJoinEvents.push_back(e);
@@ -370,10 +466,9 @@ bool Vehicle::AddPassenger(Unit* unit, int8 seatId)
         e->Seat = seat;
         if (seat->second.Passenger)
         {
-            if (Unit* passenger = ObjectAccessor::GetUnit(*GetBase(), seat->second.Passenger))
-                passenger->ExitVehicle();
-            else
-                seat->second.Passenger = 0;
+            Unit* passenger = ObjectAccessor::GetUnit(*GetBase(), seat->second.Passenger);
+            ASSERT(passenger);
+            passenger->ExitVehicle();
         }
 
         ASSERT(!seat->second.Passenger);
@@ -382,6 +477,17 @@ bool Vehicle::AddPassenger(Unit* unit, int8 seatId)
     return true;
 }
 
+/**
+ * @fn void Vehicle::RemovePassenger(Unit* unit)
+ *
+ * @brief Removes the passenger from the vehicle.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param [in,out] unit The passenger to remove..
+ */
+
 void Vehicle::RemovePassenger(Unit* unit)
 {
     if (unit->GetVehicle() != this)
@@ -390,20 +496,12 @@ void Vehicle::RemovePassenger(Unit* unit)
     SeatMap::iterator seat = GetSeatIteratorForPassenger(unit);
     ASSERT(seat != Seats.end());
 
-    sLog->outDebug(LOG_FILTER_VEHICLES, "Unit %s exit vehicle entry %u id %u dbguid %u seat %d", unit->GetName().c_str(), _me->GetEntry(), _vehicleInfo->m_ID, _me->GetGUIDLow(), (int32)seat->first);
+    sLog->outDebug(LOG_FILTER_VEHICLES, "Unit %s exit vehicle entry %u id %u dbguid %u seat %d", 
+        unit->GetName().c_str(), _me->GetEntry(), _vehicleInfo->m_ID, _me->GetGUIDLow(), (int32)seat->first);
 
     seat->second.Passenger = 0;
-    if (seat->second.SeatInfo->CanEnterOrExit())
-    {
-        if (!UsableSeatNum)
-        {
-            if (_me->GetTypeId() == TYPEID_PLAYER)
-                _me->SetFlag(UNIT_NPC_FLAGS, UNIT_NPC_FLAG_PLAYER_VEHICLE);
-            else
-                _me->SetFlag(UNIT_NPC_FLAGS, UNIT_NPC_FLAG_SPELLCLICK);
-        }
-        ++UsableSeatNum;
-    }
+    if (seat->second.SeatInfo->CanEnterOrExit() && ++UsableSeatNum)
+        _me->SetFlag(UNIT_NPC_FLAGS, (_me->GetTypeId() == TYPEID_PLAYER ? UNIT_NPC_FLAG_PLAYER_VEHICLE : UNIT_NPC_FLAG_SPELLCLICK));
 
     unit->ClearUnitState(UNIT_STATE_ONVEHICLE);
 
@@ -413,7 +511,7 @@ void Vehicle::RemovePassenger(Unit* unit)
     if (_me->IsInWorld())
     {
         unit->RemoveUnitMovementFlag(MOVEMENTFLAG_ONTRANSPORT);
-        unit->m_movementInfo.t_pos.Relocate(0, 0, 0, 0);
+        unit->m_movementInfo.t_pos.Relocate(0.0f, 0.0f, 0.0f, 0.0f);
         unit->m_movementInfo.t_time = 0;
         unit->m_movementInfo.t_seat = 0;
     }
@@ -429,7 +527,15 @@ void Vehicle::RemovePassenger(Unit* unit)
         sScriptMgr->OnRemovePassenger(this, unit);
 }
 
-//! Must be called after m_base::Relocate
+/**
+ * @fn void Vehicle::RelocatePassengers()
+ *
+ * @brief Relocate passengers. Must be called after m_base::Relocate
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ */
+
 void Vehicle::RelocatePassengers()
 {
     ASSERT(_me->GetMap());
@@ -449,16 +555,34 @@ void Vehicle::RelocatePassengers()
     }
 }
 
+/**
+ * @fn void Vehicle::Dismiss()
+ *
+ * @brief Dismiss the vehicle. Removes passengers and despawns self. Only valid for creatures.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ */
+
 void Vehicle::Dismiss()
 {
     if (GetBase()->GetTypeId() != TYPEID_UNIT)
         return;
 
-    sLog->outDebug(LOG_FILTER_VEHICLES, "Vehicle::Dismiss Entry: %u, GuidLow %u", _creatureEntry, _me->GetGUIDLow());
+    sLog->outDebug(LOG_FILTER_VEHICLES, "Vehicle::Dismiss Entry: %u, GuidLow %u, DBGuid: %u", GetCreatureEntry(), _me->GetGUIDLow(), _me->ToCreature()->GetDBTableGUIDLow());
     Uninstall();
     GetBase()->ToCreature()->DespawnOrUnsummon();
 }
 
+/**
+ * @fn void Vehicle::InitMovementInfoForBase()
+ *
+ * @brief Sets correct MovementFlags2 based on VehicleFlags from DBC.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ */
+
 void Vehicle::InitMovementInfoForBase()
 {
     uint32 vehicleFlags = GetVehicleInfo()->m_flags;
@@ -475,6 +599,19 @@ void Vehicle::InitMovementInfoForBase()
         _me->AddExtraUnitMovementFlag(MOVEMENTFLAG2_FULL_SPEED_PITCHING);
 }
 
+/**
+ * @fn VehicleSeatEntry const* Vehicle::GetSeatForPassenger(Unit* passenger)
+ *
+ * @brief Returns information on the seat of specified passenger, represented by the format in VehicleSeat.dbc
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param [in,out] The passenger for which we check the seat info.
+ *
+ * @return null if passenger not found on vehicle, else the DBC record for the seat.
+ */
+
 VehicleSeatEntry const* Vehicle::GetSeatForPassenger(Unit* passenger)
 {
     SeatMap::iterator itr;
@@ -485,6 +622,19 @@ VehicleSeatEntry const* Vehicle::GetSeatForPassenger(Unit* passenger)
     return NULL;
 }
 
+/**
+ * @fn SeatMap::iterator Vehicle::GetSeatIteratorForPassenger(Unit* passenger)
+ *
+ * @brief Gets seat iterator for specified passenger.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param [in,out] passenger Passenger to look up.
+ *
+ * @return The seat iterator for specified passenger if it's found on the vehicle. Otherwise Seats.end() (invalid iterator).
+ */
+
 SeatMap::iterator Vehicle::GetSeatIteratorForPassenger(Unit* passenger)
 {
     SeatMap::iterator itr;
@@ -495,6 +645,17 @@ SeatMap::iterator Vehicle::GetSeatIteratorForPassenger(Unit* passenger)
     return Seats.end();
 }
 
+/**
+ * @fn uint8 Vehicle::GetAvailableSeatCount() const
+ *
+ * @brief Gets the available seat count.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @return The available seat count.
+ */
+
 uint8 Vehicle::GetAvailableSeatCount() const
 {
     uint8 ret = 0;
@@ -526,17 +687,45 @@ void Vehicle::CalculatePassengerOffset(float& x, float& y, float& z, float& o)
     x = (inx + iny * tan(GetBase()->GetOrientation())) / (cos(GetBase()->GetOrientation()) + std::sin(GetBase()->GetOrientation()) * tan(GetBase()->GetOrientation()));
 }
 
+/**
+ * @fn void Vehicle::CancelJoinEvent(VehicleJoinEvent* e)
+ *
+ * @brief Aborts delayed @VehicleJoinEvent objects. 
+ * 		  Implies that the related unit will not be boarding the vehicle after all.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param [in,out] e The VehicleJoinEvent* to process.
+ */
+
 void Vehicle::CancelJoinEvent(VehicleJoinEvent* e)
 {
     e->to_Abort = true;
     _pendingJoinEvents.pop_back();
 }
 
+/**
+ * @fn bool VehicleJoinEvent::Execute(uint64, uint32)
+ *
+ * @brief Actually adds the passenger @Passenger to vehicle @Target.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param parameter1 Unused
+ * @param parameter2 Unused.
+ *
+ * @return true, cannot fail.
+ * 
+ */
+
 bool VehicleJoinEvent::Execute(uint64, uint32)
 {
-    ASSERT(Passenger->GetVehicle() == Target);
-    Passenger->m_vehicle = Target;
+    ASSERT(Passenger->IsInWorld());
+    ASSERT(Target->GetBase()->IsInWorld());
 
+    Passenger->m_vehicle = Target;
     Seat->second.Passenger = Passenger->GetGUID();
     if (Seat->second.SeatInfo->CanEnterOrExit())
     {
@@ -551,55 +740,85 @@ bool VehicleJoinEvent::Execute(uint64, uint32)
         }
     }
 
+    Passenger->InterruptNonMeleeSpells(false);
+
+    Player* player = Passenger->ToPlayer();
+    if (player)
+    {
+        // drop flag 
+        if (Battleground* bg = player->GetBattleground())
+            bg->EventPlayerDroppedFlag(player);
+
+        WorldPacket data(SMSG_ON_CANCEL_EXPECTED_RIDE_VEHICLE_AURA, 0);
+        player->GetSession()->SendPacket(&data);
+        player->UnsummonPetTemporaryIfAny();
+    }
+
     if (Seat->second.SeatInfo->m_flags && !(Seat->second.SeatInfo->m_flags & VEHICLE_SEAT_FLAG_ALLOW_TURNING))
         Passenger->AddUnitState(UNIT_STATE_ONVEHICLE);
 
     Passenger->AddUnitMovementFlag(MOVEMENTFLAG_ONTRANSPORT);
     VehicleSeatEntry const* veSeat = Seat->second.SeatInfo;
-    Passenger->m_movementInfo.t_pos.m_positionX = veSeat->m_attachmentOffsetX;
-    Passenger->m_movementInfo.t_pos.m_positionY = veSeat->m_attachmentOffsetY;
-    Passenger->m_movementInfo.t_pos.m_positionZ = veSeat->m_attachmentOffsetZ;
-    Passenger->m_movementInfo.t_pos.SetOrientation(0.0f);
+    Passenger->m_movementInfo.t_pos.Relocate(veSeat->m_attachmentOffsetX, veSeat->m_attachmentOffsetY, veSeat->m_attachmentOffsetZ);
     Passenger->m_movementInfo.t_time = 0; // 1 for player
     Passenger->m_movementInfo.t_seat = Seat->first;
     Passenger->m_movementInfo.t_guid = Target->GetBase()->GetGUID();
 
     if (Target->GetBase()->GetTypeId() == TYPEID_UNIT && Passenger->GetTypeId() == TYPEID_PLAYER &&
         Seat->second.SeatInfo->m_flags & VEHICLE_SEAT_FLAG_CAN_CONTROL)
-        ASSERT(Target->GetBase()->SetCharmedBy(Passenger, CHARM_TYPE_VEHICLE))
+        ASSERT(Target->GetBase()->SetCharmedBy(Passenger, CHARM_TYPE_VEHICLE))  // SMSG_CLIENT_CONTROL
 
-    if (Target->GetBase()->IsInWorld())
-    {
-        Passenger->SendClearTarget();                            // SMSG_BREAK_TARGET
-        Passenger->SetControlled(true, UNIT_STATE_ROOT);         // SMSG_FORCE_ROOT - In some cases we send SMSG_SPLINE_MOVE_ROOT here (for creatures)
-        // also adds MOVEMENTFLAG_ROOT
-        Movement::MoveSplineInit init(Passenger);
-        init.DisableTransportPathTransformations();
-        init.MoveTo(veSeat->m_attachmentOffsetX, veSeat->m_attachmentOffsetY, veSeat->m_attachmentOffsetZ);
-        init.SetFacing(0.0f);
-        init.SetTransportEnter();
-        init.Launch();
-
-        if (Target->GetBase()->GetTypeId() == TYPEID_UNIT)
-        {
-            if (Target->GetBase()->ToCreature()->IsAIEnabled)
-                Target->GetBase()->ToCreature()->AI()->PassengerBoarded(Passenger, Seat->first, true);
-
-            // update all passenger's positions
-            //Passenger's spline OR vehicle movement will update positions
-            //RelocatePassengers(_me->GetPositionX(), _me->GetPositionY(), _me->GetPositionZ(), _me->GetOrientation());
-        }
-    }
+    Passenger->SendClearTarget();                            // SMSG_BREAK_TARGET
+    Passenger->SetControlled(true, UNIT_STATE_ROOT);         // SMSG_FORCE_ROOT - In some cases we send SMSG_SPLINE_MOVE_ROOT here (for creatures)
+    // also adds MOVEMENTFLAG_ROOT
+    
+    Movement::MoveSplineInit init(Passenger);
+    init.DisableTransportPathTransformations();
+    init.MoveTo(veSeat->m_attachmentOffsetX, veSeat->m_attachmentOffsetY, veSeat->m_attachmentOffsetZ);
+    init.SetFacing(0.0f);
+    init.SetTransportEnter();
+    init.Launch();
 
     if (Target->GetBase()->GetTypeId() == TYPEID_UNIT)
+    {
+        if (Target->GetBase()->ToCreature()->IsAIEnabled)
+            Target->GetBase()->ToCreature()->AI()->PassengerBoarded(Passenger, Seat->first, true);
+
         sScriptMgr->OnAddPassenger(Target, Passenger, Seat->first);
 
+        // Actually quite a redundant hook. Could just use OnAddPassenger and check for unit typemask inside script.
+        if (Passenger->HasUnitTypeMask(UNIT_MASK_ACCESSORY))
+            sScriptMgr->OnInstallAccessory(Target, Passenger->ToCreature());
+
+        // update all passenger's positions
+        //Passenger's spline OR vehicle movement will update positions
+        //RelocatePassengers(_me->GetPositionX(), _me->GetPositionY(), _me->GetPositionZ(), _me->GetOrientation());
+    }
+
     return true;
 }
 
+/**
+ * @fn void VehicleJoinEvent::Abort(uint64)
+ *
+ * @brief Aborts the event. Implies that unit @Passenger will not be boarding vehicle @Target after all.
+ *
+ * @author Machiavelli
+ * @date 17-2-2013
+ *
+ * @param parameter1 Unused
+ */
+
 void VehicleJoinEvent::Abort(uint64)
 {
     sLog->outDebug(LOG_FILTER_VEHICLES, "Passenger GuidLow: %u, Entry: %u, board on vehicle GuidLow: %u, Entry: %u SeatId: %i cancelled",
         Passenger->GetGUIDLow(), Passenger->GetEntry(), Target->GetBase()->GetGUIDLow(), Target->GetBase()->GetEntry(), (int32)Seat->first);
-    // Passenger->m_vehicle = NULL;
+
+    /// @SPELL_AURA_CONTROL_VEHICLE auras can be applied even when the passenger is not (yet) on the vehicle. 
+    /// When this code is triggered it means that something went wrong in @Vehicle::AddPassenger, and we should remove
+    /// the aura manually.
+    Target->GetBase()->RemoveAurasByType(SPELL_AURA_CONTROL_VEHICLE, Passenger->GetGUID());
+    
+    if (Passenger->HasUnitTypeMask(UNIT_MASK_ACCESSORY))
+        Passenger->ToTempSummon()->UnSummon();
 }

src/server/game/Entities/Vehicle/Vehicle.h

@@ -46,7 +46,7 @@ class Vehicle : public TransportBase
 
         bool HasEmptySeat(int8 seatId) const;
         Unit* GetPassenger(int8 seatId) const;
-        int8 GetNextEmptySeat(int8 seatId, bool next) const;
+        SeatMap::const_iterator GetNextEmptySeat(int8 seatId, bool next) const;
         uint8 GetAvailableSeatCount() const;
 
         bool AddPassenger(Unit* passenger, int8 seatId = -1);
@@ -54,7 +54,6 @@ class Vehicle : public TransportBase
         void RemovePassenger(Unit* passenger);
         void RelocatePassengers();
         void RemoveAllPassengers();
-        void RemovePendingPassengers();
         void Dismiss();
         void TeleportVehicle(float x, float y, float z, float ang);
         bool IsVehicleInUse() { return Seats.begin() != Seats.end(); }
@@ -62,13 +61,13 @@ class Vehicle : public TransportBase
         void SetLastShootPos(Position const& pos) { m_lastShootPos.Relocate(pos); }
         Position GetLastShootPos() { return m_lastShootPos; }
 
-        SeatMap Seats;
+        SeatMap Seats;  ///< The collection of all seats on the vehicle. Including vacant ones.
 
         VehicleSeatEntry const* GetSeatForPassenger(Unit* passenger);
 
     protected:
         friend class VehicleJoinEvent;
-        uint32 UsableSeatNum;         // Number of seats that match VehicleSeatEntry::UsableByPlayer, used for proper display flags
+        uint32 UsableSeatNum;         ///< Number of seats that match VehicleSeatEntry::UsableByPlayer, used for proper display flags
 
     protected:
         friend bool Unit::CreateVehicleKit(uint32 id, uint32 creatureEntry);
@@ -93,15 +92,14 @@ class Vehicle : public TransportBase
         /// This method transforms supplied global coordinates into local offsets
         void CalculatePassengerOffset(float& x, float& y, float& z, float& o);
 
-        Unit* _me;
-        VehicleEntry const* _vehicleInfo;
+        Unit* _me;  ///< The underlying unit with the vehicle kit. Can be player or creature.
+        VehicleEntry const* _vehicleInfo;   ///< DBC data for vehicle
         GuidSet vehiclePlayers;
 
-        uint32 _creatureEntry;         // Can be different than me->GetBase()->GetEntry() in case of players
-        Status _status;
+        uint32 _creatureEntry;         ///< Can be different than the entry of _me in case of players
+        Status _status;     ///< Internal variable for sanity checks
         Position m_lastShootPos;
-
-        std::deque<VehicleJoinEvent*> _pendingJoinEvents;
+        std::deque<VehicleJoinEvent*> _pendingJoinEvents;   ///< Collection of delayed join events for prospective passengers
         void CancelJoinEvent(VehicleJoinEvent* e);
 };
 

src/server/game/Handlers/MiscHandler.cpp

@@ -1587,8 +1587,7 @@ void WorldSession::HandleCancelMountAuraOpcode(WorldPacket& /*recvData*/)
         return;
     }
 
-    _player->Dismount();
-    _player->RemoveAurasByType(SPELL_AURA_MOUNTED);
+    _player->RemoveAurasByType(SPELL_AURA_MOUNTED); // Calls Dismount()
 }
 
 void WorldSession::HandleMoveSetCanFlyAckOpcode(WorldPacket& recvData)