MrCrayfish / Configured

The easiest way to change the configuration of mods!
https://www.curseforge.com/minecraft/mc-mods/configured
Other
64 stars 47 forks source link

[Request] Use toString() method when displaying Enum configuration options. #78

Open EtiTheSpirit opened 1 year ago

EtiTheSpirit commented 1 year ago

Currently, Enum configurations use a technique that (seems to) just convert the constant name into a human-friendly(ish) display name by changing the formatting.

While this does work, I have found it less than ideal when striking a balance between UX and code cleanliness. Take one of my enums, for example, which has three constants ALLOW_SPREADING, NO_SPREADING, NO_EXISTING - these names work but they aren't what I would choose to show to someone using my mod, especially if they are not very confident in anything technical.

To attempt to remedy this, I added this block:

@Override
public String toString() {
    return switch (this) {
        case NO_EXISTING -> "Fully Disabled";
        case NO_SPREADING -> "Exist Without Spreading";
        case ALLOW_SPREADING -> "Fully Enabled";
    };
}

But unfortunately this does not affect the display of options within Configured. I would argue that this should be allowed, because enum has its own dedicated name() method that is used to return the name of the constant itself, thus overriding toString() has no negative side effects.

MrCrayfish commented 1 year ago

I like this idea but if the values are going to be localised, it's better to use Component and use lang files. I am not sure if there is a vanilla interface that could be implemented onto enums, best I could find was possibly using the Message interface, but that still returns a String. Best I can think of is to just call Component#getString, at least it'll be localised.

EtiTheSpirit commented 1 year ago

If you really wanted to cheese it, Enums can technically implement Function<T, R> which you could use to receive a component from.

public enum MyEnumOption implements Function<MyEnumOption , Component> {
    // stuff here
    ;

    public Component apply(MyEnumOption option) { ... }
}

It's hacky as all hell but it would work.

MrCrayfish commented 1 year ago

Only problem with that is type erasure. I can't safely assume the types will be <EnumClass, Component>.

EtiTheSpirit commented 1 year ago

Weighing in all the options, I don't see any other feasible alternatives other than Message or Function<T, R>

Message should work because not only is it an interface people are unlikely to use on an Enum normally, but also the chances of another mod using it (especially when toString() is available) are very slim. I doubt there will be any cases where collisions occur, though of course it's still possible. If worse comes to worst, constructing via Component.translatable won't have a fit if it's not a valid localization key.

Still, if the worry of collisions is too high, Function could be used with runtime type checking (read: wrapping the call in a try/catch) without too much of a fuss, mainly considering the fact that it's not exactly a performance-sensitive area of the code. It works, but it's not exactly clean. Regardless, a lazy solution is a lazy solution, so ymmv. I just don't see any other ways to remain compatible with vanilla+forge without having a compromise like that.

Speiger commented 1 year ago

What could be done is using a Remapper in the API. With multiple ways to register them.

public void onCallback(INameRegistry registry) {
   registry.registerName(MyEnum.class, T -> Component.literal(T.toString()); //Class, Function
}

Or the option to just provide a class

//INameProvider extends Function<T, Component>

@EnumProvider
public class MyEnumNameProvider implements INameProvider<MyEnum>
{
    @Override
    public Component apply(MyEnum enum) {
       return Component.literal(enum.toString());
    }

   @Override
   public Class<MyEnum> getProviderClass() {
       return MyEnum.class;
   }
}

In Forge you can just search for anything that has a Annotation applied to itself. Providing multiple ways to achieve the same xD

MrCrayfish commented 1 year ago

This works if the mod is added as a dependency, however Configured isn't designed to be dependency (unless you are using the Simple Config API). I think a simple solution would be to reflect on the enum and search for a method.

For example, search for Component getConfiguredTranslation() in the enum. This means the mods don't need to know about Configured in order to provide translations.

public enum Foo
{
    BAR(Component.translation("modid.my.translation.key"));

    final Component translation;

    Foo(Component translation) {
        this.translation = translation;
    }

    // Reflect to find this method
    @SuppressWarnings("unused")
    public Component getConfiguredTranslation() {
        return this.translation;
    }
}
Speiger commented 1 year ago

You could make it even simpler like enchantment description does it.

Just make the mod provide a translation key. "configured.enum.classpath.enumname.entry" and then provide the translation.

You yourself just can check if the translation key exist. If not just use a literal otherwise use the key.

Edit: This would allow for example also compat mods to exist that just add said enums if the modders themselves don't want to add it.

Or if you want you can add them too without having a direct dependency on the mod.

EtiTheSpirit commented 1 year ago

I think the reflective method is better because then I can return a Component, where I am free to do any additional text formatting that I may need.

Speiger commented 1 year ago

@EtiTheSpirit you do know that translation files still allow text formatting?

EtiTheSpirit commented 1 year ago

Yes, but it is limited. For example, I can't use hex color codes in a string, but I can if I supply the actual Component instance.

Speiger commented 1 year ago

@EtiTheSpirit ok yes, but that enforces that every mod has to provide it themselves. The solution I suggested would allow other resource packs to include such information too, Reducing the work on the dev itself and allowing the community to produce such information too. Yes with more limited Style information, but it would allow for much more.

What could be done is have a "API" that people have to implement if they want to have extra features and if that isn't provided allow the Resourcepacks to be the default otherwise.