Configured

Configured

33M Downloads

Configured does not properly handle array-based configuration options

FireController1847 opened this issue ยท 3 comments

commented

Hey there!

I appreciate your work on this MrCrayfish, just a slight bug that's been driving me kind of crazy with hard-to-track reports flying around recently haha.

It appears that Configured does not handle ArrayList-based configuration options very well. Easiest way to reproduce is to provide an ArrayList<Integer> configuration option. Configured will show no numbers in the list, and when you add an option it will insert it as a string, replacing all existing values as strings also.

Additionally, this mod bypasses Forge's validation system when the configuration is modified (from a file, try making it a string it'll auto-reset itself due to invalid config). I'll make a separate issue for this, however.

Hoping to see this fixed soon, at least now I know what's going on haha!

commented

This is a difficult issue and I don't know if it can even be fixed without modders providing extra data. Due to the dynamic nature of this mod, it's very difficult to understand configuration values that are not of a primitive type. This mainly comes down to the mess that is generic types. The only real option I can see happening is to allow modders to pass a reference to a static method (that I can easily invoke with reflection) that can parse my list of strings back into their list of integers or custom type.

https://gitlab.com/FireController1847/levelhearts/-/blob/forge-1.17.x/src/main/java/com/firecontroller1847/levelhearts/Config.java#L91
On closer inspection, the configuration value referenced in the crash does not provide a custom validator. If you look into ForgeConfigSpec.Builder#define you'll see that the validator is comparing the class of the default value, which is an ArrayList. This will allow an ArrayList of any type to be accepted, regardless of the generic type. I would still need to implement the validation check in Configured. I'd recommend you use ForgeConfigSpec.Builder#defineList which allows you to validate elements of the list.

commented

Hey there, thanks for that definition tip, I'll be sure to check that out and use that!

In any case, I totally understand hell with generics. I know I personally would absolutely not mind having to provide extra data to this mod to allow it to configure properly. Back in 1.12 I used the annotation system that Forge provided for GUI compatibility, and so I absolutely wouldn't mind this I miss having the GUI capability for sure.

Quite frankly if somebody provided an entire lib to re-add that annotation system I wouldn't mind using it haha.

Thanks again for your work, let me know if there's anything I can do to help

commented

This issue has been resolved. It handles most primitive types (that are supported by forge's config system).