From 7bb4940ebd69f35c03227098da7c1f1febe5dc92 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sat, 19 Jul 2025 09:29:30 +0100 Subject: [PATCH] Codechange: Use unique_ptr for all pointers in script instance. (#14339) Removes manual memory management with new/delete. --- src/ai/ai_instance.cpp | 2 +- src/script/api/script_object.cpp | 6 +++--- src/script/script_instance.cpp | 32 ++++++++++++++------------------ src/script/script_instance.hpp | 8 ++++---- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/ai/ai_instance.cpp b/src/ai/ai_instance.cpp index cb8a4a00b4..03d0f89792 100644 --- a/src/ai/ai_instance.cpp +++ b/src/ai/ai_instance.cpp @@ -83,7 +83,7 @@ void AIInstance::Died() void AIInstance::LoadDummyScript() { - ScriptAllocatorScope alloc_scope(this->engine); + ScriptAllocatorScope alloc_scope(this->engine.get()); Script_CreateDummy(this->engine->GetVM(), STR_ERROR_AI_NO_AI_FOUND, "AI"); } diff --git a/src/script/api/script_object.cpp b/src/script/api/script_object.cpp index aec20e2a41..6b7ee4b4f3 100644 --- a/src/script/api/script_object.cpp +++ b/src/script/api/script_object.cpp @@ -53,7 +53,7 @@ static ScriptStorage &GetStorage() /* static */ ScriptInstance *ScriptObject::ActiveInstance::active = nullptr; -ScriptObject::ActiveInstance::ActiveInstance(ScriptInstance &instance) : alc_scope(instance.engine) +ScriptObject::ActiveInstance::ActiveInstance(ScriptInstance &instance) : alc_scope(instance.engine.get()) { this->last_active = ScriptObject::ActiveInstance::active; ScriptObject::ActiveInstance::active = &instance; @@ -230,8 +230,8 @@ ScriptObject::DisableDoCommandScope::DisableDoCommandScope() /* static */ bool ScriptObject::CanSuspend() { - Squirrel *squirrel = ScriptObject::GetActiveInstance().engine; - return GetStorage().allow_do_command && squirrel->CanSuspend(); + Squirrel &squirrel = *ScriptObject::GetActiveInstance().engine; + return GetStorage().allow_do_command && squirrel.CanSuspend(); } /* static */ ScriptEventQueue &ScriptObject::GetEventQueue() diff --git a/src/script/script_instance.cpp b/src/script/script_instance.cpp index e321d515b8..5fbd73ceea 100644 --- a/src/script/script_instance.cpp +++ b/src/script/script_instance.cpp @@ -50,8 +50,8 @@ static void PrintFunc(bool error_msg, std::string_view message) ScriptInstance::ScriptInstance(std::string_view api_name) { - this->storage = new ScriptStorage(); - this->engine = new Squirrel(api_name); + this->storage = std::make_unique(); + this->engine = std::make_unique(api_name); this->engine->SetPrintFunction(&PrintFunc); } @@ -59,10 +59,10 @@ void ScriptInstance::Initialize(const std::string &main_script, const std::strin { ScriptObject::ActiveInstance active(*this); - this->controller = new ScriptController(company); + this->controller = std::make_unique(company); /* Register the API functions and classes */ - this->engine->SetGlobalPointer(this->engine); + this->engine->SetGlobalPointer(this->engine.get()); this->RegisterAPI(); if (this->IsDead()) { /* Failed to register API; a message has already been logged. */ @@ -81,12 +81,11 @@ void ScriptInstance::Initialize(const std::string &main_script, const std::strin } /* Create the main-class */ - this->instance = new SQObject(); - if (!this->engine->CreateClassInstance(instance_name, this->controller, this->instance)) { + this->instance = std::make_unique(); + if (!this->engine->CreateClassInstance(instance_name, this->controller.get(), this->instance.get())) { /* If CreateClassInstance has returned false instance has not been * registered with squirrel, so avoid trying to Release it by clearing it now */ - delete this->instance; - this->instance = nullptr; + this->instance.reset(); this->Died(); return; } @@ -158,11 +157,10 @@ ScriptInstance::~ScriptInstance() ScriptObject::ActiveInstance active(*this); this->in_shutdown = true; - if (instance != nullptr) this->engine->ReleaseObject(this->instance); - if (engine != nullptr) delete this->engine; - delete this->storage; - delete this->controller; - delete this->instance; + if (instance != nullptr) this->engine->ReleaseObject(this->instance.get()); + + /* Engine must be reset explicitly in scope of the active instance. */ + this->engine.reset(); } void ScriptInstance::Continue() @@ -179,11 +177,9 @@ void ScriptInstance::Died() this->last_allocated_memory = this->GetAllocatedMemory(); // Update cache - if (this->instance != nullptr) this->engine->ReleaseObject(this->instance); - delete this->instance; - delete this->engine; - this->instance = nullptr; - this->engine = nullptr; + if (this->instance != nullptr) this->engine->ReleaseObject(this->instance.get()); + this->engine.reset(); + this->instance.reset(); } void ScriptInstance::GameLoop() diff --git a/src/script/script_instance.hpp b/src/script/script_instance.hpp index 07c8df99ae..fa11a5c896 100644 --- a/src/script/script_instance.hpp +++ b/src/script/script_instance.hpp @@ -256,7 +256,7 @@ public: void ReleaseSQObject(HSQOBJECT *obj); protected: - class Squirrel *engine = nullptr; ///< A wrapper around the squirrel vm. + std::unique_ptr engine; ///< A wrapper around the squirrel vm. std::string api_version{}; ///< Current API used by this script. /** @@ -288,9 +288,9 @@ protected: virtual void LoadDummyScript() = 0; private: - class ScriptController *controller = nullptr; ///< The script main class. - class ScriptStorage *storage = nullptr; ///< Some global information for each running script. - SQObject *instance = nullptr; ///< Squirrel-pointer to the script main class. + std::unique_ptr storage; ///< Some global information for each running script. + std::unique_ptr controller; ///< The script main class. + std::unique_ptr instance; ///< Squirrel-pointer to the script main class. bool is_started = false; ///< Is the scripts constructor executed? bool is_dead = false; ///< True if the script has been stopped.