Sign in to follow this  
Nicholas Kong

Need Criticism on coding style of my AI code

Recommended Posts

Nicholas Kong    1535

Here is the AI code for the monster that moves left and right in a game.

 

Note: movementRightFlag and movementLeftFlag variable gets turned on and off at different circumstances.

 

I am trying to get into better design habits mainly because program design was not taught in college.

 

I need criticisms on the code. I welcome all feedback.

 

Code is in Java.

public BatSprite(double x, double y) {
       runOnce = true;
}

public void update(long milliseconds)
{
if(movementLeftFlag)
            {
                
                
                if(position.getY() > leastXPositionThreshold)
                {
                    

                    position.setX(position.getX() - 1);
                    
                    if(position.getX() == leastXPositionThreshold)
                    {
                        runOnce = false;

                        movementRightFlag = true;

                    }
                    
                }
                
                
            }
            

            if(movementRightFlag)
            {
                movementLeftFlag = false;
                position.setX(position.getX() + 1);
                
                if(position.getX() > mostXPositionThreshold)
                {
                    
                    runOnce = true;
                    movementRightFlag = false;
                    
                }
                
                
            }
                
            
            
        
            // extra logic that changes the state of the monster direction
            if(position.getY() == 50)
            {
                direction = "down";
                
                if(runOnce)
                {

                        movementLeftFlag = true;
                        
                    
                }
                
            }
}
Edited by warnexus

Share this post


Link to post
Share on other sites
serumas    796

You dont need that  movementLeftFlag and the other one...

just declare single variable for direction (could be vector2, dont use x,y but vector - this will give you easier writing algorithms)

 

 

                if(

                      (direction >0 && position.getX() > mostXPositionThreshold) ||

                      (direction <0 && position.getX() > leastXPositionThreshold)

                   ) direction=-direction;

                position.setX(position.getX() + direction);
 
offcouse it can be buggy if speed of your AI element variates, but the code could be something similar to this....
or:
 
                if(direction >0 && position.getX() > mostXPositionThreshold)direction=-speed;else

                if(direction <0 && position.getX() > leastXPositionThreshold)direction=speed;

                position.setX(position.getX() + direction);

Edited by serumas

Share this post


Link to post
Share on other sites
Nicholas Kong    1535

You dont need that  movementLeftFlag and the other one...

just declare single variable for direction (could be vector2, dont use x,y but vector - this will give you easier writing algorithms)

 

 

                if(

                      (direction >0 && position.getX() > mostXPositionThreshold) ||

                      (direction <0 && position.getX() > leastXPositionThreshold)

                   ) direction=-direction;

                position.setX(position.getX() + direction);
 
offcouse it can be buggy if speed of your AI element variates, but the code could be something similar to this....
or:
 
                if(direction >0 && position.getX() > mostXPositionThreshold)direction=-speed;else

                if(direction <0 && position.getX() > leastXPositionThreshold)direction=speed;

                position.setX(position.getX() + direction);

I don't think direction should be a numeric value. Why are you making direction a numeric value?

Share this post


Link to post
Share on other sites
Nicholas Kong    1535

Make your spacing perfectly consistent.  Maybe that was something that got messed up in cut/paste.

True. I thought there might have a comment about the flag variables. But I guess those are fine then?

Share this post


Link to post
Share on other sites
ApochPiQ    23003

I don't think direction should be a numeric value. Why are you making direction a numeric value?



Uh... encoding directions/offsets/positions as vectors (numeric coordinates) is a very, very standard practice. Why do you think it's wrong?

Share this post


Link to post
Share on other sites
Nicholas Kong    1535

 

I don't think direction should be a numeric value. Why are you making direction a numeric value?



Uh... encoding directions/offsets/positions as vectors (numeric coordinates) is a very, very standard practice. Why do you think it's wrong?

 

I usually see direction implemented using enums and I coded mine using strings for my own convenience and it makes it more readable for me. I do not know the standard practice because I was never exposed to it. I am a CS undergraduate.

 

It seems weird to think of direction as a number. I was always thought of them as up,down,left and right. How can one say "up is 10, 350 or 800"?

 

Is there a link anyone can refer to me in regards to "standard practices"? It would surely broaden my horizon and make me a better programmer.

Edited by warnexus

Share this post


Link to post
Share on other sites
cardinal    908


I don't think direction should be a numeric value. Why are you making direction a numeric value?

 

Direction is generally represented by a vector or an angle in games. Or rather a character's movement is represented by its velocity which is represented by a vector. So each update you can simply do:

 

position += velocity; To move your character.

 

Then if you wanted to change directions you could simply do:

 

velocity *= -1.0f; to flip it the other direction.

 

You can represent direction in another way if you want, but generally it introduces branching (if statements) which adds complexity to code. For example, each time you have an if/else block you introduce another code path, which increases the complexity of your program and requires additional testing.

Share this post


Link to post
Share on other sites
Nicholas Kong    1535

 


I don't think direction should be a numeric value. Why are you making direction a numeric value?

 

Direction is generally represented by a vector or an angle in games. Or rather a character's movement is represented by its velocity which is represented by a vector. So each update you can simply do:

 

