15
\$\begingroup\$

I created a simple 2D game that I made for myself in C++ with SDL2.0 + OpenGL + Box2D, but I'm insecure about the code and structure because I developed the game using everything I've learned on the Internet and not following some pattern. I posted the code on GitHub and I'm spreading it on some forums to get feedback about the code, but here I'll just post the parts that I consider the more important for this.

About the game: The game is a battle between two players (soldiers) that must destroy each other's tanks, but the tank is only available to attack when the tank defensor is dead (which lasts for 5 seconds). When the tank is destroyed, the tank defensor lost the game. While the game is running, there are some boosts that appear randomly on the ground. The boosts are: speed (accumulate until two), defense and life.

I would like to have it analyzed by: viability of some operations, for example, the bullets vector and the way I'm doing it, with the FPS calculator and the painter methods.

The main loop:

#include <SDL.h>

#include <iostream>
#include <stdexcept>
#include <string>

#include "game.hpp"

SDL_Window* initSDL(const std::string& title, unsigned int w, unsigned int    h);
SDL_GLContext initOpenGL(SDL_Window* window);
void events(LoopHandler* lhandler);
void logics(LoopHandler* lhandler);
void render(LoopHandler* lhandler);

int main(int argc, char* argv[]){
//fps counter
unsigned int fps_c = 0;
Timer fps_ct;

const unsigned int DISERED_DELAY = 25;
unsigned int start, end, loop_t, excess = 0;

SDL_Event event;
LoopHandler* handler = nullptr;
SDL_Window* window = nullptr;
SDL_GLContext glc = nullptr;

try{
    window = initSDL("Hardcore!", 500, 500);
    glc= initOpenGL(window);

    handler = new Game();

    handler->pre();
    fps_ct.start();
    while (handler->active()){
        start = SDL_GetTicks();

        while (SDL_PollEvent(&event)){
            handler->process(&event);
        }

        while (excess < DISERED_DELAY){
            handler->logics();
            excess -= DISERED_DELAY;
        }

        handler->logics();

        glClear(GL_COLOR_BUFFER_BIT);
        handler->render();
        SDL_GL_SwapWindow(window);

        end = SDL_GetTicks();
        loop_t = end - start;
        if (loop_t < DISERED_DELAY){
            SDL_Delay(DISERED_DELAY - loop_t);
        }
        else{
            excess += DISERED_DELAY - loop_t;
        }

        if (fps_ct.compare(1000)){
            system("cls");
            std::cout << "fps: " << fps_c << std::endl;
            fps_c = 0;
        }

        fps_c++;
    }
    handler->pos();
}
catch (std::exception& ex){
    SDL_ShowSimpleMessageBox(::SDL_MessageBoxFlags::SDL_MESSAGEBOX_ERROR, "Error", ex.what(), window);
}

delete handler;
SDL_GL_DeleteContext(glc);
SDL_DestroyWindow(window);

return 0;
}

SDL_Window* initSDL(const std::string& title, unsigned int w, unsigned int h){
SDL_Window* window;

if (SDL_Init(SDL_INIT_EVERYTHING) == -1)
    throw std::runtime_error(SDL_GetError());

window = SDL_CreateWindow(title.c_str(), SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, w, h, SDL_WINDOW_SHOWN | SDL_WINDOW_OPENGL);

if (window == NULL)
    throw std::runtime_error(SDL_GetError());

return window;
}

SDL_GLContext initOpenGL(SDL_Window* window){
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 2);
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 1);
SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1);

glClearColor(1.0, 0.0, 0.0, 1.0);

return SDL_GL_CreateContext(window);
}

The game controller:

#include "game.hpp"

#include "random.hpp"
#include "game_textures.hpp"
#include "boost.hpp"

