Fairy Lights

Fairy Lights

34M Downloads

[1.16.4] Large Fluid Tank incompatibility crash

RandomCanadian opened this issue ยท 4 comments

commented

When Large Fluid Tank is used with Fairy Lights it will cause a crash when placing down a tank from the aforementioned mod. It's reproducible and has been tested with only those two mods enabled. I've included a crashlog.
crash-2020-11-17_06.26.40-client.txt

commented

Thanks for reporting!

Unfortunately this is appears to be an issue with the other mod which I am unable to fix on my end.

commented

In this case, I think try-catch is the worst solution. Modders should not consider mixins, because the author of mixin mustn't change the specification of a class or a method. The method LazyOptional#ifPresent never fails unless you pass a consumer that throws exception. This is a common rule, and you should do your best not to break the rule. Surrounding with try-catch block means you think LazyOptional#ifPresent may throw some exceptions, though the document says no. It's bad.

commented

Okay, thanks for the explanation, I haven't been to Scala for a very long while. What you are saying totally makes sense!

commented

Oh well, a try-catch in there would've saved from the crash. There's already one for the other accept() overload, so this overload is probably just another point where there is a potential to fail with an exception? Fault-tolerance is a worthy pursuit all by itself, and a try-catch costs nothing, so imo it would be justified. Or do you think that catching this exception would be somehow harmful? Or maybe this case is too exotic since it's a version-related mixin problem?

Additional info for whoever might be interested:
There's some tested info that the latest Forge version that doesn't crash in this situation is 35.0.15
The related issue for FluidTank: Kotori316/FluidTank#64