From aaabcd6c82298eb30e1a5d28d17dde85d3599139 Mon Sep 17 00:00:00 2001 From: Causeless Date: Mon, 12 Oct 2026 07:03:27 +0100 Subject: [PATCH 1/2] Fixed sound channels being left tied to their SoundContainer after it was destroyed A SoundContainer that gets destroyed while its sounds are playing leaves them to play out, which emitters for one count on for their burst and end sounds. The channels playing them kept pointing at the destroyed SoundContainer though, and it then got used every update for the channels' 3D effects, and once more when the sounds ended. AddressSanitizer reports these as uses after free. Destroying a SoundContainer now unties its channels from it. Sounds that loop endlessly get stopped instead, since there'd be nothing left to stop them with, and the ones left to play out stay the way they were, with their 3D effects no longer updated. Co-Authored-By: Claude Opus 5.5 --- Source/Entities/SoundContainer.h | 4 ++++ Source/Managers/AudioMan.cpp | 34 +++++++++++++++++++++++++++++--- Source/Managers/AudioMan.h | 5 +++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/Source/Entities/SoundContainer.h b/Source/Entities/SoundContainer.h index 23a0647df4..48b3841d8c 100644 --- a/Source/Entities/SoundContainer.h +++ b/Source/Entities/SoundContainer.h @@ -61,6 +61,10 @@ namespace RTE { /// Destroys and resets (through Clear()) the SoundContainer object. It doesn't delete the Sound files, since they're owned by ContentFile static maps. /// @param notInherited Whether to only destroy the members defined in this derived class, or to destroy all inherited members also. void Destroy(bool notInherited = false) override { + // The channels that are still playing its sounds can't be left tied to this once it is gone. + if (!m_PlayingChannels.empty()) { + g_AudioMan.DetachSoundContainerPlayingChannels(this); + } if (!notInherited) { Entity::Destroy(); } diff --git a/Source/Managers/AudioMan.cpp b/Source/Managers/AudioMan.cpp index 4839b0af5f..a196a49157 100644 --- a/Source/Managers/AudioMan.cpp +++ b/Source/Managers/AudioMan.cpp @@ -603,6 +603,28 @@ bool AudioMan::StopSoundContainerPlayingChannels(SoundContainer* soundContainer, return result == FMOD_OK; } +void AudioMan::DetachSoundContainerPlayingChannels(SoundContainer* soundContainer) { + if (!m_AudioEnabled || !soundContainer) { + return; + } + + FMOD::Channel* soundChannel; + int loopCount; + + // Stopping a channel takes it out of the SoundContainer's own collection of them, so it's a copy that is gone through here. + const std::unordered_set channels = *soundContainer->GetPlayingChannels(); + for (int channel: channels) { + if (m_AudioSystem->getChannel(channel, &soundChannel) != FMOD_OK) { + continue; + } + if (soundChannel->getLoopCount(&loopCount) == FMOD_OK && loopCount < 0) { + soundChannel->stop(); + } else { + soundChannel->setUserData(nullptr); + } + } +} + void AudioMan::FadeOutSoundContainerPlayingChannels(SoundContainer* soundContainer, int fadeOutTime) { if (!m_AudioEnabled || !soundContainer || !soundContainer->IsBeingPlayed()) { return; @@ -673,7 +695,8 @@ void AudioMan::Update3DEffectsForSFXChannels() { float doubleMinimumDistanceForPanning = m_MinimumDistanceForPanning * 2.0F; void* userData; result = result == FMOD_OK ? soundChannel->getUserData(&userData) : result; - if (result == FMOD_OK) { + // A sound that is playing out after its SoundContainer got destroyed stays the way it was left. + if (result == FMOD_OK && userData) { const SoundContainer* soundContainer = static_cast(userData); if (sqrDistanceToPlayer < (m_MinimumDistanceForPanning * m_MinimumDistanceForPanning) || soundContainer->GetCustomPanValue() != 0.0f) { result = soundChannel->set3DLevel(0); @@ -702,6 +725,10 @@ FMOD_RESULT AudioMan::UpdatePositionalEffectsForSoundChannel(FMOD::Channel* soun } const SoundContainer* channelSoundContainer = static_cast(userData); + // A sound that is playing out after its SoundContainer got destroyed stays the way it was left. + if (!channelSoundContainer) { + return FMOD_OK; + } bool sceneWraps = g_SceneMan.SceneWrapsX(); @@ -798,11 +825,12 @@ FMOD_RESULT F_CALLBACK AudioMan::SoundChannelEndedCallback(FMOD_CHANNELCONTROL* FMOD_RESULT result = channel->getIndex(&channelIndex); // Remove this playing sound index from the SoundContainer if it has any playing sounds, i.e. it hasn't been reset before this callback happened. + // There is no SoundContainer to remove it from if that got destroyed while the sound was left to play out. void* userData; result = (result == FMOD_OK) ? channel->getUserData(&userData) : result; if (result == FMOD_OK) { SoundContainer* channelSoundContainer = static_cast(userData); - if (channelSoundContainer->IsBeingPlayed()) { + if (channelSoundContainer && channelSoundContainer->IsBeingPlayed()) { channelSoundContainer->RemovePlayingChannel(channelIndex); } result = (result == FMOD_OK) ? channel->setUserData(nullptr) : result; @@ -812,7 +840,7 @@ FMOD_RESULT F_CALLBACK AudioMan::SoundChannelEndedCallback(FMOD_CHANNELCONTROL* } if (result != FMOD_OK) { - g_ConsoleMan.PrintString("ERROR: An error occurred when Ending a sound in SoundContainer " + channelSoundContainer->GetPresetName() + ": " + std::string(FMOD_ErrorString(result))); + g_ConsoleMan.PrintString("ERROR: An error occurred when Ending a sound in SoundContainer " + (channelSoundContainer ? channelSoundContainer->GetPresetName() : "None") + ": " + std::string(FMOD_ErrorString(result))); return result; } } else { diff --git a/Source/Managers/AudioMan.h b/Source/Managers/AudioMan.h index f95ee170a0..570919b15a 100644 --- a/Source/Managers/AudioMan.h +++ b/Source/Managers/AudioMan.h @@ -396,6 +396,11 @@ namespace RTE { /// @return bool StopSoundContainerPlayingChannels(SoundContainer* soundContainer, int player); + /// Has the channels playing a SoundContainer's sounds no longer be tied to it, for when it is about to be destroyed. + /// The sounds get to play out, except for the ones that loop endlessly. Those get stopped, since there would be nothing left to stop them with. + /// @param soundContainer A pointer to a SoundContainer object. Ownership is NOT transferred! + void DetachSoundContainerPlayingChannels(SoundContainer* soundContainer); + /// Fades out playback a SoundContainer. /// @param soundContainer A pointer to a SoundContainer object. Ownership is NOT transferred! /// @param fadeOutTime The amount of time, in ms, to fade out over. From bbc8ce47396af8c423c9f7d9aef08c4a6868a27a Mon Sep 17 00:00:00 2001 From: Causeless Date: Mon, 12 Oct 2026 07:25:41 +0100 Subject: [PATCH 2/2] Fixed EquipShieldInBGArm leaving the device in the inventory after equipping it When told to put what the BG arm was holding at the front of the inventory, it went on to use the place it had found the device at before doing so, which by then was the place of the item in front of it. That item got taken out of the inventory instead, never to be seen again, and the device ended up both held and in the inventory. Its holder dying then dropped it twice over, which left MovableMan with it in its list of items twice, and the game deleting it twice when quitting. Lua could get this on any call, since its binding cast the function to one without that argument, which left the argument as whatever happened to be in its place. It is bound through an adapter now, which leaves the argument at its default. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 ++ Source/Entities/AHuman.cpp | 5 ++++- Source/Lua/LuaAdapterDefinitions.h | 1 + Source/Lua/LuaAdapters.cpp | 4 ++++ Source/Lua/LuaBindingsEntities.cpp | 2 +- 5 files changed, 12 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index caedf40c3b..6c19a6883a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -270,6 +270,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fixed a couple of uninitialized values (Atom step state and the simulation frame counter) that could cause inconsistent behavior between runs. +- Fixed `AHuman:EquipShieldInBGArm()` being able to take the wrong item out of the inventory, which left the device it equipped both held and in the inventory. The device then got dropped twice when its holder died, and the game could crash when quitting. +
Removed diff --git a/Source/Entities/AHuman.cpp b/Source/Entities/AHuman.cpp index e032cd8a02..4bfecf9edd 100644 --- a/Source/Entities/AHuman.cpp +++ b/Source/Entities/AHuman.cpp @@ -1112,7 +1112,10 @@ bool AHuman::EquipShieldInBGArm(bool depositToFront) { if (HeldDevice* heldDevice = m_pBGArm->GetHeldDevice()) { heldDevice->Deactivate(); if (depositToFront) { - AddToInventoryFront(m_pBGArm->RemoveAttachable(heldDevice)); + // What goes in at the front moves the device we are after one place further in. + if (AddToInventoryFront(m_pBGArm->RemoveAttachable(heldDevice))) { + device_offset++; + } } else { AddToInventoryBack(m_pBGArm->RemoveAttachable(heldDevice)); } diff --git a/Source/Lua/LuaAdapterDefinitions.h b/Source/Lua/LuaAdapterDefinitions.h index 3e59ae2fe2..1f6616ed99 100644 --- a/Source/Lua/LuaAdapterDefinitions.h +++ b/Source/Lua/LuaAdapterDefinitions.h @@ -321,6 +321,7 @@ namespace RTE { #pragma region AHuman Lua Adapters struct LuaAdaptersAHuman { + static bool EquipShieldInBGArm(AHuman* luaSelfObject); static void ReloadFirearms(AHuman* luaSelfObject); }; #pragma endregion diff --git a/Source/Lua/LuaAdapters.cpp b/Source/Lua/LuaAdapters.cpp index b73f7c33cc..df716cf88f 100644 --- a/Source/Lua/LuaAdapters.cpp +++ b/Source/Lua/LuaAdapters.cpp @@ -312,6 +312,10 @@ void LuaAdaptersScene::CalculatePathAsync(Scene* luaSelfObject, const luabind::o luaSelfObject->CalculatePathAsync(start, end, jumpHeight, digStrength, team, callLuaCallback); } +bool LuaAdaptersAHuman::EquipShieldInBGArm(AHuman* luaSelfObject) { + return luaSelfObject->EquipShieldInBGArm(); +} + void LuaAdaptersAHuman::ReloadFirearms(AHuman* luaSelfObject) { luaSelfObject->ReloadFirearms(false); } diff --git a/Source/Lua/LuaBindingsEntities.cpp b/Source/Lua/LuaBindingsEntities.cpp index 6b138782a3..da0a81547a 100644 --- a/Source/Lua/LuaBindingsEntities.cpp +++ b/Source/Lua/LuaBindingsEntities.cpp @@ -442,7 +442,7 @@ LuaBindingRegisterFunctionDefinitionForType(EntityLuaBindings, AHuman) { .def("EquipThrowable", &AHuman::EquipThrowable) .def("EquipDiggingTool", &AHuman::EquipDiggingTool) .def("EquipShield", &AHuman::EquipShield) - .def("EquipShieldInBGArm", (bool(AHuman::*)()) & AHuman::EquipShieldInBGArm) + .def("EquipShieldInBGArm", &LuaAdaptersAHuman::EquipShieldInBGArm) .def("EquipDeviceInGroup", &AHuman::EquipDeviceInGroup) .def("EquipNamedDevice", (bool(AHuman::*)(const std::string&, bool)) & AHuman::EquipNamedDevice) .def("EquipNamedDevice", (bool(AHuman::*)(const std::string&, const std::string&, bool)) & AHuman::EquipNamedDevice)