void Game::pre(){
camera.set(50, 50);
painter.setCamera(camera);

glClearColor(0.50, 0.50, 0.50, 1.0f);

Random::genSeed();

/*create the world with no gravity*/
world = new b2World(b2Vec2(0.0f, 0.0f));
world->SetContactListener(this);
world->SetContactFilter(this);

/*Create world bounds*/
b2Vec2 vertices[] = { b2Vec2(camera.getLeft(), camera.getTop()),   b2Vec2(camera.getRight(), camera.getTop()),
                      b2Vec2(camera.getRight(), camera.getBottom()), b2Vec2(camera.getLeft(), camera.getBottom())};

b2BodyDef bdef;
bdef.type = b2_staticBody;
world_bounds = world->CreateBody(&bdef);

b2ChainShape cshape;
cshape.CreateLoop(vertices, 4);

b2FixtureDef fdef;
fdef.shape = &cshape;
fdef.friction = 0.0f;
fdef.filter.categoryBits = SCENARIO;

world_bounds->CreateFixture(&fdef);

/*textures*/
GameTextures::simple_bullet =   loadTexture("simple_bullet.png");
GameTextures::speed_boost   =   loadTexture("speed_boost.png");
GameTextures::defense_boost =   loadTexture("defense_boost.png");
GameTextures::life_boost    =   loadTexture("life_boost.png");
GameTextures::soldier       =   loadTexture("soldier.png");
GameTextures::tank          =   loadTexture("tank.jpg");

/*players*/
p1 = new Soldier(this, FIGHTER1, {SDLK_UP,
                         SDLK_DOWN,
                         SDLK_LEFT,
                         SDLK_RIGHT,
                         SDLK_m,
                         SDLK_m
                });

p2 = new Soldier(this, FIGHTER2, {SDLK_w,
                         SDLK_s,
                         SDLK_a,
                         SDLK_d,
                         SDLK_q,
                         SDLK_e
                });

p1->setEnemy(p2);
p2->setEnemy(p1);

tank1 = new Tank(this, p1);
tank1->setEnemy(p2);

tank2 = new Tank(this, p2);
tank2->setEnemy(p1);

p1->setTank(tank1);
p2->setTank(tank2);

start();
}

void Game::process(SDL_Event* event){
switch (event->type){
case SDL_QUIT:
    running = false;
    break;
case SDL_KEYDOWN:
    p1->keyDown(&event->key);
    p2->keyDown(&event->key);
    break;
case SDL_KEYUP:
    p1->keyUp(&event->key);
    p2->keyUp(&event->key);
    break;
}
}

void Game::logics(){
if (boost_t.compare(to_next_boost)){
    TimedBoostItem* t = new TimedBoostItem(this);

    t->start();
    boosts.push_back(t);
    to_next_boost = Random::genUInteger(5000, 20000);
}

for (unsigned int i = 0; i < boosts.size(); i++){
    TimedBoostItem* t = boosts[i];

    t->calculate();

    if (t->getTState() == ENDED){
        boosts.erase(boosts.begin() + i);
        i--;
        delete t;
    }
}

tank1->calculate();
tank2->calculate();

p1->calculate();
if (p1t.isRunning()){
    if (p1t.compare(5000)){
        preparePlayer(p1);
        tank1->setTankState(INVULNERABLE);
    }
}else if (p1->getState() == DEAD){
    setToSleep(p1);
    tank1->setTankState(VULNERABLE);
}

p2->calculate();
if (p2t.isRunning()){
    if (p2t.compare(5000)){
        preparePlayer(p2);
        tank2->setTankState(INVULNERABLE);
    }
}else if (p2->getState() == DEAD){
    setToSleep(p2);
    tank2->setTankState(VULNERABLE);
}

if (tank1->getState() == DEAD && tank2->getState() == DEAD){
      SDL_ShowSimpleMessageBox(SDL_MessageBoxFlags::SDL_MESSAGEBOX_INFORMATION, "Fim!", "Empate!", NULL);
    start();
}
else if(tank1->getState() == DEAD){
    SDL_ShowSimpleMessageBox(SDL_MessageBoxFlags::SDL_MESSAGEBOX_INFORMATION, "Fim!", "Player 2 destruiu o tank!", NULL);
    start();
}else if (tank2->getState() == DEAD){
     SDL_ShowSimpleMessageBox(SDL_MessageBoxFlags::SDL_MESSAGEBOX_INFORMATION, "Fim!", "Player 1 destruiu o tank!", NULL);
    start();
}


world->Step(1.0f / 40.0f, 2, 6);
}

