Trinitycore: Core/Pet: Pets do not follow owner onto transports

[MrSmite]

Rev: All (this is a long-standing bug)

Bug:

When a player enters a transport such as the boat from Stormwind to Auberdine, the pet falls
into the water

Details:

I have made significant progress and just wanted to make an official bug report.

For now, please visit my thread on the forums for progress updates (easier than using "Github flavored markdown").

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[ghost]

isnt there any way to assign the pet as npc passenger ?

[MrSmite]

@NorthStrider

Not using the current AddNPCPassenger, no. The pet gets removed from the world due to invalid coordinates. I've written some custom handlers but the trick is getting the pet added / removed at the right time since they don't cause the same events to fire that a player does when they enter / exit a transport.

I've made good progress, just fighting with two issues:

• Keeping the pet on the transport when it moves
• Having the pet exit the transport without vanishing

Fixing one breaks the other and vice versa.

[EdwardTuring]

@MrSmite

i have test the lastest core,the pet is still not following the owner to the transport...

@MrSmite ,did you have fixed this awful bugs?

i cann't visit your pages on the forums...(ip forbiden)...

could you tell me your solutions?

[Faq]

@EdwardTuring you need to use "log in" in forum then links will do the trick :)

[EdwardTuring]

@Faq ,my ip adress has been ban ...so i coud not sign in :)

[MrSmite]

@EdwardTuring No, I haven't looked into this issue in a while. After getting side-tracked with other things, I'm now in the middle of updating my OS to a version that supports the "latest" Visual Studio since XP is what I've been using.

[MrSmite]

Update:

For those following this ancient report, I've managed to code for the pet to enter / leave the transport. Just need to figure out how to add the pet when crossing loading screens.

[Faq]

You could share it.

[Kittnz]

I think on retail they just respawn the pet. I'm not sure, i need to test this on retail.

[MrSmite]

You could share it.

Done, see below...

I just fixed a bug that caused the core to crash when removing the pet as a passenger. It turns out if you remove the player first, the iterator becomes ??? and crashes when you try to remove the pet. Since there's no way to check if an iterator is initialized, removing the pet first works.

I think on retail they just respawn the pet. I'm not sure, i need to test this on retail.

Yes, I'm pretty sure that's also how trinity works, debug messages show the player / pet removed from the transport just before the screen appears. It's just that I missed where the respawn happens so the pet does not get added as a passenger and falls into the water.

[MrSmite]

Ok if anyone wants to mess around and comment / give advice, here are some DIFF patches. I will try to create a proper branch in my repo on github but for now...

NOTES:

• The source is an older 335a core (42f1a0babfaacf64df31a934288979f8edfec940) that I have already patched with other things I've been working on (that don't interfere with these diffs). This _may_ not apply directly to _HEAD_...
• Right now the PetAI portion (CharmInfo) is not used, I included it in case there is some future need to know if the pet is on a transport.
• One known (random) issue is that the pet occasionally will warp off the dock to be on the transport next to the player. Doesn't happen all the time but that's something people can muddle over when testing.

 src/server/game/Entities/Unit/Unit.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/server/game/Entities/Unit/Unit.h b/src/server/game/Entities/Unit/Unit.h
index ddf43f3..aa46f25 100644
--- a/src/server/game/Entities/Unit/Unit.h
+++ b/src/server/game/Entities/Unit/Unit.h
@@ -1185,6 +1185,8 @@ struct CharmInfo
         bool IsReturning();
         void SaveStayPosition();
         void GetStayPosition(float &x, float &y, float &z);
+        void SetIsOnTransport(bool val) { _isOnTransport = val; }
+        bool IsOnTransport() { return _isOnTransport; }

     private:

@@ -1202,6 +1204,7 @@ struct CharmInfo
         bool _isAtStay;
         bool _isFollowing;
         bool _isReturning;
+        bool _isOnTransport;
         float _stayX;
         float _stayY;
         float _stayZ;

 src/server/game/Handlers/MovementHandler.cpp | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/src/server/game/Handlers/MovementHandler.cpp b/src/server/game/Handlers/MovementHandler.cpp
index f537af4..f933e8e 100644
--- a/src/server/game/Handlers/MovementHandler.cpp
+++ b/src/server/game/Handlers/MovementHandler.cpp
@@ -31,6 +31,7 @@
 #include "InstanceSaveMgr.h"
 #include "ObjectMgr.h"
 #include "Vehicle.h"
