From 6011478590949b219fa08c9c18b14f9a247f97e3 Mon Sep 17 00:00:00 2001 From: Hielke Morsink Date: Fri, 20 Aug 2021 23:38:15 +0200 Subject: [PATCH 1/3] Directly use std::unique_ptr prvalues This removes the unnecessary constructor calls and improves flexibility in case of typename changes. All cases, except for the one in Context.cpp, are temporaries. --- src/openrct2-ui/scripting/ScTitleSequence.hpp | 3 +-- src/openrct2-ui/windows/TitleEditor.cpp | 2 +- src/openrct2/Context.cpp | 2 +- src/openrct2/cmdline/RootCommands.cpp | 3 +-- src/openrct2/config/Config.cpp | 8 +++----- src/openrct2/title/TitleSequence.cpp | 4 ++-- 6 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/openrct2-ui/scripting/ScTitleSequence.hpp b/src/openrct2-ui/scripting/ScTitleSequence.hpp index 4e5d889015..5b158d176b 100644 --- a/src/openrct2-ui/scripting/ScTitleSequence.hpp +++ b/src/openrct2-ui/scripting/ScTitleSequence.hpp @@ -11,7 +11,6 @@ #ifdef ENABLE_SCRIPTING -# include # include # include # include @@ -209,7 +208,7 @@ namespace OpenRCT2::Scripting try { auto& objectMgr = GetContext()->GetObjectManager(); - auto parkImporter = std::unique_ptr(ParkImporter::Create(handle->HintPath)); + auto parkImporter = ParkImporter::Create(handle->HintPath); auto result = parkImporter->LoadFromStream(handle->Stream.get(), isScenario); objectMgr.LoadObjects(result.RequiredObjects.data(), result.RequiredObjects.size()); parkImporter->Import(); diff --git a/src/openrct2-ui/windows/TitleEditor.cpp b/src/openrct2-ui/windows/TitleEditor.cpp index 78b48d5211..4b9e4fe2a8 100644 --- a/src/openrct2-ui/windows/TitleEditor.cpp +++ b/src/openrct2-ui/windows/TitleEditor.cpp @@ -351,7 +351,7 @@ static void window_title_editor_mouseup(rct_window* w, rct_widgetindex widgetInd try { auto& objectMgr = OpenRCT2::GetContext()->GetObjectManager(); - auto parkImporter = std::unique_ptr(ParkImporter::Create(handle->HintPath)); + auto parkImporter = ParkImporter::Create(handle->HintPath); auto result = parkImporter->LoadFromStream(handle->Stream.get(), isScenario); objectMgr.LoadObjects(result.RequiredObjects.data(), result.RequiredObjects.size()); parkImporter->Import(); diff --git a/src/openrct2/Context.cpp b/src/openrct2/Context.cpp index d0f739970e..59438d3054 100644 --- a/src/openrct2/Context.cpp +++ b/src/openrct2/Context.cpp @@ -524,7 +524,7 @@ namespace OpenRCT2 { drawingEngine->Initialise(); drawingEngine->SetVSync(gConfigGeneral.use_vsync); - _drawingEngine = std::unique_ptr(std::move(drawingEngine)); + _drawingEngine = std::move(drawingEngine); } catch (const std::exception& ex) { diff --git a/src/openrct2/cmdline/RootCommands.cpp b/src/openrct2/cmdline/RootCommands.cpp index b074e0b785..60782dd77a 100644 --- a/src/openrct2/cmdline/RootCommands.cpp +++ b/src/openrct2/cmdline/RootCommands.cpp @@ -26,7 +26,6 @@ #include #include -#include #include #ifdef USE_BREAKPAD @@ -406,7 +405,7 @@ static exitcode_t HandleCommandScanObjects([[maybe_unused]] CommandLineArgEnumer gOpenRCT2Headless = true; gOpenRCT2NoGraphics = true; - auto context = std::unique_ptr(OpenRCT2::CreateContext()); + auto context = OpenRCT2::CreateContext(); auto env = context->GetPlatformEnvironment(); auto objectRepository = CreateObjectRepository(env); objectRepository->Construct(gConfigGeneral.language); diff --git a/src/openrct2/config/Config.cpp b/src/openrct2/config/Config.cpp index 8e2a232172..783697a7b7 100644 --- a/src/openrct2/config/Config.cpp +++ b/src/openrct2/config/Config.cpp @@ -37,8 +37,6 @@ #include "IniReader.hpp" #include "IniWriter.hpp" -#include - using namespace OpenRCT2; using namespace OpenRCT2::Ui; @@ -565,7 +563,7 @@ namespace Config { try { - auto reader = std::unique_ptr(CreateDefaultIniReader()); + auto reader = CreateDefaultIniReader(); ReadGeneral(reader.get()); ReadInterface(reader.get()); ReadSound(reader.get()); @@ -586,7 +584,7 @@ namespace Config try { auto fs = FileStream(path, FILE_MODE_OPEN); - auto reader = std::unique_ptr(CreateIniReader(&fs)); + auto reader = CreateIniReader(&fs); ReadGeneral(reader.get()); ReadInterface(reader.get()); ReadSound(reader.get()); @@ -610,7 +608,7 @@ namespace Config Path::CreateDirectory(directory); auto fs = FileStream(path, FILE_MODE_WRITE); - auto writer = std::unique_ptr(CreateIniWriter(&fs)); + auto writer = CreateIniWriter(&fs); WriteGeneral(writer.get()); WriteInterface(writer.get()); WriteSound(writer.get()); diff --git a/src/openrct2/title/TitleSequence.cpp b/src/openrct2/title/TitleSequence.cpp index c5e47ca72a..66b2b39515 100644 --- a/src/openrct2/title/TitleSequence.cpp +++ b/src/openrct2/title/TitleSequence.cpp @@ -53,7 +53,7 @@ std::unique_ptr LoadTitleSequence(const std::string& path) auto ext = Path::GetExtension(path); if (String::Equals(ext, TITLE_SEQUENCE_EXTENSION)) { - auto zip = std::unique_ptr(Zip::TryOpen(path, ZIP_ACCESS::READ)); + auto zip = Zip::TryOpen(path, ZIP_ACCESS::READ); if (zip == nullptr) { Console::Error::WriteLine("Unable to open '%s'", path.c_str()); @@ -103,7 +103,7 @@ std::unique_ptr TitleSequenceGetParkHandle(const TitleS const auto& filename = seq.Saves[index]; if (seq.IsZip) { - auto zip = std::unique_ptr(Zip::TryOpen(seq.Path, ZIP_ACCESS::READ)); + auto zip = Zip::TryOpen(seq.Path, ZIP_ACCESS::READ); if (zip != nullptr) { auto data = zip->GetFileData(filename); From cef26400cf3a7c55a91b3f5e3ea0ef5001557504 Mon Sep 17 00:00:00 2001 From: Hielke Morsink Date: Fri, 20 Aug 2021 23:42:03 +0200 Subject: [PATCH 2/3] Use std::make_unique instead of new for arrays --- src/openrct2/CmdlineSprite.cpp | 2 +- src/openrct2/core/DataSerialiserTraits.h | 2 +- src/openrct2/rct12/SawyerChunkReader.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/openrct2/CmdlineSprite.cpp b/src/openrct2/CmdlineSprite.cpp index 8d44a95546..a0ecfed2db 100644 --- a/src/openrct2/CmdlineSprite.cpp +++ b/src/openrct2/CmdlineSprite.cpp @@ -185,7 +185,7 @@ bool SpriteFile::Save(const utf8* path) static bool SpriteImageExport(const rct_g1_element& spriteElement, const char* outPath) { const auto pixelBufferSize = spriteElement.width * spriteElement.height; - std::unique_ptr pixelBuffer(new uint8_t[pixelBufferSize]); + auto pixelBuffer = std::make_unique(pixelBufferSize); auto pixels = pixelBuffer.get(); std::fill_n(pixels, pixelBufferSize, 0x00); diff --git a/src/openrct2/core/DataSerialiserTraits.h b/src/openrct2/core/DataSerialiserTraits.h index 4fdcbaf464..7284f69851 100644 --- a/src/openrct2/core/DataSerialiserTraits.h +++ b/src/openrct2/core/DataSerialiserTraits.h @@ -284,7 +284,7 @@ template<> struct DataSerializerTraits_t uint32_t length = 0; s.decode(stream, length); - std::unique_ptr buf(new uint8_t[length]); + auto buf = std::make_unique(length); stream->Read(buf.get(), length); val.Write(buf.get(), length); diff --git a/src/openrct2/rct12/SawyerChunkReader.cpp b/src/openrct2/rct12/SawyerChunkReader.cpp index cf03dfdc5b..b8240009f6 100644 --- a/src/openrct2/rct12/SawyerChunkReader.cpp +++ b/src/openrct2/rct12/SawyerChunkReader.cpp @@ -65,7 +65,7 @@ std::shared_ptr SawyerChunkReader::ReadChunk() case CHUNK_ENCODING_RLECOMPRESSED: case CHUNK_ENCODING_ROTATE: { - std::unique_ptr compressedData(new uint8_t[header.length]); + auto compressedData = std::make_unique(header.length); if (_stream->TryRead(compressedData.get(), header.length) != header.length) { throw SawyerChunkException(EXCEPTION_MSG_CORRUPT_CHUNK_SIZE); From be4159f9ac6adf33d0ea5b34b451584da77efb14 Mon Sep 17 00:00:00 2001 From: Hielke Morsink Date: Fri, 20 Aug 2021 23:47:16 +0200 Subject: [PATCH 3/3] Create std::unique_ptrinstead of raw pointer --- src/openrct2/localisation/LanguagePack.cpp | 13 +++++++------ src/openrct2/localisation/LanguagePack.h | 5 +++-- src/openrct2/localisation/LocalisationService.cpp | 5 ++--- test/tests/LanguagePackTest.cpp | 12 ++++-------- 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/openrct2/localisation/LanguagePack.cpp b/src/openrct2/localisation/LanguagePack.cpp index b17e29017d..4f75eab211 100644 --- a/src/openrct2/localisation/LanguagePack.cpp +++ b/src/openrct2/localisation/LanguagePack.cpp @@ -20,6 +20,7 @@ #include "Localisation.h" #include +#include #include #include @@ -62,7 +63,7 @@ private: ScenarioOverride* _currentScenarioOverride = nullptr; public: - static LanguagePack* FromFile(uint16_t id, const utf8* path) + static std::unique_ptr FromFile(uint16_t id, const utf8* path) { Guard::ArgumentNotNull(path); @@ -90,15 +91,15 @@ public: } // Parse the memory as text - LanguagePack* result = FromText(id, fileData); + auto result = FromText(id, fileData); Memory::Free(fileData); return result; } - static LanguagePack* FromText(uint16_t id, const utf8* text) + static std::unique_ptr FromText(uint16_t id, const utf8* text) { - return new LanguagePack(id, text); + return std::make_unique(id, text); } LanguagePack(uint16_t id, const utf8* text) @@ -579,13 +580,13 @@ private: namespace LanguagePackFactory { - ILanguagePack* FromFile(uint16_t id, const utf8* path) + std::unique_ptr FromFile(uint16_t id, const utf8* path) { auto languagePack = LanguagePack::FromFile(id, path); return languagePack; } - ILanguagePack* FromText(uint16_t id, const utf8* text) + std::unique_ptr FromText(uint16_t id, const utf8* text) { auto languagePack = LanguagePack::FromText(id, text); return languagePack; diff --git a/src/openrct2/localisation/LanguagePack.h b/src/openrct2/localisation/LanguagePack.h index e6301b9135..ef3a17bbbf 100644 --- a/src/openrct2/localisation/LanguagePack.h +++ b/src/openrct2/localisation/LanguagePack.h @@ -11,6 +11,7 @@ #include "../common.h" +#include #include #include @@ -30,6 +31,6 @@ struct ILanguagePack namespace LanguagePackFactory { - ILanguagePack* FromFile(uint16_t id, const utf8* path); - ILanguagePack* FromText(uint16_t id, const utf8* text); + std::unique_ptr FromFile(uint16_t id, const utf8* path); + std::unique_ptr FromText(uint16_t id, const utf8* text); } // namespace LanguagePackFactory diff --git a/src/openrct2/localisation/LocalisationService.cpp b/src/openrct2/localisation/LocalisationService.cpp index 15a1a23db6..965e177233 100644 --- a/src/openrct2/localisation/LocalisationService.cpp +++ b/src/openrct2/localisation/LocalisationService.cpp @@ -86,12 +86,11 @@ void LocalisationService::OpenLanguage(int32_t id) if (id != LANGUAGE_ENGLISH_UK) { filename = GetLanguagePath(LANGUAGE_ENGLISH_UK); - _languageFallback = std::unique_ptr( - LanguagePackFactory::FromFile(LANGUAGE_ENGLISH_UK, filename.c_str())); + _languageFallback = LanguagePackFactory::FromFile(LANGUAGE_ENGLISH_UK, filename.c_str()); } filename = GetLanguagePath(id); - _languageCurrent = std::unique_ptr(LanguagePackFactory::FromFile(id, filename.c_str())); + _languageCurrent = LanguagePackFactory::FromFile(id, filename.c_str()); if (_languageCurrent != nullptr) { _currentLanguage = id; diff --git a/test/tests/LanguagePackTest.cpp b/test/tests/LanguagePackTest.cpp index b6b3a0206f..d3a969e521 100644 --- a/test/tests/LanguagePackTest.cpp +++ b/test/tests/LanguagePackTest.cpp @@ -23,27 +23,25 @@ protected: TEST_F(LanguagePackTest, create_empty) { - ILanguagePack* empty = LanguagePackFactory::FromText(0, ""); + auto empty = LanguagePackFactory::FromText(0, ""); ASSERT_EQ(empty->GetId(), 0); ASSERT_EQ(empty->GetCount(), 0U); - delete empty; } TEST_F(LanguagePackTest, create_mutable_id_1) { - ILanguagePack* lang = LanguagePackFactory::FromText(1, "STR_0000:\n"); + auto lang = LanguagePackFactory::FromText(1, "STR_0000:\n"); ASSERT_EQ(lang->GetId(), 1); ASSERT_EQ(lang->GetCount(), 1U); ASSERT_STREQ(lang->GetString(0), nullptr); lang->SetString(0, "xx"); ASSERT_EQ(lang->GetCount(), 1U); ASSERT_STREQ(lang->GetString(0), "xx"); - delete lang; } TEST_F(LanguagePackTest, language_pack_simple) { - ILanguagePack* lang = LanguagePackFactory::FromText(0, LanguageEnGB); + auto lang = LanguagePackFactory::FromText(0, LanguageEnGB); ASSERT_EQ(lang->GetId(), 0); ASSERT_EQ(lang->GetCount(), 4U); ASSERT_STREQ(lang->GetString(2), "Spiral Roller Coaster"); @@ -55,12 +53,11 @@ TEST_F(LanguagePackTest, language_pack_simple) ASSERT_EQ(lang->GetString(1000), nullptr); ASSERT_EQ(lang->GetScenarioOverrideStringId("No such park", 0), STR_NONE); ASSERT_EQ(lang->GetObjectOverrideStringId(" ", 0), STR_NONE); - delete lang; } TEST_F(LanguagePackTest, language_pack_multibyte) { - ILanguagePack* lang = LanguagePackFactory::FromText(0, (const utf8*)LanguageZhTW); + auto lang = LanguagePackFactory::FromText(0, (const utf8*)LanguageZhTW); ASSERT_EQ(lang->GetId(), 0); ASSERT_EQ(lang->GetCount(), 4U); ASSERT_STREQ(lang->GetString(2), u8"懸吊式雲霄飛車"); @@ -70,7 +67,6 @@ TEST_F(LanguagePackTest, language_pack_multibyte) ASSERT_STREQ(lang->GetString(0x7002), u8"在隱藏於森林深處的清空範圍中, 建造一個很受歡迎的樂園"); ASSERT_EQ(lang->GetObjectOverrideStringId("CONDORRD", 0), 0x6000); ASSERT_STREQ(lang->GetString(0x6000), u8"神鷹暢遊"); - delete lang; } const utf8* LanguagePackTest::LanguageEnGB = "# STR_XXXX part is read and XXXX becomes the string id number.\n"