void Game::render(){
/*draw background*/

for (TimedBoostItem* t : boosts){
    t->render(&painter);
}

p1->render(&painter);
p2->render(&painter);
tank1->render(&painter);
tank2->render(&painter);

/*draw HUD*/
}

void Game::pos(){
delete tank1;
delete tank2;
delete p1;
delete p2;

delete GameTextures::simple_bullet;
delete GameTextures::speed_boost;
delete GameTextures::defense_boost;
delete GameTextures::life_boost;
delete GameTextures::soldier;
delete GameTextures::tank;
}

bool Game::active(){
return running;
}

Camera2D Game::getCamera()const{
return camera;
}

void Game::start(){
preparePlayer(p1);
preparePlayer(p2);

tank1->load();
tank1->setPosition(PointF(10.0f, 17.0f));

tank2->load();
tank2->setPosition(PointF(-10.0f, -17.0f));

running = true;

to_next_boost = Random::genUInteger(5000, 20000);

world->ClearForces();

boost_t.start();
}

void Game::preparePlayer(Soldier* soldier){
soldier->unload();
soldier->load();
if (soldier->getCategory() == FIGHTER2){
    p2t.stop();
}
else{
    p1t.stop();
}

soldier->setPosition(
    PointF(
    Random::genInteger(camera.getLeft() + soldier->getArea().getWidth() / 2.0f, camera.getRight() - soldier->getArea().getWidth() / 2.0f),
    Random::genInteger(-17 + soldier->getArea().getHeight() / 2.0f, 17 + soldier->getArea().getHeight() / 2.0f)));
}
void Game::setToSleep(Soldier* soldier){
soldier->unload();
soldier->getMachineGun()->stopShooting();
if (soldier->getCategory() == FIGHTER2){
    p2t.start();
}
else{
    p1t.start();
}
}


void Game::BeginContact(b2Contact* contact){
b2Body* a = contact->GetFixtureA()->GetBody();
b2Body* b = contact->GetFixtureB()->GetBody();
GameData* gdata_a = (GameData*)a->GetUserData();
GameData* gdata_b = (GameData*)b->GetUserData();

if (gdata_a != nullptr)
    gdata_a->thing->onCollide(gdata_b);

if (gdata_b != nullptr)
    gdata_b->thing->onCollide(gdata_a);
}

void Game::EndContact(b2Contact* contact){

}

bool Game::ShouldCollide(b2Fixture* fixtureA, b2Fixture* fixtureB){
b2Body* a = fixtureA->GetBody();
b2Body* b = fixtureB->GetBody();
GameData* gdata_a = (GameData*)a->GetUserData();
GameData* gdata_b = (GameData*)b->GetUserData();

if (gdata_b != nullptr && gdata_a != nullptr){
    if (gdata_a->bits == BULLET){
        /*avoid bullet collision with its shooter and TIMED_BOOST_ITEM(bit masks is not working :s)*/
        if (gdata_b->thing == ((Bullet*)gdata_a->thing)->getShooter() || gdata_b->bits == TIMED_BOOST_ITEM){
            return false;
        }
    }
    else if (gdata_b->bits == BULLET){
        if (gdata_a->thing == ((Bullet*)gdata_b->thing)->getShooter() || gdata_a->bits == TIMED_BOOST_ITEM){
            return false;
        }
    }
}

return true;
}

The player controller:

#include "soldier.hpp"

#include "game_textures.hpp"

Soldier::Soldier(Game* game, unsigned short category, ControlKeys controls) : Fighter(game, category){
this->mgun = nullptr;
this->control_keys = controls;
this->my = GameTextures::soldier;
}

