fix(view): Adjust default camera height to compensate for screen aspect ratio by Mauller · Pull Request #1711 · TheSuperHackers/GeneralsGameCode

Mauller · 2025-10-18T16:39:45Z

Merge with Rebase

Merge after: bugfix(view): Fix and improve map border camera constraints behavior to allow see all areas of the map comfortably #1752
Fixes: Camera height does not scale properly along with aspect ratio of game resolution #78
Closes: [ZH] Adjust field of view based on aspect ratio #518

This PR adjusts the default max camera height to compensate for different screen aspect ratios.

Previous work fixed the vertical field of view to match retail and adjusted the horizontal fields of view.
But this resulted in significant distortion being observed in the periphery of the view and lower corners.
This distortion was also observed at the 16:9 aspect ratio and became significantly worse as the aspect ratio increased.

By maintaining the original 60 degrees horizontal field of view, while allowing the game to narrow the vertical field of view, increasing the camera height results in minimal to no distortion being observed.

~~This PR does not fix any other known camera issues and focuses on giving a playable default view.~~

TODO

Implement in generals

ElTioRata · 2025-10-18T18:34:46Z

Comparison

xezon · 2025-10-19T11:08:18Z

At 4:3 "This Change" gives identical view to "Original Game + GenTool" ✅

At 16:9 "This Change" gives a bit more view than "Original Game + GenTool" ❓

Is this correct? I do not recall if GenTool view is correct.

Mauller · 2025-10-19T11:16:50Z

At 4:3 "This Change" gives identical view to "Original Game + GenTool" ✅

At 16:9 "This Change" gives a bit more view than "Original Game + GenTool" ❓

Is this correct? I do not recall if GenTool view is correct.

My goal was to try match the vertical view as close as possible to retail, I think it's slightly less than compared to 4:3 but slightly more than 16:9 gentool + retail.

I think it has a reasonable tradeoff considering the extended horizontal view.

It also gives a reasonable view for aspect ratios between 4:3 and 1:1 as well. Giving a little more vertical view.

Under 1:1 things are still quite broken but a lot needs changing in the view handling to properly support portrait mode.

xezon · 2025-10-19T11:21:40Z

Yes GenTool did take some vertical view away to compensate for the wider view, perhaps in an attempt to keep conditions fair. Additional camera height does give a competitive advantage. Are we ok with giving advantages to Wide Screen?

Mauller · 2025-10-19T11:31:30Z

I wouldn't consider it an issue since most screens have been wide aspect For the past decade. If anything widescreen is the standard now.

Ultrawide is where it becomes more of an issue. But we could look at implementing a locked aspect mode for the tactical view in future.
This could give a standardised view to all players in a competition etc.

Or another option could be implementing tactical zoom to let people zoom out to view the entire map etc.

xezon

Simple implementation, but man the EA code is confusing.

Mauller · 2025-10-19T19:11:49Z

Tweaked the code so setDefaultView is now setDefaultCameraHeight with a default argument of 1.0f for the heightScale.

Still need to look at the zoom default handling.

Mauller · 2025-10-19T19:38:17Z

Updated with tweaked max and default zoom handling code.

Mauller · 2025-10-19T19:56:57Z

Small tweak to comments

xezon · 2025-11-15T15:01:30Z

Note for future:

When raising up the camera, the following things need to be scaled as well:

farZ in W3DView::setCameraTransform

m_x, m_y in HeightMapRenderObjClass

greptile-apps · 2026-01-16T16:54:34Z

Greptile Summary

This PR introduces aspect-ratio-aware camera height scaling by adding setCameraHeightAboveGroundLimitsToDefault() and splitting zoom initialization into a separate setZoomToMax(), replacing the existing setDefaultView() calls at key initialization sites (InGameUI, GameLogic, OptionsMenu).

The approach is sound, but several previously-flagged issues remain open (base-class setZoomToMax() accumulates height instead of setting it, setZoomToDefault() no longer restores zoom, and InGameUI::init/reset lack a setZoomToMax() call). One new finding: the if/else branches inside setCameraHeightAboveGroundLimitsToDefault() are algebraically identical and can be collapsed to a single expression.

Confidence Score: 3/5

Not safe to merge — multiple P1 issues flagged in prior rounds remain unresolved in the diff

Several P1 defects visible in the current diff are documented in earlier review rounds but not yet fixed: View::setZoomToMax() adds to height instead of setting it, setZoomToDefault() no longer sets zoom, and InGameUI::init/reset omit setZoomToMax(). These constitute multiple unresolved P1s, pulling the score below the 4/5 P1 ceiling.

