null objects in ArrayList

Hey guys,

I am getting an occasional null pointer exception in my games ObjectManager class. I can’t seem to find a pattern as to why this is happening and could use some help. Every object in my game is added and removed by this ObjectManager class by checking the objects alive property. This property is set in collision methods when two objects collide so that the ObjectManager knows when to remove them.

Below is the ObjectManager update method:


public void update()
	{
		GameObject a, b;
		
		// Update the objects and cull the dead
		
		for(int i=0; i<objects.size(); i++)
		{
		
			objects.get(i).update();
			
			if(objects.get(i).isAlive() == false)
			{
				objects.get(i).destroy();
				objects.remove(i);
		
			}
			
		}
		
		// Check for collision 
		
		for(int i=0; i<objects.size(); i++)
		{
			for(int j=i; j<objects.size(); ++j)
			{
				a = objects.get(i);
				b = objects.get(j);
				
				if(a == b) continue;
				
				// Finally do the test and alert both objects
				
				if(collision(a,b))
				{
					a.collision(b);
					b.collision(a);
				}
				
				
			}
			
		}
		
		
	}

Here is my relevant stack trace:


Exception in thread "Thread-3" java.lang.NullPointerException
	at shooter.engine.ObjectManager.collision(ObjectManager.java:110)
	at shooter.engine.ObjectManager.update(ObjectManager.java:83)
	at shooter.game.PlayScreen.update(PlayScreen.java:33)
	at shooter.engine.ScreenManager.update(ScreenManager.java:15)
	at shooter.engine.GamePanel.update(GamePanel.java:137)
	at shooter.engine.GamePanel.run(GamePanel.java:127)
	at java.lang.Thread.run(Unknown Source)


Here is the collision method from the stack trace:


	private boolean collision(GameObject a, GameObject b)
	{
			
		double dx = a.getX() - b.getX();
		double dy = a.getY() - b.getY();
		
		double dist = Math.sqrt(dx * dx + dy * dy);
		if(dist < a.getR() + b.getR()) return true;
		
		return false;
	}

The line that it’s pointing to is in the collision method. I don’t see how it could be null because “dead” objects are removed before checking for collision. The crash doesn’t always occur in the collision method, sometimes it occurs in the update method posted above. Any ideas? I would be happy to post more code if needed.

Make sure you aren’t adding null objects to the arraylist.

I’m not. I’m only adding with the new operator. ???

objects.remove(i);

If you do so, the list size is one object shorter.
This should not be a problem since you are always checking objects.size() again, but you are at least skipping one object.
A little “i–” is necessary.

What’s your destroy() look like?

Also, some quick tips:
Try to have variables in the narrowest possible scope, meaning, don’t hoist your GameObject a, b out of the loop.

