From 9f091308b0c0b4814171701508f8ef347ba8db7e Mon Sep 17 00:00:00 2001
From: Deukhoofd <Deukhoofd@gmail.com>
Date: Sat, 17 Apr 2021 15:08:26 +0200
Subject: [PATCH] Reset on initialization in ScriptAggregator, fixes segfault
 when calling Reset on an empty Aggregator.

Signed-off-by: Deukhoofd <Deukhoofd@gmail.com>
---
 .../ScriptHandling/ScriptAggregator.hpp       | 22 +++--
 .../ScriptTests/ScriptAggregatorTests.cpp     | 83 ++++++++++---------
 2 files changed, 57 insertions(+), 48 deletions(-)

diff --git a/src/Battling/ScriptHandling/ScriptAggregator.hpp b/src/Battling/ScriptHandling/ScriptAggregator.hpp
index d025358..a7a7f2a 100644
--- a/src/Battling/ScriptHandling/ScriptAggregator.hpp
+++ b/src/Battling/ScriptHandling/ScriptAggregator.hpp
@@ -32,17 +32,23 @@ namespace CreatureLib::Battling {
         }
 
     public:
-        ScriptAggregator(){};
-        explicit ScriptAggregator(const ArbUt::List<ScriptWrapper>& scripts)
-            : _scripts(scripts.RawData()), _size(scripts.Count()){};
+        ScriptAggregator() {};
+
+        explicit ScriptAggregator(const ArbUt::List<ScriptWrapper> &scripts)
+                : _scripts(scripts.RawData()), _size(scripts.Count()) {
+            Reset();
+        };
 
         inline void Reset() {
             _index = 0;
-            if (_scripts[_index].IsSet()) {
-                _setIndex = -1;
+            if (_size > 0) {
+                if (_scripts[_index].IsSet()) {
+                    _setIndex = -1;
+                }
+                IncrementToNextNotNull(false);
             }
-            IncrementToNextNotNull(false);
         }
+
         inline bool HasNext() { return _index < _size; }
 
         std::optional<ArbUt::BorrowedPtr<BattleScript>> GetNextNotNull() {
@@ -54,13 +60,13 @@ namespace CreatureLib::Battling {
         }
 
         ArbUt::BorrowedPtr<BattleScript> GetNext() {
-            auto& current = _scripts[_index];
+            auto &current = _scripts[_index];
             if (!current.IsSet()) {
                 auto s = current.GetScript();
                 IncrementToNextNotNull();
                 return (*s);
             } else {
-                auto& set = current.GetScriptSet()->GetIterator();
+                auto &set = current.GetScriptSet()->GetIterator();
                 auto v = set[_setIndex];
                 IncrementToNextNotNull();
                 return v;
diff --git a/tests/BattleTests/ScriptTests/ScriptAggregatorTests.cpp b/tests/BattleTests/ScriptTests/ScriptAggregatorTests.cpp
index bee435a..5903733 100644
--- a/tests/BattleTests/ScriptTests/ScriptAggregatorTests.cpp
+++ b/tests/BattleTests/ScriptTests/ScriptAggregatorTests.cpp
@@ -12,47 +12,49 @@ private:
     ArbUt::StringView _name;
 
 public:
-    explicit TestScript(const ArbUt::StringView& name) : _name(name){};
-    const ArbUt::StringView& GetName() const noexcept override { return _name; }
+    explicit TestScript(const ArbUt::StringView &name) : _name(name) {};
 
-    void TestMethod(int& runCount) { runCount++; }
-    BattleScript* Clone() override { return new TestScript(_name); }
+    const ArbUt::StringView &GetName() const noexcept override { return _name; }
+
+    void TestMethod(int &runCount) { runCount++; }
+
+    BattleScript *Clone() override { return new TestScript(_name); }
 };
 
-TEST_CASE("Script Aggregator properly iterates containing script.") {
+TEST_CASE ("Script Aggregator properly iterates containing script.") {
     auto script = std::make_unique<TestScript>("test");
     auto ran = 0;
     auto vec = ArbUt::List<ScriptWrapper>{
-        ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script))};
+            ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript> *>(&script))};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
         auto next = aggr.GetNext();
         next.As<TestScript>()->TestMethod(ran);
     }
-    CHECK(ran == 1);
+            CHECK(ran == 1);
 }
 
-TEST_CASE("Script Aggregator properly iterates multiple scripts.") {
+TEST_CASE ("Script Aggregator properly iterates multiple scripts.") {
     auto script = std::make_unique<TestScript>("test");
     auto script2 = std::make_unique<TestScript>("test2");
     auto script3 = std::make_unique<TestScript>("test3");
     auto ran = 0;
     auto vec = ArbUt::List<ScriptWrapper>{
-        ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script)),
-        ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script2)),
-        ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script3))};
+            ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript> *>(&script)),
+            ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript> *>(&script2)),
+            ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript> *>(&script3))};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
         auto next = aggr.GetNext();
         next.As<TestScript>()->TestMethod(ran);
     }
-    CHECK(ran == 3);
+            CHECK(ran == 3);
 }
 