Core/GameEngine/Source/GameClient/View.cpp (incorrect setZoomToMax base impl), GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp (missing setZoomToMax), Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp (redundant if/else in aspect ratio calc)

Important Files Changed

Filename	Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp	Adds setCameraHeightAboveGroundLimitsToDefault() (with redundant if/else) and splits setZoomToMax() out of setZoomToDefault(); setZoomToDefault() now only stops scripted cameras without restoring zoom height
Core/GameEngine/Source/GameClient/View.cpp	Base-class setZoomToMax() incorrectly adds m_maxHeightAboveGround to current height instead of setting it; W3DView overrides this correctly so the bug is latent but may affect other View subclasses
GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp	Replaced setDefaultView() with setCameraHeightAboveGroundLimitsToDefault() in init() and reset(), but neither call is followed by setZoomToMax(), leaving camera height unset at those sites
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Correctly adds setCameraHeightAboveGroundLimitsToDefault() and setZoomToMax() around the existing default-camera reset sequence at game start
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp	Correctly calls setCameraHeightAboveGroundLimitsToDefault() then setZoomToMax() when options (resolution) are saved, keeping the shell map camera in sync
Core/GameEngine/Include/GameClient/View.h	Adds virtual setCameraHeightAboveGroundLimitsToDefault() and setZoomToMax() declarations; setZoomToMax() base implementation in View.cpp is incorrect
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DView.h	Adds overrides for setCameraHeightAboveGroundLimitsToDefault() and setZoomToMax(); informational comment added to setZoomToDefault()
GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp	doCameraSetDefault now calls setCameraHeightAboveGroundLimitsToDefault/setPitch/setAngle; parameter renamed to heighScale (typo) and setPitch/setAngle side-effect stopping scripted cameras noted in prior review
GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h	Minor: adds clarifying comment for m_maxCameraHeight/m_minCameraHeight, no logic changes
GeneralsMD/Code/GameEngine/Include/GameLogic/ScriptActions.h	Parameter renamed to heighScale (typo for heightScale) matching the .cpp change

Sequence Diagram

sequenceDiagram
    participant GL as GameLogic::tryStartNewGame
    participant OM as OptionsMenu::saveOptions
    participant UI as InGameUI::init / reset
    participant TV as TheTacticalView (W3DView)

    Note over GL: Game start sequence
    GL->>TV: setCameraHeightAboveGroundLimitsToDefault()
    GL->>TV: setAngleToDefault()
    GL->>TV: setPitchToDefault()
    GL->>TV: setZoomToDefault()
    GL->>TV: setZoomToMax()

    Note over OM: Resolution change
    OM->>TV: setCameraHeightAboveGroundLimitsToDefault()
    OM->>TV: setZoomToMax()

    Note over UI: Init / Reset (missing setZoomToMax)
    UI->>TV: setCameraHeightAboveGroundLimitsToDefault()
    Note right of TV: setZoomToMax() not called here

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp:2191-2198
**Redundant if/else — both branches compute identical values**

The `if` and `else` branches are algebraically identical:
- `if`: `fabs(1 + (currentAspectRatio - baseAspectRatio))`
- `else`: `fabs(1 - (baseAspectRatio - currentAspectRatio))`

Both simplify to `fabs(1 + currentAspectRatio - baseAspectRatio)`, so the conditional adds no value. The entire block can be replaced with one expression:

```cpp
aspectRatioScale = 1.0f + (currentAspectRatio - baseAspectRatio);
```

(For any standard landscape display `currentAspectRatio ≥ 0.333`, the result is always positive, so `fabs` is also not needed.)

_{Reviews (15): Last reviewed commit: "fix(view): Adjust default camera height ..." | Re-trigger Greptile}

greptile-apps

_{10 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-16T16:54:33Z

 /** doCameraSetDefault */
 //-------------------------------------------------------------------------------------------------
-void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real maxHeight)
+void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heighScale)


syntax: typo: heighScale should be heightScale

Suggested change

void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heighScale)

void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heightScale)

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp Line: 4611:4611 Comment: **syntax:** typo: `heighScale` should be `heightScale` ```suggestion void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heightScale) ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-01-16T16:54:34Z

 	void doCameraTetherNamed(const AsciiString& unit, Bool snapToUnit, Real play);
 	void doCameraStopTetherNamed(void);
-	void doCameraSetDefault(Real pitch, Real angle, Real maxHeight);
+	void doCameraSetDefault(Real pitch, Real angle, Real heighScale);


syntax: typo: heighScale should be heightScale

Suggested change

void doCameraSetDefault(Real pitch, Real angle, Real heighScale);

void doCameraSetDefault(Real pitch, Real angle, Real heightScale);

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngine/Include/GameLogic/ScriptActions.h Line: 114:114 Comment: **syntax:** typo: `heighScale` should be `heightScale` ```suggestion void doCameraSetDefault(Real pitch, Real angle, Real heightScale); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps

_{10 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

_{10 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-12T18:01:39Z

 		TheTacticalView->setHeight( TheDisplay->getHeight() );
 	}
-	TheTacticalView->setDefaultView(0.0f, 0.0f, 1.0f);
+	TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();


Missing setZoomToMax() call after setCameraHeightAboveGroundLimitsToDefault(). Similar to GameLogic::startNewGame() and OptionsMenu::saveOptions(), this should call setZoomToMax() to actually set the camera height.

Suggested change

TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();

TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();

TheTacticalView->setZoomToMax();

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp Line: 1380:1380 Comment: Missing `setZoomToMax()` call after `setCameraHeightAboveGroundLimitsToDefault()`. Similar to `GameLogic::startNewGame()` and `OptionsMenu::saveOptions()`, this should call `setZoomToMax()` to actually set the camera height. ```suggestion TheTacticalView->setCameraHeightAboveGroundLimitsToDefault(); TheTacticalView->setZoomToMax(); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-12T18:01:40Z

 	TheControlBar->reset();

-	TheTacticalView->setDefaultView(0.0f, 0.0f, 1.0f);
+	TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();


Missing setZoomToMax() call after setCameraHeightAboveGroundLimitsToDefault(). Without this, the camera zoom won't be properly initialized during reset.

Suggested change

TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();

TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();

TheTacticalView->setZoomToMax();

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp Line: 2150:2150 Comment: Missing `setZoomToMax()` call after `setCameraHeightAboveGroundLimitsToDefault()`. Without this, the camera zoom won't be properly initialized during reset. ```suggestion TheTacticalView->setCameraHeightAboveGroundLimitsToDefault(); TheTacticalView->setZoomToMax(); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-03-06T17:07:47Z

+	TheTacticalView->setPitch(pitch);
+	TheTacticalView->setAngle(angle);


setPitch/setAngle unexpectedly stop scripted cameras

Both W3DView::setPitch and W3DView::setAngle call stopDoingScriptedCamera() as a side effect. The original setDefaultView only updated m_defaultPitch and m_maxHeightAboveGround without touching scripted camera state at all.

This means that any call to doCameraSetDefault from a script while a scripted camera animation is running will silently terminate that animation — a behavioural change not present before this PR. The function name implies updating defaults, not interrupting live camera paths.

Additionally, the old code stored the pitch in m_defaultPitch, so a later setPitchToDefault() would correctly restore it. The new setPitch() call only writes to m_pitch (the current pitch), leaving m_defaultPitch unchanged. If the engine resets to default pitch elsewhere, the value set by this script action will be lost.

Consider using a method that updates the defaults without calling stopDoingScriptedCamera(), or explicitly documenting the intended behavioural change if stopping the scripted camera is desired.

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp Line: 4607-4608 Comment: **`setPitch`/`setAngle` unexpectedly stop scripted cameras** Both `W3DView::setPitch` and `W3DView::setAngle` call `stopDoingScriptedCamera()` as a side effect. The original `setDefaultView` only updated `m_defaultPitch` and `m_maxHeightAboveGround` without touching scripted camera state at all. This means that any call to `doCameraSetDefault` from a script while a scripted camera animation is running will silently terminate that animation — a behavioural change not present before this PR. The function name implies updating defaults, not interrupting live camera paths. Additionally, the old code stored the pitch in `m_defaultPitch`, so a later `setPitchToDefault()` would correctly restore it. The new `setPitch()` call only writes to `m_pitch` (the *current* pitch), leaving `m_defaultPitch` unchanged. If the engine resets to default pitch elsewhere, the value set by this script action will be lost. Consider using a method that updates the defaults without calling `stopDoingScriptedCamera()`, or explicitly documenting the intended behavioural change if stopping the scripted camera is desired. How can I resolve this? If you propose a fix, please make it concise.

Mauller · 2026-03-10T13:29:42Z

This is only updating at times since i use this PR for the Legi build

seacat17 · 2026-04-07T19:02:00Z

How far are we from having this in the main build? I want to try all the new things...

…ct ratio

Mauller self-assigned this Oct 18, 2025

Mauller added Bug Something is not working right, typically is user facing GUI For graphical user interface Major Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour labels Oct 18, 2025

Mauller added this to the Major bug fixes milestone Oct 18, 2025

Skyaero42 reviewed Oct 18, 2025

View reviewed changes

Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp Outdated

xezon added Enhancement Is new feature or request and removed GUI For graphical user interface labels Oct 19, 2025

Mauller force-pushed the fix-default-camera-height branch from 02a64e0 to 57fe78d Compare October 19, 2025 15:37

xezon reviewed Oct 19, 2025

View reviewed changes

Mauller force-pushed the fix-default-camera-height branch from 57fe78d to 271ca5b Compare October 19, 2025 19:09

Mauller force-pushed the fix-default-camera-height branch from 271ca5b to f6c96d8 Compare October 19, 2025 19:36

Mauller force-pushed the fix-default-camera-height branch from f6c96d8 to c3c7b85 Compare October 19, 2025 19:56

L3-M modified the milestones: Major bug fixes, GenTool features replication, Important features Oct 20, 2025

Mauller force-pushed the fix-default-camera-height branch 3 times, most recently from 968ad71 to e497ba8 Compare November 7, 2025 16:15

Skyaero42 mentioned this pull request Nov 7, 2025

[ZH] Adjust field of view based on aspect ratio #518

Closed

Mauller force-pushed the fix-default-camera-height branch from e497ba8 to a0b7db7 Compare January 16, 2026 16:48

greptile-apps Bot reviewed Jan 16, 2026

View reviewed changes

Mauller force-pushed the fix-default-camera-height branch from a0b7db7 to 22e6c6b Compare January 28, 2026 18:25

Mauller force-pushed the fix-default-camera-height branch from 22e6c6b to 6ea3918 Compare February 9, 2026 17:57

greptile-apps Bot reviewed Feb 9, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp

Mauller force-pushed the fix-default-camera-height branch from 6ea3918 to 09d7a25 Compare February 12, 2026 17:58

greptile-apps Bot reviewed Feb 12, 2026

View reviewed changes

Mauller force-pushed the fix-default-camera-height branch 2 times, most recently from b07ab06 to 499d6ef Compare February 13, 2026 14:53

ElTioRata mentioned this pull request Feb 21, 2026

Camera height does not scale properly along with aspect ratio of game resolution #78

Open

Mauller force-pushed the fix-default-camera-height branch from 499d6ef to 27aa725 Compare March 6, 2026 17:02

greptile-apps Bot reviewed Mar 6, 2026

View reviewed changes

Mauller force-pushed the fix-default-camera-height branch from 27aa725 to f0dc49b Compare March 22, 2026 08:39

fbraz3 mentioned this pull request Mar 24, 2026

fix(view): integrate PR1711 camera defaults and backport to Generals fbraz3/GeneralsX#47

Merged

Mauller force-pushed the fix-default-camera-height branch from f0dc49b to f5167b0 Compare April 6, 2026 13:20

Mauller force-pushed the fix-default-camera-height branch 3 times, most recently from 5617740 to a076193 Compare April 11, 2026 19:00

Mauller force-pushed the fix-default-camera-height branch from a076193 to 2523ec0 Compare April 20, 2026 15:46

Mauller force-pushed the fix-default-camera-height branch from 2523ec0 to bb8c588 Compare April 29, 2026 16:34

fix(view): Adjust default camera height to compensate for screen aspe…

ec0c082

…ct ratio

Mauller force-pushed the fix-default-camera-height branch from bb8c588 to ec0c082 Compare May 4, 2026 09:15

	void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heighScale)
	void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heightScale)

	void doCameraSetDefault(Real pitch, Real angle, Real heighScale);
	void doCameraSetDefault(Real pitch, Real angle, Real heightScale);

	TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();
	TheTacticalView->setCameraHeightAboveGroundLimitsToDefault();
	TheTacticalView->setZoomToMax();

		TheTacticalView->setPitch(pitch);
		TheTacticalView->setAngle(angle);

Conversation

Mauller commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ElTioRata commented Oct 18, 2025

Uh oh!

Uh oh!

xezon commented Oct 19, 2025

Uh oh!

Mauller commented Oct 19, 2025

Uh oh!

xezon commented Oct 19, 2025

Uh oh!

Mauller commented Oct 19, 2025

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mauller commented Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mauller commented Oct 19, 2025

Uh oh!

Mauller commented Oct 19, 2025

Uh oh!

xezon commented Nov 15, 2025

Uh oh!

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

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Mauller commented Mar 10, 2026

Uh oh!

seacat17 commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Mauller commented Oct 18, 2025 •

edited

Loading

Mauller commented Oct 19, 2025 •

edited

Loading

greptile-apps Bot commented Jan 16, 2026 •

edited

Loading