[?~1.16.4] Mimic argument cause client crash when used with immersive engineering
mmis1000 opened this issue ยท 6 comments
Minecraft Version
1.16.4
Forge Version
net.minecraftforge:35.1.36
Mod Version
1.16.4-7.3.0-47
Describe your problem, including steps to reproduce it
- Install this mod with immersive engineering
- Put the router on ground
- Config the Extruder MKII with mimic argument
- Try to break the block
- Client crashed (server is fine, but probably just because client crash first?)
Any other comments?
Crash log:
https://gist.github.com/mmis1000/671d05966ebbe60cae980a98cbf584db
The modular router invoked the getDigSpeed
with a null position(because it did not matter here?)
But the immersive engineer listens the BreakSpeed event
and tries to read the entity info with given location
# The function in the stack
MC 1.16.3: net/minecraft/world/World.getTileEntity
Name: c => func_175625_s => getTileEntity
Hence crash the client (The pos
is null).
I am not sure which is the fault here.
Neither do I know whether that field is supposed to be null-able.
Either the Modular Router pass null as a non null-able parameter, or the Immersive engineer use a null-able parameter without check first?
The code is conflicting that, the pos
in EntityPlayer.getDigSpeed
is indeed @Nullable
.
But the BreakSpeed event says you should use a BlockPos that has y == -1
for unknown location
Probably Modular Routers at fault here (although as you say, the blockpos is explicitly marked as @Nullable
). Can't remember why I chose null there, probably because originally I used the method variant just taking a BlockState (back before it was deprecated):
@Deprecated //Use location sensitive version below
public float getDigSpeed(BlockState state) {
return getDigSpeed(state, null);
}
public float getDigSpeed(BlockState state, @Nullable BlockPos pos) {
...
However, I think it should be OK to pass event.getPos()
in there, or failing that a pos with Y == -1. I'll do some testing with that anyway.
@desht
I also notices that there is a static field ZERO
in the BlockPos
class.
And the comment says that is intended to be used as a unknown position.
(But that is probably unsafe in a mod server or even in vanilla? That said [0, 0, 0] is a valid location)
I am actually ultimately confused which is the proper value for an unknown position.
The codes conflicts with itself everywhere as far as I see.
After a bit of investigation, I'm settling on passing a (0,-1,0) blockpos, which as the event javadoc states, is an "unknown" position. I definitely can't pass getPos()
again, since that would lead to an infinite event loop...
This should fix the crash, since world.getTileEntity()
(as IE does in its handler) will just return null
for an out-of-bounds Y pos, which is fine here.
The fix is in build 49 from https://jenkins.k-4u.nl/job/Modular%20Routers%201.14+/ if you'd like to try it out.
Well, BlockPos.ZERO
would be generally invalid in regular worlds, since that location would contain bedrock and shouldn't be interacted with. But it could be valid in a void world, so it's not ideal to use as a special marker value...