A Flappy bird game using C++ and SFML

Question

I have written a Flappy Bird using SFML and C++, kindly review my code. I'm hoping for objective criticism.

main.cpp

#include "Game.h"

int main()
{
  Game game1;

  return 0;
}

Game.h

#pragma once

#include <SFML/Graphics.hpp>

#include "Definitions.h"
#include "ResourceHolder.h"
#include "StateMachine.h"

struct GameData
{
    sf::RenderWindow window;
    StateMachine machine;
 };

typedef std::shared_ptr<GameData> GameDataRef;

class Game
{
  private:
   const float dt = 1.0f / 60.0f;

   sf::Clock clock;

   GameDataRef data = std::make_shared<GameData>();

   int loadResource();

   void run();
 public:
   Game();
 };

Game.cpp

#include "Game.h"
#include "MainMenuState.h"

int Game::loadResource()
{
   auto& tH = getGlobalTextureHolder();
   auto& fH = getGlobalFontHolder();

   return
       tH.loadFromFile("Resource\\res\\birdAnimation.png", "bird") &&
       tH.loadFromFile("Resource\\res\\background.png", "back") &&
       tH.loadFromFile("Resource\\res\\ground.png", "ground") &&
       tH.loadFromFile("Resource\\res\\UpperPipe.png", "top_pipe") &&
       tH.loadFromFile("Resource\\res\\DownPipe.png", "down_pipe") &&
       tH.loadFromFile("Resource\\res\\StartButton.png", "start_button") &&
       tH.loadFromFile("Resource\\res\\FB_Image.png", "title") &&
       tH.loadFromFile("Resource\\res\\GameOver.png", "gameOver") &&
       tH.loadFromFile("Resource\\res\\board.png", "board") &&
       tH.loadFromFile("Resource\\res\\goldMedal.png", "gold_medal") &&
       tH.loadFromFile("Resource\\res\\silverMedal.png", "silver_medal") &&
       tH.loadFromFile("Resource\\res\\bronzeMedal.png", "bronze_medal") &&
       tH.loadFromFile("Resource\\res\\ironMedal.png", "iron_medal") &&
       fH.loadFromFile("Resource\\FlappyFont.ttf", "numbers");
  }

  void Game::run()
  {
     float newTime;
     float frameTime;
     float interpolation;

     float currentTime = this->clock.getElapsedTime().asSeconds();

     float accumulator = 0.0f;

     while (this->data->window.isOpen())
     {
         this->data->machine.processStateChanges();

         newTime = this- >clock.getElapsedTime().asSeconds();
         frameTime = newTime - currentTime;

         if (frameTime > 0.25f)
         {
             frameTime = 0.25f;
         }

         currentTime = newTime;
         accumulator += frameTime;

         while (accumulator >= dt)
         {
             this->data->machine.getActiveState()->handleInput();
             this->data->machine.getActiveState()->update(dt);

             accumulator -= dt;
         }

         interpolation = accumulator / dt;
         this->data->machine.getActiveState()->draw(interpolation);
       }
    }

 Game::Game()
 {
    srand(time(NULL));

    data->window.create(sf::VideoMode(windowWidth, windowHeight), gameTitle);
  
    data->machine.addState(StateRef(new MainMenuState(this->data)));

    if (!this->loadResource())
    {
       std::cout << "Fattal error";
    }

    this->run();
  }

Background.h

#pragma once

#include "Game.h"

class Background
{
 private:
   GameDataRef data;

   sf::Sprite s_back1;
   sf::Sprite s_back2;
 public:
   Background(GameDataRef _data);

   void generateBack(float x);

   void drawBack();
 };

Background.cpp

#include "Background.h"

Background::Background(GameDataRef _data) : data(_data)
{
   sf::Texture* back = getGlobalTextureHolder().getResource("back");

   sf::Vector2u TextureSize = back->getSize();

   float ScaleX = (float)windowWidth / (float)TextureSize.x;
   float ScaleY = (float)windowHeight / (float)TextureSize.y;

   s_back1.setTexture(*back);
   s_back2.setTexture(*back);
   s_back1.setScale(ScaleX, ScaleY);
   s_back2.setScale(ScaleX, ScaleY);
 }

void Background::generateBack(float x)
{
   if (x >= s_back1.getPosition().x + 230)
   {
       s_back2.setPosition(s_back1.getPosition().x + (float)windowWidth, 0.0f);
   }
   if (x >= s_back2.getPosition().x + 230)
   {
       s_back1.setPosition(s_back2.getPosition().x + (float)windowWidth, 0.0f);
   }
}

void Background::drawBack()
{
   data->window.draw(s_back1);
   data->window.draw(s_back2);
}

Bird.h

#pragma once

#include "Game.h"

class Bird
{
  private:
     GameDataRef data;

     sf::Sprite s_bird;
     sf::Clock clock;

     int currentBirdState;

     float currentFrame = 0.0f; //Current frame
     float rotation = 0.0f; //Variable for rotate
     float time = 0.0f; // Time
  public:
      Bird(GameDataRef _data);

      void updateBird(float dt);

      void tap();

