fix(view): Change setup of default camera pitch and angle by xezon · Pull Request #2546 · TheSuperHackers/GeneralsGameCode

xezon · 2026-04-07T21:49:33Z

This change decouples the default camera pitch and angle from the camera origin point. The default pitch can be changed with a debug key mapping.

Previously, the user pitch started at 0 which actually refers to a real pitch of 37.5 because that is what TheGlobalData->m_cameraPitch is set to.

TODO

Replicate in Generals

xezon · 2026-04-07T21:51:10Z

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp

+	// TheSuperHackers @tweak To preserve the original scripted camera values, offset them by default ones.
+	constexpr const Real DefaultPitch = DEG_TO_RADF(37.5f);
+	constexpr const Real DefaultAngle = DEG_TO_RADF(0.0f);
+	pitch = -pitch + DefaultPitch;


-pitch because we changed the axis direction of pitch in a earlier change.

I did not get to test this function however, because I did not find a cutscene where this is called.

greptile-apps · 2026-04-07T21:53:17Z

Greptile Summary

This PR decouples the default camera pitch and angle from the coordinate-space origin by introducing ViewDefaultPitchRadians / ViewDefaultYawRadians constants, initialising m_defaultPitch / m_defaultAngle from TheGlobalData rather than zero, and adjusting the rotation matrices in buildCameraPosition to treat stored pitch/angle values as absolute rather than relative-to-origin. A debug key binding (Ctrl+Comma) is added to interactively tune the default pitch at runtime. The previously flagged issues (duplicate constant definition, undefined DefaultAngle/DefaultPitch identifiers, and wrong constant on the angle offset in ScriptActions) appear to have been resolved.

Confidence Score: 5/5

Safe to merge; all previous blocking issues are resolved and remaining findings are minor style concerns.

No P0 or P1 issues remain. The only findings are a hard-coded #define that leaves dead #else branches and a now-redundant m_angle = 0.0f assignment — both P2 style items that do not affect runtime correctness.

Core/GameEngine/Source/GameClient/View.cpp — dead #else branches from CLAMP_VIEW_PITCH define and redundant zero-init of m_angle.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameClient/View.h	Adds ViewDefaultPitchRadians / ViewDefaultYawRadians constants and new getDefaultAngle, setDefaultPitch, getDefaultPitch, and userSetDefaultPitch API surface; no issues found.
Core/GameEngine/Source/GameClient/View.cpp	Initialises m_defaultAngle/m_defaultPitch from global data and syncs m_angle/m_pitch to them; clamp range for pitch widened from ±36° to [0.1°, 89.9°]. Hard-coded #define CLAMP_VIEW_PITCH 1 leaves #else branches as dead code, and the existing m_angle = 0.0f assignment on line 95 is now redundant.
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp	Replaces raw TheGlobalData->m_cameraPitch * (PI/180) computations with the named constants, subtracts the default pitch/angle from the rotation matrices so they represent deltas, adds setDefaultPitch override, and routes setDefaultView through the clamping accessor.
GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp	Both init() and reset() now pass the actual global pitch/yaw (in radians) to setDefaultView instead of 0.0f, matching the new absolute-value convention.
GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp	Under PRESERVE_RETAIL_SCRIPTED_CAMERA, offsets scripted pitch by ViewDefaultPitchRadians and angle by ViewDefaultYawRadians (0) to re-base script values onto the new absolute system.
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp	Adds debug-only MSG_META_DEMO_BEGIN/END_ADJUST_DEFAULTPITCH handling that adjusts default pitch with mouse drag and resets current pitch to default; mirrors existing isPitching pattern correctly.
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp	Registers Ctrl+Comma as the key binding for the new BEGIN/END_ADJUST_DEFAULTPITCH debug messages; consistent with existing debug key registration pattern.
GeneralsMD/Code/GameEngine/Include/GameClient/LookAtXlat.h	Adds m_isPitchingToDefault bool member, properly initialised and reset alongside peer members.
GeneralsMD/Code/GameEngine/Include/Common/MessageStream.h	Adds two new MSG_META_DEMO_* enum values for the default-pitch debug feature; no issues.
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DView.h	Adds setDefaultPitch virtual override declaration; clean.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    GD[TheGlobalData\nm_cameraPitch / m_cameraYaw]
    GD -->|DEG_TO_RADF| VI["View::init()\nm_defaultPitch = radians\nm_defaultAngle = radians\nm_pitch = m_defaultPitch\nm_angle = m_defaultAngle"]
    GD -->|DEG_TO_RADF| IGUI["InGameUI::init() / reset()\nsetDefaultView(pitch, angle, 1.0f)"]
    IGUI -->|pitch radians| SDP["W3DView::setDefaultPitch()\nView::setDefaultPitch()\nm_defaultPitch = clamp(0.1°, r, 89.9°)\nm_recalcCamera = true"]
    SDP -->|user drags Ctrl+Comma| DBG["LookAtXlat (DEBUG)\nuserSetDefaultPitch(delta)\nuserSetPitchToDefault()\n→ m_pitch = m_defaultPitch"]
    VI & SDP --> BCP["W3DView::buildCameraPosition()\nsourcePos based on ViewDefaultPitchRadians\npitchTransform = pitch - ViewDefaultPitchRadians\nangleTransform = angle - ViewDefaultYawRadians"]
    BCP --> CAM["Camera rendered at\ncorrect absolute pitch & angle"]

