isXander / YetAnotherConfigLib

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

Inherited @SerialEntry fields are not saved in json #129

Open goombamaui opened 9 months ago

goombamaui commented 9 months ago

In the previous version of yacl, where config instances were built like so: GsonConfigInstance.createBuilder(c) .setPath( Path.of(CONFIG_PATH,"cfg.json")).build(); If there was an abstract class A with a @ConfigEntry field, and it was extended by class B, then GsonConfigInstance.createBuilder(B.class) would, when built and saved, end up saving an instance of the @ConfigEntry fields in the abstract class. However, in the new version, where a config class handler is created as follows: ConfigClassHandler.createBuilder(c) Inherited fields are not saved in the config. Is this intended behavior for the new version?

A simple mixin can be used to emulate the behavior of previous versions:

@Mixin(targets = "dev/isxander/yacl3/config/v2/impl/ConfigClassHandlerImpl")
public class ConfigClassHandlerImplMixin {
    private static List<Field> getAllFields(Class<?> type) {
        List<Field> fields = new ArrayList<Field>();
        for (Class<?> c = type; c != null; c = c.getSuperclass()) {
            fields.addAll(Arrays.asList(c.getDeclaredFields()));
        }
        return fields;
    }

    @Redirect(method= "<init>(Ljava/lang/Class;Lnet/minecraft/util/Identifier;Ljava/util/function/Function;)V",
            at = @At(
                    value = "INVOKE", target = "Ljava/lang/Class;getDeclaredFields()[Ljava/lang/reflect/Field;"
            ))
    public Field[] redir(Class instance){
        List<Field> fs = getAllFields(instance);
        return fs.toArray(new Field[fs.size()]);
    }
}
isXander commented 9 months ago

It is intended behaviour. However, nesting objects like this is something I would like to support.

Your mixin method does flatten the whole config into the root object in the json, so an approach like this is not what I'd like.