SRL Archive - General

Having trouble getting a object to disappear after it collides with another, instead of deleting the object the player collides with it deletes all the objects

This method loops through the var 'objects' and checks if the box of the objects collides with the player box and removes it:

public void checkCollision() {
		for (int i = 0; i < objects.size(); i++) {
			if (objects.get(i).getCurrentBounds() == null)
				return;
			else {
				if (objects.get(i).getCurrentBounds().collision(p.bounds)) {
					handleScore(objects.get(i).fileName);
					objects.remove(i);
					return;
				}
			}
		}
	}

I am using Java array list

	public ArrayList<GameObject> objects = new ArrayList<GameObject>();

Why does objects.remove(int index) remove all of the objects?

You need to throw some debugging in there to see how many times it gets called. Also, you should really use Java 8 forEach.

I doubt the problem is in this snippet. I'd guess that either handleScore or collision calls checkCollision and something goes wrong! Easiest way to figure out what's up is by using a breakpoint. If you show some more code, can probably pinpoint the location of the bug.

By the way, you don't need that else.

From: tls
You need to throw some debugging in there to see how many times it gets called. Also, you should really use Java 8 forEach.

In OP's case, a regular for loop either (for (Object O : Objects) or for (int I = 0; I < size; I++)) would be the only thing he can use, personally i would do it exactly how he has done it. forEach is useless in his scenario because he can only use the forEach with final variables. IMO forEach is bad practice for efficiency. Only really good when trying to keep things simple.

OP, should you be calling return or continue?

@Kasi; @akarigar; @tls;

Here is gameLoop.java where I suspect the issue is:

package main;

import java.applet.Applet;
import java.awt.Graphics;
import java.awt.Image;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.awt.image.BufferedImage;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;

import javax.imageio.ImageIO;

public class gameLoop extends Applet implements Runnable, KeyListener {

	static final long serialVersionUID = 1L;
	public Image offscreen;
	public Graphics d;
	public boolean up, down, left, right;
	public BufferedImage background, foreground, player;
	public int counter, score, x, y, width, height, health, speed, level;
	public Long lastTime;
	public Box bounds;
	public Player p = new Player(null);
	public ArrayList<GameObject> objects = new ArrayList<GameObject>();
	GameObject spike = new GameObject(200, 400, 10, 20, 20,
			5000, "fire.png");
	GameObject mj = new GameObject(Rand.range(200, 400), Rand.range(400, 450), 20, 20, 10,
			5000, "plant.png");

	public void addObjectList(GameObject... object) {
		for (int i = 0; i < object.length; i++)
			objects.add(object[i]);
	}

	public void deleteObject(int index) {
		objects.remove(index);
	}

	public void updateObjectPositions() {
		for (int i = 0; i < objects.size(); i++) {
			switch (objects.get(i).fileName) {
			case "fire.png": {
				objects.get(i).x--;
				break;
			}
			case "plant.png": {
				objects.get(i).x--;
				if (Rand.range(0, 8) == 1) {
					if ((objects.get(i).y < 473) && (objects.get(i).y > 356))
						objects.get(i).y = objects.get(i).y
								+ (int) ((Math.sin(Rand.integer(5)) + Math
										.cos(Rand.integer(5))) * 3);
				}
				break;
			}
			}
			if (objects.get(i).x < -2) {
				objects.remove(i);
				return;
			}
		}
	}
	
	public void addObjectWorld(GameObject g) {
		objects.add(g);
		
	}
	
	public void updateObjectBoxes() {
		for (int i = 0; i < objects.size(); i++) {
			int x1 = objects.get(i).x;
			int y1 = objects.get(i).y;
			int x2 = objects.get(i).x + objects.get(i).w;
			int y2 = objects.get(i).y + objects.get(i).h;
			GameObject.setCurrentBounds(new Box(x1, y1, x2, y2));
			if (objects.get(i).fileName == "plant.png")
				System.out.println(objects.get(i).getCurrentBounds().x1);
		}
	}
	
	public void updatePlayerBounds() {
		p.bounds = new Box(x, y, x + width, y + height);	
	}
	
	public void handleScore(String objectName) {
		switch(objectName) {
			case "plant.png": {
				health += 5;
				score += Rand.range(0, 28);
				break;
			}
		}
		
	}
	public void checkCollision() {
		for (int i = 0; i < objects.size(); i++) {
			if (objects.get(i).getCurrentBounds() == null)
				return;
			else {
				if (objects.get(i).getCurrentBounds().collision(p.bounds)) {
					handleScore(objects.get(i).fileName);
					objects.remove(i);
					return;
				}
			}
		}
	}