      void animationBird(); //Animation bird

      float getX();

      float getY();

      void drawBird();

      const sf::Sprite& getSprite();
};

Bird.cpp

#include "Bird.h"

Bird::Bird(GameDataRef _data) : data(_data)
{
   sf::Texture* bird = getGlobalTextureHolder().getResource("bird");

   s_bird.setTexture(*bird);

   s_bird.setTextureRect(sf::IntRect(0, 0, 34, 24));

   s_bird.setPosition((float)windowWidth / 2.0f, (float)windowHeight / 2.0f);

   s_bird.setOrigin(s_bird.getGlobalBounds().width/2,s_bird.getGlobalBounds().height/2);

   currentBirdState = bStateStill;
 }

 void Bird::updateBird(float dt)
 {
   if (bStateStill == currentBirdState)
   {
       s_bird.move(moveSpeed, 0);
   }
   else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Space))
   {
       s_bird.move(moveSpeed, -flyingSpeed * dt);

       rotation -= rotationSpeed * dt;

       if (rotation < -25.0f)
       {
           rotation = -25.0f;
       }

       s_bird.setRotation(rotation);
   }
   else
   {
       s_bird.move(moveSpeed, g * dt);

       rotation += rotationSpeed * dt;

       if (rotation > 90.0f)
       {
          rotation = 90.0f;
       }

       s_bird.setRotation(rotation);
   }
 }

void Bird::tap()
{
   currentBirdState = bStateFlying;
}

void Bird::animationBird() //Animation bird
{
   currentFrame += 0.005f * time;

   if (currentFrame > 3)
   {
       currentFrame -= 3;
   }

   if (bStateStill == currentBirdState)
   {
       s_bird.setTextureRect(sf::IntRect(56 * (int)currentFrame, 0, 34, 24));
   }
   else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Space))
   {
       s_bird.setTextureRect(sf::IntRect(56 * (int)currentFrame, 0, 34, 24));
   }
   else
   {
       s_bird.setTextureRect(sf::IntRect(56, 0, 34, 24));
   }

   time = clock.getElapsedTime().asMicroseconds();
   clock.restart();
   time /= 800.0f;
}

float Bird::getX()
{
   return s_bird.getPosition().x;
}

float Bird::getY()
{
   return s_bird.getPosition().y;
}

void Bird::drawBird()
{
   data->window.draw(s_bird);
}

const sf::Sprite& Bird::getSprite()
{
   return s_bird;
}

Camera.h

#pragma once

#include "Game.h"

class Camera
{
  private:
    GameDataRef data;
    sf::View view;
  public:
    Camera(GameDataRef _data);

     void getBirdCoordinateForCamera(float x, float y);

     void setCamera();
 };

Camera.cpp

#include "Camera.h"

Camera::Camera(GameDataRef _data) : data(_data)
{
   view.setCenter(sf::Vector2f(windowWidth, windowHeight));
   view.setSize(sf::Vector2f(windowWidth, windowHeight));
}

void Camera::getBirdCoordinateForCamera(float x, float y)
{
   float tempX = x;
   float tempY = y;

   if (y < 320 || y > 320)
   {
       tempY = 320;
   }

   view.setCenter(tempX, tempY);
}

void Camera::setCamera()
{
   data->window.setView(view);
}

Collision.h

#pragma once

#include "Game.h"

class Collision
{
  public:
    Collision();

    bool checkSpriteCollision(sf::Sprite sprite1, float scale1, sf::Sprite sprite2, float scale2);
};

Collision.cpp

#include "Collision.h"

Collision::Collision()
{

}

bool Collision::checkSpriteCollision(sf::Sprite sprite1, float scale1, sf::Sprite sprite2, float scale2)
{
    sprite1.setScale(scale1, scale1);
    sprite2.setScale(scale2, scale2);

    sf::Rect<float> rect1 = sprite1.getGlobalBounds();
    sf::Rect<float> rect2 = sprite2.getGlobalBounds();

    if (rect1.intersects(rect2))
    {
        return true;
    }
    else
    {
        return false;
    }
}

Definitions.h

#pragma once

#define windowWidth 460
#define windowHeight 640
#define gameTitle "Flappy Bird"

#define g 180.0f //Gravity
#define flyingSpeed 180.0f //Vertical speed
#define rotationSpeed 200.0f //Rotation speed
#define moveSpeed 2.0f //Gorizontal speed

#define distanceBetweemPipes 50.0f
#define DISTANCE 256

#define bStateStill 1
#define bStateFlying 2

GameState.h

#pragma once

#include <SFML/Audio.hpp>

#include "State.h"
#include "Game.h"

#include "Background.h"
#include "Bird.h"
#include "Ground.h"
#include "Pipes.h"
#include "Camera.h"
#include "Collision.h"
#include "Hud.h"

class GameState : public State
{
private:
    GameDataRef data;

    Bird* bird1;
    Background* back1;
    Ground* ground1;
    Pipes* pipes1;
    Collision collis1;
    Camera* camera1;
    Hud *hud1;

    sf::Clock clock;

