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;

Community · Accepted Answer · 2017-05-23 12:40:53Z

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
– 2013Asker, Commented Apr 27, 2014 at 0:31

Jamal · Accepted Answer · 2014-04-26 20:33:41Z

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
– 2013Asker, Commented Apr 27, 2014 at 0:32

Stack Exchange Network

Optimize code to track player shot

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Optimize code to track player shot

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions