Implement code review suggestions

src/openrct2-ui/windows/Ride.cpp

@@ -5009,7 +5009,7 @@ static void window_ride_music_resize(rct_window* w)
     window_set_resize(w, 316, 81, 316, 81);
 }
 
-static size_t GetMusicStyleOrder(ObjectEntryIndex musicObjectIndex)
+static std::optional<size_t> GetMusicStyleOrder(ObjectEntryIndex musicObjectIndex)
 {
     auto& objManager = GetContext()->GetObjectManager();
     auto musicObj = static_cast<MusicObject*>(objManager.GetLoadedObject(ObjectType::Music, musicObjectIndex));
@@ -5025,7 +5025,7 @@ static size_t GetMusicStyleOrder(ObjectEntryIndex musicObjectIndex)
         }
     }
 
-    return std::numeric_limits<size_t>::max();
+    return std::nullopt;
 }
 
 /**
@@ -5056,7 +5056,7 @@ static void window_ride_music_mousedown(rct_window* w, rct_widgetindex widgetInd
             if (originalStyleId == MUSIC_STYLE_CUSTOM_MUSIC_1 || originalStyleId == MUSIC_STYLE_CUSTOM_MUSIC_2)
             {
                 auto numTracks = musicObj->GetTrackCount();
-                if (numTracks > 0)
+                if (numTracks != 0)
                 {
                     auto track0 = musicObj->GetTrack(0);
                     if (!track0->Asset.IsAvailable())

src/openrct2/audio/AudioMixer.cpp

@@ -129,6 +129,28 @@ void Mixer_Channel_SetGroup(void* channel, MixerGroup group)
     static_cast<IAudioChannel*>(channel)->SetGroup(group);
 }
 
+template<typename T> static void* PlayMusic(T&& src, int32_t loop)
+{
+    auto* mixer = GetMixer();
+    if (mixer == nullptr)
+        return nullptr;
+
+    auto audioContext = GetContext()->GetAudioContext();
+    auto stream = audioContext->CreateStreamFromWAV(std::forward<T&&>(src));
+    if (stream == nullptr)
+        return nullptr;
+
+    auto* channel = mixer->Play(stream, loop, false, true);
+    if (channel == nullptr)
+    {
+        delete stream;
+        return nullptr;
+    }
+
+    channel->SetGroup(MixerGroup::RideMusic);
+    return channel;
+}
+
 void* Mixer_Play_Music(int32_t pathId, int32_t loop, int32_t streaming)
 {
     IAudioChannel* channel = nullptr;
@@ -168,50 +190,12 @@ void* Mixer_Play_Music(int32_t pathId, int32_t loop, int32_t streaming)
 
 void* Mixer_Play_Music(const char* path, int32_t loop)
 {
-    IAudioChannel* channel = nullptr;
-    IAudioMixer* mixer = GetMixer();
-    if (mixer != nullptr)
-    {
-        auto audioContext = GetContext()->GetAudioContext();
-        auto source = audioContext->CreateStreamFromWAV(path);
-        if (source != nullptr)
-        {
-            channel = mixer->Play(source, loop, false, true);
-            if (channel == nullptr)
-            {
-                delete source;
-            }
-        }
-    }
-    if (channel != nullptr)
-    {
-        channel->SetGroup(MixerGroup::RideMusic);
-    }
-    return channel;
+    return PlayMusic(path, loop);
 }
 
 void* Mixer_Play_Music(std::unique_ptr<IStream> stream, int32_t loop)
 {
-    IAudioChannel* channel = nullptr;
-    IAudioMixer* mixer = GetMixer();
-    if (mixer != nullptr)
-    {
-        auto audioContext = GetContext()->GetAudioContext();
-        auto source = audioContext->CreateStreamFromWAV(std::move(stream));
-        if (source != nullptr)
-        {
-            channel = mixer->Play(source, loop, false, true);
-            if (channel == nullptr)
-            {
-                delete source;
-            }
-        }
-    }
-    if (channel != nullptr)
-    {
-        channel->SetGroup(MixerGroup::RideMusic);
-    }
-    return channel;
+    return PlayMusic(std::move(stream), loop);
 }
 
 void Mixer_SetVolume(float volume)

src/openrct2/core/Path.cpp

@@ -45,8 +45,8 @@ namespace Path
             return std::string(b);
         if (b.empty())
             return std::string(a);
-        auto aEnd = a[a.size() - 1];
-        auto bBegin = b[0];
+        auto aEnd = a.back();
+        auto bBegin = b.front();
         if (IsPathSeparator(aEnd))
         {
             if (IsPathSeparator(bBegin))

src/openrct2/core/Zip.cpp

@@ -23,13 +23,16 @@ static std::string NormalisePath(std::string_view path)
     std::string result;
     if (!path.empty())
     {
-        // Convert back slashes to forward slashes
-        result = std::string(path);
-        for (auto ch = result.data(); *ch != '\0'; ch++)
+        result.reserve(path.size());
+        for (auto ch : path)
         {
-            if (*ch == '\\')
+            if (ch == '\\')
             {
-                *ch = '/';
+                result += '/';
+            }
+            else
+            {
+                result += ch;
             }
         }
     }
@@ -54,7 +57,7 @@ std::optional<size_t> IZipArchive::GetIndexFromPath(std::string_view path) const
             }
         }
     }
-    return -1;
+    return std::nullopt;
 }
 
 bool IZipArchive::Exists(std::string_view path) const
@@ -140,8 +143,7 @@ public:
                     uint64_t readBytes = zip_fread(zipFile, result.data(), dataSize);
                     if (readBytes != dataSize)
                     {
-                        result.clear();
-                        result.shrink_to_fit();
+                        result = {};
                     }
                     zip_fclose(zipFile);
                 }

src/openrct2/object/MusicObject.cpp

@@ -33,7 +33,7 @@ void MusicObject::Load()
     for (auto& track : _tracks)
     {
         track.BytesPerTick = DEFAULT_BYTES_PER_TICK;
-        track.Length = track.Asset.GetLength();
+        track.Size = track.Asset.GetSize();
     }
 }
 

src/openrct2/object/MusicObject.h

@@ -28,7 +28,7 @@ public:
     /**
      * The length of the PCM track in bytes.
      */
-    size_t Length;
+    size_t Size;
 };
 
 class MusicObject final : public Object

src/openrct2/object/Object.cpp

@@ -251,7 +251,7 @@ bool ObjectAsset::IsAvailable() const
     }
 }
 
-size_t ObjectAsset::GetLength() const
+size_t ObjectAsset::GetSize() const
 {
     if (_zipPath.empty())
     {

src/openrct2/object/Object.h

@@ -220,7 +220,7 @@ public:
     }
 
     bool IsAvailable() const;
-    size_t GetLength() const;
+    size_t GetSize() const;
     std::unique_ptr<OpenRCT2::IStream> GetStream() const;
 };
 

src/openrct2/ride/RideAudio.cpp

@@ -279,7 +279,7 @@ static std::pair<size_t, size_t> RideMusicGetTrackOffsetLength(const Ride& ride)
             if (ride.music_tune_id < numTracks)
             {
                 auto track = musicObj->GetTrack(ride.music_tune_id);
-                return { track->BytesPerTick, track->Length };
+                return { track->BytesPerTick, track->Size };
             }
         }
     }