Fixes for valgrind failure.

.drone.yml

@@ -41,7 +41,7 @@ steps:
       - name: conan-data
         path: /root/.conan/data
     commands:
-      - valgrind --tool=memcheck --gen-suppressions=all --leak-check=full --leak-resolution=med --track-origins=yes --vgdb=no --error-exitcode=1 build-release/bin/pkmnLibTests
+      - valgrind --tool=memcheck --gen-suppressions=all --leak-check=full --leak-resolution=med --track-origins=yes --vgdb=no --error-exitcode=1 --suppressions=angelscript.supp build-release/bin/pkmnLibTests
   - name: test-release-windows
     image: deukhoofd/windowsbuilder
     volumes:

CMakeLists.txt

@@ -38,7 +38,7 @@ if (NOT EXISTS ${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
     endif()
     if (NOT WINDOWS)
         execute_process(COMMAND conan install ${CMAKE_SOURCE_DIR} --install-folder=${CMAKE_BINARY_DIR} --build missing
-                -s compiler=clang -s compiler.libcxx=libstdc++11 -s compiler.version=${VERSION})
+                -s compiler=clang -s compiler.libcxx=libstdc++11 -s compiler.version=${VERSION} -s build_type=Debug)
     else()
         execute_process(COMMAND conan install ${CMAKE_SOURCE_DIR} --install-folder=${CMAKE_BINARY_DIR} --build missing
                 -s compiler=gcc -s compiler.libcxx=libstdc++11 -s compiler.version=${VERSION} -s os=Windows)
@@ -66,6 +66,7 @@ if (SCRIPT_PROVIDER STREQUAL "angelscript")
             "extern/angelscript_addons/*.cpp"
             "extern/angelscript_addons/*.h"
             )
+    ADD_DEFINITIONS(-D AS_USE_ACCESSORS=1)
 endif()
 message(STATUS "${FILE_SOURCE}")
 file(GLOB_RECURSE CORE_SRC_FILES ${FILE_SOURCE})

angelscript.supp

@@ -0,0 +1,6 @@
+{
+   SkipAngelScriptMemCheck
+   Memcheck:Cond
+   fun:_ZN10asCContext11ExecuteNextEv
+   fun:_ZN10asCContext7ExecuteEv
+}

conanfile.py

@@ -7,7 +7,7 @@ class PkmnLibConan(ConanFile):
     license = "TODO"
     url = "https://git.p-epsilon.com/Deukhoofd/CreatureLib"
     description = "The core implementation for turn based battling using creatures."
-    settings = "os", "compiler"
+    settings = "os", "compiler", "build_type"
     options = {"shared": [True, False], "script_handler": ["angelscript"]}
     default_options = {"shared": True, "script_handler": "angelscript"}
     generators = "cmake"
@@ -40,7 +40,7 @@ class PkmnLibConan(ConanFile):
                 self.options["AngelScript"].link_std_statically = True
 
     def requirements(self):
-            self.requires("CreatureLib/cd7ddcf78ec0ee4ae645d1862f92ec23246af949@creaturelib/master")
+            self.requires("CreatureLib/e45a36d78eda2b4236f13bdb03004493e309e1ca@creaturelib/master")
             if self.options.script_handler == "angelscript":
                 self.requires("AngelScript/2.34@AngelScript/Deukhoofd")
             else:

extern/angelscript_addons/scriptstdstring/scriptstdstring.cpp