+#include "Pet.h"

 #define MOVEMENT_PACKET_TIME_DELAY 0

@@ -253,6 +254,7 @@ void WorldSession::HandleMovementOpcodes(WorldPacket& recvData)
     ASSERT(mover != NULL);                      // there must always be a mover

     Player* plrMover = mover->ToPlayer();
+    Pet* petMover = plrMover->GetPet();

     // ignore, waiting processing in WorldSession::HandleMoveWorldportAckOpcode and WorldSession::HandleMoveTeleportAck
     if (plrMover && plrMover->IsBeingTeleported())
@@ -306,13 +308,27 @@ void WorldSession::HandleMovementOpcodes(WorldPacket& recvData)
             if (!plrMover->GetTransport())
             {
                 if (Transport* transport = plrMover->GetMap()->GetTransport(movementInfo.transport.guid))
+                {
                     transport->AddPassenger(plrMover);
+
+                    if (petMover)
+                        transport->AddPassenger(petMover);
+                }
             }
             else if (plrMover->GetTransport()->GetGUID() != movementInfo.transport.guid)
             {
+                if (petMover)
+                    plrMover->GetTransport()->RemovePassenger(petMover); // Remove pet first to avoid crash in RemovePassenger()
+
                 plrMover->GetTransport()->RemovePassenger(plrMover);
+
                 if (Transport* transport = plrMover->GetMap()->GetTransport(movementInfo.transport.guid))
+                {
                     transport->AddPassenger(plrMover);
+
+                    if (petMover)
+                        transport->AddPassenger(petMover);
+                }
                 else
                     movementInfo.transport.Reset();
             }
@@ -327,10 +343,18 @@ void WorldSession::HandleMovementOpcodes(WorldPacket& recvData)
     }
     else if (plrMover && plrMover->GetTransport())                // if we were on a transport, leave
     {
+        if (petMover)
+            plrMover->GetTransport()->RemovePassenger(petMover); // Remove pet first to avoid crash in RemovePassenger()
+
         plrMover->GetTransport()->RemovePassenger(plrMover);
+
         movementInfo.transport.Reset();
     }

+    // So the PetAI will function properly
+    if (petMover)
+        petMover->GetCharmInfo()->SetIsOnTransport(plrMover->HasUnitMovementFlag(MOVEMENTFLAG_ONTRANSPORT));
+
     // fall damage generation (ignore in flight case that can be triggered also at lags in moment teleportation to another map).
     if (opcode == MSG_MOVE_FALL_LAND && plrMover && !plrMover->IsInFlight())
         plrMover->HandleFall(movementInfo);

[Kittnz]

So all that is needed is when player is spawned again the pet should spawn again too and added on the transport. hmmm, nice work btw

[MrSmite]

So all that is needed is when player is spawned again the pet should spawn again too and added on the transport. hmmm, nice work btw