    sf::Sound so_die;
    sf::Sound so_hit;
    sf::Sound so_point;
    sf::Sound so_wing;

    sf::SoundBuffer die;
    sf::SoundBuffer hit;
    sf::SoundBuffer point;
    sf::SoundBuffer wing;

    enum GameStates
    {
        eReady,
        ePlaying,
        eGameOver
    };

    int currentGameState;
    int score;
    bool cState;
public:
    GameState(GameDataRef _data);

    void init();

    void handleInput();

    void update(float dt);

    void draw(float dt);
};

GameState.cpp

#include "GameState.h"
#include "GameOverState.h"

GameState::GameState(GameDataRef _data) : data(_data)
{

}

void GameState::init()
{
    bird1 = new Bird(data);
    back1 = new Background(data);
    ground1 = new Ground(data);
    pipes1 = new Pipes(data);
    camera1 = new Camera(data);
    hud1 = new Hud(data);
    currentGameState = eReady;
    cState = false;
    score = 0;
    hud1->updateScore(score);

    die.loadFromFile("Resource\\sounds\\sfx_die.ogg");
    hit.loadFromFile("Resource\\sounds\\sfx_hit.ogg");
    point.loadFromFile("Resource\\sounds\\sfx_point.ogg");
    wing.loadFromFile("Resource\\sounds\\sfx_wing.ogg");

    so_die.setBuffer(die);
    so_hit.setBuffer(hit);
    so_point.setBuffer(point);
    so_wing.setBuffer(wing);
}

void GameState::handleInput()
{
    sf::Event event;

    while (this->data->window.pollEvent(event))
    {
        if (sf::Event::Closed == event.type)
        {
            this->data->window.close();
        }

        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Space))
        {
            if (eGameOver != currentGameState)
            {
                currentGameState = ePlaying;
                bird1->tap();
                so_wing.play();
            }
        }
    }
}

void GameState::update(float dt)
{
    if (eGameOver != currentGameState)
    {
        bird1->animationBird();

        back1->generateBack(bird1->getX());
        ground1->generateGround(bird1->getX());
        hud1->move();

        camera1->getBirdCoordinateForCamera(bird1->getX(), bird1->getY());
        camera1->setCamera();
    }

    bird1->updateBird(dt);

    if (ePlaying == currentGameState)
    {
        pipes1->generatePipes(bird1->getX(), cState);
        cState = true;

        if (collis1.checkSpriteCollision(bird1->getSprite(), 0.7f, ground1->getFirstSprite(), 1.0f)
            || collis1.checkSpriteCollision(bird1->getSprite(), 0.7f, ground1->getSecondSprite(), 1.0f))
        {
            currentGameState = eGameOver;
            so_hit.play();
        }

        if (collis1.checkSpriteCollision(bird1->getSprite(), 0.625f, pipes1->getFirstTopSprite(), 1.0f)
            || collis1.checkSpriteCollision(bird1->getSprite(), 0.625f, pipes1->getFirstDownSprite(), 1.0f))
        {
            currentGameState = eGameOver;
            so_hit.play();
        }

        if (collis1.checkSpriteCollision(bird1->getSprite(), 0.625f, pipes1->getSecondTopSprite(), 1.0f)
            || collis1.checkSpriteCollision(bird1->getSprite(), 0.625f, pipes1->getSecondDownSprite(), 1.0f))
        {
            currentGameState = eGameOver;
            so_hit.play();
        }

        if (pipes1->score(bird1->getX()))
        {
            score++;
            hud1->updateScore(score);
            so_point.play();
        }
    }
   
    if (eGameOver == currentGameState)
    {
        this->data->machine.addState(StateRef(new GameOverState(data, score)), true);
    }
}

void GameState::draw(float dt)
{
    this->data->window.clear();

    back1->drawBack();

    if (ePlaying == currentGameState)
    {
        pipes1->drawPipes();
    }

    ground1->drawGround();
    bird1->drawBird();
    hud1->draw();

    this->data->window.display();
}

GameOverState.h

    #pragma once
    
    #include <fstream>
    #include "State.h"
    #include "Game.h"
    #include "Camera.h"
    #include "InputManager.h"
    
    class GameOverState : public State
    {
    private:
        GameDataRef data;
    
        InputManager* iM2;
        Camera* camera2;
    
        sf::Sprite s_back;
        sf::Sprite s_gameOverTitle;
        sf::Sprite s_retryButton;
        sf::Sprite s_board;
    
        sf::Sprite s_goldMedal;
        sf::Sprite s_silverMedal;
        sf::Sprite s_bronzeMedal;
        sf::Sprite s_ironMedal;
    
        sf::Text t_score;
        sf::Text t_bestScore;
    
        int score;
        int bestScore;
    public:
        GameOverState(GameDataRef _data, int _score);
    
        void init();
    
        void handleInput();
    
        void update(float dt);
    
        void draw(float dt);
    };

GameOverState.cpp

#include "GameOverState.h"
#include "GameState.h"

GameOverState::GameOverState(GameDataRef _data, int _score) : data(_data), score(_score)
{

}

