From e45a36d78eda2b4236f13bdb03004493e309e1ca Mon Sep 17 00:00:00 2001 From: Deukhoofd Date: Wed, 5 Feb 2020 14:52:50 +0100 Subject: [PATCH] Move Script ownership to script holder, added OnRemove script hook. --- src/Battling/Models/Battle.hpp | 1 + src/Battling/Models/Creature.cpp | 2 ++ src/Battling/Models/Creature.hpp | 2 ++ src/Battling/Models/ExecutingAttack.hpp | 2 +- src/Battling/ScriptHandling/Script.hpp | 1 + src/Battling/ScriptHandling/ScriptSet.hpp | 12 ++++++++++++ .../ScriptTests/ScriptAggregatorTests.cpp | 9 --------- tests/BattleTests/ScriptTests/ScriptSetTests.cpp | 9 --------- tests/BattleTests/ScriptTests/ScriptSourceTest.cpp | 3 --- 9 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/Battling/Models/Battle.hpp b/src/Battling/Models/Battle.hpp index baaf899..9ba2e48 100644 --- a/src/Battling/Models/Battle.hpp +++ b/src/Battling/Models/Battle.hpp @@ -42,6 +42,7 @@ namespace CreatureLib::Battling { for (auto s : _sides) { delete s; } + delete _currentTurnQueue; } [[nodiscard]] const BattleLibrary* GetLibrary() const; diff --git a/src/Battling/Models/Creature.cpp b/src/Battling/Models/Creature.cpp index ae4fe3a..4ffc26d 100644 --- a/src/Battling/Models/Creature.cpp +++ b/src/Battling/Models/Creature.cpp @@ -114,6 +114,8 @@ void Battling::Creature::Damage(uint32_t damage, Battling::DamageSource source) void Battling::Creature::OverrideActiveTalent(const std::string& talent) { _hasOverridenTalent = true; + _activeTalent->OnRemove(); + delete _activeTalent; _overridenTalentName = talent; _activeTalent = this->_library->LoadScript(ScriptResolver::ScriptCategory::Talent, talent); } diff --git a/src/Battling/Models/Creature.hpp b/src/Battling/Models/Creature.hpp index 1c0fcc6..b2a0e6d 100644 --- a/src/Battling/Models/Creature.hpp +++ b/src/Battling/Models/Creature.hpp @@ -67,6 +67,8 @@ namespace CreatureLib::Battling { for (auto attack : _attacks) { delete attack; } + delete _activeTalent; + delete _status; }; virtual void Initialize() { diff --git a/src/Battling/Models/ExecutingAttack.hpp b/src/Battling/Models/ExecutingAttack.hpp index 6210b9c..ed13e35 100644 --- a/src/Battling/Models/ExecutingAttack.hpp +++ b/src/Battling/Models/ExecutingAttack.hpp @@ -67,7 +67,7 @@ namespace CreatureLib::Battling { } } - virtual ~ExecutingAttack() = default; + virtual ~ExecutingAttack() { delete _script; }; TargetData& GetAttackDataForTarget(Creature* creature) { return _targets[creature]; } diff --git a/src/Battling/ScriptHandling/Script.hpp b/src/Battling/ScriptHandling/Script.hpp index 5e83a65..bace9dc 100644 --- a/src/Battling/ScriptHandling/Script.hpp +++ b/src/Battling/ScriptHandling/Script.hpp @@ -22,6 +22,7 @@ namespace CreatureLib::Battling { virtual ~Script() = default; virtual void Stack(){}; + virtual void OnRemove(){}; const std::string& GetName() { return _name; } diff --git a/src/Battling/ScriptHandling/ScriptSet.hpp b/src/Battling/ScriptHandling/ScriptSet.hpp index 3a1cb1b..09a17d1 100644 --- a/src/Battling/ScriptHandling/ScriptSet.hpp +++ b/src/Battling/ScriptHandling/ScriptSet.hpp @@ -11,6 +11,12 @@ namespace CreatureLib::Battling { std::unordered_map _lookup; public: + ~ScriptSet() { + for (auto s : _scripts) { + delete s; + } + } + void Add(Script* script) { auto f = _lookup.find(script->GetName()); if (f != _lookup.end()) { @@ -24,12 +30,18 @@ namespace CreatureLib::Battling { void Remove(const std::string& key) { auto find = _lookup.find(key); if (find != _lookup.end()) { + auto script = _scripts[find->second]; + script->OnRemove(); + delete script; _scripts.erase(_scripts.begin() + find.operator*().second); _lookup.erase(key); } } void Clear() { + for (auto s : _scripts) { + delete s; + } _scripts.clear(); _lookup.clear(); } diff --git a/tests/BattleTests/ScriptTests/ScriptAggregatorTests.cpp b/tests/BattleTests/ScriptTests/ScriptAggregatorTests.cpp index cd72118..61562b2 100644 --- a/tests/BattleTests/ScriptTests/ScriptAggregatorTests.cpp +++ b/tests/BattleTests/ScriptTests/ScriptAggregatorTests.cpp @@ -63,9 +63,6 @@ TEST_CASE("Script Aggregator properly iterates Script Set.", "[Battling, Scripti dynamic_cast(next)->TestMethod(ran); } CHECK(ran == 3); - delete script; - delete script2; - delete script3; } TEST_CASE("Script Aggregator properly iterates data of Script Set and Script.", "[Battling, Scripting]") { @@ -85,8 +82,6 @@ TEST_CASE("Script Aggregator properly iterates data of Script Set and Script.", } CHECK(ran == 3); delete script; - delete script2; - delete script3; } TEST_CASE("Script Aggregator properly iterates data of Script and Script Set.", "[Battling, Scripting]") { @@ -106,8 +101,6 @@ TEST_CASE("Script Aggregator properly iterates data of Script and Script Set.", } CHECK(ran == 3); delete script; - delete script2; - delete script3; } TEST_CASE("Script Aggregator properly iterates data of Script, Script Set and Script.", "[Battling, Scripting]") { @@ -128,8 +121,6 @@ TEST_CASE("Script Aggregator properly iterates data of Script, Script Set and Sc } CHECK(ran == 4); delete script; - delete script2; - delete script3; delete script4; } diff --git a/tests/BattleTests/ScriptTests/ScriptSetTests.cpp b/tests/BattleTests/ScriptTests/ScriptSetTests.cpp index 59dfff9..e85944b 100644 --- a/tests/BattleTests/ScriptTests/ScriptSetTests.cpp +++ b/tests/BattleTests/ScriptTests/ScriptSetTests.cpp @@ -17,7 +17,6 @@ TEST_CASE("Add script to script set", "[Battling, Scripting]") { auto s = new Script("foobar"); set.Add(s); REQUIRE(set.Count() == 1); - delete s; } TEST_CASE("Add script to script set, then retrieve it", "[Battling, Scripting]") { @@ -27,7 +26,6 @@ TEST_CASE("Add script to script set, then retrieve it", "[Battling, Scripting]") REQUIRE(set.Count() == 1); auto get = set.GetIterator()->at(0); REQUIRE(get->GetName() == "foobar"); - delete s; } TEST_CASE("Add two scripts to script set", "[Battling, Scripting]") { @@ -37,8 +35,6 @@ TEST_CASE("Add two scripts to script set", "[Battling, Scripting]") { set.Add(s); set.Add(s2); REQUIRE(set.Count() == 2); - delete s; - delete s2; } TEST_CASE("Add two scripts to script set, then retrieve them", "[Battling, Scripting]") { @@ -52,8 +48,6 @@ TEST_CASE("Add two scripts to script set, then retrieve them", "[Battling, Scrip auto get2 = set.GetIterator()->at(1); REQUIRE(get1->GetName() == "foobar"); REQUIRE(get2->GetName() == "foobar2"); - delete s; - delete s2; } TEST_CASE("Add script to script set, then remove it", "[Battling, Scripting]") { @@ -65,7 +59,6 @@ TEST_CASE("Add script to script set, then remove it", "[Battling, Scripting]") { REQUIRE(set.Count() == 0); auto it = set.GetIterator(); REQUIRE(it->empty()); - delete s; } TEST_CASE("Add two scripts to script set, then remove them", "[Battling, Scripting]") { @@ -79,8 +72,6 @@ TEST_CASE("Add two scripts to script set, then remove them", "[Battling, Scripti REQUIRE(set.Count() == 1); auto it = set.GetIterator(); REQUIRE(it->at(0)->GetName() == "foobar2"); - delete s; - delete s2; } #endif diff --git a/tests/BattleTests/ScriptTests/ScriptSourceTest.cpp b/tests/BattleTests/ScriptTests/ScriptSourceTest.cpp index ec90606..f4038f0 100644 --- a/tests/BattleTests/ScriptTests/ScriptSourceTest.cpp +++ b/tests/BattleTests/ScriptTests/ScriptSourceTest.cpp @@ -74,7 +74,6 @@ TEST_CASE("Script source with single item script set.", "[Battling, Scripting]") auto first = scripts.GetNext(); CHECK(first != nullptr); CHECK(first->GetName() == "foobar"); - delete s; } TEST_CASE("Script source with multiple item script set.", "[Battling, Scripting]") { @@ -90,8 +89,6 @@ TEST_CASE("Script source with multiple item script set.", "[Battling, Scripting] auto second = scripts.GetNext(); CHECK(second != nullptr); CHECK(second->GetName() == "foobar2"); - delete s; - delete s2; } #endif