search

isXander / YetAnotherConfigLib

YetAnotherConfigLib (yacl) is just that. A builder-based configuration library for Minecraft.
GNU Lesser General Public License v3.0
96 stars 37 forks source link

Missing json sub fields (or adding new config sub fields) crashes config screen due to deserializing with gson without default constructors #199

Closed kevinthegreat1 closed 1 month ago

kevinthegreat1 commented 1 month ago

What I wanted

I have an existing config class:

    public static class QuickNavItem {
        @SerialEntry
        public boolean render = true;

        @SerialEntry
        public String uiTitle;

        @SerialEntry
        public String clickEvent;
    }

which serializes as such:

      "render": true,
      "uiTitle": "Your Skills",
      "clickEvent": "/skills"

I want to add a new field so my config looks like this:

    public static class QuickNavItem {
        @SerialEntry
        public boolean render = true;

        @SerialEntry
        public String uiTitle;

        @SerialEntry
        public String tooltip;

        @SerialEntry
        public String clickEvent;
    }

What I tried

I've tried setting a default value to the tooltip field, adding required = false, or adding nullable = true.

What I encountered

All of the options above fails to load the old config file with the following error:

[23:46:20] [Render thread/ERROR] (Minecraft) Reported exception thrown!
net.minecraft.util.crash.CrashException: mouseClicked event handler
    at net.minecraft.client.gui.screen.Screen.wrapScreenError(Screen.java:467) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at net.minecraft.client.Mouse.onMouseButton(Mouse.java:96) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at net.minecraft.client.Mouse.method_22686(Mouse.java:192) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at net.minecraft.util.thread.ThreadExecutor.execute(ThreadExecutor.java:90) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at net.minecraft.client.Mouse.method_22684(Mouse.java:192) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at org.lwjgl.glfw.GLFWMouseButtonCallbackI.callback(GLFWMouseButtonCallbackI.java:43) ~[lwjgl-glfw-3.3.3.jar:?]
    at org.lwjgl.system.JNI.invokeV(Native Method) ~[lwjgl-3.3.3.jar:?]
    at org.lwjgl.glfw.GLFW.glfwWaitEventsTimeout(GLFW.java:3509) ~[lwjgl-glfw-3.3.3.jar:?]
    at com.mojang.blaze3d.systems.RenderSystem.limitDisplayFPS(RenderSystem.java:160) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at net.minecraft.client.MinecraftClient.render(MinecraftClient.java:1295) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at net.minecraft.client.MinecraftClient.run(MinecraftClient.java:884) [minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at net.minecraft.client.main.Main.main(Main.java:228) [minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at net.fabricmc.loader.impl.game.minecraft.MinecraftGameProvider.launch(MinecraftGameProvider.java:470) [fabric-loader-0.15.11.jar:?]
    at net.fabricmc.loader.impl.launch.knot.Knot.launch(Knot.java:74) [fabric-loader-0.15.11.jar:?]
    at net.fabricmc.loader.impl.launch.knot.KnotClient.main(KnotClient.java:23) [fabric-loader-0.15.11.jar:?]
    at net.fabricmc.devlaunchinjector.Main.main(Main.java:86) [dev-launch-injector-0.2.1+build.8.jar:?]
Caused by: java.lang.NullPointerException: Cannot invoke "String.isEmpty()" because "this.inputField" is null
    at dev.isxander.yacl3.gui.controllers.string.StringControllerElement.getValueText(StringControllerElement.java:461) ~[yet-another-config-lib-3.5.0+1.21-fabric.jar:?]
    at dev.isxander.yacl3.gui.controllers.string.StringControllerElement.setDimension(StringControllerElement.java:436) ~[yet-another-config-lib-3.5.0+1.21-fabric.jar:?]
    at dev.isxander.yacl3.gui.controllers.string.StringControllerElement.<init>(StringControllerElement.java:45) ~[yet-another-config-lib-3.5.0+1.21-fabric.jar:?]
    at dev.isxander.yacl3.gui.controllers.string.IStringController.provideWidget(IStringController.java:42) ~[yet-another-config-lib-3.5.0+1.21-fabric.jar:?]
    at dev.isxander.yacl3.gui.OptionListWidget.refreshOptions(OptionListWidget.java:75) ~[yet-another-config-lib-3.5.0+1.21-fabric.jar:?]
    at dev.isxander.yacl3.gui.OptionListWidget.<init>(OptionListWidget.java:40) ~[yet-another-config-lib-3.5.0+1.21-fabric.jar:?]
    at dev.isxander.yacl3.gui.YACLScreen$CategoryTab.<init>(YACLScreen.java:359) ~[yet-another-config-lib-3.5.0+1.21-fabric.jar:?]
    at dev.isxander.yacl3.gui.YACLScreen.lambda$init$4(YACLScreen.java:89) ~[yet-another-config-lib-3.5.0+1.21-fabric.jar:?]
    at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) ~[?:?]
    at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:1024) ~[?:?]
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?]
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?]
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:575) ~[?:?]
    at java.base/java.util.stream.AbstractPipeline.evaluateToArrayNode(AbstractPipeline.java:260) ~[?:?]
    at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:616) ~[?:?]
    at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:622) ~[?:?]
    at java.base/java.util.stream.ReferencePipeline.toList(ReferencePipeline.java:627) ~[?:?]
    at dev.isxander.yacl3.gui.YACLScreen.init(YACLScreen.java:90) ~[yet-another-config-lib-3.5.0+1.21-fabric.jar:?]
    at net.minecraft.client.gui.screen.Screen.init(Screen.java:324) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at net.minecraft.client.MinecraftClient.setScreen(MinecraftClient.java:1134) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at com.terraformersmc.modmenu.gui.widget.entries.ModListEntry.openConfig(ModListEntry.java:120) ~[modmenu-11.0.0-beta.1.jar:?]
    at com.terraformersmc.modmenu.gui.widget.entries.ModListEntry.mouseClicked(ModListEntry.java:110) ~[modmenu-11.0.0-beta.1.jar:?]
    at com.terraformersmc.modmenu.gui.widget.ModListWidget.mouseClicked(ModListWidget.java:270) ~[modmenu-11.0.0-beta.1.jar:?]
    at net.minecraft.client.gui.ParentElement.mouseClicked(ParentElement.java:45) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    at net.minecraft.client.Mouse.method_1611(Mouse.java:96) ~[minecraft-merged-f6269c2542-1.21-loom.mappings.1_21.layered+hash.998484755-v2.jar:?]
    ... 16 more
