From f19f37ae3eff84161f86e62a26fbd8b68f8f91a9 Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Sat, 7 Dec 2024 15:29:33 +0100 Subject: make SaveManager no longer a singleton --- src/crepe/manager/Mediator.h | 4 ++-- src/crepe/manager/SaveManager.cpp | 32 ++++++++++++++------------------ src/crepe/manager/SaveManager.h | 27 +++++++++++---------------- 3 files changed, 27 insertions(+), 36 deletions(-) (limited to 'src/crepe') diff --git a/src/crepe/manager/Mediator.h b/src/crepe/manager/Mediator.h index 8094d80..6507a74 100644 --- a/src/crepe/manager/Mediator.h +++ b/src/crepe/manager/Mediator.h @@ -5,13 +5,13 @@ // TODO: remove these singletons: #include "../facade/SDLContext.h" #include "EventManager.h" -#include "SaveManager.h" #include "api/LoopTimer.h" namespace crepe { class ComponentManager; class SceneManager; +class SaveManager; /** * Struct to pass references to classes that would otherwise need to be singletons down to @@ -28,7 +28,7 @@ class SceneManager; struct Mediator { OptionalRef component_manager; OptionalRef scene_manager; - OptionalRef save_manager = SaveManager::get_instance(); + OptionalRef save_manager; OptionalRef event_manager = EventManager::get_instance(); OptionalRef sdl_context = SDLContext::get_instance(); OptionalRef timer = LoopTimer::get_instance(); diff --git a/src/crepe/manager/SaveManager.cpp b/src/crepe/manager/SaveManager.cpp index d4ed1c1..292e8fd 100644 --- a/src/crepe/manager/SaveManager.cpp +++ b/src/crepe/manager/SaveManager.cpp @@ -1,13 +1,24 @@ #include "../ValueBroker.h" #include "../api/Config.h" #include "../facade/DB.h" -#include "../util/Log.h" #include "SaveManager.h" using namespace std; using namespace crepe; +SaveManager::SaveManager(Mediator & mediator) : Manager(mediator) { + mediator.save_manager = *this; +} + +DB & SaveManager::get_db() { + if (this->db == nullptr) { + Config & cfg = Config::get_instance(); + this->db = make_unique(cfg.savemgr.location); + } + return *this->db; +} + template <> string SaveManager::serialize(const string & value) const noexcept { return value; @@ -90,22 +101,6 @@ int32_t SaveManager::deserialize(const string & value) const noexcept { return deserialize(value); } -SaveManager::SaveManager() { dbg_trace(); } - -SaveManager & SaveManager::get_instance() { - dbg_trace(); - static SaveManager instance; - return instance; -} - -DB & SaveManager::get_db() { - Config & cfg = Config::get_instance(); - // TODO: make this path relative to XDG_DATA_HOME on Linux and whatever the - // default equivalent is on Windows using some third party library - static DB db(cfg.savemgr.location); - return db; -} - bool SaveManager::has(const string & key) { DB & db = this->get_db(); return db.has(key); @@ -155,7 +150,8 @@ ValueBroker SaveManager::get(const string & key) { return { [this, key](const T & target) { this->set(key, target); }, [this, key, value]() mutable -> const T & { - value = this->deserialize(this->get_db().get(key)); + DB & db = this->get_db(); + value = this->deserialize(db.get(key)); return value; }, }; diff --git a/src/crepe/manager/SaveManager.h b/src/crepe/manager/SaveManager.h index 3d8c852..d13a97a 100644 --- a/src/crepe/manager/SaveManager.h +++ b/src/crepe/manager/SaveManager.h @@ -4,6 +4,8 @@ #include "../ValueBroker.h" +#include "Manager.h" + namespace crepe { class DB; @@ -18,7 +20,7 @@ class DB; * * The underlying database is a key-value store. */ -class SaveManager { +class SaveManager : public Manager { public: /** * \brief Get a read/write reference to a value and initialize it if it does not yet exist @@ -63,8 +65,8 @@ public: */ bool has(const std::string & key); -private: - SaveManager(); +public: + SaveManager(Mediator & mediator); virtual ~SaveManager() = default; private: @@ -90,25 +92,18 @@ private: T deserialize(const std::string & value) const noexcept; public: - // singleton - static SaveManager & get_instance(); SaveManager(const SaveManager &) = delete; SaveManager(SaveManager &&) = delete; SaveManager & operator=(const SaveManager &) = delete; SaveManager & operator=(SaveManager &&) = delete; +protected: + //! Create or return DB + virtual DB & get_db(); + private: - /** - * \brief Create an instance of DB and return its reference - * - * \returns DB instance - * - * This function exists because DB is a facade class, which can't directly be used in the API - * without workarounds - * - * TODO: better solution - */ - static DB & get_db(); + //! Database + std::unique_ptr db = nullptr; }; } // namespace crepe -- cgit v1.2.3 From 2e6fcb1d048edd13a2ec69ddd226fc8ebabc2389 Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Sat, 7 Dec 2024 16:01:19 +0100 Subject: add SaveManager to Script --- src/crepe/api/Script.cpp | 11 +++++++---- src/crepe/api/Script.h | 4 ++++ src/crepe/api/Script.hpp | 6 ++---- src/crepe/util/OptionalRef.h | 10 +++++++++- src/crepe/util/OptionalRef.hpp | 7 +++++++ src/test/CMakeLists.txt | 1 + src/test/ScriptSaveManagerTest.cpp | 36 ++++++++++++++++++++++++++++++++++++ 7 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 src/test/ScriptSaveManagerTest.cpp (limited to 'src/crepe') diff --git a/src/crepe/api/Script.cpp b/src/crepe/api/Script.cpp index 4091fd4..961e6e7 100644 --- a/src/crepe/api/Script.cpp +++ b/src/crepe/api/Script.cpp @@ -8,8 +8,7 @@ using namespace crepe; using namespace std; Script::~Script() { - Mediator & mediator = this->mediator; - EventManager & mgr = mediator.event_manager; + EventManager & mgr = this->mediator->event_manager; for (auto id : this->listeners) { mgr.unsubscribe(id); } @@ -21,7 +20,11 @@ void Script::subscribe(const EventHandler & callback) { } void Script::set_next_scene(const string & name) { - Mediator & mediator = this->mediator; - SceneManager & mgr = mediator.scene_manager; + SceneManager & mgr = this->mediator->scene_manager; mgr.set_next_scene(name); } + +SaveManager & Script::get_save_manager() const { + return this->mediator->save_manager; +} + diff --git a/src/crepe/api/Script.h b/src/crepe/api/Script.h index d99ab0e..024f1d7 100644 --- a/src/crepe/api/Script.h +++ b/src/crepe/api/Script.h @@ -7,6 +7,7 @@ #include "../system/CollisionSystem.h" #include "../types.h" #include "../util/OptionalRef.h" +#include "../ValueBroker.h" namespace crepe { @@ -113,6 +114,9 @@ protected: */ void set_next_scene(const std::string & name); + //! Retrieve SaveManager reference + SaveManager & get_save_manager() const; + //! \} private: diff --git a/src/crepe/api/Script.hpp b/src/crepe/api/Script.hpp index 45f1ff1..23d69d9 100644 --- a/src/crepe/api/Script.hpp +++ b/src/crepe/api/Script.hpp @@ -20,8 +20,7 @@ T & Script::get_component() const { template RefVector Script::get_components() const { - Mediator & mediator = this->mediator; - ComponentManager & mgr = mediator.component_manager; + ComponentManager & mgr = this->mediator->component_manager; return mgr.get_components_by_id(this->game_object_id); } @@ -34,8 +33,7 @@ void Script::logf(Args &&... args) { template void Script::subscribe_internal(const EventHandler & callback, event_channel_t channel) { - Mediator & mediator = this->mediator; - EventManager & mgr = mediator.event_manager; + EventManager & mgr = this->mediator->event_manager; subscription_t listener = mgr.subscribe( [this, callback](const EventType & data) -> bool { bool & active = this->active; diff --git a/src/crepe/util/OptionalRef.h b/src/crepe/util/OptionalRef.h index 3201667..1b2cb3f 100644 --- a/src/crepe/util/OptionalRef.h +++ b/src/crepe/util/OptionalRef.h @@ -25,13 +25,21 @@ public: */ OptionalRef & operator=(T & ref); /** - * \brief Retrieve this reference + * \brief Retrieve this reference (cast) * * \returns Internal reference if it is set * * \throws std::runtime_error if this function is called while the reference it not set */ operator T &() const; + /** + * \brief Retrieve this reference (member access) + * + * \returns Internal reference if it is set + * + * \throws std::runtime_error if this function is called while the reference it not set + */ + T * operator->() const; /** * \brief Check if this reference is not empty * diff --git a/src/crepe/util/OptionalRef.hpp b/src/crepe/util/OptionalRef.hpp index 4608c9e..5e36b3a 100644 --- a/src/crepe/util/OptionalRef.hpp +++ b/src/crepe/util/OptionalRef.hpp @@ -18,6 +18,13 @@ OptionalRef::operator T &() const { return *this->ref; } +template +T * OptionalRef::operator->() const { + if (this->ref == nullptr) + throw std::runtime_error("OptionalRef: attempt to dereference nullptr"); + return this->ref; +} + template OptionalRef & OptionalRef::operator=(T & ref) { this->ref = &ref; diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 734e3ee..2cb7c7a 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -18,4 +18,5 @@ target_sources(test_main PUBLIC ScriptSceneTest.cpp Profiling.cpp SaveManagerTest.cpp + ScriptSaveManagerTest.cpp ) diff --git a/src/test/ScriptSaveManagerTest.cpp b/src/test/ScriptSaveManagerTest.cpp new file mode 100644 index 0000000..098afa0 --- /dev/null +++ b/src/test/ScriptSaveManagerTest.cpp @@ -0,0 +1,36 @@ +#include + +// stupid hack to allow access to private/protected members under test +#define private public +#define protected public + +#include +#include + +#include "ScriptTest.h" + +using namespace std; +using namespace crepe; +using namespace testing; + +class ScriptSaveManagerTest : public ScriptTest { +public: + class TestSaveManager : public SaveManager { + using SaveManager::SaveManager; + + // in-memory database for testing + DB db{}; + virtual DB & get_db() override { return this->db; } + }; + + TestSaveManager save_mgr{mediator}; +}; + +TEST_F(ScriptSaveManagerTest, GetSaveManager) { + MyScript & script = this->script; + + SaveManager & mgr = script.get_save_manager(); + + EXPECT_EQ(&mgr, &save_mgr); +} + -- cgit v1.2.3 From dd8bbe2fde97786ab29490bc7ba9962deb08fb4c Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Sat, 7 Dec 2024 16:01:59 +0100 Subject: `make format` --- src/crepe/api/Script.cpp | 5 +---- src/crepe/api/Script.h | 2 +- src/test/SaveManagerTest.cpp | 5 ++--- src/test/ScriptSaveManagerTest.cpp | 3 +-- 4 files changed, 5 insertions(+), 10 deletions(-) (limited to 'src/crepe') diff --git a/src/crepe/api/Script.cpp b/src/crepe/api/Script.cpp index 961e6e7..753a9e3 100644 --- a/src/crepe/api/Script.cpp +++ b/src/crepe/api/Script.cpp @@ -24,7 +24,4 @@ void Script::set_next_scene(const string & name) { mgr.set_next_scene(name); } -SaveManager & Script::get_save_manager() const { - return this->mediator->save_manager; -} - +SaveManager & Script::get_save_manager() const { return this->mediator->save_manager; } diff --git a/src/crepe/api/Script.h b/src/crepe/api/Script.h index 024f1d7..0d59ab6 100644 --- a/src/crepe/api/Script.h +++ b/src/crepe/api/Script.h @@ -2,12 +2,12 @@ #include +#include "../ValueBroker.h" #include "../manager/EventManager.h" #include "../manager/Mediator.h" #include "../system/CollisionSystem.h" #include "../types.h" #include "../util/OptionalRef.h" -#include "../ValueBroker.h" namespace crepe { diff --git a/src/test/SaveManagerTest.cpp b/src/test/SaveManagerTest.cpp index a1efc33..e9b0c29 100644 --- a/src/test/SaveManagerTest.cpp +++ b/src/test/SaveManagerTest.cpp @@ -1,8 +1,8 @@ #include -#include -#include #include +#include +#include using namespace std; using namespace crepe; @@ -38,4 +38,3 @@ TEST_F(SaveManagerTest, DefaultValue) { value.set(5); ASSERT_EQ(value.get(), 5); } - diff --git a/src/test/ScriptSaveManagerTest.cpp b/src/test/ScriptSaveManagerTest.cpp index 098afa0..64403c4 100644 --- a/src/test/ScriptSaveManagerTest.cpp +++ b/src/test/ScriptSaveManagerTest.cpp @@ -4,8 +4,8 @@ #define private public #define protected public -#include #include +#include #include "ScriptTest.h" @@ -33,4 +33,3 @@ TEST_F(ScriptSaveManagerTest, GetSaveManager) { EXPECT_EQ(&mgr, &save_mgr); } - -- cgit v1.2.3 From b5bf36aec3caa879d77457e3b2d1d29b6b625f27 Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Sat, 7 Dec 2024 16:03:47 +0100 Subject: minor cleanup --- src/crepe/api/Script.h | 1 - src/crepe/manager/SaveManager.h | 7 ------- 2 files changed, 8 deletions(-) (limited to 'src/crepe') diff --git a/src/crepe/api/Script.h b/src/crepe/api/Script.h index 0d59ab6..3417bb4 100644 --- a/src/crepe/api/Script.h +++ b/src/crepe/api/Script.h @@ -2,7 +2,6 @@ #include -#include "../ValueBroker.h" #include "../manager/EventManager.h" #include "../manager/Mediator.h" #include "../system/CollisionSystem.h" diff --git a/src/crepe/manager/SaveManager.h b/src/crepe/manager/SaveManager.h index d13a97a..1b55a22 100644 --- a/src/crepe/manager/SaveManager.h +++ b/src/crepe/manager/SaveManager.h @@ -91,16 +91,9 @@ private: template T deserialize(const std::string & value) const noexcept; -public: - SaveManager(const SaveManager &) = delete; - SaveManager(SaveManager &&) = delete; - SaveManager & operator=(const SaveManager &) = delete; - SaveManager & operator=(SaveManager &&) = delete; - protected: //! Create or return DB virtual DB & get_db(); - private: //! Database std::unique_ptr db = nullptr; -- cgit v1.2.3 From 8ed0c3f425b02d4f064c455270288a6c4a359109 Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Tue, 10 Dec 2024 20:10:58 +0100 Subject: use opaque type instead of forward declared DB for unique_ptr in SaveManager --- src/crepe/manager/SaveManager.cpp | 7 +++++-- src/crepe/manager/SaveManager.h | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'src/crepe') diff --git a/src/crepe/manager/SaveManager.cpp b/src/crepe/manager/SaveManager.cpp index 292e8fd..39b92d4 100644 --- a/src/crepe/manager/SaveManager.cpp +++ b/src/crepe/manager/SaveManager.cpp @@ -14,9 +14,12 @@ SaveManager::SaveManager(Mediator & mediator) : Manager(mediator) { DB & SaveManager::get_db() { if (this->db == nullptr) { Config & cfg = Config::get_instance(); - this->db = make_unique(cfg.savemgr.location); + this->db = { + new DB(cfg.savemgr.location), + [](void * db){ delete static_cast(db); } + }; } - return *this->db; + return *static_cast(this->db.get()); } template <> diff --git a/src/crepe/manager/SaveManager.h b/src/crepe/manager/SaveManager.h index 1b55a22..27e625c 100644 --- a/src/crepe/manager/SaveManager.h +++ b/src/crepe/manager/SaveManager.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "../ValueBroker.h" @@ -96,7 +97,7 @@ protected: virtual DB & get_db(); private: //! Database - std::unique_ptr db = nullptr; + std::unique_ptr> db = nullptr; }; } // namespace crepe -- cgit v1.2.3 From c45b60941b82dec2097d958b396a117b1634eada Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Tue, 10 Dec 2024 20:26:18 +0100 Subject: add SaveManager to LoopManager --- src/crepe/api/LoopManager.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/crepe') diff --git a/src/crepe/api/LoopManager.h b/src/crepe/api/LoopManager.h index 0947f94..33a61a3 100644 --- a/src/crepe/api/LoopManager.h +++ b/src/crepe/api/LoopManager.h @@ -6,6 +6,7 @@ #include "../manager/ComponentManager.h" #include "../manager/ResourceManager.h" #include "../manager/SceneManager.h" +#include "../manager/SaveManager.h" #include "../system/System.h" #include "LoopTimer.h" @@ -98,6 +99,8 @@ private: SceneManager scene_manager{mediator}; //! Resource manager instance ResourceManager resource_manager{mediator}; + //! Save manager instance + SaveManager save_manager{mediator}; //! SDL context \todo no more singletons! SDLContext & sdl_context = SDLContext::get_instance(); -- cgit v1.2.3