Avoid potentially unnecessary block acks

[pseudospace0]

Avoids block change acks for blocks which are potentially out of a players reach.

[Owen1212055]

Can you please further explain your change here? This part of the code is rather sensitive, and would appreciate why exactly you are making this change here.

Since, you are eliminating two places where the block ack occurs. But it doesn't really make sense why these acks should be cancelled, as if the client does introduce something like this (which may be due to a desync) it would make sense to do this ack to revert it clientside.

Additionally, this causes a bit of a desync as then the client is predicting this stage but an ack wasn't sent back, meaning next ack will also cause this prediction to be reverted. So.. not really sure what this is targeting.

Client Side Prediction:

this.startPrediction(this.minecraft.level, $$2 -> {
               this.destroyBlock($$0);
               return new ServerboundPlayerActionPacket(ServerboundPlayerActionPacket.Action.START_DESTROY_BLOCK, $$0, $$1, $$2);
            });

Vanilla server (always sends back ack):

case STOP_DESTROY_BLOCK:
            this.player.gameMode.handleBlockBreakAction($$1, $$2, $$0.getDirection(), this.player.level().getMaxBuildHeight(), $$0.getSequence());
            this.player.connection.ackBlockChangesUpTo($$0.getSequence());
            return;

[Owen1212055]

Additionally, you are missing Paper comments.

[pseudospace0]

Can you please further explain your change here? This part of the code is rather sensitive, and would appreciate why exactly you are making this change here.

Since, you are eliminating two places where the block ack occurs. But it doesn't really make sense why these acks should be cancelled, as if the client does introduce something like this (which may be due to a desync) it would make sense to do this ack to revert it clientside.

Additionally, this causes a bit of a desync as then the client is predicting this stage but an ack wasn't sent back, meaning next ack will also cause this prediction to be reverted. So.. not really sure what this is targeting.

Client Side Prediction:

this.startPrediction(this.minecraft.level, $$2 -> {
               this.destroyBlock($$0);
               return new ServerboundPlayerActionPacket(ServerboundPlayerActionPacket.Action.START_DESTROY_BLOCK, $$0, $$1, $$2);
            });

Vanilla server (always sends back ack):

case STOP_DESTROY_BLOCK:
            this.player.gameMode.handleBlockBreakAction($$1, $$2, $$0.getDirection(), this.player.level().getMaxBuildHeight(), $$0.getSequence());
            this.player.connection.ackBlockChangesUpTo($$0.getSequence());
            return;

My thinking was that it didn't really seem necessary to acknowledge the player breaking a block which is out of their reach. If this prediction would be reverted by the next (within breaking range) ack from the server, wouldn't this be saving on a packet being sent?

[Owen1212055]

Well, the issue is that it more leaves the client in a "still predicting" state even after the block break has finished.

[Owen1212055]

Closing due to reasons mentioned above, and in general this leaves the client in an invalid still predicting... state.