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 into
Conversation
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.
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 |
| /** 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
|
This is only updating at times since i use this PR for the Legi build |
27aa725 to
f0dc49b
Compare
f0dc49b to
f5167b0
Compare
|
How far are we from having this in the main build? I want to try all the new things... |
5617740 to
a076193
Compare
a076193 to
2523ec0
Compare
bb8c588 to
ec0c082
Compare
|
I think we can go ahead with this now. Can you rebase it? |
Sure, i will rebase and take a look back over the implementation to see if theres any tweaks i can make after your recent changes. |
ec0c082 to
baa303e
Compare
|
Just a raw rebase on current main of the original implementation, still need to look back over it, make any suggestions. I think i can move a lot of what i seperated out back into |
baa303e to
0813a5b
Compare
|
The scripted camera is too high. In my opinion the height difference compared to GenTool is too much - around 3.6% difference. Especially with Control Bar Pro it will give a significant bump is viewable terrain. For starters, I suggest to set it up exactly like GenTool did. GenTool nerfs the height by this much: |
| // m_defaultAngle = angle * M_PI/180.0f; | ||
| setDefaultPitch(pitch); | ||
| m_maxHeightAboveGround = TheGlobalData->m_maxCameraHeight*maxHeight; | ||
| m_maxHeightAboveGround = TheGlobalData->m_maxCameraHeight * aspectRatioScale * maxHeight; |
There was a problem hiding this comment.
I expect this implementation is not correct. m_maxCameraHeight is also used in getCameraOffsetZ.


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