Trinitycore: GAMEOBJECT_TYPE_TRANSPORT should be handled as a real transport rather than just a gameobject

[Ovahlord]

Description:
After digging myself into the movement system I just found out why elevators are having a hard time in displaying their passengers properly. It is plain and simple that GAMEOBJECT_TYPE_TRANSPORT is not getting treated as a transport so every AddPassenger / RemovePassenger part is not working for it.

The evidence can be seen on the master branch: Checking the movement info packet information you will get the result that entering an elevator will return you a transport GUID. This will trigger several core checks for a transport but since TC handles GAMEOBJECT_TYPE_MO_TRANSPORT as real transports only every check will fail and so the movement packets will have missing transport information which ultimately causes the broken visuals.

Current behaviour:
The core does not create Transport entities for GAMEOBJECT_TYPE_TRANSPORT but instead normal gameobjects.

Expected behaviour:
GAMEOBJECT_TYPE_TRANSPORT should get the same treatment as GAMEOBJECT_TYPE_MO_TRANSPORT so both can be handled the same way.

Steps to reproduce the problem:

1. Pick a character and place him on a eleveator.
2. Pick a 2nd character and move into sight rage of the first player.
3. Observe the elevator movement. You will see that the passenger on the elevator will get stuck mid-air which indicates that the movement packet does not contain transport information due to the mentoined issue. The core checks for a transport by the movementinfo transport guid but wont find one because well... yeah.

There are two ways to approach this issue:

1. Create Transports instead of usual gameobjects for such transport types and move the transport start / stop procedure to the transport class
2. Trust the movementinfo cmsg packets and use them to send the transport information. This however will be a vulnerable spot for cheating.

Branch(es):
master, probably 335 as well if the handling works the same way

TC rev. hash/commit:
5a0ce669f0d8ae49ab9e3456eff4e1068e3c9189

Operating system: Win10 x64

[kelno]

As a note, Sunwell core had some work done towards separating Transport as MotionTransport (the regular transports) and StaticTransport (Elevators and such), might be worth a look

[Ovahlord]

Actually I have been porting exactly this (and did some renaming like MotionTransport --> MapTransport to reflect their actual transport type).
So far, the results are fine. Just tackling some more issues and I think I might try to refine and PR this

[xvwyh]

I can share the code I wrote to calculate the position and rotation of a GAMEOBJECT_TYPE_TRANSPORT that exactly matches the client, but it's only usable on 3.3.5 due to the changes to this kind of transports in cata and afterwards (multiple floors, between which the transport can freely move, in the case of 3.3.5 transports only move forward through their animation until it ends and snap back). I have both kinds of transports working pretty much as good as it can get, even with mmaps pathfinding on MO transports and NPCs being able to freely enter and leave the transport (e.g. player pets and companion pets).

Also something worth noting: HIGHGUID_TRANSPORT and HIGHGUID_MO_TRANSPORT are interchangeable as far as the client is concerned, but there's one thing that's different between the two: HIGHGUID_MO_TRANSPORT allows spells to be ground-targeted onto their surface (and will send transport-local position in cast packets) while the other one doesn't (client reports the target position to be out of range and prevents casting). Retail servers seemingly use HIGHGUID_MO_TRANSPORT for everything now, but don't quote me on this. Regardless, spawning GAMEOBJECT_TYPE_TRANSPORT transports with HIGHGUID_MO_TRANSPORT seems preferable, even if counter-intuitive, due to this additional functionality.

[Keader]

well @xvwyh if you can share the code, will be cool.

[xvwyh]

So this is essentially a complete replacement of Transport::Update for GAMEOBJECT_TYPE_TRANSPORT except for the AI update:

uint32& timer = m_goValue.Transport.PathProgress;

// Stoppable transport case
if (uint32 pauseTime = GetUInt32Value(GAMEOBJECT_LEVEL))
{
    uint32 periodTime = GetTransportPeriod();

    if (GetGoState() == GO_STATE_ACTIVE ? timer < pauseTime || timer == periodTime - 1 : timer < periodTime - 1)
    {
        timer += diff;

        if (GetGoState() == GO_STATE_ACTIVE) // Should transition from periodTime - 1 back to 0
            timer %= GetTransportPeriod();

        if (timer >= pauseTime && GetGoState() == GO_STATE_ACTIVE)
        {
            timer = pauseTime;
            _triggeredDepartureEvent = false;
            uint32 eventid = GetGOInfo()->transport.pause2EventID;
            if (!_triggeredArrivalEvent && eventid)
            {
                _triggeredArrivalEvent = true;
                GetMap()->ScriptsStart(sEventScripts, eventid, this, this);
                EventInform(eventid);
                sScriptMgr->OnEventInform(this, eventid);
            }
        }

        if (timer >= periodTime - 1 && GetGoState() == GO_STATE_READY)
        {
            timer = periodTime - 1; // Client expects 65535 in dynamic field to draw the transport as stopped on first frame, otherwise (if 0 is sent) it will play the animation backwards
            _triggeredArrivalEvent = false;
            uint32 eventid = GetGOInfo()->transport.pause1EventID;
            if (!_triggeredDepartureEvent && eventid)
            {
                _triggeredDepartureEvent = true;
                GetMap()->ScriptsStart(sEventScripts, eventid, this, this);
                EventInform(eventid);
                sScriptMgr->OnEventInform(this, eventid);
            }
        }
    }
}
// Unstoppable transport case
else
    timer += diff;

