patternjavaMinor
Obstacle-avoiding AI
Viewed 0 times
obstacleavoidingstackoverflow
Problem
I have made a game in Java that involves two players to fight each other with spaceships. For one player mode, I have programmed a simple AI to avoid allies and asteroids and go straight towards the player. Unfortunately, the game lags quite a bit when there are 3 AIs at once. I think this is because of all the
Note: this is only a snippet, not the whole thing.
Important: angle is in counterclockwise radians.
```
angle = player.getAngle();
position = player.getPosition();
mass = player.getMass();
if(position.x > xLimit){
position.x = xLimit;
}
if(position.x yLimit){
position.y = yLimit;
}
if(position.y 2*Math.PI){
angle -= 2*Math.PI;
}
if(angle > 3 Math.PI / 2 && angToOther 3 Math.PI / 2){
angToOther -= 2 * Math.PI;
}
boolean asteroidInWay = false;
float playerDist = (float) Math.sqrt(Math.pow(otherPos.x - position.x,2) +
Math.pow(otherPos.y - position.y, 2));
ArrayList possiblePaths = new ArrayList();
ArrayList goodPaths = new ArrayList();
ArrayList toRemove = new ArrayList();
for(Body ast: asteroidField){
float radius = ast.getFixtureList().m_shape.m_radius;
float dist = (float) Math.sqrt(Math.pow(ast.getPosition().x - position.x,2) +
Math.pow(ast.getPosition().y - position.y, 2));
float angToAst = findAng(position, ast.getPosition());
float angToEdge = (float) Math.asin(radius / dist);
float side1 = angToAst+angToEdge+0.3f;
float side2 = angToAst-angToEdge-0.3f;
if(side1 > 3 Math.PI / 2 && angToOther 3 Math.PI / 2){
side1 += 2 * Math.PI;
}
if(side2 > 3 Math.PI / 2 && angToOther 3 Math.PI / 2){
side2 += 2 * Math.PI;
}
if(angToOther side2 && playerDist > dist){
asteroidInWay = true;
}
for loops. Is there a way to streamline this?Note: this is only a snippet, not the whole thing.
Important: angle is in counterclockwise radians.
player- AI body
xLimit\yLimit- Boundaries
asteroidField-ArrayListof asteroid bodies
otherAllies-ArrayListof ally bodies
findAng- returns angle from two positions
```
angle = player.getAngle();
position = player.getPosition();
mass = player.getMass();
if(position.x > xLimit){
position.x = xLimit;
}
if(position.x yLimit){
position.y = yLimit;
}
if(position.y 2*Math.PI){
angle -= 2*Math.PI;
}
if(angle > 3 Math.PI / 2 && angToOther 3 Math.PI / 2){
angToOther -= 2 * Math.PI;
}
boolean asteroidInWay = false;
float playerDist = (float) Math.sqrt(Math.pow(otherPos.x - position.x,2) +
Math.pow(otherPos.y - position.y, 2));
ArrayList possiblePaths = new ArrayList();
ArrayList goodPaths = new ArrayList();
ArrayList toRemove = new ArrayList();
for(Body ast: asteroidField){
float radius = ast.getFixtureList().m_shape.m_radius;
float dist = (float) Math.sqrt(Math.pow(ast.getPosition().x - position.x,2) +
Math.pow(ast.getPosition().y - position.y, 2));
float angToAst = findAng(position, ast.getPosition());
float angToEdge = (float) Math.asin(radius / dist);
float side1 = angToAst+angToEdge+0.3f;
float side2 = angToAst-angToEdge-0.3f;
if(side1 > 3 Math.PI / 2 && angToOther 3 Math.PI / 2){
side1 += 2 * Math.PI;
}
if(side2 > 3 Math.PI / 2 && angToOther 3 Math.PI / 2){
side2 += 2 * Math.PI;
}
if(angToOther side2 && playerDist > dist){
asteroidInWay = true;
}
Solution
As noted in the comments, this would be better broken into methods. Not only would it make it easier to profile, it would make the code easier to understand.
This seems a bad name. What is an
Are
This may or may not give a performance improvement. It depends on how this code is reached. Is it called from a loop itself? That context would make it easier to evaluate this code.
Same objection. Why do this after fetching the angle? If you fetch the same angle multiple times, you'll do this multiple times.
You may also be able to get a small performance boost by defining a constant for the value
This is confusing. Perhaps if I traced through the code enough, I could see the logic of this. But why should I need to do so? This would be a great place to comment and explain why
It's also worth noting that it would be helpful to know how
This might better be named something like
As a general rule, on the left side you want to specify the type as the interface rather than the implementation.
This makes it easier if you want to change the implementation later. Very occasionally there will be some method that only exists on the implementation that will require you to use the implementation. If that exists here, for all three, then you should comment saying so.
Most would put more spacing here and elsewhere.
This makes it easier to see that
Also, why call it
What is
If they aren't going to change, you should declare these as
You can actually get rid of the
Writing out the names makes it easier to read the code and understand what it's doing.
Changing the order can make it easier to see that you just declared
Moving the addition of
angle = player.getAngle();This seems a bad name. What is an
angle and why does a player have one? My best guess is that this angle represents the direction of travel. If so, why not call it direction? position = player.getPosition();
mass = player.getMass();
if(position.x > xLimit){
position.x = xLimit;
}
if(position.x yLimit){
position.y = yLimit;
}
if(position.y < -1 * yLimit){
position.y = -yLimit;
}Are
xLimit and yLimit always the same? If these are general limits, then they should probably be checked when the position is set or changed. Then you'd always get valid values from getPosition and wouldn't need to check them here. This may or may not give a performance improvement. It depends on how this code is reached. Is it called from a loop itself? That context would make it easier to evaluate this code.
while(angle2*Math.PI){
angle -= 2*Math.PI;
}Same objection. Why do this after fetching the angle? If you fetch the same angle multiple times, you'll do this multiple times.
You may also be able to get a small performance boost by defining a constant for the value
2*Math.PI. You use this frequently but your code has you calculating each time. Of course, your compiler may be smart enough to compile it out. Same thing for the two right angle values. if(angle > 3 * Math.PI / 2 && angToOther 3 * Math.PI / 2){
angToOther -= 2 * Math.PI;
}This is confusing. Perhaps if I traced through the code enough, I could see the logic of this. But why should I need to do so? This would be a great place to comment and explain why
angToOther should be no more than half a rotation from angle. We might be able to suggest a better way to get that same effect if it were clearer what this code was doing. It's also worth noting that it would be helpful to know how
angToOther is calculated and used. Perhaps that calculation could be adjusted to remove the need for this adjustment. Or later uses might be adjusted to not require this. That kind of context is often helpful. boolean asteroidInWay = false;This might better be named something like
obstacleInWay, as it doesn't just get set for asteroids. ArrayList possiblePaths = new ArrayList();
ArrayList goodPaths = new ArrayList();
ArrayList toRemove = new ArrayList();As a general rule, on the left side you want to specify the type as the interface rather than the implementation.
List possiblePaths = new ArrayList();
List goodPaths = new ArrayList();
List toRemove = new ArrayList();This makes it easier if you want to change the implementation later. Very occasionally there will be some method that only exists on the implementation that will require you to use the implementation. If that exists here, for all three, then you should comment saying so.
for(Body ast: asteroidField){Most would put more spacing here and elsewhere.
for (Body asteroid: asteroidField) {This makes it easier to see that
for and Body are separate keywords. And that ) and { are not one multi-character symbol. Also, why call it
ast rather than write out asteroid? Similarly, why call a list of asteroids an asteroid field? Generally an asteroid field would be a section of space, not a collection of asteroids. I'd probably call that either asteroids or asteroidsInField. float radius = ast.getFixtureList().m_shape.m_radius;
float dist = (float) Math.sqrt(Math.pow(ast.getPosition().x - position.x,2) +
Math.pow(ast.getPosition().y - position.y, 2));
float angToAst = findAng(position, ast.getPosition());
float angToEdge = (float) Math.asin(radius / dist);
float side1 = angToAst+angToEdge+0.3f;
float side2 = angToAst-angToEdge-0.3f;What is
.3? You should probably declare a constant for this. Or comment on what it does. final float distance = (float) Math.sqrt(Math.pow(ast.getPosition().x - position.x,2)
+ Math.pow(asteroid.getPosition().y - position.y, 2));
final float deviation = (float) Math.asin(asteroid.getFixtureList().m_shape.m_radius / distance) + .3f;
final float asteroidDirection = findAng(position, asteroid.getPosition());
float farEdge = asteroidDirection + deviation;
float nearEdge = asteroidDirection - deviation;If they aren't going to change, you should declare these as
final. You can actually get rid of the
radius variable entirely, as you only use it the once. Writing out the names makes it easier to read the code and understand what it's doing.
Changing the order can make it easier to see that you just declared
distance when you use it to calculate the deviation. Moving the addition of
.3 to the deviation calculation saves a Code Snippets
angle = player.getAngle();position = player.getPosition();
mass = player.getMass();
if(position.x > xLimit){
position.x = xLimit;
}
if(position.x < -1 * xLimit){
position.x = -xLimit;
}
if(position.y > yLimit){
position.y = yLimit;
}
if(position.y < -1 * yLimit){
position.y = -yLimit;
}while(angle<0){
angle += 2*Math.PI;
}
while(angle>2*Math.PI){
angle -= 2*Math.PI;
}if(angle > 3 * Math.PI / 2 && angToOther < Math.PI / 2){
angToOther += 2 * Math.PI;
}
if(angle < Math.PI / 2 && angToOther > 3 * Math.PI / 2){
angToOther -= 2 * Math.PI;
}boolean asteroidInWay = false;Context
StackExchange Code Review Q#96091, answer score: 5
Revisions (0)
No revisions yet.