HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Race car with falling objects that make the car go back to its initial position game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thecarobjectswithfallingmakepositionbackitsgame

Problem

I saw a game made in Python and PyGame in a tutorial, so I thought I'd make a game like it in Allegro5 and C++. I think I made my style a little better than my style before since I stopped putting everything in the main function, and I also stopped using 'magic numbers'. I'd like to know of your opinions, and what else I can improve to make my code better.

```
#include
#include
#include
#include
#include
#include
#include
#include
ALLEGRO_DISPLAY *display;
ALLEGRO_EVENT ev;
ALLEGRO_EVENT_QUEUE *eventqueue;
ALLEGRO_TIMER *timer;
ALLEGRO_BITMAP *car;
ALLEGRO_BITMAP *object;
float FPS = 60.0;
int width = 800;
int height = 600;
int objectX = 810;
int objectY = 610;
int objectYStep = 5;
int playerPosX = width / 2;
int playerPosY = 510;
int playerStep = 10;
bool isOpen = true;
void moveObject(){
objectY = objectY + objectYStep;
}
void movePlayerRight(){
playerPosX = playerPosX + playerStep;
}
void movePlayerLeft(){
playerPosX = playerPosX - playerStep;
}
bool objectOutOfBounds(){
return objectX > width || objectY > height;
}
bool intersects(ALLEGRO_BITMAP objecte, ALLEGRO_BITMAP cars, int objecteX, int objecteY, int carsX, int carsY){
int objecteW = al_get_bitmap_width(objecte);
int objecteH = al_get_bitmap_height(objecte);
int careW = al_get_bitmap_width(cars);
int careH = al_get_bitmap_height(cars);
return objecteXcarsX && objecteYcarsY;
}

int main(){

al_init();
if (!al_init()){
std::cout << "Failed to initialize allegro5." << std::endl;
}
al_init_image_addon();
if (!al_init_image_addon()){
std::cout << "Failed to initialize image." << std::endl;
}
display = al_create_display(width, height);
if (!display){
std::cout << "Failed to create display." << std::endl;
}
al_install_keyboard();
if (!al_install_keyboard()){
std::cout << "Failed to install keyboard." << std::endl;
}

eventqueue = al_create_event_queue();
timer = al_create_timer(1.0 / FPS);
car = al_load_bitmap("racecar.png");
object = al_load_bitmap("brickonn.jpg");

al_register_

Solution

indentation

A little bit of whitespace goes a long way in giving your code a visual structure. This makes the code more readable:

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

ALLEGRO_DISPLAY *display;
ALLEGRO_EVENT ev;
ALLEGRO_EVENT_QUEUE *eventqueue;
ALLEGRO_TIMER *timer;
ALLEGRO_BITMAP *car;
ALLEGRO_BITMAP *object;

float FPS = 60.0;
int width = 800;
int height = 600;
int objectX = 810;
int objectY = 610;
int objectYStep = 5;
int playerPosX = width / 2;
int playerPosY = 510;
int playerStep = 10;
bool isOpen = true;

void moveObject(){
    objectY = objectY + objectYStep;
}

void movePlayerRight(){
    playerPosX = playerPosX + playerStep;
}

void movePlayerLeft(){
    playerPosX = playerPosX - playerStep;
}

bool objectOutOfBounds(){
    return objectX > width || objectY > height;
}

bool intersects(ALLEGRO_BITMAP *objecte, ALLEGRO_BITMAP *cars, int objecteX, int objecteY, int carsX, int carsY){
    int objecteW = al_get_bitmap_width(objecte);
    int objecteH = al_get_bitmap_height(objecte);
    int careW = al_get_bitmap_width(cars);
    int careH = al_get_bitmap_height(cars);
    return objecteXcarsX && objecteYcarsY;
}

int main(){
    al_init();
    if (!al_init()){
        std::cout << "Failed to initialize allegro5." << std::endl;
    }
    al_init_image_addon();
    if (!al_init_image_addon()){
        std::cout << "Failed to initialize image." << std::endl;
    }
    display = al_create_display(width, height);
    if (!display){
        std::cout << "Failed to create display." << std::endl;
    }
    al_install_keyboard();
    if (!al_install_keyboard()){
        std::cout << "Failed to install keyboard." << std::endl;
    }

    eventqueue = al_create_event_queue();
    timer = al_create_timer(1.0 / FPS);
    car = al_load_bitmap("racecar.png");
    object = al_load_bitmap("brickonn.jpg");

    al_register_event_source(eventqueue, al_get_display_event_source(display));
    al_register_event_source(eventqueue, al_get_keyboard_event_source());
    al_register_event_source(eventqueue, al_get_timer_event_source(timer));

    while (isOpen){
        al_clear_to_color(al_map_rgb(255, 255, 0));
        ALLEGRO_EVENT event;
        al_wait_for_event(eventqueue, &event);
        al_start_timer(timer);
        if (objectOutOfBounds()){
            objectX = rand() % width;
            objectY = 0;
            objectYStep = objectYStep + 1;
        }
        if (event.type == ALLEGRO_EVENT_TIMER){
            moveObject();
        }
        if (event.keyboard.keycode == ALLEGRO_KEY_LEFT){
            movePlayerLeft();
        }
        else if (event.keyboard.keycode == ALLEGRO_KEY_RIGHT){
            movePlayerRight();
        }
        if (intersects(object, car, objectX, objectY, playerPosX, playerPosY)){
            std::cout << "CRASH!" << std::endl;
            playerPosX = width / 2;
        }
        al_draw_bitmap(car, playerPosX, playerPosY, 0);
        al_draw_bitmap(object, objectX, objectY, 0);
        al_flip_display();
    }

    al_flip_display();
    al_rest(10);
}


Pick a coding style and consistently use it. Prefer a coding style that's common or "standard" in your language. It greatly helps when getting help from people. If your code is easy to digest, it's more likely somebody will do so.

comments

There are no comments. That's a problem. Add some comments to your code to explain what's going on.

more functions


I stopped putting everything in the main function

That's a good idea. You can go even further. Look at the first part of your main() function. If you had to put a comment over it, it'd probably look like that:

// initialisation
al_init();
if (!al_init()){
    std::cout << "Failed to initialize allegro5." << std::endl;
}
al_init_image_addon();
if (!al_init_image_addon()){
    std::cout << "Failed to initialize image." << std::endl;
}
display = al_create_display(width, height);
if (!display){
    std::cout << "Failed to create display." << std::endl;
}
al_install_keyboard();
if (!al_install_keyboard()){
    std::cout << "Failed to install keyboard." << std::endl;
}

eventqueue = al_create_event_queue();
timer = al_create_timer(1.0 / FPS);
car = al_load_bitmap("racecar.png");
object = al_load_bitmap("brickonn.jpg");

al_register_event_source(eventqueue, al_get_display_event_source(display));
al_register_event_source(eventqueue, al_get_keyboard_event_source());
al_register_event_source(eventqueue, al_get_timer_event_source(timer));


Instead, put all this code in a function and only invoke initialise() from main(). This takes away the gory details when reading the code at first glance. Ok, there's an initialisation going on. Am I interested in that? Yes -> dive into the function, No -> go ahead with main

Code Snippets

#include <stdio.h>
#include <allegro5/allegro.h>
#include <allegro5/allegro_font.h>
#include <allegro5/allegro_primitives.h>
#include <allegro5/allegro_image.h>
#include <allegro5/allegro_color.h>
#include <allegro5/allegro_ttf.h>
#include <iostream>

ALLEGRO_DISPLAY *display;
ALLEGRO_EVENT ev;
ALLEGRO_EVENT_QUEUE *eventqueue;
ALLEGRO_TIMER *timer;
ALLEGRO_BITMAP *car;
ALLEGRO_BITMAP *object;

float FPS = 60.0;
int width = 800;
int height = 600;
int objectX = 810;
int objectY = 610;
int objectYStep = 5;
int playerPosX = width / 2;
int playerPosY = 510;
int playerStep = 10;
bool isOpen = true;

void moveObject(){
    objectY = objectY + objectYStep;
}

void movePlayerRight(){
    playerPosX = playerPosX + playerStep;
}

void movePlayerLeft(){
    playerPosX = playerPosX - playerStep;
}

bool objectOutOfBounds(){
    return objectX > width || objectY > height;
}

bool intersects(ALLEGRO_BITMAP *objecte, ALLEGRO_BITMAP *cars, int objecteX, int objecteY, int carsX, int carsY){
    int objecteW = al_get_bitmap_width(objecte);
    int objecteH = al_get_bitmap_height(objecte);
    int careW = al_get_bitmap_width(cars);
    int careH = al_get_bitmap_height(cars);
    return objecteX<careW + carsX && objecteX + objecteW>carsX && objecteY<careH + carsY && objecteX + objecteH>carsY;
}

int main(){
    al_init();
    if (!al_init()){
        std::cout << "Failed to initialize allegro5." << std::endl;
    }
    al_init_image_addon();
    if (!al_init_image_addon()){
        std::cout << "Failed to initialize image." << std::endl;
    }
    display = al_create_display(width, height);
    if (!display){
        std::cout << "Failed to create display." << std::endl;
    }
    al_install_keyboard();
    if (!al_install_keyboard()){
        std::cout << "Failed to install keyboard." << std::endl;
    }

    eventqueue = al_create_event_queue();
    timer = al_create_timer(1.0 / FPS);
    car = al_load_bitmap("racecar.png");
    object = al_load_bitmap("brickonn.jpg");

    al_register_event_source(eventqueue, al_get_display_event_source(display));
    al_register_event_source(eventqueue, al_get_keyboard_event_source());
    al_register_event_source(eventqueue, al_get_timer_event_source(timer));

    while (isOpen){
        al_clear_to_color(al_map_rgb(255, 255, 0));
        ALLEGRO_EVENT event;
        al_wait_for_event(eventqueue, &event);
        al_start_timer(timer);
        if (objectOutOfBounds()){
            objectX = rand() % width;
            objectY = 0;
            objectYStep = objectYStep + 1;
        }
        if (event.type == ALLEGRO_EVENT_TIMER){
            moveObject();
        }
        if (event.keyboard.keycode == ALLEGRO_KEY_LEFT){
            movePlayerLeft();
        }
        else if (event.keyboard.keycode == ALLEGRO_KEY_RIGHT){
            movePlayerRight();
        }
        if (intersects(object, car, objectX, objectY, playerPosX, playerPosY)){
            std::cout << "CRASH!" << std::endl;
            playerPosX = width / 2;
     
// initialisation
al_init();
if (!al_init()){
    std::cout << "Failed to initialize allegro5." << std::endl;
}
al_init_image_addon();
if (!al_init_image_addon()){
    std::cout << "Failed to initialize image." << std::endl;
}
display = al_create_display(width, height);
if (!display){
    std::cout << "Failed to create display." << std::endl;
}
al_install_keyboard();
if (!al_install_keyboard()){
    std::cout << "Failed to install keyboard." << std::endl;
}

eventqueue = al_create_event_queue();
timer = al_create_timer(1.0 / FPS);
car = al_load_bitmap("racecar.png");
object = al_load_bitmap("brickonn.jpg");

al_register_event_source(eventqueue, al_get_display_event_source(display));
al_register_event_source(eventqueue, al_get_keyboard_event_source());
al_register_event_source(eventqueue, al_get_timer_event_source(timer));

Context

StackExchange Code Review Q#126568, answer score: 5

Revisions (0)

No revisions yet.