void Soldier::load(){
if (mgun == nullptr){
    mgun = new MachineGun(this);
}
else{
    mgun->freeBullets();
}

/*set default bullets*/
SimpleBullet* sb = new SimpleBullet(this);
mgun->charge(sb);
delete sb;

my = GameTextures::soldier;
direction = DOWN;
state = STOPPED;
area.setWidth(3.0f);
area.setHeight(3.0f);
speed = c_speed = 1.0f;
setDefense(1.0f);
setLife(1.0f);
points = 0;

if (body == nullptr)
    this->createBody(SizeF(area.getWidth(), area.getHeight()), b2BodyType::b2_dynamicBody, category, getEnemyCategory() | BULLET | TANK | TIMED_BOOST_ITEM | SCENARIO, false);
}

void Soldier::calculate(){
/*calculate bullets*/
mgun->calculate();

/*calculate effects*/
for (unsigned int i = 0; i < effects.size(); i++){
    Effect* e = effects[i];
    e->calculate();

    if (e->getTState() == TIMER_STATE::ENDED){
        effects.erase(effects.begin() + i);
        delete e;
    }
}

calculateFaceDirection();

if (state != DEAD){
    /*update position*/
    area.setX(body->GetPosition().x - area.getWidth() / 2);
    area.setY(body->GetPosition().y - area.getHeight() / 2);
}
}

void Soldier::render(Painter* painter){
mgun->render(painter);

if (state != DEAD){
    painter->loadIdentity();
    painter->setColor(Color(0.0f, 0.0f, 0.0f));
    painter->drawRect(AreaF(area.getX(), area.getY() + area.getHeight() + 0.1f, area.getWidth(), 1.0f));

    painter->loadIdentity();
    painter->setColor(Color(0.0f, 1.0f, 0.0f));
    painter->drawRect(AreaF(area.getX(), area.getY() + area.getHeight() + 0.1f, area.getWidth() * clife, 1.0f));

    painter->loadIdentity();
    switch (face_direction){
    case UP:
        painter->drawTexture(my, SizeF(area.getWidth(), area.getHeight()), getPosition(), 0.0f);
        break;
    case DOWN:
        painter->drawTexture(my, SizeF(area.getWidth(), area.getHeight()), getPosition(), 180.0f);
        break;
    case LEFT:
        painter->drawTexture(my, SizeF(area.getWidth(), area.getHeight()), getPosition(), 90.0f);
        break;
    case RIGHT:
        painter->drawTexture(my, SizeF(area.getWidth(), area.getHeight()), getPosition(), 270.0f);
        break;
    }
}
else{
    painter->loadIdentity();
    painter->setColor(Color(0.3f, 0.3f, 0.3f, 0.5f));
    painter->drawRect(area);
}
}

void Soldier::unload(){
destroyBody();

for (Effect* e : effects){
    delete e;
}

effects.clear();
}

void Soldier::calculateFaceDirection(){
float dx, dy, x, y, r, b;

if (enemy->getState() != DEAD){
    x = enemy->getPosition().getX();
    y = enemy->getPosition().getY();
    r = enemy->getArea().getRight();
    b = enemy->getArea().getBottom();
}
else{
    x = ((Soldier*)enemy)->getTank()->getPosition().getX();
    y = ((Soldier*)enemy)->getTank()->getPosition().getY();
    r = ((Soldier*)enemy)->getTank()->getArea().getRight();
    b = ((Soldier*)enemy)->getTank()->getArea().getBottom();
}

dx = area.getX() - x;
dy = area.getY() - y;

if ((dx >= 0 && dy >= 0)){
    if (r < area.getX() && b < area.getY()){
        if (direction == RIGHT || direction == LEFT){
            face_direction = DOWN;
        }
        else{
            face_direction = LEFT;
        }
    }
    else if(r > area.getX()){
        face_direction = DOWN;
    }
    else/*b > area.getBottom()*/{
        face_direction = LEFT;
    }
}
else if ((dx < 0 && dy >= 0)){
    if (x > area.getRight() && b < area.getY()){
        if (direction == RIGHT || direction == LEFT){
            face_direction = DOWN;
        }
        else{
            face_direction = RIGHT;
        }
    }
    else if (x < area.getRight()){
        face_direction = DOWN;
    }
    else/*b > area.getBottom()*/{
        face_direction = RIGHT;
    }
}
else if (dx < 0 && dy <= 0){
    if (x > area.getRight() && y > area.getBottom()){
        if (direction == RIGHT || direction == LEFT){
            face_direction = UP;
        }
        else{
            face_direction = RIGHT;
        }
    }
    else if (x < area.getRight()){
        face_direction = UP;
    }
    else/*y < area.getBottom()*/{
        face_direction = RIGHT;
    }
}
else if (dx >= 0 && dy <= 0){
    if (r < area.getX() && y > area.getBottom()){
        if (direction == RIGHT || direction == LEFT){
            face_direction = UP;
        }
        else{
            face_direction = LEFT;
        }
    }
    else if (r > area.getX()){
        face_direction = UP;
    }
    else/*y < area.getBottom()*/{
        face_direction = LEFT;
    }
}
}

