bugfix(milesaudiomanager): Fix premature 2d and 3d sound cancellations from MilesAudioManager::stopAudioEvent() - #3254
Conversation
xezon
added
Audio
PR Summary by QodoPrevent premature 2D and 3D audio cancellation in MilesAudioManager
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. release is undefined
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Include/Common/AudioRequest.h | Adds initialized stop-request state so delayed sample requests can retain loop-cancellation intent. |
| Core/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h | Adds active-playback stop state and a helper distinguishing playable or rerequested audio from stopped entries. |
| Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp | Propagates stop intent through delayed playback, cancels queued streams, filters stopped entries, and simplifies completion and priority logic. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Queued audio request] --> B{Stop requested?}
B -->|No| C[Start playback]
B -->|Queued sound effect| D[Carry stop flag into PlayingAudio]
B -->|Queued stream or music| E[Remove queued request]
C --> F[Current sample portion completes]
D --> F
F --> G{Stop flag set?}
G -->|Yes| H[Do not start another loop]
G -->|No| I[Continue or rerequest next loop]
H --> J[Release during update]
E --> J
Reviews (3): Last reviewed commit: "bugfix(milesaudiomanager): No longer use..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
05603ad to
2ec9a2d
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05603adc80
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (playing->m_status != PS_Playing) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
When a looping sample has an inter-loop delay, startNextLoop() sets its status to PS_Stopping while m_rerequestOnNextUpdate remains true. If setAudioEventVolumeOverride() runs during that handoff, this new guard skips the event, so its shared DynamicAudioEventRTS retains the old volume; the subsequent rerequest does not reapply m_adjustedVolumes, and the loop resumes indefinitely at the stale volume. Treat isPlayingOrRequested() entries as adjustable (at least update their event state without calling Miles on a stopped sample).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yes, but it is also not looking into requested audio so it is consistent from that angle.
I will make a follow up fix and make a pass over all functions to also cover audio requests.
| if ((*it)->m_status != PS_Playing) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
For a looping ST_VOICE event with an inter-loop delay, startNextLoop() temporarily sets PS_Stopping together with m_rerequestOnNextUpdate, and processPlayingList() later queues the same event again. This guard reports that the object's voice is inactive during that handoff, so another voice triggered for the same object can pass violatesVoice() and overlap the original when it resumes. Use isPlayingOrRequested() here so only genuinely completed voices are excluded.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yes it is suspicious but beyond the scope for now because this function is also not looking into the request list.
…gAudio in MilesAudioManager (#3254)
…oCompletion() (#3254)
…ioritySound() (#3254)
…s from MilesAudioManager::stopAudioEvent() (#3254)
2ec9a2d to
cd11e0e
Compare
2 participants
Merge with Rebase
This change has 5 commits.
1: MilesAudioManager now uses consistent variable names for PlayingAudio pointers. Previously was a mix of
audio,aud,release,playing,looping. For ease of understanding the code.2: Simplifies function MilesAudioManager::notifyOfAudioCompletion(). For ease of understanding the code.
3: Simplifies function MilesAudioManager::findLowestPrioritySound(). For ease of understanding the code.
4: Fixes premature 2d and 3d sound cancellations from MilesAudioManager::stopAudioEvent(). Fixes #3232
5: No longer uses stopped audio in queries and updates to prevent 1 frame gaps when a stopping audio is about to be released.
AI Use
All code changes were applied by hand and Claude Opus was used for several review rounds.
TODO