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++)
{
...
}
```