LXGaming / Sledgehammer

Smashes the stupid out of the client & server.
Apache License 2.0
16 stars 5 forks source link

Work around a crash with Industrial Foregoing fluid pumps #20

Closed embeddedt closed 3 years ago

embeddedt commented 3 years ago

The original issue was that a NullPointerException occured on this line. My understanding of the Forge code is quite limited, but presumably there is some type of race condition where the pump expects fluid to be on that block, but it gets removed before the pump actually pumps from it.

The workaround I used here is to simply return a fake IFluidHandler that will get it past that line without crashing or pumping nonexistent fluid. I'm not 100% sure if the pump will actually continue to keep pumping after this happens, but this behavior is still better than a full server crash.

I couldn't figure out how to get the right artifact name for CurseForge's Maven, so I used cursemaven.com instead. Pointers on how to do that would be appreciated. :slightly_smiling_face:

Fixes InnovativeOnlineIndustries/Industrial-Foregoing#743 Fixes InnovativeOnlineIndustries/Industrial-Foregoing#673

LXGaming commented 3 years ago

I appreciate that you've gone through the effort of making this PR however there is an easier way to fix this issue which doesn't involve creating a FakeFluidHandler, Looking at line in question FluidPumpTile.java#L101 if we redirect the handler.drain(1000, true); we can check if the handler is null and return a null FluidStack as the code afterwards will gracefully handle it.

embeddedt commented 3 years ago

Ah, that's how redirections work? Good to know. For some reason, I thought it would still throw the NullPointerException even if I redirected drain.

Your solution is much easier to understand; thanks. :slightly_smiling_face: