Snake game using SFML CPP

Question

I made using SFML my first game project. It's a snake game, and I used 2 custom classes:

• snake
• fruit (snake grow by 1 when eat it)

Here's main.cpp code:

#include <SFML/Graphics.hpp>
#include "snake.h"
#include "fruit.h"
int main()
{

    sf::RenderWindow app(sf::VideoMode(800, 600), "SFML window");

    snake snake;
    fruit fruit;

    snake.fruitptr = &fruit;
    snake.window = &app;

    while (app.isOpen())
    {
        // Process events
        sf::Event event;
        while (app.pollEvent(event))
        {
            // Close window : exit
            if (event.type == sf::Event::Closed)
                app.close();
        }

        snake.movement();
        snake.checkCollisions();

        app.clear();
        app.draw(fruit);
        app.draw(snake);
        app.display();
        }

    return EXIT_SUCCESS;
}

Fruit and Snake header

#ifndef SNAKE_H
#define SNAKE_H
#include <SFML/Graphics.hpp>
#include <iostream>
#include "fruit.h"

class snake : public sf::Drawable
{
    public:
        snake();
        virtual ~snake();
        void movement();
        void addElement();
        void checkCollisions();
        fruit* fruitptr;
        virtual void draw(sf::RenderTarget& target,
        sf::RenderStates states) const;
        sf::RectangleShape oldPosition;
        sf::RenderWindow* window;

    private:
        float a, b;
        float x, y;
        int length;
        sf::Time time;
        sf::RectangleShape shape;

        sf::RectangleShape* snakeBody;
        sf::RectangleShape* snakeBodycopy;

};

#endif // SNAKE_H

#ifndef FRUIT_H
#define FRUIT_H
#include <SFML/Graphics.hpp>
#include <time.h>

class fruit : public sf::Drawable
{
    public:
        fruit();
        virtual ~fruit();
        sf::RectangleShape shape;
        virtual void draw(sf::RenderTarget& target,
        sf::RenderStates states) const;

    private:
};

#endif // FRUIT_H

And finally the .cpp files:

#include "snake.h"

snake::snake()
{
     x = 20.f;
     y = 20.f;
     length = 0;

     time = sf::seconds(0.1f);
    shape.setFillColor(sf::Color::Red);
    shape.setSize( {20.f,20.f});
    shape.setPosition( {20.f,20.f});
}

snake::~snake()
{
    //dtor
}

void snake::draw(sf::RenderTarget& target,
             sf::RenderStates states) const
{
    target.draw(this->shape, states);

    if(this->length == 1){
        this->snakeBody[0].setPosition(this->oldPosition.getPosition());
        target.draw(this->snakeBody[0] ,states);
    }
    else if(this->length>1){

        for(int i=this->length-1;i>0;i--){
                 this->snakeBody[i].setPosition(this->snakeBody[i-1].getPosition());
            this->snakeBody[i].setFillColor(sf::Color::Red);
            this->snakeBody[i].setSize({20.f ,20.f});
            target.draw(this->snakeBody[i], states);
        }
        this->snakeBody[0].setFillColor(sf::Color::Red);
        this->snakeBody[0].setSize({20.f , 20.f});
        this->snakeBody[0].setPosition(this->oldPosition.getPosition());
        target.draw(this->snakeBody[0], states);
    }
}

void snake::movement()
{
     this->oldPosition.setPosition(this->shape.getPosition());

    if(sf::Keyboard::isKeyPressed(sf::Keyboard::W))
    {
        this->a=0;
        this->b=-20.f;
    }
    else if(sf::Keyboard::isKeyPressed(sf::Keyboard::A))
    {
        this->a=-20.f;
        this->b=0;
    }
    else if(sf::Keyboard::isKeyPressed(sf::Keyboard::S))
    {
        this->a=0;
        this->b=20.f;
    }
    else if(sf::Keyboard::isKeyPressed(sf::Keyboard::D))
    {
        this->a=20.f;
        this->b=0;
    }

    this->shape.move(a,b);

    sf::sleep(this->time);
}

void snake::checkCollisions()
{

    for(int i=0; i<this->length; i++)
    {
        if(this->shape.getGlobalBounds().intersects(this->snakeBody[i].getGlobalBounds())){
        this->window->close();
        std::cout<<"You Lose!"<<std::endl;
        }
    }

    if(this->shape.getGlobalBounds().intersects(this->fruitptr->shape.getGlobalBounds()))
    {
        this->fruitptr->shape.setPosition( (rand()%40)*20, (rand()%30)*20);
        this->addElement();
        std::cout<<"+1"<<std::endl;
    }
}

