# Optimize code to track player shot

This code is for a simple 2D tile-based game. x1 and y1 are the mouse coordinates in the world. entity->x1 and entity->y1 is the point where the player is, the shot origin.

I would like to know how to keep the current output while simplifying the code and improving performance.

void shoot(Map *map, Entity *entity, int x1, int y1)
{
float slope = (y1 - entity->y1) / (x1 - entity->x1);
float y = entity->y1; //Weapon position
Block *block;

if(x1 > entity->x1){
y += slope;
for(float x = entity->x1 + 1; x < map->width && y < map->height && y > 0; ++x){

//Get block at position x y. Check if found a hit
if((block = map_block_at(map, x, y))->type != EMPTY){
block_cause_damage(block, entity->hand_item->weapon.damage);
return;
}

y += slope;
}
}

else
if(x1 < entity->x1){
y -= slope;
for(float x = entity->x1 - 1; x > 0 && y < map->height && y > 0; --x){

//Get block at position x y. Check if found a hit
if((block = map_block_at(map, x, y))->type != EMPTY){
block_cause_damage(block, entity->hand_item->weapon.damage);
return;
}

y -= slope;
}
}

//When the player shoots up or down
else {
slope = (y1 > entity->y1) ? 1 : - 1;
y += slope;
while(y < map->height && y > 0){

//Get block at position x y. Check if found a hit
if((block = map_block_at(map, x1, y))->type != EMPTY){
block_cause_damage(block, entity->hand_item->weapon.damage);
return;
}

y += slope;
}
}
}


The code for Entity and Map:

typedef struct Entity {
Renderable renderable;
plist_id render_id;
Point (*get_current_position)(struct Entity *, uint32_t);
int health, attack_damage, running, jumping;
float x0, y0, x1, y1;
uint32_t t0, t1;
enum {LEFT, RIGHT} side;
Image **texture;
Item *hand_item;
Backpack backpack;
} Entity;

typedef struct {
Renderable renderable;
plist_id render_id;
int width, height;
Block *blocks;
Camera *camera;
} Map;

-

It's a little difficult to do a good review because there are so many pieces missing, but I've guessed at a number of things and I believe I can help.

Specifically, it looks like your Map is a rectangular grid of Blocks and that all x and y coordinates are integers. If so, then your shot routine is really doing the equivalent of drawing a line from the entity location to the passed x1,y1 coordinates and one very efficient way to do that is to use Bresenham's line-drawing algorithm. Using a slight modification of that algorithm, since your shot routine seems to want to go until it either hits something or goes off the Map, we get a very efficient and very small routine:

void shoot(Map *map, Entity *entity, int x1, int y1)
{
int sx = entity->x1 < x1 ? 1 : -1;
int sy = entity->y1 < y1 ? 1 : -1;
int dx = abs(x1 - entity->x1);
int dy = abs(y1 - entity->y1);
int err = (dx>dy ? dx : -dy)/2;
int e2;

x1 = entity->x1;
y1 = entity->y1;
Block *block;
while (inbounds(map,x1,y1)) {
e2 = err;
if (e2 > -dx) {
err -= dy;
x1 += sx;
}
if (e2 < dy) {
err += dx;
y1 += sy;
}
if((block = map_block_at(map, x1, y1))->type != EMPTY){
block_cause_damage(block, entity->hand_item->weapon.damage);
return;
}
}
}


I've assumed that you have or can easily write an inbounds routine that returns true if the passed coordinates are within the Map.

-
Thank you for answering. I tried the code you provided and it works fine most of the time. But sometimes it gets stuck in the loop. I added another variable get_out, it's set to 0 on every iteration and incremented using else statements right after if (e2 > -dx) and if (e2 < dy) , then I check if get_out == 2 and exit. –  2013Asker Apr 27 '14 at 0:31

I have a few remarks:

• You may have a problem with this line of code:

float slope = (y1 - entity->y1) / (x1 - entity->x1);


If entity is already at the longitude x1, this will cause a division by 0. Such a division is undefined behaviour. Anything can happen. A plane may crash on your house. You should check first whether x1 == entity->x1 and handle this case properly. I see that the last else close of your program handles this case (//When the player shoots up or down), but the division by 0 appears before you handle it.

• I don't know whether entity.x and entity.y are int or float types. If they are int types, I guess that you can get rid of the float variables: the only thing that may need more precision than an integer is slope, and if entity.x and entity.y are integer types, then slope will contain an integer cast to a float (since it was initialized with an integer division).

It lacks a bit of context to provide a complete review. You should post the code of at least Map and Entity so that we don't have to assume things.

-
Thank you for answering. I'll post the code for entity and map. –  2013Asker Apr 27 '14 at 0:32