From dfcd8a4bbcb73a6e909099d10575baa0a567a2cc Mon Sep 17 00:00:00 2001 From: Ted John Date: Tue, 10 Nov 2020 20:18:17 +0000 Subject: [PATCH] Apply code review comments --- src/openrct2-ui/windows/TitleEditor.cpp | 58 ++++++++++++---------- src/openrct2/core/String.cpp | 2 +- src/openrct2/core/String.hpp | 2 +- src/openrct2/drawing/TTF.cpp | 6 +-- src/openrct2/interface/StdInOutConsole.cpp | 21 ++++---- 5 files changed, 46 insertions(+), 43 deletions(-) diff --git a/src/openrct2-ui/windows/TitleEditor.cpp b/src/openrct2-ui/windows/TitleEditor.cpp index 3228071251..052aac6c6b 100644 --- a/src/openrct2-ui/windows/TitleEditor.cpp +++ b/src/openrct2-ui/windows/TitleEditor.cpp @@ -929,31 +929,20 @@ static void window_title_editor_scrollpaint_commands(rct_window* w, rct_drawpixe error = true; auto ft = Formatter(); - if ((selected || hover) && !error) + if (error) { - ft.Add(STR_STRINGID); + ft.Add(selected || hover ? STR_LIGHTPINK_STRINGID : STR_RED_STRINGID); } else { - if (error) - { - ft.Add(STR_BLACK_STRING); - } - else if (selected || hover) - { - ft.Add(STR_LIGHTPINK_STRINGID); - } - else - { - ft.Add(STR_RED_STRINGID); - } + ft.Add(selected || hover ? STR_STRINGID : STR_BLACK_STRING); } - rct_string_id commandName = STR_NONE; switch (command.Type) { case TITLE_SCRIPT_LOAD: - commandName = STR_TITLE_EDITOR_COMMAND_LOAD_FILE; + { + auto commandName = STR_TITLE_EDITOR_COMMAND_LOAD_FILE; if (command.SaveIndex == SAVE_INDEX_INVALID) { commandName = STR_TITLE_EDITOR_COMMAND_LOAD_NO_SAVE; @@ -965,29 +954,39 @@ static void window_title_editor_scrollpaint_commands(rct_window* w, rct_drawpixe ft.Add(_editingTitleSequence->Saves[command.SaveIndex].c_str()); } break; + } case TITLE_SCRIPT_LOCATION: - commandName = STR_TITLE_EDITOR_COMMAND_LOCATION; + { + auto commandName = STR_TITLE_EDITOR_COMMAND_LOCATION; ft.Add(commandName); ft.Add(command.X); ft.Add(command.Y); break; + } case TITLE_SCRIPT_ROTATE: - commandName = STR_TITLE_EDITOR_COMMAND_ROTATE; + { + auto commandName = STR_TITLE_EDITOR_COMMAND_ROTATE; ft.Add(commandName); ft.Add(command.Rotations); break; + } case TITLE_SCRIPT_ZOOM: - commandName = STR_TITLE_EDITOR_COMMAND_ZOOM; + { + auto commandName = STR_TITLE_EDITOR_COMMAND_ZOOM; ft.Add(commandName); ft.Add(command.Zoom); break; + } case TITLE_SCRIPT_SPEED: - commandName = STR_TITLE_EDITOR_COMMAND_SPEED; + { + auto commandName = STR_TITLE_EDITOR_COMMAND_SPEED; ft.Add(commandName); ft.Add(SpeedNames[command.Speed - 1]); break; + } case TITLE_SCRIPT_FOLLOW: - commandName = STR_TITLE_EDITOR_COMMAND_FOLLOW; + { + auto commandName = STR_TITLE_EDITOR_COMMAND_FOLLOW; if (command.SpriteIndex == SPRITE_INDEX_NULL) { commandName = STR_TITLE_EDITOR_COMMAND_FOLLOW_NO_SPRITE; @@ -999,22 +998,29 @@ static void window_title_editor_scrollpaint_commands(rct_window* w, rct_drawpixe ft.Add(command.SpriteName); } break; + } case TITLE_SCRIPT_WAIT: - commandName = STR_TITLE_EDITOR_COMMAND_WAIT; + { + auto commandName = STR_TITLE_EDITOR_COMMAND_WAIT; ft.Add(commandName); ft.Add(command.Milliseconds); break; + } case TITLE_SCRIPT_RESTART: - commandName = STR_TITLE_EDITOR_RESTART; + { + auto commandName = STR_TITLE_EDITOR_RESTART; ft.Add(commandName); break; + } case TITLE_SCRIPT_END: - commandName = STR_TITLE_EDITOR_END; + { + auto commandName = STR_TITLE_EDITOR_END; ft.Add(commandName); break; + } case TITLE_SCRIPT_LOADSC: { - commandName = STR_TITLE_EDITOR_COMMAND_LOAD_FILE; + auto commandName = STR_TITLE_EDITOR_COMMAND_LOAD_FILE; const char* name = ""; auto scenario = GetScenarioRepository()->GetByInternalName(command.Scenario); if (command.Scenario[0] == '\0') @@ -1035,7 +1041,7 @@ static void window_title_editor_scrollpaint_commands(rct_window* w, rct_drawpixe } default: { - ft.Add(commandName); + ft.Add(STR_NONE); log_warning("Unknown command %d", command.Type); } } diff --git a/src/openrct2/core/String.cpp b/src/openrct2/core/String.cpp index 1fc0b59bc2..08349e67cf 100644 --- a/src/openrct2/core/String.cpp +++ b/src/openrct2/core/String.cpp @@ -154,7 +154,7 @@ namespace String } } - bool Equals(const std::string_view& a, const std::string_view& b, bool ignoreCase) + bool Equals(const std::string_view a, const std::string_view b, bool ignoreCase) { if (ignoreCase) { diff --git a/src/openrct2/core/String.hpp b/src/openrct2/core/String.hpp index 3961c4559d..ebb4643c41 100644 --- a/src/openrct2/core/String.hpp +++ b/src/openrct2/core/String.hpp @@ -42,7 +42,7 @@ namespace String bool IsNullOrEmpty(const utf8* str); int32_t Compare(const std::string& a, const std::string& b, bool ignoreCase = false); int32_t Compare(const utf8* a, const utf8* b, bool ignoreCase = false); - bool Equals(const std::string_view& a, const std::string_view& b, bool ignoreCase = false); + bool Equals(const std::string_view a, const std::string_view b, bool ignoreCase = false); bool Equals(const std::string& a, const std::string& b, bool ignoreCase = false); bool Equals(const utf8* a, const utf8* b, bool ignoreCase = false); bool StartsWith(const utf8* str, const utf8* match, bool ignoreCase = false); diff --git a/src/openrct2/drawing/TTF.cpp b/src/openrct2/drawing/TTF.cpp index 37068fc31e..279445434f 100644 --- a/src/openrct2/drawing/TTF.cpp +++ b/src/openrct2/drawing/TTF.cpp @@ -181,12 +181,12 @@ static void ttf_close_font(TTF_Font* font) TTF_CloseFont(font); } -static uint32_t ttf_surface_cache_hash(TTF_Font* font, std::string_view text) +static uint32_t ttf_surface_cache_hash(TTF_Font* font, const std::string_view text) { uint32_t hash = static_cast(((reinterpret_cast(font) * 23) ^ 0xAAAAAAAA) & 0xFFFFFFFF); - for (size_t i = 0; i < text.size(); i++) + for (auto& c : text) { - hash = ror32(hash, 3) ^ (text[i] * 13); + hash = ror32(hash, 3) ^ (c * 13); } return hash; } diff --git a/src/openrct2/interface/StdInOutConsole.cpp b/src/openrct2/interface/StdInOutConsole.cpp index 239e1179a4..37910f6660 100644 --- a/src/openrct2/interface/StdInOutConsole.cpp +++ b/src/openrct2/interface/StdInOutConsole.cpp @@ -111,19 +111,16 @@ void StdInOutConsole::Close() void StdInOutConsole::WriteLine(const std::string& s, FormatToken colourFormat) { std::string formatBegin; - if (colourFormat != FormatToken::ColourWindow2) + switch (colourFormat) { - switch (colourFormat) - { - case FormatToken::ColourRed: - formatBegin = "\033[31m"; - break; - case FormatToken::ColourYellow: - formatBegin = "\033[33m"; - break; - default: - break; - } + case FormatToken::ColourRed: + formatBegin = "\033[31m"; + break; + case FormatToken::ColourYellow: + formatBegin = "\033[33m"; + break; + default: + break; } if (formatBegin.empty() || !Platform::IsColourTerminalSupported())