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

[1.12] Fluid Pipe Crash With EnderIO Liquid Pipes #3028

Closed
ghost opened this issue Jun 2, 2018 · 7 comments
Closed

[1.12] Fluid Pipe Crash With EnderIO Liquid Pipes #3028

ghost opened this issue Jun 2, 2018 · 7 comments

Comments

@ghost
Copy link

ghost commented Jun 2, 2018

IE Version:ImmersiveEngineering-0.12-82
EnderIO version": "5.0.25",
mc version": "1.12.2"

You can easily trigger it, just need place a EnderIO Liquid Pipes near a Fluid Pipe(IE), And then ....

java.lang.NullPointerException: Tesselating block in world
or (2 kinds of error)
java.lang.NullPointerException: Unexpected error

Crash report for kind 2 see here:
https://github.com/A-new-Account/MinecraftCrashReportes/blob/master/crash.log

Crash report for kind 1 see here:

Details:
Block type: ID #2625 (tile.immersiveengineering.metal_device1 // blusunrize.immersiveengineering.common.blocks.metal.BlockMetalDevice1)
Block data value: 6 / 0x6 / 0b0110
Block location: World: (59,68,0), Chunk: (at 11,4,0 in 3,0; contains blocks 48,0,0 to 63,255,15), Region: (0,0; contains chunks 0,0 to 31,31, blocks 0,0,0 to 511,255,511)
Stacktrace:
at net.minecraft.client.renderer.BlockRendererDispatcher.func_175018_a(BlockRendererDispatcher.java:79)
at codechicken.lib.render.block.CCBlockRendererDispatcher.func_175018_a(CCBlockRendererDispatcher.java:64)
at net.minecraft.client.renderer.chunk.RenderChunk.func_178581_b(RenderChunk.java:200)
at net.minecraft.client.renderer.chunk.ChunkRenderWorker.func_178474_a(SourceFile:100)
at net.minecraft.client.renderer.chunk.ChunkRenderDispatcher.func_178505_b(ChunkRenderDispatcher.java:172)
at net.minecraft.client.renderer.RenderGlobal.func_174970_a(RenderGlobal.java:976)
at net.minecraft.client.renderer.EntityRenderer.func_175068_a(EntityRenderer.java:1316)
at net.minecraft.client.renderer.EntityRenderer.func_78471_a(EntityRenderer.java:1259)

@Alaberti
Copy link

Alaberti commented Jun 2, 2018

Did you also report to EnderIO?

@malte0811
Copy link
Collaborator

@tterrag1098: Is there any reason you return true in hasCapability for client-side conduits (here) but ignore them in getCapability? I'm pretty sure that's what's going wrong here.

@HenryLoenwind
Copy link

HenryLoenwind commented Jun 10, 2018

Yes, there is a reason...:

image

At least for why the conduits responding to hasCap() on the client. The getCap() call was probably an oversight---although I don't expect any of our caps to actually work on the client.

@EpicSquid I'd propose to report the orginal issue as bug to RS and revert that change after they fix it.

@HenryLoenwind
Copy link

HenryLoenwind commented Jun 10, 2018

PS: Also, this code is wrong: https://github.com/BluSunrize/ImmersiveEngineering/blob/master/src/main/java/blusunrize/immersiveengineering/common/blocks/metal/TileEntityFluidPipe.java#L434-L436

hasCap() is NOT a null-checker for getCap() It is a light-weight alternative for when you do not need the cap but only want to know of a block supports it. Because of mods misusing it like you, we now have boolean hasCap(...) {return this.getCap() != null;} as impl in most blocks. Which needlessly wastes cpu time and memory, but as mods like to crash when those two don't match to the last bit, I see no alternative that is guaranteed to absolutely never return differing results.

@malte0811
Copy link
Collaborator

PS: Also, this code is wrong: https://github.com/BluSunrize/ImmersiveEngineering/blob/master/src/main/java/blusunrize/immersiveengineering/common/blocks/metal/TileEntityFluidPipe.java#L434-L436

I asked Lex about it (Forge Discord, Modder support channel). "Potentially inefficient" might be a correct description, but "wrong" is absolutely not a correct description.
image

@Alaberti
Copy link

Alaberti commented Jun 10, 2018

Uh maybe this issue should be locked and you awesome devs work out the fix through private channels?

@HenryLoenwind
Copy link

Actually, @LexManos demo implementation is bullshit. He completely forgets to call super, so any third-party cap that is attached to that item/te will fail miserably.

Also, his inability to see that it is a very bad idea to try to write to different pieces of code that absolutely must produce the same result is puzzling at best. Because the only way to guarantee that in non-trivial cases is to not have different code, but only one piece of code that is run at both methods---which completely negates the "light weight"-ness of having the second call---you now run the full cap creation code twice, just to save a null check. Bravo.

It also has other implications: To support hasCap() for rendering purposes client-side means that you also have to support getCap() client-side, which means that you have to sync your complete data for the cap to the client---because people will bug-report client-side cap objects that do nothing.

And all of this bs because some people read "This is a light weight version of getCapability, intended for metadata uses." as "Hey, call this so you don't need to null-check t he result of getCap()".

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