Sign in to follow this  

SFML Cannot get sprite rect to change

This topic is 675 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi everyone, so i'm trying to get a animation by moving the rectangle the the sprite has selected every frame while a certain key is pressed.

My first step was getting the rectangle to actually change when I press a key, and then I can limit it later. Unfortunately, I cannot get there. If anyone could help that would be great. 

I'm posting this in a hurry as i'm about to leave. My code is here:

 

http://pastebin.com/LsWr5kDM

 

if this should be posted somewhere else, I apologize, the mistake is because i'm in a hurry to leave and didn't have a whole lot of time to look around!

 

Thx to anyone who so much as looks, anything is appreciated.

 

 

Share this post


Link to post
Share on other sites

C++ isn't my strength, but I don't see anywhere where you update the position of the rectangle on the sprite sheet. I'm assuming that you intend to do that in your moveTextureRect functions, where you update the Player's position but then set the textureRect for playerSprite with the playerRect object, which never changes.

Share this post


Link to post
Share on other sites

Khaiy is correct.

 

When you do this:

int moveTextureRectX(int diffX)
{
    playerPos.x += diffX;
           
    playerSprite.setTextureRect(playerRect); //<-- You never changed "playerRect"
    return playerPos.x;
}

...you never actually changed 'playerRect'.

 

Further, you have multiple ways to store positions and multiple ways to store sizes. That's not good. There should only be one variable to store one concept. You don't want to redundantly store the same concept (position or size) in multiple unrelated variables.

 

Also, longer more-descriptive function and variable names are better. "cPos" is not a good function name. "move()" or "changePos()" or even "changePosition()" are better names. Don't hyper-abbreviate.

 

On another note, this kind of function is not good:

int getTextureRectValues(int x)
{
    if (x == 1)
    {
        return playerPos.x;
    }
    else
    {
        return playerPos.y;
    }
}

The function should return return a single variable, not different variables depending on what is passed in.

 

The function should just return the player position directly:

vvvvvvvvvvvv
sf::Vector2i getTextureRectValues()
{
    return playerPos;
}

Share this post


Link to post
Share on other sites
You may want to use an enum for directions and make a changeDir() function that will accept a direction as its argument.
class Player {
  //blah blah
  enum Direction : int { DOWN = 0, LEFT, RIGHT, UP };
  Direction facing = DOWN;
  const int TEXTURE_CELL_HEIGHT = 24;

  void changeDir(Direction dir) {
    if(dir != facing) {
      facing = dir;
      sf::IntRect srcRect = playerSprite.getTectureRect();
      srcRect.y = dir * TEXTURE_CELL_HEIGHT;
      playerSprite.setTextureRect(srcRect);
    }
  }

    //...
    if (sf::Keyboard::isKeyPressed(sf::Keyboard::W))
    {
      changeDir(UP);
      player.cPos(0, -10);   
    }
    //...

  //blah blah
}
 
Also, the class is named 'Player', so you don't have to name it's members 'playerThis' and 'playerThat'. You can just use 'this' and 'that', since they're already scoped within Player.

Share this post


Link to post
Share on other sites

You may want to use an enum for directions and make a changeDir() function that will accept a direction as its argument.

class Player {
  //blah blah
  enum Direction : int { DOWN = 0, LEFT, RIGHT, UP };
  Direction facing = DOWN;
  const int TEXTURE_CELL_HEIGHT = 24;

  void changeDir(Direction dir) {
    if(dir != facing) {
      facing = dir;
      sf::IntRect srcRect = playerSprite.getTectureRect();
      srcRect.y = dir * TEXTURE_CELL_HEIGHT;
      playerSprite.setTextureRect(srcRect);
    }
  }

    //...
    if (sf::Keyboard::isKeyPressed(sf::Keyboard::W))
    {
      changeDir(UP);
      player.cPos(0, -10);   
    }
    //...

  //blah blah
}
 
Also, the class is named 'Player', so you don't have to name it's members 'playerThis' and 'playerThat'. You can just use 'this' and 'that', since they're already scoped within Player.

 

Thanks for this!, I actually didn't know I could do that when I started this project, but after school started last year, about august, I had learned that in one of my computer science classes. It's been almost a year since I actually put this project down to continue my studies, and coming back to it i've seen a lot of things I need to change; I would like to get it working before I go back and polish it with my new knowledge.

The enum based movement is another thing I actually was meaning to implement before I put the project down. I actually have a note telling myself to look into it in my notebook, now that I know how, it should be a quick fix, but first, a working product. 
EDIT:

I should make a note that the animation thing is actually required by what i'm doing this for. It's the next step up in the ladder of skills I wanted to make myself learn and improve on, otherwise everything does work, and I could start polishing now, but this is the last thing in the box that's actually required in my MVP.

Edited by CrookedMailman

Share this post


Link to post
Share on other sites

C++ isn't my strength, but I don't see anywhere where you update the position of the rectangle on the sprite sheet. I'm assuming that you intend to do that in your moveTextureRect functions, where you update the Player's position but then set the textureRect for playerSprite with the playerRect object, which never changes.

This was my original thought. I wasn't 100% sure, and I'm grateful that you were able to confirm this with me. I'm gonna rattle my brain some more now that I am home, and hopefully kick this out first thing tonight.

Share this post


Link to post
Share on other sites


Khaiy is correct.
 
When you do this:
int moveTextureRectX(int diffX)
{
playerPos.x += diffX;

playerSprite.setTextureRect(playerRect); //<-- You never changed "playerRect"
return playerPos.x;
}
...you never actually changed 'playerRect'.
 
Further, you have multiple ways to store positions and multiple ways to store sizes. That's not good. There should only be one variable to store one concept. You don't want to redundantly store the same concept (position or size) in multiple unrelated variables.

 

A lot of this project was me actually learning and experimenting with SFML. Now that im tons more familiar I'll clean this code up and get everything set straight once I get this working.

 

 

 


Also, longer more-descriptive function and variable names are better. "cPos" is not a good function name. "move()" or "changePos()" or even "changePosition()" are better names. Don't hyper-abbreviate.

 

I do agree 100%, I made this code about a little over a 6 months or so, the more I think about it, the further back it seems to go, but I had to put this down to continue school, and I've learned more in my computer science classes, and will definitely change that when I make it look pretty.

 


On another note, this kind of function is not good:
int getTextureRectValues(int x)
{
if (x == 1)
{
return playerPos.x;
}
else
{
return playerPos.y;
}
}
The function should return return a single variable, not different variables depending on what is passed in.

 

This was never ment to stay in, I was trying to seperate them to make sure other things were working, and it was a test function I ment to erase, and am doing so after I type this :P.

Share this post


Link to post
Share on other sites

This topic is 675 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this