void snake::addElement()
{
    if(this->length == 0){
        this->length++;
        snakeBody = new sf::RectangleShape [this->length];
        snakeBody[0].setFillColor(sf::Color::Red);
        snakeBody[0].setSize({20.f, 20.f});
    }
    else{
        this->snakeBodycopy = new sf::RectangleShape [this->length];

        for(int i=0;i<this->length;i++){


        this->snakeBodycopy[i].setPosition(this->snakeBody[i].getPosition());
        }

        this->length++;
        delete [] snakeBody;

        this->snakeBody = new sf::RectangleShape [this->length];

        for(int i=0;i<this->length-1;i++){

    this->snakeBody[i].setPosition(this->snakeBodycopy[i].getPosition());
        }

        delete [] snakeBodycopy;
    }
}

#include "fruit.h"

fruit::fruit()
{
    shape.setFillColor(sf::Color::White);
    shape.setSize( {20.f,20.f});
    shape.setPosition( (rand()%40)*20, (rand()%30)*20);
}

fruit::~fruit()
{
    //dtor
}

void fruit::draw(sf::RenderTarget& target,
    sf::RenderStates states) const{
    target.draw(this->shape, states);
}

Answer 1

Here are a list of things that I notice here.

Indentation and Spacing

Throughout your code, your indentation and spacing is inconsistent. I would recommend picking a specific tab width and setting this either to 1 tab or the respective amount of spaces. In addition, placing spaces around all operators and before braces will create clarity.

Useless Destructor

snake::~snake()
{
    //dtor
}

If you don't plan on using your destructor, then adding an implementation is unnecessary.

Usage of this

Looking through your classes, I notice that you prefix everything with this->, even though this is unnecessary in C++. If you really need to separate local variables from members, then I would recommend a good naming convention (I use a trailing underscore).

Random Numbers

shape.setPosition( (rand()%40)*20, (rand()%30)*20);

In C++, using the <random> header is recommended over plain old rand() (which you didn't seed with srand(), AFAICT so the fruit location will always be the same every run of the game).

Use of std::endl

Unless your goal is to specifically flush the console, then you shouldn't be using std::endl. Prefer plain old \n instead.

A Proper Fixed Timestep

sf::sleep(this->time);

In most games, what you want to aim for is a proper fixed timestep, which helps create deterministic runs with use of delta times in between frames. Good beginner SFML tutorials/books usually cover this and it is a must for a good game loop.

Answer 2

One thing that jumps off the page immediately is that you are dealing with raw pointer management (via new[] and delete[]). In 2018, it should be very rare that a casual programmer need to do this. There are a lot of good reasons not to.

Some of the most compelling reasons involve moving/assigning/copying objects...and you're not doing that yet, since you just have one snake and one fruit. But even still, you have a memory leak, just because your addElement() routine does allocations of the snakeBody with new [] that it never frees in the destructor.

(Note: You'd do well to go ahead and start running something like Valgrind or Address Sanitizer, or Dr. Memory on Windows, so you get a report of memory leaks and errors. But you should have a lot fewer of them if you learn and follow modern practices.)

learn and use std::vector

Your entire addElement() routine is essentially trying to implement the std::vector container's push_back(). If you're trying to learn low-level memory management on purpose, I'd encourage you to do it by breaking off that task into writing a my_vector class.

But if you're doing it because you don't know what a std::vector is, then I'd certainly urge you to look through a tutorial. Here's one and another I can find offhand on the present-day web.

So #include <vector> in your project, and start by changing:

sf::RectangleShape* snakeBody;
sf::RectangleShape* snakeBodycopy;

into:

std::vector<sf::RectangleShape> snakeBody;

Tutorials and reference guides should hopefully give you enough examples to adapt your program to that change and keep it working. If not, you can ask for help on StackOverflow. It may not seem easy at first, but it really is easier and less error prone than what you're attempting here.

---

start remembering the catchphrase: "Rule of Zero"

You don't have to fully absorb what "rule of zero" means yet...but maybe read around on it and start getting the inkling that it's important. It applies to why you don't want raw pointers in your classes.