KeyListener probs, noo not another one

Newbie & Debugging Questions
#1

Man i just cannot get this to work, could u please help!!
What am i doing wrong:

import java.awt.*;
import java.awt.geom.*;
import java.awt.event.*;
import javax.swing.*;

public class HumanPlayer extends JFrame{
	
	//
	double velocity = 0;
	double speed = 5;
	boolean upkey, downkey, leftkey, rightkey;
	
	public HumanPlayer(){
		addKeyListener(listener);
	}
	
	public double getVel(){
		if(upkey){
			velocity += speed;
		} else if(downkey){
			velocity -= speed;
		} 
		velocity = velocity * .98;
		
		return velocity;
	}
	/*
	addWindowListener(new WindowAdapter(){
		public void windowClosing(WindowEvent e){
			System.exit(0);
		}
	});
	*/	
	
	KeyListener listener = new KeyListener(){
		public void keyPressed(KeyEvent e){
			int code = e.getKeyCode(); 
			
			switch(code){
				case KeyEvent.VK_UP: upkey = true; System.out.println("Up pressed"); break; 
				case KeyEvent.VK_LEFT: leftkey = true; break; 
				case KeyEvent.VK_RIGHT: rightkey = true; break; 
				case KeyEvent.VK_DOWN: downkey = true; break; 
			}
			
		};
		
		public void keyTyped(KeyEvent e){};
		
		public void keyReleased(KeyEvent e){
			int code = e.getKeyCode(); 
			
			switch(code){
				case KeyEvent.VK_UP: upkey = false; System.out.println("Up released"); break; 
				case KeyEvent.VK_LEFT: leftkey = false; break; 
				case KeyEvent.VK_RIGHT: rightkey = false; break; 
				case KeyEvent.VK_DOWN: downkey = false; break; 
			}
			
		}
	};
}

I can’t get the keyListener to work, i must just be doing something wrong then?
I can give the other files if u need them. I’m just trying to get a box to move around atm.
Thanks,
Dr Warm

#2

Maybe you have to call getVel() in your keyPressed method.

In your code when you press a key (e.g. up key) you set upkey to true, but nobody know that happend; on release you set it to false, and again nobody “see” this change. :wink:

#3

I cannot find any errors in your KeyListener, but you should try use { } behind the case, if you use multiple commands:

case KeyEvent.VK_UP: { up = true; System.out.println(“up”); break; }

you get it?

This might be why you dont get a printline

Except that, you will need a permanent loop which asks for getvel every so and then milliseconds, I suppose you have a loop somewhere and just left it out.

Just one call to getVel wont do it.

Try using the {} around your statement blocks, I cannot find anything else.
You could try to define the listener BEFORE you add it, maybe this makes trouble?

-JAW

#4

That’s a matter of personal preference, the way it is now, the println will execute normally.

I can’t see any glaring errors in your code either, but something to watch out for with KeyListeners is keyboard focus. Try adding a requestFocus() call in your HumanPlayer constructor.

#5

Alarm bells started when I got to the first line of code ::slight_smile:

That is most definitely not an ‘is a’ relationship ???

#6

Yes getVel() is called by a Game class that contains an instance of HumanPlayer, every 30 milliseconds (every redraw).

I’ve used this println thing before, so it should work in the switch statement.


Alarm bells started when I got to the first line of code  

Quote
public class HumanPlayer extends JFrame

That is most definitely not an 'is a' relationship

Um ok, i wasn’t sure. But if i don’t ‘extends JFrame’ i can’t use addKeyListener anywhere!!!
Can u please explain how to get around this!!

#7

What Annon meant was that a “player” is not a “frame”. Usually you make a new class for in game objects that hold data like velocity. What you’ve done here is bundle two things into one class. This isnt a big deal if you are just hacking around.

So you could make two classes like this.

SampleGame extends JFrame
HumanPlayer

But like I said no big deal if you are just hacking.

