Cleanup of ScriptAggregator class.

src/Battling/Models/Battle.cpp

@@ -85,7 +85,9 @@ bool Battle::CreatureInField(const Creature* creature) const {
 
 void Battle::ForceRecall(uint8_t side, uint8_t index) { _sides[side]->SetCreature(nullptr, index); }
 
-void Battle::GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) { scripts.Append(&_volatile); }
+void Battle::GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) {
+    scripts.Append(ScriptWrapper::FromSet(&_volatile));
+}
 
 void Battle::SwitchCreature(uint8_t sideIndex, uint8_t index, Creature* c) {
     auto side = this->_sides[sideIndex];

src/Battling/Models/BattleSide.cpp

@@ -70,7 +70,7 @@ bool BattleSide::CreatureOnSide(const Creature* creature) const {
 Creature* BattleSide::GetCreature(uint8_t index) const { return _creatures[index]; }
 
 void BattleSide::GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) {
-    scripts.Append(&_volatile);
+    scripts.Append(ScriptWrapper::FromSet(&_volatile));
     _battle->GetActiveScripts(scripts);
 }
 uint8_t BattleSide::GetRandomCreatureIndex() {

src/Battling/Models/Creature.cpp

@@ -167,9 +167,9 @@ bool Battling::Creature::HasType(uint8_t type) const noexcept {
 }
 
 void Battling::Creature::GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) {
-    scripts.Append(&_activeTalent);
+    scripts.Append(ScriptWrapper::FromScript(&_activeTalent));
-    scripts.Append(&_status);
+    scripts.Append(ScriptWrapper::FromScript(&_status));
-    scripts.Append(&_volatile);
+    scripts.Append(ScriptWrapper::FromSet(&_volatile));
     if (_side != nullptr) {
         _side->GetActiveScripts(scripts);
     }

src/Battling/Models/ExecutingAttack.hpp

@@ -87,7 +87,7 @@ namespace CreatureLib::Battling {
 
     protected:
         void GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) override {
-            scripts.Append(&_script);
+            scripts.Append(ScriptWrapper::FromScript(&_script));
             _user->GetActiveScripts(scripts);
         }
     };

src/Battling/ScriptHandling/ScriptAggregator.hpp

@@ -11,54 +11,54 @@ using namespace Arbutils::Collections;
 
 namespace CreatureLib::Battling {
     class ScriptAggregator {
-        List<ScriptWrapper> _scripts;
+        const ScriptWrapper* _scripts;
+        size_t _size;
         size_t _index = 0;
-        bool _isSetSet = false;
-        const List<Script*>* _setScripts = nullptr;
         size_t _setIndex = 0;
 
     public:
         ScriptAggregator(){};
-        explicit ScriptAggregator(const List<ScriptWrapper>& scripts) : _scripts(scripts){};
+        explicit ScriptAggregator(const List<ScriptWrapper>& scripts)
+            : _scripts(scripts.RawData()), _size(scripts.Count()){};
 
-        void Reset() {
+        inline void Reset() {
             _index = 0;
-            _isSetSet = false;
             _setIndex = 0;
         }
-        bool HasNext() { return _index < _scripts.Count() || (_isSetSet && _setIndex < _setScripts->Count()); }
+        inline bool HasNext() { return _index < _size; }
 
-        Script* GetNext() {
+        Script* GetNextNotNull() {
-            // We can probably do this in a cleaner version once C++ 20 drops with Coroutine support.
+            while (HasNext()) {
-            if (_isSetSet) {
+                auto s = GetNext();
-                if (_setIndex >= _setScripts->Count()) {
+                if (s != nullptr) {
-                    _isSetSet = false;
-                    return GetNext();
-                }
-                auto s = _setScripts->At(_setIndex);
-                _setIndex++;
-                if (_setIndex >= _setScripts->Count()) {
-                    _isSetSet = false;
-                }
                     return s;
                 }
-            if (_index >= _scripts.Count())
+            }
             return nullptr;
-            auto& next = _scripts[_index];
+        }
+
+        Script* GetNext() {
+            auto& current = _scripts[_index];
+            if (!current.IsSet()) {
                 _index++;
-            if (!next.IsSet()) {
+                auto s = current.GetScript();
-                auto scriptPtr = next.GetScript();
+                if (s == nullptr)
-                if (scriptPtr == nullptr)
+                    return nullptr;
-                    return GetNext();
+                return *s;
-                return *scriptPtr;
             } else {
-                auto set = next.GetScriptSet();
+                auto& set = *current.GetScriptSet()->GetIterator();
-                if (set->Count() == 0)
+                auto count = set.Count();
-                    return GetNext();
+                if (_setIndex >= count) {
-                _setScripts = set->GetIterator();
+                    _index++;
-                _isSetSet = true;
                     _setIndex = 0;
-                return GetNext();
+                    return nullptr;
+                }
+                auto v = set[_setIndex++];
+                if (_setIndex >= count) {
+                    _index++;
+                    _setIndex = 0;
+                }
+                return v;
             }
         }
     };

src/Battling/ScriptHandling/ScriptSet.hpp

@@ -66,7 +66,7 @@ namespace CreatureLib::Battling {
 
         bool Has(uint32_t keyHash) const { return _lookup.Has(keyHash); }
 
-        size_t Count() const { return _scripts.Count(); }
+        inline size_t Count() const { return _scripts.Count(); }
 
         const Arbutils::Collections::List<Script*>* GetIterator() const { return &_scripts; }
     };

src/Battling/ScriptHandling/ScriptSource.hpp

@@ -8,7 +8,8 @@
 namespace CreatureLib::Battling {
     class ScriptSource {
         bool _areScriptsInitialized = false;
-        ScriptAggregator _scripts;
+        List<ScriptWrapper> _scripts;
+        ScriptAggregator _scriptsIterator;
 
     protected:
         virtual void GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) = 0;
@@ -17,12 +18,11 @@ namespace CreatureLib::Battling {
     public:
         const ScriptAggregator& GetScriptIterator() {
             if (!_areScriptsInitialized) {
-                List<ScriptWrapper> scripts;
+                GetActiveScripts(_scripts);
-                GetActiveScripts(scripts);
+                _scriptsIterator = ScriptAggregator(_scripts);
-                _scripts = ScriptAggregator(scripts);
                 _areScriptsInitialized = true;
             }
-            return _scripts;
+            return _scriptsIterator;
         }
     };
 }

src/Battling/ScriptHandling/ScriptWrapper.hpp

@@ -7,20 +7,24 @@
 
 namespace CreatureLib::Battling {
     class ScriptWrapper {
-        union {
-            Script** _script;
-            ScriptSet* _scriptSet;
-        };
         bool _isSet;
 
+        union {
+            Script* const* _script;
+            const ScriptSet* _scriptSet;
+        };
+
+        ScriptWrapper(Script** s, bool isSet) : _isSet(isSet), _script(s){};
+        ScriptWrapper(ScriptSet* s, bool isSet) : _isSet(isSet), _scriptSet(s){};
+
     public:
-        ScriptWrapper(Script** s) : _script(s), _isSet(false) {}
+        static inline ScriptWrapper FromScript(Script** s) { return ScriptWrapper(s, false); }
-        ScriptWrapper(ScriptSet* s) : _scriptSet(s), _isSet(true) {}
+        static inline ScriptWrapper FromSet(ScriptSet* s) { return ScriptWrapper(s, true); }
 
         bool IsSet() const { return _isSet; }
 
-        Script** GetScript() const { return _script; }
+        inline Script* const* GetScript() const { return _script; }
-        ScriptSet* GetScriptSet() const { return _scriptSet; }
+        inline const ScriptSet* GetScriptSet() const { return _scriptSet; }
     };
 }
 

src/Battling/TurnChoices/AttackTurnChoice.hpp

@@ -56,7 +56,7 @@ namespace CreatureLib::Battling {
 
     protected:
         void GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) override {
-            scripts.Append(&_attackScript);
+            scripts.Append(ScriptWrapper::FromScript(&_attackScript));
             GetUser()->GetActiveScripts(scripts);
         }
     };

tests/BattleTests/ScriptTests/ScriptAggregatorTests.cpp

@@ -21,7 +21,7 @@ public:
 TEST_CASE("Script Aggregator properly iterates containing script.", "[Battling, Scripting]") {
     Script* script = new TestScript("test");
     auto ran = 0;
-    auto vec = Arbutils::Collections::List<ScriptWrapper>{&script};
+    auto vec = Arbutils::Collections::List<ScriptWrapper>{ScriptWrapper::FromScript(&script)};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
         auto next = aggr.GetNext();
@@ -37,7 +37,8 @@ TEST_CASE("Script Aggregator properly iterates multiple scripts.", "[Battling, S
     Script* script2 = new TestScript("test2");
     Script* script3 = new TestScript("test3");
     auto ran = 0;
-    auto vec = Arbutils::Collections::List<ScriptWrapper>{&script, &script2, &script3};
+    auto vec = Arbutils::Collections::List<ScriptWrapper>{
+        ScriptWrapper::FromScript(&script), ScriptWrapper::FromScript(&script2), ScriptWrapper::FromScript(&script3)};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
         auto next = aggr.GetNext();
@@ -59,10 +60,10 @@ TEST_CASE("Script Aggregator properly iterates Script Set.", "[Battling, Scripti
     set.Add(script);
     set.Add(script2);
     set.Add(script3);
-    auto vec = Arbutils::Collections::List<ScriptWrapper>{&set};
+    auto vec = Arbutils::Collections::List<ScriptWrapper>{ScriptWrapper::FromSet(&set)};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
-        auto next = aggr.GetNext();
+        auto next = aggr.GetNextNotNull();
         REQUIRE(next != nullptr);
         dynamic_cast<TestScript*>(next)->TestMethod(ran);
     }
@@ -77,10 +78,11 @@ TEST_CASE("Script Aggregator properly iterates data of Script Set and Script.",
     auto set = ScriptSet();
     set.Add(script2);
     set.Add(script3);
-    auto vec = Arbutils::Collections::List<ScriptWrapper>{&set, &script};
+    auto vec =
+        Arbutils::Collections::List<ScriptWrapper>{ScriptWrapper::FromSet(&set), ScriptWrapper::FromScript(&script)};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
-        auto next = aggr.GetNext();
+        auto next = aggr.GetNextNotNull();
         REQUIRE(next != nullptr);
         dynamic_cast<TestScript*>(next)->TestMethod(ran);
     }
@@ -96,10 +98,11 @@ TEST_CASE("Script Aggregator properly iterates data of Script and Script Set.",
     auto set = ScriptSet();
     set.Add(script2);
     set.Add(script3);
-    auto vec = Arbutils::Collections::List<ScriptWrapper>{&script, &set};
+    auto vec =
+        Arbutils::Collections::List<ScriptWrapper>{ScriptWrapper::FromScript(&script), ScriptWrapper::FromSet(&set)};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
-        auto next = aggr.GetNext();
+        auto next = aggr.GetNextNotNull();
         REQUIRE(next != nullptr);
         dynamic_cast<TestScript*>(next)->TestMethod(ran);
     }
@@ -116,10 +119,11 @@ TEST_CASE("Script Aggregator properly iterates data of Script, Script Set and Sc
     auto set = ScriptSet();
     set.Add(script2);
     set.Add(script3);
-    auto vec = Arbutils::Collections::List<ScriptWrapper>{&script, &set, &script4};
+    auto vec = Arbutils::Collections::List<ScriptWrapper>{
+        ScriptWrapper::FromScript(&script), ScriptWrapper::FromSet(&set), ScriptWrapper::FromScript(&script4)};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
-        auto next = aggr.GetNext();
+        auto next = aggr.GetNextNotNull();
         REQUIRE(next != nullptr);
         dynamic_cast<TestScript*>(next)->TestMethod(ran);
     }
@@ -141,7 +145,7 @@ TEST_CASE("Script Aggregator properly iterates when empty.", "[Battling, Scripti
 TEST_CASE("Script Aggregator properly iterates empty Script Set.", "[Battling, Scripting]") {
     auto ran = 0;
     auto set = ScriptSet();
-    auto vec = Arbutils::Collections::List<ScriptWrapper>{&set};
+    auto vec = Arbutils::Collections::List<ScriptWrapper>{ScriptWrapper::FromSet(&set)};
     auto aggr = ScriptAggregator(vec);
     while (aggr.HasNext()) {
         auto next = aggr.GetNext();

tests/BattleTests/ScriptTests/ScriptSourceTest.cpp

@@ -24,7 +24,9 @@ public:
     Script* ScriptPtr = nullptr;
 
 protected:
-    void GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) override { scripts.Append(&ScriptPtr); }
+    void GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) override {
+        scripts.Append(ScriptWrapper::FromScript(&ScriptPtr));
+    }
 };
 
 class ScriptSourceWithScriptSet : public ScriptSource {
@@ -32,7 +34,9 @@ public:
     ScriptSet Set;
 
 protected:
-    void GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) override { scripts.Append(&Set); }
+    void GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) override {
+        scripts.Append(ScriptWrapper::FromSet(&Set));
+    }
 };
 
 TEST_CASE("Script source with unset script ptr.", "[Battling, Scripting]") {
@@ -75,7 +79,7 @@ TEST_CASE("Script source with single item script set.", "[Battling, Scripting]")
     auto s = new TestScript("foobar");
     source.Set.Add(s);
     auto scripts = source.GetScriptIterator();
-    auto first = scripts.GetNext();
+    auto first = scripts.GetNextNotNull();
     CHECK(first != nullptr);
     CHECK(first->GetName() == "foobar");
 }
@@ -87,11 +91,11 @@ TEST_CASE("Script source with multiple item script set.", "[Battling, Scripting]
     source.Set.Add(s);
     source.Set.Add(s2);
     auto scripts = source.GetScriptIterator();
-    auto first = scripts.GetNext();
+    auto first = scripts.GetNextNotNull();
-    CHECK(first != nullptr);
+    REQUIRE(first != nullptr);
     CHECK(first->GetName() == "foobar");
-    auto second = scripts.GetNext();
+    auto second = scripts.GetNextNotNull();
-    CHECK(second != nullptr);
+    REQUIRE(second != nullptr);
     CHECK(second->GetName() == "foobar2");
 }