I think it's fine.
Here's the way I do it for a deterministic approach:
// time variables
float count = 0.f;
sf::Clock clock;
// --- game loop ---
while (App.IsOpened()){
// --- process events ---
(...)
// --- update frames ---
// each update is made within a frame time
count += clock.GetElapsedTime();
while (count>=FRAME_RATE){
count -=FRAME_RATE;
updateState(...);
}
// --- draw the game state ---
drawEverything(...);
App.Display();
// --- makes the game sleep, releasing CPU usage ---
// use elapsed time to know how much time is left
// if it's too late to sleep, count remains unchanged to compensate the lag on the next game loop
count += clock.GetElapsedTime();
if (count < FRAME_RATE){
sf::Sleep(FRAME_RATE - count); // sleep the remaining time
count = FRAME_RATE; // update count to the new time (end of frame)
}
clock.Reset();
}
Every update uses fixed values (for example for velocity), and the expected effect is that there is an update every frame. If for any reason the game lags and a loop on the main cycle takes more than the FRAME_RATE time, it means there are updates missing, so they are made in a row on the next cycle without being drawn. After those updates, the game state is drawn on the screen. If there is still time left, it's passed on sleeping, if not, the lack of time is propagated to the next cycle, forcing it to do more updates to compensate.
This is assuming that drawing is a while more time consuming than updates (as it uses to be), and they can be skipped between updates. If the game lags much it means the updates are taking too long.
This approach is also good to avoid updates taking unexpected values. Like for velocity on your approach, if the game lags, the velocity can take a big value because of the elapsed time, and as result the character can overpass a wall for instance. On this approach, changes always takes the expected values.
Edit: For a framerate of 60fps:
static const float FRAME_RATE = 0.0166666;
