Trinitycore: Player connection issue after FORCED disconnection

[Goatform]

Current behaviour: When players disconnect in "force mode" which is Internet DC or ALT+F4 combo, his character will stay in world (blizzlike) BUT when he log in back before timer (which is 60 seconds) expire he would disconnect and then re-connect once again.

Expected behaviour: Player should not be disconnected and re-connected (from world) but instead just continue where he was, at state which he had.

I will give you few examples:

1) Player is in arena and he DC, arena will continue and once player come back he will be disconnected and removed from arena, which actually should not happen if force timer don't expire and you should just continue playing there. That is actually very gamebreaking issue in general, not just for PvP.

2) E.g you are driving in chopper (mount) with another player, and if you DC (in force mode) you will stay at place and when you re-connect, you will dismount you and second player.
I remember having DC on retail in past and when I reconnected, my friend was still there on mount, with me.

Steps to reproduce the problem:

1. Force DC with player 1# while e.g. mounted together with player 2#
2. Once you reconnect (when you look at another point of view, from other players) you will be temporary removed from world and them "re-spawned" once again.

Branch(es): 3.3.5 (Confirmed)

TC rev. hash/commit: 3d5c1b8

TDB version: 335.62

Operating system: Windows 7

[ariel-]

Sunwell has this implemented, need to port this: https://github.com/Kittnz/Sunwell/blob/master/src/server/game/Handlers/CharacterHandler.cpp#L774

[Takenbacon]

@ariel- There will be no porting of that clusterfuck.

[Goatform]

Ops, and yeah, @Takenbacon is right. I tried to port it on my core and test it a bit "more" and I found so many ways to crash server. I think that it is not good solution at all.

[Shauren]

damn, that code looks so bad

[ariel-]

No one said anything about copying and pasting that code

[xinef1]

@Goatform
It is, if you know what you are doing and know how the trinitycore works really well. Otherwise your core will just crash because you ported too little or just copied and pasted without analyzing.

@Takenbacon
No point in commenting on that. The core has hundreds of features that can be ported to trinitycore with benefit, even if it's just a quest or event. Are Brewfest monthly mails working? Fishing Extravaganza? Fishing Derby(1)? No one even bothered to check the sql files which alone contain hundreds of quest fixes. Everyone is waiting for Shauren to do all the work and this will simply not work. If you have time to post such a pointless comments, maybe it would be better to write a patch?

@Shauren
The proper fix is much more extensive. About this bad code, it was initially a simple while loop to handle far teleport and then a loop to handle near teleport. It is not my code, maybe it can be simplified. It was changed to that monster probably because character was alternating between near and far teleport somehow and the server froze (it may even be not needed now). If you have thousands of players angry, because server freezes, quick measurements are required. Ill put SC log on github soon, so you will be able to pick and merge some changes if you want to. I'm attaching some diffs I've just took from the log regarding this problem (not sure if all required).

Btw. please note that this code was never meant to be released, it was written just for us and by us (me and pussywizard). If you have only two people working on a server with thousands of players online where we've been fixing everything starting from quests to main core systems you don't have time to refactor old code or write quick fixes super clean (apart from rewritting whole parts of core, like lfg queue, instance system, etc.). There are almost no hacks in the core (do not mistake with codestyle) and if I had to add one, it tried to affix it with appropriate comment (like "hack" or "so ugly i'm crying"). All problems we've been fixing were fixed with as generic solution as possible, even if it required to change sensitive parts of the core.
logout.zip

Cheers.

(1) Fishing events were written back in 2011, you can use Talk with zone wide messages much easier now. Never had time to refactor or clean old working scripts.

[Goatform]

@xinef1, I don't know who you are but I didn't meant anything bad. I just agreed that code needs improvements and better solutions. Idea itself is good and it help us to make some additional progress on it. I usually don't care if something is "hack" / "ugly" or similar if it solves an issue which is gamebreaking at some point just as this one.

Anyway, I will try to reimplement this system once again on my core and test it and give additional feedback then if I really miss something (which is still possibility) and even if I figure out to fix some bugs that I find, I would instantly open PR on TC and start supporting this as much as I can. Thank you for sharing that patch once again :+1:

[Goatform]

Found what caused that crash and merged few other functions which aren't included in that diff but there is still 1 small issue with this system. Here is an example:

• Player A cast mount that allow other players (which are in party) to jump on it as well (e.g. mounts like chopper)
• Player B force logout by ALT+F4 and when he come back he will be on ground.
• Player A will be able to drive him even if both of them can't see player B on that mount and when player A dismount, he will literally teleport player B on location of player A just as he was on mount.

How it should to work:

• When player login once again after forced DC, he should be on mount still and e.g. when player A is moving him while player B is DCed, screen of player B should just support that but instead it does not.

Again I may miss something while I was transfering but I am unable to test that core since I do not have a DB etc but if someone can confirm that issue on their core as well it would help a lot.

[xinef1]

@Goatform
Well, SC may possess the same error. I would guess that vehicle data is not available for the player loading to existing character and invalid data is sent along his next movement opcodes (no transport data, wrong movementflags etc).

Try this patch
```c++
Index: Entities/Player/Player.cpp
void Player::SendInitialPacketsAfterAddToMap()
SetMovement(MOVE_ROOT);

 // manual send package (have code in HandleEffect(this, AURA_EFFECT_HANDLE_SEND_FOR_CLIENT, true); that must not be re-applied.

- if (HasAuraType(SPELL_AURA_MOD_ROOT))
+ if (HasAuraType(SPELL_AURA_MOD_ROOT) || GetVehicle())
{
WorldPacket data2(SMSG_FORCE_MOVE_ROOT, 10);
data2.append(GetPackGUID());
Index: Maps/Map.cpp
void Map::SendInitSelf(Player* player)
if (Transport* transport = player->GetTransport())
transport->BuildCreateUpdateBlockForPlayer(&data, player);

• if (Unit* vehicleUnit = player->GetVehicleBase())
• vehicleUnit->BuildCreateUpdateBlockForPlayer(&data, player);
  +
  // build data for self presence in world at own client (one time for map)
  player->BuildCreateUpdateBlockForPlayer(&data, player);
  ```
  In Player::SendInitialPacketsAfterAddToMap(), the additional check for GetVehicle should be met only in the case u presented (logging to character on vehicle), this will block ability to move on client properly. The change in Map.cpp should fix the problem where player lacks data about the vehicle hes on right from the start.

Edit: patch was hand written, hope it's not a problem.