kevinthegreat1 commented 1 month ago

Originally reported SkyblockerMod/Skyblocker#890 and here SkyblockerMod/Skyblocker#894.

In general, we've encountered many problems when the config simply refuses to load and crashes when it tries to deserialize a null or nonexistent field. I believe there should be alternative options such as default values instead of crashing.

viciscat commented 1 month ago

I have no affiliation with that man and yet I agree with all he says /j

Fluboxer commented 1 month ago

I also have no affiliation with that man and yet I agree with all he says

isXander commented 1 month ago

Is a default option not specified by just giving the field an initial value?

kevinthegreat1 commented 1 month ago

Is a default option not specified by just giving the field an initial value?

No. I also do have a constructor for this class, and I’ve tried both adding a default value to the field and making sure the constructor doesn’t set a null value. But to my knowledge, the constructor shouldn’t get called while deserializing from json?

kevinthegreat1 commented 1 month ago

I figured it out. First, this is a sub class, so this is serialized and deserialized with gson. Turns out having a constructor does matter. It means that there is no default constructor, and gson skips initialization if there are no default constructors available, which means the default values do not apply.

Solution

Have default values for the fields. And either:

  1. Have no constructor.
  2. Have a default constructor available (one with no args).

Future steps

I don't think there's much yacl van do about this, except warn users about this behavior.