Wondering effeciency of my code.

Newbie & Debugging Questions
#1

Okay so I’m new to alot of techniques in Java, such as OOP. My code is messy, execution of things are a bit wierd. Im wondering what way I could better organize my code so that it is easier to read and runs more effecient!

Please any tips are welcome!
(Paste is of my 2D tile engine)

Sorry if there is a bit much. Just want to make sure I am on the right track, and not doing any bad habits…


import java.net.*;
import java.io.*;
import java.util.*;
import java.lang.*;	
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;

public class vanMain
{
	public static Game myGame;
	public static void main(String args[])
	{
		myGame = new Game();
		myGame.setSize(640,480);
		myGame.setVisible(true);
		myGame.setResizable(false);
		myGame.setTitle("Vandrel Online");
		myGame.mainGame();	
	}
}
class Game extends Frame implements KeyListener
{
	public Image dbImage = null;
	public Graphics dbg = null;
	
	ArrayList clientList = new ArrayList();
	
	Image[] tiles = new Image[80];
	
	TileManager tileMan = new TileManager();
	Layer map01;
	Layer map02;
	Layer map03;
	Layer map04;
	Player myPlayer = new Player();
	boolean gameOn=true;
	
	long startTime = 0;
	float currentTime=0;
	float speedFactor=0;
	float deltaTime=0;
	float lastTime=0;
	
	public void mainGame()
	{
		startTime=System.currentTimeMillis();
		
		this.addKeyListener(this);
		myPlayer.loadPlayerData();
//		this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

		loadMap("nicehouse.MAP");
		while(gameOn)
		{
			//try{
			//Thread.sleep(5);
			//}catch(Exception e){}
			myPlayer.movePlayer();
			this.repaint();	
		}
	}
	public void addMapOffset(int x, int y)
	{
		map01.offsetX+=x;
		map02.offsetX+=x;
		map03.offsetX+=x;
		map04.offsetX+=x;
		
		map01.offsetY+=y;
		map02.offsetY+=y;
		map03.offsetY+=y;
		map04.offsetY+=y;
	}
	public void subMapOffset(int x, int y)
	{
		map01.offsetX-=x;
		map02.offsetX-=x;
		map03.offsetX-=x;
		map04.offsetX-=x;
		
		map01.offsetY-=y;
		map02.offsetY-=y;
		map03.offsetY-=y;
		map04.offsetY-=y;
	}
	
	public void addNewPlayer(String name,int x,int y)
	{
		Client singleClient = new Client(name,x,y);
		clientList.add(singleClient);
	}
	public void update (Graphics Screen)
	{
		if (dbImage == null)
		{
			dbImage = createImage (this.getSize().width, this.getSize().height);
			dbg = dbImage.getGraphics ();
		}
		dbg.setColor (getBackground ());
		dbg.fillRect (0, 0, this.getSize().width, this.getSize().height);
		dbg.setColor (getForeground());
		paint (dbg);
		Screen.drawImage (dbImage, 0, 0, this);
	}
	public void paint(Graphics g)
	{
		
		g.setClip(0,0,640,480);
		g.setColor(Color.black);
		g.fillRect(0,0,640,480);
		
		map01.drawLayer(g);
		map02.drawLayer(g);
		myPlayer.drawPlayer(g);
		map03.drawLayer(g);
		if(map04.grid[myPlayer.corX][myPlayer.corY]==0)
			map04.drawLayer(g);
	}
	
	
	public void loadMap(String mapName)
	{
   		try
   		{
			BufferedReader Input = new BufferedReader(new FileReader(mapName)); 
			int newSize = Integer.parseInt(Input.readLine());
			map01 = new Layer(newSize,newSize,0);
			map02 = new Layer(newSize,newSize,0);
			map03 = new Layer(newSize,newSize,0);
			map04 = new Layer(newSize,newSize,0);
			for(int x = 0;x<=newSize-1;x++)
			for(int y = 0;y<=newSize-1;y++)
			{
				map01.grid[x][y]=Integer.parseInt(Input.readLine());
				map02.grid[x][y]=Integer.parseInt(Input.readLine());
				map03.grid[x][y]=Integer.parseInt(Input.readLine());
				map04.grid[x][y]=Integer.parseInt(Input.readLine());
				map01.isWall[x][y]=Integer.parseInt(Input.readLine());
			}
			Input.close();
		}catch(IOException e){}
		
	}
	
