isXander / YetAnotherConfigLib

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

Inputting text when using an `IStringController` with input validation is overly restrictive #195

Open solonovamax opened 2 months ago

solonovamax commented 2 months ago

When using an implementation of IStringController where input validation is used, the user is unable to enter an invalid value.

This is an issue for certain kinds of input validation, for example:

Here is a concrete example:

class ModifierExpressionController(
    private val option: Option<BeaconModifierExpression>,
) : IStringController<BeaconModifierExpression> {
    override fun isInputValid(input: String): Boolean {
        return try {
            val expression = BeaconModifierExpression(input)
            expression.evaluate(0.0) // try evaluating it to make sure it gets parsed (we parse it lazily)
            true
        } catch (e: ParseException) {
            false
        }
    }

    override fun option() = option

    override fun getString() = option.pendingValue().expressionString

    override fun setFromString(value: String) {
        option.requestSet(BeaconModifierExpression(value))
    }
}

The input is expected to be an equation which can use various functions such as min, cos, log, etc., as well as standard math operations such as +, -, *, etc. The issue is, if you wanted to input an expression such as 1 + 1. you will begin typing the expression by doing 1. This is a valid expression. But then you'll attempt to input + resulting in 1+. This is not a valid expression. However, the invalid expression is an intermediary step on the way to a valid one, as next 1 will be typed, resulting in 1+1.

The same thing might happen if you're, for example, required to input a valid registry entry, such as a valid mob id. the mob id m is not a valid mob id, however it is required when attempting to type out the full id of minecraft:creeper.

I think a better alternative rather than not allowing the character to be typed would be to simply make the box red and make it the config unable to be saved, indicating that the input is invalid.

isXander commented 2 months ago

isValidInput is intended to prevent typing invalid characters completely.

You should do this sort of validation in setFromString where the user finalises their expression (clicks away).

solonovamax commented 1 month ago

isValidInput is intended to prevent typing invalid characters completely.

You should do this sort of validation in setFromString where the user finalises their expression (clicks away).

ah, I see

is there any way to show that it has not been/will not be updated in the UI? ie. making the outline of the box red.

solonovamax commented 1 month ago

also, it'd be nice if smth was added to the docs to indicate this.