fix(view): Adjust default camera height to compensate for screen aspect ratio#1711
fix(view): Adjust default camera height to compensate for screen aspect ratio#1711Mauller wants to merge 1 commit intoTheSuperHackers:mainfrom
Conversation
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
Outdated
Show resolved
Hide resolved
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. |
|
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? |
|
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. Or another option could be implementing tactical zoom to let people zoom out to view the entire map etc. |
02a64e0 to
57fe78d
Compare
xezon
left a comment
There was a problem hiding this comment.
Simple implementation, but man the EA code is confusing.
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Outdated
Show resolved
Hide resolved
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Outdated
Show resolved
Hide resolved
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Outdated
Show resolved
Hide resolved
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Outdated
Show resolved
Hide resolved
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Outdated
Show resolved
Hide resolved
57fe78d to
271ca5b
Compare
|
Tweaked the code so Still need to look at the zoom default handling. |
271ca5b to
f6c96d8
Compare
|
Updated with tweaked max and default zoom handling code. |
f6c96d8 to
c3c7b85
Compare
|
Small tweak to comments |
|
Last one was a quick rebase with main |
c5fbb86 to
52f7de7
Compare
|
Tweaked naming, should be good now |
| m_drawHeightY=m_height; | ||
| } | ||
| else { | ||
| //TheSuperHacker @bugfix Mauller 23/10/2025 Increase the terrain draw area for wide aspect ratios |
There was a problem hiding this comment.
@tweak or @fix, because in original game this is not a bug, because the widescreen camera can not see that far.
| center.y -= bottom.y; | ||
|
|
||
| Real offset = center.length(); | ||
| // TheSuperHackers @bugfix Mauller 22/10/2025 Halve the camera offset to allow the camera to move closer to the map edges |
There was a problem hiding this comment.
removing this for now till after this and your camera extents PR's are done
| m_drawHeightY=m_height; | ||
| } | ||
| else { | ||
| //TheSuperHacker @bugfix Mauller 23/10/2025 Increase the terrain draw area for wide aspect ratios |
There was a problem hiding this comment.
I think this does not work correctly in Shell Map. When changing aspect ratio from 4:3 to wide, then this code is not called. Can we run this after the WorldHeightMap was created?
There was a problem hiding this comment.
yeah i am just looking a bit more into this and it's not very straightforwards, the heightmap is nested inside BaseHeightMap inside TheTerrainVisual within TheTerrainLogic.
Just muddling through a way to cleanly get at the height map inside it all from the terrain logic which has a singleton.
|
|
||
| Real offset = center.length(); | ||
| // TheSuperHackers @bugfix Mauller 22/10/2025 Halve the camera offset to allow the camera to move closer to the map edges | ||
| Real offset = center.length() * 0.5f; |
There was a problem hiding this comment.
I tested this behavior and I suspect this was done to address #1270. However, the black area that this creates is quite substantial. I have created new change #1752 to address #1270 without extending the constraints area into blackness.
There still is merit to experiment with a greater viewable area, for example center.length() * 0.9f, but I suggest to do that after #1752.
There was a problem hiding this comment.
i will revert the offset change for now and we can look at that after this and #1752 are merged.
52f7de7 to
ab0b316
Compare
|
Reverted this to just have the camera height adjustments and nothing else. |
968ad71 to
e497ba8
Compare
|
Note for future: When raising up the camera, the following things need to be scaled as well:
|
e497ba8 to
a0b7db7
Compare
|
| Filename | Overview |
|---|---|
| Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp | Core of the PR: adds setCameraHeightAboveGroundLimitsToDefault() for aspect-ratio-aware height limits and splits setZoomToMax() out of setZoomToDefault(). The aspect-ratio scale formula has redundant if/else branches that compute identical values. Pre-flagged issues remain: setZoomToDefault() no longer applies zoom, and fabs() wraps already-positive expressions. |
| Core/GameEngine/Source/GameClient/View.cpp | Adds the base-class setZoomToMax() stub, but the implementation (adding m_maxHeightAboveGround to the current height rather than setting it to max) is incorrect — pre-flagged and still unresolved. In practice the virtual dispatch routes to W3DView::setZoomToMax(), mitigating the impact. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp | Adds the correct call sequence for game start: setCameraHeightAboveGroundLimitsToDefault() → angle/pitch defaults → setZoomToDefault() → setZoomToMax(). No issues found here. |
| GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp | Both init() and reset() now call only setCameraHeightAboveGroundLimitsToDefault() without the companion setZoomToMax(). This leaves the camera zoom uninitialized relative to the new height limits — a pre-flagged, unresolved issue. |
| GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp | Correctly pairs setCameraHeightAboveGroundLimitsToDefault() with setZoomToMax() on resolution change, with an appropriate comment explaining why setZoomToDefault() is intentionally skipped to avoid stopping the shell map scripted camera. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp | Signature of doCameraSetDefault changed to use heighScale (typo, missing 't') and now calls setPitch/setAngle which have the side effect of stopping scripted cameras. Both issues were pre-flagged and remain unresolved. |
| GeneralsMD/Code/GameEngine/Include/GameLogic/ScriptActions.h | Declaration updated to use heighScale (typo, missing 't'). Pre-flagged and unresolved. |
| Core/GameEngine/Include/GameClient/View.h | Adds setCameraHeightAboveGroundLimitsToDefault() and setZoomToMax() as new virtual methods with correct no-op base implementations in the header. |
| Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DView.h | Declares the new setCameraHeightAboveGroundLimitsToDefault() and setZoomToMax() overrides. The updated setZoomToDefault() comment correctly warns that the function stops scripted cameras. |
| GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h | Adds a clarifying comment explaining that m_maxCameraHeight and m_minCameraHeight are 4:3 baseline values. No functional changes. |
Sequence Diagram
sequenceDiagram
participant GL as GameLogic::startNewGame
participant OM as OptionsMenu::saveOptions
participant UI as InGameUI::init/reset
participant TV as TheTacticalView (W3DView)
Note over GL: Game start path (correct)
GL->>TV: setCameraHeightAboveGroundLimitsToDefault()
GL->>TV: setAngleToDefault()
GL->>TV: setPitchToDefault()
GL->>TV: setZoomToDefault()
Note right of TV: stops scripted camera,<br/>but does NOT set zoom
GL->>TV: setZoomToMax()
Note right of TV: computes & applies zoom<br/>based on new height limits
Note over OM: Resolution change path (correct)
OM->>TV: setCameraHeightAboveGroundLimitsToDefault()
OM->>TV: setZoomToMax()
Note right of TV: skips setZoomToDefault()<br/>to preserve shell-map camera
Note over UI: InGameUI path (incomplete)
UI->>TV: setCameraHeightAboveGroundLimitsToDefault()
Note right of TV: setZoomToMax() is never called —<br/>zoom remains uninitialized
Prompt To Fix All With AI
This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Line: 1906-1913
Comment:
**Both if/else branches are mathematically identical**
The two branches of the conditional compute exactly the same value:
- **if-branch:** `fabs(1 + (currentAspectRatio - baseAspectRatio))`
- **else-branch:** `fabs(1 - (baseAspectRatio - currentAspectRatio))` = `fabs(1 - baseAspectRatio + currentAspectRatio)` = `fabs(1 + (currentAspectRatio - baseAspectRatio))`
The entire `if`/`else` block is redundant and can be collapsed into one expression:
```suggestion
Real aspectRatioScale = fabs(1.0f + (currentAspectRatio - baseAspectRatio));
```
As a side effect, this also fixes the misleading implication that the narrower-than-baseline case is handled differently.
How can I resolve this? If you propose a fix, please make it concise.Reviews (8): Last reviewed commit: "fix(view): Adjust default camera height ..." | Re-trigger Greptile
| /** doCameraSetDefault */ | ||
| //------------------------------------------------------------------------------------------------- | ||
| void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real maxHeight) | ||
| void ScriptActions::doCameraSetDefault(Real pitch, Real angle, Real heighScale) |
There was a problem hiding this comment.
syntax: typo: heighScale should be heightScale
| 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.| 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); |
There was a problem hiding this comment.
syntax: typo: heighScale should be heightScale
| 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.a0b7db7 to
22e6c6b
Compare
22e6c6b to
6ea3918
Compare
6ea3918 to
09d7a25
Compare
| TheTacticalView->setHeight( TheDisplay->getHeight() ); | ||
| } | ||
| TheTacticalView->setDefaultView(0.0f, 0.0f, 1.0f); | ||
| TheTacticalView->setCameraHeightAboveGroundLimitsToDefault(); |
There was a problem hiding this comment.
Missing setZoomToMax() call after setCameraHeightAboveGroundLimitsToDefault(). Similar to GameLogic::startNewGame() and OptionsMenu::saveOptions(), this should call setZoomToMax() to actually set the camera height.
| 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.| TheControlBar->reset(); | ||
|
|
||
| TheTacticalView->setDefaultView(0.0f, 0.0f, 1.0f); | ||
| TheTacticalView->setCameraHeightAboveGroundLimitsToDefault(); |
There was a problem hiding this comment.
Missing setZoomToMax() call after setCameraHeightAboveGroundLimitsToDefault(). Without this, the camera zoom won't be properly initialized during reset.
| 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.b07ab06 to
499d6ef
Compare
499d6ef to
27aa725
Compare
| TheTacticalView->setPitch(pitch); | ||
| TheTacticalView->setAngle(angle); |
There was a problem hiding this 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.
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.|
This is only updating at times since i use this PR for the Legi build |
27aa725 to
f0dc49b
Compare


Merge with Rebase
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