Skip to content

tweak(fps): Decouple logic time step from render update #1451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

xezon
Copy link

@xezon xezon commented Aug 12, 2025

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:

  • non-network logic update
  • WW3D::Sync
  • Scripted camera movement time step
  • Drawable time step
  • Camera shaker time step
  • Camera zoom and rotation time step

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:

Key mapping Action
CTRL + Numpad+ Increase Max Render FPS with preset values
CTRL + Numpad- Decrease Max Render FPS with preset values
Shift + CTRL + Numpad+ Increase Logic Time Scale FPS by 5
Shift + CTRL + Numpad- Decrease Logic Time Scale FPS by 5

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

  • Replicate in Generals

@xezon xezon added this to the Important features milestone Aug 12, 2025
@xezon xezon added Bug Something is not working right, typically is user facing Enhancement Is new feature or request Major Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Rendering Is Rendering related labels Aug 12, 2025
@xezon xezon force-pushed the xezon/decouple-logic-render branch from 2b88c43 to 1036d47 Compare August 12, 2025 21:59
@Mauller
Copy link

Mauller commented Aug 13, 2025

You need to update your new 'm_logicTimeScaleFPS' from the game speed option in SkirmishGameOptionsMenu.cpp the idea was to allow the game to run faster. But since the logic and FPS were tied together, they just increase the FPS cap in the original implementation.

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);

@@ -100,9 +108,15 @@ class GameEngine : public SubsystemInterface
virtual ParticleSystemManager* createParticleSystemManager( void ) = 0;
virtual AudioManager *createAudioManager( void ) = 0; ///< Factory for Audio Manager

Int m_maxFPS; ///< Maximum frames per second allowed
Int m_maxFPS; ///< Maximum frames per second for rendering
Int m_logicTimeScaleFPS; ///< Maximum frames per second for logic time scale
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@xezon xezon Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not intent to rename m_maxFPS for this change to have a bit less diff.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_maxLogicFPS is not the correct terminology for this. Logic FPS is what we currently refer to as enum LOGICFRAMES_PER_SECOND=30.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_maxLogicFPS is 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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_logicTimeScaleFPS is conceptually not equivalent to LOGICFRAMES_PER_SECOND or m_maxLogicFPS. It effectively is a ratio that scales the logic fps.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect it will be possible to change LOGICFRAMES_PER_SECOND at runtime to any value above 0.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@xezon xezon Aug 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@xezon
Copy link
Author

xezon commented Aug 13, 2025

You need to update your new 'm_logicTimeScaleFPS' from the game speed option in SkirmishGameOptionsMenu.cpp the idea was to allow the game to run faster. But since the logic and FPS were tied together, they just increase the FPS cap in the original implementation.

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.

@Mauller
Copy link

Mauller commented Aug 13, 2025

You need to update your new 'm_logicTimeScaleFPS' from the game speed option in SkirmishGameOptionsMenu.cpp the idea was to allow the game to run faster. But since the logic and FPS were tied together, they just increase the FPS cap in the original implementation.

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.

@xezon
Copy link
Author

xezon commented Aug 13, 2025

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.

@Mauller
Copy link

Mauller commented Aug 13, 2025

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.

@xezon
Copy link
Author

xezon commented Aug 13, 2025

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.

@Mauller
Copy link

Mauller commented Aug 13, 2025

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.

Ah i see now, i thought you had the logic frame rate locking always enabled.

@aliendroid1
Copy link
Collaborator

Are the key mappings hard coded?

@xezon
Copy link
Author

xezon commented Aug 13, 2025

The default key mappings are. They can be overriden in CommandMap.ini

@WWB2-account
Copy link

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.

@xezon xezon force-pushed the xezon/decouple-logic-render branch from 1036d47 to a9147d6 Compare August 14, 2025 17:57
@xezon
Copy link
Author

xezon commented Aug 14, 2025

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.

The default behavior of scripts is not changed.

@xezon xezon force-pushed the xezon/decouple-logic-render branch from a9147d6 to a0d1f04 Compare August 14, 2025 18:19
@xezon xezon force-pushed the xezon/decouple-logic-render branch 2 times, most recently from 9faaab8 to 4bb9f4e Compare August 14, 2025 18:34
@WWB2-account
Copy link

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.

The default behavior of scripts is not changed.

True, but the fact remains that some scripts might interact with render frames while others interact with logic frames. Now that they are decoupled people with different settings will get different results (and potentially get mismatches if they play COOP, AOD, or even multiplayer skirmish. That should not be. It should be checked where this could happen.

@xezon
Copy link
Author

xezon commented Aug 14, 2025

True, but the fact remains that some scripts might interact with render frames while others interact with logic frames. Now that they are decoupled people with different settings will get different results (and potentially get mismatches if they play COOP, AOD, or even multiplayer skirmish. That should not be. It should be checked where this could happen.

The Network logic update is unchanged. No mismatch.

{
if (TheNetwork != NULL)
{
return TheNetwork->getFrameRate();
Copy link

@Mauller Mauller Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works at the moment because the networking module caps the value returned by this to 30, but not for the right reasons.

Need to cleanup a lot of the networking side still but what you put should work okay for the time being.

Int minFps;
Int minFpsPlayer;
getMinimumFps(minFps, minFpsPlayer);
DEBUG_LOG_LEVEL(DEBUG_LEVEL_NET, ("ConnectionManager::updateRunAhead - max latency = %f, min fps = %d, min fps player = %d old FPS = %d", getMaximumLatency(), minFps, minFpsPlayer, frameRate));
if ((minFps >= ((frameRate * 9) / 10)) && (minFps < frameRate)) {
	// if the minimum fps is within 10% of the desired framerate, then keep the current minimum fps.
	minFps = frameRate;
}
if (minFps < 5) {
	minFps = 5; // Absolutely do not run below 5 fps.
}
if (minFps > TheGlobalData->m_framesPerSecondLimit) {
	minFps = TheGlobalData->m_framesPerSecondLimit; // Cap to 30 FPS.
}

The connection manager sets the value and the networking uses this FPS value for setting the logic rate so it's quite a mess overall.

msg->setRunAhead(newRunAhead);
msg->setFrameRate(minFps);
//DEBUG_LOG(("ConnectionManager::updateRunAhead - new run ahead = %d, new frame rate = %d, execution frame %d", newRunAhead, minFps, msg->getExecutionFrame()));
sendLocalCommand(msg, 0xff ^ (1 << minFpsPlayer)); // Send the packet to everyone but the lowest FPS player.

@xezon xezon force-pushed the xezon/decouple-logic-render branch from 4bb9f4e to 95f1a50 Compare August 15, 2025 17:07
@xezon xezon force-pushed the xezon/decouple-logic-render branch from 95f1a50 to 3bf9396 Compare August 17, 2025 15:04
// TheSuperHackers @tweak The camera rotation and zoom are now decoupled from the render update.
const Real fpsRatio = (Real)BaseFps / TheGameEngine->getUpdateFps();
const Real rotateAngle = TheGlobalData->m_keyboardCameraRotateSpeed * fpsRatio;
const Real zoomHeight = (Real)View::ZoomHeightPerSecond * fpsRatio;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in this PR, but i wonder if we should make the zoom and camera rotate rates user adjustable at some point?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. I also think zooming is way too slow. It moves like snail.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it feels better, maybe we could have a followup PR to double the value like we did for the scrolling? before any user adjustable additions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working right, typically is user facing Enhancement Is new feature or request Gen Relates to Generals Major Severity: Minor < Major < Critical < Blocker Rendering Is Rendering related ZH Relates to Zero Hour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate game logic from framerate
4 participants