What is wrong with this?

[Alaric Mustoe]

package GameTest;

import GameTest.Draw.Image;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.io.IOException;

import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.Timer;

/**
 *
 * @author chaotic875
 */
public class javaproject extends JFrame implements ActionListener, KeyListener {

    static final int Frames = 1000;
    static final int Counter = 1000 / Frames;
    static final int PLY_WIDTH = 512;
    static final int PLY_HEIGHT = 512;
    static final int OL_WIDTH = 60;
    static final int OL_HEIGHT = OL_WIDTH / 3;
//defining width and height

    
    int x = (PLY_WIDTH - OL_WIDTH) / 2;
    int y = (PLY_HEIGHT - OL_HEIGHT) / 2;
    char key = 'g';
    Timer t = null;
    JFrame g;
    GamePanel mp = null;

    javaproject() {
        g = new JFrame();
        t = new Timer(Counter, this);

        g.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        g.setTitle("Delaney Why");

        mp = new GamePanel();
        g.add(mp);
        g.addKeyListener(this);
    }

    //setting JFrame etc
    public static void main(String[] args) {
        javaproject g = new javaproject();
         
         
        g.pack();
        g.setVisible(true);
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        switch (key) {
            case 'a':
                x--;    //key for left movement;
                break;
            case 'd':
                x++;    // key for right movement;
                break;
      // key for movement toward the bottom of the screen;
            //defining movement actions         
            default:
                break;
        }
        //boundary checking
        if (x < 0) {
            x = 0;
        } else if (x > (PLY_WIDTH - OL_WIDTH)) {
            x = PLY_WIDTH - OL_WIDTH;
        }
        if (y < 0) {
            y = 0;
        } else if (y > (PLY_HEIGHT - OL_HEIGHT)) {
            y = PLY_HEIGHT - OL_HEIGHT;
        }
        mp.repaint();

    }

    @Override
    public void keyPressed(KeyEvent k) {
        key = k.getKeyChar();
    }

    @Override
    public void keyReleased(KeyEvent k) {
        key = 'g';
    }

    @Override
    public void keyTyped(KeyEvent k) {

    }

    class GamePanel extends JPanel {

        GamePanel() {
            this.setPreferredSize(new Dimension(PLY_WIDTH, PLY_HEIGHT));
            setBackground(Color.black);
        }

        @Override
        public void paintComponent(Graphics g) {
            super.paintComponent(g);
            g.fillOval(x, y, OL_WIDTH, OL_HEIGHT);

        }
    }

}

[Tim Cooke]

You gotta give us some kind of clue.

Does it compile?
Does it run?
Does it do what you expected?

If the answer to any of the above is 'no', provide details stating what you expected to happen, and exactly what did happen.

[Stephan van Hulst]

You tell us.

What is wrong? We can't really help you with just a piece of code to look at.

[fred rosenberger]

Posting 100+ lines of code and just saying "What's wrong" won't get you much help around here. How do you know something is wrong? Do you get errors? What are they? Does it run? What did it do vs. what did you expect it to do?

If you want to get the most out of this site, I'd strongly recommenced you read our FAQ on How To Ask Questions On JavaRanch

[Stevens Miller]

Well, for openers, you are using K&R indentation style, not Allman style. There may be other things...

[Henry Wong]

Stevens Miller wrote:Well, for openers, you are using K&R indentation style, not Allman style. There may be other things...

Don't want to start a war, but I actually like K&R more... perhaps it is from my C background.

Henry

[Alaric Mustoe]

Ok so it should appear something in the JFrame as defined but it comes up blank why is this is it a method placement error?

[Stevens Miller]

At Line 108, you are setting the GamePanel's background color to black. I am guessing you want that to make it be black on your screen, but (not having run this code myself) I am also guessing that it isn't black. I think that's because setting the background color doesn't actually draw anything. The background color is used when certain operations take place that need to know what it is, but the call to setBackground doesn't, by itself, make anything happen on your screen.

So, again guessing, I guess you are drawing a white oval on a white background at Line 114, and not seeing anything because white on white makes the oval invisible.

[Knute Snortum]

Moving to the Swing forum. (And K&R rules.)

[Alaric Mustoe]

Ok on one computer it runs fine it should draw an oval on a white background, but i has no content just a blank no color JFrame.

[Stevens Miller]

Quick side note: package names should be all lowercase, and class names should start with a capital letter.

[Stevens Miller]

Line 3 refers to a package named "GameTest.Draw," but it's not actually used for anything. You should take that line out.

[Stevens Miller]

