TheCBProject / CBMultipart

An API for dynamically handling different functional parts in the one block space.
Other
22 stars 27 forks source link

Add a check to prevent sendDescUpdate from being called on the client #91

Closed ArcanoxDragon closed 3 years ago

ArcanoxDragon commented 3 years ago

This avoids crashes in rare cases where a MultiPartLoadHandler.TileNBTContainer gets added to a client-side world, such as when setting a multipart down after picking it up using the "Carry On" mod.

Fixes #87

covers1624 commented 3 years ago

To clarify a bit more on this issue, for documentation purposes.

TileNBTContainer is allowed to and expected to exist on the client. However, it is not expected to have any NBT data loaded. We use It on the server to hold the NBT of the tile to be loaded, which will get replaced by our TileEntity.create mixin server-side during chunk reading. We then hijack the 'getUpdateTag' method of TileMultiPart to write the desc packet data to the initial chunk NBT tag. On the client, the TileNBTContainer then handles this desc packet, replacing the tile in the client-side world with the correct TileMultiPart.

The issue occurs from CarryOn trying to be 'smart', and load the server-side tile data into the client-side TileNBTContainer, this causes the wrong logic to run on the client.

Minecraft never saves or loads NBT data for tile entities on the client-side, it explicitly uses 'update tags' to do its client-side loading of tile data. This is technically a bug in CarryOn, but this is an easy enough fix, we are probably one of the niche cases where this logic error would actually matter anyway.