2
\$\begingroup\$

I was hoping I could get some feedback on performance of my animations overall. It could just be me but I keep getting a bit of lag despite being at 60FPS constantly.

Objects on screen seem to tear a little bit.

Here's my code in full.

Here is my game loop:

//clocks and times used to get custom game loop working
    sf::Clock clock;
    sf::Time timeSinceLastUpdate = sf::Time::Zero;

    //setting g_GameState to be intro on run
    g_GameState = 0;

            //main loop to run the entire length of games
            //life
            while (g_Window.isOpen())
            {
                sf::Time dt = clock.restart();
                timeSinceLastUpdate += dt;


                while(timeSinceLastUpdate > TIME_PER_FRAME)
                {

                    timeSinceLastUpdate -= TIME_PER_FRAME;
                    processEvents();
                    update(TIME_PER_FRAME);

                }
                updateFPSCounter(dt);
                render();
            }

It's this performance issue I'd like any feedback or advice on.

Also, if anyone thinks I could do collision detection better as well, could you give me any pointers?

For the player paddles I use this to update their movement:

 //Not using deceleration
    //so setting mVelocity as 0 each time
    mVelocity.x = 0;
    mVelocity.y = 0;

    //Handle if player keys are pressed to move up
    //or down
    if(mIsMovingUp){
        mVelocity.y = -mSpeed;
    }
    else if(mIsMovingDown){
        mVelocity.y = mSpeed;
    }


    //move the play based on current mVelocity size
    this->move(mVelocity * elapsedTime.asSeconds());
\$\endgroup\$
1

1 Answer 1

2
\$\begingroup\$

First suggestion: clean up your code. There are a ton of inconsistencies in variable names, indentation, brackets, defines ...

Variables

I was confused when seeing g_ variables in your game.h file, suggesting that these were global variables yet they are declared as member variables. My guess is that you moved away from global variables (which is a good idea) but forgot to fix the naming. I know it's annoying to change all those names now, but it'll save you a lot of time in the long run and many modern IDE's offer refactoring tools. Keep in mind that full-caps member names (such as FRAMES_PER_SECOND) are used to indicate constants or defines. In another file, Player.h, you use m as prefix. Be consistent, either use the m_ and g_ or m and g prefixes everywhere.

Moving away from global variables is a good idea because they are often quick hacks but often they can do more bad than good in the long run. Imagine declaring g_GameTime as a global variable, and using that in every manager. But when we want a single manager to update at half speed to mimic slow motion, we'll have to do nasty things. So in my opinion it's better just to give the time as a parameter to every manager.

Defines

Header guards should be the first thing in a header file. In game.h this is not the case, so move those ifdef's up to save some compile time and potential compile errors.

Const correctness

If I call a function such as

bool Game::isRectCollision(sf::RectangleShape &sprite1, sf::Sprite &sprite2)

I want to be sure my parameters and base object will not be modified. (If calculating if two rectangles intersect requires non-const parameters (so possibly changing that rectangle), something fishy is going on). The general rule should be to mark everything as const unless it is inpossible:

bool Game::isRectCollision(const sf::RectangleShape &sprite1, const sf::Sprite &sprite2) const

Now, regarding your animation smoothness/stuttering problem. The following while loop is a bit weird:

while(timeSinceLastUpdate > TIME_PER_FRAME)

Since update is inside this loop and render is not, it is possible that we do not update and render the same state of the game again. The other way around: it is possible that we update the game twice but only render once. Both of these scenarios are wrong. The first one because we are going to render the exact same scene again, the second one because we're going to calculate how the scene should look like and then do nothing with the result, but rather calculate it again. For starters, try to replace this second while loop with an if and place the render functions inside of it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.