tweak(fps): Decouple logic time step from render update#1451
tweak(fps): Decouple logic time step from render update#1451xezon merged 7 commits intoTheSuperHackers:mainfrom
Conversation
2b88c43 to
1036d47
Compare
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp
Outdated
Show resolved
Hide resolved
|
You need to update your new 'm_logicTimeScaleFPS' from the game speed option in This will also have a knock on effect that the FPS cap will also need to be increased to match the increased logic rate. GameWindow *sliderGameSpeed = TheWindowManager->winGetWindowFromId( parentSkirmishGameOptions, sliderGameSpeedID );
Int maxFPS = GadgetSliderGetPosition( sliderGameSpeed );
setInt("FPS", maxFPS); |
There was a problem hiding this comment.
It would be better to rename m_maxFPS to make it more relevant to the rendering FPS.
m_maxRenderFPS for example.
For the logic one, it would be better to have the naming match the rendering.
m_maxLogicFPS etc.
There was a problem hiding this comment.
I did not intent to rename m_maxFPS for this change to have a bit less diff.
There was a problem hiding this comment.
m_maxLogicFPS is not the correct terminology for this. Logic FPS is what we currently refer to as enum LOGICFRAMES_PER_SECOND=30.
There was a problem hiding this comment.
m_maxLogicFPSis not the correct terminology for this. Logic FPS is what we currently refer to as enum LOGICFRAMES_PER_SECOND=30.
It's the currently set max logic frame rate, LOGICFRAMES_PER_SECOND is the default maximum value. But since we can / need to be able to vary the current max logic frame rate, it makes sense to call it that for the variable.
There was a problem hiding this comment.
m_logicTimeScaleFPS is conceptually not equivalent to LOGICFRAMES_PER_SECOND or m_maxLogicFPS. It effectively is a ratio that scales the logic fps.
There was a problem hiding this comment.
I expect it will be possible to change LOGICFRAMES_PER_SECOND at runtime to any value above 0.
There was a problem hiding this comment.
m_logicTickRate might be a better name for these instead of using logicTimeScale since time scale implies a period of time instead of a rate of action.
There was a problem hiding this comment.
I do not see that logic tick rate is a better name. Logic time scale is accurate, because time scale 2 means that the game runs twice as fast. If we can avoid changing the name, then that would be good, because it will take a while to rename everything.
There was a problem hiding this comment.
But this is never set to a value that acts as a multiplier, it's set to a specific Tick rate / frames per second. That is then used to cap the update of the game logic.
Which is why the naming does not look correct to me on the surface compared to what you might have in mind.
There was a problem hiding this comment.
The value Int m_logicTimeScaleFPS is a multiplier in disguise. It is synonymous to Real m_logicTimeScaleRatio, just with a different unit. As we can see in the GameEngine class interface, the time scale ratio can be inferred from the fps value.
No. This change does not intent to touch it yet as described in the pull request description. For now, the new limits are only accessible with the new key mappings and the game still applies its fps limits as it always did. The reason for that is, this change is already big enough and I do not want to risk breaking anything yet. It is a more careful rollout. |
So this is a breaking change then as it breaks the game speed adjustment in skirmish. |
Can you elaborate? When I tested, by default, it did not change behavior. |
The game speed adjustment is there to allow the game logic to tick faster than 30 frames, but this was tied to FPS at the same time in the original implementation. it was never about allowing a higher framerate. If you are only increasing render FPS limit then the game logic won't be running faster for the faster game speed. |
|
It does, because Logic Time Scale FPS is disabled by default. Unless I made a mistake or the new key mappings are pressed in game, the default game behaviour should be the exact same. I suggest to give this a test in game. |
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp
Outdated
Show resolved
Hide resolved
Ah i see now, i thought you had the logic frame rate locking always enabled. |
|
Are the key mappings hard coded? |
|
The default key mappings are. They can be overriden in CommandMap.ini |
|
It would be good to investigate how this change interacts with already existing ways to change game speed and FPS. There are scripts that do very similar things as this PR. This change has the potential to break every single script in all maps in the history of the game if we're not careful. Investigation of internal script timers, frame counters and more is probably needed to verify this doesn't break anything. Changing scripts in general should be done with utmost consideration. Additionally, I can imagine that in some cases the game logic should not be increased beyond a specific value. Examples are cinematic intros in campaign maps (eg. ZH USA 01), or maps with many audio events like Generals' Challenge maps. In such cases the logic should have a custom cap specifically tied to that map, likely specified in the map.ini. |
1036d47 to
a9147d6
Compare
3bf9396 to
3da5aed
Compare
|
Replicated to Generals with minimal conflicts. Tested and worked. |
|
Is there a way for the game to start with a custom FPS limit? Currently, 30 is the default value. GeneralsOnline starts with 60 and you can see that the shell map camera takes a bit longer to start moving, so I guess that this issue is present on this build too, only that I can't verify that because the game always starts at 30. |
|
The forced FPS limit will be removed. |
Merge with Rebase
This change has several commits to fix and improve several aspects in regards to the render frame time in relation to the logic time step.
The frame limiter has been reimplemented with a high accuracy counter for more accurate FPS capping.
The following systems are now decoupled from the render update:
There are way more things that could be decoupled but for starters this appears to be good enough and usable.
To control the Logic Time Scale FPS and Render FPS the following default key mapping are added:
Changes to the FPS are currently NOT saved to Options.ini and the game still applies its frame rate adjustements in Skirmish, Campaign and similar as usual. By default, the game behaves as originally and this feature is currently entirely opt-in with the key mappings.
TODO