@@ -31,22 +31,22 @@ END_AS_NAMESPACE
 class CStdStringFactory : public asIStringFactory
 {
 public:
-	CStdStringFactory() {}
+	CStdStringFactory() = default;
-	~CStdStringFactory() 
+	~CStdStringFactory() override
 	{
 		// The script engine must release each string 
 		// constant that it has requested
-		assert(stringCache.size() == 0);
+		assert(stringCache.empty());
 	}
 
-	const void *GetStringConstant(const char *data, asUINT length)
+	const void *GetStringConstant(const char *data, asUINT length) override
 	{
 		// The string factory might be modified from multiple 
 		// threads, so it is necessary to use a mutex.
 		asAcquireExclusiveLock();
 		
 		string str(data, length);
-		map_t::iterator it = stringCache.find(str);
+		auto it = stringCache.find(str);
 		if (it != stringCache.end())
 			it->second++;
 		else
@@ -57,9 +57,9 @@ public:
 		return reinterpret_cast<const void*>(&it->first);
 	}
 
-	int  ReleaseStringConstant(const void *str)
+	int  ReleaseStringConstant(const void *str) override
 	{
-		if (str == 0)
+		if (str == nullptr)
 			return asERROR;
 
 		int ret = asSUCCESS;
@@ -68,7 +68,7 @@ public:
 		// threads, so it is necessary to use a mutex.
 		asAcquireExclusiveLock();
 		
-		map_t::iterator it = stringCache.find(*reinterpret_cast<const string*>(str));
+		auto it = stringCache.find(*reinterpret_cast<const string*>(str));
 		if (it == stringCache.end())
 			ret = asERROR;
 		else
@@ -83,9 +83,9 @@ public:
 		return ret;
 	}
 
-	int  GetRawStringData(const void *str, char *data, asUINT *length) const
+	int  GetRawStringData(const void *str, char *data, asUINT *length) const override
 	{
-		if (str == 0)
+		if (str == nullptr)
 			return asERROR;
 
 		if (length)
@@ -98,17 +98,17 @@ public:
 	}
 
 	// THe access to the string cache is protected with the common mutex provided by AngelScript
-	map_t stringCache;
+	map_t stringCache = {};
 };
 