	public void keyTyped(KeyEvent e){}
	public void keyReleased(KeyEvent e)
	{
		//myPlayer.playerDir=0;
		//System.out.println("wee");
	}
	public void keyPressed(KeyEvent e)
	{
		if (e.getKeyCode() == KeyEvent.VK_LEFT)
		{
			if(myPlayer.playerDir==0)
				myPlayer.playerDir=1;
		}
		if (e.getKeyCode() == KeyEvent.VK_RIGHT)
		{
			if(myPlayer.playerDir==0)
				myPlayer.playerDir=2;
		}
		if (e.getKeyCode() == KeyEvent.VK_DOWN)
		{
			if(myPlayer.playerDir==0)
				myPlayer.playerDir=3;
		}	
		if (e.getKeyCode() == KeyEvent.VK_UP)
		{
			if(myPlayer.playerDir==0)
				myPlayer.playerDir=4;
		}
	}
}



import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
import java.io.*;
class Layer
{
	int[][] grid;
	int[][] isWall;
	int x,y,n;
	int offsetX;
	int offsetY;
	public Layer(int x, int y, int n)
	{
		this.n = n;
		Toolkit toolkit = Toolkit.getDefaultToolkit();
		this.x=x-1;
		this.y=y-1;
		grid = new int[x][y];
		isWall = new int[x][y];
		
	}
	public void drawLayer(Graphics g)
	{
		for(int x = 0;x<=this.x;x++)
		{
			for(int y = 0;y<=this.y;y++)
			{
				//System.out.println("drawing tile!");
				//switch(grid[x][y].tile_id)
				//{
					int tileNum=grid[x][y];
					if(tileNum!=0)
						g.drawImage(vanMain.myGame.tileMan.tileImg[tileNum],offsetX+(x*20),offsetY+(y*20),null);
				//}
			}
		}
	}
}
class TileManager
{
	BufferedReader Input=null;

	Image[] tileImg = new Image[200];
	String[] tileList = new String[100];
	int numTiles;
	public TileManager()
	{
		Toolkit toolkit = Toolkit.getDefaultToolkit();
		String inVal="";
		int lcv=1;
		try
		{
			Input = new BufferedReader(new FileReader("tiles.lis")); 
			while((inVal=Input.readLine()) != null)
			{
				tileList[lcv]=inVal;
				tileImg[lcv]= toolkit.getImage(inVal);
				System.out.println(inVal);
				lcv++;
			}
			numTiles=lcv;
	
		}catch(IOException e){}
	}
	
}



import java.util.*;
import javax.swing.*;
import java.io.*;
import java.lang.*;	
import java.awt.*;
class Player
{
	int speedX=0;
	int speedY=0;
	int x=0;
	int y=0;
	int corX=0;
	int corY=0;
	int playerDir=0;
	float moveAmnt=0;
	Image[] playerImg = new Image[4];
	public Player()
	{
	}
	public void movePlayer()
	{
		if(System.currentTimeMillis()-vanMain.myGame.startTime>=0)
		{
			//System.out.println("poo");
			int moveSpeed=2;
			//System.out.println(moveSpeed);
			if(playerDir==0 || moveAmnt==0)
			{
				corX=-1*(vanMain.myGame.map01.offsetX-320)/20;
				corY=(-1*(vanMain.myGame.map01.offsetY-240)/20)+1;
			}
			if(playerDir==1 && (vanMain.myGame.map01.isWall[corX-1][corY]==0))	
			{
				if(moveAmnt<=10)
				{
					vanMain.myGame.addMapOffset(moveSpeed, 0);
					moveAmnt++;
				}
				if(moveAmnt>=10)
					moveAmnt=0;
					
			}
			if(playerDir==2 && (vanMain.myGame.map01.isWall[corX+1][corY]==0))	
			{
				if(moveAmnt<=10)
				{
					vanMain.myGame.subMapOffset(moveSpeed, 0);
					moveAmnt++;
				}
				if(moveAmnt>=10)
					moveAmnt=0;
			}
			if(playerDir==3 && (vanMain.myGame.map01.isWall[corX][corY+1]==0))	
			{
				if(moveAmnt<=10)
				{
					vanMain.myGame.subMapOffset(0,moveSpeed);
					moveAmnt++;
				}
				if(moveAmnt>=10)
					moveAmnt=0;
			}
			if(playerDir==4 && (vanMain.myGame.map01.isWall[corX][corY-1]==0))	
			{
				if(moveAmnt<=10)
				{
					vanMain.myGame.addMapOffset(0,moveSpeed);
					moveAmnt++;
				}
				if(moveAmnt>=10)
					moveAmnt=0;
			}
			if(moveAmnt==0)
				playerDir=0;
			vanMain.myGame.startTime=System.currentTimeMillis();
		}
		
		
			


			//vanMain.myGame.map01.offsetY+=y;vanMain.myGame.map02.offsetY+=y;
			//vanMain.myGame.map03.offsetY+=y;vanMain.myGame.map04.offsetY+=y;
		//}
	}
	public int checkCollision()
	{
		int returnVar=0;
		int mapSize = vanMain.myGame.map01.x; 
		Layer tempLayer = vanMain.myGame.map01;
		for(int x = 0;x<=mapSize;x++)
		{
		for(int y = 0;y<=mapSize;y++)
		{
			// check right side
				int tileLeft = x*20 - tempLayer.offsetX;
				
				System.out.println("Tiles left side: "+tileLeft+ "Players leftside: "+x);
				if((tempLayer.isWall[x][y]==1) && ((this.x+10+tempLayer.offsetX)>=tileLeft))
				{
					returnVar=1;
					break;
				}
			}
		}
		return returnVar;
	}
	public void loadPlayerData()
	{
		Toolkit toolkit = Toolkit.getDefaultToolkit();
		playerImg[0]= toolkit.getImage("player1.png");
		//playerImg[1]= toolkit.getImage("player2.png");
		//playerImg[2]= toolkit.getImage("player3.png");
		//playerImg[3]= toolkit.getImage("player4.png");
	}
	public void drawPlayer(Graphics g)
	{
		g.drawImage(this.playerImg[0],(vanMain.myGame.getWidth()/2),6+vanMain.myGame.getHeight()/2,null);
	}
}
class Client
{
	int x,y;
	public Client(String name, int x, int y)
	{
		this.x=x;
		this.y=y;
	}
}
#2

