shooting more than once cause swing timer to repeat

Question

ok so I have a method to shoot a spell, and whenever I press space it does fire, however if I push space while it's still on the screen my spell's speed doubles and it starts to repeat itself without me pushing space again here's my code thanks :) oh and the WizardCells is just a 2D array of JLabels I know that's created and used properly so I didn't want to waste your guys' time with that code thanks :)

 public void shootSpell(){
        final BlueSpell b = new BlueSpell(GoodGuy.getx(), GoodGuy.gety() +1, BlueSpellWizard());
        int delay = 60;

        ActionListener timeListen = new ActionListener(){
            public void actionPerformed(ActionEvent e){


                    if(b.gety() != 19){
                        WizardCells[b.getx()][b.gety()].setIcon(null);
                        WizardCells[b.getx()][b.changey(b.gety()+1)].setIcon(b.getIcon());

                    }

                    else{
                        WizardCells[b.getx()][b.gety()].setIcon(null);
                        b.changex(GoodGuy.getx());
                        b.changey(GoodGuy.gety() +1);
                        timer.stop();

                    }
                }

        };


            timer = new Timer(delay, timeListen);
            if(timer.isRunning()){
                return;
            }

            else{
            timer.start();
            }


        }

Key handler:

public class WizardKeyHandeler extends WizardBattleGrid implements KeyListener {
GoodGuy Player = new GoodGuy(10, 0, GoodGuyWizardIcon());
BlueSpell GoodSpell = new BlueSpell(10, 1, BlueSpellWizard());
WizardPause pause = new WizardPause();






    @Override
    public void keyPressed(KeyEvent e) {
        int key = e.getKeyCode();

        if(key == KeyEvent.VK_W){

            if(Player.getx() != 0){

                Player.moveUp();
            }

        }

        else if(key == KeyEvent.VK_S){

            if(Player.getx() != 19){

                Player.moveDown();
            }
        }

        else if(key == KeyEvent.VK_D){

            if(Player.gety() != 9){

                Player.moveRight();
            }
        }

        else if(key == KeyEvent.VK_A){

            if(Player.gety() != 0){

                Player.moveLeft();
            }
        }

        else if(key == KeyEvent.VK_SPACE){





                }

        else if(key == KeyEvent.VK_ESCAPE){

            pause.createPause();
        }


    }   











    @Override
    public void keyReleased(KeyEvent e) {
        int key = e.getKeyCode();
        if(key == KeyEvent.VK_W){
                if(Player.getx() != 0){

                Player.remainAtPosition();
            }   
            }

        else if(key == KeyEvent.VK_S){

            if(Player.getx() != 19){

                Player.remainAtPosition();
            }
        }

        else if(key == KeyEvent.VK_D){

            if(Player.gety() != 9){

                Player.remainAtPosition();
            }
        }

        else if(key == KeyEvent.VK_A){

            if(Player.gety() != 0){

                Player.remainAtPosition();
            }
        }

        else if(key == KeyEvent.VK_SPACE){


                GoodSpell.shootSpell();


                }

        else if(key == KeyEvent.VK_ESCAPE){

                pause.createPause();
        }

    }



