From 7176f9dc2ab2144322de22546ec1c920a4f41d9d Mon Sep 17 00:00:00 2001 From: tehe Date: Fri, 31 May 2024 23:54:34 +0200 Subject: [PATCH] fix: Add additional checks for invalid MusicTheme to prevent crashes if Play(), Transition() or Stop() is called (#35) --- src/NH/Bass/Channel.cpp | 2 +- src/NH/Bass/Command.cpp | 7 ------ src/NH/Bass/Command.h | 2 -- src/NH/Bass/Engine.cpp | 10 ++++++++ src/NH/Bass/Engine.h | 3 +++ src/NH/Bass/EngineCommands.cpp | 19 ++++++++++++++ src/NH/Bass/EngineCommands.h | 9 +++++++ src/NH/Bass/MusicTheme.cpp | 46 ++++++++++++++++++++++++++++++---- 8 files changed, 83 insertions(+), 15 deletions(-) diff --git a/src/NH/Bass/Channel.cpp b/src/NH/Bass/Channel.cpp index bbac6ef..22b0859 100644 --- a/src/NH/Bass/Channel.cpp +++ b/src/NH/Bass/Channel.cpp @@ -102,7 +102,7 @@ namespace NH::Bass bool Channel::IsPlaying() const { - return BASS_ChannelIsActive(m_Stream); + return BASS_ChannelIsActive(m_Stream) == BASS_ACTIVE_PLAYING; } double Channel::Position() const diff --git a/src/NH/Bass/Command.cpp b/src/NH/Bass/Command.cpp index 6f5b0cf..4306633 100644 --- a/src/NH/Bass/Command.cpp +++ b/src/NH/Bass/Command.cpp @@ -18,13 +18,6 @@ namespace NH::Bass m_DeferredCommands.push_back(std::move(command)); } - void CommandQueue::AddCommandOnFront(std::shared_ptr command) - { - std::lock_guard lock(m_DeferredMutex); - log->Trace("Adding command to queue"); - m_DeferredCommands.push_front(std::move(command)); - } - void CommandQueue::AddPerFrameCommand(std::shared_ptr command) { std::lock_guard lock(m_PerFrameDeferredMutex); diff --git a/src/NH/Bass/Command.h b/src/NH/Bass/Command.h index ceeadd2..f460bc6 100644 --- a/src/NH/Bass/Command.h +++ b/src/NH/Bass/Command.h @@ -65,8 +65,6 @@ namespace NH::Bass public: void AddCommand(std::shared_ptr command); - void AddCommandOnFront(std::shared_ptr command); - void AddPerFrameCommand(std::shared_ptr command); void Update(Engine& engine); diff --git a/src/NH/Bass/Engine.cpp b/src/NH/Bass/Engine.cpp index c51641a..f68dd28 100644 --- a/src/NH/Bass/Engine.cpp +++ b/src/NH/Bass/Engine.cpp @@ -97,6 +97,16 @@ namespace NH::Bass return m_MasterVolume; } + std::shared_ptr Engine::GetActiveTheme() + { + return m_ActiveTheme; + } + + void Engine::SetActiveTheme(std::shared_ptr theme) + { + m_ActiveTheme = theme; + } + EventManager& Engine::GetEM() { return m_EventManager; diff --git a/src/NH/Bass/Engine.h b/src/NH/Bass/Engine.h index 2393a4d..75d4e23 100644 --- a/src/NH/Bass/Engine.h +++ b/src/NH/Bass/Engine.h @@ -41,6 +41,9 @@ namespace NH::Bass void SetVolume(float volume); [[nodiscard]] float GetVolume() const; + std::shared_ptr GetActiveTheme(); + void SetActiveTheme(std::shared_ptr theme); + std::shared_ptr AcquireFreeChannel() override; void ReleaseChannel(const std::shared_ptr& channel) override; diff --git a/src/NH/Bass/EngineCommands.cpp b/src/NH/Bass/EngineCommands.cpp index 1fcccec..0f135bc 100644 --- a/src/NH/Bass/EngineCommands.cpp +++ b/src/NH/Bass/EngineCommands.cpp @@ -7,6 +7,7 @@ namespace NH::Bass { Logger* ChangeZoneCommand::log = CreateLogger("zBassMusic::ChangeZoneCommand"); Logger* ScheduleThemeChangeCommand::log = CreateLogger("zBassMusic::ScheduleThemeChangeCommand"); + Logger* PlayThemeInstantCommand::log = CreateLogger("zBassMusic::PlayThemeInstantCommand"); CommandResult ChangeZoneCommand::Execute(Engine& engine) { @@ -44,4 +45,22 @@ namespace NH::Bass return CommandResult::DONE; } + + CommandResult PlayThemeInstantCommand::Execute(Engine& engine) + { + log->Info("Playing theme: {0} instantly, because PlayThemeInstantCommand forced it.", m_ThemeId); + auto theme = engine.GetMusicManager().GetTheme(m_ThemeId); + if (!theme) + { + log->Error("Theme {0} doesn't exist", m_ThemeId); + return CommandResult::DONE; + } + + auto activeTheme = engine.GetActiveTheme(); + if (activeTheme) { activeTheme->Stop(engine); } + theme->Play(engine); + engine.SetActiveTheme(theme); + + return CommandResult::DONE; + } } \ No newline at end of file diff --git a/src/NH/Bass/EngineCommands.h b/src/NH/Bass/EngineCommands.h index 8d3bb10..5c68cd2 100644 --- a/src/NH/Bass/EngineCommands.h +++ b/src/NH/Bass/EngineCommands.h @@ -24,4 +24,13 @@ namespace NH::Bass explicit ScheduleThemeChangeCommand(HashString themeId) : m_ThemeId(themeId) {}; CommandResult Execute(Engine& engine) override; }; + + class PlayThemeInstantCommand : public Command + { + static Logger* log; + HashString m_ThemeId; + public: + explicit PlayThemeInstantCommand(HashString themeId) : m_ThemeId(themeId) {}; + CommandResult Execute(Engine& engine) override; + }; } \ No newline at end of file diff --git a/src/NH/Bass/MusicTheme.cpp b/src/NH/Bass/MusicTheme.cpp index 699b188..102d698 100644 --- a/src/NH/Bass/MusicTheme.cpp +++ b/src/NH/Bass/MusicTheme.cpp @@ -93,14 +93,34 @@ namespace NH::Bass void MusicTheme::Transition(IEngine& engine, MusicTheme& nextTheme) { if (nextTheme.GetName() == GetName()) { return; } - auto channel = GetAcquiredChannel(); - if (!channel) channel = m_AcquiredChannels.emplace_back(engine.AcquireFreeChannel()); - const auto& transition = m_TransitionInfo.GetTransition(nextTheme.GetName()); - std::optional timePoint = transition.NextAvailableTimePoint(channel->Position()); + const auto& transition = m_TransitionInfo.GetTransition(nextTheme.GetName()); log->Debug("Transition {0} to {1}", GetName(), nextTheme.GetName()); log->PrintRaw(LoggerLevel::Trace, transition.ToString()); + if (!HasAudioFile(AudioFile::DEFAULT)) + { + log->Error("{0} needs to transition into {1} but it doesn't have a DEFAULT audio. Switching to {1} without transitions", + GetName(), nextTheme.GetName()); + return; + } + + if (!ReadyToPlay(engine, AudioFile::DEFAULT)) + { + const auto& file = GetAudioFile(AudioFile::DEFAULT); + if (file.Status == AudioFile::StatusType::FAILED) + { + log->Warning("{0} needs to transition into {1} but the audio file failed to load. Switching to {1} without transitions", + GetName(), nextTheme.GetName()); + engine.GetCommandQueue().AddCommand(std::make_shared(nextTheme.GetName())); + return; + } + } + + auto channel = GetAcquiredChannel(); + if (!channel) channel = m_AcquiredChannels.emplace_back(engine.AcquireFreeChannel()); + std::optional timePoint = transition.NextAvailableTimePoint(channel->Position()); + const std::function& playJingle = CreateSyncHandler([&engine, &transition, this]() { if (transition.Jingle) { @@ -147,6 +167,11 @@ namespace NH::Bass void MusicTheme::Play(IEngine& engine) { Play(engine, Transition::EMPTY); } void MusicTheme::Play(IEngine& engine, const struct Transition& transition, std::optional timePoint) { + if (!HasAudioFile(AudioFile::DEFAULT)) + { + log->Error("{0} can't play because it doesn't have DEFAULT audio file", GetName()); + return; + } if (!ReadyToPlay(engine, AudioFile::DEFAULT)) { return; } auto channel = m_AcquiredChannels.emplace_back(engine.AcquireFreeChannel()); @@ -186,7 +211,12 @@ namespace NH::Bass void MusicTheme::Stop(IEngine& engine, const struct Transition& transition) { auto channel = GetAcquiredChannel(); - if (!channel) { ReleaseChannels(); }; + if (!channel) + { + log->Warning("Requested to stop {0} but it does not have any playing channels.", m_Name); + ReleaseChannels(); + return; + }; std::optional timePoint = transition.NextAvailableTimePoint(channel->Position()); auto effect = timePoint.has_value() ? timePoint.value().Effect : transition.Effect; if (effect == TransitionEffect::CROSSFADE) @@ -208,6 +238,12 @@ namespace NH::Bass bool MusicTheme::ReadyToPlay(IEngine& engine, HashString audio) { + if (!HasAudioFile(AudioFile::DEFAULT)) + { + log->Error("{0} can't play because it doesn't have DEFAULT audio file", GetName()); + return false; + } + const auto& file = GetAudioFile(AudioFile::DEFAULT); if (file.Status == AudioFile::StatusType::FAILED) {