senseiwells / EssentialClient

EssentialClient is a client side mod originally forked from Carpet Client for 1.15.2 that implements new client side features
MIT License
77 stars 14 forks source link

PlaceBlock(placeAt) & block breaking support(1.16) #24

Closed aria1th closed 2 years ago

aria1th commented 2 years ago
while (world.getBlockAt(bx,by,bz).getBlockId() == 'lectern'){
player.updateBreakingBlock(bx,by,bz);
sleep(50);}

example code for block breaking should be like that, client should 'update' or 'attack' block every 50ms. Unless client 'cancels' block breaking... I'm not sure if client would need to call cancel function.

placing block functions barely work, but I don't know if I should do complex inventory swap inside same class.

senseiwells commented 2 years ago

For the placing I would just try to place the item that is currently in the players hand. If the player is not holding a BlockItem then throw an error? This allows the user to code the swapping slots instead

aria1th commented 2 years ago

hmm yeah that's way better, actually I was trying to implement piston rotating / anvil rotating things, it requires different handling by block types... but it's future problem

senseiwells commented 2 years ago

I don't think either interactionManager.interactBlock or interactionManager.updateBlockBreakingProgress are thread safe, they both access the player's inventory which is probably what is causing your concurrentModificationException

aria1th commented 2 years ago

that might explain it, updateBlockBreakingProgress seems okay, but interactBlock weirdly access player inventory.... it crashes when player picks up item at empty slot, not sure why this is happening tho. it can be tweaked a bit to directly send packet hmm...

senseiwells commented 2 years ago

Just push it back to the main thread

client.execute(() -> {
    // code
});
aria1th commented 2 years ago

yes it works, still some memory sum up but it does not exceed 4GB so it should be okay

aria1th commented 2 years ago

Still, it causes some invalid Map operations, I just tweaked to use packet instead but it will result some failure (because its packet) maybe function name should be changed to attemptInteractAt ?

senseiwells commented 2 years ago

What invalid Map operations?

aria1th commented 2 years ago

same iteration problem, so now I tweaked all into packet handling at 1.18 branch... but seems item swap code is also packet dependent so its order messes up without sleep().

senseiwells commented 2 years ago

Would it be better for us to talk on discord? Github is kinda... slow xD

senseiwells commented 2 years ago

The updateSlotPacket is pointless, Minecraft ticks this, and if you change it, it will automatically send a packet for you. No need to do this manually.

You also do this:

ArucasMinecraftExtension.getClient().execute(() -> {
    interactionManager.clickSlot(screenHandler.syncId, numberValue1.value.intValue(), 0, SlotActionType.SWAP, player);
    interactionManager.clickSlot(screenHandler.syncId, numberValue2.value.intValue(), 0, SlotActionType.SWAP, player);
    interactionManager.clickSlot(screenHandler.syncId, numberValue1.value.intValue(), 0, SlotActionType.SWAP, player);
}

Also you don't need to do all those checks for the direction, you can just do, which you do already, so I'm not sure why you check the string xD

var direction = Direction.byName(direction);
if (direction == null) {
    // Throw error
}

I also wouldn't send the PlayerInteractBlockC2SPacket, you should keep to the interactionManager.interactBlock()... This will cause desyncs, you could also make this return a boolean, whether it was successful, but you'll probably need to use an AtomicReference and a latch because it's threaded though hmm