Problem
I am making a simple game like Wolfenstein 3d on C using raycasting. To calculate the length of the rays, I use the DDA algorithm. I apply the part of the code that calculates the length of the rays and the size of the wall on the vertical line where the ray hit. How can I optimize and improve my code?
/*
** Function: void calculate()
**
** Arguments: main struct and i
**
** return: void
**
** Description: The raycasting loop is a for loop that goes through every x,
** so there is a calculation for every vertical stripe of the screen.
*/
void calculate(t_cub3d *cub)
{
int i;
i = 0;
while (i < cub->window.res_width)
{
calculate_cam(cub, &i);
// field_x and field_y represent the current square of the map the ray is in.
cub->field.field_x = cub->player.x_pos;
cub->field.field_y = cub->player.y_pos;
calculate_ray_dir(cub);
calculate_step(cub);
calculate_wall(cub);
calculate_height(cub);
draw(cub, i);
i++;
}
calculate_sprite(cub);
}
/*
** Function: void calculate_cam()
**
** Arguments: main struct, variable counter(width)
**
** return: void
**
** Description: x_camera is the x-coordinate on the camera plane
** that the current x-coordinate of the screen represents, done this way
** so that the right side of the screen will get coordinate 1, the center
** of the screen gets coordinate 0, and the left side of the screen gets coordinate -1
*/
void calculate_cam(t_cub3d *cub, int *i)
{
cub->camera.x_camera = 2 * *i / (double)(cub->window.res_width) - 1;
cub->ray.dir_ray_x = cub->player.x_dir + cub->camera.x_plane *
cub->camera.x_camera;
cub->ray.dir_ray_y = cub->player.y_dir + cub->camera.y_plane *
cub->camera.x_camera;
}
/*
** Function: void calculate_ray_dir()
**
** Arguments: main struct
**
** return: void
**
** Description: x_deltaDist and y_deltaDist are the distance the ray
** has to travel to go from 1 x-side to the next x-side, or from 1 y-side
** to the next y-side.
*/
void calculate_ray_dir(t_cub3d *cub)
{
if (cub->ray.dir_ray_y == 0)
cub->ray.x_deltadist = 0;
else
{
if (cub->ray.dir_ray_x == 0)
cub->ray.x_deltadist = 1;
else
cub->ray.x_deltadist = fabs(1 / cub->ray.dir_ray_x);
}
if (cub->ray.dir_ray_x == 0)
cub->ray.y_deltadist = 0;
else
{
if (cub->ray.dir_ray_y == 0)
cub->ray.y_deltadist = 1;
else
cub->ray.y_deltadist = fabs(1 / cub->ray.dir_ray_y);
}
}
/*
** Function: void calculate_step()
**
** Arguments: main struct
**
** return: void
**
** Description: x_sideDist and y_sideDist are initially the distance
** the ray has to travel from its start position to the first x-side and
** the first y-side.
*/
void calculate_step(t_cub3d *cub)
{
if (cub->ray.dir_ray_x < 0)
{
cub->ray.x_ray_step = -1;
cub->ray.x_sidedist = (cub->player.x_pos - (double)(cub->field.field_x))
* cub->ray.x_deltadist;
}
else
{
cub->ray.x_ray_step = 1;
cub->ray.x_sidedist = (((double)(cub->field.field_x) + 1.0 -
cub->player.x_pos) * cub->ray.x_deltadist);
}
if (cub->ray.dir_ray_y < 0)
{
cub->ray.y_ray_step = -1;
cub->ray.y_sidedist = (cub->player.y_pos - (double)(cub->field.field_y))
* cub->ray.y_deltadist;
}
else
{
cub->ray.y_ray_step = 1;
cub->ray.y_sidedist = ((double)(cub->field.field_y) + 1.0 -
cub->player.y_pos) * cub->ray.y_deltadist;
}
}
/*
** Function: void calculate_wall()
**
** Arguments: main struct
**
** return: void
**
** Description: DDA algorithm. It's a loop that increments the ray with 1 square
** every time, until a wall is hit.
*/
void calculate_wall(t_cub3d *cub)
{
int is_wall;
is_wall = 0;
cub->window.side = 0;
while (is_wall == 0)
{
if (cub->ray.x_sidedist < cub->ray.y_sidedist)
{
cub->ray.x_sidedist += cub->ray.x_deltadist;
cub->field.field_x += cub->ray.x_ray_step;
cub->window.side = 0;
}
else
{
cub->ray.y_sidedist += cub->ray.y_deltadist;
cub->field.field_y += cub->ray.y_ray_step;
cub->window.side = 1;
}
if (cub->field.map[cub->field.field_y][cub->field.field_x] == '1')
is_wall = 1;
}
calculate_distto_wall(cub);
}
/*
** Function: void calculate_distto_wall()
**
** Arguments: main struct
**
** return: void
**
** Description: calculate the distance of the ray to the wall
*/
void calculate_distto_wall(t_cub3d *cub)
{
if (cub->window.side == 0)
{
cub->ray.wall_dist = ((double)(cub->field.field_x) - cub->player.x_pos
+ (1 - cub->ray.x_ray_step) / 2) / cub->ray.dir_ray_x;
}
else
{
cub->ray.wall_dist = ((double)(cub->field.field_y) - cub->player.y_pos
+ (1 - cub->ray.y_ray_step) / 2) / cub->ray.dir_ray_y;
}
}
/*
** Function: void calculate_height()
**
** Arguments: main struct
**
** return: void
**
** Description: calculate the height of the line that has to be
** drawn on screen
*/
void calculate_height(t_cub3d *cub)
{
cub->window.height_ln = (int)(cub->window.res_height / cub->ray.wall_dist);
cub->window.top_wall = (cub->window.height_ln * -1) / 2 +
cub->window.res_height / 2;
if (cub->window.top_wall < 0)
cub->window.top_wall = 0;
cub->window.bottom_wall = cub->window.height_ln / 2 +
cub->window.res_height / 2;
if (cub->window.bottom_wall >= cub->window.res_height)
cub->window.bottom_wall = cub->window.res_height - 1;
}
Solution
Use the Doxygen language to document your functions
It looks like you use some ad-hoc way to document your functions. It’s very good to add that documentation, but there are tools out there that make it even more useful, like Doxygen. Once you reformat your comments to follow the Doxygen syntax, you can then use the Doxygen tools to generate documentation in PDF, HTML and various other formats. It can also check whether your documentation is correct, for example whether you have documented all the parameters. It would have already found an error in the first functions: you mentino i in the description of the function arguments, but there is no such thing.
Pass integers by value
Why are you passing i by reference to calculate_sam()? You should just pass it by value, otherwise you pay for unnecessary dereferencing of the pointer.
Naming things
You should try to give more descriptive names to functions. Naming everything calculate_something() is not very useful, as calculating is what your computer is basically doing all the time. It looks like calculate() is actually causing everything to be drawn, since it calls draw(). So maybe it should be names something like draw_scene() instead? And draw() should probably be draw_column()?
Why is there x_pos and x_plane but field_x? Having a consistent ordering of words makes it easier to read your code. But then also, there is a redundancy in field.field_x, maybe rename it so you can just write field.x?
What is cub? Is it representing a cube? If so, don’t arbitrarily remove a single letter from a word, you might regret it later.
Furthermore, if you have a variable holding a coordinate, use x and y instead of i.
Have functions return values
Your functions don’t return anything, but rather modify the object pointed to. Although you might think that keeps things together, it actually makes them harder to use, and will likely cause lots of temporary variables to be put into t_cub3d, which will waste memory. Try to have functions return their results, and only pass those variables to them that they need to use.
For example, calculate_cam() seems to only calculate x_camera as a temporary value, and the only thing used by other code (as far as I can see) is the ray’s direction. So, assuming cub->ray has type r_ray, I would write:
t_ray calculate_cam(const t_cub3d *cub, int x)
{
int x_camera = 2.0 * x / cub->window.res_width - 1;
t_ray ray;
ray.dir_ray_x = cub->player.x_dir + cub->camera.x_plane * x_camera;
ray.dir_ray_y = cub->player.y_dir + cub->camera.y_plane * x_camera;
return ray;
}
Use bool where appropriate
When you have a variable that hold a value that should mean “true” or “false”, use bool from <stdbool.h>. For example, is_wall is a good candidate for that.
Prefer for over while where appropriate
The advantage of a for-statement is that you can clearly put the initial value, the end condition and the increment at the top of the loop. So in calculate(), I would write:
for (int i = 0; i < cub->window.res_width; i++)
{
...
}