Looking for a Code Review

[niroshido]

Hi everyone, Okay so i will start, i participated in my first Ludum Dare during the weekend, my entry was not great… this isn’t really what i am trying to ask.

So i made a promise to myself at the start of this year, make a Java game without the use of any libraries exception being Java’s standard API. So now that the year is at its end i am stretching and crackling my fingers to get this “game” done.

and its called NiroshidoRPG (very creative name!)
So let me review what i have done, nothing really, the game loop is far from pretty, effecient, good but its a loop so i suppose thats a good sign. The game accepts keyboard input, i suppose thats good.

So what is it i am trying to get to, well you see, i respect this community and with that, by extension i hope that people will review my code, analyse it and offer recommendations on improving my approach.

Now a few things, yes the code is lacking, its pretty much a rectangle that moves left, right, up and down, but it isn’t cleaning up after it moves. There is nothing else, but i wanted you to express your opinions before i go further with the development, as i am not the brightest lunch in the toolbox

http://pastebin.java-gaming.org/76a3b807679

[Opiop]

I would very much recommend using classes. As for your issue with the pixels not clearing, I believe you have to call super.draw or something in your render method before you render anything to clear the previous buffer. But I can’t remember what the exact function is. A quick Google would find the answer though!

[ctomni231]

I feel like it is a little unfair to push a code review on someone who is just trying to code underneath a time constraint. However, I guess a little bit of guidance wouldn’t hurt.

[icode] thread.sleep(1); [/icode]

Usually if you are using sleep, you want to make sure you set the number higher so the computer doesn’t burn all the process on running your game. The higher the number, the less CPU cycles it’ll take and the less you’ll hear your fan whirl in your computer. I usually set this value around 10 for quick games because it is very close to 60 frames per second…

Getters and Setters

I was never a fan of getters and setters because I came from coding in BASIC. Now, it doesn’t mean you shouldn’t use them… but I always believed they were a waste of time, energy, and they don’t even offer any protection from people who want to hack your code anyway. It is all just, useless. [/rant]

I think one of the most important things to ask is, “Does my code work?” You get a lot more done this way than just following coding conventions given to you. Writing good modular code is something that you can emulate from looking at the tutorials, or even looking at how Java structures its classes. Just emulate it, and you’ll get those answers. For something as quick as LD, I wouldn’t waste my time making it pretty. Just focus on getting it to work so you can submit it on time. The people playing the game will thank you once it is submitted.

Anyway, I wish you the best of luck as you continue your coding career. Looks are only important if you need someone else to “read” it

[saucymeatman]

@ctomni231
I disagree with you hate of getters/setters. They can be really usefull in many cases! I dont know about stopping hackers though…
Some examples of great uses of getters/setters
-When you want somthing to be gettable but not settable or vise versa
-When you want an opperation performed before setting/getting such as having an object in a “sleep” move until somthing changes its momentum. Before the change the setter could tell it to “wake up” per say

Hey i’ve noticed your doing something wrong in your code.

Your not supposed to override the “paint” method in JPanel. It is recommended that one overrides the “repaint” method. You wont notice a difference in code, but that should clear up your problem where its not redrawing properly. I dont remember why your supposed to do this, but when I was starting off that fixed a similar problem was having. Try it. I DARE YOU.

The best of luck to you!

[Opiop]

You’re supposed to repaint because that’s what clears the color bit? Pretty simple really…

Getters and setters aren’t necessary, but they are nice to use sometimes. You can totally do away with them if you want to though. Its more of a convention really because anyone can change code…

As for code “prettyness”, don’t worry about it during LD. My game is ok I guess, but its sloppy and I would be embaressed to show anyone my code in its current state.

[niroshido]

oh shi… sorry guys, okay Ludum Dare actually finished on Monday, i was (for nothing bloody reason) just pointing out i participated in LD, i did it in HTML5 and JavaScript. So sorry for making it sound confusing.

well great scott! it works!. I adjusted the Thread.sleep(1) from 1 to 10 (this reduces the flickering, it doesn’t get rid of it but it does reduce it ^^)

i also added a call to super.paint(g) which has now resulted in the rectangle moving around without leaving a trace behind.

i read saucymeanman’s comment [quote] It is recommended that one overrides the “repaint” method.
[/quote]
what do you mean by this, make a call to super.repaint create a new method that overrides repaint…

[saucymeatman]

I meant to override the JPanel’s repaint method instead of overriding the JPanels Paint method. It would automatically clear the screen for you. But if you clear it manually thats fine too.

