From 1ef50fd3a68d718e211ee995792752545fd318d5 Mon Sep 17 00:00:00 2001
From: Deukhoofd <deukhoofd@gmail.com>
Date: Tue, 2 Jun 2020 15:03:31 +0200
Subject: [PATCH] Use unique pointers in scriptset.

---
 CInterface/Battling/Battle.cpp                |  2 +-
 src/Battling/Models/Battle.cpp                |  2 +-
 src/Battling/Models/Battle.hpp                |  4 ++--
 src/Battling/Models/Creature.cpp              |  2 +-
 .../ScriptHandling/ScriptAggregator.hpp       |  4 ++--
 src/Battling/ScriptHandling/ScriptSet.hpp     | 19 ++++++++-----------
 .../ScriptTests/ScriptSetTests.cpp            | 14 +++++++-------
 7 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/CInterface/Battling/Battle.cpp b/CInterface/Battling/Battle.cpp
index 0a73868..85f1424 100644
--- a/CInterface/Battling/Battle.cpp
+++ b/CInterface/Battling/Battle.cpp
@@ -56,7 +56,7 @@ export BattleParty* const* CreatureLib_Battle_GetParties(const Battle* p) {
 }
 
 export Script* CreatureLib_Battle_GetVolatileScript(Battle* p, const char* key) {
-    return p->GetVolatileScript(ConstString::GetHash(key));
+    return p->GetVolatileScript(ConstString::GetHash(key)).GetRaw();
 }
 export uint8_t CreatureLib_Battle_AddVolatileScriptByName(Battle* p, const char* key) {
     Try(p->AddVolatileScript(ConstString(key));)
diff --git a/src/Battling/Models/Battle.cpp b/src/Battling/Models/Battle.cpp
index bed0e85..d7fa0d3 100644
--- a/src/Battling/Models/Battle.cpp
+++ b/src/Battling/Models/Battle.cpp
@@ -137,7 +137,7 @@ void Battle::AddVolatileScript(const ConstString& key) {
         ss << "Invalid volatile script requested for battle: '" << key.c_str() << "'.";
         throw CreatureException(ss.str());
     }
-    return _volatile.Add(script);
+    return _volatile.Add(script.GetRaw());
 }
 void Battle::AddVolatileScript(Script* script) { return _volatile.Add(script); }
 void Battle::RemoveVolatileScript(Script* script) { _volatile.Remove(script->GetName()); }
diff --git a/src/Battling/Models/Battle.hpp b/src/Battling/Models/Battle.hpp
index e6b77e1..d701fa5 100644
--- a/src/Battling/Models/Battle.hpp
+++ b/src/Battling/Models/Battle.hpp
@@ -82,8 +82,8 @@ namespace CreatureLib::Battling {
 
         const ArbUt::UniquePtrList<BattleParty>& GetParties() const noexcept { return _parties; }
         const ArbUt::UniquePtrList<BattleSide>& GetSides() const noexcept { return _sides; }
-        Script* GetVolatileScript(const ConstString& key) const { return _volatile.Get(key); }
-        Script* GetVolatileScript(uint32_t keyHash) const noexcept { return _volatile.Get(keyHash); }
+        ArbUt::BorrowedPtr<Script> GetVolatileScript(const ConstString& key) const { return _volatile.Get(key); }
+        ArbUt::BorrowedPtr<Script> GetVolatileScript(uint32_t keyHash) const noexcept { return _volatile.Get(keyHash); }
         void AddVolatileScript(const ConstString& key);
         void AddVolatileScript(Script* script);
         void RemoveVolatileScript(const ConstString& name) { _volatile.Remove(name); }
diff --git a/src/Battling/Models/Creature.cpp b/src/Battling/Models/Creature.cpp
index 22a94e5..816b0fd 100644
--- a/src/Battling/Models/Creature.cpp
+++ b/src/Battling/Models/Creature.cpp
@@ -225,7 +225,7 @@ void Battling::Creature::AddVolatileScript(const ConstString& name) {
         ss << "Invalid volatile script requested for creature: '" << name.c_str() << "'.";
         throw CreatureException(ss.str());
     }
-    _volatile.Add(script);
+    _volatile.Add(script.GetRaw());
 }
 
 void Battling::Creature::AddVolatileScript(Script* script) { _volatile.Add(script); }
diff --git a/src/Battling/ScriptHandling/ScriptAggregator.hpp b/src/Battling/ScriptHandling/ScriptAggregator.hpp
index 1c0696b..7910b41 100644
--- a/src/Battling/ScriptHandling/ScriptAggregator.hpp
+++ b/src/Battling/ScriptHandling/ScriptAggregator.hpp
@@ -44,7 +44,7 @@ namespace CreatureLib::Battling {
                     return nullptr;
                 return (*s).get();
             } else {
-                auto& set = *current.GetScriptSet()->GetIterator();
+                auto& set = current.GetScriptSet()->GetIterator();
                 auto count = set.Count();
                 if (_setIndex >= count) {
                     _index++;
@@ -56,7 +56,7 @@ namespace CreatureLib::Battling {
                     _index++;
                     _setIndex = 0;
                 }
-                return v;
+                return v.GetRaw();
             }
         }
     };
