Innkeeper
Innkeeper

Reputation: 673

Why isn't this gui updating?

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

Upvotes: 3

Views: 338

Answers (1)

dic19
dic19

Reputation: 17971

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.

Upvotes: 2

Related Questions