bugfix(view): Fix ground level of bookmarks, replay camera and game world microphone#2595
bugfix(view): Fix ground level of bookmarks, replay camera and game world microphone#2595xezon wants to merge 1 commit intoTheSuperHackers:mainfrom
Conversation
…orld microphone By merging View::m_groundLevel into View::m_pos::z
|
Generals will fail to compile until replicated. |
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Source/Common/Audio/GameAudio.cpp | Bug in microphone placement: zScale uses absolute desiredHeight instead of desiredHeight - cameraPivot.z, causing the microphone to be placed terrain_height units too high on elevated terrain when zScale < maxPercentage |
| Core/GameEngine/Source/GameClient/View.cpp | Serialization correctly XFers m_pos.z; lookAt now calls resetPivotToGround() after setPosition so old saves with z=0 are correctly corrected at load time |
| Core/GameEngine/Include/GameClient/View.h | Header adds resetPivotToGround virtual, userResetPivotToGround helper, and removes m_groundLevel — the public API change looks clean |
| Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp | Core refactor: m_pos.z now stores terrain ground level; movePivotToGround, resetPivotToGround, lookAt updated correctly; stale m_groundLevel comment at line 507 |
| Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DView.h | Adds m_initialGroundLevel under PRESERVE_RETAIL_SCRIPTED_CAMERA guard for backward compat with scripted cameras; declaration of resetPivotToGround added |
| Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp | No meaningful changes — only reads listener position set by AudioManager::update() |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["View::lookAt(pos)"] --> B["setPosition(pos)"]
B --> C["resetPivotToGround()"]
C --> D["m_pos.z = terrain height at x,y"]
D --> E["m_pos now = {x, y, ground_z}"]
E --> F["View::getLocation()"]
F --> G["ViewLocation stores {x, y, ground_z, angle, pitch, zoom}"]
G --> H["Bookmarks / Replay camera\n✅ ground level preserved"]
E --> I["AudioManager::update()"]
I --> J["cameraPivot = getPosition()\ncameraPivot.z = ground_z"]
J --> K["desiredHeight = micDesiredHeight + ground_z"]
J --> L["groundToCameraVector = cameraPos − cameraPivot"]
K --> M{"cameraPos.z <= desiredHeight?"}
M -- yes --> N["bestScale = maxPercentage"]
M -- no --> O["zScale = desiredHeight / groundToCameraVector.z ⚠️\nShould be: (desiredHeight - cameraPivot.z) / groundToCameraVector.z"]
O --> P["bestScale = MIN(maxPercentage, zScale)"]
N --> Q["micPos = cameraPivot + bestScale × groundToCameraVector"]
P --> Q
Comments Outside Diff (1)
-
Core/GameEngine/Source/Common/Audio/GameAudio.cpp, line 317 (link)Incorrect
zScaleformula doubles terrain height in microphone placementdesiredHeightis an absolute world-space height (m_microphoneDesiredHeightAboveTerrain + cameraPivot.z), butgroundToCameraVectoris a relative vector (cameraPos − cameraPivot). Dividing the absolute height by the relative vector's z gives a scale that places the microphone atcameraPivot.z + desiredHeight = 2·terrain_height + m_microphoneDesiredHeightAboveTerraininstead of the intendedterrain_height + m_microphoneDesiredHeightAboveTerrain.The formula was correct before this PR only because
cameraPivot.zwas always 0. Now that it stores the terrain height, the scale must account for the pivot offset:Concretely: terrain = 100 m, camera = 300 m,
m_microphoneDesiredHeightAboveTerrain= 30 m,maxPercentage= 0.5.- Current code:
zScale = 130/200 = 0.65, capped to 0.5 → mic at 100 + 100 = 200 m (70 m too high). - Fixed code:
zScale = 30/200 = 0.15→ mic at 100 + 30 = 130 m ✓
Prompt To Fix With AI
This is a comment left during a code review. Path: Core/GameEngine/Source/Common/Audio/GameAudio.cpp Line: 317 Comment: **Incorrect `zScale` formula doubles terrain height in microphone placement** `desiredHeight` is an *absolute* world-space height (`m_microphoneDesiredHeightAboveTerrain + cameraPivot.z`), but `groundToCameraVector` is a *relative* vector (`cameraPos − cameraPivot`). Dividing the absolute height by the relative vector's z gives a scale that places the microphone at `cameraPivot.z + desiredHeight = 2·terrain_height + m_microphoneDesiredHeightAboveTerrain` instead of the intended `terrain_height + m_microphoneDesiredHeightAboveTerrain`. The formula was correct before this PR only because `cameraPivot.z` was always 0. Now that it stores the terrain height, the scale must account for the pivot offset: Concretely: terrain = 100 m, camera = 300 m, `m_microphoneDesiredHeightAboveTerrain` = 30 m, `maxPercentage` = 0.5. - **Current code**: `zScale = 130/200 = 0.65`, capped to 0.5 → mic at 100 + 100 = 200 m (70 m too high). - **Fixed code**: `zScale = 30/200 = 0.15` → mic at 100 + 30 = 130 m ✓ How can I resolve this? If you propose a fix, please make it concise.
- Current code:
Prompt To Fix All With AI
This is a comment left during a code review.
Path: Core/GameEngine/Source/Common/Audio/GameAudio.cpp
Line: 317
Comment:
**Incorrect `zScale` formula doubles terrain height in microphone placement**
`desiredHeight` is an *absolute* world-space height (`m_microphoneDesiredHeightAboveTerrain + cameraPivot.z`), but `groundToCameraVector` is a *relative* vector (`cameraPos − cameraPivot`). Dividing the absolute height by the relative vector's z gives a scale that places the microphone at `cameraPivot.z + desiredHeight = 2·terrain_height + m_microphoneDesiredHeightAboveTerrain` instead of the intended `terrain_height + m_microphoneDesiredHeightAboveTerrain`.
The formula was correct before this PR only because `cameraPivot.z` was always 0. Now that it stores the terrain height, the scale must account for the pivot offset:
```suggestion
Real zScale = (desiredHeight - cameraPivot.z) / groundToCameraVector.z;
```
Concretely: terrain = 100 m, camera = 300 m, `m_microphoneDesiredHeightAboveTerrain` = 30 m, `maxPercentage` = 0.5.
- **Current code**: `zScale = 130/200 = 0.65`, capped to 0.5 → mic at 100 + 100 = 200 m (70 m too high).
- **Fixed code**: `zScale = 30/200 = 0.15` → mic at 100 + 30 = 130 m ✓
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: 507
Comment:
**Stale comment references removed member `m_groundLevel`**
The comment still refers to `m_groundLevel` which was merged into `m_pos.z` by this PR.
```suggestion
-- they assume that all maps are height 'm_pos.z' at the edges.
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "bugfix(view): Fix ground level of bookma..." | Re-trigger Greptile
| { | ||
| Coord3D groundPos, microphonePos; | ||
| TheTacticalView->getPosition( &groundPos ); | ||
| Coord3D microphonePos; |
There was a problem hiding this comment.
Move to line 327 where it is used for the first time
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