Exlll / ConfigLib

A Minecraft library for saving, loading, updating, and commenting YAML configuration files
MIT License
135 stars 17 forks source link

Polymorphic problem #34

Closed alexdev03 closed 8 months ago

alexdev03 commented 8 months ago

Describe the bug If we have a mother class with fields and 2 subclasses, with fields, only the subclasses fields are serialized/deserialized

To Reproduce

@Getter
    @NoArgsConstructor
    @AllArgsConstructor
    @Accessors(fluent = true)
    @Polymorphic
    @PolymorphicTypes({
            @PolymorphicTypes.Type(type = IntegerBackground.class, alias = "integer"),
            @PolymorphicTypes.Type(type = HexBackground.class, alias = "hex")
    })
    public static class Background {

        protected boolean enabled;
        protected int opacity;
        protected boolean shadowed;
        protected boolean seeThrough;

        public Color getColor() {
            return Color.BLACK.setAlpha(0);
        }
    }

    @Getter
    @NoArgsConstructor
    @Configuration
    @Polymorphic
    public static class IntegerBackground extends Background {

        private int red;
        private int green;
        private int blue;

        public IntegerBackground(boolean enabled, int red, int green, int blue, int opacity, boolean shadowed, boolean seeThrough) {
            super(enabled, opacity, shadowed, seeThrough);
            this.red = red;
            this.green = green;
            this.blue = blue;
        }

        @Override
        public Color getColor() {
            return !enabled ? Color.BLACK.setAlpha(0) : Color.fromRGB(red, green, blue).setAlpha(opacity);
        }
    }

    @Getter
    @NoArgsConstructor
    @AllArgsConstructor
    @Configuration
    @Polymorphic
    public static class HexBackground extends Background {

        private String hex;

        public HexBackground(boolean enabled, String hex, int opacity, boolean shadowed, boolean seeThrough) {
            super(enabled, opacity, shadowed, seeThrough);
            this.hex = hex;
        }

        @Override
        public Color getColor() {
            int hex = Integer.parseInt(this.hex.substring(1), 16);
            return !enabled ? Color.BLACK.setAlpha(0) : Color.fromRGB(hex).setAlpha(opacity);
        }
    }

Expected behavior Mother fields should also be serialized/deserialized

Screenshots image

Exlll commented 8 months ago

If you annotate Background with @Configuration, do you get the output you want? If yes, that's the intended behavior. In that case, you also don't need to annotate the subclasses with either @Configuration or @Polymorphic.

alexdev03 commented 8 months ago

Thanks, it worked. Is it possibile to remove PolymorphicType section? background: type: integer Maybe you could check all possible classes and check if there is a match.

Exlll commented 8 months ago

You only need @PolymorphicTypes to define aliases. You don't have to use this annotation but then the Java class name is used.

If you want to use aliases without having to use @PolymorphicTypes that's not possible because there are several ambiguous cases where it is not clear which class should be selected. The most obvious example: If two sub classes have exactly the same name and structure but reside in different packages, then which of the two should I instantiate?

@Configuration
@Polymorphic
public static class Background {}

// in package com.example.bg1
public static class IntegerBackground extends Background {
    private int r, g, b;
}

// in package com.example.bg2
public static class IntegerBackground extends Background {
    private int r, g, b;
}

That's the reason that I either need to have the full Java class name or access to the class literal via the annotation.

alexdev03 commented 8 months ago

Why should you use subclasses if you have same parameters? Couldn't you just make an option in @Polymorphic to enable find class process? If it's enabled and there is a problem you just throw an exception. I just don't like having this section in the config, it could confuse the user.

Exlll commented 8 months ago

Well, as I wrote, this is just the most obvious example where there is ambiguity. Also, I'd prefer if we could discuss the details of your feature request in a separate issue (I'm not sure whether I'll be able to implement it anytime soon).