Solved

Solved
Thanks everyone :slight_smile:

Since you are not using active rendering, your paint method could be called any time by the event dispatch thread. So in the middle of your delete loop your paint method could start up. You have 2 alternatives. Synchronize on your IActors Collection or switch to active rendering.

“Synchronize on your IActors Collection or switch to active rendering”

Hmm, how do you synchronize or switch to active rendering?
(Sorry, I’m just learning about threads now, and never heard about those 2 options)

Ahhh, I tried this and my Actors disappeared

public synchronized void paint(Graphics2D g2) {
Graphics g = (Graphics) g2;
for (Actor aCurrentActorToDisplay : lActors) {
aCurrentActorToDisplay.getDisplayActor(g);
}
}

If you use active rendering then you don’t have to deal with threads because you do everything in the main thread.

With passive rendering you put your render code in the paint method and it gets called by the event dispatch thread which can interrupt the main thread at any time.

Are you saying it worked or it didn’t?

This is what I was suggesting.


public void removeActor() {
  synchronized(lActors) {
    for (int i = lActors.size() - 1; i > 0; i--)
      if (lActors.get(i).getHealth() <= 0) {
        lActors.remove(i);
      }
    }
  }
}


public void paint(Graphics2D g2) {
//This line is completely unnecessary
//Graphics g = (Graphics) g2;
  synchronized(lActors) {
    for (Actor aCurrentActorToDisplay : lActors) {
      aCurrentActorToDisplay.getDisplayActor(g);
    }
  }
}

Yeah, it didn’t work. When you create an army it doesn’t display it on the JFrame. Although when I used a toString() method, I found out the Actor object do exist. What should I look for to fix this problem?

Army Class

[quote]package army;

import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.PrintStream;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;

import util.InputGUI;
import util.InputGUI.ExhaustedInputAttempts;
import view.View;

import actors.Actor;
import actors.ActorFactory;

/**

  • Army class will manage a collection of Actor objects.

  • @author Rex Woollard
    /
    public class Army implements Serializable {
    /
    *

    /
    private static final long serialVersionUID = 1L;
    /
    * Reference to the String holding the name of the Army /
    private String sName;
    /
    * Reference to the Color object; used to identify Actor objects associated with this Army /
    @SuppressWarnings(“unused”)
    private Color cArmyColor;
    /
    * Collection to hold references to Actor objects. */
    private List lActors;
    private int nNextActorToMove;

    public Army(String sName, Color cArmyColor) {
    this.sName = sName;
    this.cArmyColor = cArmyColor;
    lActors = new ArrayList();
    nNextActorToMove = 0;
    } // end Army constructor

    /** Probes the collection to find the current number of Actor objects */
    public int getNumActors() {
    return lActors.size();
    }

    /** Returns a reference to the String object storing the Army name */
    public String getName() {
    return sName;
    }

    /**

    • On each call, adds more Actor objects to collection.

    • @param view

    •      used to set the boundaries on x,y values in Point2D coordinate objects.
      
    • @throws ExhaustedInputAttempts
      */
      public void createActors(View view) throws ExhaustedInputAttempts {
      boolean bIsAutomatic = true;
      int nNumActors = InputGUI.getIntGUI(sName + “: Number of Actors to Add:”, 1, 10000);

      for (int n = 0; n < nNumActors; ++n)
      lActors.add(ActorFactory.createNewActor(bIsAutomatic, view.getRandomLocation(getName()), getName()));
      view.repaint();
      } // end void createActors()

    /**

    • Displays text-oriented values of all Actor objects in the Army collection.
    • @param psOutputStream
    •      could receive <i>System.out</i> or an explicitly opened file stream on disk.
      

    */
    public void display(PrintStream psOutputStream) {
    for (Actor a : lActors)
    psOutputStream.println(a);
    } // end void display()

    /**

    • Method called by the paintComponent method in View.Panel.
    • @param g2
    •      reference to Graphics2D object that contains device context information
      
    • @return
      */
      public void paint(Graphics2D g2) {
      //This line is completely unnecessary
      Graphics g = (Graphics) g2;
      synchronized(lActors) {
      for (Actor aCurrentActorToDisplay : lActors) {
      aCurrentActorToDisplay.getDisplayActor(g);
      }
      }
      } // end void paint()

    /** Allows user to edit the values in a single Actor object, using an int value as the selection index */
    public void editOneActor() {
    try {
    lActors.get(InputGUI.getIntGUI(“Edit Actor”, 0, lActors.size() - 1)).setGUI();
    // Preceding line could be expanded to the following three lines. They do the same thing.
    // int nIndex = get(InputGUI.getIntGUI(“Edit Actor”, 0, lActors.size()-1));
    // Actor aActorToEdit = lActors.get(nIndex);
    // aActorToEdit.setGUI();
    } catch (InputGUI.ExhaustedInputAttempts e) {
    InputGUI.showErrorGUI(e.getMessage());
    }
    } // end editOneActor()

    public Actor getActiveActor() {
    if (nNextActorToMove >= lActors.size())
    nNextActorToMove = 0;

     return lActors.get(nNextActorToMove++);
    

    }

    public Actor findNearestActor(Actor aActorToMove) {
    int nTargetActor = 0;

     for (Actor a : lActors) {
     	if (aActorToMove.getLocation().distance(a.getLocation()) < aActorToMove.getLocation().distance(lActors.get(nTargetActor).getLocation())) {
     		nTargetActor = lActors.indexOf(a);
     	}
     }
     return lActors.get(nTargetActor);
    

    }

    public void saveToDisk() {
    try {
    ObjectOutputStream out = new ObjectOutputStream(new FileOutputStream(getName() + “.ser”));

     	// Magic starts here ******************************************************************
     	// Entire Student object is written to disk; all the related structural information is embedded in file
     	out.writeObject(this);
     	// Magic ends here ********************************************************************
    
     	out.close();
     } catch (Exception e) {
     	e.printStackTrace();
     }
    

    }

    public Army loadFromDisk() {
    Army army = null;
    // Using a try block in case there is a file I/O error
    try {
    ObjectInputStream in = new ObjectInputStream(new FileInputStream(getName() + “.ser”));

     	// Magic starts here ******************************************************************
     	// Student object is read from disk with all the structural information restored from file
     	army = (Army) in.readObject();
     	// Magic ends here ********************************************************************
    
     	in.close();
     } catch (Exception e) {
     	e.printStackTrace();
     }
     return army; // end case 2
    

    }

    public void removeActor() {
    synchronized(lActors) {
    for (int i = lActors.size() - 1; i > 0; i–)
    if (lActors.get(i).getHealth() <= 0) {
    lActors.remove(i);
    }
    }
    }

}// end class Army
[/quote]

