0

Basic Problem: I created a Game with 3 "States", each State is started by a method within the same Class. Each Method calls panel.removeAll(); on the static panel, that shows the game content and then i add the buttons, background etc again to the Panel. In the first "Loop" (by "loop" i mean the sequence of all 3 States where called once by each other, everything works just fine.

When State 3 calls State 1 again, so when the second Loop starts the following problem occurs: When i hit a JButton, the Action is performed TWICE. In the third loop its performed three times!

So whats my Problem in here? Is it so, that i add the buttons multiple times, by removeAll() and panel.add(JButton) again?

If so, whats the way around it? Should i create different Panels and set them invisible/visible one at a time? Wouldnt that cause heavy "Weight" on the processor by having all graphics loaded all the time?

Thanks for your Help!

EDIT: Here is an example:

package sscc;

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;

public class stuff extends JFrame implements ActionListener {

    static stuff frame = new stuff();
    static JPanel panel = new JPanel();


    public static void main(String[] args){
        //Setting up the Frame in MainMethod. Is that wrong?
        frame.setSize(800, 600);
        frame.setResizable(false);
        frame.setLocationRelativeTo ( null );
        frame.setUndecorated(true);
         frame.add(panel); //
         frame.setVisible(true);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.Phase1();
    }



    //Each Phase gets a Button to Enter the next one
    JButton Next1 = new JButton("Next1");
    JButton Next2 = new JButton("Next2");
    JButton Next3 = new JButton("Next3");



    public void Phase1(){
        panel.removeAll();
        panel.add(Next1);
        Next1.setBounds(200,200,200,200);
        Next1.setActionCommand("Phase1");
        Next1.addActionListener(this);
    }

    public void Phase2(){
        panel.removeAll();
        panel.add(Next2);
        Next2.setBounds(200,200,200,200);
        Next2.setActionCommand("Phase2");
        Next2.addActionListener(this);
    }

    public void Phase3(){
        panel.removeAll();
        panel.add(Next3);
        Next3.setBounds(200,200,200,200);
        Next3.setActionCommand("Phase3");
        Next3.addActionListener(this);
    }





    @Override
    public void actionPerformed(ActionEvent e) {
        if("Phase1".equals(e.getActionCommand())){System.out.println("Phase1Counter");Phase2();}
        if("Phase2".equals(e.getActionCommand())){System.out.println("Phase2Counter");Phase3();}
        if("Phase1".equals(e.getActionCommand())){System.out.println("Phase3Counter");Phase1();}
    }   
}
David Yee
  • 3,515
  • 25
  • 45
user3808217
  • 135
  • 3
  • 12
  • `removeAll` usually isn't preferable way. Try to use `setVisible(false)` on your button and reuse it later. – ferrerverck Jul 07 '14 at 10:21
  • Please create a [SSCCE](http://sscce.org/) that demonstrates your problem. – David Yee Jul 07 '14 at 10:25
  • 2
    This sounds like a mess (you may prove me wrong...), and like you should consider a http://docs.oracle.com/javase/tutorial/uiswing/layout/card.html , but ... the fact that actions are executed *twice* indicates that, for example, an `ActionListener` is added *twice* to a button... – Marco13 Jul 07 '14 at 10:43
  • updated it. Yes, i add the action listener each time.. would it be "fine" to just add all the listener stuff in a once executed method? Or is there another point to restart all the coding in a "better" way? – user3808217 Jul 07 '14 at 11:03
  • As Dawnkeeper and Macro13 said, you need to either remove the adding action listener portion from phase methods and keep commonly and will be call only once or you declare the buttons inside the current phase method. (ex. instance creation of phase1 button in phase1 method) – A Stranger Jul 07 '14 at 12:04

2 Answers2

1

With each phase you are adding the ActionListener to the buttons again. So it will be called multiple times. There is also a typo in your if clauses: you check for "Phase1" twice.

You should add the listeners to the buttons in an initial step (e.g. in the main() method) and not in the phase methods.

Dawnkeeper
  • 2,844
  • 1
  • 25
  • 41
1

The reason why your action listener is "firing" unexpectedly is due to the way you are handling the if statements in the actionPerformed method. Since you are using three separate if statements to check the action command all three conditions will be checked before finishing the method.

Here is what is happening: the first time you press the button you are executing the first if block but immediately changing the action command to "Phase2" in the Phase2 method. After completing the first if block, the second if block is reached and is determined to be true because you just changed the action command to "Phase2"! Therefore, the action listener appears to execute again when it really has not finished executing. To correct this behavior, you should be using if-else blocks as such:

@Override
public void actionPerformed(ActionEvent e) {
    if ("Phase1".equals(e.getActionCommand())) {
        System.out.println("Phase1Counter");
        Phase2();
    } else if ("Phase2".equals(e.getActionCommand())) {
        System.out.println("Phase2Counter");
        Phase3();
    } else if ("Phase1".equals(e.getActionCommand())) {
        System.out.println("Phase3Counter");
        Phase1();
    }
}

Some further suggestions:

  1. In terms of efficiency it is better to be re-using the JButton instead of creating multiple copies of them. I imagine these three buttons have very similar functions. You can create one JButton and modify the action it performs.

  2. You should avoid using setSize, setMinimumSize, setMaximumSize, and setPreferredSize. These methods are intended to be the responsibility of the Layout Manager. You may be interested in reading this question: Should I avoid the use of set(Preferred|Maximum|Minimum)Size methods in Java Swing? Also, you should familiarize yourself with the different layout managers as depicted in A Visual Guide to Layout Managers.

  3. You should not run your Swing GUI related code outside of the Event Dispatch Thread (EDT) because most Swing methods are not thread safe.

  4. You should follow Java Naming Conventions for better consistency in your code and for better clarity when sharing your code with other people.

Community
  • 1
  • 1
David Yee
  • 3,515
  • 25
  • 45