void GameOverState::init()
{
    std::fstream read("BestScore.txt", std::ios::in);

    if (read.is_open())
    {
        while (!read.eof())
        {
            read >> bestScore;
        }
    }

    read.close();

    std::fstream write("BestScore.txt", std::ios::out);

    if (write.is_open())
    {
        if (score > bestScore)
        {
            bestScore = score;
        }

        write << bestScore;
    }

    write.close();

    camera2 = new Camera(data);
    iM2 = new InputManager(data);

    sf::Texture* back = getGlobalTextureHolder().getResource("back");

    sf::Vector2u TextureSize = back->getSize();

    float ScaleX = (float)windowWidth / (float)TextureSize.x;
    float ScaleY = (float)windowHeight / (float)TextureSize.y;

    s_back.setTexture(*back);
    s_back.setScale(ScaleX, ScaleY);

    sf::Texture* gameOverTitle = getGlobalTextureHolder().getResource("gameOver");

    s_gameOverTitle.setTexture(*gameOverTitle);

    sf::Texture* button = getGlobalTextureHolder().getResource("start_button");

    s_retryButton.setTexture(*button);

    camera2->setCamera();

    s_back.setPosition(windowWidth / 2, windowHeight / 2);
    s_gameOverTitle.setPosition(windowWidth / 1.21, windowHeight / 1.51);
    s_retryButton.setPosition(windowWidth / 1.09, windowHeight / 1.0);

    sf::Texture* board = getGlobalTextureHolder().getResource("board");

    s_board.setTexture(*board);
 
    s_board.setPosition(windowWidth / 1.26, windowHeight / 1.29);

    sf::Texture* gold_medal = getGlobalTextureHolder().getResource("gold_medal");

    s_goldMedal.setTexture(*gold_medal);

    sf::Texture* silver_medal = getGlobalTextureHolder().getResource("silver_medal");

    s_silverMedal.setTexture(*silver_medal);

    sf::Texture* bronze_medal = getGlobalTextureHolder().getResource("bronze_medal");

    s_bronzeMedal.setTexture(*bronze_medal);

    sf::Texture* iron_medal = getGlobalTextureHolder().getResource("iron_medal");

    s_ironMedal.setTexture(*iron_medal);

    sf::Font* text = getGlobalFontHolder().getResource("numbers");

    t_score.setFont(*text);
    t_bestScore.setFont(*text);

    t_score.setString(std::to_string(score));
    t_score.setCharacterSize(20);
    t_score.setFillColor(sf::Color::White);
    t_score.setOrigin(sf::Vector2f(t_score.getGlobalBounds().width / 2, t_score.getGlobalBounds().height / 2));

    t_bestScore.setString(std::to_string(bestScore));
    t_bestScore.setCharacterSize(20);
    t_bestScore.setFillColor(sf::Color::White);
    t_bestScore.setOrigin(sf::Vector2f(t_score.getGlobalBounds().width / 2, t_score.getGlobalBounds().height / 2));

    t_score.setPosition(550, 534);
    t_bestScore.setPosition(551, 576);

    if (score > 0 && score < 25)
    {
        s_ironMedal.setPosition(390, 539);
    }
    else if (score >= 25 && score < 50)
    {
        s_bronzeMedal.setPosition(390, 539);
    }
    else if (score >= 50 && score < 75)
    {
        s_silverMedal.setPosition(390, 539);
    }
    else if (score >= 75)
    {
        s_goldMedal.setPosition(390, 539);
    }
}

void GameOverState::handleInput()
{
    sf::Event event;

    while (this->data->window.pollEvent(event))
    {
        if (sf::Event::Closed == event.type)
        {
            this->data->window.close();
        }

        if (iM2->IsSpriteClicked(event, this->s_retryButton, sf::Mouse::Left))
        {
            this->data->machine.addState(StateRef(new GameState(data)), true);
        }
    }
}

void GameOverState::update(float dt)
{

}

void GameOverState::draw(float dt)
{
    this->data->window.clear();

    this->data->window.draw(this->s_back);
    this->data->window.draw(this->s_retryButton);
    this->data->window.draw(this->s_gameOverTitle);
    this->data->window.draw(this->s_board);

    if (score > 0 && score < 25)
    {
        this->data->window.draw(this->s_ironMedal);
    }
    else if (score >= 25 && score < 50)
    {
        this->data->window.draw(this->s_bronzeMedal);
    }
    else if (score >= 50 && score < 75)
    {
        this->data->window.draw(this->s_silverMedal);
    }
    else if (score >= 75)
    {
        this->data->window.draw(this->s_goldMedal);
    }
    
    this->data->window.draw(t_score);
    this->data->window.draw(t_bestScore);

    this->data->window.display();
}

Ground.h

#pragma once

#include "Game.h"

class Ground
{
private:
    GameDataRef data;

    sf::Sprite s_ground;
    sf::Sprite s_ground2;
public:
    Ground(GameDataRef _data);

    void generateGround(float x);

    void drawGround();

    const sf::Sprite& getFirstSprite();

    const sf::Sprite& getSecondSprite();
};

Ground.cpp

#include "Ground.h"