First, your code isn’t that bad at all, especially if OOP is completly new to you.

Second, at first glance one of the problems would appear to be your use of repaint(). The way you’ve currently got it setup you won’t get regular updates because you’re calling plain old repaint(). repaint() with no parameters asks the AWT layer to repaint “as soon as possible”. If you ask for another repaint() before the thread has had a chance to action the first they just get compressed into one call.

While this works really nicely for GUI updates, for games it doesn’t work well. You might consider using repaint(0) which tells the thread that you want a repaint with 0 milliseconds (i.e. now!). However, you might also consider moving to an accelerated canvas where the graphics are directly under control and you utilise the system’s graphics hardware.

To toot my own trumper, here’s a tutorial on accelerated graphics in Java2D -
http://www.cokeandcode.com/info/tut2d.html

Kev

#3

Another advice I can give to you is to comment your code.
So you know what happen at any line of the code.

#4

Not bad. Maybe a bit more empty lines between methods would serve to make it easier to navigate. Also, your keyPressed method could use a switch, or if-else chaining (this would hardly affect efficiency in this case, but in general it would probably be better). What’s up with the String argument of the Client constructor (bottom)?

#5

Oh the Client I am still working on.

Thanks for the comments guys!

Im thinking about removing the Game class, as it serves no real purpose and instead using the vanMain class as the main class,

would look something like:


public class vanMain, extends Frame
{
       public static void main(String args[])
       {
              vanMain vm = new vanMain();
       }
       vanMain()
       {
              //init stuff goes here
              while(gameOn)
              {
                    this.repaint();
              }


       }
}
#6

One mreo comment.

If you really want to tune your code for best performance, start profiling!

There really is no substitute

#7

Yeah I took a look at JProfiler, I didn’t really understand any of the data it gave me though :-\

#8

Try the Netbeans profiler.

http://www.netbeans.org/servlets/NewsItemView?newsItemID=665

I think you’ll find it much clearer and easier to use.

You’ll need to install the MUstang preview and Netebans 4.1 to use it. Its all explaiend on that site.

#9

The keyboard handling just asks for trouble :slight_smile:


int [] controls = new int[0xFF];
[...]
public void keyPressed(KeyEvent ke)
	{
		controls[ke.getKeyCode()&0xff] = 1;
	}

	public void keyReleased(KeyEvent ke)
	{
		controls[ke.getKeyCode()&0xff] = 0;
	}

Moving the player can be than done as part of the logic like…


int x=controls[KeyEvent.VK_RIGHT]-controls[KeyEvent.VK_LEFT];
int y=controls[KeyEvent.VK_DOWN]-controls[KeyEvent.VK_UP];
move(x,y);

If left and right are pressed at the same time they nullify each other etc.