Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fundamental Logic Issue with Placement #76

Open
KingLemming opened this issue May 19, 2023 · 5 comments
Open

Fundamental Logic Issue with Placement #76

KingLemming opened this issue May 19, 2023 · 5 comments

Comments

@KingLemming
Copy link

KingLemming commented May 19, 2023

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.

@Theta-Dev
Copy link
Owner

Do you have any examples for blocks causing crashes?

@Heaser
Copy link

Heaser commented May 20, 2023

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

@Theta-Dev
Copy link
Owner

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

@Theta-Dev
Copy link
Owner

@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) [?:?] {}

Theta-Dev added a commit that referenced this issue Jul 7, 2023
@Heaser
Copy link

Heaser commented Nov 14, 2023

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.

Theta-Dev added a commit that referenced this issue Aug 1, 2024
Theta-Dev added a commit that referenced this issue Aug 1, 2024
Theta-Dev added a commit that referenced this issue Aug 1, 2024
Theta-Dev added a commit that referenced this issue Aug 1, 2024
Theta-Dev added a commit that referenced this issue Aug 1, 2024
Theta-Dev added a commit that referenced this issue Aug 1, 2024
Theta-Dev added a commit that referenced this issue Aug 1, 2024
Theta-Dev added a commit that referenced this issue Aug 1, 2024
Theta-Dev added a commit that referenced this issue Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants