A Trivia game with SFML

Question

So I've been making a trivia game with SFML and C++ for quite a while now for my school project and I would like to get some constructive criticism, anything helps from best practices to overall correctness.

Everything on GitHub for readability

Here goes main.cpp:

#include "Trivia.hpp"

int main()
{
    Trivia trig;
    trig.Run();
}

Trivia.cpp:

#include "Trivia.hpp"
#include <chrono>
#include <iostream>
#include <random>

static std::mt19937_64 rngEngine(std::chrono::high_resolution_clock::now().time_since_epoch().count());

Trivia::Trivia()
:triviaWindow(sf::VideoMode(1280, 720), "Trivia! :o")
{
    if(!font.loadFromFile("Assets/DejaVuSansCondensed-Bold.ttf")) //If font won't load, output error into console
    {
        std::cerr << "Could not load font.\n";
        triviaWindow.close();
        return;
    }
    triviaWindow.setActive(true);
    triviaWindow.setFramerateLimit(60);

    textScore.setFont(font);  //Outputs score
    textScore.setCharacterSize(30);
    textScore.setColor(sf::Color::Yellow);
    textScore.setPosition(550,50);

    questionText.setFont(font);  //Outputs question text
    questionText.setCharacterSize(60);
    questionText.setColor(sf::Color::Red);
    questionText.setPosition(350, 100);

    answerText.setFont(font);  //Outputs All 3 answers
    answerText.setCharacterSize(40);
    answerText.setColor(sf::Color::Green);
    answerText.setPosition(550, 250);

    gameOverText.setFont(font);
    gameOverText.setCharacterSize(80);
    gameOverText.setColor(sf::Color::Blue);
    gameOverText.setPosition(250,50);
    gameOverText.setString("\t\tGame Over\n\n\nPlay Again  [Y / N]?");


    for(int i=0;i<3;++i)
    {
        if(!heartTextures[i].loadFromFile("Assets/HeartState"+std::to_string(i+1)+".png"))
        {
            std::cerr << "Couldn't load HeartTexture" + std::to_string(i+1);
            triviaWindow.close();
            return;
        }
    }
}

void Trivia::AnswerDetector(int key)
{
    if(afterRngAnswers[key-1] == correctAnswer)
    {
        afterRngAnswers.clear();
        PointsCollector(50);
    }
    else if(afterRngAnswers[key-1] != correctAnswer)
    {
        afterRngAnswers.clear();
        PointsCollector(-50);
        badGuessCount++;
    }
}

void Trivia::TextStringHandler()
{
    QuestionPosRng(correctAnswer, inCorrectAnswer1, inCorrectAnswer2);

    questionText.setString(question + " ?" + "\n");
    answerText.setString("1.   " + afterRngAnswers[0] + "\n\n" + "2.   " + afterRngAnswers[1] + "\n\n" + "3.   " + afterRngAnswers[2]);
    textScore.setString("Score: " + std::to_string(score));
}

void Trivia::Lives(int state)
{
    if(state==0)
    heart.setTexture(heartTextures[state]);
    else if(state==1)
    heart.setTexture(heartTextures[state]);
    else if(state==2)
    heart.setTexture(heartTextures[state]);

    heart.setScale(2,2);

    heart.setPosition(50,100);
}

void Trivia::ProcessEvents() //Processes key inputs and other events
{
    while(triviaWindow.pollEvent(event))
    {
        if(event.type == sf::Event::Closed)
            {
                triviaWindow.close();
            }

        switch(event.type)
        {
            case sf::Event::KeyPressed:
            switch(event.key.code)
            {
                case sf::Keyboard::Escape:
                    triviaWindow.close();
                    break;

                case sf::Keyboard::Num1:
                    AnswerDetector(1);
                    break;

                case sf::Keyboard::Num2:
                    AnswerDetector(2);
                    break;

                case sf::Keyboard::Num3:
                {
                    AnswerDetector(3);
                    break;
                }

                case sf::Keyboard::F1:
                {
                    sf::Image screenshot = triviaWindow.capture();

                    std::chrono::system_clock::time_point p = std::chrono::system_clock::now();
                    std::time_t t = std::chrono::system_clock::to_time_t(p);

                    std::string screenshotString = "screenshot" + std::to_string(t) + ".png";

                    if(screenshot.saveToFile("Screenshots/" + screenshotString + ".png"))

                    std::cout << "Screenshot" + std::to_string(t) + "taken\n";
                    else
                    {
                        std::cerr << "Failed to save screenshot.";
                    }
                    break;
                }
                default:
                break;
            }
            default:
            break;
        }
    }
}

