From d258fcc8efdb6a968a220c4590a204292a16ad42 Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Thu, 14 Nov 2024 20:27:05 +0100 Subject: added thoughts --- src/crepe/Component.h | 12 +++++++++--- src/crepe/api/AudioSource.h | 11 ++++++++++- src/crepe/system/AudioSystem.cpp | 37 ++++++++++++++++++++++++++++++++++++- src/crepe/system/AudioSystem.h | 2 ++ src/test/ResourceManagerTest.cpp | 11 ++++++----- 5 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/crepe/Component.h b/src/crepe/Component.h index 12c10cb..7734335 100644 --- a/src/crepe/Component.h +++ b/src/crepe/Component.h @@ -16,7 +16,12 @@ class Component { public: //! Whether the component is active bool active = true; - //! The id of the GameObject this component belongs to + /** + * \brief The id of the GameObject this component belongs to + * + * \note Only systems are supposed to use this member, but since friend + * relations aren't inherited this needs to be public. + */ const game_object_id_t game_object_id; protected: @@ -24,10 +29,9 @@ protected: * \param id The id of the GameObject this component belongs to */ Component(game_object_id_t id); - //! Only the ComponentManager can create components + //! Only ComponentManager can create components friend class ComponentManager; -public: /** * \brief Get the maximum number of instances for this component * @@ -38,6 +42,8 @@ public: * \return The maximum number of instances for this component */ virtual int get_instances_max() const { return -1; } + //! Only ComponentManager needs to know the max instance count + friend class ComponentManager; }; } // namespace crepe diff --git a/src/crepe/api/AudioSource.h b/src/crepe/api/AudioSource.h index 8a78927..1264790 100644 --- a/src/crepe/api/AudioSource.h +++ b/src/crepe/api/AudioSource.h @@ -7,10 +7,19 @@ namespace crepe { +class AudioSystem; + //! Audio source component class AudioSource : public Component { -public: + //! AudioSource components are handled by AudioSystem + friend class AudioSystem; + +protected: AudioSource(game_object_id_t id, const Asset & source); + //! Only ComponentManager can create components + friend class ComponentManager; +public: + // But std::unique_ptr needs to be able to destoy this component again virtual ~AudioSource() = default; public: diff --git a/src/crepe/system/AudioSystem.cpp b/src/crepe/system/AudioSystem.cpp index 67967ef..c8dae9d 100644 --- a/src/crepe/system/AudioSystem.cpp +++ b/src/crepe/system/AudioSystem.cpp @@ -14,7 +14,42 @@ void AudioSystem::update() { AudioSource & component = component_ref.get(); if (!component.active) continue; - // TODO: fetch Sound instance from resourcemanager + /** + * How this is supposed to work: + * - Get an instance of Sound for this resource/component combo (Sound + * instance is supposed to be unique per component, even if they use the + * same underlying asset). + * OR + * - Use the same instance of Sound if this is what the cache returns + * (= what the game programmer's wishes to do). + * + * NOT supposed to happen but still the case: + * - Below function call causes assets to be cached unintentionally + * - Cached assets are deleted at the end of a scene (i think?) + * - I'm not sure if the ResourceManager is even supposed to have a public + * `.clear()` method since the control over resource lifetime is + * explicitly handed over to the game programmer by using ResourceManager + * to cache/uncache. I believe the proper methods are supposed to be: + * + * - get() get a reference to resource (used here) + * - clear() clears NON-cached assets + * - cache() marks asset as "do not delete at end of scene" + * - uncache() undoes the above + * + * I think somewhere in the above function calls a unique identifier for + * the Asset/GameObject should be given to make sure the unique instance + * shit works as intended. The resource manager is also used for things + * other than sounds. + * + * Also need to check: + * - Is it an issue if there are multiple AudioSource components playing + * the same sample (= identical Asset), while they are all triggered + * using the same underlying instance of Sound (esp. w/ + * play/pause/retrigger behavior). + */ + Sound & sound = this->resman.cache(component.source); + sound.set_context(this->context); + // TODO: lots of state diffing } } diff --git a/src/crepe/system/AudioSystem.h b/src/crepe/system/AudioSystem.h index e037f51..d0b4f9a 100644 --- a/src/crepe/system/AudioSystem.h +++ b/src/crepe/system/AudioSystem.h @@ -1,6 +1,7 @@ #pragma once #include "../facade/SoundContext.h" +#include "../api/ResourceManager.h" #include "System.h" @@ -13,6 +14,7 @@ public: private: SoundContext context {}; + ResourceManager & resman = ResourceManager::get_instance(); }; } // namespace crepe diff --git a/src/test/ResourceManagerTest.cpp b/src/test/ResourceManagerTest.cpp index 5d1ae7a..3fc9ebd 100644 --- a/src/test/ResourceManagerTest.cpp +++ b/src/test/ResourceManagerTest.cpp @@ -14,12 +14,17 @@ public: Config & cfg = Config::get_instance(); void SetUp() override { - cfg.log.level = Log::Level::TRACE; resman.clear(); } }; TEST_F(ResourceManagerTest, Main) { + // NOTE: there is no way (that I know of) to ensure the last cache call + // allocates the new Sound instance in a different location than the first, + // so this test should be verified manually using these print statements and + // debug trace messages. + cfg.log.level = Log::Level::TRACE; + Asset path1 = "mwe/audio/sfx1.wav"; Asset path2 = "mwe/audio/sfx1.wav"; ASSERT_EQ(path1, path2); @@ -43,9 +48,5 @@ TEST_F(ResourceManagerTest, Main) { Log::logf(Log::Level::DEBUG, "Get first sound again (constructor call)"); Sound & sound = resman.cache(path1); - - // NOTE: there is no way (that I know of) to ensure the above statement - // allocates the new Sound instance in a different location than the first, - // so this test was verified using the above print statements. } -- cgit v1.2.3