I think your problem is in your javaproject constructor. You have it creating a JFrame, but it actually is a JFrame. Try this for your constructor:

    javaproject() {
//        g = new JFrame();
        t = new Timer(Counter, this);
 
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setTitle("Delaney Why");
 
        mp = new GamePanel();
        add(mp);
        addKeyListener(this);
    }

[Alaric Mustoe]

Thanks the constructor worked but now it will not move when i press "a" and "d"

@Override
    public void actionPerformed(ActionEvent e) {
        switch (key) {
            case 'a':
                x--;    //key for left movement;
                break;
            case 'd':
                x++;    // key for right movement;
                break;
      // key for movement toward the bottom of the screen;
            //defining movement actions         
            default:
                break;
        }
        //boundary checking
        if (x < 0) {
            x = 0;
        } else if (x > (PLY_WIDTH - OL_WIDTH)) {
            x = PLY_WIDTH - OL_WIDTH;
        }
        if (y < 0) {
            y = 0;
        } else if (y > (PLY_HEIGHT - OL_HEIGHT)) {
            y = PLY_HEIGHT - OL_HEIGHT;
        }
        mp.repaint();

    }

    @Override
    public void keyPressed(KeyEvent k) {
        key = k.getKeyChar();
    }

    @Override
    public void keyReleased(KeyEvent k) {
        key = 'g';
    }

    @Override
    public void keyTyped(KeyEvent k) {

    }

[Stevens Miller]

Well, you've implemented all three of the methods you need for the KeyListener interface, but what would make the ActionPerformed method you've implemented for the ActionListener interface ever run?

[Fred Kleinschmidt]

1. your "a" and "d" actions do the same thing (x--)
2. Since your GamePanel instance covers the JFrame, all of the key events go to the GamePanel, not to the JFrame.
3. All of your code should be in GamePanel. You don't even need a "javaproject" class.

[Les Morgan]

t.start();

[Les Morgan]

This is not correct, take a look back at the original code that I posted:

static final int Frames = 1000;
static final int Counter = 1000 / Frames;

You get a delay of 1ms which is not enough to do anything. Originally there as a variable called FPS which you need to set to how fast, reasonably fast, that you want your refresh to be done every second. With a FPS (Frames) say of 25 then you get 1000/25 or every 40 ms a refresh will be driven for your animation.

static final int Frames = 25;
static final int Counter = 1000 / Frames;

You will also, as mentioned in my earlier post, have to start the timer for the animation loop using t.start() in a scope where t is visible.

[Stevens Miller]

Fred Kleinschmidt wrote:2. Since your GamePanel instance covers the JFrame, all of the key events go to the GamePanel, not to the JFrame.

That's true, but since his GamePanel doesn't handle them, they just get passed to the JFrame object. His KeyListener methods do get called, but they don't invoke his ActionListener method for him.

[Les Morgan]

In the code, you have a basic animation loop for the game, that is controlled by your timer, which you have never started. The key listener senses what key has been pressed and stores that so when the animation loop is triggered by the timer firing the actionListener you have a repeating key by virtue of the key not being released--so your "ship" will move in the direction defined for the key in the actionListener. Once the key is released the key variable is cleared and when the action listener is fired by the timer, there is no movement.

This is a very basic game/animation as you can easily incorporate moving background and other feature due to the animation running on a continuous basis.

[Fred Kleinschmidt]

Here's an (incomplete) version that uses a single class without extending JFrame.

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;

import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.SwingUtilities;

public class GamePanel extends JPanel {

   static final int OL_WIDTH   = 60;
   static final int OL_HEIGHT  = OL_WIDTH / 3;

   static final int PLY_WIDTH  = 512;
   static final int PLY_HEIGHT = 512;

   protected int    x;
   protected int    y;

   public GamePanel( JFrame frame ) {
      x = (PLY_WIDTH - OL_WIDTH) / 2;
      y = (PLY_HEIGHT - OL_HEIGHT) / 2;
      setBackground( Color.BLACK );
      setPreferredSize( new Dimension( PLY_WIDTH, PLY_HEIGHT ) );

      frame.addKeyListener( new KeyAdapter() {
         @Override
         public void keyPressed( KeyEvent e ) {
            final int code = e.getKeyCode();
            switch (code) {
               case KeyEvent.VK_A:
                  x--; // or x -= distanceToMove;
                  break;
               case KeyEvent.VK_D:
                  x++;
                  break;
               default:
                  break;
            }
            checkBounds();
            repaint();
         }
      } );
   }

   protected void checkBounds() {
      // TODO: Insert bounds checking here.

   }

   @Override
   public void paintComponent( Graphics g ) {
      super.paintComponent( g );
      g.setColor( Color.WHITE ); // or whatever color the oval should be
      g.fillOval( x, y, OL_WIDTH, OL_HEIGHT );

   }

