Fix general issues with breaking things#281
Conversation
| } else if (world.isRemote) { // Remove null TEs on the client | ||
| this.getWorld().setBlockToAir(this.getPos()); | ||
| } | ||
|
|
There was a problem hiding this comment.
This code is simple, but it makes a big impact. I wonder how you test it?
- How to ensure data synch between the server and the client?
- How you test it in low tps that you mentioned?
It appears that they can return to normal MTEs if the server syncs its data properly.How is this confirmed?
I think the core problem is to solve why MTE is synced to empty, but BlockMachine not.
There was a problem hiding this comment.
- I have noticed several times that if this accidentally hits, the server seems to be able to recover it; besides, I would think that TE updates on the server go to the client, which could recover the block itself. I've also noticed an occasional flicker sometimes in low TPS while placing blocks, but it always properly recovers.
- I put a Thread.sleep of 1 in MetaTileEntity's update, which quite easily makes a nice configurable lag.
- Again, it also looks like placing is made a little weird in low TPS environments, and I can see the MTE flicker for a tick, and then return, as if the client code deleted the block, but the server returned it.
There was a problem hiding this comment.
As you said, I simulated a low TPS environment (4). But didnt recovering, with a completely invisible block blocking me.
.
The good news is that I seem to have figured out how to reproduce invisible blocks inspired by you and have some ideas.
|
A quick look at the MetaTileEntityHolder code shows that I can't see when a null metaTileEntity is assigned. |
|
btw, the cheapest way is to have the |
This wouldn't do much overall to solve the issue, and besides, null MTEs almost certainly cause other problems than just its rendering on the client side. It's likely better to just remove these side effects immediately. |
Yefancy
added
the
status: high priority
| return new CPacketPluginSynced(null, buf); | ||
| } | ||
| )); | ||
| CPacketPluginSynced.registerPacket(7); |
There was a problem hiding this comment.
Maybe move these packet numbers to GregtechDataCodes?
There was a problem hiding this comment.
blueberry said he will rework a general refactoring of NetworkHandler. another pr
|
|
||
| @Override | ||
| public boolean doesSideBlockRendering(@Nonnull IBlockState state, @Nonnull IBlockAccess world, @Nonnull BlockPos pos, @Nonnull EnumFacing face) { | ||
| return state.isOpaqueCube() && getMetaTileEntity(world, pos) != null; |
There was a problem hiding this comment.
Should this have an additional check for if the face to be rendered is obscured by another block?
There was a problem hiding this comment.
this is actually a method called by another block to check side rendering.
ALongStringOfNumbers
left a comment
ALongStringOfNumbers
left a comment
There was a problem hiding this comment.
Since both of my concerns were addressed, I think it will be fine to merge this and see if any problems are reported, since it is hard to test
4 participants
This PR fixes two minor issues I've found with breaking blocks. One of these is that the server does not correctly deal with a broken block when a sound is played, which has been fixed by adding parameters to onBreakingUse to allow the server to position a sound to where the tool is applied and then play it to everyone. The other is to fix an issue with null MTEs when they are broken at a low TPS, as they would cause persistent visual errors on the client side when broken, which has simply been fixed by turning them into air on the client on update cycles. It appears that they can return to normal MTEs if the server syncs its data properly. However, they seem to still potentially appear for a very short time, although for much shorter durations than previously.