position += velocity; To move your character.

 

Then if you wanted to change directions you could simply do:

 

velocity *= -1.0f; to flip it the other direction.

 

You can represent direction in another way if you want, but generally it introduces branching (if statements) which adds complexity to code. For example, each time you have an if/else block you introduce another code path, which increases the complexity of your program and requires additional testing.

 

Ah I see. Good point. I notice the branching in my own code too. It would seem complicated in the future like you said. Okay I will change direction into a vector. Thanks.

Share this post


Link to post
Share on other sites
cardinal    908

Also, in general it's good to think about character movement in mathematical terms. We can mathematically traverse the coordinate space using vectors (velocities and  positions at the very basic).

 

When you get to a little more complex movement (such as steering) it would be ridiculous to write code than modifies the X and Y motion by hand rather than simply rotating the velocity vector.

Share this post


Link to post
Share on other sites

I'd separate the movement logic into another class. It looks like you have some data-driven behaviour in there (mostXPositionThreshold, leastXPositionThreshold) which could be moved into a HorizontalPingPong movement class or something (maybe use one class for horizontal and vertical ping-pong movement).

 

Limiting movement speed to 1 pixel at a time (you check for exact pixel positions at least once, e.g.

 

                   if(position.getX() == leastXPositionThreshold)
                    {
                        runOnce = false;

                        movementRightFlag = true;

                    }

 

is going to cause problems later on. Always check >= or <=

 

Then you ruin everything by hardcoding the number 50 :(

Share this post


Link to post
Share on other sites
Nicholas Kong    1535

I'd separate the movement logic into another class. It looks like you have some data-driven behaviour in there (mostXPositionThreshold, leastXPositionThreshold) which could be moved into a HorizontalPingPong movement class or something (maybe use one class for horizontal and vertical ping-pong movement).

 

Limiting movement speed to 1 pixel at a time (you check for exact pixel positions at least once, e.g.

 

                   if(position.getX() == leastXPositionThreshold)
                    {
                        runOnce = false;

                        movementRightFlag = true;

                    }

 

is going to cause problems later on. Always check >= or <=

 

Then you ruin everything by hardcoding the number 50 sad.png

Doh! Yeah I should def change the condition. I see what you mean.

 

What do you mean me runing everything by hardcoding the number 50? Because it is a magic number?

Share this post


Link to post
Share on other sites

Yes, it's a magic number. Is there only 1 bat in your game (checks code again: looks like it is a flying bad and not a pong refugee) and does it always appear in the same place? Presumably 50 is something to do with the height of the ceiling? In the particular place where the bat lives?

 

Try and imagine how to handle monster movement without any numbers from the program (so read from a level data file say). How would a bat be placed in a level editor, if one existed? They'd probably draw a box with the extents the bat could move between. That box and the fact it is a Bat (probably following ping-pong movement constrained to the height of the box) should be data, not code. (Static data tables are ok though if you haven't got an editor. It's better to read the data from a file though).

Share this post


Link to post
Share on other sites
Nicholas Kong    1535

Yes, it's a magic number. Is there only 1 bat in your game (checks code again: looks like it is a flying bad and not a pong refugee) and does it always appear in the same place? Presumably 50 is something to do with the height of the ceiling? In the particular place where the bat lives?

 

Try and imagine how to handle monster movement without any numbers from the program (so read from a level data file say). How would a bat be placed in a level editor, if one existed? They'd probably draw a box with the extents the bat could move between. That box and the fact it is a Bat (probably following ping-pong movement constrained to the height of the box) should be data, not code. (Static data tables are ok though if you haven't got an editor. It's better to read the data from a file though).

There is only one bat in the game.

 

50 is just a threshold variable for the bat's position y coordinate. It needs this variable for the bat movement state to be set to "down" otherwise the bat cannot move down in the game.

 

The bat moves just like the ball in the pong game.

 

What type of code should be read from a file? Just the movement data of the bat from the program?

Edited by warnexus

Share this post


Link to post
Share on other sites

Don't read code from a file read data.

 

Sounds like you have a bouncing object constrained to a 2D rectangle, with a start position, initial direction/velocity, that bounces off the edges of the rectangle. Read those values from a file (or a data table in the code) and you will be able to us the same logic for a lot of enemies, surely?

Edited by Paradigm Shifter

Share this post


Link to post
Share on other sites
Nicholas Kong    1535

Don't read code from a file read data.

 

Sounds like you have a bouncing object constrained to a 2D rectangle, with a start position, initial direction/velocity, that bounces off the edges of the rectangle. Read those values from a file (or a data table in the code) and you will be able to us the same logic for a lot of enemies, surely?

Oh I see just put monster data in the text file and read them off. Sounds like a clean approach.

 

The bouncing object does not bounce off the edges of a rectangle object. It bounces off at certain threshold denoted by the variable names with "threshold as the name" in the Bat class in Java.

 

More specifically, the update method compares the bat's x o y position coordinates with the threshold variable every frame. If the coordinates lies on that threshold variable, the bat behaves differently.

Edited by warnexus

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this