Prompt To Fix All With AI

This is a comment left during a code review.
Path: Core/GameEngine/Source/GameClient/View.cpp
Line: 165-184

Comment:
**Hard-coded macro renders `#else` branches unreachable**

`CLAMP_VIEW_PITCH` is defined as `1` unconditionally in a `.cpp` file, so the `#else` branches in both `setPitch` and `setDefaultPitch` are permanently dead code. Consider using a `constexpr bool` instead, or simply removing the conditional entirely if the clamping is always desired:

```suggestion
void View::setPitch( Real radians )
{
	m_pitch = clamp(DEG_TO_RADF(0.1f), radians, DEG_TO_RADF(89.9f));
}

void View::setDefaultPitch( Real radians )
{
	m_defaultPitch = clamp(DEG_TO_RADF(0.1f), radians, DEG_TO_RADF(89.9f));
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: Core/GameEngine/Source/GameClient/View.cpp
Line: 95

Comment:
**Redundant zero-initialization of `m_angle`**

`m_angle` is set to `0.0f` here and then overwritten to `m_defaultAngle` twelve lines later (line 107). The first assignment is now dead. You can remove this line or move the defaultAngle assignment up to replace it.

```suggestion
	m_angle = DEG_TO_RADF(TheGlobalData->m_cameraYaw);
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (6): Last reviewed commit: "Rename to yaw" | Re-trigger Greptile}

xezon · 2026-04-07T22:03:21Z

GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp

+		{
+			map->m_key = MK_COMMA;
+			map->m_transition = UP;
+			map->m_modState = CTRL;


This does not work reliably. When releasing CTRL before Comma, then this action will not trigger. Maybe can be fixed.

I believe this is common for all multi input controls at the moment.
The multi input sequence needs to be latched until the final input is cleared for a period.

I checked and there is no UP + Modifier mapping that is not MK_NONE. So it is not a practical issue in the original game, but will be with such new key mappings. I do not have an elegant idea how to address it.

I have implemented a fix proposal with #2577

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

Core/GameEngine/Source/GameClient/View.cpp

GeneralsMD/Code/GameEngine/Source/Common/MessageStream.cpp

GeneralsMD/Code/GameEngine/Include/GameClient/LookAtXlat.h

GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp

Skyaero42 · 2026-04-09T18:53:45Z

GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp

+#if defined(RTS_DEBUG)
+		case GameMessage::MSG_META_DEMO_END_ADJUST_DEFAULTPITCH:
+		{
+			DEBUG_ASSERTCRASH(m_isDefaultPitching, ("hmm, mismatched m_isDefaultPitching"));


Better debug message I think is:
"m_isDefaultPitching should be true on END, but was false"

Yes. I made it consistent with the nearby EA message :)

GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp

Skyaero42 · 2026-04-09T18:57:47Z

Core/GameEngine/Source/GameClient/View.cpp

+#if 1
+	m_pitch = clamp(DEG_TO_RADF(0.1f), radians, DEG_TO_RADF(89.9f));
+#else
+	m_pitch = WWMath::Normalize_Angle(radians);


Is the #if 1 and else code really needed here?

I kept it for when testing with unconstrained limits.

Maybe a define would be better for test use that can unconstrain it in both instances?

Or add a comment

Core/GameEngine/Include/GameClient/View.h

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

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp

Mauller · 2026-04-10T07:49:07Z

Core/GameEngine/Source/GameClient/View.cpp

-	constexpr Real limit = PI/5.0f;
-	m_pitch = clamp(-limit, radians, limit);
+#if 1
+	m_pitch = std::clamp(radians, DEG_TO_RADF(0.1f), DEG_TO_RADF(89.9f));


std::clamp is not c++98 compatible, might want to add an implementation for c++98 if we wanted to move away from the games implementation of clamp.

Oh right I forgot about that. It was only std::min, std::max

Mauller · 2026-04-10T07:51:27Z

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

-	sourcePos.Y = -(sourcePos.Z / tan(TheGlobalData->m_cameraPitch * (PI / 180.0)));
-	sourcePos.X = -(sourcePos.Y * tan(TheGlobalData->m_cameraYaw * (PI / 180.0)));
+	sourcePos.Y = -(sourcePos.Z / tan(ViewDefaultPitchRadians));
+	sourcePos.X = -(sourcePos.Y * tan(ViewDefaultAngleRadians));


I wonder if in another change we should switch angle to yaw in the naming.

Renamed to ViewDefaultYawRadians

fix(view): Change setup of default camera pitch and angle

efe08a0

xezon added this to the Camera Improvements milestone Apr 7, 2026

xezon added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Fix Is fixing something, but is not user facing labels Apr 7, 2026

xezon commented Apr 7, 2026

View reviewed changes

Revert init

9bd082e

xezon commented Apr 7, 2026

View reviewed changes

xezon commented Apr 8, 2026

View reviewed changes

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp Outdated Show resolved Hide resolved

xezon commented Apr 8, 2026

View reviewed changes

Core/GameEngine/Source/GameClient/View.cpp Show resolved Hide resolved

Mauller reviewed Apr 9, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Source/Common/MessageStream.cpp Show resolved Hide resolved

Mauller reviewed Apr 9, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Include/GameClient/LookAtXlat.h Outdated Show resolved Hide resolved

Skyaero42 reviewed Apr 9, 2026

View reviewed changes

Address review comments

ef12618

greptile-apps bot reviewed Apr 9, 2026

View reviewed changes

Core/GameEngine/Include/GameClient/View.h Outdated Show resolved Hide resolved

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp Outdated Show resolved Hide resolved

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp Outdated Show resolved Hide resolved

xezon added 2 commits April 9, 2026 23:44

Fix ViewDefaultAngleRadians

b74013d

Init angle and pitch in View

a4ced4e

Mauller reviewed Apr 10, 2026

View reviewed changes

xezon added 2 commits April 10, 2026 19:15

Fix clamp

58e067c

Rename to yaw

6b73359

xezon mentioned this pull request Apr 11, 2026

fix(metaevent): Ignore order in which modifier keys are released to trigger meta events #2577

Open

2 tasks

Conversation

xezon commented Apr 7, 2026

TODO

Uh oh!

xezon Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Apr 7, 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

Flowchart

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

Uh oh!

Mauller Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

3 participants

xezon Apr 7, 2026 •

edited

Loading

greptile-apps bot commented Apr 7, 2026 •

edited

Loading

Mauller Apr 10, 2026 •

edited

Loading