[quote]package view;

import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.geom.Point2D;

import javax.swing.ImageIcon;
import javax.swing.JFrame;
import javax.swing.JPanel;

import objects.Tree;

import actors.Actor;

import simulator.Simulator;

/**

  • View is a type of JFrame. It manages the application window. A JPanel is placed inside. The JPanel is the canvas on which Actor objects are drawn.
  • @author Rex Woollard

*/
@SuppressWarnings(“serial”)
public class View extends JFrame {
// TODO Access specifiers . . .
ImageIcon background = new ImageIcon(“Images/background.png”);
private Simulator simulator;
Tree tree = null;

public View(int nWidth, int nHeight) {
	setSize(nWidth, nHeight);
	setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
	setTitle("Lord of the Rings: Battelfield Simulator");
	this.getContentPane().setBackground(Color.BLACK);
	add(new Panel());
	setVisible(true);
} // end View constructor

public void setSimulator(Simulator s) {
	simulator = s;
}

public Point2D getRandomLocation(String name) {
	double x = Math.random() * getWidth();
	double y = Math.random() * getHeight();
	
	if(y < 325)
		y =+ 325;
	
	if(name == "Red" && x > getWidth()/2)
		x =- getWidth()/2;
	
	if(name == "Blue" && x < getWidth()/2)
		x =+ getWidth()/2;
		
	
	
	
	return new Point2D.Double(x,y);
}

public void ensureLocationIsWithinBounds(Actor aActorToMove) {
	double dX = aActorToMove.getLocation().getX();
	double dY = aActorToMove.getLocation().getY();
	if (dX > getWidth() - 86)
		dX = aActorToMove.getLocation().getX() - 2.0;
	if (dX < 0.0)
		dX = 0.0;
	if (dY > getHeight() - 132)
		dY = aActorToMove.getLocation().getY() - 2.0;
	if (dY < 325.0)
		dY = 325.0;
	aActorToMove.spriteDirection();
	aActorToMove.getLocation().setLocation(dX, dY);
}

/** <i>Panel</i> is a type of <i>JPanel</i>. It is an inner class, so automatically has access to the parent class (<i>View</i>) instance variables, in particular <i>simulator</i>. */
public class Panel extends JPanel {
	/**
	 * void paintComponent: CALLBACK method is invoked by Operating System during PAINT processing
	 * @param g
	 * Graphics: contains device context information
	 */
	public void paintComponent(Graphics g) {
		super.paintComponent(g);
		g.drawImage(background.getImage(), 0, 0, null);
		Graphics2D g2 = (Graphics2D) g;
	}
}

}
[/quote]

I didn’t check the whole source code, but this

for (int i = lActors.size() - 1; i > 0; i--)

should be

for (int i = lActors.size() - 1; i >= 0; i--)

Perhaps I am missing the point…but…

It looks to me like it is not a threading issue at all.
It looks to me like he is removing an element from the data structure he is using in his for loop definition. That will cause concurrent mod exceptions for sure.

Change your loop to build a stack of actors to remove when you go through your loop.
Then, afterward, remove the stack from your first data structure.

Thanks guys!, I fixxed it :slight_smile:

I agree, and suggest using an Iterator as it can safely remove an element while traversing the list.

for (Iterator<Actor> iterator = lActors.iterator(); iterator.hasNext();) {
    Actor a = iterator.next();
    if (a.getHealth() <= 0) {
        iterator.remove();
    }
}

You should leave your original post after your problem is solved so it can help out other people who may be having the same problem…

[quote]You should leave your original post after your problem is solved so it can help out other people who may be having the same problem…
[/quote]
Oooops, sorry. I’m new to using forums still learning. I’ll keep that in mind for next time