From c5fc0dc84b2e9f875b7dc03ed75cf6744abc4902 Mon Sep 17 00:00:00 2001 From: Lulu13022002 <41980282+Lulu13022002@users.noreply.github.com> Date: Sun, 8 Jun 2025 19:55:19 +0200 Subject: [PATCH] support block state mutation of already placed block entity in BlockStateListPopulator As seen in https://github.com/PaperMC/Paper/issues/12645, the BlockStateListPopulator always destroy block entity data when the block state change. However this is something that should be supported so plugin can retrieve block entity data of captured blocks. Additionally only create snapshots at the end of the capture so we don't need to refresh block entity data for decorator like the beehive, this is possible since multiple capture at the same position is not supported and will overwrite the previous data anyway. --- .../bukkit/event/block/SpongeAbsorbEvent.java | 6 +- .../0016-Moonrise-optimisation-patches.patch | 12 +-- ...nate-Current-redstone-implementation.patch | 4 +- .../minecraft/world/level/Level.java.patch | 9 +-- .../world/level/block/SpongeBlock.java.patch | 26 +++---- .../world/level/portal/PortalShape.java.patch | 2 +- .../craftbukkit/CraftRegionAccessor.java | 1 - .../block/CraftBlockEntityState.java | 4 - .../bukkit/craftbukkit/entity/CraftGhast.java | 3 +- .../craftbukkit/entity/CraftPhantom.java | 3 +- .../util/BlockStateListPopulator.java | 77 +++++++++++-------- .../craftbukkit/util/CapturedBlock.java | 6 ++ 12 files changed, 75 insertions(+), 78 deletions(-) create mode 100644 paper-server/src/main/java/org/bukkit/craftbukkit/util/CapturedBlock.java diff --git a/paper-api/src/main/java/org/bukkit/event/block/SpongeAbsorbEvent.java b/paper-api/src/main/java/org/bukkit/event/block/SpongeAbsorbEvent.java index ae56902a84..07495ed6b8 100644 --- a/paper-api/src/main/java/org/bukkit/event/block/SpongeAbsorbEvent.java +++ b/paper-api/src/main/java/org/bukkit/event/block/SpongeAbsorbEvent.java @@ -32,12 +32,12 @@ public class SpongeAbsorbEvent extends BlockEvent implements Cancellable { } /** - * Get a list of all blocks to be removed by the sponge. + * Get a list of all blocks to be cleared by the sponge. *
* This list is mutable and contains the blocks in their removed state, i.e. - * having a type of {@link Material#AIR}. + * having a type of {@link Material#AIR} or not waterlogged. * - * @return list of the to be removed blocks. + * @return list of the cleared blocks. */ @NotNull public List getBlocks() { diff --git a/paper-server/patches/features/0016-Moonrise-optimisation-patches.patch b/paper-server/patches/features/0016-Moonrise-optimisation-patches.patch index 690e398fdf..85916b4951 100644 --- a/paper-server/patches/features/0016-Moonrise-optimisation-patches.patch +++ b/paper-server/patches/features/0016-Moonrise-optimisation-patches.patch @@ -26353,7 +26353,7 @@ index 302841522cf990c38b1493b716048c0f2db40726..7932a6676db7b652d63be5ae4dcf9bcf } } diff --git a/net/minecraft/server/level/ServerChunkCache.java b/net/minecraft/server/level/ServerChunkCache.java -index 74c1c6f04a99817e67642cb330534c518830392f..8e9988130d9b912e5b858cd7792bdcefdeb66ada 100644 +index 92a39927d7bd9af3ab67c2595d8ed435a2da6a69..7c9a2eed4441f816723562e0012f918db265912e 100644 --- a/net/minecraft/server/level/ServerChunkCache.java +++ b/net/minecraft/server/level/ServerChunkCache.java @@ -55,7 +55,7 @@ import net.minecraft.world.level.storage.DimensionDataStorage; @@ -28633,7 +28633,7 @@ index 8cc5c0716392ba06501542ff5cbe71ee43979e5d..09fd99c9cbd23b5f3c899bfb00c9b896 + // Paper end - block counting } diff --git a/net/minecraft/world/entity/Entity.java b/net/minecraft/world/entity/Entity.java -index b1bebf925d4c1f8aa94917cffe878f092b89ad72..cd052ebeb963f1c6e717dfdc0397ec187d15b180 100644 +index a97de8eddd764e628d0f3866c5d1e99e6dae5f6e..503ac3a16c8490d7a620d43b12c4f0c80565615a 100644 --- a/net/minecraft/world/entity/Entity.java +++ b/net/minecraft/world/entity/Entity.java @@ -147,7 +147,7 @@ import net.minecraft.world.waypoints.WaypointTransmitter; @@ -29761,7 +29761,7 @@ index 300f3ed58109219d97846082941b860585f66fed..9175a7e4e6076626cb46144c5858c2f2 // Paper start - Affects Spawning API diff --git a/net/minecraft/world/level/Level.java b/net/minecraft/world/level/Level.java -index fa56dd1bdca53f7667dc6b4b2c8634483bae5262..b799d9cc54c45920a7c48d5302454c9cec3ab7c2 100644 +index 1eb8cb187d33510a4e99d229e721a2e7db4012ad..f286dd9996590e5d448ca809c34b6f640203e274 100644 --- a/net/minecraft/world/level/Level.java +++ b/net/minecraft/world/level/Level.java @@ -81,6 +81,7 @@ import net.minecraft.world.level.storage.LevelData; @@ -30497,7 +30497,7 @@ index fa56dd1bdca53f7667dc6b4b2c8634483bae5262..b799d9cc54c45920a7c48d5302454c9c } // Paper end - Option to prevent armor stands from doing entity lookups -@@ -994,7 +1642,7 @@ public abstract class Level implements LevelAccessor, UUIDLookup, AutoCl +@@ -987,7 +1635,7 @@ public abstract class Level implements LevelAccessor, UUIDLookup, AutoCl if (this.isOutsideBuildHeight(pos)) { return null; } else { @@ -30506,7 +30506,7 @@ index fa56dd1bdca53f7667dc6b4b2c8634483bae5262..b799d9cc54c45920a7c48d5302454c9c ? null : this.getChunkAt(pos).getBlockEntity(pos, LevelChunk.EntityCreationType.IMMEDIATE); } -@@ -1087,22 +1735,16 @@ public abstract class Level implements LevelAccessor, UUIDLookup, AutoCl +@@ -1080,22 +1728,16 @@ public abstract class Level implements LevelAccessor, UUIDLookup, AutoCl public List getEntities(@Nullable Entity entity, AABB boundingBox, Predicate predicate) { Profiler.get().incrementCounter("getEntities"); List list = Lists.newArrayList(); @@ -30537,7 +30537,7 @@ index fa56dd1bdca53f7667dc6b4b2c8634483bae5262..b799d9cc54c45920a7c48d5302454c9c } @Override -@@ -1116,33 +1758,94 @@ public abstract class Level implements LevelAccessor, UUIDLookup, AutoCl +@@ -1109,33 +1751,94 @@ public abstract class Level implements LevelAccessor, UUIDLookup, AutoCl this.getEntities(entityTypeTest, bounds, predicate, output, Integer.MAX_VALUE); } diff --git a/paper-server/patches/features/0019-Add-Alternate-Current-redstone-implementation.patch b/paper-server/patches/features/0019-Add-Alternate-Current-redstone-implementation.patch index 5404a4f4ca..fe1796186f 100644 --- a/paper-server/patches/features/0019-Add-Alternate-Current-redstone-implementation.patch +++ b/paper-server/patches/features/0019-Add-Alternate-Current-redstone-implementation.patch @@ -2352,10 +2352,10 @@ index f9c96bbdc54e68b9216b7f8662bfae03012d2866..34b7769663e235b93c6388ab0c92c00f @Override public void onCreated(Entity entity) { diff --git a/net/minecraft/world/level/Level.java b/net/minecraft/world/level/Level.java -index b799d9cc54c45920a7c48d5302454c9cec3ab7c2..e4b9a564aad3d9b673808caa18265b06592ceab8 100644 +index f286dd9996590e5d448ca809c34b6f640203e274..c41df4b1fff1f65532256e835dc30fadbb4f8c8b 100644 --- a/net/minecraft/world/level/Level.java +++ b/net/minecraft/world/level/Level.java -@@ -2100,6 +2100,17 @@ public abstract class Level implements LevelAccessor, UUIDLookup, AutoCl +@@ -2093,6 +2093,17 @@ public abstract class Level implements LevelAccessor, UUIDLookup, AutoCl return 0; } diff --git a/paper-server/patches/sources/net/minecraft/world/level/Level.java.patch b/paper-server/patches/sources/net/minecraft/world/level/Level.java.patch index c84ba5d6be..348a42a9a9 100644 --- a/paper-server/patches/sources/net/minecraft/world/level/Level.java.patch +++ b/paper-server/patches/sources/net/minecraft/world/level/Level.java.patch @@ -547,23 +547,16 @@ public boolean shouldTickDeath(Entity entity) { return true; -@@ -608,6 +_,19 @@ +@@ -608,6 +_,12 @@ @Nullable @Override public BlockEntity getBlockEntity(BlockPos pos) { -+ // CraftBukkit start -+ return this.getBlockEntity(pos, true); -+ } -+ -+ @Nullable -+ public BlockEntity getBlockEntity(BlockPos pos, boolean validate) { + // Paper start - Perf: Optimize capturedTileEntities lookup + net.minecraft.world.level.block.entity.BlockEntity blockEntity; + if (!this.capturedTileEntities.isEmpty() && (blockEntity = this.capturedTileEntities.get(pos)) != null) { + return blockEntity; + } + // Paper end - Perf: Optimize capturedTileEntities lookup -+ // CraftBukkit end if (this.isOutsideBuildHeight(pos)) { return null; } else { diff --git a/paper-server/patches/sources/net/minecraft/world/level/block/SpongeBlock.java.patch b/paper-server/patches/sources/net/minecraft/world/level/block/SpongeBlock.java.patch index 015dae2402..da59d59965 100644 --- a/paper-server/patches/sources/net/minecraft/world/level/block/SpongeBlock.java.patch +++ b/paper-server/patches/sources/net/minecraft/world/level/block/SpongeBlock.java.patch @@ -33,7 +33,7 @@ } else { if (!blockState.is(Blocks.KELP) && !blockState.is(Blocks.KELP_PLANT) -@@ -81,16 +_,55 @@ +@@ -81,16 +_,49 @@ return BlockPos.TraversalNodeStatus.SKIP; } @@ -43,7 +43,6 @@ + // CraftBukkit start + // BlockEntity blockEntity = blockState.hasBlockEntity() ? level.getBlockEntity(blockPos) : null; + // dropResources(blockState, level, blockPos, blockEntity); -+ // level.setBlock(blockPos, Blocks.AIR.defaultBlockState(), 3); + blockList.setBlock(blockPos, Blocks.AIR.defaultBlockState(), 3); + // CraftBukkit end } @@ -67,22 +66,17 @@ + + for (org.bukkit.craftbukkit.block.CraftBlockState snapshot : snapshots) { + BlockPos blockPos = snapshot.getPosition(); -+ BlockState state = level.getBlockState(blockPos); -+ FluidState fluid = level.getFluidState(blockPos); ++ BlockState blockState = level.getBlockState(blockPos); ++ FluidState fluidState = level.getFluidState(blockPos); + -+ if (fluid.is(FluidTags.WATER)) { -+ if (state.getBlock() instanceof BucketPickup bucketPickup && !bucketPickup.pickupBlock(null, level, blockPos, state).isEmpty()) { -+ // NOP -+ } else if (state.getBlock() instanceof LiquidBlock) { -+ // NOP -+ } else if (state.is(Blocks.KELP) || state.is(Blocks.KELP_PLANT) || state.is(Blocks.SEAGRASS) || state.is(Blocks.TALL_SEAGRASS)) { -+ BlockEntity blockEntity = state.hasBlockEntity() ? level.getBlockEntity(blockPos) : null; ++ if (!fluidState.is(FluidTags.WATER)) { ++ } else if (blockState.getBlock() instanceof BucketPickup bucketPickup && !bucketPickup.pickupBlock(null, org.bukkit.craftbukkit.util.DummyGeneratorAccess.INSTANCE, blockPos, blockState).isEmpty()) { ++ } else if (blockState.getBlock() instanceof LiquidBlock) { ++ } else if (blockState.is(Blocks.KELP) || blockState.is(Blocks.KELP_PLANT) || blockState.is(Blocks.SEAGRASS) || blockState.is(Blocks.TALL_SEAGRASS)) { ++ BlockEntity blockEntity = blockState.hasBlockEntity() ? level.getBlockEntity(blockPos) : null; + -+ // Paper start - Fix SpongeAbsortEvent handling -+ if (snapshot.getHandle().isAir()) { -+ dropResources(state, level, blockPos, blockEntity); -+ } -+ // Paper end - Fix SpongeAbsortEvent handling ++ if (snapshot.getHandle().isAir()) { ++ dropResources(blockState, level, blockPos, blockEntity); + } + } + snapshot.place(snapshot.getFlags()); diff --git a/paper-server/patches/sources/net/minecraft/world/level/portal/PortalShape.java.patch b/paper-server/patches/sources/net/minecraft/world/level/portal/PortalShape.java.patch index 89e1525889..85e23ecea4 100644 --- a/paper-server/patches/sources/net/minecraft/world/level/portal/PortalShape.java.patch +++ b/paper-server/patches/sources/net/minecraft/world/level/portal/PortalShape.java.patch @@ -140,7 +140,7 @@ BlockPos.betweenClosed(this.bottomLeft, this.bottomLeft.relative(Direction.UP, this.height - 1).relative(this.rightDir, this.width - 1)) + .forEach(pos -> this.blocks.setBlock(pos, blockState, 18)); + org.bukkit.event.world.PortalCreateEvent event = new org.bukkit.event.world.PortalCreateEvent((java.util.List) (java.util.List) this.blocks.getSnapshotBlocks(), bworld, (entity == null) ? null : entity.getBukkitEntity(), org.bukkit.event.world.PortalCreateEvent.CreateReason.FIRE); -+ level.getMinecraftWorld().getServer().server.getPluginManager().callEvent(event); ++ level.getMinecraftWorld().getServer().server.getPluginManager().callEvent(event); // todo the list is not really mutable here unlike other call and the portal frame is included + + if (event.isCancelled()) { + return false; diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegionAccessor.java b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegionAccessor.java index 1775eb659e..0a10f49ee4 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegionAccessor.java +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegionAccessor.java @@ -198,7 +198,6 @@ public abstract class CraftRegionAccessor implements RegionAccessor { BlockPos pos = CraftLocation.toBlockPosition(location); BlockStateListPopulator populator = new BlockStateListPopulator(this.getHandle()); boolean result = this.generateTree(populator, this.getHandle().getMinecraftWorld().getChunkSource().getGenerator(), pos, new RandomSourceWrapper(random), treeType); - populator.refreshTiles(); populator.placeSomeBlocks(predicate == null ? ($ -> true) : predicate); return result; } diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/block/CraftBlockEntityState.java b/paper-server/src/main/java/org/bukkit/craftbukkit/block/CraftBlockEntityState.java index b50c982d04..8067e94f06 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/block/CraftBlockEntityState.java +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/block/CraftBlockEntityState.java @@ -74,10 +74,6 @@ public abstract class CraftBlockEntityState extends Craft this.loadData(state.getSnapshotNBT()); } - public void refreshSnapshot() { - this.load(this.blockEntity); - } - private RegistryAccess getRegistryAccess() { LevelAccessor worldHandle = this.getWorldHandle(); return (worldHandle != null) ? worldHandle.registryAccess() : CraftRegistry.getMinecraftRegistry(); diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftGhast.java b/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftGhast.java index 13f79e667e..9f699fdbd2 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftGhast.java +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftGhast.java @@ -1,10 +1,9 @@ package org.bukkit.craftbukkit.entity; import org.bukkit.craftbukkit.CraftServer; -import org.bukkit.entity.Flying; import org.bukkit.entity.Ghast; -public class CraftGhast extends CraftMob implements Ghast, CraftEnemy, Flying { +public class CraftGhast extends CraftMob implements Ghast, CraftEnemy { public CraftGhast(CraftServer server, net.minecraft.world.entity.monster.Ghast entity) { super(server, entity); diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPhantom.java b/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPhantom.java index 99898ce0b3..8ef7b968b4 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPhantom.java +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPhantom.java @@ -4,11 +4,10 @@ import net.minecraft.Optionull; import org.bukkit.Location; import org.bukkit.craftbukkit.CraftServer; import org.bukkit.craftbukkit.util.CraftLocation; -import org.bukkit.entity.Flying; import org.bukkit.entity.Phantom; import java.util.UUID; -public class CraftPhantom extends CraftMob implements Phantom, CraftEnemy, Flying { +public class CraftPhantom extends CraftMob implements Phantom, CraftEnemy { public CraftPhantom(CraftServer server, net.minecraft.world.entity.monster.Phantom entity) { super(server, entity); diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/util/BlockStateListPopulator.java b/paper-server/src/main/java/org/bukkit/craftbukkit/util/BlockStateListPopulator.java index 6ece55f683..b7c3673814 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/util/BlockStateListPopulator.java +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/util/BlockStateListPopulator.java @@ -19,34 +19,30 @@ import net.minecraft.world.level.material.FluidState; import net.minecraft.world.level.storage.LevelData; import org.bukkit.block.BlockState; import org.bukkit.craftbukkit.block.CraftBlock; -import org.bukkit.craftbukkit.block.CraftBlockEntityState; import org.bukkit.craftbukkit.block.CraftBlockState; public class BlockStateListPopulator extends DummyGeneratorAccess { + private final LevelAccessor world; - private final Map dataMap = new HashMap<>(); + private final Map dataMap = new LinkedHashMap<>(); private final Map entityMap = new HashMap<>(); - private final LinkedHashMap blocks; + + private List blocks; public BlockStateListPopulator(LevelAccessor world) { - this(world, new LinkedHashMap<>()); - } - - private BlockStateListPopulator(LevelAccessor world, LinkedHashMap blocks) { this.world = world; - this.blocks = blocks; } @Override public net.minecraft.world.level.block.state.BlockState getBlockState(BlockPos pos) { - net.minecraft.world.level.block.state.BlockState state = this.dataMap.get(pos); - return (state != null) ? state : this.world.getBlockState(pos); + CapturedBlock block = this.dataMap.get(pos); + return (block != null) ? block.state() : this.world.getBlockState(pos); } @Override public FluidState getFluidState(BlockPos pos) { - net.minecraft.world.level.block.state.BlockState state = this.dataMap.get(pos); - return (state != null) ? state.getFluidState() : this.world.getFluidState(pos); + CapturedBlock block = this.dataMap.get(pos); + return (block != null) ? block.state().getFluidState() : this.world.getFluidState(pos); } @Override @@ -63,21 +59,23 @@ public class BlockStateListPopulator extends DummyGeneratorAccess { public boolean setBlock(BlockPos pos, net.minecraft.world.level.block.state.BlockState state, int flags, int recursionLeft) { pos = pos.immutable(); // remove first to keep insertion order - this.blocks.remove(pos); + this.dataMap.remove(pos); - this.dataMap.put(pos, state); - if (state.hasBlockEntity()) { - this.entityMap.put(pos, ((EntityBlock) state.getBlock()).newBlockEntity(pos, state)); + this.dataMap.put(pos, new CapturedBlock(state, flags)); + if (state.getBlock() instanceof EntityBlock entityBlock) { + // based on LevelChunk#setBlockState + BlockEntity currentBlockEntity = this.getBlockEntity(pos); + final BlockEntity newBlockEntity; + if (currentBlockEntity != null && currentBlockEntity.isValidBlockState(state)) { + newBlockEntity = currentBlockEntity; // previous block entity is still valid for this block state + currentBlockEntity.setBlockState(state); + } else { + newBlockEntity = entityBlock.newBlockEntity(pos, state); // create a new one when the block change + } + this.entityMap.put(pos, newBlockEntity); } else { this.entityMap.put(pos, null); } - - // use 'this' to ensure that the block state is the correct TileState - CraftBlockState snapshot = (CraftBlockState) CraftBlock.at(this, pos).getState(); - snapshot.setFlags(flags); - // set world handle to ensure that updated calls are done to the world and not to this populator - snapshot.setWorldHandle(this.world); - this.blocks.put(pos, snapshot); return true; } @@ -86,11 +84,19 @@ public class BlockStateListPopulator extends DummyGeneratorAccess { return this.world.getMinecraftWorld(); } - public void refreshTiles() { - for (CraftBlockState snapshot : this.blocks.values()) { - if (snapshot instanceof CraftBlockEntityState) { - ((CraftBlockEntityState) snapshot).refreshSnapshot(); - } + @Override + public ServerLevel getLevel() { + return this.getMinecraftWorld(); + } + + private void iterateSnapshots(Consumer callback) { + for (Map.Entry entry : this.dataMap.entrySet()) { + // use 'this' to ensure that the block state is the correct TileState + CraftBlockState snapshot = (CraftBlockState) CraftBlock.at(this, entry.getKey()).getState(); + snapshot.setFlags(entry.getValue().flags()); + // set world handle to ensure that updated calls are done to the world and not to this populator + snapshot.setWorldHandle(this.world); + callback.accept(snapshot); } } @@ -107,16 +113,21 @@ public class BlockStateListPopulator extends DummyGeneratorAccess { } public void placeSomeBlocks(Consumer beforeRun, Predicate filter) { - for (CraftBlockState state : this.blocks.values()) { - if (filter.test(state)) { - beforeRun.accept(state); - state.place(state.getFlags()); + for (CraftBlockState snapshot : this.getSnapshotBlocks()) { + if (filter.test(snapshot)) { + beforeRun.accept(snapshot); + snapshot.place(snapshot.getFlags()); } } } public List getSnapshotBlocks() { - return new ArrayList<>(this.blocks.values()); + if (this.blocks == null) { + List blocks = new ArrayList<>(); + this.iterateSnapshots(blocks::add); + this.blocks = blocks; + } + return blocks; } // For tree generation diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/util/CapturedBlock.java b/paper-server/src/main/java/org/bukkit/craftbukkit/util/CapturedBlock.java new file mode 100644 index 0000000000..c5644b43e4 --- /dev/null +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/util/CapturedBlock.java @@ -0,0 +1,6 @@ +package org.bukkit.craftbukkit.util; + +import net.minecraft.world.level.block.state.BlockState; + +public record CapturedBlock(BlockState state, int flags) { +}