problem switching direction when I create mult enemies in libgdx(box2d)

[game2d]

I am using:
--
Libgdx and Box2d.

-------------

what I am trying to do:
--
I want enemy to move right and left. if enemy hits the wall than move the over way.

----

My plan:
--
I have two senors set up on the enemy. one on enemy left side and other on enemy right side. so if wall hit enemy right senor than move enmey left. and if wall hit enemy left senor than move enmey right.

I am creating 3 enemies and I am storeing them in arraylist.

--------

Problem:
--
if I create 1 enemy it works fine. enemy hits the wall and switch direction. I am doing collision stuff in contectListener also called (beginContact & endContact methods).

problem is when i create 2 or more enemies and store in arraylist. so for some reason if a enemy switch direction than all of enemies switch directions together.

I know why this is happening. its bc in contectListener I have one variable called "wallSwitch " which checks, if enemy hit a wall and enemy need to switch. so that is why it works for one enmey fine. that 'wallSwitch' variable is changing every enemies direction. I need to come up way so so every enemy is different.



-----

here is I am building enemy. It was two sensors. on enemy left side & other on enemy right side.

    private void createEnemies() {
	...
        for (MapObject mo : layer.getObjects()) {
	        bdef.type = BodyType.DynamicBody;
                ....
	        bdef.position.set(x, y);
	        body = world.createBody(bdef);

			/*** 1st fixture - enemy ***/
			pshape.setAsBox(10f/PPM, 10f/PPM);
			fdef.shape = pshape;
			fdef.isSensor = false;
			fdef.filter.categoryBits = Constants.BIT_ENEMY;
			fdef.filter.maskBits = Constants.BIT_PLAYER;
			body.createFixture(fdef).setUserData("enemy");

			/*** 2nd fixture - left ***/
			pshape.setAsBox(10f/PPM, 10f/PPM, new Vector2(-10f / PPM,
					0f/PPM), 0);
			fdef.shape = pshape;
			fdef.isSensor = true;
			fdef.filter.categoryBits = Constants.BIT_ENEMY; // id
			fdef.filter.maskBits = Constants.BIT_GROUND;// collied with
			body.createFixture(fdef).setUserData("enemyLeft");
			
			/*** 3rd fixture - right ***/
			pshape.setAsBox(10f / PPM, 10f / PPM, new Vector2(10f / PPM,
					0f / PPM), 0);
			fdef.shape = pshape;
			fdef.isSensor = true;
			fdef.filter.categoryBits = Constants.BIT_ENEMY; // id
			fdef.filter.maskBits = Constants.BIT_GROUND;// collied with
			body.createFixture(fdef).setUserData("enemyRight");// myContactListener
			
			
			// create & store in array list
			Enemies e1 = new Enemy1(body);
                        Enemies e2 = new Enemy1(body);  //problem here
			enemiesStore.add(e1);
                         enemiesStore.add(e2);
		}
	}

Problem is here I think
----

here is collision stuff. when enemy sensor touching a wall than switch direction.

   boolean wallSwitch;

     public void beginContact(Contact contact) {
		Fixture fa = contact.getFixtureA();
		Fixture fb = contact.getFixtureB();

		// Enemy switch direction
		if (fa.getUserData() != null && fa.getUserData().equals("enemyLeft")) {
			wallSwitch = true;
		}
		if (fb.getUserData() != null && fb.getUserData().equals("enemyLeft")) {
			wallSwitch = true;
		}
		if (fa.getUserData() != null && fa.getUserData().equals("enemyRight")) {
			wallSwitch = true;
		}
		if (fb.getUserData() != null && fb.getUserData().equals("enemyRight")) {
			wallSwitch = true;
		}

	}