void Trivia::Run() //Calls rest of the game functions
{
    QuestionHandler();
    TextStringHandler();
    while(triviaWindow.isOpen())
    {
        if(badGuessCount<=2)
        {
            Draw();
            ProcessEvents();
            Lives(badGuessCount);
        }
        else if(badGuessCount>2)
        {
            triviaWindow.clear(sf::Color(25,25,25));
            triviaWindow.draw(gameOverText);
            triviaWindow.draw(gameOverText);
            triviaWindow.display();

            ProcessEvents();
            if(event.key.code == sf::Keyboard::Y)
            {
                GameOver(true);
                score=0;
            }
            else if(event.key.code == sf::Keyboard::N)
            {
                GameOver(false);
            }
        }
    }
}

std::string Trivia::PointsCollector(int points)
{
    //Score is a string
    score += points;
    QuestionHandler();
    TextStringHandler();

    return std::to_string(score);
}

int Trivia::RngHandler(int from, int to)
{
    std::uniform_int_distribution<> randumb(from, to); //Uniformly distributes anything you put into the function, has from and to value
    return randumb(rngEngine);
}

void Trivia::Draw() //Draws Assets
{
    triviaWindow.clear(sf::Color(25,25,25));
    triviaWindow.draw(textScore);
    triviaWindow.draw(questionText);
    triviaWindow.draw(answerText);
    triviaWindow.draw(heart);
    triviaWindow.display();
}

void Trivia::GameOver(bool pick)
{
    if(pick)
    {
        badGuessCount=0;
        textScore.setString("Score: " + std::to_string(0));
    }
    else
    {
        triviaWindow.close();
        return;
    }
}

void Trivia::QuestionPosRng(std::string answer, std::string wrongAnswer1, std::string wrongAnswer2)
{
    afterRngAnswers.push_back(answer);
    afterRngAnswers.push_back(wrongAnswer1);
    afterRngAnswers.push_back(wrongAnswer2);

    std::shuffle(std::begin(afterRngAnswers), std::end(afterRngAnswers), rngEngine);
}

void Trivia::QuestionHandler()
{
    std::ifstream database("Assets/data.ini");

    std::string words;

    if(database.is_open())
    {
        std::vector<std::string> storage;

        while(getline(database, words))
        {
            if(words.find("-") == 0) //Grabs all lines from data.ini starting with "-" and puts them into vector
            {
                storage.push_back(words);
            }
        }
        double numbOfQuestions = (storage.size()/4)-1; //Number of questions -1, so if 6 questions, this equals to 5
        double randomQuestionNumber = (RngHandler(0, numbOfQuestions))*4; //randomizes the question
        if(storage[randomQuestionNumber].size() > 4)
        {
            question = storage[randomQuestionNumber].erase(0,4);
        }
        if(storage[randomQuestionNumber+1].size() > 4)
        {
            correctAnswer = storage[randomQuestionNumber+1].erase(0,4);
        }
        if(storage[randomQuestionNumber+2].size() > 4)
        {
            inCorrectAnswer1 = storage[randomQuestionNumber+2].erase(0,4);
        }
        if(storage[randomQuestionNumber+3].size() > 4)
        {
            inCorrectAnswer2 = storage[randomQuestionNumber+3].erase(0,4);
        }
    }
    else 
    {
        std::cerr << "failed to open data file.";
        triviaWindow.close();
        return;
    }
    database.close();
}

Trivia.hpp:

#ifndef TRIVIA_H_INCLUDED
#define TRIVIA_H_INCLUDED

#include <SFML/Graphics.hpp>

#include <fstream>
#include <vector>
#include <string>



class Trivia
{
public:
    Trivia();
    void Run();

private:

    sf::Text textScore;
    sf::Text questionText;
    sf::Text answerText;
    sf::Text gameOverText;

    sf::Sprite heart;

    sf::Font font;

    sf::Event event;

    std::string question;
    std::string correctAnswer;
    std::string inCorrectAnswer1;
    std::string inCorrectAnswer2;

    sf::Texture heartTextures[3];

    int score=0;
    int badGuessCount=0;

    std::vector<std::string> afterRngAnswers;

    sf::RenderWindow triviaWindow;

    void QuestionHandler();
    void Draw();
    void QuestionPosRng(std::string answer, std::string wrongAnswer1, std::string wrongAnswer2);
    void TextStringHandler();
    void ProcessEvents();
    void Lives(int state);
    void GameOver(bool pick);

    std::string PointsCollector(int points);

    void AnswerDetector(int key);
    int RngHandler(int from, int to);
};

#endif // TRIVIA_H_INCLUDED

Answer 1

This looks pretty good! I think it's a great start and the code is pretty clean and easy-to-read. Here are a few suggestions:

Separation of Concerns

I think a single class that does everything is a little confusing. I see 2 different things going on in the code:

1. Maintaining and updating the game state
2. Displaying the current state and processing events

