Close #13000: Refactor ObjectFactory to use unique_ptr

Employs the smart pointer unique_ptr for safer memory management.

Classes involved:
 - ObjectRepository
 - ObjectManager

src/openrct2-ui/windows/EditorInventionsList.cpp

@@ -553,7 +553,7 @@ static void window_editor_inventions_list_paint(rct_window* w, rct_drawpixelinfo
     // Draw preview
     widget = &w->widgets[WIDX_PREVIEW];
 
-    void* object = object_manager_get_loaded_object(entry);
+    auto* object = object_manager_get_loaded_object(entry);
     if (object != nullptr)
     {
         rct_drawpixelinfo clipDPI;
@@ -562,7 +562,7 @@ static void window_editor_inventions_list_paint(rct_window* w, rct_drawpixelinfo
         int32_t height = widget->height() - 1;
         if (clip_drawpixelinfo(&clipDPI, dpi, screenPos, width, height))
         {
-            object_draw_preview(object, &clipDPI, width, height);
+            object->DrawPreview(&clipDPI, width, height);
         }
     }
 

src/openrct2-ui/windows/EditorObjectSelection.cpp

@@ -221,7 +221,7 @@ static bool filter_source(const ObjectRepositoryItem* item);
 static bool filter_chunks(const ObjectRepositoryItem* item);
 static void filter_update_counts();
 
-static std::string object_get_description(const void* object);
+static std::string object_get_description(const Object* object);
 static int32_t get_selected_object_type(rct_window* w);
 
 enum
@@ -260,7 +260,7 @@ using sortFunc_t = bool (*)(const list_item&, const list_item&);
 static std::vector<list_item> _listItems;
 static int32_t _listSortType = RIDE_SORT_TYPE;
 static bool _listSortDescending = false;
-static void* _loadedObject = nullptr;
+static std::unique_ptr<Object> _loadedObject;
 
 static void visible_list_dispose()
 {
@@ -414,8 +414,8 @@ static void window_editor_object_selection_close(rct_window* w)
     editor_load_selected_objects();
     editor_object_flags_free();
 
-    object_delete(_loadedObject);
+    if (_loadedObject != nullptr)
-    _loadedObject = nullptr;
+        _loadedObject->Unload();
 
     if (gScreenFlags & SCREEN_FLAGS_EDITOR)
     {
@@ -744,8 +744,8 @@ static void window_editor_object_selection_scroll_mouseover(
     {
         w->selected_list_item = selectedObject;
 
-        object_delete(_loadedObject);
+        if (_loadedObject != nullptr)
-        _loadedObject = nullptr;
+            _loadedObject->Unload();
 
         if (selectedObject == -1)
         {
@@ -1045,7 +1045,7 @@ static void window_editor_object_selection_paint(rct_window* w, rct_drawpixelinf
         int32_t height = widget->height() - 1;
         if (clip_drawpixelinfo(&clipDPI, dpi, screenPos, width, height))
         {
-            object_draw_preview(_loadedObject, &clipDPI, width, height);
+            _loadedObject->DrawPreview(&clipDPI, width, height);
         }
     }
 
@@ -1060,7 +1060,7 @@ static void window_editor_object_selection_paint(rct_window* w, rct_drawpixelinf
     }
 
     // Draw description of object
-    auto description = object_get_description(_loadedObject);
+    auto description = object_get_description(_loadedObject.get());
     if (!description.empty())
     {
         auto ft = Formatter();
@@ -1524,14 +1524,13 @@ static rct_string_id get_ride_type_string_id(const ObjectRepositoryItem* item)
     return result;
 }
 
-static std::string object_get_description(const void* object)
+static std::string object_get_description(const Object* object)
 {
-    const Object* baseObject = static_cast<const Object*>(object);
+    switch (object->GetObjectType())
-    switch (baseObject->GetObjectType())
     {
         case OBJECT_TYPE_RIDE:
         {
-            const RideObject* rideObject = static_cast<const RideObject*>(baseObject);
+            const RideObject* rideObject = static_cast<const RideObject*>(object);
             return rideObject->GetDescription();
         }
         default:

src/openrct2/object/ImageTable.cpp

@@ -193,7 +193,7 @@ std::vector<std::unique_ptr<ImageTable::RequiredImage>> ImageTable::LoadObjectIm
     auto obj = ObjectFactory::CreateObjectFromLegacyFile(context->GetObjectRepository(), objectPath.c_str());
     if (obj != nullptr)
     {
-        auto& imgTable = static_cast<const Object*>(obj)->GetImageTable();
+        auto& imgTable = static_cast<const Object*>(obj.get())->GetImageTable();
         auto numImages = static_cast<int32_t>(imgTable.GetCount());
         auto images = imgTable.GetImages();
         size_t placeHoldersAdded = 0;
@@ -210,7 +210,6 @@ std::vector<std::unique_ptr<ImageTable::RequiredImage>> ImageTable::LoadObjectIm
                 placeHoldersAdded++;
             }
         }
-        delete obj;
 
         // Log place holder information
         if (placeHoldersAdded > 0)

src/openrct2/object/ObjectFactory.cpp

@@ -164,7 +164,7 @@ namespace ObjectFactory
      * @param jRoot Must be JSON node of type object
      * @note jRoot is deliberately left non-const: json_t behaviour changes when const
      */
-    static Object* CreateObjectFromJson(
+    static std::unique_ptr<Object> CreateObjectFromJson(
         IObjectRepository& objectRepository, json_t& jRoot, const IFileDataRetriever* fileRetriever);
 
     static uint8_t ParseSourceGame(const std::string& s)
@@ -183,11 +183,11 @@ namespace ObjectFactory
         return (result != LookupTable.end()) ? result->second : OBJECT_SOURCE_CUSTOM;
     }
 
-    static void ReadObjectLegacy(Object* object, IReadObjectContext* context, OpenRCT2::IStream* stream)
+    static void ReadObjectLegacy(Object& object, IReadObjectContext* context, OpenRCT2::IStream* stream)
     {
         try
         {
-            object->ReadLegacy(context, stream);
+            object.ReadLegacy(context, stream);
         }
         catch (const IOException&)
         {
@@ -200,11 +200,11 @@ namespace ObjectFactory
         }
     }
 
-    Object* CreateObjectFromLegacyFile(IObjectRepository& objectRepository, const utf8* path)
+    std::unique_ptr<Object> CreateObjectFromLegacyFile(IObjectRepository& objectRepository, const utf8* path)
     {
         log_verbose("CreateObjectFromLegacyFile(..., \"%s\")", path);
 
-        Object* result = nullptr;
+        std::unique_ptr<Object> result;
         try
         {
             auto fs = OpenRCT2::FileStream(path, OpenRCT2::FILE_MODE_OPEN);
@@ -225,7 +225,7 @@ namespace ObjectFactory
 
                 auto chunkStream = OpenRCT2::MemoryStream(chunk->GetData(), chunk->GetLength());
                 auto readContext = ReadObjectContext(objectRepository, objectName, !gOpenRCT2NoGraphics, nullptr);
-                ReadObjectLegacy(result, &readContext, &chunkStream);
+                ReadObjectLegacy(*result, &readContext, &chunkStream);
                 if (readContext.WasError())
                 {
                     throw std::runtime_error("Object has errors");
@@ -236,19 +236,17 @@ namespace ObjectFactory
         catch (const std::exception& e)
         {
             log_error("Error: %s when processing object %s", e.what(), path);
-            delete result;
-            result = nullptr;
         }
         return result;
     }
 
-    Object* CreateObjectFromLegacyData(
+    std::unique_ptr<Object> CreateObjectFromLegacyData(
         IObjectRepository& objectRepository, const rct_object_entry* entry, const void* data, size_t dataSize)
     {
         Guard::ArgumentNotNull(entry, GUARD_LINE);
         Guard::ArgumentNotNull(data, GUARD_LINE);
 
-        Object* result = CreateObject(*entry);
+        auto result = CreateObject(*entry);
         if (result != nullptr)
         {
             utf8 objectName[DAT_NAME_LENGTH + 1];
@@ -256,12 +254,11 @@ namespace ObjectFactory
 
             auto readContext = ReadObjectContext(objectRepository, objectName, !gOpenRCT2NoGraphics, nullptr);
             auto chunkStream = OpenRCT2::MemoryStream(data, dataSize);
-            ReadObjectLegacy(result, &readContext, &chunkStream);
+            ReadObjectLegacy(*result, &readContext, &chunkStream);
 
             if (readContext.WasError())
             {
-                delete result;
+                log_error("Error when processing object.");
-                result = nullptr;
             }
             else
             {
@@ -271,52 +268,51 @@ namespace ObjectFactory
         return result;
     }
 
-    Object* CreateObject(const rct_object_entry& entry)
+    std::unique_ptr<Object> CreateObject(const rct_object_entry& entry)
     {
-        Object* result;
+        std::unique_ptr<Object> result;
         switch (entry.GetType())
         {
             case OBJECT_TYPE_RIDE:
-                result = new RideObject(entry);
+                result = std::make_unique<RideObject>(entry);
                 break;
             case OBJECT_TYPE_SMALL_SCENERY:
-                result = new SmallSceneryObject(entry);
+                result = std::make_unique<SmallSceneryObject>(entry);
                 break;
             case OBJECT_TYPE_LARGE_SCENERY:
-                result = new LargeSceneryObject(entry);
+                result = std::make_unique<LargeSceneryObject>(entry);
                 break;
             case OBJECT_TYPE_WALLS:
-                result = new WallObject(entry);
+                result = std::make_unique<WallObject>(entry);
                 break;
             case OBJECT_TYPE_BANNERS:
-                result = new BannerObject(entry);
+                result = std::make_unique<BannerObject>(entry);
                 break;
             case OBJECT_TYPE_PATHS:
-                result = new FootpathObject(entry);
+                result = std::make_unique<FootpathObject>(entry);
                 break;
             case OBJECT_TYPE_PATH_BITS:
-                result = new FootpathItemObject(entry);
+                result = std::make_unique<FootpathItemObject>(entry);
                 break;
             case OBJECT_TYPE_SCENERY_GROUP:
-                result = new SceneryGroupObject(entry);
+                result = std::make_unique<SceneryGroupObject>(entry);
                 break;
             case OBJECT_TYPE_PARK_ENTRANCE:
-                result = new EntranceObject(entry);
+                result = std::make_unique<EntranceObject>(entry);
                 break;
             case OBJECT_TYPE_WATER:
-                result = new WaterObject(entry);
+                result = std::make_unique<WaterObject>(entry);
                 break;
             case OBJECT_TYPE_SCENARIO_TEXT:
-                result = nullptr;
                 break;
             case OBJECT_TYPE_TERRAIN_SURFACE:
-                result = new TerrainSurfaceObject(entry);
+                result = std::make_unique<TerrainSurfaceObject>(entry);
                 break;
             case OBJECT_TYPE_TERRAIN_EDGE:
-                result = new TerrainEdgeObject(entry);
+                result = std::make_unique<TerrainEdgeObject>(entry);
                 break;
             case OBJECT_TYPE_STATION:
-                result = new StationObject(entry);
+                result = std::make_unique<StationObject>(entry);
                 break;
             default:
                 throw std::runtime_error("Invalid object type");
@@ -355,9 +351,8 @@ namespace ObjectFactory
         return 0xFF;
     }
 
-    Object* CreateObjectFromZipFile(IObjectRepository& objectRepository, const std::string_view& path)
+    std::unique_ptr<Object> CreateObjectFromZipFile(IObjectRepository& objectRepository, const std::string_view& path)
     {
-        Object* result = nullptr;
         try
         {
             auto archive = Zip::Open(path, ZIP_ACCESS::READ);
@@ -369,46 +364,38 @@ namespace ObjectFactory
 
             json_t jRoot = Json::FromVector(jsonBytes);
 
-            Object* obj = nullptr;
-
             if (jRoot.is_object())
             {
                 auto fileDataRetriever = ZipDataRetriever(*archive);
-                obj = CreateObjectFromJson(objectRepository, jRoot, &fileDataRetriever);
+                return CreateObjectFromJson(objectRepository, jRoot, &fileDataRetriever);
             }
-
-            return obj;
         }
         catch (const std::exception& e)
         {
             Console::Error::WriteLine("Unable to open or read '%s': %s", path.data(), e.what());
-
-            delete result;
-            result = nullptr;
         }
-        return result;
+        return nullptr;
     }
 
-    Object* CreateObjectFromJsonFile(IObjectRepository& objectRepository, const std::string& path)
+    std::unique_ptr<Object> CreateObjectFromJsonFile(IObjectRepository& objectRepository, const std::string& path)
     {
         log_verbose("CreateObjectFromJsonFile(\"%s\")", path.c_str());
 
-        Object* result = nullptr;
         try
         {
             json_t jRoot = Json::ReadFromFile(path.c_str());
             auto fileDataRetriever = FileSystemDataRetriever(Path::GetDirectory(path));
-            result = CreateObjectFromJson(objectRepository, jRoot, &fileDataRetriever);
+            return CreateObjectFromJson(objectRepository, jRoot, &fileDataRetriever);
         }
         catch (const std::runtime_error& err)
         {
             Console::Error::WriteLine("Unable to open or read '%s': %s", path.c_str(), err.what());
         }
 
-        return result;
+        return nullptr;
     }
 
-    static void ExtractSourceGames(const std::string& id, json_t& jRoot, Object* result)
+    static void ExtractSourceGames(const std::string& id, json_t& jRoot, Object& result)
     {
         auto sourceGames = jRoot["sourceGame"];
         if (sourceGames.is_array() || sourceGames.is_string())
@@ -420,33 +407,34 @@ namespace ObjectFactory
             }
             if (!sourceGameVector.empty())
             {
-                result->SetSourceGames(sourceGameVector);
+                result.SetSourceGames(sourceGameVector);
             }
             else
             {
                 log_error("Object %s has an incorrect sourceGame parameter.", id.c_str());
-                result->SetSourceGames({ OBJECT_SOURCE_CUSTOM });
+                result.SetSourceGames({ OBJECT_SOURCE_CUSTOM });
             }
         }
         // >90% of objects are custom, so allow omitting the parameter without displaying an error.
         else if (sourceGames.is_null())
         {
-            result->SetSourceGames({ OBJECT_SOURCE_CUSTOM });
+            result.SetSourceGames({ OBJECT_SOURCE_CUSTOM });
         }
         else
         {
             log_error("Object %s has an incorrect sourceGame parameter.", id.c_str());
-            result->SetSourceGames({ OBJECT_SOURCE_CUSTOM });
+            result.SetSourceGames({ OBJECT_SOURCE_CUSTOM });
         }
     }
 
-    Object* CreateObjectFromJson(IObjectRepository& objectRepository, json_t& jRoot, const IFileDataRetriever* fileRetriever)
+    std::unique_ptr<Object> CreateObjectFromJson(
+        IObjectRepository& objectRepository, json_t& jRoot, const IFileDataRetriever* fileRetriever)
     {
         Guard::Assert(jRoot.is_object(), "ObjectFactory::CreateObjectFromJson expects parameter jRoot to be object");
 
         log_verbose("CreateObjectFromJson(...)");
 
-        Object* result = nullptr;
+        std::unique_ptr<Object> result;
 
         auto objectType = ParseObjectType(Json::GetString(jRoot["objectType"]));
         if (objectType != 0xFF)
@@ -472,7 +460,6 @@ namespace ObjectFactory
             result->ReadJson(&readContext, jRoot);
             if (readContext.WasError())
             {
-                delete result;
                 throw std::runtime_error("Object has errors");
             }
 
@@ -487,7 +474,7 @@ namespace ObjectFactory
             }
             result->SetAuthors(std::move(authorVector));
 
-            ExtractSourceGames(id, jRoot, result);
+            ExtractSourceGames(id, jRoot, *result);
         }
         return result;
     }

src/openrct2/object/ObjectFactory.h

@@ -11,6 +11,7 @@
 
 #include "../common.h"
 
+#include <memory>
 #include <string_view>
 
 struct IObjectRepository;
@@ -19,11 +20,11 @@ struct rct_object_entry;
 
 namespace ObjectFactory
 {
-    Object* CreateObjectFromLegacyFile(IObjectRepository& objectRepository, const utf8* path);
+    std::unique_ptr<Object> CreateObjectFromLegacyFile(IObjectRepository& objectRepository, const utf8* path);
-    Object* CreateObjectFromLegacyData(
+    std::unique_ptr<Object> CreateObjectFromLegacyData(
         IObjectRepository& objectRepository, const rct_object_entry* entry, const void* data, size_t dataSize);
-    Object* CreateObjectFromZipFile(IObjectRepository& objectRepository, const std::string_view& path);
+    std::unique_ptr<Object> CreateObjectFromZipFile(IObjectRepository& objectRepository, const std::string_view& path);
-    Object* CreateObject(const rct_object_entry& entry);
+    std::unique_ptr<Object> CreateObject(const rct_object_entry& entry);
 
-    Object* CreateObjectFromJsonFile(IObjectRepository& objectRepository, const std::string& path);
+    std::unique_ptr<Object> CreateObjectFromJsonFile(IObjectRepository& objectRepository, const std::string& path);
 } // namespace ObjectFactory

src/openrct2/object/ObjectManager.cpp

@@ -35,7 +35,7 @@ class ObjectManager final : public IObjectManager
 {
 private:
     IObjectRepository& _objectRepository;
-    std::vector<Object*> _loadedObjects;
+    std::vector<std::unique_ptr<Object>> _loadedObjects;
     std::array<std::vector<ObjectEntryIndex>, RIDE_TYPE_COUNT> _rideTypeToObjectMap;
 
     // Used to return a safe empty vector back from GetAllRideEntries, can be removed when std::span is available
@@ -62,7 +62,7 @@ public:
         {
             return nullptr;
         }
-        return _loadedObjects[index];
+        return _loadedObjects[index].get();
     }
 
     Object* GetLoadedObject(int32_t objectType, size_t index) override
@@ -114,14 +114,15 @@ public:
                 int32_t slot = FindSpareSlot(objectType);
                 if (slot != -1)
                 {
-                    loadedObject = GetOrLoadObject(ori);
+                    auto object = GetOrLoadObject(ori);
-                    if (loadedObject != nullptr)
+                    if (object != nullptr)
                     {
                         if (_loadedObjects.size() <= static_cast<size_t>(slot))
                         {
                             _loadedObjects.resize(slot + 1);
                         }
-                        _loadedObjects[slot] = loadedObject;
+                        loadedObject = object.get();
+                        _loadedObjects[slot] = std::move(object);
                         UpdateSceneryGroupIndexes();
                         ResetTypeToRideEntryIndexMap();
                     }
@@ -140,7 +141,7 @@ public:
         size_t numNewLoadedObjects = 0;
         auto loadedObjects = LoadObjects(requiredObjects, &numNewLoadedObjects);
 
-        SetNewLoadedObjectList(loadedObjects);
+        SetNewLoadedObjectList(std::move(loadedObjects));
         LoadDefaultObjects();
         UpdateSceneryGroupIndexes();
         ResetTypeToRideEntryIndexMap();
@@ -177,9 +178,9 @@ public:
 
     void UnloadAll() override
     {
-        for (auto object : _loadedObjects)
+        for (auto& object : _loadedObjects)
         {
-            UnloadObject(object);
+            UnloadObject(object.get());
         }
         UpdateSceneryGroupIndexes();
         ResetTypeToRideEntryIndexMap();
@@ -187,7 +188,7 @@ public:
 
     void ResetObjects() override
     {
-        for (auto loadedObject : _loadedObjects)
+        for (auto& loadedObject : _loadedObjects)
         {
             if (loadedObject != nullptr)
             {
@@ -334,7 +335,8 @@ private:
         Guard::ArgumentNotNull(object, GUARD_LINE);
 
         auto result = std::numeric_limits<size_t>().max();
-        auto it = std::find(_loadedObjects.begin(), _loadedObjects.end(), object);
+        auto it = std::find_if(
+            _loadedObjects.begin(), _loadedObjects.end(), [object](auto& obj) { return obj.get() == object; });
         if (it != _loadedObjects.end())
         {
             result = std::distance(_loadedObjects.begin(), it);
@@ -342,7 +344,7 @@ private:
         return result;
     }
 
-    void SetNewLoadedObjectList(const std::vector<Object*>& newLoadedObjects)
+    void SetNewLoadedObjectList(std::vector<std::unique_ptr<Object>> newLoadedObjects)
     {
         if (newLoadedObjects.empty())
         {
@@ -352,13 +354,15 @@ private:
         {
             UnloadObjectsExcept(newLoadedObjects);
         }
-        _loadedObjects = newLoadedObjects;
+        _loadedObjects = std::move(newLoadedObjects);
     }
 
     void UnloadObject(Object* object)
     {
         if (object != nullptr)
         {
+            object->Unload();
+
             // TODO try to prevent doing a repository search
             const ObjectRepositoryItem* ori = _objectRepository.FindObject(object->GetObjectEntry());
             if (ori != nullptr)
@@ -370,40 +374,37 @@ private:
             // slots, we have to make sure find and set all of them to nullptr
             for (auto& obj : _loadedObjects)
             {
-                if (obj == object)
+                if (obj.get() == object)
                 {
                     obj = nullptr;
                 }
             }
-
-            object->Unload();
-            delete object;
         }
     }
 
-    void UnloadObjectsExcept(const std::vector<Object*>& newLoadedObjects)
+    void UnloadObjectsExcept(const std::vector<std::unique_ptr<Object>>& newLoadedObjects)
     {
         // Build a hash set for quick checking
         auto exceptSet = std::unordered_set<Object*>();
-        for (auto object : newLoadedObjects)
+        for (auto& object : newLoadedObjects)
         {
             if (object != nullptr)
             {
-                exceptSet.insert(object);
+                exceptSet.insert(object.get());
             }
         }
 
         // Unload objects that are not in the hash set
         size_t totalObjectsLoaded = 0;
         size_t numObjectsUnloaded = 0;
-        for (auto object : _loadedObjects)
+        for (auto& object : _loadedObjects)
         {
             if (object != nullptr)
             {
                 totalObjectsLoaded++;
-                if (exceptSet.find(object) == exceptSet.end())
+                if (exceptSet.find(object.get()) == exceptSet.end())
                 {
-                    UnloadObject(object);
+                    UnloadObject(object.get());
                     numObjectsUnloaded++;
                 }
             }
@@ -414,7 +415,7 @@ private:
 
     void UpdateSceneryGroupIndexes()
     {
-        for (auto loadedObject : _loadedObjects)
+        for (auto& loadedObject : _loadedObjects)
         {
             if (loadedObject != nullptr)
             {
@@ -423,26 +424,26 @@ private:
                 {
                     case OBJECT_TYPE_SMALL_SCENERY:
                         sceneryEntry = static_cast<rct_scenery_entry*>(loadedObject->GetLegacyData());
-                        sceneryEntry->small_scenery.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject);
+                        sceneryEntry->small_scenery.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject.get());
                         break;
                     case OBJECT_TYPE_LARGE_SCENERY:
                         sceneryEntry = static_cast<rct_scenery_entry*>(loadedObject->GetLegacyData());
-                        sceneryEntry->large_scenery.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject);
+                        sceneryEntry->large_scenery.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject.get());
                         break;
                     case OBJECT_TYPE_WALLS:
                         sceneryEntry = static_cast<rct_scenery_entry*>(loadedObject->GetLegacyData());
-                        sceneryEntry->wall.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject);
+                        sceneryEntry->wall.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject.get());
                         break;
                     case OBJECT_TYPE_BANNERS:
                         sceneryEntry = static_cast<rct_scenery_entry*>(loadedObject->GetLegacyData());
-                        sceneryEntry->banner.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject);
+                        sceneryEntry->banner.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject.get());
                         break;
                     case OBJECT_TYPE_PATH_BITS:
                         sceneryEntry = static_cast<rct_scenery_entry*>(loadedObject->GetLegacyData());
-                        sceneryEntry->path_bit.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject);
+                        sceneryEntry->path_bit.scenery_tab_id = GetPrimarySceneryGroupEntryIndex(loadedObject.get());
                         break;
                     case OBJECT_TYPE_SCENERY_GROUP:
-                        auto sgObject = dynamic_cast<SceneryGroupObject*>(loadedObject);
+                        auto sgObject = dynamic_cast<SceneryGroupObject*>(loadedObject.get());
                         sgObject->UpdateEntryIndexes();
                         break;
                 }
@@ -507,17 +508,15 @@ private:
             }
             else
             {
-                Object* loadedObject = nullptr;
+                auto loadedObject = ori->LoadedObject;
-                loadedObject = ori->LoadedObject;
                 if (loadedObject == nullptr)
                 {
-                    loadedObject = _objectRepository.LoadObject(ori);
+                    auto object = _objectRepository.LoadObject(ori);
-                    if (loadedObject == nullptr)
+                    if (object == nullptr)
                     {
                         invalidEntries.push_back(entry);
                         ReportObjectLoadProblem(&entry);
                     }
-                    delete loadedObject;
                 }
             }
         }
@@ -577,9 +576,10 @@ private:
         }
     }
 
-    std::vector<Object*> LoadObjects(std::vector<const ObjectRepositoryItem*>& requiredObjects, size_t* outNewObjectsLoaded)
+    std::vector<std::unique_ptr<Object>> LoadObjects(
+        std::vector<const ObjectRepositoryItem*>& requiredObjects, size_t* outNewObjectsLoaded)
     {
-        std::vector<Object*> objects;
+        std::vector<std::unique_ptr<Object>> objects;
         std::vector<Object*> loadedObjects;
         std::vector<rct_object_entry> badObjects;
         objects.resize(OBJECT_ENTRY_COUNT);
@@ -589,14 +589,14 @@ private:
         std::mutex commonMutex;
         ParallelFor(requiredObjects, [this, &commonMutex, requiredObjects, &objects, &badObjects, &loadedObjects](size_t i) {
             auto ori = requiredObjects[i];
-            Object* loadedObject = nullptr;
+            std::unique_ptr<Object> object;
             if (ori != nullptr)
             {
-                loadedObject = ori->LoadedObject;
+                auto loadedObject = ori->LoadedObject;
                 if (loadedObject == nullptr)
                 {
-                    loadedObject = _objectRepository.LoadObject(ori);
+                    object = _objectRepository.LoadObject(ori);
-                    if (loadedObject == nullptr)
+                    if (object == nullptr)
                     {
                         std::lock_guard<std::mutex> guard(commonMutex);
                         badObjects.push_back(ori->ObjectEntry);
@@ -605,13 +605,13 @@ private:
                     else
                     {
                         std::lock_guard<std::mutex> guard(commonMutex);
-                        loadedObjects.push_back(loadedObject);
+                        loadedObjects.push_back(object.get());
                         // Connect the ori to the registered object
-                        _objectRepository.RegisterLoadedObject(ori, loadedObject);
+                        _objectRepository.RegisterLoadedObject(ori, object.get());
                     }
                 }
             }
-            objects[i] = loadedObject;
+            objects[i] = std::move(object);
         });
 
         // Load objects
@@ -637,22 +637,23 @@ private:
         return objects;
     }
 
-    Object* GetOrLoadObject(const ObjectRepositoryItem* ori)
+    std::unique_ptr<Object> GetOrLoadObject(const ObjectRepositoryItem* ori)
     {
-        Object* loadedObject = ori->LoadedObject;
+        std::unique_ptr<Object> object;
+        auto loadedObject = ori->LoadedObject;
         if (loadedObject == nullptr)
         {
             // Try to load object
-            loadedObject = _objectRepository.LoadObject(ori);
+            object = _objectRepository.LoadObject(ori);
-            if (loadedObject != nullptr)
+            if (object != nullptr)
             {
-                loadedObject->Load();
+                object->Load();
 
                 // Connect the ori to the registered object
-                _objectRepository.RegisterLoadedObject(ori, loadedObject);
+                _objectRepository.RegisterLoadedObject(ori, object.get());
             }
         }
-        return loadedObject;
+        return object;
     }
 
     void ResetTypeToRideEntryIndexMap()
@@ -717,18 +718,18 @@ std::unique_ptr<IObjectManager> CreateObjectManager(IObjectRepository& objectRep
     return std::make_unique<ObjectManager>(objectRepository);
 }
 
-void* object_manager_get_loaded_object_by_index(size_t index)
+Object* object_manager_get_loaded_object_by_index(size_t index)
 {
     auto& objectManager = OpenRCT2::GetContext()->GetObjectManager();
     Object* loadedObject = objectManager.GetLoadedObject(index);
-    return static_cast<void*>(loadedObject);
+    return loadedObject;
 }
 
-void* object_manager_get_loaded_object(const rct_object_entry* entry)
+Object* object_manager_get_loaded_object(const rct_object_entry* entry)
 {
     auto& objectManager = OpenRCT2::GetContext()->GetObjectManager();
     Object* loadedObject = objectManager.GetLoadedObject(entry);
-    return static_cast<void*>(loadedObject);
+    return loadedObject;
 }
 
 ObjectEntryIndex object_manager_get_loaded_object_entry_index(const void* loadedObject)

src/openrct2/object/ObjectManager.h

@@ -44,8 +44,8 @@ struct IObjectManager
 
 std::unique_ptr<IObjectManager> CreateObjectManager(IObjectRepository& objectRepository);
 
-void* object_manager_get_loaded_object_by_index(size_t index);
+Object* object_manager_get_loaded_object_by_index(size_t index);
-void* object_manager_get_loaded_object(const rct_object_entry* entry);
+Object* object_manager_get_loaded_object(const rct_object_entry* entry);
 ObjectEntryIndex object_manager_get_loaded_object_entry_index(const void* loadedObject);
 void* object_manager_load_object(const rct_object_entry* entry);
 void object_manager_unload_objects(const std::vector<rct_object_entry>& entries);

src/openrct2/object/ObjectRepository.cpp

@@ -94,7 +94,7 @@ public:
 public:
     std::tuple<bool, ObjectRepositoryItem> Create([[maybe_unused]] int32_t language, const std::string& path) const override
     {
-        Object* object = nullptr;
+        std::unique_ptr<Object> object;
         auto extension = Path::GetExtension(path);
         if (String::Equals(extension, ".json", true))
         {
@@ -117,7 +117,6 @@ public:
             item.Authors = object->GetAuthors();
             item.Sources = object->GetSourceGames();
             object->SetRepositoryItem(&item);
-            delete object;
             return std::make_tuple(true, item);
         }
         return std::make_tuple(false, ObjectRepositoryItem());
@@ -290,7 +289,7 @@ public:
         return nullptr;
     }
 
-    Object* LoadObject(const ObjectRepositoryItem* ori) override
+    std::unique_ptr<Object> LoadObject(const ObjectRepositoryItem* ori) override
     {
         Guard::ArgumentNotNull(ori, GUARD_LINE);
 
@@ -332,7 +331,7 @@ public:
         object_entry_get_name_fixed(objectName, sizeof(objectName), objectEntry);
 
         // Check that the object is loadable before writing it
-        Object* object = ObjectFactory::CreateObjectFromLegacyData(*this, objectEntry, data, dataSize);
+        auto object = ObjectFactory::CreateObjectFromLegacyData(*this, objectEntry, data, dataSize);
         if (object == nullptr)
         {
             Console::Error::WriteLine("[%s] Unable to export object.", objectName);
@@ -694,9 +693,9 @@ const rct_object_entry* object_list_find(rct_object_entry* entry)
     return result;
 }
 
-void* object_repository_load_object(const rct_object_entry* objectEntry)
+std::unique_ptr<Object> object_repository_load_object(const rct_object_entry* objectEntry)
 {
-    Object* object = nullptr;
+    std::unique_ptr<Object> object;
     auto& objRepository = GetContext()->GetObjectRepository();
     const ObjectRepositoryItem* ori = objRepository.FindObject(objectEntry);
     if (ori != nullptr)
@@ -707,7 +706,7 @@ void* object_repository_load_object(const rct_object_entry* objectEntry)
             object->Load();
         }
     }
-    return static_cast<void*>(object);
+    return object;
 }
 
 void scenario_translate(scenario_index_entry* scenarioEntry)
@@ -750,22 +749,6 @@ const ObjectRepositoryItem* object_repository_find_object_by_name(const char* na
     return objectRepository.FindObject(name);
 }
 
-void object_delete(void* object)
-{
-    if (object != nullptr)
-    {
-        Object* baseObject = static_cast<Object*>(object);
-        baseObject->Unload();
-        delete baseObject;
-    }
-}
-
-void object_draw_preview(const void* object, rct_drawpixelinfo* dpi, int32_t width, int32_t height)
-{
-    const Object* baseObject = static_cast<const Object*>(object);
-    baseObject->DrawPreview(dpi, width, height);
-}
-
 bool object_entry_compare(const rct_object_entry* a, const rct_object_entry* b)
 {
     // If an official object don't bother checking checksum

src/openrct2/object/ObjectRepository.h

@@ -74,7 +74,7 @@ struct IObjectRepository
     virtual const ObjectRepositoryItem* FindObject(const std::string_view& legacyIdentifier) const abstract;
     virtual const ObjectRepositoryItem* FindObject(const rct_object_entry* objectEntry) const abstract;
 
-    virtual Object* LoadObject(const ObjectRepositoryItem* ori) abstract;
+    virtual std::unique_ptr<Object> LoadObject(const ObjectRepositoryItem* ori) abstract;
     virtual void RegisterLoadedObject(const ObjectRepositoryItem* ori, Object* object) abstract;
     virtual void UnregisterLoadedObject(const ObjectRepositoryItem* ori, Object* object) abstract;
 
@@ -93,7 +93,4 @@ size_t object_repository_get_items_count();
 const ObjectRepositoryItem* object_repository_get_items();
 const ObjectRepositoryItem* object_repository_find_object_by_entry(const rct_object_entry* entry);
 const ObjectRepositoryItem* object_repository_find_object_by_name(const char* name);
-void* object_repository_load_object(const rct_object_entry* objectEntry);
+std::unique_ptr<Object> object_repository_load_object(const rct_object_entry* objectEntry);
-
-void object_delete(void* object);
-void object_draw_preview(const void* object, rct_drawpixelinfo* dpi, int32_t width, int32_t height);

src/openrct2/rct1/S4Importer.cpp

@@ -1983,13 +1983,12 @@ private:
             }
             else
             {
-                Object* object = objectRepository.LoadObject(ori);
+                auto object = objectRepository.LoadObject(ori);
                 if (object == nullptr && objectType != OBJECT_TYPE_SCENERY_GROUP)
                 {
                     missingObjects.push_back(entry);
                     Console::Error::WriteLine("[%s] Object could not be loaded.", objectName);
                 }
-                delete object;
             }
         }
     }

src/openrct2/rct2/T6Importer.cpp

@@ -217,16 +217,16 @@ public:
         if (RCT2RideTypeNeedsConversion(td->type))
         {
             std::scoped_lock<std::mutex> lock(_objectLookupMutex);
-            auto* rawObject = object_repository_load_object(&td->vehicle_object);
+            auto rawObject = object_repository_load_object(&td->vehicle_object);
             if (rawObject != nullptr)
             {
                 const auto* rideEntry = static_cast<const rct_ride_entry*>(
-                    static_cast<RideObject*>(rawObject)->GetLegacyData());
+                    static_cast<RideObject*>(rawObject.get())->GetLegacyData());
                 if (rideEntry != nullptr)
                 {
                     td->type = RCT2RideTypeToOpenRCT2RideType(td->type, rideEntry);
                 }
-                object_delete(rawObject);
+                rawObject->Unload();
             }
         }
     }