// called when two fixtures no longer collide - run it onces

   public void endContact(Contact contact) {
		Fixture fa = contact.getFixtureA();
		Fixture fb = contact.getFixtureB();

		// Enemy switch direction
		if (fa.getUserData() != null && fa.getUserData().equals("enemyLeft")) {
			wallSwitch = false;
		}
		if (fb.getUserData() != null && fb.getUserData().equals("enemyLeft")) {
			wallSwitch = false;
		}
		if (fa.getUserData() != null && fa.getUserData().equals("enemyRight")) {
			wallSwitch = false;
		}
		if (fb.getUserData() != null && fb.getUserData().equals("enemyRight")) {
			wallSwitch = false;
		}
	}

[Nepomuk]

Without me having to reading your code, you seem to have found the problem yourself: The wallSwitch variable. If you have several enemies and this field is used for each one then you will have to have several wallSwitches as well. You could either save this in the Enemy class or have a Collection of wallSwitches (e.g. a HashMap<Enemy, Boolean> or a List<Boolean>) with one value for every Enemy.

[chaarmann]

Maybe you are not aware of it, but this code from you below written in a "copy-and-paste" style is very problematic regarding performance and multiple threads.

 Fixture fa = contact.getFixtureA();
 ... 
        if (fa.getUserData() != null && fa.getUserData().equals("enemyLeft")) {
            wallSwitch = true;
        }
...
       if (fa.getUserData() != null && fa.getUserData().equals("enemyRight")) {
            wallSwitch = false;
        }

Change it to:

 Fixture fa = contact.getFixtureA();
String userData = (fa == null) ? "" : fa.getUserData();
 ... 
        if (userData.equals("enemyLeft")) {
            wallSwitch = true;
        }
...
       if (userData.equals("enemyRight")) {
            wallSwitch = false;
        }

These are the differences:
- your if-statement can crash: if you call getUserData() inside if-statement to check for null, it may be not null, therefore the second call getUserData().e quals() will be executed. But in the meantime between both calls (parallel threads) the userData can become null and then the second call crashes.

- wrong exeution: opposite if-statements can both be executed (or none of them). In all if-statements, you get the user-data again and again. The compiler cannot inline or optimize here, it really must call the method again and again. But after executing first if-statement, userData could change to a different string and then both if-statements will be executed or none!!! Example: first, getUserData() returns "enemyRight ", so the if-statement at line 8 (first listing) will not be executed, because it checks for "enemyLeft" . Then it goes to line 14, but in the meantime, getUserData() will be changed and deliver "enemyLeft" . So the if-statement at line 14 will also not be executed. So finally your resulting variable wallSwitch will not have the value that fits to the string! In another case, when it first returns "enemyLeft" and in the meantime switches to "enemyRight ", both if-statements will be executed.

- unreadable code: copy-and-paste style programming leads to a lot of bugs. Look how short my refactored code is and how much better the readability! With copy-and-paste-style programming, you can program faster, but usually a single line of code is written once and read 10 times. So all other who read the code (inclusive you) will waste a lot of time. Magnitues more than you save by not extracting a local variable.

- slow code: usually the compiler can only inline a method (like getData()) if the method is final (and therefore cannot be overloaded) and only if the method does a simple thing, like getting a variable value (setter and getter). So if this is the case, calling getData() may result in a simgle call and storing the value in an internal register for later re-use. But what happens if somebody in the future must change the getData() method to get the data from a database or internet connection instead, which takes a loong time. No inlining possible anymore! Then your main program suddenly runs very slow! So do not rely on compiler inlining!

[game2d]

ok, so I took what you guys said and try to make changes to my code.

problem: see code section 3

I took Nepomuk suggestion and made 'wallSwitch' into hash array. If there are two enemy than there will be two values in 'wallSwitch'. different values for different enemy store in there.

public class MyContactListener ... {
    public static Array<Boolean> wallSwitch = new Array<Boolean>();

    public MyContactListener() {
	...
	wallSwitch.add(false); 
	wallSwitch.add(false);
    }

here I change 'enemyLeft' to 'enemyLeft0' or enemyLeft1'(depending on howmany enemy there are).
and I am using loop to go though it.


inside for loop i have two if statments. if(i == 0) or if(i == 1). these are two checks. If enemy one hit the wall than put value true with 0 index in wallSwitch. And if enemy two hit the wall than put value true with 1 index. same thing for false.

