Random Chitchat 2012-2016

[xoft]

I think you don't need the IsDestroyed() check; it is unreliable (another thread can destroy the cPlayer object anywhere in the middle of this entire function - if the player is in another world). Your code is more or less breaking the very purpose of callbacks - not to have objects stored outside of the safety of the callbacks.

To be safe now, I'd use the scenario of remembering only the player name and using the DoWithPlayer() callback to directly set the destination, rather than remembering the player object:

if (IsTame())
{
    class cCallback :
        public cPlayerListCallback
    {
        virtual bool Item(cPlayer * a_Player) override
        {
            OwnerPos = a_Player->GetPosition();
            return false;
        }
    public:
        Vector3f OwnerPos;
    } Callback;
    if (m_World->DoWithPlayer(m_OwnerName, Callback))
    {
        // The player is present in the world, follow them:
        double Distance = (Callback.OwnerCoords - GetPosition()).Length();
        if (Distance < 3)
        {
            m_bMovingToDestination = false;
        }
        else if ((Distance > 30) && (!IsSitting()))
        {
            TeleportToCoords(Callback.OwnerCoords.x, Callback.OwnerCoords.y, Callback.OwnerCoords.z);
        }
        else
        {
            m_Destination = Callback.OwnerCoords;
        }
    }
}  // if (IsTame())

Also, consider moving this entire piece of code to a separate function, TickFollowPlayer() for example; that way the code for Tick() will be more readable and it will be easier to split and extract it once we start implementing AI behaviors.