Ground::Ground(GameDataRef _data) : data(_data)
{
    sf::Texture* ground = getGlobalTextureHolder().getResource("ground");

    s_ground.setTexture(*ground);
    s_ground2.setTexture(*ground);

    s_ground.setScale(1.003f, 1.0f);
    s_ground2.setScale(1.003f, 1.0f);

    s_ground.setPosition(0.0f, windowHeight / 1.3f);
}

void Ground::generateGround(float x)
{
    if (x >= s_ground.getPosition().x + 230)
    {
        s_ground2.setPosition(s_ground.getPosition().x + (float)windowWidth, windowHeight / 1.3f);
    }
    if (x >= s_ground2.getPosition().x + 230)
    {
        s_ground.setPosition(s_ground2.getPosition().x + (float)windowWidth, windowHeight / 1.3f);
    }
}

void Ground::drawGround()
{
    data->window.draw(s_ground);
    data->window.draw(s_ground2);
}

const sf::Sprite& Ground::getFirstSprite()
{
    return s_ground;
}

const sf::Sprite& Ground::getSecondSprite()
{
    return s_ground2;
}

Hud.h

#pragma once

#include "Game.h"

#include <string>

class Hud
{
private:
    GameDataRef data;

    sf::Text scoreText;
public:
    Hud(GameDataRef _data);

    void move();

    void updateScore(int score);

    void draw();
};

Hud.cpp

#include "Hud.h"

Hud::Hud(GameDataRef _data) : data(_data)
{
    sf::Font* text = getGlobalFontHolder().getResource("numbers");
    
    scoreText.setFont(*text);

    scoreText.setString("0");

    scoreText.setCharacterSize(64);

    scoreText.setFillColor(sf::Color::White);

    scoreText.setOrigin(sf::Vector2f(scoreText.getGlobalBounds().width / 2, scoreText.getGlobalBounds().height / 2));

    scoreText.setPosition(sf::Vector2f(windowWidth / 2, windowHeight / 7));
}

void Hud::move()
{
    scoreText.move(moveSpeed, 0.0f);
}

void Hud::updateScore(int score)
{
    scoreText.setString(std::to_string(score));
}

void Hud::draw()
{
    data->window.draw(scoreText);
}

InputManager.h

#pragma once

#include "Game.h"
#include <SFML/Graphics.hpp>

class InputManager
{
private:
    GameDataRef data;
public:
    InputManager(GameDataRef _data);

    bool IsSpriteClicked(sf::Event event, sf::Sprite object, sf::Mouse::Button button);

    sf::Vector2i GetMousePosition();
};

InputManager.cpp

#include "InputManager.h"

InputManager::InputManager(GameDataRef _data) : data(_data)
{

}

bool InputManager::IsSpriteClicked(sf::Event event, sf::Sprite object, sf::Mouse::Button button)
{
    if (event.type == sf::Event::MouseButtonReleased)
    { // on click
        auto pos = data->window.mapPixelToCoords(sf::Vector2i(event.mouseButton.x, event.mouseButton.y));

        if (event.mouseButton.button == sf::Mouse::Left)
        { // on left click
            if (object.getGlobalBounds().contains(pos))
            {
                return true;
            }
        }
    }

    return false;
}

sf::Vector2i InputManager::GetMousePosition()
{
    return sf::Mouse::getPosition(data->window);
}

MainMenuState.h

#pragma once

#include "Game.h"
#include "InputManager.h"

class MainMenuState : public State
{
private:
    GameDataRef data;

    InputManager *iM1;

    sf::Sprite s_title;
    sf::Sprite s_background;
    sf::Sprite s_button;
public:
    MainMenuState(GameDataRef _data);

    void init();
    void handleInput();

    void update(float dt);
    void draw(float dt);
};

MainMenuState.cpp

#include "MainMenuState.h"
#include "GameState.h"

MainMenuState::MainMenuState(GameDataRef _data) : data(_data)
{
    
}

void MainMenuState::init()
{
    iM1 = new InputManager(data);

    sf::Texture* back = getGlobalTextureHolder().getResource("back");
  
    sf::Vector2u TextureSize = back->getSize();

    float ScaleX = (float)windowWidth / (float)TextureSize.x;
    float ScaleY = (float)windowHeight / (float)TextureSize.y;

    s_background.setTexture(*back);
    s_background.setScale(ScaleX, ScaleY);

    sf::Texture* title = getGlobalTextureHolder().getResource("title");

    s_title.setTexture(*title);

    sf::Texture* button = getGlobalTextureHolder().getResource("start_button");

    s_button.setTexture(*button);

    s_title.setPosition(windowWidth / 3.15, windowHeight / 6);
    s_button.setPosition(windowWidth / 2.46, windowHeight / 2);
}

void MainMenuState::handleInput()
{
    sf::Event event;

    while (this->data->window.pollEvent(event))
    {
        if (sf::Event::Closed == event.type)
        {
            this->data->window.close();
        }

        if (iM1->IsSpriteClicked(event, this->s_button, sf::Mouse::Left))
        {
            this->data->machine.addState(StateRef(new GameState(data)), true);
        }
    }
}

