best way to handle this? (Player class question)

[GunnDawg]

So I'm creating a player class that holds the players name, health, and mana. I figured this would be a good class to also hold the players sprite and texture to that sprite as well, though I'm starting to see where that might be an issue. Trying to wrap my head around the logical way to go about this. I'll leave my code here so you can see what I'm talking about.

Player.cpp

#include "Player.h"
#include "SFML/Graphics.hpp"

Player::Player(std::string p_Name, int p_Health, int p_Mana, sf::Texture p_Texture)
{
        Name = p_Name;
        newHealth = p_Health;
        newMana = p_Mana;
        newTexture = p_Texture;
}

Player::~Player()
{

}

//Get class data.
std::string Player::getName()
{
        return Name;
}

int Player::getHealth()
{
        return newHealth;
}

int Player::getMana()
{
        return newMana;
}

//Set class data.
void Player::setName(std::string p_Name)
{
        Name = p_Name;
}

void Player::setHealth(int p_Health)
{
        newHealth = p_Health;
}

void Player::setMana(int p_Mana)
{
        newMana = p_Mana;
}

void Player::setTexture(sf::Texture pTexture)
{
        newTexture = pTexture;
}

void Player::setSprite(sf::Sprite pSprite)
{
        Sprite = pSprite;
}
 

Player.h

#pragma once
#ifndef PLAYER_H
#define PLAYER_H

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

class Player
{
public:
        Player();

        Player(std::string, int, int, sf::Texture);

        ~Player();

        std::string getName();
        int getHealth();
        int getMana();


        void setName(std::string);
        void setHealth(int);
        void setMana(int);
        void setSprite(sf::Sprite);
        void setTexture(sf::Texture);

private:
        std::string Name;
        int newHealth;
        int newMana;
        sf::Texture newTexture;
        sf::Sprite Sprite;
};


#endif
 

main.cpp

#include "SFML/Graphics.hpp"
#include "Player.h"
#include <conio.h>
#include <iostream>
#include <string>

int main()
{
        std::string name;
        int health = 0, mana = 10;
        sf::Texture DarthVader;
        sf::Texture pTexture;
        sf::Sprite mainChar;

        Player Player1(name, health, mana, pTexture);
        std::cin >> name;
        Player1.setName(name);
        Player1.setTexture(DarthVader);
        Player1.setSprite(mainChar);
       
        std::cout << Player1.getMana();
        std::cout << Player1.getName() << std::endl;
       
        return 0;
}
 

Now obviously having a player class function "setTexture" is going to cause a conflict (at least a think). Secondly, do I even need to set the sprite and the texture with a class function? The player will be able to choose to play from a few different characters, but does that require setting the sprite and texture for each one or should I just have a setTexture for the sprite? I'm a little confused on how to best go about this.

Any input is appreciated. Thank you and Merry Christmas!

[Ixrec]

It's true that right now anyone looking at the header would have no idea how SFML's setTexture() interacts with your setSprite() and setTexture() functions.  I can think of two sane directions to go with this, though I'd recommend #2.

1) The Player class "fully owns" textures, in the sense that it handles loading them and calling setTexture() on the sprite.  In this case the Player class API would probably take an enum value like PLAYER_CHARACTER_DARTHVADER, rather than an actual Texture.  As long as you're sure you never load the same file twice, this is okay (of course, as soon as you want multiplayer or AI companions, it's not).

2) The Player simply does not own a texture in any way, shape or form, instead relying on the client code to load, own and set the player's texture.  I believe this is the more commonly recommended solution, and it probably is the more architecturally sound option because it avoids the mutliple players problem I pointed out above, and managing resources is non-trivial but logically separate from the Player class.  Someday you may want a "proper" resource manager class for your textures that does the same thing for all Player/Enemy/Background textures.


Regarding the rest of your code:
- You should pass non-fundamental types like std::string or sf::Texture by reference, unless you're very sure you want them to get copied for some reason.
- Don't explicitly write out the destructor unless you plan to put some code in it.  The compiler won't forget to generate it.
- Are you sure Player should be default constructible?  A zero-by-zero sprite with name '' and zero health/mana doesn't seem like a "valid" default player to me.
- At the moment your Player class is just for getters and four setters with no "real code" in it, so it would be a lot more straightforward to simply make all four members public (unless of course you're about to add some real code in there).
- Why do the Player members have "new" in their names?
- I would recommend using some kind of prefix or suffix for member variables.  Right now there's no ambiguity because you prefixed all the parameters instead, but as soon as you have non-trivial methods with local variables it the risk of ambiguity comes back.

[GunnDawg]

2) The Player simply does not own a texture in any way, shape or form, instead relying on the client code to load, own and set the player's texture.  I believe this is the more commonly recommended solution, and it probably is the more architecturally sound option because it avoids the mutliple players problem I pointed out above, and managing resources is non-trivial but logically separate from the Player class.  Someday you may want a "proper" resource manager class for your textures that does the same thing for all Player/Enemy/Background textures.

So with this option I would simply not handle setting the player Texture and Sprite in the class but instead creating the sprite and assigning its texture outside of the scope of the class? All events after that would need to interact with the sprite itself, correct? So there would be no, Player1.move(); (Player1 being the object name), but instead would call the sprite it self to move? Then handle all health, mana, etc changes using the class functions (setHealth, setMana, etc)? Was trying to go for a full on way to have all events reference to the player object and not have to juggle between independent data outside of the class, but if this is the preferred method (without the use of a resource manager), then I suppose it'll do.

[Ixrec]

I guess I should have split that into 2a and 2b.  What you said is 2a.

2b) The Player class duplicates (at least some of) the sf::Sprite interface rather than being passed an sf::Sprite, thereby removing all the ambiguity but keeping all the convenience.  It would probably still store a sprite internally and most of this interface would simply forward to the sprite (including setTexture()), but the client code interacts with Player as if it was a sprite.  This gives you the freedom to do both advanced resource management and advanced rendering (ie, more complex than an sf::Sprite) in the future without changing the architecture.

The only downside to this is writing (and maintaining!) the wrapper functions.  But I think that's a smaller issue than all the ones you rightly pointed out just now.

[GunnDawg]

Now I'm just confused. Are there examples out of there that I'm not finding that shows a standard player class for simple things like name, health, and sprite and/or texture use that I can look and to try and implement myself?

[Ixrec]

There's no such thing as a "standard Player class" because every game is different.

What I was attempting and failing to suggest was this:

class Player
{
public:
    Player(std::string, int, int, sf::Texture);
    std::string name;
    int health;
    int mana;

    move(...) { d_sprite.move(...); }
    setTexture(...) { d_sprite.setTexture(...); }
    /* the other sf::Sprite methods you need */
private:
    sf::Sprite d_sprite;
};
 

Since this allows you to do Player1.move() and Player1.setTexture() without any of the ambiguity over who owns the sprite or the texture or who is responsible for linking one to the other, and while keeping the heavy resource (the texture) outside of the class.