1

I have currently started working with SFML after learning the basics of C++. I have learnt about Arrays, References and everything that comes before it but have struggled to grasp the concept of using classes.

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? (Please don't say go back and learn about classes, this is where I want to learn about them!)

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;
}
Emile Bergeron
  • 17,074
  • 5
  • 83
  • 129
guitarmatt99
  • 21
  • 1
  • 5

1 Answers1

5

Disclaimer: You shouldn't expect that kind of answer, you really should read more on OOP to get the point, this has nothing to do with SFML, this is just basic refactoring.

How to think with OOP

First thing first, before coding a feature, you should design the OOP structure that really suits the situation. See each class as part of a whole, that is your 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++ (more the OOP part for you) 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, your player is only a sprite and a position. So you encapsulate those data into your Player class as private members. That keeps other code from messing with the player data. To use the player data, you need to provide methods in the class that each affects only the Player.

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 took the time to change your code much, but you could change the way you handle the movement to only make one "move" method that takes a Player::Direction has 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 change your own code to avoid getting you lost.

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;
    };
Community
  • 1
  • 1
Emile Bergeron
  • 17,074
  • 5
  • 83
  • 129
  • Thanks for the reply, I have read through the code and so far I am understanding what is going on, I have researched bits that I didn't such as creating your own namespace which now I can see why it would be useful, I guess my brain isn't yet to the stage where I know how to work out more complex things! It would have been better if you explained what I needed to do rather than just giving me the code because I would have understood it easier. It seems quite complex just to do what I wanted, is that just me? – guitarmatt99 Oct 02 '13 at 19:51
  • Also, is there anything you recommend me looking at or reading about before I continue? I don't actually know why I didn't think about using references but I guess I just need to put all these into practise to get to a level where I don't need help! – guitarmatt99 Oct 02 '13 at 20:02
  • Yes you're right, it would benefit you more if I had explained a little bit what was going on. Let me try this by editing my answer. – Emile Bergeron Oct 02 '13 at 20:20
  • I tried to be clear, but there's so much information I could share... Maybe you could ask me question via comments so I could add the information that is missing. – Emile Bergeron Oct 02 '13 at 20:59
  • Also, is there not a less complex way just to check for player input? (sorry if that is a bad question). The only thing I wasn't able to do was access my sprite. Could I not load the texture and sprite and initialize the Vector2 and enumeration in the constructor of the class? – guitarmatt99 Oct 03 '13 at 17:50
  • Forward declaration isn't required, it's a c++ thing. See http://stackoverflow.com/a/4757718/1218980 for more info. I don't really get the question you ask, but what I could assure you is that it's a bad idea to load the Texture inside the constructor because it's a code that may fail (file not found, etc) and it will be kind of impossible to keep the program from crashing. – Emile Bergeron Oct 03 '13 at 17:56
  • Okay thanks for the info! Let me just ask, let's say I wanted to keep all my sprite loading in my main.cpp(just for now to help me understand better), and I wanted various functions to move that sprite(that are in a class), what is the best way to access that sprite from the class? Also, I'll try not to ask any more questions in the comments, if I need to I'll move it to a chat! – guitarmatt99 Oct 03 '13 at 18:26
  • Keep the Texture in the main function, it's ok right now, then create new instance of Sprite by passing the same Texture to those new Sprite object. Or use another Sprite to do so: `Sprite myNewSprite(myPlayer.getSprite());` – Emile Bergeron Oct 03 '13 at 19:00