void MainMenuState::update(float dt)
{

}

void MainMenuState::draw(float dt)
{
    this->data->window.clear();

    this->data->window.draw(this->s_background);
    this->data->window.draw(this->s_title);
    this->data->window.draw(this->s_button);

    this->data->window.display();
}

Pipes.h

#pragma once

#include "Game.h"

#include <random>

class Pipes
{
private:
    GameDataRef data;

    sf::Sprite s_top1;
    sf::Sprite s_down1;

    sf::Sprite s_top2;
    sf::Sprite s_down2;

    int sizeGroundTexture;
    float originPipe = 0.0f;
    float randY = 0.0f;
public:
    Pipes(GameDataRef _data);

    void randomizeY();

    void generatePipes(float x, bool state);

    void drawPipes();

    bool score(float x);

    const sf::Sprite& getFirstTopSprite();

    const sf::Sprite& getFirstDownSprite();

    const sf::Sprite& getSecondTopSprite();

    const sf::Sprite& getSecondDownSprite();
};

Pipes.cpp

#include "Pipes.h"

Pipes::Pipes(GameDataRef _data) : data(_data)
{
    sf::Texture* top_pipe = getGlobalTextureHolder().getResource("top_pipe");

    sf::Texture* down_pipe = getGlobalTextureHolder().getResource("down_pipe");

    s_top1.setTexture(*top_pipe);
    s_top2.setTexture(*top_pipe);

    originPipe = top_pipe->getSize().x / 2.0f;

    s_down1.setTexture(*down_pipe);
    s_down2.setTexture(*down_pipe);

    sf::Texture* gTexture = getGlobalTextureHolder().getResource("ground");

    sizeGroundTexture = gTexture->getSize().y;
}

void Pipes::randomizeY()
{
    std::mt19937 gen;
    gen.seed(time(0));

    std::uniform_int_distribution<> uid(1, sizeGroundTexture);

    randY = uid(gen);
}

void Pipes::generatePipes(float x, bool state)
{
    if (state == false)
    {
        randomizeY();

        s_top1.setPosition(DISTANCE + x, 0.0f - distanceBetweemPipes - randY);
        s_down1.setPosition(DISTANCE + x,
            (float)windowHeight - s_top1.getGlobalBounds().height + distanceBetweemPipes - randY);

        randomizeY();

        s_top2.setPosition(s_top1.getPosition().x + DISTANCE, 0.0f - distanceBetweemPipes - randY);        
        s_down2.setPosition(s_down1.getPosition().x + DISTANCE,
            (float)windowHeight - s_top2.getGlobalBounds().height + distanceBetweemPipes - randY);
    }
    else if (x == s_top2.getPosition().x + 24)
    {
        randomizeY();

        s_top1.setPosition(s_top2.getPosition().x + DISTANCE, 0.0f - distanceBetweemPipes - randY);     
        s_down1.setPosition(s_down2.getPosition().x + DISTANCE,
            (float)windowHeight - s_top1.getGlobalBounds().height + distanceBetweemPipes - randY);
    }
    else if (x == s_top1.getPosition().x + 24)
    {
        randomizeY();

        s_top2.setPosition(s_top1.getPosition().x + DISTANCE, 0.0f - distanceBetweemPipes - randY);
        s_down2.setPosition(s_down1.getPosition().x + DISTANCE,
            (float)windowHeight - s_top2.getGlobalBounds().height + distanceBetweemPipes - randY);
    }
}

bool Pipes::score(float x)
{
    if (x == s_top1.getPosition().x + 24)
    {
        return true;
    }
    else if (x == s_top2.getPosition().x + 24)
    {
        return true;
    }
    else
    {
        return false;
    }
}

void Pipes::drawPipes()
{
    data->window.draw(s_top1);
    data->window.draw(s_down1);

    data->window.draw(s_top2);
    data->window.draw(s_down2);
}

const sf::Sprite& Pipes::getFirstTopSprite()
{
    return s_top1;
}

const sf::Sprite& Pipes::getFirstDownSprite()
{
    return s_down1;
}

const sf::Sprite& Pipes::getSecondTopSprite()
{
    return s_top2;
}

const sf::Sprite& Pipes::getSecondDownSprite()
{
    return s_down2;
}

ResourceHolder.h

#pragma once

#include <SFML/Graphics.hpp>

#ifndef RESOURCEHOLDER_H
#define RESOURCEHOLDER_H

template<typename ResourceType, typename KeyType>
class ResourceHolder
{
    using MapType = std::map<KeyType, ResourceType*>;
public:
    template<typename ... Args>
    ResourceType* loadFromFile(const std::string& fileName, const KeyType& key, Args &&... args)
    {
        ResourceType*& resourceRef = mResources[key];
        if (resourceRef != nullptr) {
            return resourceRef;
        }
        resourceRef = new ResourceType();
        if (!resourceRef->loadFromFile(fileName, std::forward<Args>(args)...)) {
            delete resourceRef;
            resourceRef = nullptr;
        }
        return resourceRef;
    }
    ResourceType* getResource(const KeyType& key)
    {
        typename MapType::iterator resourcePairIt = mResources.find(key);
        if (resourcePairIt == mResources.end()) {
            return nullptr;
        }
        return resourcePairIt->second;
    }