You don’t have to test boolean expressions with == false, so just if(!objects.get(i).isAlive()) { ...
Also just return the result of a boolean expression: return dist < a.getR() + b.getR();

Edit: Drenius is right, you need remove(i--); or else it will skip objects. Alternatively, you can iterate backwards.

For my Enemy class:


@Override
	public void destroy()
	{
		SoundManager.play("explosion");
		ObjectManager.add(new Explosion(x,y));
		
		// Type 4 and 5 enemies break into smaller types
		
		if(type == MAMA_YELLOW)
		{
			ObjectManager.add(new Enemy(x,y,BABY_YELLOW));
			ObjectManager.add(new Enemy(x,y,BABY_YELLOW));
			ObjectManager.add(new Enemy(x,y,BABY_YELLOW));
		}
		
		if(type == MAMA_MAGENTA)
		{
			for(int i=0; i<10; i++)
			{
				ObjectManager.add(new Enemy(x,y,BABY_MAGENTA));
			}
		}
		
		// Chance for powerup
		
		int chance = (int) (Math.random() * 100);
		
		if(chance < 5) ObjectManager.add(new PowerUp(x,y,PowerUp.EXTRA_LIFE));
		else if(chance < 20) ObjectManager.add(new PowerUp(x,y,PowerUp.SLOW_DOWN));
		else if(chance < 60) ObjectManager.add(new PowerUp(x,y,PowerUp.POWER));
		
	}

The destroy method was intended to allow one object to “change” into another. Most of the logic for setting an object as “dead” occurs in the collision events.

EDIT: @BurntPizza: Thanks for the info on i–. I’ve often wondered why people do this. This doesn’t seem to be the cause of the problem though as the crash occurs with or without it.

Any other ideas? This is annoying bug that’s slowing my progress on the game and I’m not even sure how to start debugging it. Sometimes I can play through the game 10 times without incident and then suddenly the crash occurs.

perhaps put in some checks for null pointers and print an error instead of calling the desired function, then you might be able to see whats going wrong without it actually crashing.

Could this be some sort of multi threading issue? I am posting my GamePanel class below:


package shooter.engine;

import java.awt.Canvas;
import java.awt.Color;
import java.awt.Cursor;
import java.awt.Dimension;
import java.awt.Graphics2D;
import java.awt.RenderingHints;
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 java.awt.image.BufferStrategy;

import shooter.game.PlayScreen;

@SuppressWarnings("serial")
public class GamePanel extends Canvas implements Runnable, KeyListener, MouseMotionListener, MouseListener
{
	public static final String GAME_NAME = "SHOOTER v1.0";
	public static final int WIDTH = 1024;
	public static final int HEIGHT = 768;
	
	private Thread thread;
	private boolean running; 
	
	private int FPS = 60;
	private int targetTime = 1000 / FPS;
	
	private Graphics2D g;
	private BufferStrategy bs;
	
	private ScreenManager scm;
	private GameLoop gameLoop;
   
	public GamePanel()
	{
		super();
		setPreferredSize(new Dimension(WIDTH, HEIGHT));
		setFocusable(true);
		requestFocus();
		setCursor(Cursor.getPredefinedCursor(Cursor.CROSSHAIR_CURSOR));
	}
	
	private void init()
	{
		running = true;
		
		createBufferStrategy(2);
		bs = getBufferStrategy();
		setIgnoreRepaint(true);
		
		Graphics2D gTemp = (Graphics2D) bs.getDrawGraphics();
		
		gTemp.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON);
		gTemp.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING, RenderingHints.VALUE_TEXT_ANTIALIAS_ON);
		
		scm = new ScreenManager();
		scm.changeScreen(new PlayScreen(scm));
		
		gameLoop = new GameLoop(targetTime);
		
		// Load game sounds
		
		SoundManager.add("laser");
		SoundManager.add("explosion");
		SoundManager.add("powerup");
		SoundManager.add("hit");
		SoundManager.add("powerup");
		SoundManager.add("boost");
		
	}
	
	@Override
	public void addNotify()
	{
		super.addNotify();
		addKeyListener(this);
		addMouseListener(this);
		addMouseMotionListener(this);
		init();
		
		// THREAD START
		if(thread == null)
		{
			thread = new Thread(this);
			thread.setDaemon(true);
			thread.start();
		}
	}

	@Override
	public void run() 
	{
		/*long startTime;
		long urdTime;
		long waitTime;
			
		while(running)
		{
			startTime = System.nanoTime();
			
			update();
			draw();
			Toolkit.getDefaultToolkit().sync();
			
			urdTime = (System.nanoTime() - startTime) / 1000000;
			waitTime = (long) (targetTime - urdTime);
			
			if(waitTime < 0) waitTime = 5;
			try
			{
				Thread.sleep(waitTime);
			}
			
			catch(Exception e) 
			{
				e.printStackTrace();
			}
		}*/
		
		while(running)
		{
			if(gameLoop.redraw())
			{
				update();
				draw();
				gameLoop.setRedraw(false);
			}
				
		}	
	}
	
	private void update()
	{
		scm.update();
	}
	
	private void draw()
	{
		g = (Graphics2D) bs.getDrawGraphics();
		g.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON);
		g.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING, RenderingHints.VALUE_TEXT_ANTIALIAS_ON);
		
		// Clear the screen
		
		g.setColor(Color.BLACK);
		g.fillRect(0, 0, WIDTH, HEIGHT);
		
		// Draw game content here
		
		scm.draw(g);
		
		// Render to the screen
		
		g.dispose();
		bs.show();
	}
	
	@Override
	public void keyPressed(KeyEvent key) 
	{
		int keyCode = key.getKeyCode();
		scm.keyPressed(keyCode);
	}

	@Override
	public void keyReleased(KeyEvent key) 
	{
		int keyCode = key.getKeyCode();
		scm.keyReleased(keyCode);
	}

	@Override
	public void keyTyped(KeyEvent key) 
	{
		
	}

	@Override
	public void mouseDragged(MouseEvent e) 
	{
		scm.mouseMoved(e);
	}

	@Override
	public void mouseMoved(MouseEvent e) 
	{
		scm.mouseMoved(e);
	}

	@Override
	public void mouseClicked(MouseEvent e)
	{
		
	}

	@Override
	public void mouseEntered(MouseEvent e)
	{
		
	}

	@Override
	public void mouseExited(MouseEvent e)
	{
		
	}

	@Override
	public void mousePressed(MouseEvent e)
	{
		scm.mousePressed(e);
	}

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


