Skip to content

Commit

Permalink
Fixed block entity update race condition
Browse files Browse the repository at this point in the history
  • Loading branch information
@RaphiMC
RaphiMC committed Dec 12, 2023
1 parent 1829a7a commit f06f3b8
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 42 deletions.
68 changes: 39 additions & 29 deletions src/main/java/net/raphimc/viabedrock/protocol/packets/WorldPackets.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.viaversion.viaversion.api.protocol.packet.PacketWrapper;
import com.viaversion.viaversion.api.protocol.remapper.PacketHandlers;
import com.viaversion.viaversion.api.type.Type;
import com.viaversion.viaversion.libs.fastutil.ints.IntObjectPair;
import com.viaversion.viaversion.libs.opennbt.tag.builtin.*;
import com.viaversion.viaversion.protocols.protocol1_20_3to1_20_2.packet.ClientboundPackets1_20_3;
import com.viaversion.viaversion.util.MathUtil;
Expand All @@ -41,6 +42,7 @@
import net.raphimc.viabedrock.api.chunk.section.BedrockChunkSection;
import net.raphimc.viabedrock.api.chunk.section.BedrockChunkSectionImpl;
import net.raphimc.viabedrock.api.model.entity.ClientPlayerEntity;
import net.raphimc.viabedrock.api.util.PacketFactory;
import net.raphimc.viabedrock.protocol.BedrockProtocol;
import net.raphimc.viabedrock.protocol.ClientboundBedrockPackets;
import net.raphimc.viabedrock.protocol.data.enums.bedrock.MovePlayerModes;
Expand Down Expand Up @@ -341,13 +343,19 @@ protected void register() {
return;
}

final int remappedBlockState = chunkTracker.handleBlockChange(position, layer, blockState);
if (remappedBlockState == -1) {
final IntObjectPair<BlockEntity> remappedBlock = chunkTracker.handleBlockChange(position, layer, blockState);
if (remappedBlock == null) {
wrapper.cancel();
return;
}

wrapper.write(Type.VAR_INT, remappedBlockState); // block state
wrapper.write(Type.VAR_INT, remappedBlock.keyInt()); // block state

if (remappedBlock.value() != null) {
wrapper.send(BedrockProtocol.class);
wrapper.cancel();
PacketFactory.sendBlockEntityData(wrapper.user(), position, remappedBlock.value());
}
});
}
});
Expand All @@ -368,47 +376,46 @@ protected void register() {
return;
}