    @Override
    public void keyTyped(KeyEvent e) {
        int key = e.getKeyCode();
        if(key == KeyEvent.VK_W){
            if(Player.getx() != 0){

                Player.moveUp();

            }

            }

        else if(key == KeyEvent.VK_S){

            if(Player.getx() != 19){

                Player.moveDown();
            }
        }

        else if(key == KeyEvent.VK_D){

            if(Player.gety() != 9){

                Player.moveRight();
            }
        }

        else if(key == KeyEvent.VK_A){

            if(Player.gety() != 0){

                Player.moveLeft();
            }
        }

        else if(key == KeyEvent.VK_SPACE){





                }

        else if(key == KeyEvent.VK_ESCAPE){

            pause.createPause();
        }
    }

You should have a single timer which is responsible for updating the state of the game/model and scheduling the repaints, don't use multiple timers — MadProgrammer, Apr 02 '14 at 22:53
Don't use a KeyListener to control player movement. See [Motion Using the Keyboard](http://tips4java.wordpress.com/2013/06/09/motion-using-the-keyboard/) for more information and an alternative approach using `Key Bindings`. — camickr, Apr 02 '14 at 23:20
Use standard Java naming conventions. Variable names should NOT start with an upper case character!!! — camickr, Apr 02 '14 at 23:21
As @camickr said, use key bindings. I made a post about it [here](http://stackoverflow.com/questions/22741215/how-to-use-key-bindings-instead-of-key-listeners). — user1803551, Apr 03 '14 at 00:43

score 1 · Accepted Answer · edited May 23 '17 at 11:49

Two good points by both MadPrgrammer and camickr. I'm like to illustrate those points.

MapPrgogrammer: You should have a single timer which is responsible for updating the state of the game/model and scheduling the repaints, don't use multiple timers

What you should do is have a have a List of Spell or BlueSpell (whatever). What you want to do is iterate through the list in the Timer and your painting method. Something like

public class GamePanel extends JPanel {
    List<BlueSpell> spells;
    Timer timer = null;

    public GamePanel() {
        spells = new ArrayList<>();

        timer = new Timer(40, new ActionListener(){
            public void actionPerformed(ActionEvent e) {
                If (spells.size() > 0) {
                    Iterator spellIt = spells.iterator();
                    while (spellIt.hasNext()) {
                        BlueSpell spell = (BlueSpell)spellIt.next();
                        if (spell.collidesWithSomthing()) {
                            // do something, then remove from list
                            spellIt.remove();     
                        } else if (spell.isOffScreen()) {
                            spellIt.remove();  // remove the spell from list
                        } else {
                            spell.animate();  // animate for each tick of timer
                        }
                    }
                }
                repaint();  // just repaint once in each timer tick after all 
            }               // state id update.
        });
        timer.start(); 
    }

    protected void paintComponent(Graphics g) {
        super.paintComponent(g);
        If (spells.size() > 0) {
             for (BlueSpell spell: spells) {
                  spell.drawSpell(g);
             }
        }
    }
}

You can see the basic idea of the spells list in the Timer callback. It iterates through the timer and if the list has BlueSpells, then it will continue to check if the spells collides with something (if you need to), it checks if the spells is off the screen. With either, you'll want to remove it from the list. (Don't be tempted to use a for each loop. In this case you should use an iterator). If it is still on screen and hasn't hit anything, then the spell animates (maybe move it x position or something) then finally for each tick, the panel is repainted. Notice the if checks in both the paintComponent and Timer. There's no need to paint or iterate through the list if it is empty.

You'll want to maintain all the game's state in the single timer, not just the spell. If the spells are the only thing that animates, you may want to add a else { timer.stop() } in the timer, if the list is empty.

camickr: Don't use a KeyListener to control player movement. See Motion Using the Keyboard for more information and an alternative approach using Key Bindings

What you can do is when the space is pressed, add a new BlueSpell to the list (and start the timer [only if necessary] ). Something like

public GamePanel() {
    ...
    InputMap inputMap = getInputMap(JComponent.WHEN_IN_FOCUSED_WINDOW);
    inputMap.put(KeyStroke.getKeyStroke("SPACE"), "shootSpell");
    getActionMap().put("shootSpell", new AbstractAction() {
        @Override
        public void actionPerformed(ActionEvent e) {
            spells.add(new BlueSpell(...));
        }
    });
}

Once the spell is added to the list, the spell will begin to animate until it collides with something or is off the screen.

You can see a working example of both points in this answer.

enter image description here

Also you can see a bunch of other "animating multiple objects" answers here and here and here and here and here and here.

UPDATE

"the only question is in my repaint method do I just call the ImageIcon I created or do I have to create a new one?"

public class BlueSpell {
    BufferedImage bi;
    int x, y;
    int width = someWidth;
    int height = someHeight;
    JPanel panel;

    public BlueSpell(BufferedImage bi, int x, int y, panel) {
        this.bi = bi;
        this.x = x;
        this.y = y;
        this.panel = panel;
    }

    public void drawSpell(Graphics g) {
        g.drawImage(bi, x, y, width, height, panel);
    }

    public void animate() {
        x += 5;
    }
}

You should create just the one BufferedImage in the GamePanel and when you create a new BlueSpell in the in the key binding, just pass the image to it, along with the other arguments.

Thank you for this! this is my first actual big java project and I had no idea how keybindings worked compared to keylisteners and how to really do timers. the only question is in my repaint method do I just call the ImageIcon I created or do I have to create a new one? — user3471511, Apr 03 '14 at 01:08
Here's what I would do. Just use one `BufferedImage`, and not `ImageIcon`. For each new `BlueSpell`, pass the same `BufferedImage` to it. In the `paintSpell` method of the `BlueSpell` use that `BufferrImage` to `drawImage(spellImage, x, y, ...)` — Paul Samsotha, Apr 03 '14 at 01:18
Alright thank you i'll give that a try and when creating the main timer using a swing timer is fine right? or would I want to use the timer in the util package? — user3471511, Apr 03 '14 at 01:19
Yeah swing timer is what you want, not java.util.Timer. Also see my update, maybe that will help. — Paul Samsotha, Apr 03 '14 at 01:25
ok ya I just saw it, and if you don't mind one more question how does x+= 5 move it? I saw that on all the tutorials I watched when moving a character they did a variable += anumber and I don't understand how that works if you can try to explain it to me thanks :) — user3471511, Apr 03 '14 at 01:28
Basically `x += 5` mean `x = x + 5`. So you are increasing `x` when you call animate. You can see in the timer, that I call `spell.animate()`, which makes `x` increase by 5. When it repaints, the change will be noticed, as you can see when repaint is called, it call paintComponent internally, which loops through the spells again and the new `x` values will be recognized, appearing to be animated — Paul Samsotha, Apr 03 '14 at 01:31

shooting more than once cause swing timer to repeat

1 Answers1