diff --git a/src/ai/ai_instance.cpp b/src/ai/ai_instance.cpp index cb8a4a00b4..2f30bddea0 100644 --- a/src/ai/ai_instance.cpp +++ b/src/ai/ai_instance.cpp @@ -43,7 +43,7 @@ void AIInstance::Initialize(AIInfo *info) this->api_version = info->GetAPIVersion(); /* Register the AIController (including the "import" command) */ - SQAIController_Register(*this->engine); + SQAIController_Register(this->GetEngine()); ScriptInstance::Initialize(info->GetMainScript(), info->GetInstanceName(), _current_company); } @@ -53,7 +53,7 @@ void AIInstance::RegisterAPI() ScriptInstance::RegisterAPI(); /* Register all classes */ - SQAI_RegisterAll(*this->engine); + SQAI_RegisterAll(this->GetEngine()); if (!this->LoadCompatibilityScripts(AI_DIR, AIInfo::ApiVersions)) this->Died(); } @@ -83,8 +83,8 @@ void AIInstance::Died() void AIInstance::LoadDummyScript() { - ScriptAllocatorScope alloc_scope(this->engine); - Script_CreateDummy(this->engine->GetVM(), STR_ERROR_AI_NO_AI_FOUND, "AI"); + ScriptAllocatorScope alloc_scope(&this->GetEngine()); + Script_CreateDummy(this->GetEngine().GetVM(), STR_ERROR_AI_NO_AI_FOUND, "AI"); } int AIInstance::GetSetting(const std::string &name) diff --git a/src/game/game_instance.cpp b/src/game/game_instance.cpp index 2a619a0e98..461cedfa2d 100644 --- a/src/game/game_instance.cpp +++ b/src/game/game_instance.cpp @@ -38,7 +38,7 @@ void GameInstance::Initialize(GameInfo *info) this->api_version = info->GetAPIVersion(); /* Register the GameController */ - SQGSController_Register(*this->engine); + SQGSController_Register(this->GetEngine()); ScriptInstance::Initialize(info->GetMainScript(), info->GetInstanceName(), OWNER_DEITY); } @@ -48,11 +48,11 @@ void GameInstance::RegisterAPI() ScriptInstance::RegisterAPI(); /* Register all classes */ - SQGS_RegisterAll(*this->engine); + SQGS_RegisterAll(this->GetEngine()); if (!this->LoadCompatibilityScripts(GAME_DIR, GameInfo::ApiVersions)) this->Died(); - if (this->IsAlive()) RegisterGameTranslation(*this->engine); + if (this->IsAlive()) RegisterGameTranslation(this->GetEngine()); } int GameInstance::GetSetting(const std::string &name) diff --git a/src/script/api/script_controller.cpp b/src/script/api/script_controller.cpp index 84b7c1ae73..774b089ef5 100644 --- a/src/script/api/script_controller.cpp +++ b/src/script/api/script_controller.cpp @@ -76,7 +76,7 @@ ScriptController::ScriptController(::CompanyID company) : /* static */ uint ScriptController::GetTick() { - return ScriptObject::GetActiveInstance().GetController()->ticks; + return ScriptObject::GetActiveInstance().GetController().ticks; } /* static */ int ScriptController::GetOpsTillSuspend() @@ -96,9 +96,9 @@ ScriptController::ScriptController(::CompanyID company) : /* static */ HSQOBJECT ScriptController::Import(const std::string &library, const std::string &class_name, int version) { - ScriptController *controller = ScriptObject::GetActiveInstance().GetController(); - Squirrel *engine = ScriptObject::GetActiveInstance().engine; - HSQUIRRELVM vm = engine->GetVM(); + ScriptController &controller = ScriptObject::GetActiveInstance().GetController(); + Squirrel &engine = *ScriptObject::GetActiveInstance().engine; + HSQUIRRELVM vm = engine.GetVM(); ScriptInfo *lib = ScriptObject::GetActiveInstance().FindLibrary(library, version); if (lib == nullptr) { @@ -114,11 +114,11 @@ ScriptController::ScriptController(::CompanyID company) : std::string fake_class; - LoadedLibraryList::iterator it = controller->loaded_library.find(library_name); - if (it != controller->loaded_library.end()) { + LoadedLibraryList::iterator it = controller.loaded_library.find(library_name); + if (it != controller.loaded_library.end()) { fake_class = (*it).second; } else { - int next_number = ++controller->loaded_library_count; + int next_number = ++controller.loaded_library_count; /* Create a new fake internal name */ fake_class = fmt::format("_internalNA{}", next_number); @@ -128,14 +128,14 @@ ScriptController::ScriptController(::CompanyID company) : sq_pushstring(vm, fake_class); sq_newclass(vm, SQFalse); /* Load the library */ - if (!engine->LoadScript(vm, lib->GetMainScript(), false)) { + if (!engine.LoadScript(vm, lib->GetMainScript(), false)) { throw sq_throwerror(vm, fmt::format("there was a compile error when importing '{}' version {}", library, version)); } /* Create the fake class */ sq_newslot(vm, -3, SQFalse); sq_pop(vm, 1); - controller->loaded_library[library_name] = fake_class; + controller.loaded_library[library_name] = fake_class; } /* Find the real class inside the fake class (like 'sets.Vector') */ diff --git a/src/script/api/script_object.cpp b/src/script/api/script_object.cpp index b656a95c09..6b7ee4b4f3 100644 --- a/src/script/api/script_object.cpp +++ b/src/script/api/script_object.cpp @@ -45,7 +45,7 @@ void SimpleCountedObject::Release() * Get the storage associated with the current ScriptInstance. * @return The storage. */ -static ScriptStorage *GetStorage() +static ScriptStorage &GetStorage() { return ScriptObject::GetActiveInstance().GetStorage(); } @@ -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; @@ -65,7 +65,7 @@ ScriptObject::ActiveInstance::~ActiveInstance() } ScriptObject::DisableDoCommandScope::DisableDoCommandScope() - : AutoRestoreBackup(GetStorage()->allow_do_command, false) + : AutoRestoreBackup(GetStorage().allow_do_command, false) {} /* static */ ScriptInstance &ScriptObject::GetActiveInstance() @@ -78,181 +78,181 @@ ScriptObject::DisableDoCommandScope::DisableDoCommandScope() /* static */ void ScriptObject::SetDoCommandDelay(uint ticks) { assert(ticks > 0); - GetStorage()->delay = ticks; + GetStorage().delay = ticks; } /* static */ uint ScriptObject::GetDoCommandDelay() { - return GetStorage()->delay; + return GetStorage().delay; } /* static */ void ScriptObject::SetDoCommandMode(ScriptModeProc *proc, ScriptObject *instance) { - GetStorage()->mode = proc; - GetStorage()->mode_instance = instance; + GetStorage().mode = proc; + GetStorage().mode_instance = instance; } /* static */ ScriptModeProc *ScriptObject::GetDoCommandMode() { - return GetStorage()->mode; + return GetStorage().mode; } /* static */ ScriptObject *ScriptObject::GetDoCommandModeInstance() { - return GetStorage()->mode_instance; + return GetStorage().mode_instance; } /* static */ void ScriptObject::SetDoCommandAsyncMode(ScriptAsyncModeProc *proc, ScriptObject *instance) { - GetStorage()->async_mode = proc; - GetStorage()->async_mode_instance = instance; + GetStorage().async_mode = proc; + GetStorage().async_mode_instance = instance; } /* static */ ScriptAsyncModeProc *ScriptObject::GetDoCommandAsyncMode() { - return GetStorage()->async_mode; + return GetStorage().async_mode; } /* static */ ScriptObject *ScriptObject::GetDoCommandAsyncModeInstance() { - return GetStorage()->async_mode_instance; + return GetStorage().async_mode_instance; } /* static */ void ScriptObject::SetLastCommand(const CommandDataBuffer &data, Commands cmd) { - ScriptStorage *s = GetStorage(); - Debug(script, 6, "SetLastCommand company={:02d} cmd={} data={}", s->root_company, cmd, FormatArrayAsHex(data)); - s->last_data = data; - s->last_cmd = cmd; + ScriptStorage &s = GetStorage(); + Debug(script, 6, "SetLastCommand company={:02d} cmd={} data={}", s.root_company, cmd, FormatArrayAsHex(data)); + s.last_data = data; + s.last_cmd = cmd; } /* static */ bool ScriptObject::CheckLastCommand(const CommandDataBuffer &data, Commands cmd) { - ScriptStorage *s = GetStorage(); - Debug(script, 6, "CheckLastCommand company={:02d} cmd={} data={}", s->root_company, cmd, FormatArrayAsHex(data)); - if (s->last_cmd != cmd) return false; - if (s->last_data != data) return false; + ScriptStorage &s = GetStorage(); + Debug(script, 6, "CheckLastCommand company={:02d} cmd={} data={}", s.root_company, cmd, FormatArrayAsHex(data)); + if (s.last_cmd != cmd) return false; + if (s.last_data != data) return false; return true; } /* static */ void ScriptObject::SetDoCommandCosts(Money value) { - GetStorage()->costs = CommandCost(INVALID_EXPENSES, value); // Expense type is never read. + GetStorage().costs = CommandCost(INVALID_EXPENSES, value); // Expense type is never read. } /* static */ void ScriptObject::IncreaseDoCommandCosts(Money value) { - GetStorage()->costs.AddCost(value); + GetStorage().costs.AddCost(value); } /* static */ Money ScriptObject::GetDoCommandCosts() { - return GetStorage()->costs.GetCost(); + return GetStorage().costs.GetCost(); } /* static */ void ScriptObject::SetLastError(ScriptErrorType last_error) { - GetStorage()->last_error = last_error; + GetStorage().last_error = last_error; } /* static */ ScriptErrorType ScriptObject::GetLastError() { - return GetStorage()->last_error; + return GetStorage().last_error; } /* static */ void ScriptObject::SetLastCost(Money last_cost) { - GetStorage()->last_cost = last_cost; + GetStorage().last_cost = last_cost; } /* static */ Money ScriptObject::GetLastCost() { - return GetStorage()->last_cost; + return GetStorage().last_cost; } /* static */ void ScriptObject::SetRoadType(RoadType road_type) { - GetStorage()->road_type = road_type; + GetStorage().road_type = road_type; } /* static */ RoadType ScriptObject::GetRoadType() { - return GetStorage()->road_type; + return GetStorage().road_type; } /* static */ void ScriptObject::SetRailType(RailType rail_type) { - GetStorage()->rail_type = rail_type; + GetStorage().rail_type = rail_type; } /* static */ RailType ScriptObject::GetRailType() { - return GetStorage()->rail_type; + return GetStorage().rail_type; } /* static */ void ScriptObject::SetLastCommandRes(bool res) { - GetStorage()->last_command_res = res; + GetStorage().last_command_res = res; } /* static */ bool ScriptObject::GetLastCommandRes() { - return GetStorage()->last_command_res; + return GetStorage().last_command_res; } /* static */ void ScriptObject::SetLastCommandResData(CommandDataBuffer data) { - GetStorage()->last_cmd_ret = std::move(data); + GetStorage().last_cmd_ret = std::move(data); } /* static */ const CommandDataBuffer &ScriptObject::GetLastCommandResData() { - return GetStorage()->last_cmd_ret; + return GetStorage().last_cmd_ret; } /* static */ void ScriptObject::SetCompany(::CompanyID company) { - if (GetStorage()->root_company == INVALID_OWNER) GetStorage()->root_company = company; - GetStorage()->company = company; + if (GetStorage().root_company == INVALID_OWNER) GetStorage().root_company = company; + GetStorage().company = company; _current_company = company; } /* static */ ::CompanyID ScriptObject::GetCompany() { - return GetStorage()->company; + return GetStorage().company; } /* static */ ::CompanyID ScriptObject::GetRootCompany() { - return GetStorage()->root_company; + return GetStorage().root_company; } /* 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() { - return GetStorage()->event_queue; + return GetStorage().event_queue; } /* static */ ScriptLogTypes::LogData &ScriptObject::GetLogData() { - return GetStorage()->log_data; + return GetStorage().log_data; } /* static */ void ScriptObject::SetCallbackVariable(int index, int value) { - if (static_cast(index) >= GetStorage()->callback_value.size()) GetStorage()->callback_value.resize(index + 1); - GetStorage()->callback_value[index] = value; + if (static_cast(index) >= GetStorage().callback_value.size()) GetStorage().callback_value.resize(index + 1); + GetStorage().callback_value[index] = value; } /* static */ int ScriptObject::GetCallbackVariable(int index) { - return GetStorage()->callback_value[index]; + return GetStorage().callback_value[index]; } /* static */ CommandCallbackData *ScriptObject::GetDoCommandCallback() @@ -310,8 +310,8 @@ ScriptObject::DisableDoCommandScope::DisableDoCommandScope() IncreaseDoCommandCosts(res.GetCost()); if (!_generating_world) { /* Charge a nominal fee for asynchronously executed commands */ - Squirrel *engine = ScriptObject::GetActiveInstance().engine; - Squirrel::DecreaseOps(engine->GetVM(), 100); + Squirrel &engine = *ScriptObject::GetActiveInstance().engine; + Squirrel::DecreaseOps(engine.GetVM(), 100); } if (callback != nullptr) { /* Insert return value into to stack and throw a control code that diff --git a/src/script/script_instance.cpp b/src/script/script_instance.cpp index 58ba727b62..d4b0653be3 100644 --- a/src/script/script_instance.cpp +++ b/src/script/script_instance.cpp @@ -47,9 +47,8 @@ static void PrintFunc(bool error_msg, std::string_view message) } ScriptInstance::ScriptInstance(std::string_view api_name) + : storage(std::make_unique()), engine(std::make_unique(api_name)) { - this->storage = new ScriptStorage(); - this->engine = new Squirrel(api_name); this->engine->SetPrintFunction(&PrintFunc); } @@ -57,10 +56,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. */ @@ -79,12 +78,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; } @@ -98,7 +96,7 @@ void ScriptInstance::Initialize(const std::string &main_script, const std::strin void ScriptInstance::RegisterAPI() { - squirrel_register_std(this->engine); + squirrel_register_std(*this->engine); } bool ScriptInstance::LoadCompatibilityScript(std::string_view api_version, Subdirectory dir) @@ -144,11 +142,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 as object destruction requires the instance to exist. */ + this->engine.reset(); } void ScriptInstance::Continue() @@ -165,11 +162,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() @@ -316,9 +311,10 @@ void ScriptInstance::CollectGarbage() } -ScriptStorage *ScriptInstance::GetStorage() +ScriptStorage &ScriptInstance::GetStorage() { - return this->storage; + assert(this->storage != nullptr); + return *this->storage; } ScriptLogTypes::LogData &ScriptInstance::GetLogData() diff --git a/src/script/script_instance.hpp b/src/script/script_instance.hpp index 247d89363a..28ebb7f260 100644 --- a/src/script/script_instance.hpp +++ b/src/script/script_instance.hpp @@ -91,7 +91,7 @@ public: /** * Get the storage of this script. */ - class ScriptStorage *GetStorage(); + class ScriptStorage &GetStorage(); /** * Get the log pointer of this script. @@ -146,7 +146,11 @@ public: /** * Get the controller attached to the instance. */ - class ScriptController *GetController() { return controller; } + class ScriptController &GetController() + { + assert(this->controller != nullptr); + return *this->controller; + } /** * Return the "this script died" value @@ -252,7 +256,6 @@ public: void ReleaseSQObject(HSQOBJECT *obj); protected: - class Squirrel *engine = nullptr; ///< A wrapper around the squirrel vm. std::string api_version{}; ///< Current API used by this script. /** @@ -283,10 +286,20 @@ protected: */ virtual void LoadDummyScript() = 0; + /** + * Get the storage of this script. + */ + inline class Squirrel &GetEngine() + { + assert(this->engine != nullptr); + return *this->engine; + } + 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 engine; ///< A wrapper around the squirrel vm. + 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. diff --git a/src/script/squirrel.cpp b/src/script/squirrel.cpp index 2086e74f75..3484454c15 100644 --- a/src/script/squirrel.cpp +++ b/src/script/squirrel.cpp @@ -536,7 +536,7 @@ void Squirrel::Initialize() sq_setforeignptr(this->vm, this); sq_pushroottable(this->vm); - squirrel_register_global_std(this); + squirrel_register_global_std(*this); /* Set consts table as delegate of root table, so consts/enums defined via require() are accessible */ sq_pushconsttable(this->vm); diff --git a/src/script/squirrel_std.cpp b/src/script/squirrel_std.cpp index d00c2ca023..d400597586 100644 --- a/src/script/squirrel_std.cpp +++ b/src/script/squirrel_std.cpp @@ -82,20 +82,20 @@ SQInteger SquirrelStd::notifyallexceptions(HSQUIRRELVM vm) return SQ_ERROR; } -void squirrel_register_global_std(Squirrel *engine) +void squirrel_register_global_std(Squirrel &engine) { /* We don't use squirrel_helper here, as we want to register to the global * scope and not to a class. */ - engine->AddMethod("require", &SquirrelStd::require, ".s"); - engine->AddMethod("notifyallexceptions", &SquirrelStd::notifyallexceptions, ".b"); + engine.AddMethod("require", &SquirrelStd::require, ".s"); + engine.AddMethod("notifyallexceptions", &SquirrelStd::notifyallexceptions, ".b"); } -void squirrel_register_std(Squirrel *engine) +void squirrel_register_std(Squirrel &engine) { /* We don't use squirrel_helper here, as we want to register to the global * scope and not to a class. */ - engine->AddMethod("min", &SquirrelStd::min, ".ii"); - engine->AddMethod("max", &SquirrelStd::max, ".ii"); + engine.AddMethod("min", &SquirrelStd::min, ".ii"); + engine.AddMethod("max", &SquirrelStd::max, ".ii"); - sqstd_register_mathlib(engine->GetVM()); + sqstd_register_mathlib(engine.GetVM()); } diff --git a/src/script/squirrel_std.hpp b/src/script/squirrel_std.hpp index 5e977045f8..1a208eeae1 100644 --- a/src/script/squirrel_std.hpp +++ b/src/script/squirrel_std.hpp @@ -52,12 +52,12 @@ public: /** * Register all standard functions we want to give to a script. */ -void squirrel_register_std(Squirrel *engine); +void squirrel_register_std(Squirrel &engine); /** * Register all standard functions that are available on first startup. * @note this set is very limited, and is only meant to load other scripts and things like that. */ -void squirrel_register_global_std(Squirrel *engine); +void squirrel_register_global_std(Squirrel &engine); #endif /* SQUIRREL_STD_HPP */