[Lagaam]

The title says I can't move my character around with the arrow keys, but I tried using the "a" key to move left and that didn't work either. If someone could point out what I'm doing wrong and give me a push in the right direction that would be great!~

engine <- (2D engine I'm working on)

package rand.LearningJava.Lagaan;

import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;
import java.awt.event.MouseMotionListener;

import javax.swing.JComponent;
import javax.swing.JFrame;

public class engine {

	public static void start(Game game) {
		JFrame frame = new JFrame(game.getTitle());
		frame.setSize(game.width(), game.height());
		frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
		GameCanvas canvas = new GameCanvas(game);
		frame.add(canvas);
		frame.setVisible(true);
		GameLoop loop = new GameLoop(game, canvas);
		loop.start();
	}
}

abstract class Game implements KeyListener, MouseListener, MouseMotionListener {
	
	protected String title = "GameEngine";
	protected int width = 640, height = 480, delay = 30;
	protected boolean over;
	
	public abstract void init();
	public abstract void update();
	public abstract void draw(Graphics2D g);
	
	public String getTitle() {
		
		return title;
	}
	
	public int width() {
		return width;
	}
	
	public int height() {
		return height;
	}
	
	public int getDelay() {
		return delay;
	}
	
	public boolean isOver() {
		return over;
	}
	
	@Override
	public void keyPressed(KeyEvent e) {
		
	}

	@Override
	public void keyTyped(KeyEvent e) {
		
	}

	@Override
	public void keyReleased(KeyEvent e) {
		
	}

	@Override
	public void mouseClicked(MouseEvent e) {
		
	}

	@Override
	public void mousePressed(MouseEvent e) {
		
	}

	@Override
	public void mouseReleased(MouseEvent e) {
		
	}

	@Override
	public void mouseEntered(MouseEvent e) {
		
	}

	@Override
	public void mouseExited(MouseEvent e) {
		
	}

	@Override
	public void mouseDragged(MouseEvent e) {
		
	}

	@Override
	public void mouseMoved(MouseEvent e) {
		
	}
}

class GameCanvas extends JComponent {
	
	private static final long serialVersionUID = 1L;
	private final Game game;
	
	public GameCanvas(Game game) {
		this.game = game;
		addKeyListener(this.game);
		addMouseListener(this.game);
		addMouseMotionListener(this.game);
		requestFocus();
	}
	
	public void paintComponent(Graphics g) {
		game.draw((Graphics2D) g);
	}
	
}

class GameLoop extends Thread {
	
	private final Game game;
	private final GameCanvas canvas;
	
	public GameLoop(Game game, GameCanvas canvas) {
		this.game = game;
		this.canvas = canvas;
	}
	
	public void run() {
		game.init();
		
		while (!game.isOver()) {
			game.update();
			canvas.repaint();
			try {
				Thread.sleep(game.getDelay());
			} catch (Exception e) {
				e.printStackTrace();
			}
		}
	}
}

gameTest <- (Actual game)

package rand.LearningJava.Lagaan;

import java.awt.Graphics2D;
import java.awt.event.KeyEvent;
import java.awt.image.BufferedImage;
import java.io.File;
import java.io.IOException;

import javax.imageio.ImageIO;

public class gameTest extends Game {
	
	public static final void main(String[] args) {
		engine.start(new gameTest());
	}
	
	public BufferedImage player;
	
	final int STEP = 1;
	int frame, x = 100, y = 100, dir;
	
	public gameTest() {
		title = "gameTest";
		frame = 0;
		
		dir = KeyEvent.VK_DOWN;
		
		try {
			player = ImageIO.read(new File("images/FinalFantasy3BlueKnight.png"));
		} catch (IOException e) {
			e.printStackTrace();
		}
	}

	@Override
	public void init() {
		
	}
	

	@Override
	public void keyPressed(KeyEvent e) {
		dir = e.getKeyCode();
	}

	@Override
	public void update() {
		frame++;
		if (frame > 1) {
			frame = 0;
		}
		switch (dir) {
		case KeyEvent.KEY_RELEASED:
			x += 0;
			y += 0;
			break;
		case KeyEvent.VK_A:	//LEFT
			x -= STEP;
			break;
		case KeyEvent.VK_RIGHT:	//RIGHT
			x += STEP;
			break;
		case KeyEvent.VK_UP:	//UP
			y -= STEP;
			break;
		case KeyEvent.VK_DOWN:	//DOWN
			y += STEP;
		}
	}

	@Override
	public void draw(Graphics2D g) {
		g.drawImage(player.getSubimage(frame*16, 32, 16, 16), x, y, null);
	}
}

[farrell2k]

I don't even want to touch this mess of code. your movement issue is due to you needing to call setFocusable(true) on your JComponent.

[Lagaam]

How did I miss that!?!? I thought I added that when I made the engine. Well thanks, you saved me a lot of stress trying to figure out my silly mistake

[Lagaam]

farrell2k, on 05 October 2012 - 05:48 PM, said:

I don't even want to touch this mess of code.

Was in a bit of a rush when I last replied, but what do you mean by mess? I though I was doing pretty good at keeping it all nice and organized...

[farrell2k]

To each his own, my friend, but you have too many classes in one file making things difficult to follow, and you're not farming out responsibility properly e.g game objects should draw themselves and handle their own input. You're still very early on, but this route is going to take you down the path of synchronization problems as well. You should not be handling key or mouse event code in the event dispatch thread, while executing your update() on your gameloop thread. It's a recipe for
problems.

You should have a player class that inherits from a base entity class, a separate canvas that runs your game loop, and a JFrame class that does one thing, which is displays your canvas.

[CasiOo]

farrell2k, on 06 October 2012 - 04:23 PM, said:

To each his own, my friend, but you have too many classes in one file making things difficult to follow, and you're not farming out responsibility properly e.g game objects should draw themselves and handle their own input. You're still very early on, but this route is going to take you down the path of synchronization problems as well. You should not be handling key or mouse event code in the event dispatch thread, while executing your update() on your gameloop thread. It's a recipe for
problems.

You should have a player class that inherits from a base entity class, a separate canvas that runs your game loop, and a JFrame class that does one thing, which is displays your canvas.

Hmmmmm I don't agree on all you say

He is saving the key presses in the event thread, and that is fine. He just shouldn't do any big work, but saving the key press is fine.
He is repainting the screen on the event dispatch thread, so that is also fine.
Updating the game in the game thread is also fine, so I don't really agree with you on this.

Though I agree that he should think about the object's responsibilities
The code is not a mess to read, but the design could be better, but it is not that bad

Your gameloop will drift, because of your way of handling sleep. There is all kind of problems with Thread.sleep that can make your gameloop not following the delay you wanted

This post has been edited by CasiOo: 06 October 2012 - 09:53 AM

[Lagaam]

Thank you guys for all the input. I wanted to try making my engine into 1 class file because I thought in the long run it might be easier to work with 1 file than multiple, but if you guys say I should split it up into multiple files, this method of writing it was just something I would have preferred.

As I'm still new to programming, could you explain what you mean by object responsibilities?

You say my gameloop will drift, is there a better alternative I could take? I just learned that off a tutorial.

[farrell2k]

CasiOo, on 06 October 2012 - 04:50 PM, said:

farrell2k, on 06 October 2012 - 04:23 PM, said:

To each his own, my friend, but you have too many classes in one file making things difficult to follow, and you're not farming out responsibility properly e.g game objects should draw themselves and handle their own input. You're still very early on, but this route is going to take you down the path of synchronization problems as well. You should not be handling key or mouse event code in the event dispatch thread, while executing your update() on your gameloop thread. It's a recipe for
problems.

You should have a player class that inherits from a base entity class, a separate canvas that runs your game loop, and a JFrame class that does one thing, which is displays your canvas.

Hmmmmm I don't agree on all you say

He is saving the key presses in the event thread, and that is fine. He just shouldn't do any big work, but saving the key press is fine.
He is repainting the screen on the event dispatch thread, so that is also fine.
Updating the game in the game thread is also fine, so I don't really agree with you on this.

Think about it a little bit more.

Imagine you're writing a silly game with a few enemies moving on screen. Your job is to click on them to destroy them with your mouse cursor. You have an update method like this in your main game loop thread.

public void update() {
    if (enemy != null)
        enemy.move(); //if enemy is not null, move it wherever.
}

Now imagine in your mousePressed you have this

public void mousePressed(MouseEvent e) {
   if (enemy.contains(e.getX(), e.getY))
        enemy = null; //enemy has been killed by clicking on it.
}

Now, image a scenario where your game loop thread is running, does the conditional check in update() as to whether or not the enemy is null, and discovers that it is in fact !null, so it is safe to move. Cool. Now imagine before the enemy.move() method is executed, the event dispatch thread is preempted, as this can happen any time, and mousePressed is executed setting enemy to null. The game loop thread still thinks that enemy is !null, so it executes enemy.move(). What happens? Your game throws a NullPointerException.

How do you fix this? You can synchronize, but then you get into a situation where you have to synchronize a lot, so the best option it to take that MouseEvent and push it onto the game loop thread and execute it there so there are no potential threading issues.

This post has been edited by farrell2k: 06 October 2012 - 12:33 PM

[CasiOo]

You have to take the OS into account. The OS chooses when to give a thread a timeslice, and for how long the timeslice should be. You see how this could be problem together with Thread.sleep ?

You also have to take the execution of your update and render into account Lets say that your delay is 16ms, what if your gameloop takes 5ms, should you still sleep 16ms ?

Also there are other problem with Thread.sleep, what if it was interrupted?

This post has been edited by CasiOo: 06 October 2012 - 12:40 PM

[CasiOo]

farrel2k he should still be handling key events on his event dispatching thread
What he should do is keeping track of which key is being pressed, then when the update method is about to execute, he saves the keys in some way where they can not be modified. In the update method the nonmodifyable(Is this a word?) keys will be used. This way the user can press the keys he want (will be used in the next update), while the keys used in the update will be constant and not change during the update.

[farrell2k]

CasiOo, on 06 October 2012 - 07:39 PM, said:

You have to take the OS into account. The OS chooses when to give a thread a timeslice, and for how long the timeslice should be. You see how this could be problem together with Thread.sleep ?

You also have to take the execution of your update and render into account Lets say that your delay is 16ms, what if your gameloop takes 5ms, should you still sleep 16ms ?

Also there are other problem with Thread.sleep, what if it was interrupted?

Thread.sleep is accurate enough.

You can vary the sleep time, you can have a set sleep time, or you can sleep constantly e.g. 2ms and update your game and objects based on time. I like the third option.

What other thread is going to call interrupt on your game loop? If the JVM does it, something very wrong has occurred, and there is no way to save your app anyway.

CasiOo, on 06 October 2012 - 07:55 PM, said:

farrel2k he should still be handling key events on his event dispatching thread
What he should do is keeping track of which key is being pressed, then when the update method is about to execute, he saves the keys in some way where they can not be modified. In the update method the nonmodifyable(Is this a word?) keys will be used. This way the user can press the keys he want (will be used in the next update), while the keys used in the update will be constant and not change during the update.

The best way to handle input is to put all AWTEvents of key and mouse into an ArrayList, then have a method in your main game loop that iterates over the List and handles the key and mouse events so that everything is done on one thread.

This post has been edited by farrell2k: 06 October 2012 - 01:22 PM