BuiltBrokenModding / ICBM-Classic

Classic 1.6.4 version of ICBM remastered for newer MC versions
Other
44 stars 45 forks source link

[1.12] Client log spam when loading chunks or moving away after opening a GUI #234

Closed LemADEC closed 2 years ago

LemADEC commented 5 years ago

As of "1.12.2-3.3.0b65-UNSTABLE build 65", client logs are spammed with invalid PacketTile updates.

[00:43:14] [Netty Client IO #1/ERROR] [icbmclassic]: icbm.classic.lib.network.ex.PacketTileReadException: Null tile
Pos: WorldLocation [-234.0x,67.0y,259.0z,0d]
Tile: null
Block: null
[00:43:14] [Netty Client IO #1/ERROR] [icbmclassic]: icbm.classic.lib.network.ex.PacketTileReadException: Null tile
Pos: WorldLocation [-234.0x,67.0y,259.0z,0d]
Tile: null
Block: null
[00:43:14] [Netty Client IO #1/ERROR] [icbmclassic]: icbm.classic.lib.network.ex.PacketTileReadException: Null tile
Pos: WorldLocation [-234.0x,67.0y,259.0z,0d]
Tile: null
Block: null

This happens in 2 different use cases: 1- whenever server is loading chunks containing a Launcher Control Panel. 2- whenever a player opens a Launcher Control Panel GUI, then close it, then move far enough to unload client side chunk containing that block. This happens without changing dimension. This means there's probably a leak of player objects on server side.

On server side, the first use case happens because server is sending update packets when loading chunks, before client has loaded the chunks.

On server side, the second use case happens because of multiple reasons: 2a- the container prefab can't report that the GUI is closed because it never saved the tile entity reference (host property is never set).

2b- the tile machine prefab assumes the player container is null when a GUI is closed while the latest is always defined and defaults to player's inventory view. Consequently, the recovery mechanism also fails to detect that the player is no longer a valid user.

On client side, in both cases, packet handler fails to see that the chunk is actually not loaded. Specifically here: https://github.com/BuiltBrokenModding/ICBM-Classic/blob/1.12/src/main/java/icbm/classic/lib/network/packet/PacketTile.java#L130 isBlockLoaded(BlockPos) is always true on client side. isBlockLoaded(BlockPos, false) should be called instead.

DarkGuardsman commented 5 years ago

This is caused by a packet getting to the client when a chunk doesn't exist. In the current rewrite this error is ignored and not longer displays outside of debug mode.

LemADEC commented 5 years ago

As explained above, there's more than just non loaded chunk error.

github-actions[bot] commented 4 years ago

Stale issue message