3

I have a Gui and a Game class, and I'm unable to update the gui from the game. I'm not using threads, but I've seen it update before, so that isn't the problem. The game logic is really simple, there is no need for threads. No matter how furiously I call repaint() and revalidate(), it doesn't work now, no matter where I put it.

class Gui {

    //...

    public Gui(Game game) {
        this.game = game;
        initialize();       
    }

    private void initialize() {
        //...
        okButton.addActionListener(new ActionListener() {

            @Override
            public void actionPerformed(ActionEvent arg0) {
                okAction(textField.getText());

                textField.setVisible(false);
                okButton.setVisible(false);
                textField.setText("");
            }           
        });
    }

    private void okAction(String input) {
        game.receiveInput(input);
    }

    public void output(String msg) {        
        textArea.append(msg + "\n");        
    }

    public void getInput() {
        textField.setVisible(true);
        okButton.setVisible(true);
        textField.setText("");      
    }
}

Also I want to be able to pass a String back to the game instance. I thought I'd call getInput() from the game, which will show a JTextField to type in, and a JButton to submit. In the actionPerformed() method I would just get the text entered, and call a method back in the game class. I don't know if this would work, since the gui is not updating, and I never had the input field and button appear. Is this right?

This would be the method which the gui "calls back":

class Game {

    //...

    public void receiveInput(String input) {
        int n = Integer.parseInt(input);
        if ( validInput(input, actualDecision.choices.size()) ) {
            parser.setAction(actualDecision.choices.get(n-1).action);
        }
    }
}

From the game class, I just want to call gui.output() and gui.getInput() a few times.

Where is my problem? Why isn't it updating, nor freezing? If I use the debugger, the both output() and getInput() is executed, but nothing happens...

EDIT:

Ok I see a problem myself, with the getting input part... Since it returns quickly, it can never receive an input. But that doesn't explain why aren't the input field and the button, or any text is showing up

EDIT 2:

Oh god, sorry, I really don't know how to make it shorter, but you only ever need to look at the Game and the Gui, others are just there to compile.

The code: https://gist.github.com/anonymous/53bad714592792316b4d

An xml to test against: https://gist.github.com/anonymous/30b56facb78fe6ecd482

Innkeeper
  • 663
  • 1
  • 10
  • 22
  • 1
    You should do GUI operations in the EDT thread. – Ross Drew Dec 10 '13 at 10:53
  • @RossDrew Assuming all logic can be done within the "tick" (how ever long that is; 1/60th of a second is ideal) there is no need for threads – Richard Tingle Dec 10 '13 at 10:54
  • Could you post the gui.output() as well. Preferably as a complete (small) program – Richard Tingle Dec 10 '13 at 10:56
  • @RichardTingle You mean a call? The game has a gui instance, and it just calls `gui.output("something");` – Innkeeper Dec 10 '13 at 10:58
  • 2
    Suppose, for unknowing reason your `receiveInput(String input)` function of the `Game` class never gets invoked. Now show us the invoking code: wherever you have put them. Make a [SSCCE](http://sscce.org) – Sage Dec 10 '13 at 10:59
  • Basically what we want is to copy some code (preferably as short as possible) paste it onto our computer, run it and go "O, you're right, thats strange", from there we can start trying to find solutions – Richard Tingle Dec 10 '13 at 11:00
  • Ok, i'll make an edit. – Innkeeper Dec 10 '13 at 11:01
  • @Richard Tingle It's not just any thread, it's he EDT thread. GUI operations should be passed into here, it's standard practice so that multiple threads (the EDT and yours) don't modify GUIs. – Ross Drew Dec 10 '13 at 11:09
  • 2
    @RossDrew is in fact correct. Richard i have written an answer reflecting the EDT mechanism and `SwingUtilities` class in [this answer with linked documentation](http://stackoverflow.com/questions/2899682/unresponsive-threading-involving-swing-and-awt-eventqueue/20359861#20359861). You can have a look if you want. – Sage Dec 10 '13 at 11:13

1 Answers1

2

Honestly I have just taken a look to Gui class code and don't know why it doesn't update properly when it interacts with Game class. BUT I have several remarks on your code and I hope these can lead you in the right way by making this class simpler and then you can focus on the interaction with Game class.

Menu bar

You're adding the JMenuBar to a JPanel instead of setting it to the JFrame:

panel.add(menuBar, gbc);
//frame.setJMenuBar(menuBar); Use this instead

As JFrame is a top level container prepared to handle a menu bar you should consider make the suggested change.

Saving validate() call

As the JFrame is initialized at the start of initialize() method and the JPanel is added after making the frame visible then you have to call frame.revalidate() method to revalidate the components hierarchy. If you just initialize the panel before make the frame visible then you don't need to call revalidate() method. Take a look to this answer for further details.

Missing pack() call

There's no call to frame.pack() method to lay out the frame's subcomponents. Take a look to Window.pack().

Missing GridBagConstraints when adding okButton

There's no GridBagConstraints as argument when adding okButton to panel:

panel.add(okButton);
//panel.add(okButton, gbc); This is the correct way.

Use of setSize()

In this line:

frame.setSize(800, 600);

We should avoid using set(Preferred | Minimum | Maximum)Size() because of reasons discussed in this topic: Should I avoid the use of set(Preferred|Maximum|Minimum)Size methods in Java Swing?

Use of GridBagLayout

This is just a suggestion there's nothing wrong on how you are using GridBagLayout. As probably you have noted this layout manager is a little hard to use (and I really don't like it by the way :) You can use a Nested Layout approach to make the components layout easier and your code more readable. Maybe this approach is good enough:

  • Set the JMenuBar to the JFrame. It's one less component to lay out ;)
  • Add the scroll pane with the text area directly to the frame's content pane using BorderLayout constraints: frame.getContentPane().add(scrollPane, BorderLayout.CENTER);
  • Create a new JPanel to hold the text field and the button used to ask for user's input and add it to the frame's content pane.

Translated to code:

JPanel usersInput = new JPanel(new FlowLayout(FlowLayout.CENTER));
usersInput.add(textField);
usersInput.add(okButton);

frame = new JFrame();
frame.setJMenuBar(menuBar);
frame.getContentPane().add(scrollPane, BorderLayout.CENTER);
frame.getContentPane().add(usersInput, BorderLayout.SOUTH);
frame.setTitle("Choose your own adventure");
frame.pack();
frame.setLocationRelativeTo(null);
frame.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE); // It's better practice DISPOSE_ON_CLOSE ;)
frame.setVisible(true); 