    ~ResourceHolder()
    {
        for (auto& e : mResources) {
            delete e.second;
        }
    }

    ResourceHolder() = default;
    ResourceHolder(const ResourceHolder&) = delete;
    ResourceHolder(ResourceHolder&&) = delete;
    ResourceHolder& operator=(const ResourceHolder&) = delete;
    ResourceHolder& operator=(ResourceHolder&&) = delete;
private:
    MapType mResources;
};

template<typename ResourceType, typename KeyType>
ResourceHolder<ResourceType, KeyType>& getGlobalResourceHolder()
{
    static ResourceHolder<ResourceType, KeyType> holder;
    return holder;
}

inline auto getGlobalTextureHolder = getGlobalResourceHolder<sf::Texture, std::string>;
inline auto getGlobalFontHolder = getGlobalResourceHolder<sf::Font, std::string>;

#endif

State.h

#pragma once

class State
{
public:
    virtual void init() = 0;

    virtual void handleInput() = 0;
    virtual void update(float dt) = 0;
    virtual void draw(float dt) = 0;

    virtual void pause() {}
    virtual void resume() {}
};

StateMachine.h

#pragma once

#include <stack>
#include <memory>

#include "State.h"

typedef std::unique_ptr <State> StateRef;

class StateMachine
{
private:
    std::stack <StateRef> states;
    StateRef newState;

    bool isRemoving;
    bool isAdding;
    bool isReplacing;
public:
    StateMachine() {}

    void addState(StateRef _newState, bool _isReplacing = true);

    void removeState();
   
    void processStateChanges();
    
    StateRef& getActiveState();

    ~StateMachine() {}
};

StateMachine.cpp

#include "StateMachine.h"

void StateMachine::addState(StateRef _newState, bool _isReplacing)
{
    this->isAdding = true;
    this->isReplacing = _isReplacing;

    this->newState = std::move(_newState);
}

void StateMachine::removeState()
{
    this->isRemoving = true;
}

void StateMachine::processStateChanges()
{
    if (this->isRemoving && !this->states.empty())
    {
        this->states.pop();

        if (!this->states.empty())
        {
            this->states.top()->resume();
        }

        this->isRemoving = false;
    }

    if (this->isAdding)
    {
        if (!this->states.empty())
        {
            if (this->isReplacing)
            {
                this->states.pop();
            }
            else
            {
                this->states.top()->pause();
            }
        }

        this->states.push(std::move(this->newState));
        this->states.top()->init();
        this->isAdding = false;
    }
}

StateRef& StateMachine::getActiveState()
{
    return this->states.top();
}

G. Sliepen · Accepted Answer · 2022-08-31 21:23:01Z

About your `StateMachine`

You can model some things as state machines, but writing a state machine class and having that implement your state machine logic is often more work and less clear than just writing what you want to do directly in C++. It's even worse if your state machine isn't used to encode all the possible states and their transitions up front, but when you dynamically add and remove states.

But perhaps it's just an issue of naming; maybe SceneManager would be better, as what you call a "state" is usually called a "scene" in game engines such as Godot and Unity. Then similarly, State should be renamed Scene.

Instead of addState() with a flag _isReplacing and removeState(), I would have member functions replace(), push() and pop().

Naming things

Naming things correctly is important. I would change the way these things are named:

GameDataRef: making a type alias is fine, but I would associate "Ref" with a reference, not with a std::shared_ptr<>. So GameDataPtr would be more appropriate.
game1: why the 1? That implies there is more than one. I would name it game, but in fact it doesn't need a name at all, you can just write:
```
int main() {
    Game();
}
```
loadResource(): it loads multiple resources, so I would name it loadResources().
TextureSize: this is inconsistent with the way you name other variables, so it should be textureSize.
s_bird, so_die, t_score: it's weird to see you use a prefix only for some types of variables. I would avoid using a prefix, especially if they are abbreviated so much. Either write sprite_bird, sound_die and so on, or consider grouping them inside a struct, like so:
```
struct {
     sf::Text score;
     sf::Text bestScore;
} text;
```

Unnecessary use of `this->`

You almost never have to write this-> in C++. I would remove it everywhere.

Updating state

In Game::run(), you have the main loop which first calls processStateChanges(), but then goes into another loop that might call handleInput() and update() multiple times. This might be problematic, as this means there can be multiple changes submitted to the state machine before they are processed. Conversely, if accumulator < dt you call processStateChanges() unnecessarily, as nothing will have changed since the last call.

I would make sure processStateChanges() is called inside the inner while-loop. In fact, you'll have three function calls there that I would replace by just one, which then takes care of processing state changes, handling input and updating the state:

while (data->window.isOpen())
{
    ...
    while (accumulator >= dt)
    {
        data->machine.update(dt);
    }
    ...
    data->machine.draw(interpolation);
}