void Soldier::keyDown(SDL_KeyboardEvent* event){
SDL_Keycode key = event->keysym.sym;

if (state == DEAD)
    return;

if (key == control_keys.up){
    direction = UP;
    state = WALKING;
    body->SetLinearVelocity(b2Vec2(0.0f, 30.0f * c_speed));
}
else if (key == control_keys.down){
    direction = DOWN;
    state = WALKING;
    body->SetLinearVelocity(b2Vec2(0.0, -30.0f * c_speed));
}
else if (key == control_keys.left){
    direction = LEFT;
    state = WALKING;
    body->SetLinearVelocity(b2Vec2(-30.0f * c_speed, 0.0f));
}
else if (key == control_keys.right){
    direction = RIGHT;
    state = WALKING;
    body->SetLinearVelocity(b2Vec2(30.0f * c_speed, 0.0f));
}
else if (key == control_keys.shoot){
    mgun->startShooting();
}
}

void Soldier::keyUp(SDL_KeyboardEvent* event){
SDL_Keycode key = event->keysym.sym;

    if (state == DEAD)
        return;

if (key == control_keys.up){
    if (state == WALKING && direction == UP){
        state = STOPPED;
        body->SetLinearVelocity(b2Vec2(0.0f, 0.0f));
    }
}
else if (key == control_keys.down){
    if (state == WALKING && direction == DOWN){
        state = STOPPED;
        body->SetLinearVelocity(b2Vec2(0.0f, 0.0f));
    }
}
else if (key == control_keys.left){
    if (state == WALKING && direction == LEFT){
        state = STOPPED;
        body->SetLinearVelocity(b2Vec2(0.0f, 0.0f));
    }
}
else if (key == control_keys.right){
    if (state == WALKING && direction == RIGHT){
        state = STOPPED;
        body->SetLinearVelocity(b2Vec2(0.0f, 0.0f));
    }
}
else if (key == control_keys.shoot){
    mgun->stopShooting();
}
}

The Painter class:

#include "painter.hpp"

Painter::Painter(){}
Painter::~Painter(){}

void Painter::rotate(float angle){
/*rotate from z-axis, thats all needed for this game*/
glRotatef(angle, 0.0f, 0.0f, 1.0f);
}

void Painter::translate(float tx, float tz, float ty){
glTranslatef(tx, ty, tz);
}

void Painter::scale(float sx, float, float sy, float sz){
glScalef(sx, sy, sz);
}

void Painter::drawTexture(Texture2D* texture, const PointF& position){
camera.set(camera.getWidth(), camera.getHeight());

glEnable(GL_TEXTURE_2D);
texture->bind();
glBegin(GL_QUADS);
glTexCoord2f(0.0f, 1.0f);
glVertex2f(position.getX(), position.getY());
glTexCoord2f(1.0f, 1.0f);
glVertex2f(position.getX() + texture->getWidth(), position.getY());
glTexCoord2f(1.0f, 0.0f);
glVertex2f(position.getX() + texture->getWidth(), position.getY() + texture->getHeight());
glTexCoord2f(0.0f, 0.0f);
glVertex2f(position.getX(), position.getY() + texture->getHeight());
glEnd();
glDisable(GL_TEXTURE_2D);
}