enter image description here


Update

Well I'm curious and I really want to test your game (quite nice work by the way :) The thing is I have found at least two problems in the logic:

Use of == to compare strings

In Parser class there are several string comparisons using == but this is not the proper way to compare strings. We must use .equals() method to compare strings equality. Take a look to this topic: How do I compare strings in Java?

Game.processStory() has an endless loop

This method has an endless loop here:

while ( !end() ) { // this condition never is false so the loop is infinite
    ...
}

Looking closer to Game.end() method I have found an incorrect string comparison:

private boolean end() {
    return ( parser.getAction() == "end" );
    //It should be: return parser.getAction().equals("end");
}

But fixing that doesn't solve the problem either: it turns parser.getAction() always returns "d1" and consequently it will never be equal to "end".

Having said this as Game.play() is executed in the Event Dispatching Thread (a.k.a. EDT) triggered by newGameItem menu item and Game.processStory() has this endless loop, then the EDT is getting blocked and Gui is never updated.

In this case I would suggest you take a look to Concurrency in Swing trail to learn about how to avoid blocking the EDT.

Community
  • 1
  • 1
dic19
  • 17,821
  • 6
  • 40
  • 69
  • Thank you for the insights, I really appreciate it! I've learned quite a lot! – Innkeeper Dec 11 '13 at 12:26
  • @Innkeeper you're welcome :) Please take a look to my updated answer. I think I've found the problem now :) – dic19 Dec 11 '13 at 15:32
  • Thank you again, I've corrected the spotted mistakes with string comparisons, as you might have guessed, I'm not that comfortable with Java... The `parser.getAction()` only returns d1 because the action would be set by the user, since it's his decision. The problem is, the extra button and textfield for the input never gets shown when `gui.getInput()` is called... Strange thing is, everything is running on the EDT now, and still neither `gui.output()` nor `gui.getInput()` changes the gui. – Innkeeper Dec 11 '13 at 23:10
  • If I stop the infinite loop by commenting out `gui.getInput()`, and simulating what would happen (calling `receiveInput("1")` for example), It continues as intended, finds out it should end, and when no more work needs to be done, vomits out everything in the text area. – Innkeeper Dec 11 '13 at 23:14
  • @Inkeeper you are in the right way. The issue actually is everything is running in the EDT and this endless loop blocks it and doesn't allow updating the gui. Does it make sense? You have to find a way to do the interaction between Game and Gui without blocking the EDT. – dic19 Dec 13 '13 at 02:29