fix: Add additional checks for invalid MusicTheme to prevent crashes …

…if Play(), Transition() or Stop() is called (#35)
Silver-Ore-Team · May 31, 2024 · 7176f9d · 7176f9d
1 parent c05cadb
commit 7176f9d
Show file tree

Hide file tree

Showing 8 changed files with 83 additions and 15 deletions.
diff --git 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
@@ -18,13 +18,6 @@ namespace NH::Bass
         m_DeferredCommands.push_back(std::move(command));
     }
 
-    void CommandQueue::AddCommandOnFront(std::shared_ptr<Command> command)
-    {
-        std::lock_guard<std::mutex> lock(m_DeferredMutex);
-        log->Trace("Adding command to queue");
-        m_DeferredCommands.push_front(std::move(command));
-    }
-
     void CommandQueue::AddPerFrameCommand(std::shared_ptr<Command> command)
     {
         std::lock_guard<std::mutex> lock(m_PerFrameDeferredMutex);

diff --git 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> command);
 
-        void AddCommandOnFront(std::shared_ptr<Command> command);
-
         void AddPerFrameCommand(std::shared_ptr<Command> command);
 
         void Update(Engine& engine);

diff --git 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<MusicTheme> Engine::GetActiveTheme()
+    {
+        return m_ActiveTheme;
+    }
+
+    void Engine::SetActiveTheme(std::shared_ptr<MusicTheme> theme)
+    {
+        m_ActiveTheme = theme;
+    }
+
     EventManager& Engine::GetEM()
     {
         return m_EventManager;

diff --git 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<MusicTheme> GetActiveTheme();
+        void SetActiveTheme(std::shared_ptr<MusicTheme> theme);
+
         std::shared_ptr<IChannel> AcquireFreeChannel() override;
         void ReleaseChannel(const std::shared_ptr<IChannel>& channel) override;
 

diff --git 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;
+    }
 }
diff --git 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;
+    };
 }
diff --git 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<Transition::TimePoint> 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<PlayThemeInstantCommand>(nextTheme.GetName()));
+                return;
+            }
+        }
+
+        auto channel = GetAcquiredChannel();
+        if (!channel) channel = m_AcquiredChannels.emplace_back(engine.AcquireFreeChannel());
+        std::optional<Transition::TimePoint> timePoint = transition.NextAvailableTimePoint(channel->Position());
+
         const std::function<void()>& 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<Transition::TimePoint> 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<Transition::TimePoint> 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)
         {