From bfb4dffccec0a902586927c41b2454c8ddacd9e3 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Wed, 20 Nov 2024 19:57:24 +0100
Subject: add Log function to script

---
 src/crepe/api/Script.hpp | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'src/crepe/api/Script.hpp')

diff --git a/src/crepe/api/Script.hpp b/src/crepe/api/Script.hpp
index a064a90..4593d69 100644
--- a/src/crepe/api/Script.hpp
+++ b/src/crepe/api/Script.hpp
@@ -25,4 +25,9 @@ std::vector<std::reference_wrapper<T>> Script::get_components() const {
 	return mgr.get_components_by_id<T>(this->game_object_id);
 }
 
+template <typename... Args>
+void Script::logf(Args &&... args) {
+	Log::logf(std::forward<Args>(args)...);
+}
+
 } // namespace crepe
-- 
cgit v1.2.3


From da379a58033c0ef3c9c854326a3fca25d6e54319 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Wed, 20 Nov 2024 22:18:54 +0100
Subject: add Script::subscribe

---
 src/crepe/api/BehaviorScript.hpp |  1 +
 src/crepe/api/CMakeLists.txt     |  1 +
 src/crepe/api/Script.cpp         | 11 +++++++++++
 src/crepe/api/Script.h           | 36 +++++++++++++++++++++++++++++++++---
 src/crepe/api/Script.hpp         |  9 ++++++++-
 5 files changed, 54 insertions(+), 4 deletions(-)
 create mode 100644 src/crepe/api/Script.cpp

(limited to 'src/crepe/api/Script.hpp')

diff --git a/src/crepe/api/BehaviorScript.hpp b/src/crepe/api/BehaviorScript.hpp
index d80321d..dd1efd5 100644
--- a/src/crepe/api/BehaviorScript.hpp
+++ b/src/crepe/api/BehaviorScript.hpp
@@ -16,6 +16,7 @@ BehaviorScript & BehaviorScript::set_script() {
 	Script * s = new T();
 	s->game_object_id = this->game_object_id;
 	s->component_manager_ref = &this->component_manager;
+	s->event_manager_ref = &EventManager::get_instance();
 	this->script = std::unique_ptr<Script>(s);
 	return *this;
 }
diff --git a/src/crepe/api/CMakeLists.txt b/src/crepe/api/CMakeLists.txt
index a185ca6..a63c5ff 100644
--- a/src/crepe/api/CMakeLists.txt
+++ b/src/crepe/api/CMakeLists.txt
@@ -23,6 +23,7 @@ target_sources(crepe PUBLIC
 	LoopManager.cpp
 	LoopTimer.cpp
 	EventHandler.cpp
+	Script.cpp
 )
 
 target_sources(crepe PUBLIC FILE_SET HEADERS FILES
diff --git a/src/crepe/api/Script.cpp b/src/crepe/api/Script.cpp
new file mode 100644
index 0000000..9ebb074
--- /dev/null
+++ b/src/crepe/api/Script.cpp
@@ -0,0 +1,11 @@
+#include "Script.h"
+
+using namespace crepe;
+
+Script::~Script() {
+	EventManager & evmgr = *this->event_manager_ref;
+	for (auto id : this->listeners) {
+		evmgr.unsubscribe(id);
+	}
+}
+
diff --git a/src/crepe/api/Script.h b/src/crepe/api/Script.h
index ddb499c..c9eb211 100644
--- a/src/crepe/api/Script.h
+++ b/src/crepe/api/Script.h
@@ -4,6 +4,8 @@
 
 #include "../types.h"
 
+#include "EventManager.h"
+
 namespace crepe {
 
 class ScriptSystem;
@@ -85,6 +87,14 @@ protected:
 	template <typename... Args>
 	void logf(Args &&... args);
 
+	/**
+	 * \brief Subscribe to an event
+	 *
+	 * \see EventManager::subscribe
+	 */
+	template <typename EventType>
+	void subscribe(const EventHandler<EventType> & callback, event_channel_t channel = EventManager::CHANNEL_ALL);
+
 	//! \}
 
 protected:
@@ -93,17 +103,37 @@ protected:
 	Script() = default;
 	//! Only \c BehaviorScript instantiates Script
 	friend class BehaviorScript;
+public:
+	// std::unique_ptr destroys script
+	virtual ~Script();
+
+	Script(const Script &) = delete;
+	Script(Script &&) = delete;
+	Script & operator=(const Script &) = delete;
+	Script & operator=(Script &&) = delete;
 
 private:
-	// These references are set by BehaviorScript immediately after calling the constructor of
-	// Script.
+	/**
+	 * \name Late references
+	 *
+	 * These references are set by BehaviorScript immediately after calling the constructor of
+	 * Script.
+	 *
+	 * \{
+	 */
+	//! Game object ID of game object parent BehaviorScript is attached to
 	game_object_id_t game_object_id = -1;
+	//! Reference to component manager instance
 	ComponentManager * component_manager_ref = nullptr;
-	// TODO: use OptionalRef instead of pointer
+	//! Reference to event manager instance
+	EventManager * event_manager_ref = nullptr;
+	//! \}
 
 private:
 	//! Flag to indicate if \c init() has been called already
 	bool initialized = false;
+	//! List of subscribed events
+	std::vector<subscription_t> listeners;
 };
 
 } // namespace crepe
diff --git a/src/crepe/api/Script.hpp b/src/crepe/api/Script.hpp
index 4593d69..42c8f0b 100644
--- a/src/crepe/api/Script.hpp
+++ b/src/crepe/api/Script.hpp
@@ -20,7 +20,7 @@ T & Script::get_component() const {
 
 template <typename T>
 std::vector<std::reference_wrapper<T>> Script::get_components() const {
-	auto & mgr = *this->component_manager_ref;
+	ComponentManager & mgr = *this->component_manager_ref;
 
 	return mgr.get_components_by_id<T>(this->game_object_id);
 }
@@ -30,4 +30,11 @@ void Script::logf(Args &&... args) {
 	Log::logf(std::forward<Args>(args)...);
 }
 
+template <typename EventType>
+void Script::subscribe(const EventHandler<EventType> & callback, event_channel_t channel) {
+	EventManager & mgr = *this->event_manager_ref;
+	subscription_t listener = mgr.subscribe<EventType>(callback, channel);
+	this->listeners.push_back(listener);
+}
+
 } // namespace crepe
-- 
cgit v1.2.3


From 502fb8e8d1dcfe10f55fdef2cdfb71afec806204 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Thu, 21 Nov 2024 09:43:13 +0100
Subject: pull script/event changes from `loek/collision-system`

---
 Doxyfile                         |  6 ++--
 src/crepe/api/BehaviorScript.h   |  7 +++--
 src/crepe/api/BehaviorScript.hpp | 11 +++++---
 src/crepe/api/Event.h            | 12 ++++----
 src/crepe/api/EventManager.h     |  4 +--
 src/crepe/api/Script.cpp         |  6 ++++
 src/crepe/api/Script.h           | 60 +++++++++++++++++++++++++++++++++++-----
 src/crepe/api/Script.hpp         | 20 ++++++++++++--
 src/doc/layout.xml               |  2 +-
 src/test/ScriptTest.cpp          | 60 ++++++++++++++++++++++++++++++++++++++--
 10 files changed, 159 insertions(+), 29 deletions(-)

(limited to 'src/crepe/api/Script.hpp')

diff --git a/Doxyfile b/Doxyfile
index e0a31df..f2714cd 100644
--- a/Doxyfile
+++ b/Doxyfile
@@ -19,15 +19,17 @@ TAB_SIZE = 2
 
 HTML_INDEX_NUM_ENTRIES = 2
 HTML_EXTRA_STYLESHEET = src/doc/style.css
+SHOW_HEADERFILE = NO
 
 USE_MDFILE_AS_MAINPAGE = ./readme.md
 REPEAT_BRIEF = NO
 
-INTERNAL_DOCS = YES
-EXTRACT_PRIVATE = YES
 EXTRACT_STATIC = YES
 HIDE_UNDOC_NAMESPACES = YES
 HIDE_UNDOC_CLASSES = YES
 
 QUIET = YES
 
+# set these to NO for user-only docs
+INTERNAL_DOCS = YES
+EXTRACT_PRIVATE = YES
diff --git a/src/crepe/api/BehaviorScript.h b/src/crepe/api/BehaviorScript.h
index 9d85d4c..d556fe5 100644
--- a/src/crepe/api/BehaviorScript.h
+++ b/src/crepe/api/BehaviorScript.h
@@ -39,11 +39,14 @@ public:
 	 * \brief Set the concrete script of this component
 	 *
 	 * \tparam T Concrete script type (derived from \c crepe::Script)
+	 * \tparam Args Arguments for concrete script constructor
+	 *
+	 * \param args Arguments for concrete script constructor (forwarded using perfect forwarding)
 	 *
 	 * \returns Reference to BehaviorScript component (`*this`)
 	 */
-	template <class T>
-	BehaviorScript & set_script();
+	template <class T, typename... Args>
+	BehaviorScript & set_script(Args &&... args);
 
 protected:
 	//! Script instance
