# How to optimize my movement & collision code?

I am trying to implement movement & collision the simplest way possible.

    public void MovePlayer()
{
KeyboardState keyState = Keyboard.GetState();

Vector2 checkVec = location; //get current location

if (keyState.IsKeyDown(Keys.Left))
{
//Check what the next position would be
Rectangle checkRect = new Rectangle((int)(checkVec.X -= speed), (int)location.Y, texture.Width, texture.Height);
//Check if the next position would collide with any obstacles
bool collides = CheckCollision(checkRect);

//If it does not collide move..
if (!collides)
{
location.X -= speed;
}

}

if (keyState.IsKeyDown(Keys.Right))
{
Rectangle checkRect = new Rectangle((int)(checkVec.X += speed), (int)checkVec.Y, texture.Width, texture.Height);
bool collides = CheckCollision(checkRect);

if (!collides)
{
location.X += speed;
}
}

if (keyState.IsKeyDown(Keys.Up))
{
Rectangle checkRect = new Rectangle((int)checkVec.X, (int)(checkVec.Y -= speed), texture.Width, texture.Height);
bool collides = CheckCollision(checkRect);

if (!collides)
{
location.Y -= speed;
}
}

if (keyState.IsKeyDown(Keys.Down))
{

Rectangle checkRect = new Rectangle((int)checkVec.X, (int)(checkVec.Y += speed), texture.Width, texture.Height);
bool collides = CheckCollision(checkRect);

if (!collides)
{
location.Y += speed;
}
}
}


Here ist the CheckCollision() method:

        private bool CheckCollision(Rectangle checkRect)
{
bool collides = false;

foreach (Tile t in blockedTiles)
{
if (t.rectBounds.Intersects(checkRect))
{
return true;
}
else
{

}
}
return collides;
}


So my problem is that due to the order in which I am checking button input when I get a collision from up or down everything works fine. But when it is collision from left or right the code does not recognize if I am simulatneously moving up or down, so in this case the Textture freezes as long as I provoke left or right collision.

Any ideas how to fix this?

## 1 Answer

### Step 1 - Don't check collisions in each key, test them at the end

In your example, you are checking collisions for each key independantly. This will cause some trouble and code replication. In order to avoid that, I would suggest you use temporary variables that will stock the movement, increment it for each key and test collisions only once at the end:

public void MovePlayer()
{
KeyboardState keyState = Keyboard.GetState();

float xPlus = 0;
float yPlus = 0;

if (keyState.IsKeyDown(Keys.Left))
{
xPlus -= speed;
}
if (keyState.IsKeyDown(Keys.Right))
{
xPlus += speed;
}
if (keyState.IsKeyDown(Keys.Up))
{
yPlus -= speed;
}
if (keyState.IsKeyDown(Keys.Down))
{
yPlus += speed;
}

Rectangle checkRect = new Rectangle((int)(location.X + xPlus), (int)(location.Y + yPlus), texture.Width, texture.Height);
bool collides = CheckCollision(checkRect);
if (!collides) {
location.x += xPlus;
location.y += yPlus;
}
}


### Step 2 - Independant X and Y movement

A nice thing to add if independant movement for X and Y so that if the player is moving diagonaly, and is hitting a wall on one axis, it will still move on the other axis:

Rectangle checkRect = new Rectangle((int)(location.X + xPlus), (int)location.Y, texture.Width, texture.Height);
bool collides = CheckCollision(checkRect);
if (!collides) {
location.x += xPlus;
}
checkRect = new Rectangle((int)location.X, (int)(location.Y + yPlus), texture.Width, texture.Height);
collides = CheckCollision(checkRect);
if (!collides) {
location.y += yPlus;
}


### Step 3 - Simplify the CheckCollision method

You have a lot of useless lines in your collision method. It can be simplified as such:

private bool CheckCollision(Rectangle checkRect)
{
foreach (Tile t in blockedTiles)
{
if (t.rectBounds.Intersects(checkRect))
{
return true;
}
}
return false;
}


### Tip - Always use the true position of your object

You were assigning an intermediary variable that stocked the position of your player. The problem with this is the following: if you move your player, the checkVec variable will not be updated, so the next collision check will most likely fail. Always use the true position of the player, or at least update the checkVec when you move it.

### Warning - Don't use += in tests!

When you use += in your ifs, it WILL modify the value! Use only + instead! This is probably the reason why your collisions won't work.

### EDIT: Fixed-step vs variable-time

As Shiro pointed out in the comments, this method is not 100% applicable in the case the user modifies the default tiem step method for the variable time step. In this case, we should take into account the time elapsed between two Update(). More details in the link Shiro provided in the comments, and on this site.

• I want to add that this is not correct way to move players or objects in games, because calling location.x += xPlus; in your Update() method will result in your player to move slower, when he gets lower FPS. The correct way to move objects on the screen is relative to the gameTime.ElapsedTime property, which is the time elapsed between each frame. I have answered in this question how to move an object this way. Jun 19, 2015 at 17:35
• @Shiro This is assuming OP uses a delta-time approach. I wrote a simple algorithm that works well with fixed-time approach, the one OP is using. Jun 19, 2015 at 17:38
• Yea I just thought I would add this just on the side, so that the people who read this question know Jun 19, 2015 at 17:41
• @Shiro I have edited the answer to address this. Thanks for the heads-up. Jun 19, 2015 at 17:47