From ccb6eb06f810e6968deaa2d14e9f88c8ec7ac295 Mon Sep 17 00:00:00 2001 From: Matt Date: Sun, 27 Sep 2020 15:55:29 +0300 Subject: [PATCH] Use unique_ptr for TitleSequence instances Co-authored-by: Gabriel Guedes --- src/openrct2-ui/title/TitleSequencePlayer.cpp | 11 +- .../windows/TitleCommandEditor.cpp | 3 +- src/openrct2-ui/windows/TitleEditor.cpp | 34 ++--- src/openrct2/title/TitleSequence.cpp | 132 +++++++++--------- src/openrct2/title/TitleSequence.h | 32 +++-- src/openrct2/title/TitleSequenceManager.cpp | 6 +- 6 files changed, 108 insertions(+), 110 deletions(-) diff --git a/src/openrct2-ui/title/TitleSequencePlayer.cpp b/src/openrct2-ui/title/TitleSequencePlayer.cpp index 9d6608d568..5238853986 100644 --- a/src/openrct2-ui/title/TitleSequencePlayer.cpp +++ b/src/openrct2-ui/title/TitleSequencePlayer.cpp @@ -47,7 +47,7 @@ class TitleSequencePlayer final : public ITitleSequencePlayer private: GameState& _gameState; - TitleSequence* _sequence = nullptr; + std::unique_ptr _sequence; int32_t _position = 0; int32_t _waitCounter = 0; @@ -73,7 +73,10 @@ public: void Eject() override { - FreeTitleSequence(_sequence); + if (_sequence == nullptr) + return; + + FreeTitleSequence(*_sequence); _sequence = nullptr; } @@ -93,7 +96,7 @@ public: } Eject(); - _sequence = sequence; + _sequence = std::move(sequence); Reset(); return true; @@ -277,7 +280,7 @@ private: { bool loadSuccess = false; uint8_t saveIndex = command->SaveIndex; - TitleSequenceParkHandle* parkHandle = TitleSequenceGetParkHandle(_sequence, saveIndex); + TitleSequenceParkHandle* parkHandle = TitleSequenceGetParkHandle(*_sequence, saveIndex); if (parkHandle != nullptr) { loadSuccess = LoadParkFromStream(static_cast(parkHandle->Stream), parkHandle->HintPath); diff --git a/src/openrct2-ui/windows/TitleCommandEditor.cpp b/src/openrct2-ui/windows/TitleCommandEditor.cpp index 33f0e4c3a4..03a588d81a 100644 --- a/src/openrct2-ui/windows/TitleCommandEditor.cpp +++ b/src/openrct2-ui/windows/TitleCommandEditor.cpp @@ -340,9 +340,8 @@ static void window_title_command_editor_mouseup(rct_window* w, rct_widgetindex w else { _sequence->Commands[_window_title_command_editor_index] = command; - TitleSequenceSave(_sequence); } - TitleSequenceSave(_sequence); + TitleSequenceSave(*_sequence); rct_window* title_editor_w = window_find_by_class(WC_TITLE_EDITOR); if (title_editor_w != nullptr) diff --git a/src/openrct2-ui/windows/TitleEditor.cpp b/src/openrct2-ui/windows/TitleEditor.cpp index 51e8405725..17531c3f44 100644 --- a/src/openrct2-ui/windows/TitleEditor.cpp +++ b/src/openrct2-ui/windows/TitleEditor.cpp @@ -173,7 +173,7 @@ static rct_widget window_title_editor_widgets[] = { static size_t _selectedTitleSequence = SIZE_MAX; static bool _isSequenceReadOnly; -static TitleSequence * _editingTitleSequence = nullptr; +static std::unique_ptr _editingTitleSequence; static const utf8 * _sequenceName; static utf8 * _renameSavePath = nullptr; @@ -255,7 +255,7 @@ static void window_title_editor_close(rct_window* w) // Close the related windows window_close_by_class(WC_TITLE_COMMAND_EDITOR); - FreeTitleSequence(_editingTitleSequence); + FreeTitleSequence(*_editingTitleSequence); _editingTitleSequence = nullptr; _sequenceName = nullptr; @@ -322,7 +322,7 @@ static void window_title_editor_mouseup(rct_window* w, rct_widgetindex widgetInd { if (w->selected_list_item != -1) { - TitleSequenceRemovePark(_editingTitleSequence, w->selected_list_item); + TitleSequenceRemovePark(*_editingTitleSequence, w->selected_list_item); if (w->selected_list_item >= static_cast(_editingTitleSequence->NumSaves)) { w->selected_list_item--; @@ -344,7 +344,7 @@ static void window_title_editor_mouseup(rct_window* w, rct_widgetindex widgetInd case WIDX_TITLE_EDITOR_LOAD_SAVE: if (w->selected_list_item >= 0 && w->selected_list_item < static_cast(_editingTitleSequence->NumSaves)) { - auto handle = TitleSequenceGetParkHandle(_editingTitleSequence, w->selected_list_item); + auto handle = TitleSequenceGetParkHandle(*_editingTitleSequence, w->selected_list_item); auto stream = static_cast(handle->Stream); auto hintPath = String::ToStd(handle->HintPath); bool isScenario = ParkImporter::ExtensionIsScenario(hintPath); @@ -378,10 +378,10 @@ static void window_title_editor_mouseup(rct_window* w, rct_widgetindex widgetInd if (window_title_editor_check_can_edit()) { if (w->selected_list_item != -1) - window_title_command_editor_open(_editingTitleSequence, w->selected_list_item + 1, true); + window_title_command_editor_open(_editingTitleSequence.get(), w->selected_list_item + 1, true); else window_title_command_editor_open( - _editingTitleSequence, static_cast(_editingTitleSequence->NumCommands), true); + _editingTitleSequence.get(), static_cast(_editingTitleSequence->NumCommands), true); } break; case WIDX_TITLE_EDITOR_EDIT: @@ -390,7 +390,7 @@ static void window_title_editor_mouseup(rct_window* w, rct_widgetindex widgetInd if (w->selected_list_item != -1 && w->selected_list_item < static_cast(_editingTitleSequence->NumCommands)) { - window_title_command_editor_open(_editingTitleSequence, w->selected_list_item, false); + window_title_command_editor_open(_editingTitleSequence.get(), w->selected_list_item, false); } } break; @@ -410,7 +410,7 @@ static void window_title_editor_mouseup(rct_window* w, rct_widgetindex widgetInd { w->selected_list_item--; } - TitleSequenceSave(_editingTitleSequence); + TitleSequenceSave(*_editingTitleSequence); } } break; @@ -438,7 +438,7 @@ static void window_title_editor_mouseup(rct_window* w, rct_widgetindex widgetInd *a = *b; *b = tmp; w->selected_list_item++; - TitleSequenceSave(_editingTitleSequence); + TitleSequenceSave(*_editingTitleSequence); } } break; @@ -454,7 +454,7 @@ static void window_title_editor_mouseup(rct_window* w, rct_widgetindex widgetInd *b = *a; *a = tmp; w->selected_list_item--; - TitleSequenceSave(_editingTitleSequence); + TitleSequenceSave(*_editingTitleSequence); } } break; @@ -1059,7 +1059,7 @@ static void window_title_editor_load_sequence(size_t index) return; const char* path = title_sequence_manager_get_path(index); - TitleSequence* titleSequence = LoadTitleSequence(path); + auto titleSequence = LoadTitleSequence(path); if (titleSequence == nullptr) { context_show_error(STR_FAILED_TO_LOAD_FILE_CONTAINS_INVALID_DATA, STR_NONE, {}); @@ -1070,8 +1070,8 @@ static void window_title_editor_load_sequence(size_t index) size_t predefinedIndex = title_sequence_manager_get_predefined_index(index); _isSequenceReadOnly = (predefinedIndex != SIZE_MAX); _sequenceName = title_sequence_manager_get_name(index); - FreeTitleSequence(_editingTitleSequence); - _editingTitleSequence = titleSequence; + FreeTitleSequence(*_editingTitleSequence); + _editingTitleSequence = std::move(titleSequence); window_close_by_class(WC_TITLE_COMMAND_EDITOR); } @@ -1099,7 +1099,7 @@ static bool window_title_editor_check_can_edit() static bool save_filename_exists(const utf8* filename) { - TitleSequence* seq = _editingTitleSequence; + auto& seq = _editingTitleSequence; for (size_t i = 0; i < seq->NumSaves; i++) { const utf8* savePath = seq->Saves[i]; @@ -1128,7 +1128,7 @@ static void window_title_editor_add_park_callback(int32_t result, const utf8* pa return; } - TitleSequenceAddPark(_editingTitleSequence, path, filename); + TitleSequenceAddPark(*_editingTitleSequence, path, filename); } static void window_title_editor_rename_park(size_t index, const utf8* name) @@ -1152,8 +1152,8 @@ static void window_title_editor_rename_park(size_t index, const utf8* name) } } - if (TitleSequenceRenamePark(_editingTitleSequence, index, name)) + if (TitleSequenceRenamePark(*_editingTitleSequence, index, name)) { - TitleSequenceSave(_editingTitleSequence); + TitleSequenceSave(*_editingTitleSequence); } } diff --git a/src/openrct2/title/TitleSequence.cpp b/src/openrct2/title/TitleSequence.cpp index 6054343d16..fb3bc1f613 100644 --- a/src/openrct2/title/TitleSequence.cpp +++ b/src/openrct2/title/TitleSequence.cpp @@ -1,4 +1,4 @@ -/***************************************************************************** +/***************************************************************************** * Copyright (c) 2014-2020 OpenRCT2 developers * * For a complete list of all authors, please refer to contributors.md @@ -35,16 +35,14 @@ static std::vector GetSaves(IZipArchive* zip); static std::vector LegacyScriptRead(utf8* script, size_t scriptLength, std::vector saves); static void LegacyScriptGetLine(OpenRCT2::IStream* stream, char* parts); static std::vector ReadScriptFile(const utf8* path); -static std::string LegacyScriptWrite(TitleSequence* seq); +static std::string LegacyScriptWrite(TitleSequence& seq); -TitleSequence* CreateTitleSequence() +std::unique_ptr CreateTitleSequence() { - TitleSequence* seq = Memory::Allocate(); - *seq = {}; - return seq; + return std::make_unique(); } -TitleSequence* LoadTitleSequence(const utf8* path) +std::unique_ptr LoadTitleSequence(const utf8* path) { std::vector script; std::vector saves; @@ -90,7 +88,7 @@ TitleSequence* LoadTitleSequence(const utf8* path) auto commands = LegacyScriptRead(reinterpret_cast(script.data()), script.size(), saves); - TitleSequence* seq = CreateTitleSequence(); + auto seq = CreateTitleSequence(); seq->Name = Path::GetFileNameWithoutExtension(path); seq->Path = String::Duplicate(path); seq->NumSaves = saves.size(); @@ -101,31 +99,27 @@ TitleSequence* LoadTitleSequence(const utf8* path) return seq; } -void FreeTitleSequence(TitleSequence* seq) +void FreeTitleSequence(TitleSequence& seq) { - if (seq != nullptr) + Memory::Free(seq.Name); + Memory::Free(seq.Path); + Memory::Free(seq.Commands); + for (size_t i = 0; i < seq.NumSaves; i++) { - Memory::Free(seq->Name); - Memory::Free(seq->Path); - Memory::Free(seq->Commands); - for (size_t i = 0; i < seq->NumSaves; i++) - { - Memory::Free(seq->Saves[i]); - } - Memory::Free(seq->Saves); - Memory::Free(seq); + Memory::Free(seq.Saves[i]); } + Memory::Free(seq.Saves); } -TitleSequenceParkHandle* TitleSequenceGetParkHandle(TitleSequence* seq, size_t index) +TitleSequenceParkHandle* TitleSequenceGetParkHandle(TitleSequence& seq, size_t index) { TitleSequenceParkHandle* handle = nullptr; - if (index <= seq->NumSaves) + if (index <= seq.NumSaves) { - const utf8* filename = seq->Saves[index]; - if (seq->IsZip) + const utf8* filename = seq.Saves[index]; + if (seq.IsZip) { - auto zip = std::unique_ptr(Zip::TryOpen(seq->Path, ZIP_ACCESS::READ)); + auto zip = std::unique_ptr(Zip::TryOpen(seq.Path, ZIP_ACCESS::READ)); if (zip != nullptr) { auto data = zip->GetFileData(filename); @@ -140,13 +134,13 @@ TitleSequenceParkHandle* TitleSequenceGetParkHandle(TitleSequence* seq, size_t i } else { - Console::Error::WriteLine("Failed to open zipped path '%s' from zip '%s'", filename, seq->Path); + Console::Error::WriteLine("Failed to open zipped path '%s' from zip '%s'", filename, seq.Path); } } else { utf8 absolutePath[MAX_PATH]; - String::Set(absolutePath, sizeof(absolutePath), seq->Path); + String::Set(absolutePath, sizeof(absolutePath), seq.Path); Path::Append(absolutePath, sizeof(absolutePath), filename); OpenRCT2::FileStream* fileStream = nullptr; @@ -180,20 +174,20 @@ void TitleSequenceCloseParkHandle(TitleSequenceParkHandle* handle) } } -bool TitleSequenceSave(TitleSequence* seq) +bool TitleSequenceSave(TitleSequence& seq) { try { auto script = LegacyScriptWrite(seq); - if (seq->IsZip) + if (seq.IsZip) { auto fdata = std::vector(script.begin(), script.end()); - auto zip = Zip::Open(seq->Path, ZIP_ACCESS::WRITE); + auto zip = Zip::Open(seq.Path, ZIP_ACCESS::WRITE); zip->SetFileData("script.txt", std::move(fdata)); } else { - auto scriptPath = Path::Combine(seq->Path, "script.txt"); + auto scriptPath = Path::Combine(seq.Path, "script.txt"); File::WriteAllBytes(scriptPath, script.data(), script.size()); } return true; @@ -204,13 +198,13 @@ bool TitleSequenceSave(TitleSequence* seq) } } -bool TitleSequenceAddPark(TitleSequence* seq, const utf8* path, const utf8* name) +bool TitleSequenceAddPark(TitleSequence& seq, const utf8* path, const utf8* name) { // Get new save index size_t index = SIZE_MAX; - for (size_t i = 0; i < seq->NumSaves; i++) + for (size_t i = 0; i < seq.NumSaves; i++) { - if (String::Equals(seq->Saves[i], path, true)) + if (String::Equals(seq.Saves[i], path, true)) { index = i; break; @@ -218,22 +212,22 @@ bool TitleSequenceAddPark(TitleSequence* seq, const utf8* path, const utf8* name } if (index == SIZE_MAX) { - seq->Saves = Memory::ReallocateArray(seq->Saves, seq->NumSaves + 1); - Guard::Assert(seq->Saves != nullptr, GUARD_LINE); - index = seq->NumSaves; - seq->NumSaves++; + seq.Saves = Memory::ReallocateArray(seq.Saves, seq.NumSaves + 1); + Guard::Assert(seq.Saves != nullptr, GUARD_LINE); + index = seq.NumSaves; + seq.NumSaves++; } - seq->Saves[index] = String::Duplicate(name); + seq.Saves[index] = String::Duplicate(name); - if (seq->IsZip) + if (seq.IsZip) { try { auto fdata = File::ReadAllBytes(path); - auto zip = Zip::TryOpen(seq->Path, ZIP_ACCESS::WRITE); + auto zip = Zip::TryOpen(seq.Path, ZIP_ACCESS::WRITE); if (zip == nullptr) { - Console::Error::WriteLine("Unable to open '%s'", seq->Path); + Console::Error::WriteLine("Unable to open '%s'", seq.Path); return false; } zip->SetFileData(name, std::move(fdata)); @@ -247,7 +241,7 @@ bool TitleSequenceAddPark(TitleSequence* seq, const utf8* path, const utf8* name { // Determine destination path utf8 dstPath[MAX_PATH]; - String::Set(dstPath, sizeof(dstPath), seq->Path); + String::Set(dstPath, sizeof(dstPath), seq.Path); Path::Append(dstPath, sizeof(dstPath), name); if (!File::Copy(path, dstPath, true)) { @@ -258,17 +252,17 @@ bool TitleSequenceAddPark(TitleSequence* seq, const utf8* path, const utf8* name return true; } -bool TitleSequenceRenamePark(TitleSequence* seq, size_t index, const utf8* name) +bool TitleSequenceRenamePark(TitleSequence& seq, size_t index, const utf8* name) { - Guard::Assert(index < seq->NumSaves, GUARD_LINE); + Guard::Assert(index < seq.NumSaves, GUARD_LINE); - utf8* oldRelativePath = seq->Saves[index]; - if (seq->IsZip) + utf8* oldRelativePath = seq.Saves[index]; + if (seq.IsZip) { - auto zip = Zip::TryOpen(seq->Path, ZIP_ACCESS::WRITE); + auto zip = Zip::TryOpen(seq.Path, ZIP_ACCESS::WRITE); if (zip == nullptr) { - Console::Error::WriteLine("Unable to open '%s'", seq->Path); + Console::Error::WriteLine("Unable to open '%s'", seq.Path); return false; } zip->RenameFile(oldRelativePath, name); @@ -277,9 +271,9 @@ bool TitleSequenceRenamePark(TitleSequence* seq, size_t index, const utf8* name) { utf8 srcPath[MAX_PATH]; utf8 dstPath[MAX_PATH]; - String::Set(srcPath, sizeof(srcPath), seq->Path); + String::Set(srcPath, sizeof(srcPath), seq.Path); Path::Append(srcPath, sizeof(srcPath), oldRelativePath); - String::Set(dstPath, sizeof(dstPath), seq->Path); + String::Set(dstPath, sizeof(dstPath), seq.Path); Path::Append(dstPath, sizeof(dstPath), name); if (!File::Move(srcPath, dstPath)) { @@ -288,23 +282,23 @@ bool TitleSequenceRenamePark(TitleSequence* seq, size_t index, const utf8* name) } } - Memory::Free(seq->Saves[index]); - seq->Saves[index] = String::Duplicate(name); + Memory::Free(seq.Saves[index]); + seq.Saves[index] = String::Duplicate(name); return true; } -bool TitleSequenceRemovePark(TitleSequence* seq, size_t index) +bool TitleSequenceRemovePark(TitleSequence& seq, size_t index) { - Guard::Assert(index < seq->NumSaves, GUARD_LINE); + Guard::Assert(index < seq.NumSaves, GUARD_LINE); // Delete park file - utf8* relativePath = seq->Saves[index]; - if (seq->IsZip) + utf8* relativePath = seq.Saves[index]; + if (seq.IsZip) { - auto zip = Zip::TryOpen(seq->Path, ZIP_ACCESS::WRITE); + auto zip = Zip::TryOpen(seq.Path, ZIP_ACCESS::WRITE); if (zip == nullptr) { - Console::Error::WriteLine("Unable to open '%s'", seq->Path); + Console::Error::WriteLine("Unable to open '%s'", seq.Path); return false; } zip->DeleteFile(relativePath); @@ -312,7 +306,7 @@ bool TitleSequenceRemovePark(TitleSequence* seq, size_t index) else { utf8 absolutePath[MAX_PATH]; - String::Set(absolutePath, sizeof(absolutePath), seq->Path); + String::Set(absolutePath, sizeof(absolutePath), seq.Path); Path::Append(absolutePath, sizeof(absolutePath), relativePath); if (!File::Delete(absolutePath)) { @@ -323,16 +317,16 @@ bool TitleSequenceRemovePark(TitleSequence* seq, size_t index) // Remove from sequence Memory::Free(relativePath); - for (size_t i = index; i < seq->NumSaves - 1; i++) + for (size_t i = index; i < seq.NumSaves - 1; i++) { - seq->Saves[i] = seq->Saves[i + 1]; + seq.Saves[i] = seq.Saves[i + 1]; } - seq->NumSaves--; + seq.NumSaves--; // Update load commands - for (size_t i = 0; i < seq->NumCommands; i++) + for (size_t i = 0; i < seq.NumCommands; i++) { - TitleCommand* command = &seq->Commands[i]; + TitleCommand* command = &seq.Commands[i]; if (command->Type == TITLE_SCRIPT_LOAD) { if (command->SaveIndex == index) @@ -552,17 +546,17 @@ static std::vector ReadScriptFile(const utf8* path) return result; } -static std::string LegacyScriptWrite(TitleSequence* seq) +static std::string LegacyScriptWrite(TitleSequence& seq) { utf8 buffer[128]; auto sb = StringBuilder(128); sb.Append("# SCRIPT FOR "); - sb.Append(seq->Name); + sb.Append(seq.Name); sb.Append("\n"); - for (size_t i = 0; i < seq->NumCommands; i++) + for (size_t i = 0; i < seq.NumCommands; i++) { - const TitleCommand* command = &seq->Commands[i]; + const TitleCommand* command = &seq.Commands[i]; switch (command->Type) { case TITLE_SCRIPT_LOAD: @@ -573,7 +567,7 @@ static std::string LegacyScriptWrite(TitleSequence* seq) else { sb.Append("LOAD "); - sb.Append(seq->Saves[command->SaveIndex]); + sb.Append(seq.Saves[command->SaveIndex]); } break; case TITLE_SCRIPT_LOADSC: diff --git a/src/openrct2/title/TitleSequence.h b/src/openrct2/title/TitleSequence.h index 6bd86ed3d0..4a4e98d628 100644 --- a/src/openrct2/title/TitleSequence.h +++ b/src/openrct2/title/TitleSequence.h @@ -12,6 +12,8 @@ #include "../common.h" #include "../localisation/Localisation.h" +#include + #define TITLE_COMMAND_SCENARIO_LENGTH 64 struct TitleCommand @@ -40,16 +42,16 @@ struct TitleCommand struct TitleSequence { - const utf8* Name; - const utf8* Path; + const utf8* Name = nullptr; + const utf8* Path = nullptr; - size_t NumCommands; - TitleCommand* Commands; + size_t NumCommands = 0; + TitleCommand* Commands = 0; - size_t NumSaves; - utf8** Saves; + size_t NumSaves = 0; + utf8** Saves = nullptr; - bool IsZip; + bool IsZip = false; }; struct TitleSequenceParkHandle @@ -78,20 +80,20 @@ enum TITLE_SCRIPT constexpr const utf8* TITLE_SEQUENCE_EXTENSION = ".parkseq"; constexpr uint8_t SAVE_INDEX_INVALID = UINT8_MAX; -TitleSequence* CreateTitleSequence(); -TitleSequence* LoadTitleSequence(const utf8* path); -void FreeTitleSequence(TitleSequence* seq); +std::unique_ptr CreateTitleSequence(); +std::unique_ptr LoadTitleSequence(const utf8* path); +void FreeTitleSequence(TitleSequence& seq); -TitleSequenceParkHandle* TitleSequenceGetParkHandle(TitleSequence* seq, size_t index); +TitleSequenceParkHandle* TitleSequenceGetParkHandle(TitleSequence& seq, size_t index); /** * Close a title sequence park handle. * The pointer to the handle is invalid after calling this function. */ void TitleSequenceCloseParkHandle(TitleSequenceParkHandle* handle); -bool TitleSequenceSave(TitleSequence* seq); -bool TitleSequenceAddPark(TitleSequence* seq, const utf8* path, const utf8* name); -bool TitleSequenceRenamePark(TitleSequence* seq, size_t index, const utf8* name); -bool TitleSequenceRemovePark(TitleSequence* seq, size_t index); +bool TitleSequenceSave(TitleSequence& seq); +bool TitleSequenceAddPark(TitleSequence& seq, const utf8* path, const utf8* name); +bool TitleSequenceRenamePark(TitleSequence& seq, size_t index, const utf8* name); +bool TitleSequenceRemovePark(TitleSequence& seq, size_t index); bool TitleSequenceIsLoadCommand(const TitleCommand* command); diff --git a/src/openrct2/title/TitleSequenceManager.cpp b/src/openrct2/title/TitleSequenceManager.cpp index ee605dc5fb..4bab37842c 100644 --- a/src/openrct2/title/TitleSequenceManager.cpp +++ b/src/openrct2/title/TitleSequenceManager.cpp @@ -138,13 +138,13 @@ namespace TitleSequenceManager size_t CreateItem(const utf8* name) { std::string path = GetNewTitleSequencePath(std::string(name), true); - TitleSequence* seq = CreateTitleSequence(); + auto seq = CreateTitleSequence(); seq->Name = String::Duplicate(name); seq->Path = String::Duplicate(path.c_str()); seq->IsZip = true; - bool success = TitleSequenceSave(seq); - FreeTitleSequence(seq); + bool success = TitleSequenceSave(*seq); + FreeTitleSequence(*seq); size_t index = SIZE_MAX; if (success)