-TEST_CASE("Script Aggregator properly iterates Script Set.") {
-    BattleScript* script = new TestScript("test");
-    BattleScript* script2 = new TestScript("test2");
-    BattleScript* script3 = new TestScript("test3");
+TEST_CASE ("Script Aggregator properly iterates Script Set.") {
+    BattleScript *script = new TestScript("test");
+    BattleScript *script2 = new TestScript("test2");
+    BattleScript *script3 = new TestScript("test3");
     auto ran = 0;
     auto set = ScriptSet();
     set.Add(script);
@@ -64,79 +66,80 @@ TEST_CASE("Script Aggregator properly iterates Script Set.") {
         auto next = aggr.GetNextNotNull();
         next.value().As<TestScript>()->TestMethod(ran);
     }
-    CHECK(ran == 3);
+            CHECK(ran == 3);
 }
 
-TEST_CASE("Script Aggregator properly iterates data of Script Set and Script.") {
+TEST_CASE ("Script Aggregator properly iterates data of Script Set and Script.") {
     auto script = std::make_unique<TestScript>("test");
-    BattleScript* script2 = new TestScript("test2");
-    BattleScript* script3 = new TestScript("test3");
+    BattleScript *script2 = new TestScript("test2");
+    BattleScript *script3 = new TestScript("test3");
     auto ran = 0;
     auto set = ScriptSet();
     set.Add(script2);
     set.Add(script3);
     auto vec = ArbUt::List<ScriptWrapper>{
-        ScriptWrapper::FromSet(&set),
-        ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script))};
+            ScriptWrapper::FromSet(&set),
+            ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript> *>(&script))};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
         auto next = aggr.GetNextNotNull();
         next.value().As<TestScript>()->TestMethod(ran);
     }
-    CHECK(ran == 3);
+            CHECK(ran == 3);
 }
 
-TEST_CASE("Script Aggregator properly iterates data of Script and Script Set.") {
+TEST_CASE ("Script Aggregator properly iterates data of Script and Script Set.") {
     auto script = std::make_unique<TestScript>("test");
-    BattleScript* script2 = new TestScript("test2");
-    BattleScript* script3 = new TestScript("test3");
+    BattleScript *script2 = new TestScript("test2");
+    BattleScript *script3 = new TestScript("test3");
     auto ran = 0;
     auto set = ScriptSet();
     set.Add(script2);
     set.Add(script3);
     auto vec =
-        ArbUt::List<ScriptWrapper>{ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script)),
-                                   ScriptWrapper::FromSet(&set)};
+            ArbUt::List<ScriptWrapper>{
+                    ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript> *>(&script)),
+                    ScriptWrapper::FromSet(&set)};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
         auto next = aggr.GetNextNotNull();
         next.value().As<TestScript>()->TestMethod(ran);
     }
-    CHECK(ran == 3);
+            CHECK(ran == 3);
 }
 
-TEST_CASE("Script Aggregator properly iterates data of Script, Script Set and Script.") {
+TEST_CASE ("Script Aggregator properly iterates data of Script, Script Set and Script.") {
     auto script = std::make_unique<TestScript>("test");
-    BattleScript* script2 = new TestScript("test2");
-    BattleScript* script3 = new TestScript("test3");
+    BattleScript *script2 = new TestScript("test2");
+    BattleScript *script3 = new TestScript("test3");
     auto script4 = std::make_unique<TestScript>("test4");
     auto ran = 0;
     auto set = ScriptSet();
     set.Add(script2);
     set.Add(script3);
     auto vec = ArbUt::List<ScriptWrapper>{
-        ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script)),
-        ScriptWrapper::FromSet(&set),
-        ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript>*>(&script4))};
+            ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript> *>(&script)),
+            ScriptWrapper::FromSet(&set),
+            ScriptWrapper::FromScript(reinterpret_cast<std::unique_ptr<BattleScript> *>(&script4))};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
         auto next = aggr.GetNextNotNull();
         next.value().As<TestScript>()->TestMethod(ran);
     }
-    CHECK(ran == 4);
+            CHECK(ran == 4);
 }
 
-TEST_CASE("Script Aggregator properly iterates when empty.") {
+TEST_CASE ("Script Aggregator properly iterates when empty.") {
     auto ran = 0;
     auto vec = ArbUt::List<ScriptWrapper>{};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
         THROW("Aggregator returned a script, but should have been empty.");
     }
-    CHECK(ran == 0);
+            CHECK(ran == 0);
 }
 
-TEST_CASE("Script Aggregator properly iterates empty Script Set.") {
+TEST_CASE ("Script Aggregator properly iterates empty Script Set.") {
     auto ran = 0;
     auto set = ScriptSet();
     auto vec = ArbUt::List<ScriptWrapper>{ScriptWrapper::FromSet(&set)};
@@ -145,7 +148,7 @@ TEST_CASE("Script Aggregator properly iterates empty Script Set.") {
     while (aggr.HasNext()) {
         ran++;
     }
-    CHECK(ran == 0);
+            CHECK(ran == 0);
 }
 
 #endif
\ No newline at end of file