	public void addRandomObjects() {

	}

	public void run() {
		level = 1;
		x = 100;
		y = 400;
		width = 20;
		height = 26;
		health = 10000;
		addObjectList(spike, mj);
		try {
			background = ImageIO.read(new File("main/background.png"));
			foreground = ImageIO.read(new File("main/foreground.png"));
			//player = ImageIO.read(new File("main/player.png"));
			p.playerSprite = ImageIO.read(new File("main/player.png"));
		} catch (IOException e1) {
			e1.printStackTrace();
		}

		while (true) {
			updatePlayerBounds();
			level = ((int) score / 1000) + 1;
			counter++;
			if (Rand.range(0, 1000000) == 5) {
				addObjectList(new GameObject(Rand.range(200, 400), Rand.range(400, 450), 20, 20, 10,
						5000, "plant.png"));
			}
			if (counter > 10) {
				//health -= 1;
				counter = 0;
				updateObjectPositions();
				updateObjectBoxes();
			}
			if (p.bounds != null)
				checkCollision();
			//if ((System.currentTimeMillis() - lastTime) > (5000) {
				
			//}
			if (y <= 356)
				up = false;
			if (x > 845)
				right = false;
			if (x < 1)
				left = false;
			// if ( y <= 348 && jump != true) {
			// y+=4;
			// }
			if (left == true) {
				x--;
			}
			if (right == true) {
				x++;
			}
			if (up == true) {
				y--;
			}
			if (down == true) {
				y++;
			}
			if (y > 473) {
				y = 473;
			}
			repaint();
			try {
				Thread.sleep(6 + speed);
			} catch (InterruptedException e) {
				e.printStackTrace();
			}
		}
	}

	public void keyPressed(KeyEvent e) {
		if (e.getKeyCode() == 37) {
			left = true;
		}
		if (e.getKeyCode() == 38) {
			up = true;
		}
		if (e.getKeyCode() == 39) {
			right = true;
		}
		if (e.getKeyCode() == 40) {
			down = true;
		}

	}

	public void keyReleased(KeyEvent e) {
		if (e.getKeyCode() == 37) {
			left = false;
		}
		if (e.getKeyCode() == 38) {
			up = false;
			counter = 4;
		}
		if (e.getKeyCode() == 39) {
			right = false;
		}
		if (e.getKeyCode() == 40) {
			down = false;
		}
	}

	public void keyTyped(KeyEvent e) {

	}
}

I know there is a lot of sloppy code but I am just trying to fix this bug before I go any further

[spoiler=Game.java]

package main;

import java.awt.Graphics;


public class Game extends gameLoop {
	
	private static final long serialVersionUID = 1L;
	
	public void init() {
		setSize(854, 480);
		Thread main = new Thread(this);
		main.start();
		offscreen = createImage(854, 480);
		d = offscreen.getGraphics();
		addKeyListener(this);
	}
	
	public void paint(Graphics g) {
		d.clearRect(0,  0,  854,  480);
		d.drawImage(background,  0,  0, this);
		d.drawImage(foreground,  0, 356,  this);
		for (int i = 0; i < objects.size();i++) {
			d.drawImage(objects.get(i).Img, objects.get(i).x, objects.get(i).y, this);
			d.drawString("Object: " + objects.get(i).fileName + " " + objects.get(i).x + "," + objects.get(i).y, 2, 75 + (i *15));
		}
		d.drawImage(p.playerSprite, x, y, this);
		d.drawString("Scores acquired: " + score, 2,  15);
		//if (objects != null) 
			//d.drawString("Objects: " + objects.size, 2,  30);
		d.drawString("Health: " + health, 2,  45);
		d.drawString("Position:" + x + "," + y, 2, 60);
		g.drawImage(offscreen,  0,  0,  this);
	}
	public void update(Graphics g) {
		paint(g);
	}

}

[/spoiler]
[spoiler=GameObject.java]

package main;