final int remappedBlockState = chunkTracker.handleBlockChange(position, layer, blockState);
if (remappedBlockState == -1) {
final IntObjectPair<BlockEntity> remappedBlock = chunkTracker.handleBlockChange(position, layer, blockState);
if (remappedBlock == null) {
wrapper.cancel();
return;
}

wrapper.write(Type.VAR_INT, remappedBlockState); // block state
wrapper.write(Type.VAR_INT, remappedBlock.keyInt()); // block state

if (remappedBlock.value() != null) {
wrapper.send(BedrockProtocol.class);
wrapper.cancel();
PacketFactory.sendBlockEntityData(wrapper.user(), position, remappedBlock.value());
}
});
}
});
protocol.registerClientbound(ClientboundBedrockPackets.UPDATE_SUB_CHUNK_BLOCKS, null, wrapper -> {
wrapper.cancel(); // Need multiple packets because offsets can go over chunk boundaries
final ChunkTracker chunkTracker = wrapper.user().get(ChunkTracker.class);
wrapper.read(BedrockTypes.BLOCK_POSITION); // position | Seems to be unused by the Mojang client
final BlockChangeEntry[] layer0Blocks = wrapper.read(BedrockTypes.BLOCK_CHANGE_ENTRY_ARRAY); // standard blocks
final BlockChangeEntry[] layer1Blocks = wrapper.read(BedrockTypes.BLOCK_CHANGE_ENTRY_ARRAY); // extra blocks
final BlockChangeEntry[][] blockUpdatesArray = new BlockChangeEntry[2][];
blockUpdatesArray[0] = wrapper.read(BedrockTypes.BLOCK_CHANGE_ENTRY_ARRAY); // standard blocks
blockUpdatesArray[1] = wrapper.read(BedrockTypes.BLOCK_CHANGE_ENTRY_ARRAY); // extra blocks

final Map<Position, List<BlockChangeRecord>> blockChanges = new HashMap<>();
for (BlockChangeEntry entry : layer0Blocks) {
final int remappedBlockState = chunkTracker.handleBlockChange(entry.position(), 0, entry.blockState());
if (remappedBlockState == -1) {
continue;
}
final Map<Position, BlockEntity> blockEntities = new HashMap<>();
for (int layer = 0; layer < blockUpdatesArray.length; layer++) {
for (BlockChangeEntry entry : blockUpdatesArray[layer]) {
final IntObjectPair<BlockEntity> remappedBlock = chunkTracker.handleBlockChange(entry.position(), layer, entry.blockState());
if (remappedBlock == null) {
continue;
}
if (remappedBlock.value() != null) {
blockEntities.put(entry.position(), remappedBlock.value());
}

final Position chunkPosition = new Position(entry.position().x() >> 4, entry.position().y() >> 4, entry.position().z() >> 4);
final Position relative = new Position(entry.position().x() & 0xF, entry.position().y() & 0xF, entry.position().z() & 0xF);
blockChanges.computeIfAbsent(chunkPosition, k -> new ArrayList<>()).add(new BlockChangeRecord1_16_2(relative.x(), relative.y(), relative.z(), remappedBlockState));
}
for (BlockChangeEntry entry : layer1Blocks) {
final int remappedBlockState = chunkTracker.handleBlockChange(entry.position(), 1, entry.blockState());
if (remappedBlockState == -1) {
continue;
final Position chunkPosition = new Position(entry.position().x() >> 4, entry.position().y() >> 4, entry.position().z() >> 4);
final Position relative = new Position(entry.position().x() & 0xF, entry.position().y() & 0xF, entry.position().z() & 0xF);
blockChanges.computeIfAbsent(chunkPosition, k -> new ArrayList<>()).add(new BlockChangeRecord1_16_2(relative.x(), relative.y(), relative.z(), remappedBlock.keyInt()));
}

final Position chunkPosition = new Position(entry.position().x() >> 4, entry.position().y() >> 4, entry.position().z() >> 4);
final Position relative = new Position(entry.position().x() & 0xF, entry.position().y() & 0xF, entry.position().z() & 0xF);
blockChanges.computeIfAbsent(chunkPosition, k -> new ArrayList<>()).add(new BlockChangeRecord1_16_2(relative.x(), relative.y(), relative.z(), remappedBlockState));
}

if (blockChanges.isEmpty()) {
return;
}

for (Map.Entry<Position, List<BlockChangeRecord>> entry : blockChanges.entrySet()) {
Expand All @@ -423,6 +430,9 @@ protected void register() {
multiBlockChange.write(Type.VAR_LONG_BLOCK_CHANGE_RECORD_ARRAY, changes.toArray(new BlockChangeRecord[0])); // block change records
multiBlockChange.send(BedrockProtocol.class);
}
for (Map.Entry<Position, BlockEntity> entry : blockEntities.entrySet()) {
PacketFactory.sendBlockEntityData(wrapper.user(), entry.getKey(), entry.getValue());
}
});
protocol.registerClientbound(ClientboundBedrockPackets.BLOCK_ENTITY_DATA, ClientboundPackets1_20_3.BLOCK_ENTITY_DATA, new PacketHandlers() {
@Override
Expand Down
19 changes: 6 additions & 13 deletions src/main/java/net/raphimc/viabedrock/protocol/storage/ChunkTracker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@
import com.viaversion.viaversion.api.protocol.packet.PacketWrapper;
import com.viaversion.viaversion.api.type.Type;
import com.viaversion.viaversion.api.type.types.chunk.ChunkType1_20_2;
import com.viaversion.viaversion.libs.fastutil.ints.Int2IntMap;
import com.viaversion.viaversion.libs.fastutil.ints.Int2IntOpenHashMap;
import com.viaversion.viaversion.libs.fastutil.ints.IntIntImmutablePair;
import com.viaversion.viaversion.libs.fastutil.ints.IntIntPair;
import com.viaversion.viaversion.libs.fastutil.ints.*;
import com.viaversion.viaversion.libs.opennbt.tag.builtin.CompoundTag;
import com.viaversion.viaversion.libs.opennbt.tag.builtin.ListTag;
import com.viaversion.viaversion.libs.opennbt.tag.builtin.NumberTag;
Expand Down Expand Up @@ -355,10 +352,10 @@ public boolean mergeSubChunk(final int chunkX, final int subChunkY, final int ch
return true;
}

public int handleBlockChange(final Position blockPosition, final int layer, final int blockState) throws Exception {
public IntObjectPair<BlockEntity> handleBlockChange(final Position blockPosition, final int layer, final int blockState) throws Exception {
final BedrockChunkSection section = this.getChunkSection(blockPosition);
if (section == null) {
return -1;
return null;
}

final BlockStateRewriter blockStateRewriter = this.getUser().get(BlockStateRewriter.class);
Expand All @@ -369,7 +366,7 @@ public int handleBlockChange(final Position blockPosition, final int layer, fina

if (section.hasPendingBlockUpdates()) {
section.addPendingBlockUpdate(sectionX, sectionY, sectionZ, layer, blockState);
return -1;
return null;
}

while (section.palettesCount(PaletteType.BLOCKS) <= layer) {
Expand Down Expand Up @@ -407,18 +404,14 @@ public int handleBlockChange(final Position blockPosition, final int layer, fina
}

if (javaBlockEntity != null && javaBlockEntity.tag() != null) {
final PacketWrapper blockEntityData = PacketWrapper.create(ClientboundPackets1_20_3.BLOCK_ENTITY_DATA, this.getUser());
blockEntityData.write(Type.POSITION1_14, blockPosition); // position
blockEntityData.write(Type.VAR_INT, javaBlockEntity.typeId()); // type
blockEntityData.write(Type.COMPOUND_TAG, javaBlockEntity.tag()); // block entity tag
blockEntityData.scheduleSend(BedrockProtocol.class);
return new IntObjectImmutablePair<>(remappedBlockState, javaBlockEntity);
}
} else if (BlockStateRewriter.TAG_ITEM_FRAME.equals(tag)) {
this.getUser().get(EntityTracker.class).spawnItemFrame(blockPosition, blockStateRewriter.blockState(blockState));
}
}

return remappedBlockState;
return new IntObjectImmutablePair<>(remappedBlockState, null);
}

public BedrockChunkSection handleBlockPalette(final BedrockChunkSection section) {
Expand Down

0 comments on commit f06f3b8

Please sign in to comment.