[FIXED][Bug] Client Crash on Revive

[TILT]

Appreciate the effort and time you have put into this. Hopefully this will be ironed out soon. In the mean time, I'm going to try testing a few things locally and see if I can come to any conclusions.

[Jabantiz]

I looked into this today but was unable to make any progress, I was using a DoV client and never crashed on the first revive (was stated earlier in this thread that Dov always crashed). My client finally spit out something in alertlog.txt after several crashes.

Code: Select all

alert="E:\live\eq2\game\client\src\GameScene.cpp(17610): CRC missmatch in EqCreateGhostCmd state buf."
alert="E:\live\eq2\game\gamecommon\client\CompressedXOR.cpp(243): Bogus compressed data.  This is normally caused by mismatched client and server."
alert="E:\live\eq2\game\client\src\GameScene.cpp(12158): Unhandled client cmd 0, last: 550"
alert="E:\live\eq2\game\client\src\GameScene.cpp(6995): Unhandled network message ()"
alert="E:\live\eq2\game\client\src\GameScene.cpp(12158): Unhandled client cmd 0, last: 591"

I focused my effort on the first alert but after going through WS_SetPOVGhostCmd, WS_SetControlGhost, and WS_SpawnStruct_Header, spawn_id was set the same way in all 3 packets and they were always the same value as each other when they were sent as a group (log in and revives).

I did also go through Client::HandlePlayerRevive(int32 point_id) and compared the packets to ones from a 1161 logs, where I got Xinux killed several times while collecting chardok, and they appear to be sent the same. Went though Client::SendCharInfo(), where both control and pov packets are sent, and the only thing I noticed in there that seemed wrong was that both the control and pov packets were sent before the player was added to the zone, moving the packet below had no effect however and everything worked the same. I am currently out of ideas for this bug.

[John Adams]

There is one more bit of info about Revive bugs, which is not necessarily limited to Revive (but possible the LD code). The whole "disappearing client" bug, where one minute Scatman and I can see each other, he dies, and can see me, but I can no longer see his toon. Or, post-zoning, or camping/coming back in from LD.

We've been trying to debug this for years. Hopefully, we can add enough LogWrites() to eventually follow the path of a client fully from login to logout. Someday.

[Jabantiz]

I forgot to mention that the spawn_id in those packets is set with player->GetIDWithPlayerSpawn(player), on the first login it gives a value of 1 (both in an empty zone and a zone with spawns) after revive it is 0 and stays 0 for the second revive. I have an idea that I will try later but I am not expecting it to work.

[Jabantiz]

Ok my idea didn't work as expected (changed GetIdWithPlayerSpawn to spawn_id and id, allowed me in game but crashed on first revive with the same error). Looking at the actual function

Code: Select all

int32 GetIDWithPlayerSpawn(Spawn* spawn){
		if(player_spawn_reverse_id_map.count(spawn) > 0)
			return player_spawn_reverse_id_map[spawn];
		return 0;
	}

I would assume when it returns 0 it is bad (returns 1 on first login) so I am guessing that player_spawn_reverse_id_map (map<Spawn*, int32>) is getting cleared on revive and not set properly after the zone, that is my best guess at this point at least.

[Jabantiz]

The map used in GetIDWithPlayerSpawn is only ever set for the player when new Player() is called, and that is only called for a new client. However in Client::HandleRevive we call ZoneServer::RemoveSpawn() wich will call ZoneServer::SendRemoveSpawn() wich removes the player out of the map, never to be set again. I changed SendRemoveSpawn to the following

Code: Select all

bool ZoneServer::SendRemoveSpawn(Client* client, Spawn* spawn, PacketStruct* packet, bool delete_spawn){
	if(!client || !spawn)
		return false;

	int16 index = client->GetPlayer()->GetIndexForSpawn(spawn);
	if(packet && index > 0 && client->GetPlayer()->WasSpawnRemoved(spawn) == false)
	{
		if (!spawn->IsPlayer() || (Player*)spawn != client->GetPlayer()) {
		client->GetPlayer()->RemoveSpawn(spawn); // sets it as removed
		client->GetPlayer()->GetPlayerSpawnMap()->erase(client->GetPlayer()->GetIndexForSpawn(spawn));
		client->GetPlayer()->GetPlayerSpawnIndexMap()->erase(spawn);
		client->GetPlayer()->player_spawn_id_map.erase(client->GetPlayer()->GetIDWithPlayerSpawn(spawn));
		client->GetPlayer()->player_spawn_reverse_id_map.erase(spawn);

		packet->setDataByName("spawn_index", index);
		//if(delete_spawn)
		packet->setDataByName("delete", 1);	
		client->QueuePacket(packet->serialize());
		//client->GetPlayer()->SetSpawnDeleteTime(spawn->GetID(), Timer::GetCurrentTime2());
		}
		
		return true;
	}
	return false;
}

Now when the player dies the screen doesn't go black right away, and the player can revive without crashing, however there are now duplicates because the destroyghost packet never gets sent for the player.


My attempts to move the packet to other spots (like at the end of HandleRevive() ) usually causes a crash on the first revive.

[John Adams]

wich removes the player out of the map, never to be set again.

Here's the problem I have with this. When I stepped through the code of SendRemoveSpawn(), my first death, index == 1.

This is where my break point was set:

Code: Select all

int16 index = client->GetPlayer()->GetIndexForSpawn(spawn);

Continuing, the Revive button pops and I can click it and I am once again sent to SendRemoveSpawn() and my breakpoint (above) is hit again - this time, index == 0. With index == 0, the QueuePacket() call is skipped - everything inside

Code: Select all

if(packet && index > 0 && client->GetPlayer()->WasSpawnRemoved(spawn) == false)

is skipped and we return false.


Now, on my 2nd death, my break point is hit

Code: Select all

int16 index = client->GetPlayer()->GetIndexForSpawn(spawn);

and index == 2. If the spawn was never added back to the map, this could not be true. Correct?

And again, after hitting the Revive in the client, I get sent finally to SendRemoveSpawn() one last time, and again, index == 0 and the QueuePacket() is skipped.

The crash is happening outside SendRemoveSpawn(), which I can never get to because the stupid connection timeout happens and disconnects me. So I'll have to trace the functions manually and do what you did, putting logging in everywhere til I see the exact line of code that sends the offending packet to the client. I do not think it's in SendRemoveSpawn() tho -- and maybe that's what you're trying to tell me

[Jabantiz]

I think the problem is with these 2 lines of code

Code: Select all

client->GetPlayer()->player_spawn_id_map.erase(client->GetPlayer()->GetIDWithPlayerSpawn(spawn));
client->GetPlayer()->player_spawn_reverse_id_map.erase(spawn);

that only needs to be called once to remove the player from those maps, and I don't see where the player will ever be added back in. Those maps are used for spawn_id in OP_EqSetPOVGhostCmd, OP_EqSetControlGhostCmd, and OP_EqCreateGhostCmd for the player.

[John Adams]

By Jove, I believe he's fixed it. We have one more issue to resolve before I put this code on Test Center, so stand by.

[Jabantiz]

With the help of Zcoretri last night and John tonight I believe we have fixed this bug, and the issues the fix caused. It still needs a lot of testing but the client should no longer crash when you revive, all code has been submitted to the Dev SVN