Note that the above doesn't call getActiveState() anymore, but instead assumes update() and draw() are member functions of StateMachine. The latter should then handle getting the currently active state and calling the relevant member functions of the currently active State:

void StateMachine::update(float dt)
{
    processStateChanges();
    auto state = getActiveState();
    state->handleInput();
    state->update(dt);
}

Use of smart pointers

It's good to use smart pointers to avoid manual memory management. However, some types of smart pointers have some overhead, particularly std::shared_ptr<>. If you don't need the reference counted semantics, use std::unique_ptr<> and/or regular pointers or references instead. Sometimes you don't even need (smart) pointers, and should just store by value instead.

Consider Game::data. Instead of making that a std::shared_ptr<>, you can store it by value, and have classes that need to access that data store a reference to it instead, as the instance of Game is guaranteed to be alive while any of the other game objects are. So:

class Game
{
    ...
    GameData data;
    ...
};

class Bird
{
    GameData& data;
    ...
public:
    Bird(GameData& _data);
    ...
};

Bird::Bird(GameData& _data): data(_data)
{
    ...
}

Another case where smart pointers are unnecessary is in ResourceHolder. A std::map already allocates memory for the keys and values, so you don't have to. Just store resources in them directly, for example:

template<typename ResourceType, typename KeyType>
class ResourceHolder
{
    using MapType = std::map<KeyType, ResourceType>;
public:
    template<typename ... Args>
    ResourceType& loadFromFile(const std::string& fileName, const KeyType& key, Args&&... args)
    {
        if (auto it = mResources.find(key); it != mResources.end()) {
            return it->second;
        }

        auto& resource = mResources[key];
        if (!resource.loadFromFile(fileName, std::forward<Args>(args)...)) {
            throw std::runtime_error("Could not load resource");
        }

        return resource;
    }

    ResourceType& getResource(const KeyType& key) {
        return mResources.at(key);
    }
    ...
};

You also should either use std::unique_ptr<> for iM2 and camera2, or store them by value instead. At the moment those two variables will leak memory, since they are never deleted.

When you do use smart pointers, avoid manual calls to new. You can use std::make_unique<> instead. Note that a std::unique_ptr<Derived> can be assigned to a std::unique_ptr<Base>, so this is valid:

data->machine.addState(std::make_unique<MainMenuState>(data));

Make functions easy to use

Don't be afraid to modify or add more functions to take care of boring tasks. For example, addState() requires you to create a unique pointer first. Why not let addState() do that for you? It could look like this:

class StateMachine
{
    ...
    template<typename T, typename... Args>
    void addState(Args&&... args)
    {
        newState = std::make_unique<T>(std::forward<Args>(args)...);
    }
    ...
};

And then it could be called like so:

data->machine.addState<GameOverState>(data, score);

Of course, with a variable number of arguments you really want to have two functions, say replace() and push(), to distinguish between adding and replacing a state.

You are already using using to avoid typing long names, but you can also make aliases for variables. Consider:

void GameOverState::draw(float dt)
{
    auto& window = data->window;

    window.clear();
    window.draw(s_back);
    window.draw(s_retryButton);
    ...
    window.display();
}

Error handling

Make sure you handle errors correctly. If something goes wrong, print a clear error message to std::cerr (not std::cout), then make sure you exit the program with a non-zero exit code, preferrably EXIT_FAILURE. Alternatively, throw an exception; unhandled exceptions will cause a program to abort with an error message. Failure to do so will cause the program to continue running with incorrect data.

In the constructor of Game(), where you cannot return an error code, I would write:

if (!loadResources())
{
    throw std::runtime_error("Could not load all resources");
}

However, if you are going to use exceptions, it's better to throw directly at the place where things actually go wrong. For resource loading, that's in ResourceHolder::loadFromFile().

When reading or writing a file, errors can happen at any point, not just when opening the file. Also note that .eof() is only true when the end of a file has been reached, but not when an error occured halfway. Your read loop in GameOverState::init() may thus loop indefinitely it it cannot read an int from the file.

Random number generation

I see you call srand(time(NULL)), and I was afraid you were going to use the rather bad C random number generator. However, you are using std::mt19937, which is great, but then you don't need to call srand() anymore.

However, you still seed the std::mt19937 generator using time(0). I recommend that you avoid any C functions, and use the C++ way where possible:

std::random_device rd;
std::mt19937 gen(rd());

Also note that creating a generator is costly, so you should only do it once. You can do so by making gen a static variable, or a member variable of Pipes.

Stack Exchange Network

A Flappy bird game using C++ and SFML

1 Answer 1

About your `StateMachine`

Naming things

Unnecessary use of `this->`

Updating state

Use of smart pointers

Make functions easy to use

Error handling

Random number generation

Your Answer

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

Hot Network Questions

A Flappy bird game using C++ and SFML

1 Answer 1

About your StateMachine

Naming things

Unnecessary use of this->

Updating state

Use of smart pointers

Make functions easy to use

Error handling

Random number generation

Your Answer

Sign up or log in

Post as a guest

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

Related

Hot Network Questions

About your `StateMachine`

Unnecessary use of `this->`

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