-static CStdStringFactory *stringFactory = 0;
+static CStdStringFactory *stringFactory = nullptr;
 
 // TODO: Make this public so the application can also use the string 
 //       factory and share the string constants if so desired, or to
 //       monitor the size of the string factory cache.
 CStdStringFactory *GetStdStringFactorySingleton()
 {
-	if( stringFactory == 0 )
+	if( stringFactory == nullptr )
 	{
 		// The following instance will be destroyed by the global 
 		// CStdStringFactoryCleaner instance upon application shutdown
@@ -122,7 +122,7 @@ class CStdStringFactoryCleaner
 public:
 	~CStdStringFactoryCleaner()
 	{
-		if (stringFactory)
+		if (stringFactory != nullptr)
 		{
 			// Only delete the string factory if the stringCache is empty
 			// If it is not empty, it means that someone might still attempt
@@ -133,13 +133,13 @@ public:
 			if (stringFactory->stringCache.empty())
 			{
 				delete stringFactory;
-				stringFactory = 0;
+				stringFactory = nullptr;
 			}
 		}
 	}
 };
 
-static CStdStringFactoryCleaner cleaner;
+static CStdStringFactoryCleaner cleaner = {};
 
 
 static void ConstructString(string *thisPointer)
@@ -461,12 +461,12 @@ static void StringResize(asUINT l, string &str)
 // string formatInt(int64 val, const string &in options, uint width)
 static string formatInt(asINT64 value, const string &options, asUINT width)
 {
-	bool leftJustify = options.find("l") != string::npos;
+	bool leftJustify = options.find('l') != string::npos;
-	bool padWithZero = options.find("0") != string::npos;
+	bool padWithZero = options.find('0') != string::npos;
-	bool alwaysSign  = options.find("+") != string::npos;
+	bool alwaysSign  = options.find('+') != string::npos;
-	bool spaceOnSign = options.find(" ") != string::npos;
+	bool spaceOnSign = options.find(' ') != string::npos;
-	bool hexSmall    = options.find("h") != string::npos;
+	bool hexSmall    = options.find('h') != string::npos;
-	bool hexLarge    = options.find("H") != string::npos;
+	bool hexLarge    = options.find('H') != string::npos;
 
 	string fmt = "%";
 	if( leftJustify ) fmt += "-";
@@ -505,12 +505,12 @@ static string formatInt(asINT64 value, const string &options, asUINT width)
 // string formatUInt(uint64 val, const string &in options, uint width)
 static string formatUInt(asQWORD value, const string &options, asUINT width)
 {
-	bool leftJustify = options.find("l") != string::npos;
+	bool leftJustify = options.find('l') != string::npos;
-	bool padWithZero = options.find("0") != string::npos;
+	bool padWithZero = options.find('0') != string::npos;
-	bool alwaysSign  = options.find("+") != string::npos;
+	bool alwaysSign  = options.find('+') != string::npos;
-	bool spaceOnSign = options.find(" ") != string::npos;
+	bool spaceOnSign = options.find(' ') != string::npos;
-	bool hexSmall    = options.find("h") != string::npos;
+	bool hexSmall    = options.find('h') != string::npos;
-	bool hexLarge    = options.find("H") != string::npos;
+	bool hexLarge    = options.find('H') != string::npos;
 
 	string fmt = "%";
 	if( leftJustify ) fmt += "-";
@@ -549,12 +549,12 @@ static string formatUInt(asQWORD value, const string &options, asUINT width)
 // string formatFloat(double val, const string &in options, uint width, uint precision)
 static string formatFloat(double value, const string &options, asUINT width, asUINT precision)
 {
-	bool leftJustify = options.find("l") != string::npos;
+	bool leftJustify = options.find('l') != string::npos;
-	bool padWithZero = options.find("0") != string::npos;
+	bool padWithZero = options.find('0') != string::npos;
-	bool alwaysSign  = options.find("+") != string::npos;
+	bool alwaysSign  = options.find('+') != string::npos;
-	bool spaceOnSign = options.find(" ") != string::npos;
+	bool spaceOnSign = options.find(' ') != string::npos;
-	bool expSmall    = options.find("e") != string::npos;
+	bool expSmall    = options.find('e') != string::npos;
-	bool expLarge    = options.find("E") != string::npos;
+	bool expLarge    = options.find('E') != string::npos;
 
 	string fmt = "%";
 	if( leftJustify ) fmt += "-";
@@ -734,7 +734,7 @@ static string StringSubString(asUINT start, int count, const string &str)
 // makro, so this wrapper was introduced as work around.
 static bool StringEquals(const std::string& lhs, const std::string& rhs)
 {
-	return lhs == rhs;
+    return lhs == rhs;
 }
 
 void RegisterStdString_Native(asIScriptEngine *engine)

src/ScriptResolving/AngelScript/AngelScripResolver.cpp

@@ -1,11 +1,9 @@
 #include "AngelScripResolver.hpp"
-#define AS_USE_ACCESSORS
 #include "../../../extern/angelscript_addons/scriptarray/scriptarray.h"
-#undef AS_USE_ACCESSORS
-#include <cassert>
 #include "../../../extern/angelscript_addons/scripthandle/scripthandle.h"
 #include "../../../extern/angelscript_addons/scripthelper/scripthelper.h"
 #include "../../../extern/angelscript_addons/scriptstdstring/scriptstdstring.h"
+#include <cassert>
 #include "TypeRegistry/BasicScriptClass.hpp"
 #include "TypeRegistry/Battling/RegisterExecutingAttack.hpp"
 #include "TypeRegistry/Battling/RegisterPokemonClass.hpp"
@@ -35,6 +33,7 @@ void AngelScripResolver::Initialize(CreatureLib::Battling::BattleLibrary* arg) {
     _engine->SetEngineProperty(asEP_AUTO_GARBAGE_COLLECT, false);
     _engine->SetEngineProperty(asEP_REQUIRE_ENUM_SCOPE, true);
     _engine->SetEngineProperty(asEP_PROPERTY_ACCESSOR_MODE, 2);
+    _engine->SetEngineProperty(asEP_COMPILER_WARNINGS , 2);
 
 
     RegisterStdString(_engine);

src/ScriptResolving/AngelScript/AngelScriptScript.hpp

@@ -43,7 +43,9 @@ public:
     }                                                                                                                  \
     _ctxPool->ReturnContextToPool(ctx);
 
-    void Stack() override { CALLHOOK(Stack, {}); }
+    void Stack() override { CALLHOOK(Stack,); }
+
+    void OnRemove() override { CALLHOOK(OnRemove,); }
 
     void OnBeforeTurn(const CreatureLib::Battling::BaseTurnChoice* choice) override {
         throw NotImplementedException(); // TODO

src/ScriptResolving/AngelScript/AngelScriptTypeInfo.hpp

@@ -70,6 +70,7 @@ public:
     const FunctionInfo& Get##name() const { return __##name; }
 
     SCRIPT_HOOK_FUNCTION(Stack, "void Stack()");
+    SCRIPT_HOOK_FUNCTION(OnRemove, "void OnRemove()");
     SCRIPT_HOOK_FUNCTION(PreventAttack, "void PreventAttack(ExecutingMove@ attack, bool& result)");
     SCRIPT_HOOK_FUNCTION(FailAttack, "void FailAttack(ExecutingMove@ attack, bool& result)");
     SCRIPT_HOOK_FUNCTION(StopBeforeAttack, "void StopBeforeAttack(ExecutingMove@ attack, bool& result)");

src/ScriptResolving/AngelScript/TypeRegistry/BasicScriptClass.cpp

@@ -7,6 +7,7 @@ void BasicScriptClass::Register(asIScriptEngine* engine) {
     [[maybe_unused]] int r = engine->GetModuleByIndex(0)->AddScriptSection("PkmnScript", R"(
 shared abstract class PkmnScript {
     void Stack(){};
+    void OnRemove(){};
     void PreventAttack(ExecutingMove@ attack, bool& result){};
     void FailAttack(ExecutingMove@ attack, bool& result){};
     void StopBeforeAttack(ExecutingMove@ attack, bool& result){};

tests/ScriptTests/BaseScriptClassTests.cpp

@@ -10,6 +10,7 @@
 static std::unordered_map<const char*, const char*> _scripts = std::unordered_map<const char*, const char*>{
     AS_CLASS(blankScript, ),
     AS_CLASS(stackScript, "int value = 0; void Stack() override { value++; } int GetValue() { return value; }"),
+    AS_CLASS(onRemoveScript, "int value = 0; void OnRemove() override { value++; } int GetValue() { return value; }"),
     {"doubleInheritanceScript", R"(
 class doubleInheritanceScriptBase : PkmnScript {
     int value = 0;
@@ -105,6 +106,22 @@ TEST_CASE("Invoke Stack script function") {
     delete script;
 }
 
+TEST_CASE("Invoke OnRemove script function") {
+    auto mainLib = TestLibrary::GetLibrary();
+    auto script = GetScript(mainLib, "onRemoveScript");
+    script->OnRemove();
+
+    auto ctxPool = script->GetContextPool();
+    auto ctx = ctxPool->RequestContext();
+    script->PrepareMethod("GetValue", ctx);
+    REQUIRE(ctx->Execute() == asEXECUTION_FINISHED);
+    REQUIRE(ctx->GetReturnDWord() == 1);
+    ctxPool->ReturnContextToPool(ctx);
+
+    delete script;
+}
+
+
 TEST_CASE("Invoke Stack script function with implementation in base class") {
     auto mainLib = TestLibrary::GetLibrary();
     auto script = GetScript(mainLib, "doubleInheritanceScript");

tests/ScriptTests/ScriptTypeTests/Battle/PokemonTests.cpp

@@ -15,7 +15,7 @@ class testScript1 {
     bool testShiny(Pokemon@ p, bool shiny){ return p.Shiny == shiny; }
     bool testHeldItem(Pokemon@ p, Item@ item){ return p.HeldItem is item; }
     bool testCurrentHealth(Pokemon@ p, uint health){ return p.CurrentHealth == health; }
-    bool testNickname(Pokemon@ p, const string &in name){ return p.Nickname == name; }
+    bool testNickname(Pokemon@ p, const string& name){ return p.Nickname == name; }
     bool testActiveAbility(Pokemon@ p, const string &in ability){ return p.ActiveAbility == ability; }
     bool testIsFainted(Pokemon@ p, bool b){ return p.IsFainted == b; }
     bool testType(Pokemon@ p, uint index, uint8 type){ return p.GetTypes()[index] == type; }
@@ -29,10 +29,10 @@ class testScript1 {
 static const char* _testLoadFunc(const char* name) { return _scripts[name]; }
 
 struct ScriptData {
-    AngelScriptScript* Script;
+    AngelScriptScript* Script = nullptr;
-    AngelScripResolver* Resolver;
+    AngelScripResolver* Resolver = nullptr;
-    asIScriptFunction* Func;
+    asIScriptFunction* Func = nullptr;
-    asIScriptContext* Context;
+    asIScriptContext* Context = nullptr;
 
     ~ScriptData() {
         Script->GetContextPool()->ReturnContextToPool(Context);
@@ -207,7 +207,7 @@ TEST_CASE("Validate Pokemon Active Ability in Script") {
                    .WithForme("default")
                    ->WithGender(CreatureLib::Library::Gender::Male)
                    ->Build();
-    data.Context->SetArgObject(0, const_cast<PkmnLib::Battling::Pokemon*>(mon));
+    data.Context->SetArgObject(0, (void*)mon);
     auto name = mon->GetActiveTalent();
     data.Context->SetArgAddress(1, &name);
 

tests/ScriptTests/ScriptTypeTests/Library/FormesTests.cpp

@@ -20,10 +20,10 @@ class testScript1 {
 static const char* _testLoadFunc(const char* name) { return _scripts[name]; }
 
 struct ScriptData {
-    AngelScriptScript* Script;
+    AngelScriptScript* Script = nullptr;
-    AngelScripResolver* Resolver;
+    AngelScripResolver* Resolver = nullptr;
-    asIScriptFunction* Func;
+    asIScriptFunction* Func = nullptr;
-    asIScriptContext* Context;
+    asIScriptContext* Context = nullptr;
 
     ~ScriptData() {
         Script->GetContextPool()->ReturnContextToPool(Context);
@@ -32,8 +32,8 @@ struct ScriptData {
 };
 
 static AngelScripResolver* _resolverCache = nullptr;
-static AngelScripResolver* GetScriptResolver(PkmnLib::Battling::BattleLibrary* mainLib){
+static AngelScripResolver* GetScriptResolver(PkmnLib::Battling::BattleLibrary* mainLib) {
-    if (_resolverCache == nullptr){
+    if (_resolverCache == nullptr) {
         _resolverCache = dynamic_cast<AngelScripResolver*>(PkmnLib::Battling::BattleLibrary::CreateScriptResolver());
         _resolverCache->Initialize(mainLib);
         _resolverCache->SetCreateFunction(&_testLoadFunc);
@@ -171,5 +171,4 @@ TEST_CASE("Validate Forme GetAbility in Script") {
     REQUIRE((bool)data.Context->GetReturnWord());
 }
 
-
 #endif

tests/ScriptTests/ScriptTypeTests/Library/ItemDataTests.cpp

@@ -16,10 +16,10 @@ class testScript1 {
 static const char* _testLoadFunc(const char* name) { return _scripts[name]; }
 
 struct ScriptData {
-    AngelScriptScript* Script;
+    AngelScriptScript* Script = nullptr;
-    AngelScripResolver* Resolver;
+    AngelScripResolver* Resolver = nullptr;
-    asIScriptFunction* Func;
+    asIScriptFunction* Func = nullptr;
-    asIScriptContext* Context;
+    asIScriptContext* Context = nullptr;
 
     ~ScriptData() {
         Script->GetContextPool()->ReturnContextToPool(Context);

tests/ScriptTests/ScriptTypeTests/Library/MoveTests.cpp

@@ -20,10 +20,10 @@ class testScript1 {
 static const char* _testLoadFunc(const char* name) { return _scripts[name]; }
 
 struct ScriptData {
-    AngelScriptScript* Script;
+    AngelScriptScript* Script = nullptr;
-    AngelScripResolver* Resolver;
+    AngelScripResolver* Resolver = nullptr;
-    asIScriptFunction* Func;
+    asIScriptFunction* Func = nullptr;
-    asIScriptContext* Context;
+    asIScriptContext* Context = nullptr;
 
     ~ScriptData() {
         Script->GetContextPool()->ReturnContextToPool(Context);
@@ -61,7 +61,7 @@ TEST_CASE("Validate Move Name in Script") {
     auto data = GetScript(mainLib, "testName");
 
     auto move = mainLib->GetMoveLibrary()->GetAttack("testMove");
-    data.Context->SetArgObject(0, const_cast<PkmnLib::Library::MoveData*>(move));
+    data.Context->SetArgObject(0, (void*)move);
     auto name = move->GetName();
     data.Context->SetArgAddress(1, &name);