0
votes

I' creating a simple game, using game states. I have a gameLoop() method that calls a running()method, which contains the switching states, which works correctly. This is the gameLoop() method:

void Game::gameLoop()
{
  while (window.isOpen()) {
    Event event;
    while (window.pollEvent(event)) {
      if (event.type == Event::Closed){
        window.close();
      }
    }
    if (Keyboard::isKeyPressed(Keyboard::Escape)) {
      window.close();
    }

    window.clear();
    running();              //this calls the states
    window.display();
}

running() method calls all the states:

void Game::running()
{

    switch (state)
    {
        //other cases here

        case s_play:
            play();
            break;

        default:
            break;
    }
}

And the play() method draws the sprite and moves it:

void Game::play()
{
    Texture myTexture;   //I've also tried to declare these outside the method
    Sprite mySprite;

///////////Graphics

    myTexture.loadFromFile("res/img/player.png");

    mySprite.setTexture(myTexture);

//////////Movement

    static sf::Clock clock;
    float dt = clock.restart().asSeconds();
    Vector2f move;

    if (Keyboard::isKeyPressed(Keyboard::A)) 
        {
            move.x--;
        }

        std::cout << mySprite.getPosition().x << "\n";
    }

    if (Keyboard::isKeyPressed(Keyboard::D))
        {
            move.x++;
        }

        std::cout << mySprite.getPosition().x << "\n";
    }

    mySprite.move(move*300.0f*dt);

    window.draw(mySprite);

}

The problem is that the sprite just moves on the spot, and the output obtained from the std::cout is the following, when pressing A or D:

enter image description here

The function of the movement works, because it was tested elsewhere. I think that I'm updating correctly or initializing something in the wrong way, but I can't figure it out.

2
Its a waste of resources to create the texture and sprite every iteration.Arnav Borborah
Why load the texture and set the sprite in every call? That's just wasted cycles. Do it once only. Perhaps the texture, the sprite and the clock should be member variables in the Game class instead?Some programmer dude
sf::Clock::restart() returns the elapsed time and then restarts it @Someprogrammerdude. The clock is static, so that shouldn't make a differenceArnav Borborah
Lastly, is the code you show your actual code, or just something you written in the question? Because the code you show is invalid. You should really create a Minimal, Complete, and Verifiable Example to show us.Some programmer dude
@Someprogrammerdude yeah, I had those as member of the class, I put a comment in the code to say that. About the clock, it works fine like it is in another projectKoosshh56

2 Answers

1
votes

I am not familiar with SFML so I hope I am not off base here, however, looking at your code. In your play() method, you create a local variable move. From the looks of it, move contains the x and y coordinate for your Sprite. Since you define move inside the play() method, a local copy of this variable is created on the stack at run time each time your code runs through the play() method. Then you check for key presses, and inc or dec move accordingly. move needs to be at global scope so that it is not reset every time you call play(). You were correct when you moved myTexture and mySprite outside the function play(). You should also move move outside of the play() method.

Something like this:

Texture myTexture;   //I've also tried to declare these outside the method
Sprite mySprite;
Vector2f move;

    void Game::play()
    {

    ///////////Graphics

        myTexture.loadFromFile("res/img/player.png");

        mySprite.setTexture(myTexture);

    //////////Movement

        static sf::Clock clock;
        float dt = clock.restart().asSeconds();

        if (Keyboard::isKeyPressed(Keyboard::A)) 
            {
                move.x--;
            }

            std::cout << mySprite.getPosition().x << "\n";
        }

        if (Keyboard::isKeyPressed(Keyboard::D))
            {
                move.x++;
            }

            std::cout << mySprite.getPosition().x << "\n";
        }

        mySprite.move(move*300.0f*dt);

        window.draw(mySprite);

    }

Hope that helps

0
votes

You should declare myTextureand and mySprite outside of the loop on a place where they stay as long as they should. Currently you are creating them again in each iteration, which is bad for performance (espacially myTexture.loadFromFile("res/img/player.png");). The recreation resets also the transform (position, rotation, etc.)