From c11da2d966795db6f5a5d77cf266033b0c8372be Mon Sep 17 00:00:00 2001 From: Deukhoofd Date: Thu, 16 Apr 2020 14:25:20 +0200 Subject: [PATCH] Changes for EffectParameter type in AngelScript, fixes for ConstString memory leak in AngelScript. --- src/Library/Evolutions/EvolutionData.cpp | 9 ++ src/Library/Evolutions/EvolutionData.hpp | 95 ++++++++++++------- src/Library/Species/PokemonSpecies.hpp | 12 ++- .../AngelScript/AngelScripResolver.cpp | 3 - .../AngelScript/TypeRegistry/ConstString.cpp | 12 ++- .../Library/RegisterEffectParameter.cpp | 11 ++- tests/LibraryTests/SpeciesTests.cpp | 7 +- tests/ScriptTests/BaseScriptClassTests.cpp | 20 ++-- 8 files changed, 105 insertions(+), 64 deletions(-) create mode 100644 src/Library/Evolutions/EvolutionData.cpp diff --git a/src/Library/Evolutions/EvolutionData.cpp b/src/Library/Evolutions/EvolutionData.cpp new file mode 100644 index 0000000..82fad07 --- /dev/null +++ b/src/Library/Evolutions/EvolutionData.cpp @@ -0,0 +1,9 @@ +#include "EvolutionData.hpp" +#include "../Moves/MoveData.hpp" +using namespace PkmnLib::Library; + +const EvolutionData* PkmnLib::Library::EvolutionData::CreateKnownMoveEvolution(const MoveData* move, + const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::KnownMove, {new CreatureLib::Library::EffectParameter(move->GetName())}, + into); +} diff --git a/src/Library/Evolutions/EvolutionData.hpp b/src/Library/Evolutions/EvolutionData.hpp index 9878c5b..945ffa2 100644 --- a/src/Library/Evolutions/EvolutionData.hpp +++ b/src/Library/Evolutions/EvolutionData.hpp @@ -1,73 +1,100 @@ #ifndef PKMNLIB_EVOLUTIONDATA_HPP #define PKMNLIB_EVOLUTIONDATA_HPP +#include +#include #include -#include #include #include -#include #include "../TimeOfDay.hpp" #include "EvolutionMethod.hpp" namespace PkmnLib::Library { class PokemonSpecies; - class Move; + class MoveData; class Item; class EvolutionData { private: const PokemonSpecies* _evolvesInto; EvolutionMethod _method; - std::vector _evolutionData; // This can probably be done in a better way, can't think of it now. + Arbutils::Collections::List _evolutionData; - EvolutionData(EvolutionMethod method, std::vector data, const PokemonSpecies* next) + EvolutionData(EvolutionMethod method, + Arbutils::Collections::List data, + const PokemonSpecies* next) : _evolvesInto(next), _method(method), _evolutionData(std::move(data)) {} public: - static EvolutionData CreateLevelEvolution(uint8_t level, const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::Level, {std::any(level)}, into); + ~EvolutionData() { + for (auto v : _evolutionData) { + delete v; + } } - static EvolutionData CreateFriendshipEvolution(uint8_t friendship, const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::HighFriendship, {std::any(friendship)}, into); + + static inline const EvolutionData* CreateLevelEvolution(uint8_t level, const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::Level, + {new CreatureLib::Library::EffectParameter((int64_t)level)}, into); } - static EvolutionData CreateKnownMoveEvolution(const Move* move, const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::KnownMove, {std::any(move)}, into); + static inline const EvolutionData* CreateFriendshipEvolution(uint8_t friendship, const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::HighFriendship, + {new CreatureLib::Library::EffectParameter((int64_t)friendship)}, into); } - static EvolutionData CreateLocationEvolution(const std::string& location, const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::LocationBased, {std::any(location)}, into); + static const EvolutionData* CreateKnownMoveEvolution(const MoveData* move, const PokemonSpecies* into); + static inline const EvolutionData* CreateLocationEvolution(const Arbutils::CaseInsensitiveConstString& location, + const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::LocationBased, + {new CreatureLib::Library::EffectParameter(location)}, into); } - static EvolutionData CreateTimeEvolution(TimeOfDay time, const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::TimeBased, {std::any(time)}, into); + static inline const EvolutionData* CreateTimeEvolution(TimeOfDay time, const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::TimeBased, + {new CreatureLib::Library::EffectParameter((int64_t)time)}, into); } - static EvolutionData CreateHeldItemEvolution(const Item* item, const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::HoldsItem, {std::any(item)}, into); + static inline const EvolutionData* CreateHeldItemEvolution(const Item* item, const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::HoldsItem, {new CreatureLib::Library::EffectParameter(item)}, + into); } - static EvolutionData CreateGenderBasedEvolution(CreatureLib::Library::Gender gender, uint8_t level, - const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::IsGenderAndLevel, {std::any(gender), std::any(level)}, into); + static inline const EvolutionData* CreateGenderBasedEvolution(CreatureLib::Library::Gender gender, + uint8_t level, const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::IsGenderAndLevel, + {new CreatureLib::Library::EffectParameter((int64_t)gender), + new CreatureLib::Library::EffectParameter((int64_t)level)}, + into); } - static EvolutionData CreateItemUseEvolution(const Item* item, const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::EvolutionItemUse, {std::any(item)}, into); + static inline const EvolutionData* CreateItemUseEvolution(const Item* item, const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::EvolutionItemUse, + {new CreatureLib::Library::EffectParameter(item)}, into); } - static EvolutionData CreateItemUseWithGenderEvolution(const Item* item, CreatureLib::Library::Gender gender, - const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::EvolutionItemUseWithGender, {std::any(item), std::any(gender)}, into); + static inline const EvolutionData* CreateItemUseWithGenderEvolution(const Item* item, + CreatureLib::Library::Gender gender, + const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::EvolutionItemUseWithGender, + {new CreatureLib::Library::EffectParameter(item), + new CreatureLib::Library::EffectParameter((int64_t)gender)}, + into); } - static EvolutionData CreateTradeEvolution(const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::Trade, {}, into); + static inline const EvolutionData* CreateTradeEvolution(const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::Trade, {}, into); } - static EvolutionData CreateTradeWithItemEvolution(const Item* item, const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::TradeWithHeldItem, {std::any(item)}, into); + static inline const EvolutionData* CreateTradeWithItemEvolution(const Item* item, const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::TradeWithHeldItem, + {new CreatureLib::Library::EffectParameter(item)}, into); } - static EvolutionData CreateTradeWithSpeciesEvolution(const PokemonSpecies* traded, const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::TradeWithSpecificPokemon, {std::any(traded)}, into); + static inline const EvolutionData* CreateTradeWithSpeciesEvolution(const PokemonSpecies* traded, + const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::TradeWithSpecificPokemon, + {new CreatureLib::Library::EffectParameter(traded)}, into); } - static EvolutionData CreateCustomEvolution(std::vector data, const PokemonSpecies* into) { - return EvolutionData(EvolutionMethod::Custom, std::move(data), into); + static inline const EvolutionData* + CreateCustomEvolution(const Arbutils::Collections::List& data, + const PokemonSpecies* into) { + return new EvolutionData(EvolutionMethod::Custom, data, into); } [[nodiscard]] inline const PokemonSpecies* GetNewSpecies() const { return _evolvesInto; } [[nodiscard]] inline EvolutionMethod GetMethod() const { return _method; } - [[nodiscard]] inline const std::any& GetData(size_t index) const { return _evolutionData.at(index); } + [[nodiscard]] inline const CreatureLib::Library::EffectParameter* GetData(size_t index) const { + return _evolutionData.At(index); + } }; } diff --git a/src/Library/Species/PokemonSpecies.hpp b/src/Library/Species/PokemonSpecies.hpp index f8ff163..61f13ee 100644 --- a/src/Library/Species/PokemonSpecies.hpp +++ b/src/Library/Species/PokemonSpecies.hpp @@ -8,7 +8,7 @@ namespace PkmnLib::Library { class PokemonSpecies : public CreatureLib::Library::CreatureSpecies { private: uint8_t _baseHappiness; - std::vector _evolutions; + std::vector _evolutions; public: PokemonSpecies(uint16_t id, const Arbutils::CaseInsensitiveConstString& name, @@ -18,7 +18,11 @@ namespace PkmnLib::Library { : CreatureSpecies(id, name, defaultVariant, genderRatio, growthRate, captureRate), _baseHappiness(baseHappiness) {} - ~PokemonSpecies() override = default; + ~PokemonSpecies() override { + for (auto evo : _evolutions) { + delete evo; + } + } inline uint8_t GetBaseHappiness() const { return _baseHappiness; } @@ -36,8 +40,8 @@ namespace PkmnLib::Library { return reinterpret_cast(CreatureSpecies::GetVariant(key)); } - inline void AddEvolution(const EvolutionData& data) { _evolutions.push_back(data); } - const std::vector& GetEvolutions() const { return _evolutions; } + inline void AddEvolution(const EvolutionData* data) { _evolutions.push_back(data); } + const std::vector& GetEvolutions() const { return _evolutions; } }; } diff --git a/src/ScriptResolving/AngelScript/AngelScripResolver.cpp b/src/ScriptResolving/AngelScript/AngelScripResolver.cpp index 47f1a42..bf7009d 100644 --- a/src/ScriptResolving/AngelScript/AngelScripResolver.cpp +++ b/src/ScriptResolving/AngelScript/AngelScripResolver.cpp @@ -2,7 +2,6 @@ #include #include #include -#include "../../../extern/angelscript_addons/scriptarray/scriptarray.h" #include "../../../extern/angelscript_addons/scripthandle/scripthandle.h" #include "../../../extern/angelscript_addons/scripthelper/scripthelper.h" #include "../../../extern/angelscript_addons/scriptstdstring/scriptstdstring.h" @@ -226,14 +225,12 @@ uint8_t* AngelScripResolver::WriteByteCodeToMemory(size_t& size, bool stripDebug } void AngelScripResolver::LoadByteCodeFromMemory( uint8_t* byte, size_t size, const Dictionary>& types) { - std::cout << "Loading from RAM" << std::endl; auto stream = new MemoryByteCodeStream(byte, size); InitializeByteCode(stream, types); delete stream; } void AngelScripResolver::InitializeByteCode(asIBinaryStream* stream, const Dictionary>& types) { - std::cout << "Loading byte code" << std::endl; int result = _mainModule->LoadByteCode(stream); Assert(result == asSUCCESS); diff --git a/src/ScriptResolving/AngelScript/TypeRegistry/ConstString.cpp b/src/ScriptResolving/AngelScript/TypeRegistry/ConstString.cpp index adaf9e5..7acefe4 100644 --- a/src/ScriptResolving/AngelScript/TypeRegistry/ConstString.cpp +++ b/src/ScriptResolving/AngelScript/TypeRegistry/ConstString.cpp @@ -1,9 +1,11 @@ #include "ConstString.hpp" using ConstString = Arbutils::CaseInsensitiveConstString; -static void ConstructConstString(ConstString* self) { self = new ConstString(); } -static void CopyConstructConstString(const ConstString& other, ConstString* self) { self = new ConstString(other); } -static void DestructConstString(ConstString* self) { self->~ConstString(); } +static void ConstructConstString(void* self) { new (self)ConstString(); } +static void CopyConstructConstString(const ConstString& other, void* self) { new (self)ConstString(other); } +static void DestructConstString(void* self) { + ((ConstString*)self)->~ConstString(); +} static bool ConstStringEquality(const ConstString& a, const ConstString& b) { return a == b; } static bool ConstStringStdStringEquality(const ConstString& a, const std::string& b) { return a == b; } static ConstString ImplStdStringConstStringConv(const std::string& s) { return ConstString(s); } @@ -12,7 +14,7 @@ static uint32_t ImplConstStringHashConv(const ConstString& s) { return s.GetHash void ConstStringRegister::Register(asIScriptEngine* engine) { auto r = engine->RegisterObjectType("constString", sizeof(Arbutils::CaseInsensitiveConstString), - asOBJ_VALUE | asOBJ_POD | asGetTypeTraits()); + asOBJ_VALUE | asOBJ_APP_CLASS_CDAK); Assert(r >= 0); r = engine->RegisterObjectBehaviour("constString", asBEHAVE_CONSTRUCT, "void f()", asFUNCTION(ConstructConstString), @@ -27,7 +29,7 @@ void ConstStringRegister::Register(asIScriptEngine* engine) { asCALL_CDECL_OBJLAST); Assert(r >= 0); - r = engine->RegisterObjectMethod("string", "constString &opAssign(const constString &in)", + r = engine->RegisterObjectMethod("constString", "constString &opAssign(const constString &in)", asMETHODPR(ConstString, operator=,(const ConstString&), ConstString&), asCALL_THISCALL); Assert(r >= 0); diff --git a/src/ScriptResolving/AngelScript/TypeRegistry/Library/RegisterEffectParameter.cpp b/src/ScriptResolving/AngelScript/TypeRegistry/Library/RegisterEffectParameter.cpp index 627c3e6..46d7a04 100644 --- a/src/ScriptResolving/AngelScript/TypeRegistry/Library/RegisterEffectParameter.cpp +++ b/src/ScriptResolving/AngelScript/TypeRegistry/Library/RegisterEffectParameter.cpp @@ -4,7 +4,9 @@ static CreatureLib::Library::EffectParameter* Ref_Factory() { return new CreatureLib::Library::EffectParameter(); } -static std::string AsString(const CreatureLib::Library::EffectParameter* p) { return p->AsString(); } +static const Arbutils::CaseInsensitiveConstString& AsString(const CreatureLib::Library::EffectParameter* p) { + return p->AsString(); +} void RegisterEffectParameter::Register(asIScriptEngine* engine) { [[maybe_unused]] int r = engine->RegisterEnum("EffectParameterType"); @@ -34,8 +36,9 @@ void RegisterEffectParameter::Register(asIScriptEngine* engine) { r = engine->RegisterObjectMethod("EffectParameter", "float AsFloat() const", asMETHOD(CreatureLib::Library::EffectParameter, AsFloat), asCALL_THISCALL); Assert(r >= 0); - r = engine->RegisterObjectMethod( - "EffectParameter", "string AsString() const", - asFUNCTIONPR(AsString, (const CreatureLib::Library::EffectParameter*), std::string), asCALL_CDECL_OBJFIRST); + r = engine->RegisterObjectMethod("EffectParameter", "const constString& AsString() const", + asFUNCTIONPR(AsString, (const CreatureLib::Library::EffectParameter*), + const Arbutils::CaseInsensitiveConstString&), + asCALL_CDECL_OBJFIRST); Assert(r >= 0); } diff --git a/tests/LibraryTests/SpeciesTests.cpp b/tests/LibraryTests/SpeciesTests.cpp index 29345a7..a505d9a 100644 --- a/tests/LibraryTests/SpeciesTests.cpp +++ b/tests/LibraryTests/SpeciesTests.cpp @@ -148,9 +148,10 @@ TEST_CASE("Able to set and get evolution", "library") { auto evolutions = species->GetEvolutions(); REQUIRE(evolutions.size() == 1); auto evo = evolutions[0]; - CHECK(evo.GetMethod() == PkmnLib::Library::EvolutionMethod::Level); - CHECK(evo.GetNewSpecies() == species2); - CHECK(std::any_cast(evo.GetData(0)) == 16); + CHECK(evo->GetMethod() == PkmnLib::Library::EvolutionMethod::Level); + CHECK(evo->GetNewSpecies() == species2); + INFO(CreatureLib::Library::EffectParameterTypeHelper::ToString(evo->GetData(0)->GetType())); + CHECK(evo->GetData(0)->AsInt() == 16); delete species; delete species2; diff --git a/tests/ScriptTests/BaseScriptClassTests.cpp b/tests/ScriptTests/BaseScriptClassTests.cpp index 6a0b634..6ee5e4b 100644 --- a/tests/ScriptTests/BaseScriptClassTests.cpp +++ b/tests/ScriptTests/BaseScriptClassTests.cpp @@ -12,7 +12,7 @@ static std::unordered_map _scripts = std::unordered_ma AS_CLASS(initializeScript, R"( bool boolValue = false; int64 intValue = 0; -string stringValue = ""; +constString stringValue; void OnInitialize(const array &in parameters) override { boolValue = parameters[0].AsBool(); intValue = parameters[1].AsInt(); @@ -20,7 +20,7 @@ void OnInitialize(const array &in parameters) override { } bool GetBoolValue() { return boolValue; } int64 GetIntValue() { return intValue; } -string GetStringValue() { return stringValue; } +constString GetStringValue() { return stringValue; } )"), AS_CLASS(stackScript, "int value = 0; void Stack() override { value++; } int GetValue() { return value; }"), @@ -108,11 +108,9 @@ TEST_CASE("Invoke OnInitialize script function") { auto script = GetScript(mainLib, "initializeScript"_cnc); REQUIRE(script != nullptr); - auto parameters = { - new CreatureLib::Library::EffectParameter(true), - new CreatureLib::Library::EffectParameter((int64_t)684), - new CreatureLib::Library::EffectParameter(std::string("foobar")) - }; + auto parameters = {new CreatureLib::Library::EffectParameter(true), + new CreatureLib::Library::EffectParameter((int64_t)684), + new CreatureLib::Library::EffectParameter(Arbutils::CaseInsensitiveConstString("foobar"))}; script->OnInitialize(parameters); @@ -133,12 +131,12 @@ TEST_CASE("Invoke OnInitialize script function") { ctx = ctxPool->RequestContext(); script->PrepareMethod("GetStringValue"_cnc, ctx); REQUIRE(ctx->Execute() == asEXECUTION_FINISHED); - std::string s; - s = *(std::string*)ctx->GetReturnAddress(); - REQUIRE(s == "foobar"); +// Arbutils::CaseInsensitiveConstString s; +// s = *(Arbutils::CaseInsensitiveConstString*)ctx->GetReturnAddress(); +// REQUIRE(s == "foobar"_cnc); ctxPool->ReturnContextToPool(ctx); - for (auto p : parameters){ + for (auto p : parameters) { delete p; } delete script;