search

Bernasss12 / BetterEnchantedBooks

Makes it easier to identify different enchantment books.
MIT License
12 stars 9 forks source link

Crash when observing empty enchanted book, like the one in the Advancements screen #66

Closed Partonetrain closed 4 months ago

Partonetrain commented 4 months ago

The game crashes when trying to render an enchanted book without enchantment data, such as the icon of the advancement Enchanter (minecraft:story/enchant_item).

Not a Kotlin user, but it looks like there's no check for this

To Reproduce Open Advancements screen

Expected behavior No crash

Screenshots

Please include:

Crash stack:

java.util.NoSuchElementException: List is empty.
    at kotlin.collections.CollectionsKt___CollectionsKt.first(_Collections.kt:214)
    at dev.bernasss12.bebooks.util.NBTUtil.getPriorityEnchantmentData(NBTUtil.kt:45)
    at dev.bernasss12.bebooks.manage.BookColorManager.itemColorProvider$lambda$1(BookColorManager.kt:22)
    at net.minecraft.class_325.method_1704(class_325.java:86)
    at net.fabricmc.fabric.impl.client.indigo.renderer.render.ItemRenderContext.colorizeQuad(ItemRenderContext.java:182)
    at net.fabricmc.fabric.impl.client.indigo.renderer.render.ItemRenderContext.renderQuad(ItemRenderContext.java:175)
    at net.fabricmc.fabric.impl.client.indigo.renderer.render.ItemRenderContext$1.emitDirectly(ItemRenderContext.java:73)
    at net.fabricmc.fabric.impl.client.indigo.renderer.mesh.MutableQuadViewImpl.emit(MutableQuadViewImpl.java:220)
    at net.fabricmc.fabric.impl.client.indigo.renderer.mesh.MutableQuadViewImpl.emit(MutableQuadViewImpl.java:56)
    at net.fabricmc.fabric.impl.renderer.VanillaModelEncoder.emitItemQuads(VanillaModelEncoder.java:81)
    at net.fabricmc.fabric.api.renderer.v1.model.FabricBakedModel.emitItemQuads(FabricBakedModel.java:128)
    at net.fabricmc.fabric.api.renderer.v1.model.ForwardingBakedModel.emitItemQuads(ForwardingBakedModel.java:56)
    at me.pepperbell.continuity.client.model.EmissiveBakedModel.emitItemQuads(EmissiveBakedModel.java:84)
    at net.fabricmc.fabric.impl.client.indigo.renderer.render.ItemRenderContext.renderModel(ItemRenderContext.java:131)
    at net.minecraft.class_918.handler$dpd000$fabric-renderer-indigo$hook_renderItem(class_918.java:5549)
    at net.minecraft.class_918.method_23179(class_918.java:125)
    at net.minecraft.class_332.method_51425(class_332.java:531)
    at net.minecraft.class_332.method_51424(class_332.java:511)
    at net.minecraft.class_332.method_51445(class_332.java:503)
    at betteradvancements.gui.BetterAdvancementWidget.draw(BetterAdvancementWidget.java:239)
    at betteradvancements.gui.BetterAdvancementWidget.draw(BetterAdvancementWidget.java:243)
    at betteradvancements.gui.BetterAdvancementWidget.draw(BetterAdvancementWidget.java:243)
    at betteradvancements.gui.BetterAdvancementWidget.draw(BetterAdvancementWidget.java:243)
    at betteradvancements.gui.BetterAdvancementWidget.draw(BetterAdvancementWidget.java:243)
    at betteradvancements.gui.BetterAdvancementWidget.draw(BetterAdvancementWidget.java:243)
    at betteradvancements.gui.BetterAdvancementWidget.draw(BetterAdvancementWidget.java:243)
    at betteradvancements.gui.BetterAdvancementTab.drawContents(BetterAdvancementTab.java:100)
    at betteradvancements.gui.BetterAdvancementsScreen.renderInside(BetterAdvancementsScreen.java:386)
    at betteradvancements.gui.BetterAdvancementsScreen.method_25394(BetterAdvancementsScreen.java:220)
    at net.minecraft.class_437.method_47413(class_437.java:110)
    at net.minecraft.class_757.mixinextras$bridge$method_47413$219(class_757.java)
    at net.minecraft.class_757.wrapOperation$efa000$fancymenu$wrapRenderScreenFancyMenu(class_757.java:5607)
    at net.minecraft.class_757.mixinextras$bridge$wrapOperation$efa000$fancymenu$wrapRenderScreenFancyMenu$220(class_757.java)
    at net.minecraft.class_757.wrapOperation$eli000$konkrete$wrapRenderScreenKonkrete(class_757.java:6106)
    at net.minecraft.class_757.method_3192(class_757.java:945)
    at net.minecraft.class_310.method_1523(class_310.java:1219)
    at net.minecraft.class_310.method_1514(class_310.java:802)
    at net.minecraft.client.main.Main.main(Main.java:250)
    at net.fabricmc.loader.impl.game.minecraft.MinecraftGameProvider.launch(MinecraftGameProvider.java:470)
    at net.fabricmc.loader.impl.launch.knot.Knot.launch(Knot.java:74)
    at net.fabricmc.loader.impl.launch.knot.KnotClient.main(KnotClient.java:23)

(crash stack mentions other mods, but the issue happens anywhere with an enchanted book without NBT data)

Fourmisain commented 4 months ago

I see Bernass is already on it.

Just wanted to mention: The Java code (linked above) doesn't have this issue, findFirst returns an empty Optional if the collection is empty (and the whole method returns "" in this case).

The Kotlin code would need to use something like firstOrNull instead of first here and make the whole method nullable probably.

Bernasss12 commented 4 months ago

I added a check stack.hasEnchantments() before any NBT parsing so I could avoid making the method return nullable and add complexity when it's really not useful (at least at first sight, might change opinion in the future as I usually do :P).

Bernasss12 commented 4 months ago

The fix commit was pushed now I'm just manually building and uploading the files. Really should look into automating that hahaha

Partonetrain commented 4 months ago

Needs to be uploaded to CurseForge :)

Bernasss12 commented 4 months ago

It's under manual review since saturday morning already, I'm not sure what to do about it at this point :(

Edit: image

The first fix patched the crash but broke the rest of the mod, oops :P But the second version is the right one and still under review.