MehVahdJukaar / sawmill

Other
3 stars 8 forks source link

Extended Mushrooms: AssertionError: Wood item is empty #43

Open cech12 opened 2 months ago

cech12 commented 2 months ago

Hey,

I am the developer of Extended Mushrooms and I got an compatibility issue with your mod: https://github.com/cech12/ExtendedMushrooms/issues/87 It seems that an Exception is thrown if a generated ItemStack is empty instead of trying the next wood type. https://github.com/MehVahdJukaar/sawmill/blob/master/common/src/main/java/net/mehvahdjukaar/sawmill/CarpenterTrades.java#L131 For now, I don't know exactly why the generated ItemStack is empty there for my mushroom wood type. (maybe the woodprice is 0 or w is an empty item?), but a game crash is not the best way to react to that. :)

Versions:

Log: https://pastebin.com/7xnBuyCQ

MehVahdJukaar commented 2 months ago

well this can only mean that the wood detection failed and somehow put a "minecraft:air" item as the log type of that mushroom wood type. I believe the only way for this to happen is throguh the hardcoded list of a few odd modde wood types i have. These data some versions back and its likely that their IDS are outdated

MehVahdJukaar commented 2 months ago

I think thats the only way that could have happened. FOr an itemStack to be empty its must either have a 0 count (which the trade surely doesnt have), or its item must be air

MehVahdJukaar commented 1 month ago

honestly i really dont see how can that ever be possible. The only way i see that could have ever happened is if the blockItem map got messed up (for all items likely) and that mushroom log block returned an air block in its .asItem method

cech12 commented 1 month ago

Unfortunately, I could not reproduce the crash in my environment. I found a possible root of the issue on my side: I did not add the children "log" and "planks" to the woodtypes. https://github.com/cech12/ExtendedMushrooms/commit/81d9e5446d7faeb2c0c230a5c83026aca1ae28cb#diff-859e7594ace916b929875b9e733e53326eece56a5d83dbbeb352c213a9fc52c8

But as I said, without these I also could not reproduce the crash the user had here. I asked him/her to test the new version with my fix.

MehVahdJukaar commented 1 month ago

hm yeah that is definitely not correct since its mising those two. However i dont think it explains this as we know the returned item was not null (because of the if after it), as it would be the case when no child is added. Quite a mistery, i do believe that it was caused by some other mod messing up the block item map. That would have many many more side effects lika making all block items not work for example

cech12 commented 1 month ago

The user only gets the crash very rarely and has already tried a lot. He/she has a modpack that contains both Fabric and Forge mods. So it was a reasonable assumption that problems were occurring in the registry due to the combination of the various loader mods. But since it is not easy to reproduce, the user will have to live with the behavior. The related issue has been closed.

I would still recommend not reacting so harshly and crashing the game with the assertion, but logging it as an error and continuing to run through the loop. Then at least you can keep playing and it might crash in other places that do not affect our mods, where it might be more obvious what the problem is. :)