Optimize code to track player shot

Question

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;

Answer 1

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.

Answer 2

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.