From 7a8657dfe019104aced61a5b63e63f61ad919f7a Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Mon, 2 Dec 2024 16:13:08 +0100 Subject: remove `Private` --- src/crepe/api/AudioSource.h | 15 +++- src/crepe/facade/Sound.h | 6 -- src/crepe/facade/SoundContext.cpp | 23 +++--- src/crepe/facade/SoundContext.h | 15 +++- src/crepe/facade/SoundHandle.h | 13 ++++ src/crepe/system/AudioSystem.cpp | 44 +++++------ src/crepe/system/AudioSystem.h | 29 ++----- src/crepe/util/CMakeLists.txt | 3 - src/crepe/util/Private.cpp | 27 ------- src/crepe/util/Private.h | 89 ---------------------- src/crepe/util/Private.hpp | 31 -------- src/test/AudioTest.cpp | 8 +- src/test/CMakeLists.txt | 1 - src/test/PrivateTest.cpp | 155 -------------------------------------- 14 files changed, 78 insertions(+), 381 deletions(-) create mode 100644 src/crepe/facade/SoundHandle.h delete mode 100644 src/crepe/util/Private.cpp delete mode 100644 src/crepe/util/Private.h delete mode 100644 src/crepe/util/Private.hpp delete mode 100644 src/test/PrivateTest.cpp (limited to 'src') diff --git a/src/crepe/api/AudioSource.h b/src/crepe/api/AudioSource.h index 330e8e1..7c1f161 100644 --- a/src/crepe/api/AudioSource.h +++ b/src/crepe/api/AudioSource.h @@ -2,7 +2,7 @@ #include "../Component.h" #include "../types.h" -#include "../util/Private.h" +#include "../facade/SoundHandle.h" #include "Asset.h" #include "GameObject.h" @@ -59,10 +59,17 @@ private: //! Stop this sample bool oneshot_stop = false; //! \} + /** + * \name State diffing variables + * \{ + */ + typeof(active) last_active = false; + typeof(volume) last_volume = volume; + typeof(loop) last_loop = loop; + //! \} + //! This source's voice handle + SoundHandle voice{}; -private: - //! AudioSystem::ComponentPrivate - Private private_data; }; } // namespace crepe diff --git a/src/crepe/facade/Sound.h b/src/crepe/facade/Sound.h index a78a2a7..85d141b 100644 --- a/src/crepe/facade/Sound.h +++ b/src/crepe/facade/Sound.h @@ -20,12 +20,6 @@ public: Sound(const Asset & src); ~Sound(); // dbg_trace - //! Voice handle wrapper - struct Handle { - //! Voice handle (soloud), used by SoundContext - SoLoud::handle handle; - }; - private: //! Deserialized resource (soloud) SoLoud::Wav sample; diff --git a/src/crepe/facade/SoundContext.cpp b/src/crepe/facade/SoundContext.cpp index 8bd7e74..d18afc6 100644 --- a/src/crepe/facade/SoundContext.cpp +++ b/src/crepe/facade/SoundContext.cpp @@ -15,18 +15,23 @@ SoundContext::~SoundContext() { this->engine.deinit(); } -Sound::Handle SoundContext::play(Sound & resource) { - return { - .handle = this->engine.play(resource.sample, 1.0f), - }; +SoundHandle SoundContext::play(Sound & resource) { + SoLoud::handle real_handle = this->engine.play(resource.sample, 1.0f); + SoundHandle handle = this->next_handle; + this->registry[handle] = real_handle; + this->next_handle++; + return handle; } -void SoundContext::stop(Sound::Handle & handle) { this->engine.stop(handle.handle); } +void SoundContext::stop(const SoundHandle & handle) { + this->engine.stop(this->registry[handle]); +} -void SoundContext::set_volume(Sound::Handle & handle, float volume) { - this->engine.setVolume(handle.handle, volume); +void SoundContext::set_volume(const SoundHandle & handle, float volume) { + this->engine.setVolume(this->registry[handle], volume); } -void SoundContext::set_loop(Sound::Handle & handle, bool loop) { - this->engine.setLooping(handle.handle, loop); +void SoundContext::set_loop(const SoundHandle & handle, bool loop) { + this->engine.setLooping(this->registry[handle], loop); } + diff --git a/src/crepe/facade/SoundContext.h b/src/crepe/facade/SoundContext.h index 3bc8be5..102f928 100644 --- a/src/crepe/facade/SoundContext.h +++ b/src/crepe/facade/SoundContext.h @@ -4,6 +4,7 @@ #include "../api/Config.h" +#include "SoundHandle.h" #include "Sound.h" namespace crepe { @@ -33,7 +34,7 @@ public: * * \returns Handle to control this voice */ - virtual Sound::Handle play(Sound & resource); + virtual SoundHandle play(Sound & resource); /** * \brief Stop a voice immediately if it is still playing * @@ -42,7 +43,7 @@ public: * * \param handle Voice handle returned by SoundContext::play */ - virtual void stop(Sound::Handle & handle); + virtual void stop(const SoundHandle & handle); /** * \brief Change the volume of a voice * @@ -52,7 +53,7 @@ public: * \param handle Voice handle returned by SoundContext::play * \param volume New gain value (0=silent, 1=default) */ - virtual void set_volume(Sound::Handle & handle, float volume); + virtual void set_volume(const SoundHandle & handle, float volume); /** * \brief Set the looping behavior of a voice * @@ -62,7 +63,7 @@ public: * \param handle Voice handle returned by SoundContext::play * \param loop Looping behavior (false=oneshot, true=loop) */ - virtual void set_loop(Sound::Handle & handle, bool loop); + virtual void set_loop(const SoundHandle & handle, bool loop); private: //! Abstracted class @@ -70,6 +71,12 @@ private: //! Config reference Config & config = Config::get_instance(); + + //! Sound handle registry + std::unordered_map registry; + //! Unique handle counter + SoundHandle next_handle = 0; + }; } // namespace crepe diff --git a/src/crepe/facade/SoundHandle.h b/src/crepe/facade/SoundHandle.h new file mode 100644 index 0000000..131d28c --- /dev/null +++ b/src/crepe/facade/SoundHandle.h @@ -0,0 +1,13 @@ +#pragma once + +#include + +namespace crepe { + +/** + * \brief Voice handle returned by + */ +typedef size_t SoundHandle; + +} + diff --git a/src/crepe/system/AudioSystem.cpp b/src/crepe/system/AudioSystem.cpp index 26913c0..c1cde8b 100644 --- a/src/crepe/system/AudioSystem.cpp +++ b/src/crepe/system/AudioSystem.cpp @@ -16,56 +16,50 @@ void AudioSystem::update() { for (AudioSource & component : components) { Sound & resource = resource_manager.get(component.source); - if (component.private_data.empty()) { - auto & data = component.private_data.set(); - this->update_last(component, data); - data.last_active = false; - } - auto & data = component.private_data.get(); - - this->diff_update(component, data, resource); + this->diff_update(component, resource); - this->update_last(component, data); + this->update_last(component); } } -void AudioSystem::diff_update(AudioSource & component, ComponentPrivate & data, - Sound & resource) { +void AudioSystem::diff_update(AudioSource & component, Sound & resource) { SoundContext & context = this->get_context(); - if (component.active != data.last_active) { + if (component.active != component.last_active) { if (component.active) { component.oneshot_play = component.play_on_awake; } else { - context.stop(data.handle); + context.stop(component.voice); return; } } if (!component.active) return; if (component.oneshot_play) { - data.handle = context.play(resource); + component.voice = context.play(resource); component.oneshot_play = false; } if (component.oneshot_stop) { - context.stop(data.handle); + context.stop(component.voice); component.oneshot_stop = false; } - if (component.volume != data.last_volume) { - context.set_volume(data.handle, component.volume); + if (component.volume != component.last_volume) { + context.set_volume(component.voice, component.volume); } - if (component.loop != data.last_loop) { - context.set_loop(data.handle, component.loop); + if (component.loop != component.last_loop) { + context.set_loop(component.voice, component.loop); } } -void AudioSystem::update_last(const AudioSource & component, ComponentPrivate & data) { - data.last_active = component.active; - data.last_loop = component.loop; - data.last_volume = component.volume; +void AudioSystem::update_last(AudioSource & component) { + component.last_active = component.active; + component.last_loop = component.loop; + component.last_volume = component.volume; } SoundContext & AudioSystem::get_context() { - if (this->context.empty()) this->context.set(); - return this->context.get(); + if (this->context == nullptr) + this->context = make_unique(); + return *this->context.get(); } + diff --git a/src/crepe/system/AudioSystem.h b/src/crepe/system/AudioSystem.h index 4d21883..2ddc443 100644 --- a/src/crepe/system/AudioSystem.h +++ b/src/crepe/system/AudioSystem.h @@ -14,30 +14,14 @@ public: void update() override; private: - //! Private data stored by AudioSystem on AudioSource component - struct ComponentPrivate { - //! This sample's voice handle - Sound::Handle handle; - - /** - * \name State diffing variables - * \{ - */ - typeof(AudioSource::active) last_active; - typeof(AudioSource::volume) last_volume; - typeof(AudioSource::loop) last_loop; - //! \} - }; - /** - * \brief Update `last_*` members of \c data + * \brief Update `last_*` members of \c component * * Copies all component properties stored for comparison between AudioSystem::update() calls * - * \param component Source properties - * \param data Destination properties + * \param component AudioSource component to update */ - void update_last(const AudioSource & component, ComponentPrivate & data); + void update_last(AudioSource & component); /** * \brief Compare update component @@ -46,10 +30,9 @@ private: * applicable. * * \param component AudioSource component to update - * \param data AudioSource's private data * \param resource Sound instance for AudioSource's Asset */ - void diff_update(AudioSource & component, ComponentPrivate & data, Sound & resource); + void diff_update(AudioSource & component, Sound & resource); protected: /** @@ -61,8 +44,8 @@ protected: virtual SoundContext & get_context(); private: - //! Actually stores SoundContext if the base AudioSystem::get_context implementation is used - Private context; + //! SoundContext + std::unique_ptr context = nullptr; }; } // namespace crepe diff --git a/src/crepe/util/CMakeLists.txt b/src/crepe/util/CMakeLists.txt index f49d851..94ed906 100644 --- a/src/crepe/util/CMakeLists.txt +++ b/src/crepe/util/CMakeLists.txt @@ -1,7 +1,6 @@ target_sources(crepe PUBLIC LogColor.cpp Log.cpp - Private.cpp ) target_sources(crepe PUBLIC FILE_SET HEADERS FILES @@ -12,7 +11,5 @@ target_sources(crepe PUBLIC FILE_SET HEADERS FILES Proxy.hpp OptionalRef.h OptionalRef.hpp - Private.h - Private.hpp ) diff --git a/src/crepe/util/Private.cpp b/src/crepe/util/Private.cpp deleted file mode 100644 index 262620d..0000000 --- a/src/crepe/util/Private.cpp +++ /dev/null @@ -1,27 +0,0 @@ -#include "Private.h" - -using namespace crepe; - -bool Private::empty() const noexcept { return this->instance == nullptr; } - -Private::~Private() { - if (this->instance == nullptr) return; - this->destructor(this->instance); -} - -Private::Private(Private && other) { *this = std::move(other); } - -Private & Private::operator=(Private && other) { - // TODO: ideally this function checks for self-assignment - this->instance = other.instance; - this->destructor = other.destructor; - this->type = other.type; - - other.instance = nullptr; - other.destructor = [](void *) {}; - - return *this; -} - -Private::Private(const Private & other) {} -Private & Private::operator=(const Private & other) { return *this; } diff --git a/src/crepe/util/Private.h b/src/crepe/util/Private.h deleted file mode 100644 index d725a5e..0000000 --- a/src/crepe/util/Private.h +++ /dev/null @@ -1,89 +0,0 @@ -#pragma once - -#include -#include - -namespace crepe { - -/** - * \brief Utility for storing type hidden from user - * - * This class can be used to store types which cannot be used in the API directly due to header - * distribution limitations. This class is similar to `std::any`, but provides a method for - * retrieving a mutable reference to the stored object. - */ -class Private { -public: - Private() = default; - ~Private(); - /** - * \name Copy - * - * \note These functions do not do anything, resulting in `*this` being an empty (default) - * instance. - * - * \{ - */ - Private(const Private &); - Private & operator=(const Private &); - //! \} - /** - * \name Move - * - * These functions actually move the stored type if present. - * - * \{ - */ - Private(Private &&); - Private & operator=(Private &&); - //! \} - - /** - * \brief Get the stored object - * - * \tparam T Type of stored object - * - * \returns Mutable reference to stored object - * - * \throws std::out_of_range if this instance does not contain any object - * \throws std::logic_error if the stored type and requested type differ - */ - template - T & get() const; - - /** - * \brief Create and store an arbitrary object - * - * \tparam T Type of object - * \tparam Args Perfect forwarding arguments - * \param args Perfect forwarding arguments - * - * All arguments to this function are forwarded using `std::forward` to the constructor of T. - * - * \returns Mutable reference to stored object - * - * \note If this instance already contained an object, this function implicitly destroys the - * previous object. - */ - template - T & set(Args &&... args); - - /** - * \brief Check if this instance contains an object - * - * \returns `true` if this instance is empty, `false` if it contains an object - */ - bool empty() const noexcept; - -private: - //! Wrapper for destructor call of stored object type - std::function destructor; - //! Stored object's type - std::type_index type = typeid(void); - //! Stored object - void * instance = nullptr; -}; - -} // namespace crepe - -#include "Private.hpp" diff --git a/src/crepe/util/Private.hpp b/src/crepe/util/Private.hpp deleted file mode 100644 index b2174c0..0000000 --- a/src/crepe/util/Private.hpp +++ /dev/null @@ -1,31 +0,0 @@ -#pragma once - -#include -#include - -#include "Private.h" - -namespace crepe { - -template -T & Private::set(Args &&... args) { - if (!this->empty()) this->destructor(this->instance); - T * instance = new T(std::forward(args)...); - this->instance = static_cast(instance); - this->destructor = [](void * instance) { delete static_cast(instance); }; - this->type = typeid(T); - return *instance; -} - -template -T & Private::get() const { - using namespace std; - if (this->empty()) throw out_of_range("Private: get() called on empty object"); - type_index requested_type = typeid(T); - if (this->type != requested_type) - throw logic_error(format("Private: get() called with [T = {}] (actual is [T = {}])", - requested_type.name(), this->type.name())); - return *static_cast(this->instance); -} - -} // namespace crepe diff --git a/src/test/AudioTest.cpp b/src/test/AudioTest.cpp index 7644ab7..774fdb8 100644 --- a/src/test/AudioTest.cpp +++ b/src/test/AudioTest.cpp @@ -16,10 +16,10 @@ class AudioTest : public Test { private: class TestSoundContext : public SoundContext { public: - MOCK_METHOD(Sound::Handle, play, (Sound & resource), (override)); - MOCK_METHOD(void, stop, (Sound::Handle &), (override)); - MOCK_METHOD(void, set_volume, (Sound::Handle &, float), (override)); - MOCK_METHOD(void, set_loop, (Sound::Handle &, bool), (override)); + MOCK_METHOD(SoundHandle, play, (Sound & resource), (override)); + MOCK_METHOD(void, stop, (const SoundHandle &), (override)); + MOCK_METHOD(void, set_volume, (const SoundHandle &, float), (override)); + MOCK_METHOD(void, set_loop, (const SoundHandle &, bool), (override)); }; class TestAudioSystem : public AudioSystem { diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 8c4b855..4174926 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -16,5 +16,4 @@ target_sources(test_main PUBLIC Vector2Test.cpp ScriptEventTest.cpp ScriptSceneTest.cpp - PrivateTest.cpp ) diff --git a/src/test/PrivateTest.cpp b/src/test/PrivateTest.cpp deleted file mode 100644 index 454789e..0000000 --- a/src/test/PrivateTest.cpp +++ /dev/null @@ -1,155 +0,0 @@ -#include - -#include - -using namespace std; -using namespace crepe; -using namespace testing; - -class PrivateTest : public Test { -public: - static unsigned constructors; - static unsigned destructors; - - void SetUp() override { - PrivateTest::constructors = 0; - PrivateTest::destructors = 0; - } - - class TestClass { - public: - TestClass() { PrivateTest::constructors++; } - ~TestClass() { PrivateTest::destructors++; } - }; - class Unrelated {}; -}; -unsigned PrivateTest::constructors; -unsigned PrivateTest::destructors; - -TEST_F(PrivateTest, Empty) { - { Private foo; } - - EXPECT_EQ(PrivateTest::constructors, 0); - EXPECT_EQ(PrivateTest::destructors, 0); -} - -TEST_F(PrivateTest, WithObject) { - { - Private foo; - foo.set(); - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 0); - } - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 1); -} - -TEST_F(PrivateTest, EmptyException) { - Private foo; - EXPECT_THROW(foo.get(), std::out_of_range); - - foo.set(); - EXPECT_NO_THROW(foo.get()); -} - -TEST_F(PrivateTest, IncorrectTypeException) { - Private foo; - foo.set(); - - EXPECT_THROW(foo.get(), std::logic_error); - EXPECT_NO_THROW(foo.get()); -} - -TEST_F(PrivateTest, MoveConstructor) { - { - Private foo; - foo.set(); - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 0); - - Private bar(std::move(foo)); - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 0); - } - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 1); -} - -TEST_F(PrivateTest, MoveOperator) { - { - Private foo; - foo.set(); - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 0); - - Private bar = std::move(foo); - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 0); - } - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 1); -} - -TEST_F(PrivateTest, CopyConstructor) { - { - Private foo; - foo.set(); - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 0); - - Private bar(foo); - - EXPECT_TRUE(bar.empty()); - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 0); - } - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 1); -} - -TEST_F(PrivateTest, CopyOperator) { - { - Private foo; - foo.set(); - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 0); - - Private bar = foo; - - EXPECT_TRUE(bar.empty()); - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 0); - } - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 1); -} - -TEST_F(PrivateTest, DoubleAssignment) { - { - Private foo; - foo.set(); - - EXPECT_EQ(PrivateTest::constructors, 1); - EXPECT_EQ(PrivateTest::destructors, 0); - - foo.set(); - - EXPECT_EQ(PrivateTest::constructors, 2); - EXPECT_EQ(PrivateTest::destructors, 1); - } - - EXPECT_EQ(PrivateTest::constructors, 2); - EXPECT_EQ(PrivateTest::destructors, 2); -} -- cgit v1.2.3