diff --git a/src/crepe/api/BehaviorScript.hpp b/src/crepe/api/BehaviorScript.hpp
index dd1efd5..5b5a418 100644
--- a/src/crepe/api/BehaviorScript.hpp
+++ b/src/crepe/api/BehaviorScript.hpp
@@ -9,14 +9,17 @@
 
 namespace crepe {
 
-template <class T>
-BehaviorScript & BehaviorScript::set_script() {
+template <class T, typename... Args>
+BehaviorScript & BehaviorScript::set_script(Args &&... args) {
 	dbg_trace();
 	static_assert(std::is_base_of<Script, T>::value);
-	Script * s = new T();
-	s->game_object_id = this->game_object_id;
+	Script * s = new T(std::forward<Args>(args)...);
+
+	s->game_object_id_ref = &this->game_object_id;
+	s->active_ref = &this->active;
 	s->component_manager_ref = &this->component_manager;
 	s->event_manager_ref = &EventManager::get_instance();
+
 	this->script = std::unique_ptr<Script>(s);
 	return *this;
 }
diff --git a/src/crepe/api/Event.h b/src/crepe/api/Event.h
index 06cf7f3..ab4be2b 100644
--- a/src/crepe/api/Event.h
+++ b/src/crepe/api/Event.h
@@ -1,10 +1,12 @@
-// TODO discussing the location of these events
 #pragma once
+// TODO discussing the location of these events
 
 #include <string>
 
 #include "KeyCodes.h"
 
+namespace crepe {
+
 /**
  * \brief Base class for all event types in the system.
  */
@@ -91,11 +93,7 @@ public:
 /**
  * \brief Event triggered during a collision between objects.
  */
-class CollisionEvent : public Event {
-public:
-	//! Data describing the collision (currently not implemented).
-	// Collision collisionData;
-};
+class CollisionEvent : public Event { };
 
 /**
  * \brief Event triggered when text is submitted, e.g., from a text input.
@@ -110,3 +108,5 @@ public:
  * \brief Event triggered to indicate the application is shutting down.
  */
 class ShutDownEvent : public Event {};
+
+}
diff --git a/src/crepe/api/EventManager.h b/src/crepe/api/EventManager.h
index 348a04d..1a33023 100644
--- a/src/crepe/api/EventManager.h
+++ b/src/crepe/api/EventManager.h
@@ -77,7 +77,7 @@ public:
 	 * \param channel The channel to trigger the event on (default is CHANNEL_ALL, which triggers on all channels).
 	 */
 	template <typename EventType>
-	void trigger_event(const EventType & event, event_channel_t channel = CHANNEL_ALL);
+	void trigger_event(const EventType & event = {}, event_channel_t channel = CHANNEL_ALL);
 
 	/**
 	 * \brief Queue an event for later processing.
@@ -89,7 +89,7 @@ public:
 	 * \param channel The channel to associate with the event (default is CHANNEL_ALL).
 	 */
 	template <typename EventType>
-	void queue_event(const EventType & event, event_channel_t channel = CHANNEL_ALL);
+	void queue_event(const EventType & event = {}, event_channel_t channel = CHANNEL_ALL);
 
 	/**
 	 * \brief Process all queued events.
diff --git a/src/crepe/api/Script.cpp b/src/crepe/api/Script.cpp
index 9ebb074..d3fae07 100644
--- a/src/crepe/api/Script.cpp
+++ b/src/crepe/api/Script.cpp
@@ -9,3 +9,9 @@ Script::~Script() {
 	}
 }
 
+template <>
+void Script::subscribe(const EventHandler<CollisionEvent> & callback) {
+	const game_object_id_t & game_object_id = *this->game_object_id_ref;
+	this->subscribe_internal(callback, game_object_id);
+}
+
diff --git a/src/crepe/api/Script.h b/src/crepe/api/Script.h
index c9eb211..373b253 100644
--- a/src/crepe/api/Script.h
+++ b/src/crepe/api/Script.h
@@ -18,8 +18,10 @@ class ComponentManager;
  * This class is used as a base class for user-defined scripts that can be added to game
  * objects using the \c BehaviorScript component.
  *
- * \note Additional *events* (like Unity's OnDisable and OnEnable) should be implemented as
+ * \info Additional *events* (like Unity's OnDisable and OnEnable) should be implemented as
  * member or lambda methods in derivative user script classes and registered in \c init().
+ *
+ * \see feature_script
  */
 class Script {
 protected:
@@ -88,18 +90,40 @@ protected:
 	void logf(Args &&... args);
 
 	/**
-	 * \brief Subscribe to an event
-	 *
+	 * \brief Subscribe to an event with an explicit channel
+	 * \see EventManager::subscribe
+	 */
+	template <typename EventType>
+	void subscribe(const EventHandler<EventType> & callback, event_channel_t channel);
+	/**
+	 * \brief Subscribe to an event on EventManager::CHANNEL_ALL
 	 * \see EventManager::subscribe
 	 */
 	template <typename EventType>
-	void subscribe(const EventHandler<EventType> & callback, event_channel_t channel = EventManager::CHANNEL_ALL);
+	void subscribe(const EventHandler<EventType> & callback);
 
 	//! \}
 
+private:
+	/**
+	 * \brief Internal subscribe function
+	 *
+	 * This function exists so certain template specializations of Script::subscribe can be
+	 * explicitly deleted, and does the following:
+	 * - Wrap the user-provided callback in a check that tests if the parent BehaviorScript
+	 *   component is still active
+	 * - Store the subscriber handle returned by the event manager so this listener is
+	 *   automatically unsubscribed at the end of this Script instance's life
+	 *
+	 * \tparam EventType concrete Event class
+	 * \param callback User-provided callback function
+	 * \param channel Event channel (may have been overridden by template specializations)
+	 */
+	template <typename EventType>
+	void subscribe_internal(const EventHandler<EventType> & callback, event_channel_t channel);
+
 protected:
-	// NOTE: Script must have a constructor without arguments so the game programmer doesn't need
-	// to manually add `using Script::Script` to their concrete script class.
+	// NOTE: This must be the only constructor on Script, see "Late references" below
 	Script() = default;
 	//! Only \c BehaviorScript instantiates Script
 	friend class BehaviorScript;
@@ -107,6 +131,7 @@ public:
 	// std::unique_ptr destroys script
 	virtual ~Script();
 
+private:
 	Script(const Script &) = delete;
 	Script(Script &&) = delete;
 	Script & operator=(const Script &) = delete;
@@ -119,10 +144,19 @@ private:
 	 * These references are set by BehaviorScript immediately after calling the constructor of
 	 * Script.
 	 *
+	 * \note Script must have a constructor without arguments so the game programmer doesn't need
+	 * to manually add `using Script::Script` to their concrete script class if they want to
+	 * implement a non-default constructor (e.g. for passing references to their own concrete
+	 * Script classes).
+	 *
+	 * \todo These should be converted to OptionalRef<> once `loek/util` is merged
+	 *
 	 * \{
 	 */
 	//! Game object ID of game object parent BehaviorScript is attached to
-	game_object_id_t game_object_id = -1;
+	const game_object_id_t * game_object_id_ref = nullptr;
+	//! Reference to parent component
+	bool * active_ref = nullptr;
 	//! Reference to component manager instance
 	ComponentManager * component_manager_ref = nullptr;
 	//! Reference to event manager instance
@@ -136,6 +170,18 @@ private:
 	std::vector<subscription_t> listeners;
 };
 
+/**
+ * \brief Subscribe to CollisionEvent for the current GameObject
+ *
+ * This is a template specialization for Script::subscribe which automatically sets the event
+ * channel so the callback handler is only called for CollisionEvent events that apply to the
+ * current GameObject the parent BehaviorScript is attached to.
+ */
+template <>
+void Script::subscribe(const EventHandler<CollisionEvent> & callback);
+template <>
+void Script::subscribe(const EventHandler<CollisionEvent> & callback, event_channel_t) = delete;
+
 } // namespace crepe
 
 #include "Script.hpp"
diff --git a/src/crepe/api/Script.hpp b/src/crepe/api/Script.hpp
index 42c8f0b..8186bd4 100644
--- a/src/crepe/api/Script.hpp
+++ b/src/crepe/api/Script.hpp
@@ -22,7 +22,7 @@ template <typename T>
 std::vector<std::reference_wrapper<T>> Script::get_components() const {
 	ComponentManager & mgr = *this->component_manager_ref;
 
-	return mgr.get_components_by_id<T>(this->game_object_id);
+	return mgr.get_components_by_id<T>(*this->game_object_id_ref);
 }
 
 template <typename... Args>
@@ -31,10 +31,24 @@ void Script::logf(Args &&... args) {
 }
 
 template <typename EventType>
-void Script::subscribe(const EventHandler<EventType> & callback, event_channel_t channel) {
+void Script::subscribe_internal(const EventHandler<EventType> & callback, event_channel_t channel) {
 	EventManager & mgr = *this->event_manager_ref;
-	subscription_t listener = mgr.subscribe<EventType>(callback, channel);
+	subscription_t listener = mgr.subscribe<EventType>([this, callback](const EventType & data) -> bool {
+		bool & active = *this->active_ref;
+		if (!active) return false;
+		return callback(data);
+	}, channel);
 	this->listeners.push_back(listener);
 }
 
+template <typename EventType>
+void Script::subscribe(const EventHandler<EventType> & callback, event_channel_t channel) {
+	this->subscribe_internal(callback, channel);
+}
+
+template <typename EventType>
+void Script::subscribe(const EventHandler<EventType> & callback) {
+	this->subscribe_internal(callback, EventManager::CHANNEL_ALL);
+}
+
 } // namespace crepe
diff --git a/src/doc/layout.xml b/src/doc/layout.xml
index 2244fa7..7f514d4 100644
--- a/src/doc/layout.xml
+++ b/src/doc/layout.xml
@@ -42,6 +42,7 @@
 	</navindex>
 	<class>
 		<briefdescription visible="yes"/>
+		<detaileddescription title=""/>
 		<includes visible="$SHOW_HEADERFILE"/>
 		<inheritancegraph visible="yes"/>
 		<collaborationgraph visible="yes"/>
@@ -79,7 +80,6 @@
 			<related title="" subtitle=""/>
 			<membergroups visible="yes"/>
 		</memberdecl>
-		<detaileddescription title=""/>
 		<memberdef>
 			<inlineclasses title=""/>
 			<typedefs title=""/>
diff --git a/src/test/ScriptTest.cpp b/src/test/ScriptTest.cpp
index 19fef6d..875c422 100644
--- a/src/test/ScriptTest.cpp
+++ b/src/test/ScriptTest.cpp
@@ -9,26 +9,44 @@
 #include <crepe/api/GameObject.h>
 #include <crepe/api/Script.h>
 #include <crepe/api/Vector2.h>
+#include <crepe/api/Event.h>
+#include <crepe/api/EventManager.h>
 #include <crepe/system/ScriptSystem.h>
 
 using namespace std;
 using namespace crepe;
 using namespace testing;
 
+class MyEvent : public Event { };
+
 class ScriptTest : public Test {
 public:
 	ComponentManager component_manager{};
 	ScriptSystem system{component_manager};
+	EventManager & evmgr = EventManager::get_instance();
 
 	class MyScript : public Script {
 		// NOTE: default (private) visibility of init and update shouldn't cause
 		// issues!
-		void init() { this->init_count++; }
-		void update() { this->update_count++; }
+		void init() {
+			this->init_count++;
+
+			subscribe<MyEvent>([this](const MyEvent &) {
+				this->event_count++;
+				return true;
+			});
+
+			// init should never be called more than once
+			EXPECT_LE(this->init_count, 1);
+		}
+		void update() {
+			this->update_count++;
+		}
 
 	public:
 		unsigned init_count = 0;
 		unsigned update_count = 0;
+		unsigned event_count = 0;
 	};
 
 	BehaviorScript * behaviorscript_ref = nullptr;
@@ -46,21 +64,58 @@ public:
 
 		this->script_ref = (MyScript *) this->behaviorscript_ref->script.get();
 		ASSERT_NE(this->script_ref, nullptr);
+
+		// sanity
+		ASSERT_EQ(script_ref->init_count, 0);
+		ASSERT_EQ(script_ref->update_count, 0);
+		ASSERT_EQ(script_ref->event_count, 0);
 	}
 };
 
 TEST_F(ScriptTest, Default) {
 	EXPECT_EQ(0, this->script_ref->init_count);
 	EXPECT_EQ(0, this->script_ref->update_count);
+	EXPECT_EQ(0, this->script_ref->event_count);
 }
 
 TEST_F(ScriptTest, UpdateOnce) {
+	this->system.update();
+	EXPECT_EQ(1, this->script_ref->init_count);
+	EXPECT_EQ(1, this->script_ref->update_count);
+	EXPECT_EQ(0, this->script_ref->event_count);
+}
+
+TEST_F(ScriptTest, UpdateInactive) {
+	this->behaviorscript_ref->active = false;
+	this->system.update();
 	EXPECT_EQ(0, this->script_ref->init_count);
 	EXPECT_EQ(0, this->script_ref->update_count);
+	EXPECT_EQ(0, this->script_ref->event_count);
+
+	this->behaviorscript_ref->active = true;
+	this->system.update();
+	EXPECT_EQ(1, this->script_ref->init_count);
+	EXPECT_EQ(1, this->script_ref->update_count);
+	EXPECT_EQ(0, this->script_ref->event_count);
+}
 
+TEST_F(ScriptTest, EventInactive) {
 	this->system.update();
+	this->behaviorscript_ref->active = false;
 	EXPECT_EQ(1, this->script_ref->init_count);
 	EXPECT_EQ(1, this->script_ref->update_count);
+	EXPECT_EQ(0, this->script_ref->event_count);
+
+	this->evmgr.trigger_event<MyEvent>();
+	EXPECT_EQ(1, this->script_ref->init_count);
+	EXPECT_EQ(1, this->script_ref->update_count);
+	EXPECT_EQ(0, this->script_ref->event_count);
+
+	this->behaviorscript_ref->active = true;
+	this->evmgr.trigger_event<MyEvent>();
+	EXPECT_EQ(1, this->script_ref->init_count);
+	EXPECT_EQ(1, this->script_ref->update_count);
+	EXPECT_EQ(1, this->script_ref->event_count);
 }
 
 TEST_F(ScriptTest, ListScripts) {
@@ -70,3 +125,4 @@ TEST_F(ScriptTest, ListScripts) {
 	}
 	ASSERT_EQ(1, script_count);
 }
+
-- 
cgit v1.2.3


From 70b1bf50de703330436f2ae9cb103fe33cbb567e Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Thu, 21 Nov 2024 10:00:35 +0100
Subject: `make format`

---
 src/crepe/api/Event.h         |  4 ++--
 src/crepe/api/Script.cpp      |  1 -
 src/crepe/api/Script.h        |  4 +++-
 src/crepe/api/Script.hpp      | 15 +++++++++------
 src/test/SceneManagerTest.cpp |  4 ++--
 src/test/ScriptTest.cpp       | 11 ++++-------
 6 files changed, 20 insertions(+), 19 deletions(-)

(limited to 'src/crepe/api/Script.hpp')

diff --git a/src/crepe/api/Event.h b/src/crepe/api/Event.h
index ab4be2b..b267e3e 100644
--- a/src/crepe/api/Event.h
+++ b/src/crepe/api/Event.h
@@ -93,7 +93,7 @@ public:
 /**
  * \brief Event triggered during a collision between objects.
  */
-class CollisionEvent : public Event { };
+class CollisionEvent : public Event {};
 
 /**
  * \brief Event triggered when text is submitted, e.g., from a text input.
@@ -109,4 +109,4 @@ public:
  */
 class ShutDownEvent : public Event {};
 
-}
+} // namespace crepe
diff --git a/src/crepe/api/Script.cpp b/src/crepe/api/Script.cpp
index d3fae07..0e73848 100644
--- a/src/crepe/api/Script.cpp
+++ b/src/crepe/api/Script.cpp
@@ -14,4 +14,3 @@ void Script::subscribe(const EventHandler<CollisionEvent> & callback) {
 	const game_object_id_t & game_object_id = *this->game_object_id_ref;
 	this->subscribe_internal(callback, game_object_id);
 }
-
diff --git a/src/crepe/api/Script.h b/src/crepe/api/Script.h
index 373b253..8356fd2 100644
--- a/src/crepe/api/Script.h
+++ b/src/crepe/api/Script.h
@@ -127,6 +127,7 @@ protected:
 	Script() = default;
 	//! Only \c BehaviorScript instantiates Script
 	friend class BehaviorScript;
+
 public:
 	// std::unique_ptr destroys script
 	virtual ~Script();
@@ -180,7 +181,8 @@ private:
 template <>
 void Script::subscribe(const EventHandler<CollisionEvent> & callback);
 template <>
-void Script::subscribe(const EventHandler<CollisionEvent> & callback, event_channel_t) = delete;
+void Script::subscribe(const EventHandler<CollisionEvent> & callback, event_channel_t)
+	= delete;
 
 } // namespace crepe
 
diff --git a/src/crepe/api/Script.hpp b/src/crepe/api/Script.hpp
index 8186bd4..e60be07 100644
--- a/src/crepe/api/Script.hpp
+++ b/src/crepe/api/Script.hpp
@@ -31,13 +31,16 @@ void Script::logf(Args &&... args) {
 }
 
 template <typename EventType>
-void Script::subscribe_internal(const EventHandler<EventType> & callback, event_channel_t channel) {
+void Script::subscribe_internal(const EventHandler<EventType> & callback,
+								event_channel_t channel) {
 	EventManager & mgr = *this->event_manager_ref;
-	subscription_t listener = mgr.subscribe<EventType>([this, callback](const EventType & data) -> bool {
-		bool & active = *this->active_ref;
-		if (!active) return false;
-		return callback(data);
-	}, channel);
+	subscription_t listener = mgr.subscribe<EventType>(
+		[this, callback](const EventType & data) -> bool {
+			bool & active = *this->active_ref;
+			if (!active) return false;
+			return callback(data);
+		},
+		channel);
 	this->listeners.push_back(listener);
 }
 
diff --git a/src/test/SceneManagerTest.cpp b/src/test/SceneManagerTest.cpp
index dab2ce9..1efcfb2 100644
--- a/src/test/SceneManagerTest.cpp
+++ b/src/test/SceneManagerTest.cpp
@@ -21,7 +21,7 @@ public:
 		GameObject object3 = mgr.new_object("scene_1", "tag_scene_1", Vector2{2, 0}, 0, 1);
 	}
 
-	string get_name() const { return "scene1";}
+	string get_name() const { return "scene1"; }
 };
 
 class ConcreteScene2 : public Scene {
@@ -36,7 +36,7 @@ public:
 		GameObject object4 = mgr.new_object("scene_2", "tag_scene_2", Vector2{0, 3}, 0, 1);
 	}
 
-	string get_name() const { return "scene2";}
+	string get_name() const { return "scene2"; }
 };
 
 class SceneManagerTest : public ::testing::Test {
diff --git a/src/test/ScriptTest.cpp b/src/test/ScriptTest.cpp
index 875c422..c35c3e2 100644
--- a/src/test/ScriptTest.cpp
+++ b/src/test/ScriptTest.cpp
@@ -6,18 +6,18 @@
 
 #include <crepe/ComponentManager.h>
 #include <crepe/api/BehaviorScript.h>
+#include <crepe/api/Event.h>
+#include <crepe/api/EventManager.h>
 #include <crepe/api/GameObject.h>
 #include <crepe/api/Script.h>
 #include <crepe/api/Vector2.h>
-#include <crepe/api/Event.h>
-#include <crepe/api/EventManager.h>
 #include <crepe/system/ScriptSystem.h>
 
 using namespace std;
 using namespace crepe;
 using namespace testing;
 
-class MyEvent : public Event { };
+class MyEvent : public Event {};
 
 class ScriptTest : public Test {
 public:
@@ -39,9 +39,7 @@ public:
 			// init should never be called more than once
 			EXPECT_LE(this->init_count, 1);
 		}
-		void update() {
-			this->update_count++;
-		}
+		void update() { this->update_count++; }
 
 	public:
 		unsigned init_count = 0;
@@ -125,4 +123,3 @@ TEST_F(ScriptTest, ListScripts) {
 	}
 	ASSERT_EQ(1, script_count);
 }
-
-- 
cgit v1.2.3


From db66bb4babb19e8a86d5c61281c8b94469729d03 Mon Sep 17 00:00:00 2001
From: Loek Le Blansch <loek@pipeframe.xyz>
Date: Thu, 21 Nov 2024 10:52:22 +0100
Subject: use OptionalRef instead of pointer references

---
 src/crepe/api/BehaviorScript.hpp  |   8 +--
 src/crepe/api/Script.cpp          |   4 +-
 src/crepe/api/Script.h            |  11 ++--
 src/crepe/api/Script.hpp          |   8 +--
 src/crepe/system/ScriptSystem.cpp |  30 ++++-------
 src/crepe/system/ScriptSystem.h   |  11 ----
 src/test/ScriptTest.cpp           | 109 ++++++++++++++++++++------------------
 7 files changed, 81 insertions(+), 100 deletions(-)

(limited to 'src/crepe/api/Script.hpp')

diff --git a/src/crepe/api/BehaviorScript.hpp b/src/crepe/api/BehaviorScript.hpp
index 5b5a418..bd59337 100644
--- a/src/crepe/api/BehaviorScript.hpp
+++ b/src/crepe/api/BehaviorScript.hpp
@@ -15,10 +15,10 @@ BehaviorScript & BehaviorScript::set_script(Args &&... args) {
 	static_assert(std::is_base_of<Script, T>::value);
 	Script * s = new T(std::forward<Args>(args)...);
 
-	s->game_object_id_ref = &this->game_object_id;
-	s->active_ref = &this->active;
-	s->component_manager_ref = &this->component_manager;
-	s->event_manager_ref = &EventManager::get_instance();
+	s->game_object_id = this->game_object_id;
+	s->active = this->active;
+	s->component_manager = this->component_manager;
+	s->event_manager = EventManager::get_instance();
 
 	this->script = std::unique_ptr<Script>(s);
 	return *this;
diff --git a/src/crepe/api/Script.cpp b/src/crepe/api/Script.cpp
index 0e73848..8bb3ef6 100644
--- a/src/crepe/api/Script.cpp
+++ b/src/crepe/api/Script.cpp
@@ -3,7 +3,7 @@
 using namespace crepe;
 
 Script::~Script() {
-	EventManager & evmgr = *this->event_manager_ref;
+	EventManager & evmgr = this->event_manager;
 	for (auto id : this->listeners) {
 		evmgr.unsubscribe(id);
 	}
@@ -11,6 +11,6 @@ Script::~Script() {
 
 template <>
 void Script::subscribe(const EventHandler<CollisionEvent> & callback) {
-	const game_object_id_t & game_object_id = *this->game_object_id_ref;
+	const game_object_id_t & game_object_id = this->game_object_id;
 	this->subscribe_internal(callback, game_object_id);
 }
diff --git a/src/crepe/api/Script.h b/src/crepe/api/Script.h
index 43efd15..6418b04 100644
--- a/src/crepe/api/Script.h
+++ b/src/crepe/api/Script.h
@@ -2,6 +2,7 @@
 
 #include <vector>
 
+#include "../util/OptionalRef.h"
 #include "../types.h"
 
 #include "EventManager.h"
@@ -150,18 +151,16 @@ private:
 	 * implement a non-default constructor (e.g. for passing references to their own concrete
 	 * Script classes).
 	 *
-	 * \todo These should be converted to OptionalRef<> once `loek/util` is merged
-	 *
 	 * \{
 	 */
 	//! Game object ID of game object parent BehaviorScript is attached to
-	const game_object_id_t * game_object_id_ref = nullptr;
+	OptionalRef<const game_object_id_t> game_object_id;
 	//! Reference to parent component
-	bool * active_ref = nullptr;
+	OptionalRef<bool> active;
 	//! Reference to component manager instance
-	ComponentManager * component_manager_ref = nullptr;
+	OptionalRef<ComponentManager> component_manager;
 	//! Reference to event manager instance
-	EventManager * event_manager_ref = nullptr;
+	OptionalRef<EventManager> event_manager;
 	//! \}
 
 private:
diff --git a/src/crepe/api/Script.hpp b/src/crepe/api/Script.hpp
index e94278d..a2463bf 100644
--- a/src/crepe/api/Script.hpp
+++ b/src/crepe/api/Script.hpp
@@ -20,9 +20,9 @@ T & Script::get_component() const {
 
 template <typename T>
 RefVector<T> Script::get_components() const {
-	ComponentManager & mgr = *this->component_manager_ref;
+	ComponentManager & mgr = this->component_manager;
 
-	return mgr.get_components_by_id<T>(*this->game_object_id_ref);
+	return mgr.get_components_by_id<T>(this->game_object_id);
 }
 
 template <typename... Args>
@@ -33,10 +33,10 @@ void Script::logf(Args &&... args) {
 template <typename EventType>
 void Script::subscribe_internal(const EventHandler<EventType> & callback,
 								event_channel_t channel) {
-	EventManager & mgr = *this->event_manager_ref;
+	EventManager & mgr = this->event_manager;
 	subscription_t listener = mgr.subscribe<EventType>(
 		[this, callback](const EventType & data) -> bool {
-			bool & active = *this->active_ref;
+			bool & active = this->active;
 			if (!active) return false;
 			return callback(data);
 		},
diff --git a/src/crepe/system/ScriptSystem.cpp b/src/crepe/system/ScriptSystem.cpp
index c33309c..d54bbc9 100644
--- a/src/crepe/system/ScriptSystem.cpp
+++ b/src/crepe/system/ScriptSystem.cpp
@@ -1,5 +1,3 @@
-#include <functional>
-
 #include "../ComponentManager.h"
 #include "../api/BehaviorScript.h"
 #include "../api/Script.h"
@@ -12,30 +10,20 @@ using namespace crepe;
 void ScriptSystem::update() {
 	dbg_trace();
 
-	RefVector<Script> scripts = this->get_scripts();
-
-	for (auto & script_ref : scripts) {
-		Script & script = script_ref.get();
-		if (!script.initialized) {
-			script.init();
-			script.initialized = true;
-		}
-		script.update();
-	}
-}
-
-RefVector<Script> ScriptSystem::get_scripts() const {
-	RefVector<Script> scripts = {};
 	ComponentManager & mgr = this->component_manager;
 	RefVector<BehaviorScript> behavior_scripts = mgr.get_components_by_type<BehaviorScript>();
 
-	for (auto behavior_script_ref : behavior_scripts) {
-		BehaviorScript & behavior_script = behavior_script_ref.get();
+	for (BehaviorScript & behavior_script : behavior_scripts) {
 		if (!behavior_script.active) continue;
+
 		Script * script = behavior_script.script.get();
 		if (script == nullptr) continue;
-		scripts.push_back(*script);
-	}
 
-	return scripts;
+		if (!script->initialized) {
+			script->init();
+			script->initialized = true;
+		}
+		script->update();
+	}
 }
+
diff --git a/src/crepe/system/ScriptSystem.h b/src/crepe/system/ScriptSystem.h
index 32e1fcd..936e9ca 100644
--- a/src/crepe/system/ScriptSystem.h
+++ b/src/crepe/system/ScriptSystem.h
@@ -2,8 +2,6 @@
 
 #include "System.h"
 
-#include "../types.h"
-
 namespace crepe {
 
 class Script;
@@ -25,15 +23,6 @@ public:
 	 * the \c BehaviorScript instance.
 	 */
 	void update() override;
-
-private:
-	/**
-	 * \brief Aggregate all active \c BehaviorScript components and return a list
-	 * of references to their \c Script instances (utility)
-	 *
-	 * \returns List of active \c Script instances
-	 */
-	RefVector<Script> get_scripts() const;
 };
 
 } // namespace crepe
diff --git a/src/test/ScriptTest.cpp b/src/test/ScriptTest.cpp
index c35c3e2..73009a7 100644
--- a/src/test/ScriptTest.cpp
+++ b/src/test/ScriptTest.cpp
@@ -23,7 +23,7 @@ class ScriptTest : public Test {
 public:
 	ComponentManager component_manager{};
 	ScriptSystem system{component_manager};
-	EventManager & evmgr = EventManager::get_instance();
+	EventManager & event_manager = EventManager::get_instance();
 
 	class MyScript : public Script {
 		// NOTE: default (private) visibility of init and update shouldn't cause
@@ -47,79 +47,84 @@ public:
 		unsigned event_count = 0;
 	};
 
-	BehaviorScript * behaviorscript_ref = nullptr;
-	MyScript * script_ref = nullptr;
+	OptionalRef<BehaviorScript> behaviorscript;
+	OptionalRef<MyScript> script;
 
 	void SetUp() override {
 		auto & mgr = this->component_manager;
 		GameObject entity = mgr.new_object("name");
 		BehaviorScript & component = entity.add_component<BehaviorScript>();
 
-		this->behaviorscript_ref = &component;
-		EXPECT_EQ(this->behaviorscript_ref->script.get(), nullptr);
+		this->behaviorscript = component;
+		ASSERT_TRUE(this->behaviorscript);
+		EXPECT_EQ(component.script.get(), nullptr);
 		component.set_script<MyScript>();
-		ASSERT_NE(this->behaviorscript_ref->script.get(), nullptr);
+		ASSERT_NE(component.script.get(), nullptr);
 
-		this->script_ref = (MyScript *) this->behaviorscript_ref->script.get();
-		ASSERT_NE(this->script_ref, nullptr);
+		this->script = *(MyScript *) component.script.get();
+		ASSERT_TRUE(this->script);
 
 		// sanity
-		ASSERT_EQ(script_ref->init_count, 0);
-		ASSERT_EQ(script_ref->update_count, 0);
-		ASSERT_EQ(script_ref->event_count, 0);
+		MyScript & script = this->script;
+		ASSERT_EQ(script.init_count, 0);
+		ASSERT_EQ(script.update_count, 0);
+		ASSERT_EQ(script.event_count, 0);
 	}
 };
 
 TEST_F(ScriptTest, Default) {
-	EXPECT_EQ(0, this->script_ref->init_count);
-	EXPECT_EQ(0, this->script_ref->update_count);
-	EXPECT_EQ(0, this->script_ref->event_count);
+	MyScript & script = this->script;
+	EXPECT_EQ(0, script.init_count);
+	EXPECT_EQ(0, script.update_count);
+	EXPECT_EQ(0, script.event_count);
 }
 
 TEST_F(ScriptTest, UpdateOnce) {
-	this->system.update();
-	EXPECT_EQ(1, this->script_ref->init_count);
-	EXPECT_EQ(1, this->script_ref->update_count);
-	EXPECT_EQ(0, this->script_ref->event_count);
+	MyScript & script = this->script;
+
+	system.update();
+	EXPECT_EQ(1, script.init_count);
+	EXPECT_EQ(1, script.update_count);
+	EXPECT_EQ(0, script.event_count);
 }
 
 TEST_F(ScriptTest, UpdateInactive) {
-	this->behaviorscript_ref->active = false;
-	this->system.update();
-	EXPECT_EQ(0, this->script_ref->init_count);
-	EXPECT_EQ(0, this->script_ref->update_count);
-	EXPECT_EQ(0, this->script_ref->event_count);
-
-	this->behaviorscript_ref->active = true;
-	this->system.update();
-	EXPECT_EQ(1, this->script_ref->init_count);
-	EXPECT_EQ(1, this->script_ref->update_count);
-	EXPECT_EQ(0, this->script_ref->event_count);
+	BehaviorScript & behaviorscript = this->behaviorscript;
+	MyScript & script = this->script;
+
+	behaviorscript.active = false;
+	system.update();
+	EXPECT_EQ(0, script.init_count);
+	EXPECT_EQ(0, script.update_count);
+	EXPECT_EQ(0, script.event_count);
+
+	behaviorscript.active = true;
+	system.update();
+	EXPECT_EQ(1, script.init_count);
+	EXPECT_EQ(1, script.update_count);
+	EXPECT_EQ(0, script.event_count);
 }
 
 TEST_F(ScriptTest, EventInactive) {
-	this->system.update();
-	this->behaviorscript_ref->active = false;
-	EXPECT_EQ(1, this->script_ref->init_count);
-	EXPECT_EQ(1, this->script_ref->update_count);
-	EXPECT_EQ(0, this->script_ref->event_count);
-
-	this->evmgr.trigger_event<MyEvent>();
-	EXPECT_EQ(1, this->script_ref->init_count);
-	EXPECT_EQ(1, this->script_ref->update_count);
-	EXPECT_EQ(0, this->script_ref->event_count);
-
-	this->behaviorscript_ref->active = true;
-	this->evmgr.trigger_event<MyEvent>();
-	EXPECT_EQ(1, this->script_ref->init_count);
-	EXPECT_EQ(1, this->script_ref->update_count);
-	EXPECT_EQ(1, this->script_ref->event_count);
+	BehaviorScript & behaviorscript = this->behaviorscript;
+	MyScript & script = this->script;
+	EventManager & evmgr = this->event_manager;
+
+	system.update();
+	behaviorscript.active = false;
+	EXPECT_EQ(1, script.init_count);
+	EXPECT_EQ(1, script.update_count);
+	EXPECT_EQ(0, script.event_count);
+
+	evmgr.trigger_event<MyEvent>();
+	EXPECT_EQ(1, script.init_count);
+	EXPECT_EQ(1, script.update_count);
+	EXPECT_EQ(0, script.event_count);
+
+	behaviorscript.active = true;
+	evmgr.trigger_event<MyEvent>();
+	EXPECT_EQ(1, script.init_count);
+	EXPECT_EQ(1, script.update_count);
+	EXPECT_EQ(1, script.event_count);
 }
 
-TEST_F(ScriptTest, ListScripts) {
-	size_t script_count = 0;
-	for (auto & _ : this->system.get_scripts()) {
-		script_count++;
-	}
-	ASSERT_EQ(1, script_count);
-}
-- 
cgit v1.2.3