void Painter::drawTexture(Texture2D* texture, const AreaF& area, const PointF& position){
camera.set(camera.getWidth(), camera.getHeight());
glEnable(GL_TEXTURE_2D);
texture->bind();
glBegin(GL_QUADS);
glTexCoord2f(0.0f, 1.0f/area.getY());
glVertex2f(position.getX(), position.getY());
glTexCoord2f(1.0f / area.getX(), 1.0f / area.getY());
glVertex2f(position.getX() + area.getWidth(), position.getY());
glTexCoord2f(1.0f / area.getX(), 0.0f);
glVertex2f(position.getX() + area.getWidth(), position.getY() + area.getHeight());
glTexCoord2f(0.0f, 0.0f);
glVertex2f(position.getX(), position.getY() + area.getHeight());
glEnd();
glDisable(GL_TEXTURE_2D);
}

void Painter::drawTexture(Texture2D* texture, const SizeF& size, const PointF& position, float angle){
camera.set(camera.getWidth(), camera.getHeight());

if (angle != 0.0f){
    glTranslatef(position.getX() + size.getWidth() / 2, position.getY() + size.getHeight() / 2, 0.0f);
    glRotatef(angle, 0.0f, 0.0f, 1.0f);
    glTranslatef(-(position.getX() + size.getWidth() / 2), -(position.getY() + size.getHeight()/2), 0.0f);
}

glEnable(GL_BLEND);
glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
glEnable(GL_TEXTURE_2D);
texture->bind();
glBegin(GL_QUADS);
glColor4f(1, 1, 1, 1);
glTexCoord2f(0.0f, 1.0f);
glVertex2f(position.getX(), position.getY());
glTexCoord2f(1.0f, 1.0f);
glVertex2f(position.getX() + size.getWidth(), position.getY());
glTexCoord2f(1.0f, 0.0f);
glVertex2f(position.getX() + size.getWidth(), position.getY() + size.getHeight());
glTexCoord2f(0.0f, 0.0f);
glVertex2f(position.getX(), position.getY() + size.getHeight());
glEnd();
glDisable(GL_TEXTURE_2D);
glDisable(GL_BLEND);
}

void Painter::drawTexture(Texture2D* texture, const AreaF& area, const SizeF& size, const PointF& position){
camera.set(camera.getWidth(), camera.getHeight());
glEnable(GL_TEXTURE_2D);
texture->bind();
glBegin(GL_QUADS);
glTexCoord2f(0.0f, 1.0f / area.getY());
glVertex2f(position.getX(), position.getY());
glTexCoord2f(1.0f / area.getX(), 1.0f / area.getY());
glVertex2f(position.getX() + size.getWidth(), position.getY());
glTexCoord2f(1.0f / area.getX(), 0.0f);
glVertex2f(position.getX() + size.getWidth(), position.getY() + size.getHeight());
glTexCoord2f(0.0f, 0.0f);
glVertex2f(position.getX(), position.getY() + size.getHeight());
glEnd();
glDisable(GL_TEXTURE_2D);
}

void Painter::drawRect(const AreaF& area){
camera.set(camera.getWidth(), camera.getHeight());

glBegin(GL_QUADS);
glColor4f(color.getR(), color.getG(), color.getB(), color.getA());
glVertex2f(area.getX(), area.getY());
glVertex2f(area.getX() + area.getWidth(), area.getY());
glVertex2f(area.getX() + area.getWidth(), area.getY() + area.getHeight());
glVertex2f(area.getX(), area.getY() + area.getHeight());
glEnd();
}
\$\endgroup\$
12
\$\begingroup\$

General Structure:

Indentation. Be consistent. It makes the code really hard to read when this is all over the place. Also add a layer of indentation after every block opens { and remove it when the block closes }.

Memory Management