import java.awt.image.BufferedImage;
import java.io.File;
import java.io.IOException;
import javax.imageio.ImageIO;

 class GameObject {
	
	public int x, y, w, h;
	private static Box currentBounds;
	public final int damage, value;
	public final String fileName;
	public BufferedImage Img;
	
	public GameObject(int x, int y, int w, int h, int damage, int value, String fileName) {
		this.x = x;
		this.y = y;
		this.w = w;
		this.h = h;
		this.damage = damage;
		this.value = value;
		this.fileName = fileName;
		try {
			this.Img = ImageIO.read(new File("main/" + fileName));
		} catch (IOException e) {
			this.Img = null;
			System.out.println("Error loading image");
		}
	}

	public static Box getCurrentBounds() {
		return currentBounds;
	}

	public static void setCurrentBounds(Box currentBounds) {
		GameObject.currentBounds = currentBounds;
	}	
	
	
}

[/spoiler]

class GameObject {
  ...
  private static Box currentBounds;
  ...
}

This might have something to do with your problem.

From: akarigar

class GameObject {
  ...
  private static Box currentBounds;
  ...
}

This might have something to do with your problem.

Fixed that and made it public still having the issue, and I know that it has to be deleting all of the objects because the debug no longer shows objects once I collide with plant.png

Do you have skpye or something? Since we're both on it'll be easier.

just debug everything so you know where the bug is?

From: Robert
Fixed that and made it public still having the issue, and I know that it has to be deleting all of the objects because the debug no longer shows objects once I collide with plant.png

Wait, the problem wasn't that it was private, that was fine. The problem was that it was static.

From: hoodz
just debug everything so you know where the bug is?

it is debugged, i collide with the object and they all disappear it's probably a issue with the way I delete the arraylist

From: Robert
it is debugged, i collide with the object and they all disappear it's probably a issue with the way I delete the arraylist

Is this issue still occurring without the static bounds? I can take a closer look if you confirm.

As pointed out, the answer is because you have "bounds" static.. All objects will have the same bounds and so all objects with those bounds will be removed.

From: Kasi
forEach is useless in his scenario because he can only use the forEach with final variables. IMO forEach is bad practice for efficiency. Only really good when trying to keep things simple.

False.. entirely.. Sorry :l


Java 8's forEach is semantically equivalent to Java 7's for ranged and for loop.. but faster..

for (char c : l) {
   foo(c);
}

for (int i = 0; i < l.size(); ++i) {
   foo(l.get(i));
}

l.forEach((c) -> foo(c));

l.stream().forEach((c) -> foo(c));
l.stream().forEach((c) -> {foo(c);}); //wrapped in an anonymous lambda.. same performance.

nothing has to be final at all.. l.stream.forEach performs the fastest. l.forEach second fastest. Other two tradition for and for-ranged perform the same but less that the aforementioned.

The for each is similar to:

final Consumer<Char> con = new Consumer<Char>() {
    @Override
    public void accept(Char c) {
        foo(c)
    }
};
for (Char c : l) {
    con.accept(c);
}

In addition there is a for loop for multi-core processing & multi-threading & performance:

l.parallelStream().forEach((c) -> foo(c));

All in all, there's no loss and only gains. :l You can test and see that it doesn't have to be final.

Yes, akarigar helped me fix the issue over TV much appreciated

From: Brandon
Java 8's forEach is semantically equivalent to Java 7's for ranged and for loop.. but faster..

This isn't strictly true, is it? In typical cases you can assume that it is exactly equivalent to a sequential for loop, but isn't one of the design goals of it to abstract that away from the loop? That way any library writer can do the logic for the forEach loop however they want, which is more work with the typical

for (i : a)

syntax.

To be clear, I don't know if I'm correct. I'm only mentioning this because the possible behaviors are not (as far as I can tell) semantically equivalent for all situations. Although not related to the problems Robert was having, it is an interesting design choice and one that is very powerful.

Source(s): http://stackoverflow.com/questions/16635398/java-8-iterable-foreach-vs-foreach-loop (and other resources linked in the discussion)

From: i luffs yeww
This isn't strictly true, is it? In typical cases you can assume that it is exactly equivalent to a sequential for loop, but isn't one of the design goals of it to abstract that away from the loop? That way any library writer can do the logic for the forEach loop however they want, which is more work with the typical

for (i : a)

syntax.

To be clear, I don't know if I'm correct. I'm only mentioning this because the possible behaviors are not (as far as I can tell) semantically equivalent for all situations. Although not related to the problems Robert was having, it is an interesting design choice and one that is very powerful.

Source(s): http://stackoverflow.com/questions/16635398/java-8-iterable-foreach-vs-foreach-loop (and other resources linked in the discussion)