diff --git a/src/Battling/ScriptHandling/ScriptSet.hpp b/src/Battling/ScriptHandling/ScriptSet.hpp
index b5f76a5..adde538 100644
--- a/src/Battling/ScriptHandling/ScriptSet.hpp
+++ b/src/Battling/ScriptHandling/ScriptSet.hpp
@@ -2,21 +2,17 @@
 #define CREATURELIB_SCRIPTSET_HPP
 
 #include <Arbutils/Collections/Dictionary.hpp>
-#include <Arbutils/Collections/List.hpp>
+#include <Arbutils/Memory/UniquePtrList.hpp>
 #include <any>
 #include "Script.hpp"
 
 namespace CreatureLib::Battling {
     class ScriptSet {
-        ArbUt::List<Script*> _scripts;
+        ArbUt::UniquePtrList<Script> _scripts;
         ArbUt::Dictionary<uint32_t, size_t> _lookup;
 
     public:
-        ~ScriptSet() {
-            for (auto s : _scripts) {
-                delete s;
-            }
-        }
+        ~ScriptSet() = default;
 
         static constexpr size_t defaultCapacity = 8;
         ScriptSet() : _scripts(defaultCapacity), _lookup(defaultCapacity){};
@@ -33,9 +29,11 @@ namespace CreatureLib::Battling {
             _lookup.Insert(script->GetName(), _scripts.Count() - 1);
         }
 
-        Script* Get(const ArbUt::CaseInsensitiveConstString& key) const { return Get(key.GetHash()); }
+        ArbUt::BorrowedPtr<Script> Get(const ArbUt::CaseInsensitiveConstString& key) const {
+            return Get(key.GetHash());
+        }
 
-        Script* Get(uint32_t keyHash) const noexcept {
+        ArbUt::BorrowedPtr<Script> Get(uint32_t keyHash) const noexcept {
             size_t v;
             if (_lookup.TryGet(keyHash, v)) {
                 return _scripts[v];
@@ -50,7 +48,6 @@ namespace CreatureLib::Battling {
             if (_lookup.TryGet(keyHash, v)) {
                 auto script = _scripts[v];
                 script->OnRemove();
-                delete script;
                 _scripts.Remove(v);
                 _lookup.Remove(keyHash);
             }
@@ -71,7 +68,7 @@ namespace CreatureLib::Battling {
 
         inline size_t Count() const { return _scripts.Count(); }
 
-        const ArbUt::List<Script*>* GetIterator() const { return &_scripts; }
+        const ArbUt::UniquePtrList<Script>& GetIterator() const { return _scripts; }
     };
 }
 
diff --git a/tests/BattleTests/ScriptTests/ScriptSetTests.cpp b/tests/BattleTests/ScriptTests/ScriptSetTests.cpp
index 6d66c24..666f091 100644
--- a/tests/BattleTests/ScriptTests/ScriptSetTests.cpp
+++ b/tests/BattleTests/ScriptTests/ScriptSetTests.cpp
@@ -33,7 +33,7 @@ TEST_CASE("Add script to script set, then retrieve it", "[Battling, Scripting]")
     auto s = new TestScript("foobar");
     set.Add(s);
     REQUIRE(set.Count() == 1);
-    auto get = set.GetIterator()->At(0);
+    auto get = set.GetIterator().At(0);
     REQUIRE(get->GetName() == "foobar");
 }
 
@@ -53,8 +53,8 @@ TEST_CASE("Add two scripts to script set, then retrieve them", "[Battling, Scrip
     set.Add(s);
     set.Add(s2);
     REQUIRE(set.Count() == 2);
-    auto get1 = set.GetIterator()->At(0);
-    auto get2 = set.GetIterator()->At(1);
+    auto get1 = set.GetIterator().At(0);
+    auto get2 = set.GetIterator().At(1);
     REQUIRE(get1->GetName() == "foobar");
     REQUIRE(get2->GetName() == "foobar2");
 }
@@ -66,8 +66,8 @@ TEST_CASE("Add script to script set, then remove it", "[Battling, Scripting]") {
     REQUIRE(set.Count() == 1);
     set.Remove("foobar"_cnc.GetHash());
     REQUIRE(set.Count() == 0);
-    auto it = set.GetIterator();
-    REQUIRE(it->Count() == 0);
+    auto& it = set.GetIterator();
+    REQUIRE(it.Count() == 0);
 }
 
 TEST_CASE("Add two scripts to script set, then remove them", "[Battling, Scripting]") {
@@ -79,8 +79,8 @@ TEST_CASE("Add two scripts to script set, then remove them", "[Battling, Scripti
     REQUIRE(set.Count() == 2);
     set.Remove("foobar"_cnc.GetHash());
     REQUIRE(set.Count() == 1);
-    auto it = set.GetIterator();
-    REQUIRE(it->At(0)->GetName() == "foobar2");
+    auto& it = set.GetIterator();
+    REQUIRE(it.At(0)->GetName() == "foobar2");
 }
 
 #endif