c++classooprefactoringsfml

How to make my own classes with C++ and SFML


In SFML I have created a simple sprite movement program but, I would like to move this information into a class (lets say it will be called "Player"). I have messed around a lot but I can not get it to work.

I have tried creating a function in a class that would check for player input, but I can not access my sprite that I created in main. I would like to move everything related to the player into a Player class but need some advice.

What is the correct way to do this?

main.cpp

#include <SFML/Graphics.hpp>
#include <string>
#include <iostream>

int main()
{
    //character position
    enum Direction{ Down, Left, Right, Up };
    sf::Vector2i source(1, Down);

    //window
    sf::RenderWindow window(sf::VideoMode(1200, 700), "Testing");
    window.setKeyRepeatEnabled(false);

    //player character
    sf::Texture pTexture;
    sf::Sprite pSprite;
    if(!pTexture.loadFromFile("image/playerSprite.png"))
        std::cout << "Texture Error" << std::endl;
    pSprite.setTexture(pTexture);
    pSprite.setScale(1.5f, 1.5f);

    //game loop
    while (window.isOpen())
    {
        sf::Event event;
        while (window.pollEvent(event))
        {
            if (event.type == sf::Event::Closed)
                window.close();
        }

        window.clear();

        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Up)) //move up
        {

            source.y = Up;
            pSprite.move(0, -0.2);

            //animation
            source.x++;
            if(source.x * 32 >= pTexture.getSize().x)
            {
                source.x = 0;
            }
        }
        else if(sf::Keyboard::isKeyPressed(sf::Keyboard::Down)) //move down
        {
            source.y = Down;
            pSprite.move(0, 0.2);

            //animation
            source.x++;
            if(source.x * 32 >= pTexture.getSize().x)
            {
                source.x = 0;
            }
        }
        else if(sf::Keyboard::isKeyPressed(sf::Keyboard::Right)) //move right
        {
            source.y = Right;
            pSprite.move(0.2, 0);

            //animation
            source.x++;
            if(source.x * 32 >= pTexture.getSize().x)
            {
                source.x = 0;
            }
        }
        else if(sf::Keyboard::isKeyPressed(sf::Keyboard::Left)) //move left
        {
            source.y = Left;
            pSprite.move(-0.2, 0);

            //animation
            source.x++;
            if(source.x * 32 >= pTexture.getSize().x)
            {
                source.x = 0;
            }
        }

        pSprite.setTextureRect(sf::IntRect(source.x * 32, source.y * 32, 32, 32));
        window.draw(pSprite);
        window.display();

    }

    return 0;
}