I was under the impression that it is semantically the same.. See the following..


One - for (char c : l):

       0: aload_0
       1: invokespecial #1                  // Method java/lang/Object."<init>":
()V
       4: aload_1
       5: invokevirtual #2                  // Method java/util/ArrayList.iterat
or:()Ljava/util/Iterator;
       8: astore_2
       9: aload_2
      10: invokeinterface #3,  1            // InterfaceMethod java/util/Iterato
r.hasNext:()Z
      15: ifeq          38
      18: aload_2
      19: invokeinterface #4,  1            // InterfaceMethod java/util/Iterato
r.next:()Ljava/lang/Object;
      24: checkcast     #5                  // class java/lang/Character
      27: invokevirtual #6                  // Method java/lang/Character.charVa
lue:()C
      30: istore_3
      31: iload_3
      32: invokestatic  #7                  // Method testfor/Testfor.foo:(C)V
      35: goto          9
      38: return

Two - for (int i = 0; i < l.size(); ++i):

       0: aload_0
       1: invokespecial #1                  // Method java/lang/Object."<init>":
()V
       4: iconst_0
       5: istore_2
       6: iload_2
       7: aload_1
       8: invokevirtual #2                  // Method java/util/ArrayList.size:(
)I
      11: if_icmpge     34
      14: aload_1
      15: iload_2
      16: invokevirtual #3                  // Method java/util/ArrayList.get:(I
)Ljava/lang/Object;
      19: checkcast     #4                  // class java/lang/Character
      22: invokevirtual #5                  // Method java/lang/Character.charVa
lue:()C
      25: invokestatic  #6                  // Method testfor/Testfor.foo:(C)V
      28: iinc          2, 1
      31: goto          6
      34: return

Three - l.forEach(© -> foo©):

       0: aload_0
       1: invokespecial #1                  // Method java/lang/Object."<init>":
()V
       4: aload_1
       5: invokedynamic #2,  0              // InvokeDynamic #0:accept:()Ljava/u
til/function/Consumer;
      10: invokevirtual #3                  // Method java/util/ArrayList.forEac
h:(Ljava/util/function/Consumer;)V
      13: return

Four - l.stream().forEach(© -> foo©):

       0: aload_0
       1: invokespecial #1                  // Method java/lang/Object."<init>":
()V
       4: aload_1
       5: invokevirtual #2                  // Method java/util/ArrayList.stream
:()Ljava/util/stream/Stream;
       8: invokedynamic #3,  0              // InvokeDynamic #0:accept:()Ljava/u
til/function/Consumer;
      13: invokeinterface #4,  2            // InterfaceMethod java/util/stream/
Stream.forEach:(Ljava/util/function/Consumer;)V
      18: return

java.util.ArrayList.forEach:

  public void forEach(java.util.function.Consumer<? super E>);
    Code:
       0: aload_1
       1: invokestatic  #331                // Method java/util/Objects.requireN
onNull:(Ljava/lang/Object;)Ljava/lang/Object;
       4: pop
       5: aload_0
       6: getfield      #283                // Field modCount:I
       9: istore_2
      10: aload_0
      11: getfield      #287                // Field elementData:[Ljava/lang/Obj
ect;
      14: checkcast     #11                 // class "[Ljava/lang/Object;"
      17: astore_3
      18: aload_0
      19: getfield      #284                // Field size:I
      22: istore        4
      24: iconst_0
      25: istore        5
      27: aload_0
      28: getfield      #283                // Field modCount:I
      31: iload_2
      32: if_icmpne     58
      35: iload         5
      37: iload         4
      39: if_icmpge     58
      42: aload_1
      43: aload_3
      44: iload         5
      46: aaload
      47: invokeinterface #334,  2          // InterfaceMethod java/util/functio
n/Consumer.accept:(Ljava/lang/Object;)V
      52: iinc          5, 1
      55: goto          27
      58: aload_0
      59: getfield      #283                // Field modCount:I
      62: iload_2
      63: if_icmpeq     74
      66: new           #178                // class java/util/ConcurrentModific
ationException
      69: dup
      70: invokespecial #330                // Method java/util/ConcurrentModifi
cationException."<init>":()V
      73: athrow
      74: return

https://docs.oracle.com/javase/8/docs/api/java/lang/Iterable.html

states:

forEach

