Construction Wand

Construction Wand

22M Downloads

Fundamental Logic Issue with Placement

KingLemming opened this issue ยท 5 comments

commented

Hey there! So I've received some reports that various blocks crash after being placed with your wand, so I did some investigating and found the culprit.

It's right here: https://github.com/Theta-Dev/ConstructionWand/blob/1.19/src/main/java/thetadev/constructionwand/wand/undo/PlaceSnapshot.java#L113

You're calling a function on a BlockState before the block exists in world. I understand why you feel you can do this, but you absolutely cannot. Even vanilla blocks which are waterloggable will generate world ticks in that function - the method does not return a simple blockstate and can have impacts on the world itself. The fact that the world must be passed in means that it's entirely valid for blocks to use the world to determine the state. That includes tile entities. But when you call this without the block being in world, the tile entity logic breaks completely. Don't do this.

Instead, create a BlockPlaceContext and use the getStateForPlacement method (which you already do). Then PLACE the block, and call updateShape if you feel you absolutely need to.

commented

Do you have any examples for blocks causing crashes?

commented

Do you have any examples for blocks causing crashes?

All ducts from thermal dynamics cause an exception when placed with the construction wand.

Edit:
I should've probably mentioned, it was tested on Minecraft version 1.19.2

commented

Okay, I will remove these function calls. Seems like they are not necessary.

commented

@KingLemming I removed the line you mentioned. However Minecraft still shows an exception and creates ghost blocks when trying to place multiple ThermalDynamics ducts using a wand.

Here is the exception. Do you know what is causing this?

[22:34:07] [Server thread/ERROR] [minecraft/PacketUtils]: Failed to handle packet net.minecraft.network.protocol.game.ServerboundUseItemOnPacket@6eca91fc, suppressing error
java.lang.NullPointerException: Cannot read field "nodeGraph" because "e" is null
	at cofh.thermal.dynamics.handler.GridContainer.lambda$mergeGrids$0(GridContainer.java:140) ~[thermal_dynamics-1.19.2-10.2.1b.14.jar%23161!/:10.2.1b] {re:classloading}
	at cofh.requack.collection.ColUtils.maxBy(ColUtils.java:73) ~[thermal_dynamics-1.19.2-10.2.1b.14.jar%23161!/:10.2.1b] {re:classloading}
	at cofh.thermal.dynamics.handler.GridContainer.mergeGrids(GridContainer.java:140) ~[thermal_dynamics-1.19.2-10.2.1b.14.jar%23161!/:10.2.1b] {re:classloading}
	at cofh.thermal.dynamics.handler.GridContainer.gridHostPlaced(GridContainer.java:106) ~[thermal_dynamics-1.19.2-10.2.1b.14.jar%23161!/:10.2.1b] {re:classloading}
	at cofh.thermal.dynamics.handler.GridContainer.onDuctPlaced(GridContainer.java:71) ~[thermal_dynamics-1.19.2-10.2.1b.14.jar%23161!/:10.2.1b] {re:classloading}
	at cofh.thermal.dynamics.block.DuctBlock.m_6807_(DuctBlock.java:214) ~[thermal_dynamics-1.19.2-10.2.1b.14.jar%23161!/:10.2.1b] {re:classloading}
	at net.minecraft.world.level.block.state.BlockBehaviour$BlockStateBase.m_60696_(BlockBehaviour.java:686) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading,re:mixin}
	at net.minecraftforge.common.ForgeHooks.onPlaceItemIntoWorld(ForgeHooks.java:732) ~[forge-1.19.2-43.2.11-universal.jar%23168!/:?] {re:classloading}
	at net.minecraft.world.item.ItemStack.m_41661_(ItemStack.java:236) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:mixin,xf:fml:forge:itemstack,re:classloading,xf:fml:forge:itemstack}
	at net.minecraft.server.level.ServerPlayerGameMode.m_7179_(ServerPlayerGameMode.java:355) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading}
	at net.minecraft.server.network.ServerGamePacketListenerImpl.m_6371_(ServerGamePacketListenerImpl.java:1060) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading}
	at net.minecraft.network.protocol.game.ServerboundUseItemOnPacket.m_5797_(ServerboundUseItemOnPacket.java:34) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading}
	at net.minecraft.network.protocol.game.ServerboundUseItemOnPacket.m_5797_(ServerboundUseItemOnPacket.java:8) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading}
	at net.minecraft.network.protocol.PacketUtils.m_131356_(PacketUtils.java:22) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading}
	at net.minecraft.server.TickTask.run(TickTask.java:18) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading}
	at net.minecraft.util.thread.BlockableEventLoop.m_6367_(BlockableEventLoop.java:157) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:mixin,pl:accesstransformer:B,re:computing_frames,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B}
	at net.minecraft.util.thread.ReentrantBlockableEventLoop.m_6367_(ReentrantBlockableEventLoop.java:23) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:mixin,re:computing_frames,re:classloading}
	at net.minecraft.server.MinecraftServer.m_6367_(MinecraftServer.java:763) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading,pl:accesstransformer:B}
	at net.minecraft.server.MinecraftServer.m_6367_(MinecraftServer.java:157) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading,pl:accesstransformer:B}
	at net.minecraft.util.thread.BlockableEventLoop.m_7245_(BlockableEventLoop.java:131) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:mixin,pl:accesstransformer:B,re:computing_frames,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B}
	at net.minecraft.server.MinecraftServer.m_129961_(MinecraftServer.java:746) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading,pl:accesstransformer:B}
	at net.minecraft.server.MinecraftServer.m_7245_(MinecraftServer.java:740) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading,pl:accesstransformer:B}
	at net.minecraft.util.thread.BlockableEventLoop.m_18701_(BlockableEventLoop.java:140) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:mixin,pl:accesstransformer:B,re:computing_frames,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B}
	at net.minecraft.server.MinecraftServer.m_130012_(MinecraftServer.java:726) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading,pl:accesstransformer:B}
	at net.minecraft.server.MinecraftServer.m_130011_(MinecraftServer.java:658) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading,pl:accesstransformer:B}
	at net.minecraft.server.MinecraftServer.m_206580_(MinecraftServer.java:244) ~[client-1.19.2-20220805.130853-srg.jar%23163!/:?] {re:classloading,pl:accesstransformer:B}
	at java.lang.Thread.run(Thread.java:833) [?:?] {}
commented

For my mod, this issue was resolved by stop using the @onlyin(Dist.CLIENT) annotation

Once swapped to distExecutor the issue with thermal ducts didn't happen anymore.