Refactor to avoid unnecessary copies (#13736)

* Refactor to avoid unnecessary copies

* Fix dangling references

src/openrct2-ui/UiContext.Win32.cpp

@@ -235,7 +235,7 @@ namespace OpenRCT2::Ui
         static std::wstring GetFilterString(const std::vector<FileDialogDesc::Filter> filters)
         {
             std::wstringstream filtersb;
-            for (auto filter : filters)
+            for (const auto& filter : filters)
             {
                 filtersb << String::ToWideChar(filter.Name) << '\0' << String::ToWideChar(filter.Pattern) << '\0';
             }

src/openrct2-ui/scripting/CustomListView.cpp

@@ -278,7 +278,7 @@ void CustomListView::SetItems(const std::vector<ListViewItem>& items, bool initi
 
 void CustomListView::SetItems(std::vector<ListViewItem>&& items, bool initialising)
 {
-    Items = items;
+    Items = std::move(items);
     SortItems(0, ColumnSortOrder::None);
     if (!initialising)
     {

src/openrct2-ui/scripting/CustomListView.h

@@ -61,7 +61,7 @@ namespace OpenRCT2::Ui::Windows
             Cells.emplace_back(text);
         }
         explicit ListViewItem(std::vector<std::string>&& cells)
-            : Cells(cells)
+            : Cells(std::move(cells))
         {
         }
     };

src/openrct2-ui/scripting/CustomWindow.cpp

@@ -703,7 +703,7 @@ namespace OpenRCT2::Ui::Windows
         const auto& customInfo = GetInfo(w);
         const auto& tabs = customInfo.Desc.Tabs;
         size_t tabIndex = 0;
-        for (auto tab : tabs)
+        for (const auto& tab : tabs)
         {
             auto widgetIndex = static_cast<rct_widgetindex>(WIDX_TAB_0 + tabIndex);
             auto widget = &w->widgets[widgetIndex];
@@ -1244,11 +1244,11 @@ namespace OpenRCT2::Ui::Windows
 
     rct_window* FindCustomWindowByClassification(std::string_view classification)
     {
-        for (auto w : g_window_list)
+        for (const auto& w : g_window_list)
         {
             if (w->classification == WC_CUSTOM)
             {
-                auto& customInfo = GetInfo(w.get());
+                const auto& customInfo = GetInfo(w.get());
                 if (customInfo.Desc.Classification == classification)
                 {
                     return w.get();

src/openrct2-ui/scripting/ScUi.hpp

@@ -160,7 +160,7 @@ namespace OpenRCT2::Scripting
             {
                 auto index = a.as_int();
                 auto i = 0;
-                for (auto w : g_window_list)
+                for (const auto& w : g_window_list)
                 {
                     if (i == index)
                     {
@@ -171,7 +171,7 @@ namespace OpenRCT2::Scripting
             }
             else if (a.type() == DukValue::Type::STRING)
             {
-                auto classification = a.as_string();
+                const auto& classification = a.as_string();
                 auto w = FindCustomWindowByClassification(classification);
                 if (w != nullptr)
                 {

src/openrct2-ui/windows/Changelog.cpp

@@ -219,7 +219,7 @@ static void window_changelog_scrollpaint(rct_window* w, rct_drawpixelinfo* dpi,
     const int32_t lineHeight = font_get_line_height(gCurrentFontSpriteBase);
 
     ScreenCoordsXY screenCoords(3, 3 - lineHeight);
-    for (auto line : _changelogLines)
+    for (const auto& line : _changelogLines)
     {
         screenCoords.y += lineHeight;
         if (screenCoords.y + lineHeight < dpi->y || screenCoords.y >= dpi->y + dpi->height)

src/openrct2/ParkImporter.h

@@ -71,7 +71,7 @@ public:
     std::vector<rct_object_entry> const MissingObjects;
 
     explicit ObjectLoadException(std::vector<rct_object_entry>&& missingObjects)
-        : MissingObjects(missingObjects)
+        : MissingObjects(std::move(missingObjects))
     {
     }
 };

src/openrct2/ReplayManager.cpp

@@ -147,7 +147,7 @@ namespace OpenRCT2
 
         void AddChecksum(uint32_t tick, rct_sprite_checksum&& checksum)
         {
-            _currentRecording->checksums.emplace_back(std::make_pair(tick, checksum));
+            _currentRecording->checksums.emplace_back(std::make_pair(tick, std::move(checksum)));
         }
 
         // Function runs each Tick.

src/openrct2/actions/GameAction.h

@@ -41,7 +41,7 @@ public:
     }
 
     StringVariant(std::string&& s)
-        : String(s)
+        : String(std::move(s))
     {
     }
 

src/openrct2/core/Collections.hpp

@@ -37,10 +37,10 @@ namespace Collections
     }
 
     template<typename TCollection, typename TItem, typename TComparer>
-    static size_t IndexOf(TCollection& collection, TItem needle, TComparer comparer)
+    static size_t IndexOf(const TCollection& collection, TItem needle, TComparer comparer)
     {
         size_t index = 0;
-        for (TItem item : collection)
+        for (const auto& item : collection)
         {
             if (comparer(item, needle))
             {
@@ -51,10 +51,10 @@ namespace Collections
         return SIZE_MAX;
     }
 
-    template<typename TCollection, typename TPred> static size_t IndexOf(TCollection& collection, TPred predicate)
+    template<typename TCollection, typename TPred> static size_t IndexOf(const TCollection& collection, TPred predicate)
     {
         size_t index = 0;
-        for (auto item : collection)
+        for (const auto& item : collection)
         {
             if (predicate(item))
             {

src/openrct2/core/FileIndex.hpp

@@ -233,7 +233,7 @@ private:
 
             jobPool.Join(reportProgress);
 
-            for (auto&& itr : containers)
+            for (const auto& itr : containers)
             {
                 allItems.insert(allItems.end(), itr.begin(), itr.end());
             }

src/openrct2/core/JobPool.cpp

@@ -35,7 +35,7 @@ JobPool::~JobPool()
         _condPending.notify_all();
     }
 
-    for (auto&& th : _threads)
+    for (auto& th : _threads)
     {
         assert(th.joinable() != false);
         th.join();

src/openrct2/core/MemoryStream.cpp

@@ -52,8 +52,16 @@ namespace OpenRCT2
     }
 
     MemoryStream::MemoryStream(MemoryStream&& mv) noexcept
+        : _access(mv._access)
+        , _dataCapacity(mv._dataCapacity)
+        , _dataSize(mv._dataSize)
+        , _data(mv._data)
+        , _position(mv._position)
     {
-        *this = std::move(mv);
+        mv._data = nullptr;
+        mv._position = nullptr;
+        mv._dataCapacity = 0;
+        mv._dataSize = 0;
     }
 
     MemoryStream::~MemoryStream()
@@ -69,15 +77,19 @@ namespace OpenRCT2
 
     MemoryStream& MemoryStream::operator=(MemoryStream&& mv) noexcept
     {
-        _access = mv._access;
-        _dataCapacity = mv._dataCapacity;
-        _data = mv._data;
-        _position = mv._position;
+        if (this != &mv)
+        {
+            _access = mv._access;
+            _dataCapacity = mv._dataCapacity;
+            _data = mv._data;
+            _dataSize = mv._dataSize;
+            _position = mv._position;
 
-        mv._data = nullptr;
-        mv._position = nullptr;
-        mv._dataCapacity = 0;
-        mv._dataSize = 0;
+            mv._data = nullptr;
+            mv._position = nullptr;
+            mv._dataCapacity = 0;
+            mv._dataSize = 0;
+        }
 
         return *this;
     }

src/openrct2/interface/Viewport.cpp

@@ -1020,7 +1020,7 @@ void viewport_paint(
         _paintJobs->Join();
     }
 
-    for (auto&& column : columns)
+    for (auto column : columns)
     {
         viewport_paint_column(column);
     }

src/openrct2/network/NetworkBase.cpp

@@ -1830,7 +1830,7 @@ void NetworkBase::ProcessPlayerList()
             std::vector<uint8_t> newPlayers;
             std::vector<uint8_t> removedPlayers;
 
-            for (auto&& pendingPlayer : itPending->second.players)
+            for (const auto& pendingPlayer : itPending->second.players)
             {
                 activePlayerIds.push_back(pendingPlayer.Id);
 

src/openrct2/network/NetworkUser.cpp

@@ -210,7 +210,7 @@ const NetworkUser* NetworkUserManager::GetUserByHash(const std::string& hash) co
 
 const NetworkUser* NetworkUserManager::GetUserByName(const std::string& name) const
 {
-    for (auto kvp : _usersByHash)
+    for (const auto& kvp : _usersByHash)
     {
         const NetworkUser* networkUser = kvp.second;
         if (String::Equals(name.c_str(), networkUser->Name.c_str(), true))

src/openrct2/object/Object.cpp

@@ -141,9 +141,9 @@ const std::vector<std::string>& Object::GetAuthors() const
     return _authors;
 }
 
-void Object::SetAuthors(const std::vector<std::string>&& authors)
+void Object::SetAuthors(std::vector<std::string>&& authors)
 {
-    _authors = authors;
+    _authors = std::move(authors);
 }
 
 std::optional<uint8_t> rct_object_entry::GetSceneryType() const

src/openrct2/object/Object.h

@@ -319,7 +319,7 @@ public:
     void SetSourceGames(const std::vector<ObjectSourceGame>& sourceGames);
 
     const std::vector<std::string>& GetAuthors() const;
-    void SetAuthors(const std::vector<std::string>&& authors);
+    void SetAuthors(std::vector<std::string>&& authors);
 
     const ImageTable& GetImageTable() const
     {

src/openrct2/object/ObjectRepository.cpp

@@ -399,7 +399,7 @@ private:
     void AddItems(const std::vector<ObjectRepositoryItem>& items)
     {
         size_t numConflicts = 0;
-        for (auto item : items)
+        for (const auto& item : items)
         {
             if (!AddItem(item))
             {

src/openrct2/platform/Crash.cpp

@@ -59,7 +59,7 @@ const wchar_t* _wszArchitecture = WSZ(OPENRCT2_ARCHITECTURE);
 // https://documentation.backtrace.io/product_integration_minidump_breakpad/
 static bool UploadMinidump(const std::map<std::wstring, std::wstring>& files, int& error, std::wstring& response)
 {
-    for (auto file : files)
+    for (const auto& file : files)
     {
         wprintf(L"files[%s] = %s\n", file.first.c_str(), file.second.c_str());
     }

src/openrct2/platform/Platform.Linux.cpp

@@ -104,7 +104,7 @@ namespace Platform
             "/var/lib/openrct2",
             "/usr/share/openrct2",
         };
-        for (auto prefix : prefixes)
+        for (const auto& prefix : prefixes)
         {
             for (auto searchLocation : SearchLocations)
             {

src/openrct2/scripting/ScriptEngine.cpp

@@ -492,7 +492,7 @@ void ScriptEngine::StopPlugin(std::shared_ptr<Plugin> plugin)
         RemoveIntervals(plugin);
         RemoveSockets(plugin);
         _hookEngine.UnsubscribeAll(plugin);
-        for (auto callback : _pluginStoppedSubscriptions)
+        for (const auto& callback : _pluginStoppedSubscriptions)
         {
             callback(plugin);
         }

test/tests/CryptTests.cpp

@@ -68,7 +68,7 @@ TEST_F(CryptTests, SHA1_Multiple)
     };
 
     auto alg = Crypt::CreateSHA1();
-    for (auto s : input)
+    for (const auto& s : input)
     {
         alg->Update(s.data(), s.size());
     }
@@ -99,7 +99,7 @@ TEST_F(CryptTests, SHA1_Many)
         "This park is really clean and tidy",
         "This balloon from Balloon Stall 1 is really good value",
     };
-    for (auto s : inputA)
+    for (const auto& s : inputA)
     {
         alg->Update(s.data(), s.size());
     }
@@ -113,7 +113,7 @@ TEST_F(CryptTests, SHA1_Many)
         "This park is really clean and tidy",
         "Merry-go-round 2 looks too intense for me",
     };
-    for (auto s : inputB)
+    for (const auto& s : inputB)
     {
         alg->Update(s.data(), s.size());
     }