feat(frame-pacer): overhaul logic and render FPS limits and presets by githubawn · Pull Request #2699 · TheSuperHackers/GeneralsGameCode

githubawn · 2026-05-10T20:12:03Z

Added 15 FPS to render presets and expanded logic presets (1 to 960 FPS). (15 is the minimum in skirmish, not 30)
Implemented array-based preset snapping for logic FPS.
Removed redundant logic speed scaling guards and safety asserts.
Removed debug lock for lower logic rates, they are fun features.

This change was generated with AI assistance. All generated code has been reviewed, tested, and verified for functionality.

- Added 15 FPS to render presets and expanded logic presets (1 to 960 FPS). - Implemented array-based preset snapping for logic FPS. - Renamed extraStep to snapValue for clarity. - Removed redundant logic speed scaling guards and safety asserts. - Cleaned up stale comments and dead code in CommandXlat.

greptile-apps · 2026-05-10T20:18:38Z

Greptile Summary

This PR replaces the fixed-step (+5/-5) logic FPS arithmetic with array-based preset snapping and widens both the render and logic preset ranges. The logic preset now spans 1–960 + Uncapped, render gains 15 FPS as its new minimum, and changeFpsValue accepts an optional snapValue so the caller can provide the current render FPS as an implicit intermediate stop.

Render presets: 15 FPS added as the minimum entry, matching the documented skirmish floor; existing entries unchanged.
Logic presets & snapping: A new s_fpsValues array drives stepping for logic FPS; the snap mechanism ensures that render FPS is always a reachable intermediate point between presets, and the per-step minimum floor is replaced by the array floor of 1.
CommandXlat cleanup: The old step-based ceiling/remainder calculation is removed; setLogicTimeScaleFps is now only called when timescale is being enabled, which preserves the stored value so re-enabling resumes from the last real FPS.

Confidence Score: 5/5

Safe to merge — the reworked step logic correctly handles snap, floor, and uncapped edge cases, and the CommandXlat simplification preserves the stored FPS value when timescale is disabled.

The array-based preset arithmetic is correct across all traced edge cases (floor at 1, snap to render FPS, UncappedFpsValue on both sides, equal-value snap skip). The CommandXlat change intentionally skips setLogicTimeScaleFps when disabling timescale, matching the inline comment's explanation. No defects were found in the changed paths.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/Common/FrameRateLimit.h	Removed the enum-based step/min constants from LogicTimeScaleFpsPreset, replaced with a private array declaration and updated public API to accept an optional snapValue parameter.
Core/GameEngine/Source/Common/FrameRateLimit.cpp	Defines the new logic-preset array (1–960 + Uncapped), adds 15 to render presets, and replaces fixed-step math with array-based search that incorporates an optional snap candidate (render FPS).
Generals/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp	Simplifies changeLogicTimeScale: removes the old step-based ceiling/remainder math, passes maxRenderFps as snap hint, and only calls setLogicTimeScaleFps when timescale is being enabled.
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp	Identical simplification to the Generals variant — mirrors the same changeLogicTimeScale overhaul for the Zero Hour expansion.

_{Reviews (2): Last reviewed commit: "greptile fixes" | Re-trigger Greptile}

Skyaero42 · 2026-05-12T08:18:59Z

-	// Set value after potentially enabling it.
-	if (TheFramePacer->isLogicTimeScaleEnabled())
+	const bool enableTimescale = (logicTimeScaleFps < maxRenderFps);
+	// TheSuperHackers @info Preserve the last real FPS in m_logicTimeScaleFPS so re-enabling timescale resumes from a sane value.


Add empty line between const bool declaration and comment for better readability

Skyaero42 · 2026-05-12T08:20:31Z

 	{
 		TheFramePacer->setLogicTimeScaleFps(logicTimeScaleFps);
 	}
+	TheFramePacer->enableLogicTimeScale(enableTimescale);


Add empty line between closing bracket and TheFramePacer->enableLogicTimeScale for better readability.

Optionally: remove whiteline below TheFramePacer->enableLogicTimeScale

Skyaero42 · 2026-05-12T08:21:59Z

 	{
 		TheFramePacer->setLogicTimeScaleFps(logicTimeScaleFps);
 	}
+	TheFramePacer->enableLogicTimeScale(enableTimescale);


Add empty line between closing bracket and TheFramePacer->enableLogicTimeScale for better readability.

Optionally: remove whiteline below TheFramePacer->enableLogicTimeScale

Skyaero42 · 2026-05-12T08:22:08Z

-	// Set value after potentially enabling it.
-	if (TheFramePacer->isLogicTimeScaleEnabled())
+	const bool enableTimescale = (logicTimeScaleFps < maxRenderFps);
+	// TheSuperHackers @info Preserve the last real FPS in m_logicTimeScaleFPS so re-enabling timescale resumes from a sane value.


Add empty line between const bool declaration and comment for better readability

Skyaero42 · 2026-05-12T08:28:22Z

-	case FpsValueChange_Increase: return getNextFpsValue(value);
-	case FpsValueChange_Decrease: return getPrevFpsValue(value);
+	case FpsValueChange_Increase: return getNextFpsValue(value, snapValue);
+	case FpsValueChange_Decrease: return getPrevFpsValue(value, snapValue);


Shouldn't default and case indented here?

Skyaero42 · 2026-05-12T08:30:18Z

 {
 	switch (change)
 	{
 	default:


The default falls through to case FpsValueChange_Increase which is quite the curious (and maybe unexpected behaviour. I think it warrants a comment.

Better (if possible): change should only have value FpsValueChange_Increase or FpsValueChange_Decrease, so default should never be called. Maybe put it at the end and add an assert instead.

Skyaero42 · 2026-05-12T08:31:40Z

+	for (int i = (int)ARRAY_SIZE(s_fpsValues) - 1; i >= 0; --i)
 	{
-		return value - StepFpsValue;
+		if (s_fpsValues[i] < value)


I could be wrong, but wouldn't

int fpsValue = s_fpsValues[i];

add a slight performance increase, as otherwise the array value needs to be looked up three times?

Skyaero42 · 2026-05-12T08:39:50Z

+	// Check if snapValue (e.g. current render FPS) is the previous closest candidate.
+	// Note: if snapValue == value, neither branch below fires and the snap point is
+	// intentionally skipped — the caller must step to a different preset.
+	if (snapValue < value && snapValue > prevValue)


swapping conditions

if (snapValue > prevValue && snapValue < value)

has better readability

Skyaero42 · 2026-05-12T08:41:15Z

+	// Check predefined steps
+	for (size_t i = 0; i < ARRAY_SIZE(s_fpsValues); ++i)
+	{
+		if (s_fpsValues[i] > value)


I could be wrong, but wouldn't

int fpsValue = s_fpsValues[i];

add a slight performance increase, as otherwise the array value needs to be looked up three times?

Skyaero42 · 2026-05-12T08:45:40Z

+UnsignedInt LogicTimeScaleFpsPreset::getNextFpsValue(UnsignedInt value, UnsignedInt snapValue)
 {
-	return value + StepFpsValue;
+	UnsignedInt nextValue = s_fpsValues[ARRAY_SIZE(s_fpsValues) - 1]; // Default to Uncapped


greptile-apps Bot reviewed May 10, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/Common/FrameRateLimit.cpp

greptile fixes

922e5d5

Skyaero42 requested changes May 12, 2026

View reviewed changes

Conversation

githubawn commented May 10, 2026

Uh oh!

greptile-apps Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented May 10, 2026 •

edited

Loading