default void forEach(Consumer<? super T> action)
Performs the given action for each element of the Iterable until all elements have been processed or the action throws an exception. Unless otherwise specified by the implementing class, actions are performed in the order of iteration (if an iteration order is specified). Exceptions thrown by the action are relayed to the caller.
Implementation Requirements:
The default implementation behaves as if:


for (T t : this)
action.accept(t);

Parameters:
action - The action to be performed for each element
Throws:
NullPointerException - if the specified action is null
Since:
1.8

In other words: `java.util.ArrayList.forEach` is just a wrapper around `for (T t : L)`. Since `for (T t : L)` is a wrapper around `for (int i = 0; i < size; ++i)` OR `while(it.hasNext()) it.next()` then they are equivalent to eachother; no?

@Brandon; I shall compare bytecode myself when I get to my computer, (ill use ArrayLists with both forEach and for-each). I was under the impression that they were similar. Unless you use parallel/streams. I had the assumption that he didn't wanna use lambdas because he wanted to edit the the objects value / remove it, (sorta made a typo with lambdas and forEach in my original post).

Offtopic: Where you been, haven't spoke in a good couple months man??!

From: Kasi
@Brandon; I shall compare bytecode myself when I get to my computer, (ill use ArrayLists with both forEach and for-each). I was under the impression that they were similar. Unless you use parallel/streams. I had the assumption that he didn't wanna use lambdas because he wanted to edit the the objects value / remove it, (sorta made a typo with lambdas and forEach in my original post).

Offtopic: Where you been, haven't spoke in a good couple months man??!

:S Is my bytecode not enough?
To get the bytecode I used the command (jdk1.8.0_25 x64):

javap -c com.testfor.TestFor
javap -c java.util.ArrayList

etc..

I'm always online on skype. Iunno..

From: Brandon
:S Is my bytecode not enough?
To get the bytecode I used the command (jdk1.8.0_25 x64):

javap -c com.testfor.TestFor
javap -c java.util.ArrayList

etc..

I'm always online on skype. Iunno..

I was gonna use OP's source mate.

From: Brandon
In other words: `java.util.ArrayList.forEach` is just a wrapper around `for (T t : L)`. Since `for (T t : L)` is a wrapper around `for (int i = 0; i < size; ++i)` OR `while(it.hasNext()) it.next()` then they are equivalent to eachother; no?

From your quote from the docs (which is also the source I also looked at), I think it disagrees with you:

Unless otherwise specified by the implementing class, actions are performed in the order of iteration (if an iteration order is specified).

So yes, in all of the cases that you used (and thanks for showing the bytecode so it's not just pure speculation), they are equivalent. However, as the quote says, the implementing class may change how a forEach loop actually works. That's the major point of implementing it. It gives more control over the flow of how exactly some class should loop. It's similar to how operator overloading allows for any class to change the default behavior of the language. I think of the forEach loop as almost a "loop overload," if you will.

That's not to say that there's anything you can't do with the old for each loops that you can do now, it's just that it changes how the two work. The new forEach loop is just more "functionally focused," I suppose.

From: i luffs yeww
From your quote from the docs (which is also the source I also looked at), I think it disagrees with you:



So yes, in all of the cases that you used (and thanks for showing the bytecode so it's not just pure speculation), they are equivalent. However, as the quote says, the implementing class may change how a forEach loop actually works. That's the major point of implementing it. It gives more control over the flow of how exactly some class should loop. It's similar to how operator overloading allows for any class to change the default behavior of the language. I think of the forEach loop as almost a "loop overload," if you will.

That's not to say that there's anything you can't do with the old for each loops that you can do now, it's just that it changes how the two work. The new forEach loop is just more "functionally focused," I suppose.

With everything I have read, forEach is the new standard to loop objects.

From: i luffs yeww
It's similar to how operator overloading allows for any class to change the default behavior of the language.

I wasn't arguing against customisability.. I was arguing efficiency, simplicity, and semantics based on the following (reasons given to avoid it):

forEach is bad practice for efficiency. Only really good when trying to keep things simple.

and in that case, it isn't bad for efficiency because the implementation is the same.. we'd be arguing the tradition for, the for-ranged, the stream.forEach and the iterable.forEach default implementations (which all happen to be almost exactly the same or a wrapper around another).


TLDR: There's no reason not to use it.. Premature optimisation is the root of many hours spent. Netbeans even recommends using the stream.forEach over the for-ranged (warning/underlined) for reasons unknown to me.