From ead31e63f2a6030bbfc8cf7d10d38ba3d126ed6c Mon Sep 17 00:00:00 2001 From: hispidence Date: Wed, 4 Aug 2021 16:51:28 +0100 Subject: [PATCH] Replace WarningsAsErrors usage with ScriptAssert usage. For recoverable errors, add some recovery behaviour and logging so a level designer can see in the log what was done as a result of the error. Warn about default behaviour if no callbacks are added. --- TR5Main/Game/control.cpp | 2 +- TR5Main/Scripting/GameLogicScript.cpp | 33 +++++++++---------- TR5Main/Scripting/GameScriptAIObject.cpp | 5 ++- TR5Main/Scripting/GameScriptCameraInfo.cpp | 6 ++-- TR5Main/Scripting/GameScriptItemInfo.cpp | 13 +++----- TR5Main/Scripting/GameScriptMeshInfo.cpp | 6 ++-- TR5Main/Scripting/GameScriptNamedBase.h | 20 +++-------- TR5Main/Scripting/GameScriptSinkInfo.cpp | 6 ++-- .../Scripting/GameScriptSoundSourceInfo.cpp | 6 ++-- 9 files changed, 36 insertions(+), 61 deletions(-) diff --git a/TR5Main/Game/control.cpp b/TR5Main/Game/control.cpp index 8c76c9186..28615d8b3 100644 --- a/TR5Main/Game/control.cpp +++ b/TR5Main/Game/control.cpp @@ -650,7 +650,7 @@ unsigned CALLBACK GameMain(void *) // Initialise legacy memory buffer and game timer init_game_malloc(); TIME_Init(); - if (g_GameFlow->IntroImagePath.empty() && WarningsAsErrors) + if (g_GameFlow->IntroImagePath.empty()) { throw TENScriptException("Intro image path is not set."); } diff --git a/TR5Main/Scripting/GameLogicScript.cpp b/TR5Main/Scripting/GameLogicScript.cpp index ff33b9d02..3e25790c1 100644 --- a/TR5Main/Scripting/GameLogicScript.cpp +++ b/TR5Main/Scripting/GameLogicScript.cpp @@ -1,5 +1,6 @@ #include "framework.h" #include "GameLogicScript.h" +#include "ScriptAssert.h" #include "items.h" #include "box.h" #include "lara.h" @@ -28,7 +29,6 @@ functions and callbacks for game specific scripts extern GameFlow* g_GameFlow; GameScript* g_GameScript; -extern bool const WarningsAsErrors = true; GameScript::GameScript(sol::state* lua) : LuaHandler{ lua }, m_itemsMapId{}, m_itemsMapName{}, m_meshesMapName{} { @@ -621,8 +621,7 @@ void LuaVariables::SetVariable(std::string key, sol::object value) variables[key] = value; break; default: - if (WarningsAsErrors) - throw std::runtime_error("unsupported variable type"); + ScriptAssert(false, "Variable " + key + " has an unsupported type.", ERROR_MODE::TERMINATE); break; } } @@ -631,19 +630,19 @@ void GameScript::ExecuteFunction(std::string const & name) { sol::protected_function func = (*m_lua)[name.c_str()]; auto r = func(); - if (WarningsAsErrors && !r.valid()) + if (!r.valid()) { sol::error err = r; - throw TENScriptException(err.what()); + ScriptAssert(false, err.what(), ERROR_MODE::TERMINATE); } } static void doCallback(sol::protected_function const & func) { auto r = func(); - if (WarningsAsErrors && !r.valid()) + if (!r.valid()) { sol::error err = r; - throw TENScriptException(err.what()); + ScriptAssert(false, err.what(), ERROR_MODE::TERMINATE); } } @@ -655,40 +654,38 @@ void GameScript::OnStart() void GameScript::OnLoad() { - if (m_onLoad.valid()) + if(m_onLoad.valid()) doCallback(m_onLoad); } void GameScript::OnControlPhase() { - if (m_onControlPhase.valid()) + if(m_onControlPhase.valid()) doCallback(m_onControlPhase); } void GameScript::OnSave() { - if (m_onSave.valid()) + if(m_onSave.valid()) doCallback(m_onSave); } void GameScript::OnEnd() { - if (m_onEnd.valid()) + if(m_onEnd.valid()) doCallback(m_onEnd); } void GameScript::InitCallbacks() { - auto assignCB = [this](sol::protected_function& func, char const* luaFunc) { + auto assignCB = [this](sol::protected_function& func, std::string const & luaFunc) { func = (*m_lua)[luaFunc]; - if (WarningsAsErrors && !func.valid()) - { - std::string err{ "Level's script file requires callback \"" }; - err += std::string{ luaFunc }; - err += "\""; - throw TENScriptException(err); + std::string err{ "Level's script does not define callback " + luaFunc}; + if (!ScriptAssert(func.valid(), err)) { + TENLog("Defaulting to no " + luaFunc + " behaviour.", LogLevel::Warning, LogConfig::All); } }; + assignCB(m_onStart, "OnStart"); assignCB(m_onLoad, "OnLoad"); assignCB(m_onControlPhase, "OnControlPhase"); diff --git a/TR5Main/Scripting/GameScriptAIObject.cpp b/TR5Main/Scripting/GameScriptAIObject.cpp index ff4af6571..347b95978 100644 --- a/TR5Main/Scripting/GameScriptAIObject.cpp +++ b/TR5Main/Scripting/GameScriptAIObject.cpp @@ -1,6 +1,7 @@ #pragma once #include "framework.h" #include "GameScriptAIObject.h" +#include "ScriptAssert.h" #include "GameScriptPosition.h" #include /*** @@ -10,7 +11,6 @@ AI object @pragma nostrip */ -extern bool const WarningsAsErrors; constexpr auto LUA_CLASS_NAME{ "AIObject" }; @@ -105,8 +105,7 @@ std::string GameScriptAIObject::GetName() const void GameScriptAIObject::SetName(std::string const & id) { - if (id.empty() && WarningsAsErrors) - throw TENScriptException("Name cannot be blank"); + ScriptAssert(!id.empty(), "Name cannot be blank", ERROR_MODE::TERMINATE); // remove the old name if we have one s_callbackRemoveName(m_aiObject.luaName); diff --git a/TR5Main/Scripting/GameScriptCameraInfo.cpp b/TR5Main/Scripting/GameScriptCameraInfo.cpp index 21628a481..33c676bfa 100644 --- a/TR5Main/Scripting/GameScriptCameraInfo.cpp +++ b/TR5Main/Scripting/GameScriptCameraInfo.cpp @@ -1,4 +1,5 @@ #include "framework.h" +#include "ScriptAssert.h" #include "GameScriptCameraInfo.h" #include "GameScriptPosition.h" /*** @@ -8,8 +9,6 @@ Camera info @pragma nostrip */ -extern bool const WarningsAsErrors; - static constexpr auto LUA_CLASS_NAME{ "CameraInfo" }; static auto index_error = index_error_maker(GameScriptCameraInfo, LUA_CLASS_NAME); @@ -63,8 +62,7 @@ std::string GameScriptCameraInfo::GetName() const void GameScriptCameraInfo::SetName(std::string const & id) { - if (id.empty() && WarningsAsErrors) - throw TENScriptException("Name cannot be blank"); + ScriptAssert(!id.empty(), "Name cannot be blank", ERROR_MODE::TERMINATE); // remove the old name if we have one s_callbackRemoveName(m_camera.luaName); diff --git a/TR5Main/Scripting/GameScriptItemInfo.cpp b/TR5Main/Scripting/GameScriptItemInfo.cpp index 1c278291e..d61d1856b 100644 --- a/TR5Main/Scripting/GameScriptItemInfo.cpp +++ b/TR5Main/Scripting/GameScriptItemInfo.cpp @@ -1,4 +1,5 @@ #include "framework.h" +#include "ScriptAssert.h" #include "GameScriptItemInfo.h" #include "items.h" #include "objectslist.h" @@ -19,8 +20,6 @@ pickups, and Lara herself. @pragma nostrip */ -extern bool const WarningsAsErrors; - constexpr auto LUA_CLASS_NAME{ "ItemInfo" }; static auto index_error = index_error_maker(GameScriptItemInfo, LUA_CLASS_NAME); @@ -294,8 +293,7 @@ std::string GameScriptItemInfo::GetName() const void GameScriptItemInfo::SetName(std::string const & id) { - if (id.empty() && WarningsAsErrors) - throw std::runtime_error("Name cannot be blank"); + ScriptAssert(!id.empty(), "Name cannot be blank", ERROR_MODE::TERMINATE); // remove the old name if we have one s_callbackRemoveName(m_item->luaName); @@ -347,8 +345,7 @@ void GameScriptItemInfo::SetHP(short hp) if(Objects[m_item->objectNumber].intelligent && (hp < 0 || hp > Objects[m_item->objectNumber].hitPoints)) { - if (WarningsAsErrors) - throw TENScriptException("invalid HP"); + ScriptAssert(false, "Invalid value: " + hp); if (hp < 0) { hp = 0; @@ -484,8 +481,8 @@ void GameScriptItemInfo::SetRoom(short room) { if (room < 0 || static_cast(room) >= g_Level.Rooms.size()) { - if (WarningsAsErrors) - throw TENScriptException("invalid room number"); + ScriptAssert(false, "Invalid room number: " + room); + TENLog("Room number will not be set", LogLevel::Warning, LogConfig::All); return; } diff --git a/TR5Main/Scripting/GameScriptMeshInfo.cpp b/TR5Main/Scripting/GameScriptMeshInfo.cpp index dd706e4eb..16b45127a 100644 --- a/TR5Main/Scripting/GameScriptMeshInfo.cpp +++ b/TR5Main/Scripting/GameScriptMeshInfo.cpp @@ -1,5 +1,6 @@ #pragma once #include "framework.h" +#include "ScriptAssert.h" #include "GameScriptMeshInfo.h" #include "GameScriptPosition.h" #include "GameScriptColor.h" @@ -11,8 +12,6 @@ Mesh info @pragma nostrip */ -extern bool const WarningsAsErrors; - constexpr auto LUA_CLASS_NAME{ "MeshInfo" }; static auto index_error = index_error_maker(GameScriptMeshInfo, LUA_CLASS_NAME); @@ -88,8 +87,7 @@ std::string GameScriptMeshInfo::GetName() const void GameScriptMeshInfo::SetName(std::string const & id) { - if (id.empty() && WarningsAsErrors) - throw TENScriptException("Name cannot be blank"); + ScriptAssert(!id.empty(), "Name cannot be blank", ERROR_MODE::TERMINATE); // remove the old name if we have one s_callbackRemoveName(m_mesh.luaName); diff --git a/TR5Main/Scripting/GameScriptNamedBase.h b/TR5Main/Scripting/GameScriptNamedBase.h index 920b6f147..3c1ee3b63 100644 --- a/TR5Main/Scripting/GameScriptNamedBase.h +++ b/TR5Main/Scripting/GameScriptNamedBase.h @@ -1,16 +1,13 @@ #pragma once +#include "ScriptAssert.h" #include #include -extern bool const WarningsAsErrors; #define index_error_maker(CPP_TYPE, LUA_CLASS_NAME) [](CPP_TYPE & item, sol::object key) \ { \ - if (WarningsAsErrors) \ - { \ - std::string err = "Attempted to read non-existant var \"" + key.as() + "\" from " + LUA_CLASS_NAME; \ - throw std::runtime_error(err); \ - } \ + std::string err = "Attempted to read non-existant var \"" + key.as() + "\" from " + LUA_CLASS_NAME; \ + throw TENScriptException(err); \ } template using callbackSetName = std::function; @@ -37,20 +34,13 @@ protected: // default callbacks template callbackSetName GameScriptNamedBase::s_callbackSetName = [](std::string const& n, S identifier) { std::string err = "\"Set Name\" callback is not set."; - if (WarningsAsErrors) - { - throw TENScriptException(err); - } + throw TENScriptException(err); return false; }; template callbackRemoveName GameScriptNamedBase::s_callbackRemoveName = [](std::string const& n) { std::string err = "\"Remove Name\" callback is not set."; - if (WarningsAsErrors) - { - throw TENScriptException(err); - } + throw TENScriptException(err); return false; - }; diff --git a/TR5Main/Scripting/GameScriptSinkInfo.cpp b/TR5Main/Scripting/GameScriptSinkInfo.cpp index 7234878e5..89671fece 100644 --- a/TR5Main/Scripting/GameScriptSinkInfo.cpp +++ b/TR5Main/Scripting/GameScriptSinkInfo.cpp @@ -1,5 +1,6 @@ #pragma once #include "framework.h" +#include "ScriptAssert.h" #include "GameScriptSinkInfo.h" #include "GameScriptPosition.h" #include @@ -10,8 +11,6 @@ Sink info @pragma nostrip */ -extern bool const WarningsAsErrors; - constexpr auto LUA_CLASS_NAME{ "SinkInfo" }; static auto index_error = index_error_maker(GameScriptSinkInfo, LUA_CLASS_NAME); @@ -71,8 +70,7 @@ std::string GameScriptSinkInfo::GetName() const void GameScriptSinkInfo::SetName(std::string const & id) { - if (id.empty() && WarningsAsErrors) - throw TENScriptException("Name cannot be blank"); + ScriptAssert(!id.empty(), "Name cannot be blank", ERROR_MODE::TERMINATE); // remove the old name if we have one s_callbackRemoveName(m_sink.luaName); diff --git a/TR5Main/Scripting/GameScriptSoundSourceInfo.cpp b/TR5Main/Scripting/GameScriptSoundSourceInfo.cpp index b17c49f56..3c0c17389 100644 --- a/TR5Main/Scripting/GameScriptSoundSourceInfo.cpp +++ b/TR5Main/Scripting/GameScriptSoundSourceInfo.cpp @@ -1,4 +1,5 @@ #include "framework.h" +#include "ScriptAssert.h" #include "GameScriptSoundSourceInfo.h" #include "GameScriptPosition.h" /*** @@ -8,8 +9,6 @@ Sound source info @pragma nostrip */ -extern bool const WarningsAsErrors; - static constexpr auto LUA_CLASS_NAME{ "SoundSourceInfo" }; static auto index_error = index_error_maker(GameScriptSoundSourceInfo, LUA_CLASS_NAME); @@ -67,8 +66,7 @@ std::string GameScriptSoundSourceInfo::GetName() const void GameScriptSoundSourceInfo::SetName(std::string const & id) { - if (id.empty() && WarningsAsErrors) - throw TENScriptException("Name cannot be blank"); + ScriptAssert(!id.empty(), "Name cannot be blank", ERROR_MODE::TERMINATE); // remove the old name if we have one s_callbackRemoveName(m_soundSource.luaName);