QuiltMC / quilt-config

Quilt Config (aka "haven king's metadata of madness") is a library designed to facilitate the creation and management of config files.
Apache License 2.0
21 stars 8 forks source link

Final primitives in wrappedconfig don't seem to read from files #9

Closed sisby-folk closed 7 months ago

sisby-folk commented 1 year ago

In the docs for WrappedConfig, there's a recommendation to use boolean, and an assertion that all fields must be final image

The latter is definitely right - there's a startup crash if you don't, but the former seemingly prevents values other than the default from ever loading in.

To reproduce:

Using Boolean doesn't seem to have this problem. Not really sure how, but it's our workaround for now. This is all 1.19.2, so I'm unsure if anything's moved around.

TheGlitch76 commented 1 year ago

This is a fundamental problem with wrapped configs. The new ReflectiveConfig system will replace WrappedConfig starting in Loader 0.20 (hopefully!), and won't have this issue.

sisby-folk commented 1 year ago

can the docs be updated to reflect this?

On Thu, 11 May 2023, 12:49 pm Glitch, @.***> wrote:

This is a fundamental problem with wrapped configs. The new ReflectiveConfig system will replace WrappedConfig starting in Loader 0.20 (hopefully!), and won't have this issue.

— Reply to this email directly, view it on GitHub https://github.com/QuiltMC/quilt-config/issues/9#issuecomment-1543253479, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANJ34KNTBHFLAQNGUCTCX23XFRHU5ANCNFSM6AAAAAAXUDVEBA . You are receiving this because you authored the thread.Message ID: @.***>

TheGlitch76 commented 1 year ago

None of this is released yet

sisby-folk commented 1 year ago

I meant changing boolean in the docs to reflect the fact that it can't be used

On Thu, 11 May 2023, 12:53 pm Glitch, @.***> wrote:

None of this is released yet

— Reply to this email directly, view it on GitHub https://github.com/QuiltMC/quilt-config/issues/9#issuecomment-1543255905, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANJ34KM6CA4Y6G6MP5HG5Z3XFRIBXANCNFSM6AAAAAAXUDVEBA . You are receiving this because you authored the thread.Message ID: @.***>

OroArmor commented 1 year ago

Was this fixed with the new update?

sisby-folk commented 12 months ago

Uh, how do you even do primitives in ReflectiveConfig? I don't think i'm getting it.

when field is final boolean in reflective in 1.1.0 beta 3: Class 'java.lang.Boolean' of field 'preventAdvancementBroadcasts' of config class 'folk.sisby.crunchy_crunchy_advancements.CrunchyConfig'is not a valid config value: it must be a TrackedValue or implement org.quiltmc.loader.api.Config.Section

AlexIIL commented 12 months ago

I think every field in a ReflectiveConfig must be a TrackedValue (or Section), and not a primitive - you can create them with ReflectiveConfig.value()

sisby-folk commented 12 months ago

Ah so you can just call value(false) seeing as you're in a ReflectiveConfig - works for me.

To answer the question - no, this does not solve the problem. TrackedValue<Boolean> (for reflective) and Boolean (for wrapped) are both not the same as boolean, so the docs still need to be updated to use the object types in both cases

A basic type (int, long, float, double, boolean, String, or enum)

should be something like

A basic type (Integer, Long, Float, Double, Boolean, String, or an enum)

for wrapped.

And then whatever TrackedValue<?> stuff for reflective.

ix0rai commented 9 months ago

all javadocs have been corrected as far as I can tell, this can be closed

sisby-folk commented 8 months ago

? image