#8

Also you need to add the listener to the Game Frame.
In your Game Frame you have some instaces of your game objects (like Human Player), so when the event is received, you update your Human Player accordingly.

Some little pieces of code:

class HumanPlayer{
  public void update(Event e){
    //Process the event
  }
}

class GameFrame extends JFrame{
  HumanPlayer player1;
  ...  
  public GameFrame(){
     ...
      addKeyListener(new KeyListener(){
         public void keyPressed(KeyEvent ke){
           player1.update(ke);
         }
      });
      ...
  }
  ...
}

Note that this is one way to solve your problem there are other ways to do it.

Rafael.-
#9

AAAAAAARrrrrrggghhhhhhhh!!! This has to be one of the most simple things, but i can’t get it right!! I feel that i’ve tried everything!!

I think the problem is in the KeyListener or in the addKeyListener, but i’m not sure what!!

I’ve included all my source code in this post, just in one txt file, so u’ll have to break it up (Sorry i wasn’t sure what
the convention was). But if you have a spare 5 mins or so, could you have a peek.

Or here’s my new GameFrame.java:

import javax.swing.*; // For JPanel, etc.
import java.awt.*; // For Graphics, Color etc.
import java.awt.geom.*; // For Rectangle2D, etc
import java.awt.event.*;
public class GameFrame extends JPanel {
	
	// the angle generated
	double x = 300;
	double y = 300;
	
	HumanPlayer Player = new HumanPlayer();
	// and main circle
	
	// constructor
	public GameFrame(){
		requestFocus();
		addKeyListener(listener);
	}
	KeyListener listener = new KeyListener(){
		public void keyPressed(KeyEvent ke){
			System.out.println("Key is pressed");
			Player.pressedKey(ke);
		}
		public void keyTyped(KeyEvent ke){};
		public void keyReleased(KeyEvent ke){
			Player.releasedKey(ke);
		}
	};
	
	public void paintComponent(Graphics g){
		
		super.paintComponent(g);
		Graphics2D g2d = (Graphics2D)g;
		
		// 
		g2d.setColor(Color.black);
		
		y = y+Player.getVel();
		Rectangle2D.Double player = new Rectangle2D.Double(x, y, 10, 30);
		
		g2d.draw(player);
	}
}

Maybe there’s something inherently wrong in there i’m not picking up!

Thanks for all the quick replies
Dr Wam

#10

You’ve got a lot of issues. Mostly caused by bad style I suppose. E.g. requestFocus() and setVisible() are things that you generally don’t call in a constructor. You should probably stick to the Java coding convention of starting identifier names with a lowercase letter and class names with an uppercase letter.

You’ve construction code scattered all over your source files, much of it outside of constructors.

You have physics/player state updates inside paintComponent, instead of in your main game loop.

You have methods named “pressedKey” and “releasedKey” in your player class, which is really a UI thing. You really want methods to set the player’s direction of acceleration - call those methods from your KeyListener if you like.

The solution to your main problem though is that an empty panel is by default not focusable.

Simply call

setFocusable(true)

in the GameFrame constructor.

#11

I was just reading through and was amazed nobody had said that yet. Yes, you can either setFocusable to true in the constructor, or


    public boolean isFocusable()
    {
        return true;
    }

You can override the isFocusable method like the above. Generally, you should go with simply calling setFocusable, but sometimes when making games you don’t want the main window to ever lose focus, or you want two panels to have focus. Say, for example, you have the game window on the left and scoreboard/interface on the right. Rather than have the game panel pass its key commands to the scoreboard (or a master model) or vise versa, you can trick Java into giving them both focus all the time. Or if you have Swing elements like buttons which will steal focus upon pressing this can be useful as well.

I usually use setFocusable(true) and then every time step say requestFocus() (this gives the focus to the component requesting it) so that I am not potentially messing anything up, but I have found myself twice doing the above method for whatever advantage.

