Fix bug: remote player on different level writes to dPlayer[][] on the host

[jwkeating]

Bug repro: Player A creates multiplayer game and goes to dungeon level 1. Player B joins and goes to dungeon level 1. Player B moves a few tiles and then exits the game. Player B rejoins the game. Player A can now see player B's cursor selection in dungeon level one despite the fact player B has rejoined in town.

Cause: On Player A's computer, StartStand() is called for Player B when Player B is not valid ("" for player name, 0 health, etc), and StartStand() writes to dPlayer[][] for Player A's dungeon level 1 despite the fact Player B is not on dungeon level 1. Bug was introduced in recent commit 5a08031 "[Bugfix] Update players directions on joining"

See debugger screenshot:

[jwkeating]

@yuripourre

[StephenCWills]

The call to StartStand() activates the player's idle animation for the direction that player is facing. If you change _pdir without calling it, then you won't actually see their sprite turn in that direction. Instead of removing the function call, we can just check player.isOnActiveLevel() to determine if player direction needs to be synchronized.

That said, I am still a little concerned about what you said regarding the player struct not yet having been initialized. Looks like we need to adjust the logic around here.

DevilutionX/Source/multi.cpp

Lines 708 to 718 in 4f6a172

	if (!IsNetPlayerValid(player)) {
	const _cmd_id cmd = *(const _cmd_id *)(pkt + 1);
	if (gbBufferMsgs == 0 && IsNoneOf(cmd, CMD_SEND_PLRINFO, CMD_ACK_PLRINFO)) {
	// Distrust all messages until
	// player info is received
	continue;
	}
	}
	const Point syncPosition = { pkt->px, pkt->py };
	player.position.last = syncPosition;
	if (&player != MyPlayer) {

Probably, the easiest thing to do is just skip packet header processing for CMD_SEND_PLRINFO and CMD_ACK_PLRINFO messages, since receipt of either of those messages implies that the player data hasn't been initialized yet.

EDIT: On further inspection, we should probably move the _pdir synchronization inside the conditional block that's handling position synchronization. It's checking a lot more than just player.isOnActiveLevel(), and it's probably all relevant to both walking and idling.

DevilutionX/Source/multi.cpp

Lines 738 to 739 in 4f6a172

	if (!cond && player.plractive && !player.hasNoLife()) {
	if (player.isOnActiveLevel() && !player._pLvlChanging) {

[jwkeating]

player.isOnActiveLevel() returns true in this case, so checking it won't help. See above debugger screenshot which shows player.currentDungeonLevel == 1 and currLevel == 1 also.

[jwkeating]

I updated the pull request. Is this what you had in mind?

[yuripourre]

@yuripourre

Understood, I haven't tested this case. Sorry for the trouble.

Edit: The PR looks correct now. I think that's what @StephenCWills mentioned.

Thanks for working on it @jwkeating

[AJenbo]

nice work