1
votes

Goal

I'm currently building (to practice with java) a basic command line multiplayer turn-based game. In this game, each player has 5 seconds to make his move. When he makes his move (or when the timer ends) the other player starts his turn, etc etc. The server sends a TimerEnded message every time the timer ends. My current goal is to achieve flawless input reading that could be interrupted when a TimerEnded message arrives to the client.

Design

To achieve this I created a singleton called InputManager. This class handles all the input reading stuff. I created a method called ask which takes a callback as parameter. In this method I create a new thread and inside it I wait for an input with Scanner.hasNextInt. This class has also the method closeInput which sends an Interrupt message to the thread described above. Here's the current implementation of the class:

class InputManager{
    private Thread thread;
    private InputManager(){}
    private static InputManager instance;
    private static InputManager getInstance(){
        if(instance == null){
            instance = new InputManager();
        }
        return instance;
    }

    /**
     * Ask user to type a number.
     * @param onSelected When the user has made his choice, this callback will be executed
     */
    public static void ask( Consumer<Integer> onSelected){
        getInstance().thread = new Thread(() -> {
            System.out.println("Type a number:");

            Scanner sc = new Scanner(System.in);
            int selection = -1;
            while (selection == -1) {
                if(Thread.currentThread().isInterrupted()){
                    return;
                }
                if(sc.hasNextInt()){
                    selection = sc.nextInt();
                    onSelected.accept(selection);
                } else {
                    sc.next();
                    selection = -1;
                }
            }
        });
        getInstance().thread.start();
    }

    /**
     * Reset input stream (?)
     */
    public static void closeInput(){
        try {
            getInstance().thread.interrupt();
        } catch(NullPointerException e){
            // do nothing
        }
    }
}

Problem

This code is extremely unreliable. I'll show you what I mean in just a moment. I made a toy class called Client and in the main I simulated the TimerEnd message income with a timer.

class Client {
    /**
     * Ask user to type a number and send it to the server
     */
    void makeRequest(){
        InputManager.closeInput();
        InputManager.ask((selected) -> {
            System.out.println("Sent message: " + selected);
        });
    }

    public static void main(String[] args) {
        Client client = new Client();

        client.makeRequest();

        // Simulate Server messages
        Timer timer = new Timer();
        timer.scheduleAtFixedRate(new TimerTask() {
            @Override
            public void run() {
                System.out.println("Message received");
                client.makeRequest();
            }
        }, 5000, 5000);
    }
}

Here's how it works in action:

Type a number:
2
Sent message: 2
Message received
Type a number:
3
Sent message: 3
Message received
Type a number:     // Here I don't type anything
Message received
Type a number:
Message received
Type a number:
Message received
Type a number:     // Here I can send multiple messages on the same "turn"
1
Sent message: 1
2
Message received

Non-educated guess

Currently, I guess that Scanner remains waiting for input and so the if(isInterrupted) statement is not hit until an input is given. If so, how can I avoid this behaviour?

I understand that this question is extremely (and maybe unnecessarily) long, and since you read it let me thank you for taking your time.

Minimal, Complete and Verifiable code

package com.company;

import java.util.*;
import java.util.function.Consumer;



class InputManager{
    private Thread thread;
    private InputManager(){}
    private static InputManager instance;
    private static InputManager getInstance(){
        if(instance == null){
            instance = new InputManager();
        }
        return instance;
    }

    /**
     * Ask user to type a number.
     * @param onSelected When the user has made his choice, this callback will be executed
     */
    public static void ask( Consumer<Integer> onSelected){
        getInstance().thread = new Thread(() -> {
            System.out.println("Type a number:");

            Scanner sc = new Scanner(System.in);
            int selection = -1;
            while (selection == -1) {
                if(Thread.currentThread().isInterrupted()){
                    return;
                }
                if(sc.hasNextInt()){
                    selection = sc.nextInt();
                    onSelected.accept(selection);
                } else {
                    sc.next();
                    selection = -1;
                }
            }
        });
        getInstance().thread.start();
    }