   public static void main( String[] args ) {
      // Schedule a job for the event dispatch thread:
      SwingUtilities.invokeLater( new Runnable() {
         @Override
         public void run() {
            // Create and set up the window.
            JFrame jf = new JFrame( "My Game" );
            GamePanel item = new GamePanel( jf );
            jf.add( item, BorderLayout.CENTER );
            jf.pack();
            jf.setVisible( true );
            jf.setDefaultCloseOperation( JFrame.EXIT_ON_CLOSE );
         }
      } );
   }

}

[Stevens Miller]

That's pretty good, Fred, and does show that subclassing JFrame wasn't necessary. Alaric's code is trying to use the Timer to make his oval move continuously while the "a" or "d" key is held down. Looks like yours moves it one time per keypress, right? (You did say it was incomplete, I know.)

Alaric, what you're doing with your actionPerformed method and your Timer is called "polling." You're checking, over and over, to see if key is "a" or "d" (and not "g"). Nothing inherently wrong with polling, but it's not needed in this kind of program and can use up a lot of CPU cycles for no reason. A better approach might be to start a timer when a key is pressed, and have that timer move your oval each time the timer's interval expires. When the key is released, you can just stop the timer.

This is a pretty ambitious little project for a new Java programmer. You're handling events, using a Timer, and managing the redrawing of your display. It's a good project idea, but some of the things you're doing indicate you are still learning the basics, like what it means to subclass an object from the Java library, how to follow naming conventions, and what a Timer does. I would suggest you put this one aside for a bit, and study the basics for now. Come back to this one later on.

[Fred Kleinschmidt]

In my example, if you hold down the "a" or "d" keys, the oval will continually move, since holding a key down will continue to send keyPressed events.

[Les Morgan]

Here is the link to the original post and full code: https://coderanch.com/t/657015/GUI/java/Moving-oval

[Fred Kleinschmidt]

there really isn't any need for a timer if motion happens only while a key is being held down. You can change the speed by changing x++ and x-- to x+=speed, etc. and add a setSpeed() method.
If one pixel per time unit is too fast, change x and y from int to double and have the increment less than 1.0. this can easily be adapted to multiple ovals moving at different speeds, all without using a timer.

[Les Morgan]

Good idea, but you will tend to get a buffering problem depending on your OS and the speed of your CPU: you will release the key and you will still get a movement until the key buffer is empty and that can be a considerable distance at times.

Fred Kleinschmidt wrote:In my example, if you hold down the "a" or "d" keys, the oval will continually move, since holding a key down will continue to send keyPressed events.

[Fred Kleinschmidt]

Les Morgan wrote:
you will release the key and you will still get a movement until the key buffer is empty and that can be a considerable distance at times.

that can be solved by implementing keyReleased and adding a flag that says to honor/ignore key events, (But care has to be taken to allow future key-presses)

No - I take that back; the key release event will just be queued after all of the other key presses, so won't be processed until all of the presses have been handled.

[Les Morgan]

Yep, that and the expectation that he will want a moving background or other possible animation features, at some time, was why the code has the timer for the animation loop in the first place. With taking advantage of the animation loop to check for the state of the key, decoding what is in "key", then you get instantaneous response from key changes--no buffer build ups. This becomes extremely important if you ever want to shoot anything in your game or calculate collisions.

The original post was from a couple weeks back, same original poster.

Fred Kleinschmidt wrote:

Les Morgan wrote:
you will release the key and you will still get a movement until the key buffer is empty and that can be a considerable distance at times.

that can be solved by implementing keyReleased and adding a flag that says to honor/ignore key events, (But care has to be taken to allow future key-presses)

No - I take that back; the key release event will just be queued after all of the other key presses, so won't be processed until all of the presses have been handled.

[Alaric Mustoe]

So the oval is appearing but not moving I have taken into account that maybe it is moving a slow amount but that is not the case. Rather it isn't moving at all. Can anyone explain why this is?

[Stevens Miller]

Alaric Mustoe wrote:So the oval is appearing but not moving I have taken into account that maybe it is moving a slow amount but that is not the case. Rather it isn't moving at all. Can anyone explain why this is?

Post your code as it now is, Alaric, so we can see what you've changed.

[Les Morgan]

Because he never starts the timer to activate the actionPrefored method of the ActionListener which then appropriately increments the position variables decoded from the KeyListener input stored in "key".

Stevens Miller wrote:

Alaric Mustoe wrote:So the oval is appearing but not moving I have taken into account that maybe it is moving a slow amount but that is not the case. Rather it isn't moving at all. Can anyone explain why this is?

