KelvinShadewing / brux-gdk

Free runtime and development kit using SDL and Squirrel
GNU Affero General Public License v3.0
40 stars 20 forks source link

Frame-timing logic causes unintended frame rates #31

Closed ghost closed 2 years ago

ghost commented 2 years ago

It seems that there's a small issue in the code used to delay each frame for the FPS, and it's not particularly obvious. The only way I was able to even find this was through using an FPS monitor and subsequently finding it with trial-and-error. Easiest way to explain this is with the code in question:

gvTicks = SDL_GetTicks();
Uint32 fLength = gvTicks - gvTickLast;
Uint32 max_delay = (1000 / gvMaxFPS);
if (fLength < max_delay) {
    if (gvMaxFPS != 0) SDL_Delay(max_delay - fLength);
}
if(fLength != 0) gvFPS = 1000 / (SDL_GetTicks() - gvTickLast);
gvTickLast = gvTicks; //the problem

The problem lies in the last line, the assignment of gvTickLast to gvTicks isn't doing what's intended. The intention seems to be to save the value of gvTicks from the prior frame, so that when fLength is calculated it accounts for any delay from the logic in the main loop.

However, the issue is that the value of gvTickLast is assigned after the SDL_Delay has been performed, meaning that gvTickLast is equal to the value of SDL_GetTicks prior to the delay. In practice, assuming 60FPS, this means that 16ms will have passed between gvTicks being gotten and gvTickLast being assigned to gvTicks.

The result is that the value of gvTickLast will be one delay behind, and so when fLength is calculated on the following frame, there is almost no delay performed. But then since there is no delay, gvTickLast will now roughly equal gvTicks, and so fLength will then perform correctly on the next frame when SDL_GetTicks() is retrieved again. What this means is that the intended frame-rate is sort-of doubled, so 60FPS will actually result in something like 120FPS but within the same 60FPS time frame.

Now as for why this isn't readily obvious, that's because the calculation for gvFPS is still working technically. On the frames with no delay, gvTickLast is behind by the value of max_delay, so subtracting SDL_GetTicks() from it results in a correct FPS calculation.

With all that explained, the fix on the engine-level is simple, just change gvTickLast to be another call to SDL_GetTicks(). Then the fLength calculation is doing what's intended: calculating the delay between the end of the last delay and the start of the next one.

However, despite the fix itself being easy, the true crux of this issue lies in the consequences from this error. If you just apply this fix as-is, you'll immediately notice that some Brux apps are slowed down by roughly a factor of 2. This is a result of the game logic being subjectively defined based on the frame rate, so what was assumed to work for 60FPS was actually being programmed for something like 90-120FPS. I don't have a solution for this part, other than that whichever way you decide to tackle this will probably have to be in one fell-swoop, as this issue with the engine and the apps is interconnected.

KelvinShadewing commented 2 years ago

Oh dang, it's that serious?! Yeah, I'm gonna fix this right away. I'll just go through and fix the speed values in my own game as well. Thanks so much for pointing this out! Even though fixing the game is gonna take a while, I think this'll be a plus overall, since now it'll be doing the needier calculations like hit-detection half as often. Thanks again for this!