[niroshido]

Hi, i realised i had not updated this in a bit and tbh my problem still exists. So i will give a tl;dr regarding my problem
I am encountering white screen flashes every so often, this is a pain in the arse for me tbh. I have implemented Eli’s game loop for the better part

[ctomni231]

Does this code segment flicker for you? (Copy, paste, and run in your IDE.)

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;
import java.awt.image.BufferedImage;
import java.util.Random;

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

public class SnowExample extends JComponent implements Runnable{

	private JFrame window;
	private BufferedImage bimg;
	
	public static void main(String args[]){
		SnowExample cool = new SnowExample();
		cool.showWindow();
	}
	
	/** How often the snow is produced */
	public final int FREQUENCY = 500;
	/** The maximum amount of particles allowed */
	public final int MAX_PARTICLE = 100;
	
	/** The random number generator */
	public Random rand;
	/** The type of snow ball to draw */
	public int[] type;
	/** The x-axis position of the snow ball */
	public double[] posx;
	/** The y-axis position of the snow ball */
	public double[] posy;
	/** Temporary variable */
	public int temp;
	
	public void init(){
		type = new int[MAX_PARTICLE];
		posx = new double[MAX_PARTICLE];
		posy = new double[MAX_PARTICLE];
		rand = new Random();
		for(int i = 0; i < MAX_PARTICLE; i++)
			type[i] = -1;
	}
	
	public void updateRender(Graphics2D g, int w, int h){
		
		//Create new snow particles
		for(int i = 0; i < MAX_PARTICLE; i++){
			if(type[i] == -1 && rand.nextInt(FREQUENCY) == 0){
				type[i] = rand.nextInt(3);
				posx[i] = (rand.nextInt((w+100)/10)*10) - 100;
				posy[i] = -10;
				break;
			}
		}
		
		//Snow particle updates
		for(int i = 0; i < MAX_PARTICLE; i++){
			if(type[i] == -1)
				continue;
			
			if(type[i] == 2)
				posx[i] += 0.5;

			posx[i] += 0.5;
			posy[i] += 2;
			
			//Destroy particles
			if(posy[i] > h)
				type[i] = -1;
		}
		
		//Render particles
		g.setColor(Color.WHITE);
		for(int i = 0; i < MAX_PARTICLE; i++){
			if(type[i] == -1)
				continue;
			if(type[i] == 0)
				g.fillOval((int)posx[i], (int)posy[i], 4, 4);
			else if(type[i] == 1)
				g.fillOval((int)posx[i], (int)posy[i], 6, 6);
			else if(type[i] == 2)
				g.fillOval((int)posx[i], (int)posy[i], 8, 8);
		}
	}
	
	/////////////////////////////////////
	//The infrastructure stuff under here...
	/////////////////////////////////////
	
	public SnowExample(){
		 window = new JFrame("Snow Demo");
        window.setBackground(Color.BLACK);
        setBackground(Color.BLACK);
        init();
	}
	
	public void showWindow(){
		 Thread looper = new Thread(this);
         looper.start();
		 window.add(this, BorderLayout.CENTER);
		 window.addWindowListener(new WindowAdapter() {
             @Override
              public void windowClosing(WindowEvent e) {
                  window.dispose();
                  System.exit(0);
              }
          });
	     window.validate();
	     window.setVisible(true);
	     window.pack();
	}
	
	@Override
    public void paintComponent(Graphics g){
        super.paintComponent(g);
        createGraphics2D((Graphics2D)g, getSize().width, getSize().height);
        //Draws a non-flickering image
        g.drawImage(bimg, 0, 0, this);
    }
	
	private void createGraphics2D(Graphics2D g2, int w, int h) {
        if (bimg == null || bimg.getWidth() != w || bimg.getHeight() != h)
            bimg = (BufferedImage) createImage(w, h);

        g2 = bimg.createGraphics();
        g2.setColor(Color.BLACK);
        g2.fillRect(0, 0, w, h);
        
        updateRender(g2, w, h);
    }
	
	@Override
    public Dimension getPreferredSize(){
        return new Dimension(400, 300);
    }
	
	@Override
	public final void run() {
        try{           
            while(true){
                Thread.sleep(10);
                repaint();
            }
        }catch(Exception e){
            System.err.println(e.getMessage());
            System.exit(0);
        }
    }

}

If it doesn’t then you can consider using this as a template. If it does, then it might be a graphics card issue :P.

[niroshido]

I have copied and pasted your code, it looks nice, i am going to be analysing it for a while. Thanks for your help.