From b67cf7f94a53327edb8d15ab1b76bd7b85d9faf9 Mon Sep 17 00:00:00 2001 From: PeterN Date: Sat, 6 May 2023 15:54:58 +0100 Subject: [PATCH] Change: Replace ScriptLog data array with std::deque. (#10770) Due to cyclic header dependency this requires moving the data types used by ScriptLog out of the ScriptLog class. --- src/script/api/CMakeLists.txt | 1 + src/script/api/script_controller.cpp | 4 +- src/script/api/script_log.cpp | 67 ++++++++-------------------- src/script/api/script_log.hpp | 34 +------------- src/script/api/script_log_types.hpp | 47 +++++++++++++++++++ src/script/api/script_object.cpp | 2 +- src/script/api/script_object.hpp | 3 +- src/script/script_gui.cpp | 54 +++++++++++----------- src/script/script_instance.cpp | 5 +-- src/script/script_instance.hpp | 3 +- src/script/script_storage.hpp | 7 +-- 11 files changed, 107 insertions(+), 120 deletions(-) create mode 100644 src/script/api/script_log_types.hpp diff --git a/src/script/api/CMakeLists.txt b/src/script/api/CMakeLists.txt index de559d2229..0cab134930 100644 --- a/src/script/api/CMakeLists.txt +++ b/src/script/api/CMakeLists.txt @@ -180,6 +180,7 @@ add_files( script_league.hpp script_list.hpp script_log.hpp + script_log_types.hpp script_map.hpp script_marine.hpp script_newgrf.hpp diff --git a/src/script/api/script_controller.cpp b/src/script/api/script_controller.cpp index 44a4b152b8..23568df621 100644 --- a/src/script/api/script_controller.cpp +++ b/src/script/api/script_controller.cpp @@ -53,7 +53,7 @@ char log_message[1024]; seprintf(log_message, lastof(log_message), "Break: %s", message); - ScriptLog::Log(ScriptLog::LOG_SQ_ERROR, log_message); + ScriptLog::Log(ScriptLogTypes::LOG_SQ_ERROR, log_message); /* Inform script developer that their script has been paused and * needs manual action to continue. */ @@ -66,7 +66,7 @@ /* static */ void ScriptController::Print(bool error_msg, const char *message) { - ScriptLog::Log(error_msg ? ScriptLog::LOG_SQ_ERROR : ScriptLog::LOG_SQ_INFO, message); + ScriptLog::Log(error_msg ? ScriptLogTypes::LOG_SQ_ERROR : ScriptLogTypes::LOG_SQ_INFO, message); } ScriptController::ScriptController(CompanyID company) : diff --git a/src/script/api/script_log.cpp b/src/script/api/script_log.cpp index 12c4533ca2..dd3a78c071 100644 --- a/src/script/api/script_log.cpp +++ b/src/script/api/script_log.cpp @@ -8,6 +8,7 @@ /** @file script_log.cpp Implementation of ScriptLog. */ #include "../../stdafx.h" +#include "script_log_types.hpp" #include "script_log.hpp" #include "../../core/alloc_func.hpp" #include "../../debug.h" @@ -18,75 +19,45 @@ /* static */ void ScriptLog::Info(const char *message) { - ScriptLog::Log(LOG_INFO, message); + ScriptLog::Log(ScriptLogTypes::LOG_INFO, message); } /* static */ void ScriptLog::Warning(const char *message) { - ScriptLog::Log(LOG_WARNING, message); + ScriptLog::Log(ScriptLogTypes::LOG_WARNING, message); } /* static */ void ScriptLog::Error(const char *message) { - ScriptLog::Log(LOG_ERROR, message); + ScriptLog::Log(ScriptLogTypes::LOG_ERROR, message); } -/* static */ void ScriptLog::Log(ScriptLog::ScriptLogType level, const char *message) +/* static */ void ScriptLog::Log(ScriptLogTypes::ScriptLogType level, const char *message) { - if (ScriptObject::GetLogPointer() == nullptr) { - ScriptObject::GetLogPointer() = new LogData(); - LogData *log = (LogData *)ScriptObject::GetLogPointer(); + ScriptLogTypes::LogData &logdata = ScriptObject::GetLogData(); - log->lines = CallocT(400); - log->type = CallocT(400); - log->count = 400; - log->pos = log->count - 1; - log->used = 0; - } - LogData *log = (LogData *)ScriptObject::GetLogPointer(); + /* Limit the log to 400 lines. */ + if (logdata.size() >= 400U) logdata.pop_front(); - /* Go to the next log-line */ - log->pos = (log->pos + 1) % log->count; - - if (log->used != log->count) log->used++; - - /* Free last message, and write new message */ - free(log->lines[log->pos]); - log->lines[log->pos] = stredup(message); - log->type[log->pos] = level; + auto &line = logdata.emplace_back(); + line.type = level; /* Cut string after first \n */ - char *p; - while ((p = strchr(log->lines[log->pos], '\n')) != nullptr) { - *p = '\0'; - break; - } + const char *newline = strchr(message, '\n'); + line.text = std::string(message, 0, newline == nullptr ? strlen(message) : newline - message); char logc; switch (level) { - case LOG_SQ_ERROR: logc = 'S'; break; - case LOG_ERROR: logc = 'E'; break; - case LOG_SQ_INFO: logc = 'P'; break; - case LOG_WARNING: logc = 'W'; break; - case LOG_INFO: logc = 'I'; break; - default: logc = '?'; break; + case ScriptLogTypes::LOG_SQ_ERROR: logc = 'S'; break; + case ScriptLogTypes::LOG_ERROR: logc = 'E'; break; + case ScriptLogTypes::LOG_SQ_INFO: logc = 'P'; break; + case ScriptLogTypes::LOG_WARNING: logc = 'W'; break; + case ScriptLogTypes::LOG_INFO: logc = 'I'; break; + default: logc = '?'; break; } /* Also still print to debug window */ - Debug(script, level, "[{}] [{}] {}", (uint)ScriptObject::GetRootCompany(), logc, log->lines[log->pos]); + Debug(script, level, "[{}] [{}] {}", (uint)ScriptObject::GetRootCompany(), logc, line.text); InvalidateWindowData(WC_SCRIPT_DEBUG, 0, ScriptObject::GetRootCompany()); } - -/* static */ void ScriptLog::FreeLogPointer() -{ - LogData *log = (LogData *)ScriptObject::GetLogPointer(); - - for (int i = 0; i < log->count; i++) { - free(log->lines[i]); - } - - free(log->lines); - free(log->type); - delete log; -} diff --git a/src/script/api/script_log.hpp b/src/script/api/script_log.hpp index b6767d3f86..b2acd1e171 100644 --- a/src/script/api/script_log.hpp +++ b/src/script/api/script_log.hpp @@ -22,32 +22,6 @@ class ScriptLog : public ScriptObject { friend class ScriptController; public: - /** - * Log levels; The value is also feed to Debug() lvl. - * This has no use for you, as script writer. - * @api -all - */ - enum ScriptLogType { - LOG_SQ_ERROR = 0, ///< Squirrel printed an error. - LOG_ERROR = 1, ///< User printed an error. - LOG_SQ_INFO = 2, ///< Squirrel printed some info. - LOG_WARNING = 3, ///< User printed some warning. - LOG_INFO = 4, ///< User printed some info. - }; - - /** - * Internal representation of the log-data inside the script. - * This has no use for you, as script writer. - * @api -all - */ - struct LogData { - char **lines; ///< The log-lines. - ScriptLog::ScriptLogType *type; ///< Per line, which type of log it was. - int count; ///< Total amount of log-lines possible. - int pos; ///< Current position in lines. - int used; ///< Total amount of used log-lines. - }; - /** * Print an Info message to the logs. * @param message The message to log. @@ -69,17 +43,11 @@ public: */ static void Error(const char *message); - /** - * Free the log pointer. - * @api -all - */ - static void FreeLogPointer(); - private: /** * Internal command to log the message in a common way. */ - static void Log(ScriptLog::ScriptLogType level, const char *message); + static void Log(ScriptLogTypes::ScriptLogType level, const char *message); }; #endif /* SCRIPT_LOG_HPP */ diff --git a/src/script/api/script_log_types.hpp b/src/script/api/script_log_types.hpp new file mode 100644 index 0000000000..b10ea0cea3 --- /dev/null +++ b/src/script/api/script_log_types.hpp @@ -0,0 +1,47 @@ +/* + * This file is part of OpenTTD. + * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2. + * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see . + */ + +/** @file script_log_types.hpp Data types for script log messages. */ + +#ifndef SCRIPT_LOG_TYPES_HPP +#define SCRIPT_LOG_TYPES_HPP + +#include + +namespace ScriptLogTypes { + /** + * Log levels; The value is also feed to Debug() lvl. + * This has no use for you, as script writer. + * @api -all + */ + enum ScriptLogType { + LOG_SQ_ERROR = 0, ///< Squirrel printed an error. + LOG_ERROR = 1, ///< User printed an error. + LOG_SQ_INFO = 2, ///< Squirrel printed some info. + LOG_WARNING = 3, ///< User printed some warning. + LOG_INFO = 4, ///< User printed some info. + }; + + /** + * Internal representation of the log-data inside the script. + * This has no use for you, as script writer. + * @api -all + */ + struct LogLine { + std::string text; ///< The text + ScriptLogType type; ///< Text type + }; + + /** + * Internal representation of the log-data inside the script. + * This has no use for you, as script writer. + * @api -all + */ + using LogData = std::deque; ///< The log type +}; + +#endif /* SCRIPT_LOG_TYPES_HPP */ diff --git a/src/script/api/script_object.cpp b/src/script/api/script_object.cpp index f15b9d56ec..b0692c2aa7 100644 --- a/src/script/api/script_object.cpp +++ b/src/script/api/script_object.cpp @@ -213,7 +213,7 @@ ScriptObject::ActiveInstance::~ActiveInstance() return GetStorage()->event_data; } -/* static */ void *&ScriptObject::GetLogPointer() +/* static */ ScriptLogTypes::LogData &ScriptObject::GetLogData() { return GetStorage()->log_data; } diff --git a/src/script/api/script_object.hpp b/src/script/api/script_object.hpp index a8d01d5c0d..606bc7c584 100644 --- a/src/script/api/script_object.hpp +++ b/src/script/api/script_object.hpp @@ -18,6 +18,7 @@ #include "../../core/random_func.hpp" #include "script_types.hpp" +#include "script_log_types.hpp" #include "../script_suspend.hpp" #include "../squirrel.hpp" @@ -276,7 +277,7 @@ protected: /** * Get the pointer to store log message in. */ - static void *&GetLogPointer(); + static ScriptLogTypes::LogData &GetLogData(); /** * Get an allocated string with all control codes stripped off. diff --git a/src/script/script_gui.cpp b/src/script/script_gui.cpp index aeec99445f..e77c1c7ede 100644 --- a/src/script/script_gui.cpp +++ b/src/script/script_gui.cpp @@ -704,10 +704,10 @@ struct ScriptDebugWindow : public Window { int highlight_row; ///< The output row that matches the given string, or -1 Scrollbar *vscroll; ///< Cache of the vertical scrollbar. - ScriptLog::LogData *GetLogPointer() const + ScriptLogTypes::LogData &GetLogData() const { - if (script_debug_company == OWNER_DEITY) return (ScriptLog::LogData *)Game::GetInstance()->GetLogPointer(); - return (ScriptLog::LogData *)Company::Get(script_debug_company)->ai_instance->GetLogPointer(); + if (script_debug_company == OWNER_DEITY) return Game::GetInstance()->GetLogData(); + return Company::Get(script_debug_company)->ai_instance->GetLogData(); } /** @@ -845,9 +845,9 @@ struct ScriptDebugWindow : public Window { /* If there are no active companies, don't display anything else. */ if (script_debug_company == INVALID_COMPANY) return; - ScriptLog::LogData *log = this->GetLogPointer(); + ScriptLogTypes::LogData &log = this->GetLogData(); - int scroll_count = (log == nullptr) ? 0 : log->used; + int scroll_count = (int)log.size(); if (this->vscroll->GetCount() != scroll_count) { this->vscroll->SetCount(scroll_count); @@ -855,15 +855,15 @@ struct ScriptDebugWindow : public Window { this->SetWidgetDirty(WID_SCRD_SCROLLBAR); } - if (log == nullptr) return; + if (log.empty()) return; /* Detect when the user scrolls the window. Enable autoscroll when the * bottom-most line becomes visible. */ if (this->last_vscroll_pos != this->vscroll->GetPosition()) { - this->autoscroll = this->vscroll->GetPosition() >= log->used - this->vscroll->GetCapacity(); + this->autoscroll = this->vscroll->GetPosition() >= log.size() - this->vscroll->GetCapacity(); } if (this->autoscroll) { - int scroll_pos = std::max(0, log->used - this->vscroll->GetCapacity()); + int scroll_pos = std::max(0, (int)log.size() - this->vscroll->GetCapacity()); if (this->vscroll->SetPosition(scroll_pos)) { /* We need a repaint */ this->SetWidgetDirty(WID_SCRD_SCROLLBAR); @@ -900,32 +900,31 @@ struct ScriptDebugWindow : public Window { if (widget != WID_SCRD_LOG_PANEL) return; - ScriptLog::LogData *log = this->GetLogPointer(); - if (log == nullptr) return; + ScriptLogTypes::LogData &log = this->GetLogData(); + if (log.empty()) return; Rect br = r.Shrink(WidgetDimensions::scaled.bevel); Rect tr = r.Shrink(WidgetDimensions::scaled.framerect); - for (int i = this->vscroll->GetPosition(); this->vscroll->IsVisible(i) && i < log->used; i++) { - int pos = (i + log->pos + 1 - log->used + log->count) % log->count; - if (log->lines[pos] == nullptr) break; + for (int i = this->vscroll->GetPosition(); this->vscroll->IsVisible(i) && (size_t)i < log.size(); i++) { + const ScriptLogTypes::LogLine &line = log[i]; TextColour colour; - switch (log->type[pos]) { - case ScriptLog::LOG_SQ_INFO: colour = TC_BLACK; break; - case ScriptLog::LOG_SQ_ERROR: colour = TC_WHITE; break; - case ScriptLog::LOG_INFO: colour = TC_BLACK; break; - case ScriptLog::LOG_WARNING: colour = TC_YELLOW; break; - case ScriptLog::LOG_ERROR: colour = TC_RED; break; - default: colour = TC_BLACK; break; + switch (line.type) { + case ScriptLogTypes::LOG_SQ_INFO: colour = TC_BLACK; break; + case ScriptLogTypes::LOG_SQ_ERROR: colour = TC_WHITE; break; + case ScriptLogTypes::LOG_INFO: colour = TC_BLACK; break; + case ScriptLogTypes::LOG_WARNING: colour = TC_YELLOW; break; + case ScriptLogTypes::LOG_ERROR: colour = TC_RED; break; + default: colour = TC_BLACK; break; } /* Check if the current line should be highlighted */ - if (pos == this->highlight_row) { + if (i == this->highlight_row) { GfxFillRect(br.left, tr.top, br.right, tr.top + this->resize.step_height - 1, PC_BLACK); if (colour == TC_BLACK) colour = TC_WHITE; // Make black text readable by inverting it to white. } - DrawString(tr, log->lines[pos], colour, SA_LEFT | SA_FORCE); + DrawString(tr, line.text, colour, SA_LEFT | SA_FORCE); tr.top += this->resize.step_height; } } @@ -1041,11 +1040,11 @@ struct ScriptDebugWindow : public Window { * This needs to be done in gameloop-scope, so the AI is suspended immediately. */ if (!gui_scope && data == script_debug_company && this->IsValidDebugCompany(script_debug_company) && this->break_check_enabled && !this->break_string_filter.IsEmpty()) { /* Get the log instance of the active company */ - ScriptLog::LogData *log = this->GetLogPointer(); + ScriptLogTypes::LogData &log = this->GetLogData(); - if (log != nullptr) { + if (!log.empty()) { this->break_string_filter.ResetState(); - this->break_string_filter.AddLine(log->lines[log->pos]); + this->break_string_filter.AddLine(log.back().text); if (this->break_string_filter.GetState()) { /* Pause execution of script. */ if (!this->IsDead()) { @@ -1062,7 +1061,7 @@ struct ScriptDebugWindow : public Window { } /* Highlight row that matched */ - this->highlight_row = log->pos; + this->highlight_row = (int)(log.size() - 1); } } } @@ -1071,8 +1070,7 @@ struct ScriptDebugWindow : public Window { this->SelectValidDebugCompany(); - ScriptLog::LogData *log = script_debug_company != INVALID_COMPANY ? this->GetLogPointer() : nullptr; - this->vscroll->SetCount((log == nullptr) ? 0 : log->used); + this->vscroll->SetCount(script_debug_company != INVALID_COMPANY ? (int)this->GetLogData().size() : 0); /* Update company buttons */ for (CompanyID i = COMPANY_FIRST; i < MAX_COMPANIES; i++) { diff --git a/src/script/script_instance.cpp b/src/script/script_instance.cpp index c6ab1d6008..b7a4497aae 100644 --- a/src/script/script_instance.cpp +++ b/src/script/script_instance.cpp @@ -36,7 +36,6 @@ ScriptStorage::~ScriptStorage() { /* Free our pointers */ if (event_data != nullptr) ScriptEventController::FreeEventPointer(); - if (log_data != nullptr) ScriptLog::FreeLogPointer(); } /** @@ -315,11 +314,11 @@ ScriptStorage *ScriptInstance::GetStorage() return this->storage; } -void *ScriptInstance::GetLogPointer() +ScriptLogTypes::LogData &ScriptInstance::GetLogData() { ScriptObject::ActiveInstance active(this); - return ScriptObject::GetLogPointer(); + return ScriptObject::GetLogData(); } /* diff --git a/src/script/script_instance.hpp b/src/script/script_instance.hpp index 65a6414bb5..dae1f6940d 100644 --- a/src/script/script_instance.hpp +++ b/src/script/script_instance.hpp @@ -14,6 +14,7 @@ #include #include #include "script_suspend.hpp" +#include "script_log_types.hpp" #include "../command_type.h" #include "../company_type.h" @@ -95,7 +96,7 @@ public: /** * Get the log pointer of this script. */ - void *GetLogPointer(); + ScriptLogTypes::LogData &GetLogData(); /** * Return a true/false reply for a DoCommand. diff --git a/src/script/script_storage.hpp b/src/script/script_storage.hpp index 44d17af837..b7a16dcb15 100644 --- a/src/script/script_storage.hpp +++ b/src/script/script_storage.hpp @@ -17,6 +17,8 @@ #include "../goal_type.h" #include "../story_type.h" +#include "script_log_types.hpp" + #include "table/strings.h" #include @@ -54,7 +56,7 @@ private: RailType rail_type; ///< The current railtype we build. void *event_data; ///< Pointer to the event data storage. - void *log_data; ///< Pointer to the log data storage. + ScriptLogTypes::LogData log_data;///< Log data storage. public: ScriptStorage() : @@ -72,8 +74,7 @@ public: /* calback_value (can't be set) */ road_type (INVALID_ROADTYPE), rail_type (INVALID_RAILTYPE), - event_data (nullptr), - log_data (nullptr) + event_data (nullptr) { } ~ScriptStorage();