Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
solonovamax opened this issue Jul 11, 2024 · 3 comments

Comments

@solonovamax
Copy link

solonovamax commented Jul 11, 2024

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:

  • validating if the input is a valid element of a registry
  • validating if the input can be properly parsed
  • etc.

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants