GeyserMC / Geyser

A bridge/proxy allowing you to connect to Minecraft: Java Edition servers with Minecraft: Bedrock Edition.
https://geysermc.org
MIT License
4.71k stars 674 forks source link

Setting blockstate on java server doesn't update on geyser until chunk reloads #4625

Open Karltroid opened 5 months ago

Karltroid commented 5 months ago

Describe the bug

In the plugin I am making for my server I am setting the facing direction of a shulker box when placed. This works on java edition as expected but the block state remains default (UP) on bedrock edition until the bedrock client reloads the chunk (leaving and coming back or relogging).

To Reproduce

  1. Make a plugin that sets the facing direction of a shulkerbox (or any block with a facing property).
  2. View this change in real time on a java and geyser client, it will update on java but not on bedrock.

Expected behaviour

Updating blockstate on geyser client when its updated on java.

Screenshots / Videos

image

Server Version and Plugins

No response

Geyser Dump

DUMP

Geyser Version

2.3.0

Minecraft: Bedrock Edition Device/Version

v1.20.81

Additional Context

No response

Karltroid commented 5 months ago

image I don't know if this stems from the same issue or bedrock just handles blockstates differently, but it seems to ignore me setting stair nbt to straight instead of corner as well.

Difference with the stairs in particular is that reloading chunks doesn't fix the issue

Karltroid commented 5 months ago

image Here is proof showing the shulker facing block state fixes after reloading the chunk

Camotoy commented 5 months ago

Please update to the latest version (and install ViaVersion as well) and re-test this and provide a Geyser dump. Thanks!

Karltroid commented 5 months ago

https://dump.geysermc.org/CAVfs9mo8A92a7vJASKQdDRKsu7C9FSi

Paper: 1.20.4 # 496 Geyser: 2.3.0 ViaVersion: 4.10.0

Same issue persists after updating everything

Karltroid commented 5 months ago

For context, this is the code on my spigot plugin that is setting the block state of the shulker

private void convertToFatChest(Block block, Material fatChestType, BlockFace facing)
{
    if (!(block.getState() instanceof Container)) return;

    // save pre conversion container
    ItemStack[] containerContents = ((Container) block.getState()).getInventory().getContents();

    // convert container to desired chest type
    BlockData newFatChest = fatChestType.createBlockData();
    ((Directional) newFatChest).setFacing(facing);
    block.setBlockData(newFatChest);

    // load pre conversion container to new container
    ((Container) block.getState()).getInventory().setContents(containerContents);
}
valaphee commented 4 months ago

When do you change the block state? As Vanilla always sends the block state on placing and interacting, and when you modify it there it might send two block state updates.

Karltroid commented 4 months ago

When do you change the block state? As Vanilla always sends the block state on placing and interacting, and when you modify it there it might send two block state updates.

@EventHandler
public void onChestPlace(BlockPlaceEvent event) {

  // get if block representing a chest was placed
  Block placedBlock = event.getBlockPlaced();
  if (!placedBlock.getType().equals(GUI_CHEST)) return;

  // can't place chest in the middle of two chests
  List<Block> adjacentChests = getAdjacentChests(placedBlock);
  if (adjacentChests.size() > 1) {
      event.setCancelled(true);
      return;
  }

  // can't place adjacent chest next to double chest, if normal chest then create double chest!
  for (Block chest : adjacentChests) {
      Material adjacentType = chest.getType();
      if (isDoubleChest(adjacentType)) {
          event.setCancelled(true);
          return;
      } else if (isSingleChest(adjacentType)) {
          convertToDoubleChest(placedBlock, chest);
          return;
      }
  }

  // not a double chest, make a single chest!
  if (adjacentChests.isEmpty()) convertToFatChest(placedBlock, SINGLE_CHEST);
}

I am setting the placed block's block data right on BlockPlaceEvent

Karltroid commented 4 months ago
// not a double chest, make a single chest!
if (adjacentChests.isEmpty())
{
    convertToFatChest(placedBlock, SINGLE_CHEST);
    placedBlock.getState().update(true, true);
}

So by adding the forced state update it halfway fixes it. The fake chest is still facing upwards (default shulker state) but with the forced update it will correct its orientation when opened. So whatever is happening when the player opens the shulker needs to somehow be done on state update.

Karltroid commented 4 months ago

https://github.com/GeyserMC/Geyser/assets/22510609/45dc046b-765b-4545-9cfe-0753c506b68d

Heres a video of what I mean in my previous comment

Karltroid commented 4 months ago

New discoveries:

Karltroid commented 4 months ago

Got it working! Since using //set worked I decided to set the blockdata using Fawe/WorldEdit's API in my plugin and worked like a charm!

private void convertToFatChest(Block block, Material fatChestType) {

    // save pre conversion container
    ItemStack[] containerContents = new ItemStack[0];
    if ((block.getState() instanceof Container))
        containerContents = ((Container) block.getState()).getInventory().getContents();

    // convert container to desired chest type
    BlockFace facing = ((Directional) block.getBlockData()).getFacing();
    BlockData fatChest = fatChestType.createBlockData();
    ((Directional) fatChest).setFacing(facing);

    // convert block data to World Edit API and set block via said API
    try (EditSession editSession = WorldEdit.getInstance().newEditSession(FaweAPI.getWorld(block.getWorld().getName()))) {
        com.sk89q.worldedit.world.block.BlockState fatChestFAWE = BukkitAdapter.adapt(fatChest);
        editSession.setBlock(block.getX(), block.getY(), block.getZ(), fatChestFAWE);
    } catch (Exception e) {
        ModernBeta.log("An error occurred creating fat chest: " + e.getMessage());
    }

    // load pre conversion container to new container
    if (containerContents.length > 0) ((Container) block.getState()).getInventory().setContents(containerContents);
}

So this could be closed, but I think there still is an issue here, this is just a bandaid fix for me right now.

Camotoy commented 4 months ago

Working with refactoring our block code, I think I know what's going on. I don't know the best way to fix it globally, though, but just for shulkers is likely doable.

Shulker box direction is part of the block entity in Bedrock. However, we only update the block entity if a Java block entity packet is sent. Because the Bedrock block state doesn't have a direction, we just send a block update packet with no changes. We already have a similar mitigation in place for double chests.