Post your code as it now is, Alaric, so we can see what you've changed.

[Stevens Miller]

Les Morgan wrote:Because he never starts the timer to activate the actionPrefored method of the ActionListener which then appropriately increments the position variables decoded from the KeyListener input stored in "key".

Yeah, that was evident when you pointed it out before. But I think he's having some problems that reflect a larger lack of understanding that won't go away if we just tell him to add a line of code. I want him to post his code so we can ask him what he expects will cause his actionPerformed method to run. If he can see from his existing code that nothing makes his actionPerformed method run, the next step will be to ask him what can he do to get it to run. There are other ways to help him learn, but asking instructive questions is often a good one.

[Alaric Mustoe]

I found my error, I was defining the x and y as static values there for never moving as I referenced back to Les Morgans code I realized this.

[Campbell Ritchie]

The moral of the story is, never make anything static unless you have a good reason so to do.

[Alaric Mustoe]

Les Morgan does have a point. I would like to have it shoot and do collision detection. But if I define a bullet which i have. Would it do better to use one and loop it or to have multiple and shoot all then refresh? Along with I need help with collision detection I know that you use intersecting point checks. But don't know how to utilize them.

[Les Morgan]

If you make your Classes extend any Class that implements Shape or implements Shape directly, then you get a whole API that is centered toward doing what you want--including collision detection methods.

You can use the same type of trigger detection for shooting bullets as you do moving, but define another Timer and ActionListener for the burst speed if you choose to do multiple projectiles. The javax.swing.Timer Class does not create a new thread with each timer, so very little overhead is taken up with new timer requests.

Alaric Mustoe wrote:Les Morgan does have a point. I would like to have it shoot and do collision detection. But if I define a bullet which i have. Would it do better to use one and loop it or to have multiple and shoot all then refresh? Along with I need help with collision detection I know that you use intersecting point checks. But don't know how to utilize them.

[Les Morgan]

Also now you have moved beyond having a single object move across a screen. Make your classes for things like ship, bullet, and etc independent Classes and add them, an add(MyObject), to the "game board", the animation loop. I like to do this by using an ArrayList and changing the paintComponent(Graphics g) to be similar to this:

  public void paintComponent(Graphics g){
    super.paintComponents(g);
    for(MyObject o: MyCollection) o.update(g);
  }

Where each Class has an update(Graphics) method that controls what to do for each update cycle for the object. If you want the objects to interact, then you need to pass a reference to MyCollection to each object too, so you get o.update(Graphics, MyCollection);

  public void paintComponent(Graphics g){
    super.paintComponents(g);
    for(MyObject o: MyCollection) o.update(g, MyCollection);
  }

Alaric Mustoe wrote:Les Morgan does have a point. I would like to have it shoot and do collision detection. But if I define a bullet which i have. Would it do better to use one and loop it or to have multiple and shoot all then refresh? Along with I need help with collision detection I know that you use intersecting point checks. But don't know how to utilize them.

[Les Morgan]

IMO: there is no difference between multiple bullets and single bullets--besides a little complexity--the real question is: What do you think looks cooler in your game?

Also you need to consider: bullets tend to be independent projectiles--what objects will they interact with? Will they just blowup opponents' ships or do they do damage to friendlies, planets, and etc too?

Here is an implementation question for you: rectangular bullets or round? If you have rectangular bullets, or some shape not round, then you need to be able to have them draw themselves along a vector to give proper orientation to the projectile, ie: say you have your ship facing Northwest, it will look very funny to have bullets firing that are horizontal or vertical in nature instead of being aligning along a Northwest vector.

Also, you need to take into account ship movement if you fire multiple projectiles--it looks really funny to have your ship 3 moves over and your 7th projectile in a grouping of 10 still coming from the location where the bullets were first fired.

Alaric Mustoe wrote:Les Morgan does have a point. I would like to have it shoot and do collision detection. But if I define a bullet which i have. Would it do better to use one and loop it or to have multiple and shoot all then refresh? Along with I need help with collision detection I know that you use intersecting point checks. But don't know how to utilize them.

[Campbell Ritchie]

Les Morgan wrote:. . . public void paintComponent(Graphics g) . . .

It would have protected access, surely? And remember the @Override annotation.

[Les Morgan]

BTW: you have pretty much received the answer to your "What's wrong with this?" query, further discussion should probably be taken to the game development forum with new questions.

Alaric Mustoe wrote:Les Morgan does have a point. I would like to have it shoot and do collision detection. But if I define a bullet which i have. Would it do better to use one and loop it or to have multiple and shoot all then refresh? Along with I need help with collision detection I know that you use intersecting point checks. But don't know how to utilize them.