Mekanism Additions

Mekanism Additions

21M Downloads

[1.16.1] Crash when attempting to rename an item in a Robit that's not docked

NielsPilgaard opened this issue ยท 6 comments

commented

Relayed from: EnigmaticaModpacks/Enigmatica6#296

Issue description:

Crash when attempting to rename an item in a Robit that's not docked.

I use Robit's all the time as anvils, but they're typically docked when doing so. The last two times I've tried renaming something, I had the Robit in follow mode and it resulted in a crash.

Steps to reproduce:

  1. Make and place Robit
  2. Try to rename something using the Robit's built in Anvil
  3. Crash

Version (make sure you are on the latest version before reporting):

Forge: 1.16.1-32.0.108
Mekanism: 1.16.1-10.0.9.432

If a (crash)log is relevant for this issue, link it here: (It's almost always relevant)

crash-2020-09-22_11.04.52-client.txt

commented

Yep you're right, works just fine without Apotheosis, couldn't reproduce with JEI + Mekanism. I'm relaying to Apotheosis :)
Edit: Thanks for the quick reply as always!

commented

I am actually going to leave this open for now just as a reminder for me to change our returning to be a bit safer with other mods potentially doing odd things given it doesn't really hurt us and truth be told in this case I am not quite sure if there is anything that Apotheosis can do to reasonably fix it

commented

So uh, what's null in here? The IWorldPosCallable on your repair container? I probably shouldn't have to null check that, but I can if necessary (when I start doing 1.16 stuff again).

commented

Oh, no, I see, it's the lack of the tile not existing there and the use of Optional.of requiring a non-null return there. Yeah there's probably some stupid garbage I can do to fix it, but I think the easiest thing would be for you to change EntityRobit.java:566 from Optional.of to Optional.ofNullable since you have the chance that TE's don't exist

commented

Ya hence why I reopened the issue given strictly speaking the error is on your end given if I was using one of the defaults that vanilla has for IWorldPosCallable based on position they also use Optional.of, but there isn't necessarily a great way for you to handle the TE being null and changing from Optional.of to Optional.ofNullable shouldn't really cause us any pain. Though it may make sense to be potentially checking if the RepairContainer's class is the same (not instanceof but class comparision) for your checks

commented

Does this happen without apotheosis? Given I have a sneaking suspicion that at shadows.apotheosis.ench.EnchModuleEvents.applyUnbreaking(EnchModuleEvents.java:189) ~[?:4.1.3] {re:classloading} may be the real cause, as I am pretty sure the last time I tested renaming an item was post port of mekanism to 1.16 not just before porting.

Edit: In fact I am pretty sure that it is because of this line that is in the stack trace: https://github.com/Shadows-of-Fire/Apotheosis/blob/1.16/src/main/java/shadows/apotheosis/ench/EnchModuleEvents.java#L189 It tries to get the TE at the position, but there is no TE so they are returning null and based on what I have looked at of IWorldPosCallable the BifFunction passed into it is not supposed to return null (or the same crash could be reproduced with a vanilla callable instead of a custom one like we implement. I am not really sure if there is something they can do about it on their end, and I will probably just for safety on our end change https://github.com/mekanism/Mekanism/blob/1.16.x/src/main/java/mekanism/common/entity/EntityRobit.java#L568 to being Optional.ofNullable to safeguard against it, though I am pretty sure the bug is on Apotheosis' end.