TextField Validation for Teleport Feature in Game

Question

I've implemented a teleport feature for my game and I would like some feedback on the way that I validate the user input. I am using the libGDX library.

When the player clicks the Teleport Menu button, a menu will pop up that has two TextFields pre-populated with zeros. The user can click the TextField to begin input, and then can input any string that they want to. Once the player clicks Teleport, the current Strings of the TextFields are retrieved and handed off to a method that turns them into valid integers.

Here is a screenshot of the Teleport Menu:

Here is the TeleportMenu class, it is not too large:

public class TeleportMenu extends BZMenuTable {

    private final NinePatch menuBackground;
    private final MainGame game;
    private final float stageWidth;
    private final float stageHeight;

    private TextField longitudeTextField;
    private TextField latitudeTextField;

    public TeleportMenu(Skin skin, Stage hudStage, NinePatch menuBackground, MainGame game, float stageWidth, float stageHeight) {
        super(skin, hudStage);
        this.menuBackground = menuBackground;
        this.game = game;
        this.stageWidth = stageWidth;
        this.stageHeight = stageHeight;
    }

    @Override
    public void build() {

        TextButton teleportButton = new TextButton("Teleport", skin);
        teleportButton.addListener(new ClickListener() {
            @Override
            public void clicked(InputEvent event, float x, float y) {
                TeleportMenu.this.activateTeleport();
                TeleportMenu.this.close();
            }
        });

        this.menuTable.setBackground(new NinePatchDrawable(this.menuBackground));
        this.menuTable.setPosition(this.stageWidth/10, this.stageHeight/8);
        this.menuTable.setWidth((float)(this.stageWidth * 0.8));
        this.menuTable.setHeight((float)(this.stageHeight * 0.6));

        this.menuTable.add("Teleport");
        this.menuTable.row();

        this.menuTable.add("Latitude 0 - 50");
        this.menuTable.row();
        this.latitudeTextField = new TextField("0", skin);
        this.menuTable.add(latitudeTextField);
        this.menuTable.row();

        this.menuTable.add("Longitude 0 - 50");
        this.menuTable.row();
        this.longitudeTextField = new TextField("0", skin);
        this.menuTable.add(longitudeTextField);
        this.menuTable.row();

        this.menuTable.add(teleportButton).pad(10).width(this.stageWidth/5).height(this.stageHeight/12).fill();
    }

    private void activateTeleport() {
        int xPos = this.processTextField(this.latitudeTextField.getText());
        int yPos = this.processTextField(this.longitudeTextField.getText());
        this.game.playerActivatedTeleport(xPos, yPos);
    }

    private int processTextField(String string) {
        Integer position;
        try {
            position = Integer.parseInt(string);
        } catch (Exception e) {
            System.out.println("Invalid input");
            position = 0;
        }
        if (position < 0) {
            position = 0;
        }
        if (position >= World.WORLD_WIDTH) {
            position = World.WORLD_WIDTH - 1; //because it is the index of an array
        }
        return position;
    }
}

Specifically I am curious about the processTextField method, but I am always interested in any kind of feedback.

Answer 1

Table organization

this.menuTable.setPosition(this.stageWidth/10, this.stageHeight/8);
this.menuTable.setWidth((float)(this.stageWidth * 0.8));
this.menuTable.setHeight((float)(this.stageHeight * 0.6));

I have mentioned before that I do not like this. This is not the "correct" way to setup a table on your screen. When you solve this issue, you will no longer need to pass around stageWidth and stageHeight.

Guess what the magic number is today?

this.menuTable.add("Latitude 0 - 50");
this.menuTable.add("Longitude 0 - 50");

Extract 50 to a static final int constant and use string concatenation to set the string:

this.menuTable.add("Latitude 0 - " + MAX);

processTextField

The first thing that comes to my mind is: If a user enters invalid input, should they then really be teleported?

If I enter the coordinate -42, 1not2a3number4, I would expect an error message, not being teleported to (0, 0). Also remember that users will not be able to see System.out.println output.

That being said, I'd also make some other changes:

• Use int instead of Integer for position variable
• Instead of if (position < 0) do position = Math.max(0, position);
• Instead of if (position >= MAX) do position = Math.min(MAX, position);

The both Math.max and Math.min uses can be put together into:

position = Math.min(MAX, Math.max(0, position));

However, there is a LibGDX method in MathUtils that already does this:

position = MathUtils.clamp(position, 0, MAX);

Answer 2

Don't treat invalid input by replacing it to a default value, unless it's meaningful to do so. In case of teleportation, I would not consider it valid to be teleported to 0,0 when I'm entering invalid positions.

I would treat an invalid value by returning a message next to the textfield saying that the value is not in the range or format expected. Remember to provide an example of an expected value, this really helps user providing good information. You should check both input before returning any error message, I think users prefers to tackle more than one error, that to fix one and be stop because it did not verify the other one too.

After a bit of research, you could use a TextFieldFilter, to filter out invalid input. One advantage of this approach is it make less prone to have invalid input, what you need to make sure is that it's not just a client-side validation that could be potentially de-activated by the user. If it so, it would not change the code since you would need to valid it server-side too.

---

try {
        position = Integer.parseInt(string);
    } catch (Exception e) {
        System.out.println("Invalid input");
        position = 0;
    }

I find it a bad habit to output to Sysout when you manage an exception. Not all environments have a console ready to output, use a logger instead. It can configure by each of your environment and it's easier when you want to debug something. The second thing here is that your catching Exception blindly and not what you're looking for. Integer.parseint() can throw a NumberFormatException if the string does not contain a parsable integer, so catch that instead. Now it's clear that you only what to validate input.