Hope that works.

#12

[quote]Simply call

setFocusable(true)
[/quote]
Thankyou, thankyou!! I was seriously going mental over that!!! <-- i thought it would be simple

[quote]requestFocus() and setVisible() are things that you generally don’t call in a constructor
[/quote]
Would you believe that DrawingFrame actually came from my computer science lecturer, and has been in the course notes for prob 5 years!! So if i’ve learnt bad programming blame those guys!

[quote]You should probably stick to the Java coding convention of starting identifier names with a lowercase letter and class names with an uppercase letter.
[/quote]
I thought i was pretty good with this, the only time i didn’t do it was with the instance of HumanPlayer (i named it Player).

[quote]You’ve construction code scattered all over your source files, much of it outside of constructors.
[/quote]
I’m not sure what you mean by this? I can’t see what i’m doing wrong, but then i was taught to do it like this, so please tell me.

[quote]You have physics/player state updates inside paintComponent, instead of in your main game loop.
[/quote]
I know this is wrong, but i was just doing sort of a hack-and-slash to try and get the thing to actually move. Won’t happen again :smiley:

[quote]You have methods named “pressedKey” and “releasedKey” in your player class, which is really a UI thing. You really want methods to set the player’s direction of acceleration - call those methods from your KeyListener if you like.
[/quote]
Makes sense, I’ll try to call it something different next time. The only thing was i wanted to process the KeyEvents inside the HumanPlayer class, but probably is better to do it in the game class.

@demonpants: Thanks for the info, but i’m not totally up with the focus yet, so i think i’ll have to keep reading you’re post to make sense of it!

Thanks every1 you’ve been tons of help, i’m sure i’ll more problems in a day or two! Should i make a new thread if i do or not?

#13

RE: construction code…

In GameFrame you have initialization of x & y, then a constructor, then initialization of the key listener. The member variables are usually kept together, but I sometimes also make a distiction with inner classes done this way.

Put any new issues, unrelated to KeyListener, in a new thread with an appropriate subject.

#14

Would you believe that DrawingFrame actually came from my computer science lecturer, and has been in the course notes for prob 5 years!! So if i’ve learnt bad programming blame those guys!

@demonpants: Thanks for the info, but i’m not totally up with the focus yet, so i think i’ll have to keep reading you’re post to make sense of it!

Thanks every1 you’ve been tons of help, i’m sure i’ll more problems in a day or two! Should i make a new thread if i do or not?
[/quote]
I actually called setVisible(true) in most contructors for JFrames, personally. Basically I initialize everything, set the size of the window, and then make it visible. It’s always worked fine for me. I don’t see why you wouldn’t call getFocus() in the constructor also, as long as it is after the setVisible() call. So your teacher is not necessarily teaching you bad practices, it’s just another way of doing things. swpalmer probably keeps variable initialization in the constructor and window intialization elsewhere because it reduces coupling in your code. Sure, good practice, but not necessary.

Also, if you need help understanding anything I said I’ll certainly explain it. And don’t bother making a new post - I always check my “show new replies to your posts” link when I log in, so I’ll see it.
[/quote]

#15

I know that calling those methods in a constructor can work. But things like requestFocus() in particular are rather indirect for a constructor. As you said, I like to construct the objects in the constructor - but not the entire UI… i.e. make the object, but don’t make it visible. That way I can construct objects and save them to be made visible later, the code is more generic/reusable. I would definately say that the profs code was using poor style.

The new posts would be for the benfit of keeping the boards organised. Sure threads drift off-topic all the time, but there is no sense forcing threads off topic on-purpose. Many people skip posts based on the subject.

#16

Hmm, that’s a good point. Kepping things robust is very important. I think the only times I ever really use setVisible are in the actual main class, so it’s not like it can contain a bunch of itself. But if you were dealing with multiple windows or eveer wanted more robust visibility, that’s a very good point.