3
\$\begingroup\$

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 <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;
    }
    al_draw_bitmap(car, playerPosX, playerPosY, 0);
    al_draw_bitmap(object, objectX, objectY, 0);
    al_flip_display();
}

al_flip_display();
al_rest(10);
}
\$\endgroup\$
  • \$\begingroup\$ not against Allegro5 but have you tried SFML since you were interesting in C++ \$\endgroup\$ – MORTAL Apr 24 '16 at 15:36
  • \$\begingroup\$ Yes, I have, and I made some games with it using primitives, and I actually found it much easier than Allegro5, but I had problems displaying bitmaps, and I couldn't find any solutions to it no matter how much or what I tried and no matter where I asked, so I switched to Allegro5. \$\endgroup\$ – Luna Apr 24 '16 at 15:42
  • \$\begingroup\$ you can use sf::Image to display bitmap format. they have a website already, you can ask help from them in en.sfml-dev.org/forums/index.php. \$\endgroup\$ – MORTAL Apr 24 '16 at 15:49
  • 1
    \$\begingroup\$ Have you learned anything about classes yet? You have at least 2 candidates for classes, object and player. \$\endgroup\$ – pacmaninbw Apr 24 '16 at 18:01
  • \$\begingroup\$ Oh yes, but unfortunately, I'm not yet very good at using them, however, I will try to use them later on in my next project in order to train myself to use them, thank you! \$\endgroup\$ – Luna Apr 24 '16 at 23:06
5
\$\begingroup\$

indentation

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

#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;
        }
        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

\$\endgroup\$
  • \$\begingroup\$ Thank you so much! I'll definitely try my best to follow the tips. \$\endgroup\$ – Luna Apr 24 '16 at 15:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.