How often/when does the error occur?

It’s pretty rare and random which is what makes it so annoying. At one point I thought that it seemed to occur when there were many objects on the screen at once, but there’s no definite pattern that I can find.

EDIT: At no point in the game do I actually set any object to null, I only set it’s alive boolean to false. In my mind, this means that only the ObjectManager class will remove objects from the list which I guess is where garbage collection comes in.

Yep, typical symptoms of a race condition, aka a multithreading issue like you said. I’m pretty busy right now, but I’ll try to find it here in a sec.

Thanks I appreciate your time. The GamePanel is my engine which runs on it’s own thread. The GameLoop class is just a class which extends java.util.Timer and java.util.TimerTask to tell me when to update and redraw. It doesn’t do any heavy lifting and was just an experiment because I wasn’t happy with the commented game loop code you see above. I should mention that the bug occurred even without a third thread (i.e. the GameLoop class).

I have yet to find an “ideal” game loop that works for me without occasional lag, but that’s another story. :wink:

I know I’m not helping, but damn man. Three threads for what? That’s just over the top, you don’t need that many and you will get issues like this from using too many threads.

You should always avoid using iteration through a list that can be modified in another Thread. You could try to clone the list or adding everything to remove to a “toRemove” list so you could manage it someway…
But @opiop64:
Yep, normally multithreading is not even necessary.

@opiop64: Well I’m assuming there is three based on the output of the stack trace. The application itself, the GamePanel class, and the GameLoop class which as I mentioned is just an experiment and the bug occurred before I implemented that.

@Drenius should I remove the Runnable interface altogether? I’m actually more experienced with LWJGL than I am with Java 2D, but every tutorial/example code I’ve ever seen implements Runnable.

I would recommend several things:

‘Proper’ game loops: it’s a bit old, but this is alright: http://www.java-gaming.org/topics/game-loops/24220/view.html
Stay away from Timer/TimerTask, System.nanoTime() is all you ever need.

Read the Oracle tutorials about Swing and it’s threading model, and also the tuts on concurrency. They will make you a better programmer.

@BurntPizza: I have read that post and found it interesting. My issue seemed to be due to a bad implementation of the Thread.sleep on windows platform (or so I’ve read) which caused the sleep time to be innacurate so I just wanted to see how the game would run from a steady timer “tick” if you will. Also, I’m not using swing with the exception of JFrame I suppose.

It’s pretty much a non-issue, but of course you can code defensively, compensate for any inaccuracies.
Also, I would not be at all surprised if TimerTask uses sleep().
Apparently, ScheduledThreadPoolExecutor is good, although I have not used it.