shedaniel / cloth-config

Client-Sided API for Minecraft 1.14
Other
194 stars 71 forks source link

Autoconfig Requirement API discussion #217

Open MattSturgeon opened 1 year ago

MattSturgeon commented 1 year ago

I'm opening this Draft PR hoping to get some feedback and spark discussion on the user-facing annotation API, hopefully before I spend too much time implementing the backend for it...

Each commit takes a slightly different approach. I also have some other examples in some different branches (see below)...

1

The first commit takes a simplistic approach.

All requirement annotations must reference a handler method and optionally provide arguments to it. Additionally, DefaultRequirements handlers are provided to prevent users needing to re-implement common requirement handlers such as simple equality checks.

This can lead to somewhat clunky annotation declarations to do fairly simple requirements:

// Enabled when coolToogle is `true`
@ConfigEntry.Requirements.EnableIf(
        @Requirement(
                value = @Ref(cls = DefaultRequirements.class, value = DefaultRequirements.TRUE),
                refArgs = @Ref("coolToggle")
        )
)

2

The second commit attempts to improve the UX by introducing a concept of "simple" requirements. These are requirements that reference a Config Entry GUI instead of an actual handler method.

Simple requirements would be built into actual requirements during dependency setup. We would need to follow the requirement reference looking for a method handler, and then try again looking for a config entry field if no method exists.

// Same example as above
@ConfigEntry.Requirements.EnableIf(
        @Requirement(@Ref("coolToggle"))
)

3

The third commit removes the separate @Requirement annotation that was used by both @EnableIf and @DisplayIf and instead moves all of its parameters into each of those.

This is a bit of a painful compromise since it leads to having to maintain the same interface in two places, however it significantly improves the user experience by reducing boilerplate from dependency annotations.

In my opinion it's well worth it:

// Same again
@ConfigEntry.Requirements.EnableIf(@Ref("coolToggle"))

Other work

Other examples are in the original draft PR and my newer autoconfig dependencies branch

MattSturgeon commented 1 year ago

@shedaniel do you have any thoughts on this and #215 or should I assume this is heading in the right direction and continue?

KingContaria commented 1 year ago

would it be possible to add some way to define a different translation key for when the option is disabled by requirement?

something like this:

@ConfigEntry.Requirements.EnableIf(
        @Requirement(@Ref("coolToggle"))
)
@ConfigEntry.Requirements.CustomDisabledTranslation
public boolean coolOption;

with a language file like this:

"text.autoconfig.modid.option.coolOption.@CustomDisabledTranslation": "Disabled"

additionally cloth config could even just add its own translation string for "Disabled" (in grey i suppose) which could be used with @ConfigEntry.Requirements.DisabledTranslation

MattSturgeon commented 1 year ago

@KingContaria something like that would be possible, but it'd be better tackled as a separate issue because it'll also need changed on the cloth-config side, not just autoconfig.

KingContaria commented 1 year ago

that makes sense, i might make a fork for it once this pr is finished then