It's been a while since I've traced through the spawning procedure but I believe the pet is temporarily unsummoned and not actually despawned. When the pet is resummoned it needs to check if the owner is on a transport (or use the CharmInfo if it hasn't been invalidated).

Shouldn't be too difficult, I just got sidetracked and had to go out of town for a couple days.

[MrSmite]

So I've been thinking it might be better to add the pet within AddPassenger() if the WorldObject passed in is a player. That way there's only one code block to maintain.

I'll test it and try to push it to my repo instead of posting it in a reply.

[Kittnz]

A PR can be made to, so we an give some comments, i'm interested in your approach

[MrSmite]

A PR can be made to, so we an give some comments, i'm interested in your approach

I've been working on it here and there which is why I've held off. I just added code to _Pet::AddToWorld()_ to see if I can get it on the transport at map change. Haven't tested yet.

[ghost]

Not working your patch. Tested in durotar on transport, pet can't go on the transport, and if i spawn him on transport, he just fall. My suggestion, maybe if owner of pet is on transport, then set for pet UNIT_FLAG_IGNORE_PATHFINDING

[MrSmite]

Not working your patch.

Just checking: you did use a 335 core?

The patch above won't work with spawning or logging in because it only works when the player receives the movement opcode which isn't sent in case of spawning. I've got a version that attempts to put the pet on the transport in _AddToWorld()_ but I haven't published it yet.

[ghost]

Ye i try patch on 3.3.5

[MrSmite]

I'll check it out hopefully this week, been a bit busy with IRL stuff.

[ghost]

Anyway it's crashing, when i come home i will Send you crashlog

[Aokromes]

If this fix is valid, plz open a pr for it.

[MrSmite]

If this fix is valid, plz open a pr for it.

I doubt that this patch will work in its current state. Still needs to be worked on.

[silent-wow-roots]

Any News on this?

[MrSmite]

Any News on this?

Nothing recent. I still work on it from time to time but the pets don't receive the same packets players do so it's difficult to know when a pet is on a boat.

I tried various things such as adding the pet to the transport when the player receives the packet but that results in weird behavior because the pet isn't always entering the transport at the same time. It also stops the pet from following the player around the transport.

This is one of the things that's frustrated me since the beginning when I rewrote the original pet AI. I always feel like there's something missing in the pet packets but can't quite put a finger on it.

[Solzz]

Is it worth looking into SunwellCore? as it seems to work perfectly fine on that core, but I'm guessing it's a hack. It would be nice to get this working hack or not and then work on it from there.

[Shauren]

The main part that is missing is that while gameobjects are part of vmaps (doors block line of sight and everything) they are not part of mmaps (pathfinding) - so it cannot be currently solved without giant hacks

[Solzz]

@Shauren I've only seen it working on Sunwell and not looked into the code but with what you've said I'm sure it's a big hack in their code.

[ccrs]

"I've only seen it working on Sunwell"

allow me to doubt it, most likely they just set straight mid air paths to pretend its fixed

[Solzz]

I don't know @ccrs The pet followed me down stairs in a zeppelin and followed the pathing of the stairs and walls etc. I will double check it for my own sanity tonight but I'm sure it was working pretty well.

[MrSmite]

Shauren is correct. Anything I came up with that was remotely successful was buggy at best.

Example: You tell your pet to "stay" on the dock in Stormwind and then step on a boat. Using the player's event to AddPassenger() for the pet causes the pet to start floating through the air as the boat moves.
.
Example: The pet follows you onto the boat and AddPassenger() adds it. You then jump off the boat into the water, triggering the event you left the transport. The pet is subsequently removed from the transport and falls through the floor.
.
Example: The pet follows you onto the boat and AddPassenger() adds it. You run around the boat but the pet does not follow properly, sometimes falling through floors, sometimes rubberbanding.

I personally do not understand the MMaps system well enough to attempt a solution there which is why I stopped devoting so much time to it. Occasionally I get an idea and try it but since I've posted no new code, you get the idea.

[Shauren]

@MrSmite thats not even a matter of understanding mmaps - the pathfinding library we use (Detour) does not support dynamic obstacles at all in the current version

[Solzz]

Found out how Sunwell do it, on TrinityCore the pet stays on the ground when the player uses .gm fly on Sunwell this happens.

http://i.imgur.com/Olh9wq1.jpg

The pets just force to stay next to your character when flying.

[MrSmite]

thats not even a matter of understanding mmaps - the pathfinding library we use (Detour) does not support dynamic obstacles at all in the current version

You know as I think about this more I wonder, how does it work for the player then? When you enter a transport, an event is sent and the client / server know to update your position so you don't fall through the deck. Also, it plays the "wood" footstep sound as you move around. From the player's perspective, it's clearly an object. From the pet's perspective it doesn't exist.

I think the real issue here is that pets do not receive the ENTERING_TRANSPORT / LEAVING_TRANSPORT events, among other issues.

Found out how Sunwell do it, on TrinityCore the pet stays on the ground when the player uses .gm fly on Sunwell this happens.

Yes, I tried doing this. Problem is the pet can tend to teleport to the player if they are far enough apart when the player enters the transport. Also, fighting with the pet is difficult since it is stuck to the player.

Here's something that may be worth trying: Match the owner's Z position if it is on a transport and the pet is within X yards. Given the size of the transport, we can extend that check from the usual melee range to MELEE_RANGE + 15 (or some arbitrary number)

Example:

// NOTE: Pseudocode, I didn't search the sources for actual function names and other things simply don't exist yet ;)
// Also, I'm not quite sure where this would go

#DEFINE TRANSPORT_SIZE 15;   // Allow for expanding distance search from pet to owner

// Assuming this is placed in TargetedMovementGenerator for pets..., i_target is the owner
if (i_target->IsOnTransport() && me->IsWithinDistInMap(MELEE_RANGE + TRANSPORT_SIZE, i_target))
    pet->MatchOwnerPosZ();

[Shauren]

It works for players because its the client telling us that you are entering transport (while its the server that needs to move the pet)

[Solzz]

Yes, I tried doing this. Problem is the pet can tend to teleport to the player if they are far enough apart when the player enters the transport. Also, fighting with the pet is difficult since it is stuck to the player.

As far as I can see this system only happens when on transport when actually at ground level the pet acts normal. Is there any real world scenario without cheats where a pet would fly? mounts despawn them, so only cheats would make the pet come off the ground. As for jumping onto the transport, it actually doesn't teleport and seems pretty seamless. The only thing obviously missing is pathfinding throughout the transport, but from what @Shauren said it sounds like it's a long time away. Edit: the pet is removed on teleport when on transports.

[MrSmite]

It works for players because its the client telling us that you are entering transport (while its the server that needs to move the pet)

Yes, I understand that but ultimately it is the server that moves the player too, after the initial notification. I guess I don't see why the player can move freely without falling but a pet can't. Even after AddToTransport(), things don't work right.

[ccrs]

"I understand that but ultimately it is the server that moves the player too, after the initial notification"
incorrect, take a look to MovementHandler
"I don't see why the player can move freely without falling"
incorrect, a player cant move freely without falling

@solzz I told you 💃

[Solzz]

good call @ccrs

[MrSmite]

incorrect, take a look to MovementHandler

As far as I understood, the client receives input from the player to move. This packet is sent to the server which calculates collision and pathing and sends results back to the client. This is why I said the server moves the player. If it was completely up to the client, you'd be subject to movement hacks.

So what I don't understand is why the player is treated differently than a pet. If the server / client are capable of knowing when a player enters a boat, keeping the player on the boat and playing the proper footstep sounds, why can't it do this for a pet too?

Even if the client isn't sending the enter transport message for a pet, why does adding the pet to the transport the same way a player is added not have the same effect?

incorrect, a player cant move freely without falling

Well I've never fallen through a boat, that's what I mean by "move freely". It just seems wierd that once you add the pet to the boat it can't behave the same way a player does.

@Solzz I told you 💃

What, that I would ask questions?

[Shauren]

Oh but the thing is... the server is not protected against movement hacks! It is completely up to the client (which is why there are various anticheat patches out there)

[ccrs]

client moves, server receives the message with the related movementInfo, checks against a list of flags combinations to prevent errors, moves the player serverside updating if necessary things like transport
c++ void WorldSession::HandleMovementOpcodes(WorldPacket& recvData

its the server that does all the job, the client just says "hey! I moved!"

a pet, guardian is just a "creature" following, you need a movement generator for it to decide where to move. And thats the only issue that should be discussed here (https://github.com/TrinityCore/TrinityCore/issues/8772#issuecomment-300723872)

The "I told you" was because of this https://github.com/TrinityCore/TrinityCore/issues/8772#issuecomment-300818632

[Shauren]

But its the client telling you that you entered a transport and most importantly, which transport so no, server does not do all the job in the context of this issue (!)

[killerwife]

The way it works on sunwell is simple. They query VMAP every once in a while in pet to check if a transport is near (we update VMAPs of transports). This means that when it comes close, its boarded onto the transport, and miniported, and then uses transport pathfinding. The true fix would be to implement pathways on end of for example bridge in booty bay which would be togglable, and would be a wormhole onto part of transport temporarily.

The complete lack of tools for this kind of shenanigans, like lets say modeling tool for detour, is what keeps us mostly from implementing any solution.

Also another kind of fix for pets, is boarding them, when player boards. Its not a nice fix, but its a fix.

[ariel-]

I don't think it has wormhole mechanic and that retail ports or respawns creatures. I observed this while watching videos of Deathbringer Saurfang alliance outro event.

Ref: https://youtu.be/V4MmMhtgxvM?t=55s

[Shauren]

Permanent transport spawns can never leave their transport, thats a despawn and new creature - same happens during horde intro

[killerwife]

Ugh, you two misunderstood me. What I meant that the booty bay bridge would be kind of a motion into pocket of space, so you can walk onto it. (keep in mind, transport WMO is physically in corner in game engine, at least ships).

With the despawn respawn mechanic youve shown, that just means it works only for Players and Pets. So if you miniport pet when player boards, everything should work nicely. Doesn't fix the movement hack susceptibility though.

[MrSmite]

Also another kind of fix for pets, is boarding them, when player boards. Its not a nice fix, but its a fix.

Yes, I tried this and it would not pass the Trinity standard so I scrapped it.

The main problem with adding a pet to a transport at the same time as the player is if the pet is on "stay" or in combat, when the transport begins moving the pet starts to float through the air. I do like the idea of setting up a VMAP tile at the edge of the dock and adding the pet if the conditions are right.

I haven't looked at the Trinity code in months though so I'm sure most of what I have wouldn't even merge anymore. I just built a new rig (yes, I finally got off XP) so hopefully I'll be able to jump in again sometime soon.

[killerwife]

I do agree that this solution is slightly sketchy, but it would also require to implement the handling of everything the "true" solution would require, like boarding of pets onto transports, and player future movement prediction for smoother pet movegen. It would also make troubleshooting of transport pet movement possible, until such occasion arises so that Pets can board by themselves (since like was talked about above, this is a pet only case scenario). But hey, we are at least on the same track.

[MrSmite]

One of my first pieces of code was something like:

// Pseudocode follows, DO NOT ATTEMPT TO USE :)
// Pet::Update()

if (pet->GetOwner()->IsOnTransport())
{
    if (pet->IsWithinDistanceOf(pet->GetOwner(), whateverTheSizeOfTransportIs)
        if (!pet->IsOnTransport())
            pet->SetOnTransport(true)
} else (

    if (pet->IsOnTransport())
        pet->SetOnTransport(false)
}

But the problem was there is no predefined size in the DBC for the transport object so the pet would randomly fall through the transport. Also, it had a small side effect of crashing the server though I could never pinpoint why.

[killerwife]

Okay, but I was more of the notion of fixing it for MO 15 transports first, i.e. ships and zeppelings. I am quite sure that there is a WMO for the elevators too somewhere.

[MrSmite]

Okay, but I was more of the notion of fixing it for MO 15 transports first, i.e. ships

Yes, I was working in Stormwind Harbor on the ships. I wasn't able to find any information on length, width of the ships. Also, when monitoring the PLAYER_ENTER_TRANSPORT message (or whatever it is), the pet would usually fall off the end of the dock before being added to the transport (when guessing the size) and remain stuck in the bottom of the ship.

One of the things that frutstrated me the most was I couldn't get the ships to stay in the harbor while I was testing. There used to be a way to stop them from pathing but I couldn't remember.

[killerwife]

There is a WMO which is not extracted by default which completely describes the transport (and should be in corner of map somewhere). Thats how most russian pservers got it working. I once talked to someone who bragged about it.

[Shauren]

that WMO model itself is extracted and moves with transport (LoS on transport is handled) and most likely not to be found in some corner of map (otherwise wtb coords for map viewer)

[Warpten]

My guess is they got confused with the fact that transport position never updates, only path progress does. Afaik there's no such wmo

[killerwife]

Transport position doesnt update for 15 or 11, but when you are on 15 your position (player one) changes only relative to transport, whereas on 11 it changes globally.

[MrSmite]

I don't know if it was an early Trinity or a different core completely (mangos?) but years ago I remember being able to issue a console command that would stop the ships (and thereby the loading screens).

[Shauren]

Maybe it was possible in some earlier client version but 3.3.5 client will not stop moving transports if they don't have this set to 1 (data8 in gameobject_template)
https://github.com/TrinityCore/TrinityCore/blob/eb7d22d23eedb0d1b5374f80518cd7994d7c7b3a/src/server/game/Entities/GameObject/GameObjectData.h#L239
What would happen is that even if you did the same on the server (basically just set gameobject state) to transport without that is client would continue to move it by itself anyway

[killerwife]

I can second what @Shauren has said. I was fixing some stuff on tbc client, and the server spline was being calculated wrongly for UC Gromgol zeppelin. On server, zeppelin was somewhere above stormwind, and on client it was chugging along over undercity, not caring what the server does at all, as long createobject was sent.

[MrSmite]

Maybe it was possible in some earlier client version

Now that you mention it, I think it was the TBC client.