You should not be using new/delete. Most of the time automatic variables are preferred. And when you want dynamically created objects std::unique_ptr generates code that is as quick as using raw pointers (if you can measure the difference I would be highly surprised (especially since 99% of the time they compile down to the same code).

LoopHandler* handler = nullptr;

try{

    handler = new Game();
}
catch(std::exception const& e)
{}

delete handler;

This code is not excetion safe (not all exceptions need to derive from std::exception. Just make this a local variable and all the problems of management disappear.

LoopHandler handler;

try{

}
catch(std::exception const& e)
{}

Why on why do this:

/*set default bullets*/
SimpleBullet* sb = new SimpleBullet(this);
mgun->charge(sb);                           
delete sb;

There are several problems with this one small bit of code. There are no ownership semantics defined for sb. You pass sb as a pointer to charge() so I am not 100% convinced that you need to delete them (I will now need to check how that method works to make sure you are doing it correctly). But assuming you are this means internally you are making a copy of sb so why do you need to create them dynamically like this and delete them if you are going to make a copy.

Those three lines just make my ears bleed.

I would have done.

/*set default bullets*/
mgun.charge(SimpleBullet(this));  // Pass by value (or ref)
                                  // This mean if `charge()` wants the bullets he needs to copy them

OR.

/*set default bullets*/
std::unique_ptr<SimpleBullet> sb(new SimpleBullet(this));
mgun.charge(std::move(sb));          // I am moving them to `charge()`
                                     // So he is taking ownership (anyway they are not coming back).
                                     // So unique_ptr will do nothing.

OR.

/*set default bullets*/
std::unique_ptr<SimpleBullet> sb(new SimpleBullet(this));
mgun.charge(sb);                    // I am passing a reference to charge (you can't copy).
                                    // This means `charge()` has the option of taking ownership.
                                    // If he does then its his responsibility to delete them
                                    // If not they stay in `sb` which will automatically
                                    // delete them in an exception safe way when it goes
                                    // out of scope.

Use RAII for Initialization/Destruction control.

When ever you see code that looks like this:

 void func()
 {
   DoSomeInit();

   CODE()

   SoSomeCleanUp();
 }

You are doing it wrong. This code is not exception safe. If CODE() throws then then cleanup will not be done. You should use RAII to do this work.

  class MyGameEnvironment
  {
      public:
          MyGameEnvironment()
          {
               DoSomeInit();
          }
          ~MyGameEnvironment()
          {
               SoSomeCleanUp();
          }
  };

  void func()
  {
    MyGameEnvironment  game;
    CODE();
  }

I know this looks like a lot of extra work. Its not really. And the style will save your ass more than you think it will. It also makes your functions smaller and easier to read.

Global Variables are a no-no

More precisely. Global mutable state is a bad thing. It makes your code hard to test. Hard to get correct and leads to spaghetti style code. Create your objects locally in a function and pass them around as parameters.

 int main()
 {
     Tank       tank1;
     Tank       tank2;
     MyGame     game(tank1, tank2);

     game.play();
 }
\$\endgroup\$
  • \$\begingroup\$ The charge method must receive a pointer to a Bullet(mother of all bullets), and there is a virtual method in the Bullet called buildNew that must return a pointer to a Bullet, and each child class returns a pointer to its own type, so doing this the mgun can handle any type of bullet and also can generate multiple bullets to shoot. Inside the charge method the buildNew method is called, i'm doing it so the soldier don't need to have a reference to the current bullet. But now i see that the way i was calling charge is weird. \$\endgroup\$ – user2542813 Nov 28 '15 at 22:50
  • \$\begingroup\$ About the pointers, i am just afraid to use the smart pointers because i dont see they being used very much, even in some open source projects, seem like that there is the right time to use them, but i'll study them more. I'm not using global variables, the closest i get to it is the GameTextures, and i'm glad you dont talk about them cause i was in doubt about this method, the RAII i always try to use it, but sometimes i just can't wait to code that becomes hard to organize all this and this escapes from my head... :s \$\endgroup\$ – user2542813 Nov 28 '15 at 22:55
  • 1
    \$\begingroup\$ @user2542813: They are not used much in C code. In C++ they are used all the time. They cost nothing (unique_ptr) and given the way you code will probably make your code faster. \$\endgroup\$ – Martin York Nov 28 '15 at 23:07

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.