Skip to content

fix(view): Introduce PRESERVE_RETAIL_SCRIPTED_CAMERA and fix or improve camera offset and zoom calculations#2524

Open
xezon wants to merge 1 commit intoTheSuperHackers:mainfrom
xezon:xezon/fix-camera-offset
Open

fix(view): Introduce PRESERVE_RETAIL_SCRIPTED_CAMERA and fix or improve camera offset and zoom calculations#2524
xezon wants to merge 1 commit intoTheSuperHackers:mainfrom
xezon:xezon/fix-camera-offset

Conversation

@xezon
Copy link
Copy Markdown

@xezon xezon commented Apr 2, 2026

This change is quite a bit complicated.

PRESERVE_RETAIL_SCRIPTED_CAMERA

Introduces a new PRESERVE_RETAIL_SCRIPTED_CAMERA preprocessor macro to preserve the original look of the scripted camera. Most importantly the initial ground level is cached and used during scripted camera movements.

Camera Offset

The camera offset is now freshly calculated inside W3DView::buildCameraPosition, instead of various other locations, to make sure that it is always correctly built when the camera pitch or angle changes.

Camera Zoom

The camera zoom was changed from originally

initialGroundLevel + TheGlobalData->m_cameraHeight

to

currentGroundLevel + TheGlobalData->m_maxCameraHeight

This effectively means that the camera zoom range is now 0 to 1. Originally it was 0 to N, where N would generally be 1.33 or higher. This was confusing. The new setup gives a much better expectation for what a zoom value represents. 0.5 now means half way zoomed from max possible height to ground.

Known issues

  • Viewing Bookmark position twice over elevation change looks buggy
  • Replay Camera looks buggy on elevation changes
  • Replay Camera zoom from legacy Single Player replays will no longer represent the height correctly (will not be fixed because it is too complicated to maintain this level of backwards compatibility)

TODO

  • Replicate in Generals

@xezon xezon added this to the Camera Improvements milestone Apr 2, 2026
@xezon xezon added Enhancement Is new feature or request Major 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 2, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 2, 2026

Greptile Summary

This PR refactors camera positioning logic in W3DView by introducing the PRESERVE_RETAIL_SCRIPTED_CAMERA macro, removing the stale m_cameraOffset member, and consolidating offset and zoom calculations into five new private helper methods (getCameraOffsetZ, getDesiredHeight, getMaxHeight, getDesiredZoom, getMaxZoom). The zoom range is normalized to 0–1 (where 1 = max height above ground), replacing the previous confusing non-normalized scale.

  • PRESERVE_RETAIL_SCRIPTED_CAMERA macro is added to GameDefines.h (defaulting to 1) and guards the cached m_initialGroundLevel field, the m_cameraHeight INI field, and the scripted-path branches inside the new helpers.
  • Camera offset is now computed fresh inside buildCameraPosition on every call, eliminating the three separate sites in W3DView(), initHeightForMap(), and moveAlongWaypointPath() that previously maintained m_cameraOffset (often with the .z update already commented out).
  • Camera zoom denominator changes from initialGroundLevel + m_cameraHeight (for scripted) or groundLevel + m_cameraHeight (for user) to groundLevel + m_maxCameraHeight, making zoom 0..1 represent the full range from ground to max height.
  • setUserControlled now remaps m_zoom across the scripted/user boundary to avoid a visible jump when handing camera control back to the player.
  • Two minor style issues were found: newly introduced comment annotations on lines 649 and 666 carry dates in 2025 (prior to the current year), and constexpr const on line 2509 carries a redundant const.

Confidence Score: 4/5

Safe to merge; the refactor is well-structured and the two flagged items are minor style issues only.

The logic is carefully guarded behind the PRESERVE_RETAIL_SCRIPTED_CAMERA macro, existing behaviors are preserved or intentionally changed with clear rationale, and the helper methods centralise previously duplicated (and partially broken) camera offset math. The only deductions are for two outdated year references in comments and a redundant const keyword — neither affects runtime behavior.

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp — check the two date-annotated comments (lines 649, 666) and the constexpr const on line 2509.

Important Files Changed

Filename Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp Major refactor: removes m_cameraOffset member, introduces getCameraOffsetZ/getDesiredHeight/getMaxHeight/getDesiredZoom/getMaxZoom helpers, recalculates camera offset inline in buildCameraPosition, and adds PRESERVE_RETAIL_SCRIPTED_CAMERA guards throughout. Minor issues: two new comments reference 2025 dates, and a constexpr const redundancy.
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DView.h Removes m_cameraOffset field, adds m_initialGroundLevel under PRESERVE_RETAIL_SCRIPTED_CAMERA guard, and declares the five new private helper methods. Clean change.
Core/GameEngine/Include/Common/GameDefines.h Adds the PRESERVE_RETAIL_SCRIPTED_CAMERA preprocessor macro with a default value of 1, following the same pattern as PRESERVE_RETAIL_BEHAVIOR. No issues.
GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h Guards m_cameraHeight field behind PRESERVE_RETAIL_SCRIPTED_CAMERA, correctly pairing with the new macro. No issues.
GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp Guards the CameraHeight INI parser entry and its default initializer behind PRESERVE_RETAIL_SCRIPTED_CAMERA. Column alignment in the INI parse table is preserved. No issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    BCP["buildCameraPosition()"]
    GCZ["getCameraOffsetZ()"]
    PRSC{PRESERVE_RETAIL_SCRIPTED_CAMERA
&& !m_isUserControlled}
    SCRIPTED["m_initialGroundLevel + m_cameraHeight"]
    USER["m_groundLevel + m_maxCameraHeight"]
    SRCPOS["sourcePos.Z = cameraOffsetZ\nsourcePos.Y = -(Z / tan(pitch))\nsourcePos.X = -(Y * tan(yaw))"]
    GDZ["getDesiredZoom(x,y)"]
    ZLIMIT{"isZoomLimited()
&& !isUserControlLocked()"}
    KEEPZ["return m_zoom (unchanged)"]
    GDH["getDesiredHeight(x,y)"]
    DIVZ["desiredHeight / getCameraOffsetZ()"]
    GMZ["getMaxZoom(x,y)"]
    GMH["getMaxHeight(x,y)"]
    DIVM["maxHeight / getCameraOffsetZ()"]
    SUC["setUserControlled(value)"]
    REMAP["m_zoom = getDesiredZoom(pos.x, pos.y)\n(remap zoom across scripted/user boundary)"]
    BCP --> GCZ
    GCZ --> PRSC
    PRSC -->|yes| SCRIPTED
    PRSC -->|no| USER
    SCRIPTED --> SRCPOS
    USER --> SRCPOS
    GDZ --> ZLIMIT
    ZLIMIT -->|early return| KEEPZ
    ZLIMIT -->|compute| GDH
    GDH --> DIVZ
    DIVZ --> GCZ
    GMZ --> GMH
    GMH --> DIVM
    DIVM --> GCZ
    SUC -->|PRESERVE_RETAIL_SCRIPTED_CAMERA| REMAP
    REMAP --> GDZ
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Line: 649-666

Comment:
**Date references prior to current year (2026)**

Two newly introduced comment annotations reference dates in 2025, which is prior to the current year (2026). Per the project's policy, newly created code comments should reference the current year.

Line 649:
```suggestion
	// TheSuperHackers @info xezon 04/12/2026 It is necessary to use the initial ground level for the
```

Line 666:
```suggestion
	// TheSuperHackers @info xezon 06/12/2026 The height above ground must be relative to the current
```

**Rule Used:** What: Flag newly created code comments that refere... ([source](https://app.greptile.com/review/custom-context?memory=fd72a556-4fd8-4db4-8b08-8e51516a64ad))

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/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Line: 2509

Comment:
**`constexpr const` is redundant**

`constexpr` already implies `const` in C++, so `const` here is redundant.

```suggestion
	constexpr Real MAX_GROUND_LEVEL = 120.0f;
```

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

Reviews (1): Last reviewed commit: "fix(view): Move camera offset calculatio..." | Re-trigger Greptile

Copy link
Copy Markdown

@Skyaero42 Skyaero42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Is new feature or request Fix Is fixing something, but is not user facing Gen Relates to Generals Major Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants