From 3a11bba91344d80a6d5407f1613a8ccdd97b34d6 Mon Sep 17 00:00:00 2001
From: Deukhoofd <deukhoofd@gmail.com>
Date: Thu, 23 Apr 2020 19:57:45 +0200
Subject: [PATCH] Rework ScriptAggregator so it only needs to be created once,
 and can then be reset.

---
 src/Battling/ScriptHandling/ScriptAggregator.hpp | 15 ++++++++++-----
 src/Battling/ScriptHandling/ScriptMacros.hpp     |  1 +
 src/Battling/ScriptHandling/ScriptSource.hpp     | 15 +++++++--------
 src/Battling/ScriptHandling/ScriptWrapper.hpp    | 14 ++++++++------
 4 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/src/Battling/ScriptHandling/ScriptAggregator.hpp b/src/Battling/ScriptHandling/ScriptAggregator.hpp
index 57a683a..3f141ee 100644
--- a/src/Battling/ScriptHandling/ScriptAggregator.hpp
+++ b/src/Battling/ScriptHandling/ScriptAggregator.hpp
@@ -14,12 +14,18 @@ namespace CreatureLib::Battling {
         List<ScriptWrapper> _scripts;
         size_t _index = 0;
         bool _isSetSet = false;
-        const List<Script*>* _setScripts;
-        size_t _setIndex;
+        const List<Script*>* _setScripts = nullptr;
+        size_t _setIndex = 0;
 
     public:
-        ScriptAggregator(List<ScriptWrapper> scripts) : _scripts(std::move(scripts)){};
+        ScriptAggregator(){};
+        explicit ScriptAggregator(const List<ScriptWrapper>& scripts) : _scripts(scripts){};
 
+        void Reset() {
+            _index = 0;
+            _isSetSet = false;
+            _setIndex = 0;
+        }
         bool HasNext() { return _index < _scripts.Count() || (_isSetSet && _setIndex < _setScripts->Count()); }
 
         Script* GetNext() {
@@ -38,7 +44,7 @@ namespace CreatureLib::Battling {
             }
             if (_index >= _scripts.Count())
                 return nullptr;
-            auto next = _scripts[_index];
+            auto& next = _scripts[_index];
             _index++;
             if (!next.IsSet()) {
                 auto scriptPtr = next.GetScript();
@@ -54,7 +60,6 @@ namespace CreatureLib::Battling {
                 _setIndex = 0;
                 return GetNext();
             }
-            throw NotReachableException();
         }
     };
 }
diff --git a/src/Battling/ScriptHandling/ScriptMacros.hpp b/src/Battling/ScriptHandling/ScriptMacros.hpp
index 29b2f43..dbd14a4 100644
--- a/src/Battling/ScriptHandling/ScriptMacros.hpp
+++ b/src/Battling/ScriptHandling/ScriptMacros.hpp
@@ -1,6 +1,7 @@
 #define HOOK(hookName, source, ...)                                                                                    \
     {                                                                                                                  \
         auto aggregator = source->GetScriptIterator();                                                                 \
+        aggregator.Reset();                                                                                            \
         while (aggregator.HasNext()) {                                                                                 \
             auto next = aggregator.GetNext();                                                                          \
             if (next == nullptr)                                                                                       \
diff --git a/src/Battling/ScriptHandling/ScriptSource.hpp b/src/Battling/ScriptHandling/ScriptSource.hpp
index a1aa195..a861584 100644
--- a/src/Battling/ScriptHandling/ScriptSource.hpp
+++ b/src/Battling/ScriptHandling/ScriptSource.hpp
@@ -8,22 +8,21 @@
 namespace CreatureLib::Battling {
     class ScriptSource {
         bool _areScriptsInitialized = false;
-        Arbutils::Collections::List<ScriptWrapper> _scripts;
+        ScriptAggregator _scripts;
 
     protected:
         virtual void GetActiveScripts(Arbutils::Collections::List<ScriptWrapper>& scripts) = 0;
-        void ResetActiveScripts() {
-            _areScriptsInitialized = false;
-            _scripts.Clear();
-        }
+        void ResetActiveScripts() { _areScriptsInitialized = false; }
 
     public:
-        ScriptAggregator GetScriptIterator() {
+        const ScriptAggregator& GetScriptIterator() {
             if (!_areScriptsInitialized) {
-                GetActiveScripts(_scripts);
+                List<ScriptWrapper> scripts;
+                GetActiveScripts(scripts);
+                _scripts = ScriptAggregator(scripts);
                 _areScriptsInitialized = true;
             }
-            return ScriptAggregator(_scripts);
+            return _scripts;
         }
     };
 }
diff --git a/src/Battling/ScriptHandling/ScriptWrapper.hpp b/src/Battling/ScriptHandling/ScriptWrapper.hpp
index 3c6ce4a..9fe04b0 100644
--- a/src/Battling/ScriptHandling/ScriptWrapper.hpp
+++ b/src/Battling/ScriptHandling/ScriptWrapper.hpp
@@ -7,18 +7,20 @@
 
 namespace CreatureLib::Battling {
     class ScriptWrapper {
-        std::variant<Script**, ScriptSet*> _value;
+        union {
+            Script** _script;
+            ScriptSet* _scriptSet;
+        };
         bool _isSet;
 
     public:
-        ScriptWrapper(Script** s) : _value(s), _isSet(false) {}
-        ScriptWrapper(ScriptSet* s) : _value(s), _isSet(true) {}
+        ScriptWrapper(Script** s) : _script(s), _isSet(false) {}
+        ScriptWrapper(ScriptSet* s) : _scriptSet(s), _isSet(true) {}
 
         bool IsSet() const { return _isSet; }
 
-        Script** GetScript() const { return std::get<Script**>(_value); }
-
-        ScriptSet* GetScriptSet() const { return std::get<ScriptSet*>(_value); }
+        Script** GetScript() const { return _script; }
+        ScriptSet* GetScriptSet() const { return _scriptSet; }
     };
 }