-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
Fix issue with poorly implemented modded consumables returning null as foodcomponents #235
Conversation
Could you provide information about how this can be reproduced? As far as I can tell, this should be impossible since Is some mod overriding if (stack.getItem().isFood()) {
return this.getFoodComponent().isSnack() ? 16 : 32;
} |
Too be honest, I don't really know the details of the mod causing this, I only faced the bug itself. |
As for how the bug really happened, I cannot tell really, for the game only crashed spontaneously about anytime I was opening any form of inventory |
I suggest returning foodvalues.zero instead of null when it is null, and returning null causes all code(appleskin and user code of events) to add unknown branches |
That might be a good idea indeed, that's something I was not aware of. |
Until I understand more about what's going on, it's hard to know if this is the right fix (or if this is something that AppleSkin should try to protect against, or if it's a bug in some other mod). My suggestion would be to do something like this locally in if (itemFood == null) {
throw new RuntimeException("food with null FoodComponent: " + itemStack)
} and then reproduce the crash with that code. The crash should then let you know which |
From my understanding of the actual situation, that issue is something that might have already occurred with Forge as well, for there are these checks in the forge code (in int hunger = itemFood != null ? itemFood.getNutrition() : 0;
float saturationModifier = itemFood != null ? itemFood.getSaturationModifier() : 0; Which only makes me think that this was an attempt to prevent that same error to happen, although with a set of ternary operators rather than the option I went for in the end, after @SAGESSE-CN 's suggestion |
I'm convinced that this is a fine change. Could you remove the version bump from this PR, though? Out of curiosity, it might be worth tracking down the cause anyway. Here's a build of https://www.ryanliptak.com/misc/appleskin-fabric-mc1.19-2.4.1+issue235.jar |
Oh, sorry, I did that version change mostly to avoid getting confused between the builds. I can do that right away ofc |
…ood components raises a NullPointerException, causing the client to crash.
Thanks! Ended up just going with the same implementation as the Forge branch. |
Fair enough |
Prevents client crash upon trying to get how to render a modded consumable's details when its
getFoodComponent
method returns null.Includes modification for versions from 1.19 to 1.19.4 + version bump (done via cherry-picking commits)