ConcurrentModification Exception thrown up when spell cast

Started by EB5473, July 28, 2012, 01:40:38

Previous topic - Next topic

EB5473

Hello all!
I am currently developing a dynamic spell system and i am trying to get it working with a few different functions. One oif these, a really basic function, is to spawn a pig entity. To my dismay, it diddn't work, and instead threw a ConcurrentModification Exception. However, the same code worked in "Game" as a spawn key with a timer.
TL;DR:I'm trying to spawn a pig through a spell, but it is throwing up an exception.
Code: (sorry it's so long :-[ )
Game:
package main;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.Writer;
import java.rmi.server.UID;
import java.util.ConcurrentModificationException;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Random;
import java.util.Set;

import javax.xml.crypto.dsig.keyinfo.PGPData;

import main.gui.GuiHandle;
import main.gui.PlayerGUI;

import org.json.simple.JSONArray;
import org.json.simple.JSONObject;
import org.json.simple.JSONValue;
import org.lwjgl.Sys;
import org.lwjgl.input.Keyboard;

import paulscode.sound.SoundSystem;

import static org.lwjgl.input.Keyboard.*;


public class Game {
	public String name;
	public SaveGame saver = new SaveGame();
	public PlayerGUI pGui = new PlayerGUI(this);
	public Set<Entity> entities = new HashSet<Entity>();
	public Set<Entity> deleted = new HashSet<Entity>();
	public Random entitynumber;
	public int UID = 1;
	public boolean saveReady = true;
	public int saveCounter;
	public boolean spawnReady = true;
	public int spawnCounter;
	public int screenX = 1024;
	public int screenY = 768;
	public float px;
	public float py;
	public float ph;
	public int pm;
	
	
	JSONObject ents = new JSONObject();
	
	
	int fps;
	long lastFrame;
	long lastFPS;
	
	public void addEntity(Entity ent) {
		entities.add(ent);
	}
	
	public void setDeath(Entity ent) {
		if(ent.health <= 0) {
			ent.isDead = true;
			ent.health = 0;
		}
	}

	public void removeEntity(Entity ent) {
		spawnReady = false;
		entities.remove(ent);
		if(spawnCounter >= 5000)
		spawnReady = true;
	}
	
	public void tick() {
	
		if(isKeyDown(KEY_F)) {
			if(saveReady) {
				saveReady=false;
		for(Entity ent : entities) {	
			saver.save(ent, this);
			}
			saveCounter = 0;
		}
		}
		saveCounter++;
		spawnCounter++;
		if(saveCounter >= 5000) {
			saveReady = true;
		}
		if(spawnCounter >= 5000) {
			spawnReady = true;
		}
		
		
		
		for(Entity ent : entities) {
			ent.tick();
			if(ent.health <= 0) 
				{
				System.out.println(ent.name + " has died.");
				deleted.add(ent);
				}
		}
		
		
		for(Entity ent : deleted) {
			entities.remove(ent);
		}
		pGui.render();
	
	}
	
	
	public void doNothing() {
		
	}
	
	
	 public long getTime() {
		    return (Sys.getTime() * 1000) / Sys.getTimerResolution();
		}
	 
	 public void collisions() {
		
	 }
	 
	 public void cleanUp(){
		 for(Entity ent : entities) {
			 ent.cleanup();
		 }
	 }
	
	public Game(String name) {
		this.name = name;
	}

}


EntityPlayer:
package main;

import static org.lwjgl.input.Keyboard.KEY_G;
import static org.lwjgl.input.Keyboard.isKeyDown;
import static org.lwjgl.opengl.GL11.GL_QUADS;
import static org.lwjgl.opengl.GL11.glBegin;
import static org.lwjgl.opengl.GL11.glColor3f;
import static org.lwjgl.opengl.GL11.glEnd;
import static org.lwjgl.opengl.GL11.glVertex2f;

import org.lwjgl.input.Keyboard;

public class EntityPlayer extends EntityLogical {

	public SpellSpawnPig spell1 = new SpellSpawnPig(game);
	public EntityPlayer(Game game) {
		super("Player",game);
		spells.addSpell(spell1);
		maxHealth = 50;
		health = 50;
		r = 0;
		b = 1;
		g = 1;
		game.ph = health;
		selectSpell(spell1);
	}

	public void tick() {
			super.tick();
			if(Keyboard.isKeyDown(Keyboard.KEY_W)) {
				this.pos.y += 1f;
			}
			if(Keyboard.isKeyDown(Keyboard.KEY_S)) {
				this.pos.y -= 1f;
			}
			if(Keyboard.isKeyDown(Keyboard.KEY_A)) {
				this.pos.x -= 1f;
			}
			if(Keyboard.isKeyDown(Keyboard.KEY_D)) {
				this.pos.x += 1f;
			}
			spells.tick();
			regenerateVariables();
			if(isKeyDown(KEY_G)) {
				System.out.println("G");
				spells.cast(selectedSpell, this);
			}
			game.ph = health;
			game.pm = mana;
	}
	
	public void render() {
		super.render(r,g,b);
		
	}
}


SpellBook:
package main;

import static org.lwjgl.input.Keyboard.KEY_G;
import static org.lwjgl.input.Keyboard.isKeyDown;

import java.util.HashSet;
import java.util.Set;

public class SpellBook {

	public Set<SpellBase> spellList = new HashSet<SpellBase>();
	public Entity keeper;
	public SpellBook(Entity ent, Game game) {
		keeper = ent;
	}
	
	public void tick() {
		for(SpellBase spell : spellList) {
			spell.tick();
		}
	}
	
	public void cast(SpellBase spell,EntityLogical caster) {
		if(spellList.contains(spell)) {
			spell.onCast(caster);
		}
	}
	
	public void addSpell(SpellBase spell){
		if(!spellList.contains(spell)) {
		spellList.add(spell);
		System.out.println("Spell " + spell.name + " added to " + keeper.name);
		}
		else System.out.println(keeper.name + " already knows " + spell.name);
	}
	
}

SpellBase:
package main;

public class SpellBase {

	public String name;
	public Game game;
	public int cost;
	public int coolDown;
	public int curCool;
	
	public SpellBase(String name, Game game) {
		this.name = name;
		this.game = game;
		cost = 0;
		coolDown = 0;
		curCool = 0;
	}
	
	public void tick() {
		if(curCool <= coolDown && !(curCool <= 0)) {
			curCool--;
		}
	}
	
	public void onCast(EntityLogical caster){
		System.out.println("CAST");
		if(caster.mana >= cost && !caster.isDead && curCool == 0) {
		
			caster.mana -= cost;
			effect();
		}
		curCool = coolDown;
	}
	public void effect() {
		
	}
}


The actual command being used by "SpellSpawnPig" is
game.addEntity(new EntityPig(game,game.UID));

Again, sorry everything is so long  :-[
Does anyone have any ideas as to what the exception could be caused by?
Coder, Mastermind, Friend

CodeBunny

A ConcurrentModificationException is always thrown by non-synchronized JVM Collection classes when you try to modify their contents while iterating through them. They do this because modifying the contents suddenly makes it impossible to be sure that iteration will continue to be accurate in a variety of ways.

There's an easy way around this that will also give you more predictable behavior in other areas. Basically, maintain a queue of "modification" objects. Whenever you want to start an update (including your iteration), you would "lock" your game class, causing add or remove events to be stored into this queue. Then, once you finish your update, you "unlock" your game, which will then perform all stored operations.

EB5473

So I would lock the game class with a Boolean and wait until the iteration is done?
Coder, Mastermind, Friend

CodeBunny

Yup. It'd look something like:

private boolean locked;
public boolean locked() { return locked; }

// Flag all events to be queued
public void lock()
{
	locked = true;
}

// Process all queued events
public void unlock()
{
	if(locked)
	{
		while(!eventQueue.isEmpty())
			eventQueue.removeFirst().processEvent();
		locked = false;
	}
}

private LinkedList<Event> eventQueue;

public void add(Object obj)
{
	if(locked)
		eventQueue.addLast(new AddEvent(obj));
	else
	{
		//process add normally
	}
}

public void remove(Object obj)
{
	if(locked)
		eventQueue.addLast(new RemoveEvent(obj));
	else
	{
		//process remove normally
	}
}

public void update(float delta)
{
	lock();
	
	// Regular update code
	
	unlock();
}


It's not tailored to your stuff, but that's more or less usable code. You'll need to create an Event interface and the AddEvent and RemoveEvent class implementations, however.

EB5473

Looks good, thanks for your help! I'll post if I have any problems (like the dummy I am  :-X)
Coder, Mastermind, Friend

basil

locking with booleans is always a bit dangerous.

span synchronised or use a http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/locks/ReentrantLock.html around all read write spots to the sets lists and maps that came up with the exceptions. :)