	public void beginContact(Contact contact) {
	    Fixture fa = contact.getFixtureA();
	    Fixture fb = contact.getFixtureB();

	    String userDataA = (String) ((fa == null) ? "" : fa.getUserData());
	    String userDataB = (String) ((fb == null) ? "" : fb.getUserData());

	    // Enemy switch direction
	    for (int i = 0; i < 5; i++) {
	        if (userDataA.equals("enemyLeft" + i)) {
		    if (i == 0)
			wallSwitch.set(0, true);
		    if (i == 1)
			wallSwitch.set(1, true);
		}
		if (userDataB.equals("enemyLeft" + i)) {
		    if (i == 0)
		        wallSwitch.set(0, true);
		    if (i == 1)
			wallSwitch.set(1, true);
		}
		if (userDataA.equals("enemyRight" + i)) {
		    if (i == 0)
		        wallSwitch.set(0, true);
		    if (i == 1)
			wallSwitch.set(1, true);
		}
		if (userDataB.equals("enemyRight" + i)) {
		    if (i == 0)
			wallSwitch.set(0, true);
		    if (i == 1)
			wallSwitch.set(1, true);
		 }
           }
	}

PROBLEM:
I have put error in bold so you can see where the error is.

I have two enemy one screen going left.
Enemy 1 is going left, it hit wall than, switch direction
Enemy 2 is going left, it hit wall than, doesnt switch direction --- error
soon as Enemy 2 hit a wall for some reason Enemy 1 switch direction again(doesnt hit wall) and Enemy 2 keep on going left.

WHAT I AM THINKING:
I have create 'wallSwitch' separate variables still the problem is happening. may be the variables value are be switch.

// called when two fixtures no longer collide - run it onces
	public void endContact(Contact contact) {
		Fixture fa = contact.getFixtureA();
		Fixture fb = contact.getFixtureB();

		String userDataA = (String) ((fa == null) ? "" : fa.getUserData());
		String userDataB = (String) ((fb == null) ? "" : fb.getUserData());


		// Enemy switch direction
		for (int i = 0; i < 5; i++) {
			if (userDataA.equals("enemyLeft" + i)) {
				if (i == 0)
					wallSwitch.set(0, false);
				if (i == 1)
					wallSwitch.set(1, false);
			}
			if (userDataB.equals("enemyLeft" + i)) {
				if (i == 0) {
					wallSwitch.set(0, false);
				}
				if (i == 1) { [B]// error - doesnt come in[/B]
					wallSwitch.set(1, false);
				}
			}
			if (userDataA.equals("enemyRight" + i)) {
				if (i == 0)
					wallSwitch.set(0, false);
				if (i == 1)
					wallSwitch.set(1, false);
			}
			if (userDataB.equals("enemyRight" + i)) {
				if (i == 0)
					wallSwitch.set(0, false);
				if (i == 1)
					wallSwitch.set(1, false);
			}
		}
	}

i see there is alot of copy-paste in my code. So I will change it later when I get this bug fix

[chaarmann]

You create variable "wallSwitch " of class "Array". But that class does not behave like a normal array. It is used for reflection only, that is, if you want to access a normal array of another class that already has an array defined. You do not have such a class, and it would be very slow, too. So this is the wrong tool here. You should use class "ArrayList" or just a normal array. I would use a normal array here if the number of enemies (5, see for-loop) does not change and can be hardcoded (else use ArrayList).
So you can use:

public class MyContactListener ... {
    public static boolean[] wallSwitch = new boolean[5];

Just note that this array is already pre-filled with false values.
Then use:

...
       // Enemy switch direction
        for (int i = 0; i < 5; i++) {
            if (userDataA.equals("enemyLeft" + i)) {
                if (i == 0)
                    wallSwitch[0] = true;
                if (i == 1)
                    wallSwitch[1] = true;
            }
 ...

You can also refactor this code to:

...
       // Enemy switch direction
        for (int i = 0; i < 5; i++) {
            if (userDataA.equals("enemyLeft" + i)) {
                     wallSwitch[i] = true;
             }
 ...

Another thing you should check is some code not shown here. This code checks the wallSwitch variable and then switches the direction of each enemy. Make sure that in this code enemy 1 really checks wallSwitch[0] only and enemy 2 checks wallSwitch[1] only! It seems that enemy 1 somehow checks wallSwitch[0] and wallSwitch[1], and enemy 2 checks nothing. Just look for correct indices regarding enemy number!

[game2d]

Ok-_-; I think I have found the issue. The issue is in code which i didnt posted, so I have posted it below in code section 1. its where I am checking the wallSwitch variable and switches direction of each enemy.

public class Enemy1 extends Enemies{
   ....

   // collion - enemy collsion with wall so switch dir
   for (int i = 0; i < MyContactListener.wallSwitch.length; i++) {
	if (MyContactListener.wallSwitch[i]) {
            //if enemy face is left, than switch face & dir
	    if (faceLeft) {
                this.getBody().setLinearVelocity(-speed, 0f);
		faceLeft = false;
	     } else {
                 this.getBody().setLinearVelocity(speed, 0f);
	         faceLeft = true;
	     }
              //already switch dir so set to false
	      MyContactListener.wallSwitch[i] = false;
	  }
     }
}

so to fix this issue. I broke above code down.
when I just check for enemy 1(index 0) than it works with no problems(no issues).

  if (MyContactListener.wallSwitch[0]) {
	if (faceLeft) {
		 this.getBody().setLinearVelocity(-speed, 0f);
		faceLeft = false;
	} else {
                this.getBody().setLinearVelocity(speed, 0f);
		faceLeft = true;
	}
	MyContactListener.wallSwitch[0] = false;
}

when I just check for enemy 2(index 1), than it than it has problem. for some reason its checks for Enemy 1(index 0) too.

if (MyContactListener.wallSwitch[1]) {
	if (faceLeft) {
		 this.getBody().setLinearVelocity(-speed, 0f);
		 faceLeft = false;
	} else {
		 this.getBody().setLinearVelocity(speed, 0f);
		faceLeft = true;
	}
	MyContactListener.wallSwitch[1] = false;
}

[chaarmann]

Your class is called "Enemy1", that is confusing. Maybe you think that you have created the first enemy this way, but it is really a subclass name of the "Enemy" main class. You still must create two objects. But how does each object know which enemy it is, first or second? The easiest way is to add an ID "id", that you can set after creating an object of the class. Then you do not need a for-loop (which woud check wallSwitch for ALL enemies), but you only check for yourself, that means check MyContactListen er.wallSwitch[id].

corrected code:
creating both enemies:

Enemy1 first = new Enemy1();
first.setId(0);
Enemy1 second = new Enemy1();
second.setId(1);
...

Then changing direction:

public class Enemy1 extends Enemies{
   private int id;
   public void setId(int id)
   {
      this.id=id;
   }
...
if (MyContactListener.wallSwitch[id]) {
    if (faceLeft) {
         this.getBody().setLinearVelocity(-speed, 0f);
        faceLeft = false;
    } else {
                this.getBody().setLinearVelocity(speed, 0f);
        faceLeft = true;
    }
    MyContactListener.wallSwitch[id] = false;

}
By the way, you can refactor the latter one:

if (MyContactListener.wallSwitch[id])
{
   int newSpeed = faceleft ? -speed : speed;
   this.getBody().setLinearVelocity(newSpeed, 0f);
   faceleft = !faceLeft;
   MyContactListener.wallSwitch[id] = false;
}

[game2d]

OMG it works!!!! I been working to fix this bug for past 3 months. so thanks you so much!!!!!!