timer %= GetTransportPeriod();

// Ugly workaround. We want the timer to be 0 when the transport is stopped to properly calculate position, but client expects 65535 in dynamic field
uint32 realTimer = timer % (GetTransportPeriod() - 1);

if (_delayedAddModel)
{
    _delayedAddModel = false;
    if (m_model)
        GetMap()->InsertGameObjectModel(*m_model);
}

G3D::Vector3 origin { m_stationaryPosition.GetPositionX(), m_stationaryPosition.GetPositionY(), m_stationaryPosition.GetPositionZ() };

G3D::Vector3 pos;
{
    TransportAnimationEntry const* prev = nullptr;
    TransportAnimationEntry const* next = nullptr;
    for (auto itr = m_goValue.Transport.AnimationInfo->Path.begin(); itr != m_goValue.Transport.AnimationInfo->Path.end(); prev = (itr++)->second)
    {
        next = itr->second;
        if (realTimer <= next->TimeSeg)
            break;
    }
    if (prev == next)
        prev = nullptr;

    if (prev)
        pos = G3D::Vector3 { prev->X, prev->Y, prev->Z }.lerp({ next->X, next->Y, next->Z }, (float)(realTimer - prev->TimeSeg) / (next->TimeSeg - prev->TimeSeg));
    else if (next)
        pos = G3D::Vector3 { next->X, next->Y, next->Z };

    if (prev || next)
    {
        // GAMEOBJECT_PARENTROTATION contains path rotation, while GameObject's own rotation (sent as part of movement update) allows it to spin around its axis while still maintaining the same path
        G3D::Quat parentRotation { GetFloatValue(GAMEOBJECT_PARENTROTATION + 0), GetFloatValue(GAMEOBJECT_PARENTROTATION + 1), GetFloatValue(GAMEOBJECT_PARENTROTATION + 2), GetFloatValue(GAMEOBJECT_PARENTROTATION + 3) };
        G3D::Matrix4 parentRotationMatrix { parentRotation.toRotationMatrix() };
        G3D::Matrix4 translationMatrix = G3D::Matrix4::translation(pos);
        pos = origin + (parentRotationMatrix * translationMatrix).column(3).xyz(); // Last column of 4x4 transformation matrix stores translation transformation, we only need it
    }
    else
        pos = G3D::Vector3 { GetPositionX(), GetPositionY(), GetPositionZ() };
}

G3D::Quat rot;
float o;
{
    TransportRotationEntry const* prev = nullptr;
    TransportRotationEntry const* next = nullptr;
    for (auto itr = m_goValue.Transport.AnimationInfo->Rotations.begin(); itr != m_goValue.Transport.AnimationInfo->Rotations.end(); prev = (itr++)->second)
    {
        next = itr->second;
        if (realTimer <= next->TimeSeg)
            break;
    }
    if (prev == next)
        prev = nullptr;

    if (prev)
        rot = G3D::Quat { prev->X, prev->Y, prev->Z, prev->W }.slerp({ next->X, next->Y, next->Z, next->W }, (float)(realTimer - prev->TimeSeg) / (next->TimeSeg - prev->TimeSeg));
    else if (next)
        rot = G3D::Quat { next->X, next->Y, next->Z, next->W };

    if (prev || next)
    {
        float pitch, roll; // Currently unused, a few transport animations use them, but these GameObjects are not present in game
        rot.toRotationMatrix().toEulerAnglesZYX(o, pitch, roll);
        o = m_stationaryPosition.GetOrientation() + o;
    }
    else
        o = GetOrientation();
}

UpdatePosition(pos.x, pos.y, pos.z, o);

The first part of the code, up until the first position calculations, would be different after 3.3.5.

GetTransportPeriod() in the case of GAMEOBJECT_TYPE_TRANSPORT returns m_goValue.Transport.AnimationInfo->TotalTime.

GAMEOBJECT_TYPE_TRANSPORT share the same Transport class for me, only the Transport::Create is different for them, but the changes should be pretty straightforward. I'll only drop this here:

    if (goinfo->transport.pause)
        m_goValue.Transport.PathProgress = goinfo->transport.startOpen ? goinfo->transport.pause : m_goValue.Transport.AnimationInfo->TotalTime - 1;

Transport::DelayedUpdate is removed, grid management in Transport::UpdatePosition is removed, Transport::EnableMovement becomes a simple switch between GO_STATE_ACTIVE and GO_STATE_READY. Adapt Map::GetTransport to also allow non-MO transports. How to handle transport spawning - I'll leave to you to decide, but they have to be of Transport class or something derived from that, not just simply GameObject.

I have tested this on every non-MO transport in 3.3.5 and it works flawlessly except for some very edge cases when a transport would reverse its animation to return to a previous point, which shouldn't occur under regular gameplay conditions anyway.

[Warpten]

I have the math for cata, I just don't know if it will apply to live (and I'm too lazy to check)

[Ovahlord]

Show it and let lord shauren be the judge then.

[Faq]

@Warpten any time would be good to share :)