Solution

  • Disclaimer: This has nothing to do with SFML, this is just basic refactoring.

    How to think with OOP

    First thing first, before coding a feature, it's best to design the OOP structure that really suits the situation. Each class is part of a whole, that is the program. A class, in fact, is just an aggregation of data with useful methods that only affects the data inside the class (or the data provided via method parameters) in a meaningful way.

    See the basics of C++ (specifically the OOP section) to understand how to get it to work in C++. The concepts are similar in other programming languages.

    Working with your provided code

    What you asked for was a Player class and it's a great idea to get the player code out of the program main logic. You need to ask yourself: "What my player code needs to work?"

    The player class

    Basically, a player is only a sprite and a position. So let's start by encapsulating this data into the Player class as private members. That keeps other code from messing with the player data. To use the player data, we provide methods on the class that each affects only the Player instance.

    Texture and Sprite

    I have kept the Texture outside of the player on purpose. Textures are heavy objects, that's why the Sprite object only keeps a pointer to it. Sprites are lightweight and can be changed and copied easily. The managing of texture objects and other assets is another subject, though here's my own resource manager code.

    Optional

    I did not take the time to change your code much, but movement handling could be changed to only use one "move" method that takes a Player::Direction as a parameter.

    To help you a little more and to give you some more guidelines on the subject, I used "forward declaration" and moved your Direction enum inside the class. It's maybe not the best way to achieve what you want, but I've only changed your own code to avoid any confusion.

    The Code

    Anyway, here's my go at this.

    Player.h

    #ifndef PLAYER_H_
    #define PLAYER_H_
    
    #include <SFML/Graphics/Drawable.hpp>
    #include <SFML/Graphics/Sprite.hpp>
    
    // Forward Declaration
    namespace sf {
    class Texture;
    }
    
    // provide your namespace to avoid collision/ambiguities
    namespace test {
    /*
     *
     */
    class Player: public sf::Drawable {
    public:
    
        enum Direction {
            Down, Left, Right, Up
        };
    
        Player(const sf::Texture& playerTexture);
        virtual ~Player();
    
        virtual void draw(sf::RenderTarget& target, sf::RenderStates states) const;
    
        void moveUp();
        void moveDown();
        void moveLeft();
        void moveRight();
    
    private:
    
        sf::Sprite mSprite;
        sf::Vector2i mSource;
    
    };
    
    } /* end namespace test */
    #endif /* PLAYER_H_ */
    

    Player.cpp

    #include "Player.h"
    
    // you need this because of forward declaration
    #include <SFML/Graphics/Texture.hpp>
    #include <SFML/Graphics/Rect.hpp>
    #include <SFML/Graphics/RenderTarget.hpp>
    
    namespace test {
    Player::Player(const sf::Texture& imagePath) :
                    mSprite(imagePath),
                    mSource(1, Player::Down) {
    
        // do not need that line anymore, thanks to initialiser list
        //pSprite.setTexture(pTexture);
    
        mSprite.setScale(1.5f, 1.5f);
    
    }
    
    Player::~Player() {
        // TODO Auto-generated destructor stub
    }
    
    void Player::draw(sf::RenderTarget& target, sf::RenderStates states) const {
        target.draw(mSprite, states);
    }
    
    void Player::moveUp() {
        mSource.y = Up;
        mSprite.move(0, -0.2);
    
        //animation
        mSource.x++;
        if (mSource.x * 32 >= (int) mSprite.getTexture()->getSize().x) {
            mSource.x = 0;
        }
    
        mSprite.setTextureRect(sf::IntRect(mSource.x * 32, mSource.y * 32, 32, 32));
    }
    
    void Player::moveDown() {
        mSource.y = Down;
        mSprite.move(0, 0.2);
    
        //animation
        mSource.x++;
        if (mSource.x * 32 >= (int) mSprite.getTexture()->getSize().x) {
            mSource.x = 0;
        }
    }
    
    void Player::moveLeft() {
        mSource.y = Left;
        mSprite.move(-0.2, 0);
    
        //animation
        mSource.x++;
        if (mSource.x * 32 >= (int) mSprite.getTexture()->getSize().x) {
            mSource.x = 0;
        }
    }
    
    void Player::moveRight() {
        mSource.y = Right;
        mSprite.move(0.2, 0);
    
        //animation
        mSource.x++;
        if (mSource.x * 32 >= (int) mSprite.getTexture()->getSize().x) {
            mSource.x = 0;
        }
    }
    
    } /* end namespace test */
    

    main.cpp

    #include <SFML/Graphics.hpp>
    //#include <string> // not used for now
    #include <iostream>
    
    // don't forget to include your own header
    #include "Player.h"
    
    int main() {
    
        // just to save typing the "std::"
        using std::cout;
        using std::endl;
        using std::cerr;
    
        //window
        sf::RenderWindow window(sf::VideoMode(1200, 700), "Testing");
        window.setKeyRepeatEnabled(false);
    
        //player texture
        sf::Texture pTexture;
        if (!pTexture.loadFromFile("image/playerSprite.png")) {
            cerr << "Texture Error" << endl;
        }
    
        test::Player thePlayer(pTexture);
    
        //game loop
        while (window.isOpen()) {
            sf::Event event;
            while (window.pollEvent(event)) {
                if (event.type == sf::Event::Closed) {
                    window.close();
                }
            }
    
            window.clear();
    
            if (sf::Keyboard::isKeyPressed(sf::Keyboard::Up)) //move up
                    {
                thePlayer.moveUp();
            } else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Down)) //move down
                    {
                thePlayer.moveDown();
            } else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Right)) //move right
                    {
                thePlayer.moveRight();
            } else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Left)) //move left
                    {
                thePlayer.moveLeft();
            }
    
            window.draw(thePlayer);
            window.display();
    
        }
    
        return 0;
    }
    

    Other good practices

    Accessors, or Getters/Setters, are member functions that gives one the access to a class private member.

    In your code, you could do something like that:

     class Player {
        public: 
            Player(const sf::Texture& playerTexture);
            virtual ~Player();
    
           // to give access to a const reference of the sprite
           // One could call it like: sf::Sprite mySprite = myPlayerObject.getSprite();
           // notice also that the method itself is const, which assure you that
           // myPlayerObject won't change by calling getSprite()
           const sf::Sprite& getSprite() const{
               return mSprite;
           }
    
           // setSprite is not a const method, so it will change the data
           // inside myPlayerObject
           void setSprite(const sf::Sprite& newSprite){
               mSprite = newSprite;
           }
        
        private:
            sf::Sprite mSprite;
            sf::Vector2i mSource;
        };