So I recommend breaking this up into 2 classes: a window class and a game class. This is known as Separation of Concerns. It allows you to change or replace parts of the code without affecting the other parts of the code. And it keeps things separated by functionality. (If this were more complex I might recommend a third class that sends messages between the other 2, but that's overcomplicated for this case, I think.)

If you had a TriviaWindow class and a Trivia class, your main() function would look something like this:

#include "Trivia.hpp"

int main()
{
    TriviaWindow window;
    Trivia trig;
    trig.setWindow(&window);
    trig.Run();
}

I'd put all of the window, text, sprite, etc. members into the window class. And all of the game state into the Trivia class. That way, if you decide you want to display your game differently, you don't have to touch the logic at all. You just write a new TriviaWindow class (or subclass), and use that instead. For example, if you wanted to change the hearts to stars, or you wanted send the text over the network and get responses back from the network, or whatever.

However, doing that will change other things a little bit. As you can see above, the Trivia class now needs a pointer to the TriviaWindow class. Another change is that event processing is now in the window class, so it needs to return information about whether and how it processed events.

I'd create a enum like this:

typedef enum EventResponse {
    EVT_HANDLED = 0, // Some non-game event like window resized, etc.
    EVT_PRESSED_1 = 1,
    EVT_PRESSED_2 = 2,
    EVT_PRESSED_3 = 3,
    EVT_PRESSED_Y = 4,
    EVT_PRESSED_N = 5
} EventResponse;

So Trivia::Run() would do something like this:

void Trivia::Run() //Calls rest of the game functions
{
    QuestionHandler();
    TextStringHandler();
    while(window.isOpen())
    {
        if(badGuessCount<=2)
        {
            Draw();
            EventResponse evtResult = ProcessEvents();
            switch (evtResult)
            {
                case EVT_PRESSED_1:
                    AnswerDetector(1);
                break;

                case EVT_PRESSED_2:
                    AnswerDetector(2);
                break;

                case EVT_PRESSED_3:
                    AnswerDetector(3);
                break;
            }
            Lives(badGuessCount);
        }
        else
        {
            window.SetGameOver();

            EventResponse evtRslt = ProcessEvents();
            if(evtRslt == EVT_PRESSED_Y)
            {
                GameOver(true);
                score=0;
            }
            else if(evtRslt == EVT_PRESSED_N)
            {
                GameOver(false);
            }
        }
    }
}

So you can see that you'll need a couple of additional methods in the TriviaWindow class. You'll also need some public setters to set the text fields. Probably something like:

void setQuestionText(const std::string& newQuestionText);
void setAnswerText(const std::string& newAnswerText);
void setScore(const std::string& newScore);
void setLives(const int numLives);
void setGameOver();

This way, each class does it’s one thing. The window class handles the user - displaying the text and hearts, and getting input from the user, and the Trivia class just handles keeping track of questions and scores.

Handling Construction Failures

I notice in your constructor for Trivia that if anything goes wrong, you simply close the window and return. This is dangerous because what happens is you end up with a half-constructed object. The caller doesn't know that it didn't complete construction, and thinks it's a valid object but it's not. You have 2 choices:

1. Use exceptions to tell the caller that something went wrong
2. Construct those objects outside of the constructor and pass them in

Exceptions are a big subject and different programmers have very strong feelings about how and when to use them. It’s easier to construct the objects that might fail and pass them in if it goes OK, so let’s do that.

So main() becomes:

#include "Trivia.hpp"

int main()
{
    sf::Font font;
    if(!font.loadFromFile("Assets/DejaVuSansCondensed-Bold.ttf")) //If font won't load, output error into console
    {
        std::cerr << "Could not load font.\n";
        return 0; // Indicates failure
    }
    sf::Texture hearts[3];
    for(int i=0;i<3;++i)
    {
        if(!heartTextures[i].loadFromFile("Assets/HeartState"+std::to_string(i+1)+".png"))
        {
            std::cerr << "Couldn't load HeartTexture" + std::to_string(i+1);
            return 0;
        }
    }

    TriviaWindow window(font, hearts);
    Trivia trig;
    trig.setWindow(&window);
    trig.Run();
    return 1; // Success!
}

You’ll then need to add a new constructor to the TriviaWindow class:

TriviaWindow::TriviaWindow(const sf::Font& inFont, const sf::Texture inHearts[3])
:triviaWindow(sf::VideoMode(1280, 720), "Trivia! :o”)
,font(inFont)
{
    for (int i = 0; i < 3; i++)
    {
        hearts[i] = inHearts[i];
    }
    //… rest of the constructor here
}

Now if you do that, you should also make the default constructor (the one with no arguments) private so that it can’t be called. That way you never end up with a TriviaWindow that doesn’t already have fonts and hearts in it:

class TriviaWindow {
public:
    TriviaWindow(const sf::Font& inFont, const sf::Texture inHearts[3]);
    // … rest of public members

private:
    TriviaWindow();
    // … rest of private members
};