diasurgical / devilutionX

Diablo build for modern operating systems
Other
8.1k stars 796 forks source link

Multiplayer - Buggy Portals and Pre-Generated Portals in Town #3085

Closed kphoenix137 closed 3 years ago

kphoenix137 commented 3 years ago

Important information Windows 10, Commit 98334454, Compiled myself

Describe In Multiplayer, in both ZeroTier and TCP connection methods in both games with another player and games where I connected to myself locally, upon joining a game and going to the portal area, 4 portals already exist.

In the game where I joined another player's game, 4 portals were open. When attempting to use the portals that didn't have player names (since there weren't 3 or 4 people in the game), the portal leads back to the same spot in town. Upon attempting to use the portals with our names on them, the portals lead back to the same spot in town.

In the game where I connected to myself over TCP, the character that I created the game with casted a portal in dungeon level 13. The next character joined the game to see 4 portals. Upon attempting to use ANY of these portals, including the one that actually had a destination (dlvl 13), the game crashed with the message LoadLvlGFX during the loading screen.

To Reproduce Steps to reproduce the behavior:

  1. Join Multiplayer game
  2. Walk to portal area
  3. Use any Portal
  4. See error

Expected behavior Portals should not be generated until a player casts a Town Portal spell in a dungeon level

Screenshots Video: https://www.twitch.tv/videos/1173302929

Additional context This problem does not always happen, but happens frequently enough to reproduce without much effort.

AJenbo commented 3 years ago

image so-really

kphoenix137 commented 3 years ago

image so-really

The latest one I experienced was creating the town portal with the player remaining waiting in the dungeon, and then joining in with a second character. Try that

AJenbo commented 3 years ago

:+1: image

ephphatha commented 3 years ago

This appears to be due to players getting garbage data in msg.cpp:DeltaImportData when joining a game. Guessing PkwareDecompress isn't getting called right?

Portals are one of the easiest and most visible ways to trigger the bug, but it also affects items, objects and monsters.

image In this game I hosted through the bottom client then:

  1. went to level 1
  2. cast town portal (this causes corrupt portal information to appear when players join)
  3. dropped a health potion near the stairs (new players receive corrupt item information and cannot see dropped items)
  4. broke a barrel (new players receive corrupt object state and cannot see changes since the level was generated)

Doing any of these actions while a player is in the game causes the action message to be sent across and the other player sees (for example) the object update or the item drop, but this doesn't persist when leaving and rejoining the game.

Edit: even easier way to trigger it, host a game, interact with the wounded townsman, join from a second client, observe the quest state isn't synced (wounded townsman appears on the second player, the host sees a slain townsman)

julealgon commented 3 years ago

With the consistent repro, I imagine someone would be able to bisect this?

ephphatha commented 3 years ago

PKWare.lib:explode() is failing with CMP_INVALID_DICTSIZE when trying to decode the CMD_DLEVEL_JUNK message. I still haven't worked out what's causing that error but I did notice that MSVC (contrary to the spec for operator new[]?) doesn't initialise the ptr buffers used in encrypt.cpp:PkwareCompress and PkwareDecompress, which explode() and implode() have comments noting they expect a clean 0-initialised buffer. This isn't the cause of the error because even with an explicit initialiser the call to explode still fails, something is getting corrupted when the message is sent or received as far as I can tell.

qndel commented 3 years ago

Bisected to 614fba6af62975180e1cc71cf89404ea0e31c0c7

image

qndel commented 3 years ago

Fixed by https://github.com/diasurgical/devilutionX/pull/3094