bugfix(view): Fix ground level of bookmarks, replay camera and game world microphone#2595
Conversation
|
Generals will fail to compile until replicated. |
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Include/GameClient/View.h | Introduces setPosition2D, getPosition2D, resetPivotToGround, and userResetPivotToGround; removes m_groundLevel field, consolidating ground-level semantics into m_pos.z. Clean implementation. |
| Core/GameEngine/Source/GameClient/View.cpp | View::lookAt now uses setPosition2D to leave m_pos.z (ground level) unchanged; xfer serializes m_pos.z but lookAt immediately recalculates it via resetPivotToGround on load, so save compatibility is preserved. |
| Core/GameEngine/Source/Common/Audio/GameAudio.cpp | Microphone placement now correctly derives desiredHeightAbs from cameraPivot.z (terrain ground level via m_pos.z) instead of calling getGroundHeight separately — a clean simplification. |
| Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp | Most changes are correct: lookAt now calls resetPivotToGround, buildCameraTransform (slave mode) uses setPosition2D, and new movePivotToGround / resetPivotToGround implementations are solid. One regression: line 1430 sets m_pos.z = objpos.z (camera-lock follow), which was harmless when m_pos.z was unused but now corrupts ground level for airborne locked objects. |
| Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DView.h | Adds movePivotToGround() declaration, removes m_groundLevel field declaration, and retains m_initialGroundLevel for the PRESERVE_RETAIL_SCRIPTED_CAMERA path. No issues. |
| Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp | Debug display now shows lookPos.z (terrain ground level) instead of the old ground level accessor, consistent with the new semantics. No issues. |
| Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp | Removes TMoveAlongWaypointPathInfo::groundHeight usage and replaces with lookAt-based camera positioning; replicated cleanly from the core changes. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/ScriptActions.cpp | Same changes as Generals/ScriptActions.cpp replicated for Zero Hour, consistent and correct. |
| Generals/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp | Minor updates to remove ground height references and adapt to the new View position API; no issues found. |
| GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp | Zero Hour counterpart to the Generals CommandXlat.cpp changes; correctly replicated. |
| Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp | Removes now-unnecessary getGroundHeight call that was compensating for the missing ground level in view position; the fix is consistent with the consolidated approach. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["View::m_pos (x, y, z)"] -->|"z = terrain ground level (NEW)"| B["AudioManager::update()"]
A -->|"z used in getDesiredHeight()"| C["W3DView camera transform"]
A -->|"z saved/restored via xfer"| D["Save/Load system"]
E["W3DView::lookAt()"] -->|"setPosition2D (x,y only)"| A
E -->|"calls"| F["W3DView::resetPivotToGround()"]
F -->|"m_pos.z = getHeightAroundPos()"| A
G["buildCameraTransform (slave mode)"] -->|"setPosition2D (x,y only)"| A
H["Camera-lock follow (stepView)"] -->|"⚠ m_pos.z = objpos.z (line 1430)"| A
B -->|"cameraPivot.z = terrain height (or aerial if locked)"| I["Microphone placement"]
C --> J["Camera eye position"]
style H fill:#f88,stroke:#c00
style I fill:#ffd,stroke:#aa0
Prompt To Fix All With AI
This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Line: 1430
Comment:
**Airborne camera-lock sets `m_pos.z` to aerial altitude, not terrain height**
Before this PR, `m_pos.z` was unused and a separate `m_groundLevel` held the terrain height. This line was therefore a no-op for ground-level semantics. Now that `m_pos.z` *is* the ground level, assigning `objpos.z` here corrupts it for airborne camera-locked objects (planes, helicopters): `m_pos.z` ends up at their flying altitude instead of terrain height.
This directly flows into `AudioManager::update()` where `cameraPivot = TheTacticalView->getPosition()`, so `cameraPivot.z` equals the flying altitude. The `groundToCameraVector.z` becomes very small, and `bestScaleFactor = maxPercentage` always applies, placing the audio microphone at the aerial pivot position rather than near the terrain.
Use terrain height instead, consistent with how `resetPivotToGround` computes it:
```suggestion
m_pos.z = TheTerrainLogic->getGroundHeight(curpos.x, curpos.y);
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (7): Last reviewed commit: "Fix ground level and zoom in W3DView::lo..." | Re-trigger Greptile
…orld microphone By merging View::m_groundLevel into View::m_pos::z
3aa88fb to
e5ff233
Compare
Mauller
left a comment
There was a problem hiding this comment.
Looks good, although what greptile brought up might want checking. But i don't see how it changes behaviour.
|
There are cutscene discrepancies in Generals that need looking into. |
8a416cf to
e4d85b4
Compare
| m_snapImmediate = FALSE; | ||
|
|
||
| m_groundLevel = objpos.z; | ||
| m_pos.z = objpos.z; |
There was a problem hiding this comment.
Airborne camera-lock sets
m_pos.z to aerial altitude, not terrain height
Before this PR, m_pos.z was unused and a separate m_groundLevel held the terrain height. This line was therefore a no-op for ground-level semantics. Now that m_pos.z is the ground level, assigning objpos.z here corrupts it for airborne camera-locked objects (planes, helicopters): m_pos.z ends up at their flying altitude instead of terrain height.
This directly flows into AudioManager::update() where cameraPivot = TheTacticalView->getPosition(), so cameraPivot.z equals the flying altitude. The groundToCameraVector.z becomes very small, and bestScaleFactor = maxPercentage always applies, placing the audio microphone at the aerial pivot position rather than near the terrain.
Use terrain height instead, consistent with how resetPivotToGround computes it:
| m_pos.z = objpos.z; | |
| m_pos.z = TheTerrainLogic->getGroundHeight(curpos.x, curpos.y); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Line: 1430
Comment:
**Airborne camera-lock sets `m_pos.z` to aerial altitude, not terrain height**
Before this PR, `m_pos.z` was unused and a separate `m_groundLevel` held the terrain height. This line was therefore a no-op for ground-level semantics. Now that `m_pos.z` *is* the ground level, assigning `objpos.z` here corrupts it for airborne camera-locked objects (planes, helicopters): `m_pos.z` ends up at their flying altitude instead of terrain height.
This directly flows into `AudioManager::update()` where `cameraPivot = TheTacticalView->getPosition()`, so `cameraPivot.z` equals the flying altitude. The `groundToCameraVector.z` becomes very small, and `bestScaleFactor = maxPercentage` always applies, placing the audio microphone at the aerial pivot position rather than near the terrain.
Use terrain height instead, consistent with how `resetPivotToGround` computes it:
```suggestion
m_pos.z = TheTerrainLogic->getGroundHeight(curpos.x, curpos.y);
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
This exactly replicates what the original did. I see no mistake.
|
I have appended new fixes for the scripted camera in regards to W3DView::lookAt. It now looks correct in Generals compaign cutscene. |
|
Tested a few more cinematic scenes in Generals and it looked fine. |
This change fixes the ground level of bookmarks, replay camera and the game world microphone by merging
View::m_groundLevelintoView::m_pos::z.View::m_pos::zwas originally unused andView::m_groundLevelwas effectively that. They are now consolidated which automatically adds the ground level into bookmarks and the replay camera throughViewLocation(cool).The game world microphone, used for positional audio, did not calculate the microphone height correctly which was fixed as well.
In
View::lookAtthe pivot is now reset to the ground which corrects behavior as well.TODO