    /**
     * Reset input stream (?)
     */
    public static void closeInput(){
        try {
            getInstance().thread.interrupt();
        } catch(NullPointerException e){
            // do nothing
        }
    }
}

class Client {
    /**
     * Ask user to type a number and send it to the server
     */
    void makeRequest(){
        InputManager.closeInput();
        InputManager.ask((selected) -> {
            System.out.println("Sent message: " + selected);
        });
    }

    public static void main(String[] args) {
        Client client = new Client();

        client.makeRequest();
        Timer timer = new Timer();
        timer.scheduleAtFixedRate(new TimerTask() {
            @Override
            public void run() {
                System.out.println("Message received: thread interrupted");
                client.makeRequest();
            }
        }, 5000, 5000);
    }
}
2
You either have to use non-blocking IO (for the keyboard) or use multiple threads, with one thread waiting on keyboard input. - markspace
Isn't the second option literally what I am doing? - Michele Lambertucci
I think you're trying, but your code is so convoluted I'm not sure. You're starting a thread, but I'm not sure the Consumer is working like you expect. - markspace
I would recommend not doing this as a command-line program at all, but use Swing/JavaFX instead, so you don't interrupt a partial user entry with output. - Andreas
A couple of concerns: getInstance is not thread safe and it should be. Consumer is not thread safe and it should be. You're hiding a NullPointerException in closeInput and you never should do that. I think over-use of lambdas is hiding problems in your code. Try re-writing it with out the lamdas and pay close attention to thread safe operations. Then we might have something we can actually comment on. - markspace

2 Answers

1
votes

As I see it, you can use 3 types of threads:

  1. The main thread switches between users, announces players to play, checks the winning condition and starts the timer at each turn.
  2. A second thread reads constantly the user input. After reading user input, it notifies the main thread.
  3. Finally a thread waits for 5 seconds and then notifies the main thread.

So I will use 2 Producers and 1 Consumer as follows:

  1. A Producer which "produces" the scanned user input (it provides it to the Consumer).
  2. A Producer which "produces" time out events (which notify the Consumer).
  3. A Consumer which switches between players and starts the producers.

All this, so that you don't have to mess around with interrupting any running thread and there is no need to check if the Scanner is ready

import java.util.Scanner;

public class Main {
    private static final Scanner SCAN = new Scanner(System.in);

    //This is the Scanner's input Producer:
    private static class UserInputProducer extends Thread {
        private final UserInputConsumer uInConsumer;

        public UserInputProducer(final UserInputConsumer uInConsumer) {
            this.uInConsumer = uInConsumer;
        }

        @Override
        public void run() {
            while (true) {
                final int input = SCAN.nextInt();
                SCAN.nextLine(); //Ignore the new line character.
                uInConsumer.userInput(input); //Fire user input event (for the current user).
            }
        }
    }

    //This is the time out event Producer:
    private static class TimeOutEventProducer {
        private final UserInputConsumer uInConsumer;

        private int validReportId = Integer.MIN_VALUE; //IDs starting from Integer.MIN_VALUE and
        //going step by step to Integer.MAX_VALUE, which means about 4 billion resets can be done
        //to this Producer before an unhandled overflow occurs.

        public TimeOutEventProducer(final UserInputConsumer uInConsumer) {
            this.uInConsumer = uInConsumer;
        }

        public synchronized void reset() {
            new TimerOnce(this, ++validReportId).start(); //Start a new TimerOnce. Could be javax.swing.Timer with "setRepeats(false)".
        }

        /*sleepDone(...) is called by ALL TimerOnce objects... So we need an up-to-date id (the
        reportId) to verify that the LAST one TimerOnce finished, rather than any other.*/
        public synchronized void sleepDone(final int reportId) {
            if (reportId == validReportId) //Only the last one timeout is valid...
                uInConsumer.timedOut(); //Fire time out event (for the current user).
        }
    }

    //This is just a "Timer" object which blocks for 5 seconds:
    private static class TimerOnce extends Thread {
        private final TimeOutEventProducer timeout;
        private final int reportId;

        public TimerOnce(final TimeOutEventProducer timeout,
                         final int reportId) {
            this.timeout = timeout;
            this.reportId = reportId;
        }

        @Override
        public void run() {
            try { Thread.sleep(5000); } catch (final InterruptedException ie) {} //Wait.
            timeout.sleepDone(reportId); //Report that the time elapsed...
        }
    }

    //This is the Consumer:
    private static class UserInputConsumer {
        private final String[] names;
        private int input;
        private boolean timedOut, hasInput;

        public UserInputConsumer(final String[] names) {
            this.names = names;
        }

        public synchronized int play() {
            new UserInputProducer(this).start(); //Start scanning any user's input...
            final TimeOutEventProducer timeout = new TimeOutEventProducer(this);
            int i = -1;
            do {
                i = (i + 1) % names.length;
                hasInput = false;
                timedOut = false;
                timeout.reset(); //Start the input wait timer...
                System.out.print("User " + names[i] + " enter a number: "); //Clarify who's player is the turn.
                while (!hasInput && !timedOut)
                    try { wait(); } catch (final InterruptedException ie) {} //Wait for user input or timeout.

                //Interpret notification event (either user input, either timeout):
                if (timedOut)
                    System.out.println("Sorry, out of time.");
                else if (!hasInput)
                    throw new UnsupportedOperationException("Probably messed with the flags in the while-condition.");
            }
            while (input != 5); //Here you test the win/loss condition.
            //Lets say, for example, the user that enters number '5' wins...

            return i; //Return the winner's index.
        }

        public synchronized void timedOut() {
            timedOut = true;
            notify();
        }

        public synchronized void userInput(final int input) {
            this.input = input;
            hasInput = true;
            notify();
        }
    }

    public static void main(final String[] args) {
        System.out.print("Enter number of players: ");
        final int numPlayers = SCAN.nextInt();
        SCAN.nextLine(); //Ignore the new line character.
        final String[] names = new String[numPlayers];
        for (int i=0; i<names.length; ++i) {
            System.out.print("User " + (i+1) + " enter your name: ");
            names[i] = SCAN.nextLine();
        }

        //Start the consumer (which in turn starts the producers) and start the main logic:
        System.out.println(names[new UserInputConsumer(names).play()] + " wins!");
    }
}

Note, the program never terminates because the Scanning is infinite. But you may alter this behavior by messing with the while (true) condition of the UserInputProducer.

0
votes

Alright, I worked out a solution. As I thought, the problem is that the while loop was (of course) blocking in Scanner.hasNext. To avoid blocking, I used a BufferedReader, which has this handy function, ready, which returns true whenever a new line is input in System.in.

Basically, I changed the InputManager.ask method to:

void ask(Consumer<Integer> onSelected){
    getInstance().thread = new Thread(() -> {
        System.out.println("Type a number:");

        BufferedReader reader = new BufferedReader(new InputStreamReader(System.in));

        Scanner sc = new Scanner(reader);
        int selection = -1;
        try {
            while (selection == -1) {
                //While there is no input to be processed
                while (!reader.ready()) {
                    //This lets throw an InterruptedException
                    Thread.sleep(100);
                }
                if (sc.hasNextInt()) {
                    selection = sc.nextInt();
                    onSelected.accept(selection);
                } else {
                    sc.next();
                    selection = -1;
                }
            }
        } catch (IOException | InterruptedException e) {
            // do nothing: function ends
        }
    });
    getInstance().thread.start();
}

I also added this (extremely ugly) piece of code to consume any input before resetting, to prevent any previous line to be detected as typed now (basically flush the last line). (If anyone has a suggestion on how this can be done in a more elegant way, I'm more than happy to hear your thoughs)

public static void closeInput(){
    try {
        BufferedReader tmp = new BufferedReader(new InputStreamReader(System.in));
        if(tmp.ready()){
            tmp.readLine();
        }
        getInstance().thread.interrupt();
    } catch(NullPointerException e){
        // do nothing
    } catch (IOException e) {
        e.printStackTrace();
    }
}