From 7c21e34ae1898fce8c8051b5f1380e268da32140 Mon Sep 17 00:00:00 2001 From: max-001 Date: Fri, 22 Nov 2024 09:36:35 +0100 Subject: Replaced ComponentManager reference by OptionalRef for late binding --- src/crepe/api/CMakeLists.txt | 1 - src/crepe/api/Scene.cpp | 5 ----- src/crepe/api/Scene.h | 22 ++++++++++++++++------ src/crepe/api/SceneManager.h | 4 ++-- src/crepe/api/SceneManager.hpp | 9 ++++++--- 5 files changed, 24 insertions(+), 17 deletions(-) delete mode 100644 src/crepe/api/Scene.cpp (limited to 'src/crepe/api') diff --git a/src/crepe/api/CMakeLists.txt b/src/crepe/api/CMakeLists.txt index d6b6801..4025a63 100644 --- a/src/crepe/api/CMakeLists.txt +++ b/src/crepe/api/CMakeLists.txt @@ -12,7 +12,6 @@ target_sources(crepe PUBLIC SaveManager.cpp Config.cpp Metadata.cpp - Scene.cpp SceneManager.cpp Vector2.cpp Camera.cpp diff --git a/src/crepe/api/Scene.cpp b/src/crepe/api/Scene.cpp deleted file mode 100644 index 849945e..0000000 --- a/src/crepe/api/Scene.cpp +++ /dev/null @@ -1,5 +0,0 @@ -#include "Scene.h" - -using namespace crepe; - -Scene::Scene(ComponentManager & mgr) : component_manager(mgr) {} diff --git a/src/crepe/api/Scene.h b/src/crepe/api/Scene.h index 869bf6f..22aadab 100644 --- a/src/crepe/api/Scene.h +++ b/src/crepe/api/Scene.h @@ -1,5 +1,6 @@ #pragma once +#include "util/OptionalRef.h" #include namespace crepe { @@ -15,11 +16,8 @@ class ComponentManager; */ class Scene { protected: - //TODO: Use Loek's custom reference class to set ComponentManger via SceneManager instead of via constructor - /** - * \param mgr Reference to the ComponentManager - */ - Scene(ComponentManager & mgr); + // NOTE: This must be the only constructor on Scene, see "Late references" below + Scene() = default; //! SceneManager instances Scene friend class SceneManager; @@ -36,8 +34,20 @@ public: virtual std::string get_name() const = 0; protected: + /** + * \name Late references + * + * These references are set by SceneManager immediately after calling the constructor of Scene. + * + * \note Scene must have a constructor without arguments so the game programmer doesn't need to + * manually add `using Scene::Scene` to their concrete scene class, if they want to add a + * constructor with arguments (e.g. for passing references to their own concrete Scene classes). + * + * \{ + */ //! Reference to the ComponentManager - ComponentManager & component_manager; + OptionalRef component_manager; + //! \} }; } // namespace crepe diff --git a/src/crepe/api/SceneManager.h b/src/crepe/api/SceneManager.h index 45ba668..f6f62cd 100644 --- a/src/crepe/api/SceneManager.h +++ b/src/crepe/api/SceneManager.h @@ -26,8 +26,8 @@ public: * * \tparam T Type of concrete scene */ - template - void add_scene(); + template + void add_scene(Args &&... args); /** * \brief Set the next scene * diff --git a/src/crepe/api/SceneManager.hpp b/src/crepe/api/SceneManager.hpp index 94e5946..1edaa7d 100644 --- a/src/crepe/api/SceneManager.hpp +++ b/src/crepe/api/SceneManager.hpp @@ -4,12 +4,15 @@ namespace crepe { -template -void SceneManager::add_scene() { +template +void SceneManager::add_scene(Args &&... args) { using namespace std; static_assert(is_base_of::value, "T must be derived from Scene"); - Scene * scene = new T(this->component_manager); + Scene * scene = new T(std::forward(args)...); + + scene->component_manager = this->component_manager; + this->scenes.emplace_back(unique_ptr(scene)); // The first scene added, is the one that will be loaded at the beginning -- cgit v1.2.3 From ef6acbee3bd00c322a56221d992d08a64e15e3b9 Mon Sep 17 00:00:00 2001 From: max-001 Date: Fri, 22 Nov 2024 09:46:10 +0100 Subject: Little fix (typo) --- src/crepe/api/SceneManager.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/crepe/api') diff --git a/src/crepe/api/SceneManager.hpp b/src/crepe/api/SceneManager.hpp index 1edaa7d..ccc755f 100644 --- a/src/crepe/api/SceneManager.hpp +++ b/src/crepe/api/SceneManager.hpp @@ -9,7 +9,7 @@ void SceneManager::add_scene(Args &&... args) { using namespace std; static_assert(is_base_of::value, "T must be derived from Scene"); - Scene * scene = new T(std::forward(args)...); + Scene * scene = new T(std::forward(args)...); scene->component_manager = this->component_manager; -- cgit v1.2.3 From a1a95627c8c2a77530feeee8507b5409616bc54a Mon Sep 17 00:00:00 2001 From: WBoerenkamps Date: Fri, 22 Nov 2024 10:59:19 +0100 Subject: fixed loop fix --- src/crepe/api/LoopManager.cpp | 2 +- src/crepe/api/LoopTimer.cpp | 2 +- src/crepe/api/LoopTimer.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src/crepe/api') diff --git a/src/crepe/api/LoopManager.cpp b/src/crepe/api/LoopManager.cpp index a64366f..7edf4d1 100644 --- a/src/crepe/api/LoopManager.cpp +++ b/src/crepe/api/LoopManager.cpp @@ -57,7 +57,7 @@ void LoopManager::loop() { void LoopManager::setup() { this->game_running = true; LoopTimer::get_instance().start(); - LoopTimer::get_instance().set_fps(60); + LoopTimer::get_instance().set_fps(200); } void LoopManager::render() { diff --git a/src/crepe/api/LoopTimer.cpp b/src/crepe/api/LoopTimer.cpp index a9800b7..15a0e3a 100644 --- a/src/crepe/api/LoopTimer.cpp +++ b/src/crepe/api/LoopTimer.cpp @@ -47,7 +47,7 @@ double LoopTimer::get_fixed_delta_time() const { return this->fixed_delta_time.c void LoopTimer::set_fps(int fps) { this->fps = fps; // target time per frame in seconds - this->frame_target_time = std::chrono::seconds(1) / fps; + this->frame_target_time = std::chrono::duration(1.0) / fps; } int LoopTimer::get_fps() const { return this->fps; } diff --git a/src/crepe/api/LoopTimer.h b/src/crepe/api/LoopTimer.h index f277d7b..9393439 100644 --- a/src/crepe/api/LoopTimer.h +++ b/src/crepe/api/LoopTimer.h @@ -130,9 +130,9 @@ private: //! Delta time for the current frame in seconds std::chrono::duration delta_time{0.0}; //! Target time per frame in seconds - std::chrono::duration frame_target_time = std::chrono::seconds(1) / fps; + std::chrono::duration frame_target_time = std::chrono::duration(1.0) / fps; //! Fixed delta time for fixed updates in seconds - std::chrono::duration fixed_delta_time = std::chrono::seconds(1) / 50; + std::chrono::duration fixed_delta_time = std::chrono::duration(1.0) / 50.0; //! Total elapsed game time in seconds std::chrono::duration elapsed_time{0.0}; //! Total elapsed time for fixed updates in seconds -- cgit v1.2.3 From 1499363d85abedbdb571e33801b821f4dfabc638 Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Fri, 22 Nov 2024 17:28:28 +0100 Subject: more documentation --- contributing.md | 24 ++++++++++++++++++++++++ src/crepe/api/Script.h | 7 +++++-- src/doc/internal/component.dox | 41 +++++++++++++++++++++++++++++++++++++++++ src/doc/internal/resource.dox | 12 ++++++++++++ src/doc/internal/style.dox | 9 +++++++++ src/doc/internal/system.dox | 26 ++++++++++++++++++++++++++ src/doc/internals.dox | 10 ++++++++++ src/doc/layout.xml | 16 ++++++++-------- src/doc/style.css | 2 ++ 9 files changed, 137 insertions(+), 10 deletions(-) create mode 100644 src/doc/internal/component.dox create mode 100644 src/doc/internal/resource.dox create mode 100644 src/doc/internal/style.dox create mode 100644 src/doc/internal/system.dox create mode 100644 src/doc/internals.dox (limited to 'src/crepe/api') diff --git a/contributing.md b/contributing.md index 77a2908..0a90e86 100644 --- a/contributing.md +++ b/contributing.md @@ -17,6 +17,7 @@ that you can click on to open them. working/compiling version of the project - Pull requests for new code include either automated tests for the new code or an explanation as to why the code can not (reliably) be tested + @@ -495,6 +496,12 @@ that you can click on to open them. -
Ensure const-correctness + + > [!IMPORTANT] + > C-style APIs that work on (possibly internal) references to structs can be + > called from const member functions in C++. If the compiler allows you to + > mark a function as `const` even though it has side effects, it should + > **not** be marked as `const`.
GoodBad
```cpp @@ -795,6 +802,23 @@ that you can click on to open them. ```
- Do not implement new classes as singletons +-
+ Retrieving the first or last indices for iterators with a known or expected + size should be done using .front() or .back() + instead of by index +
GoodBad
+ + ```cpp + vector foo = { 1, 2, 3 }; + int bar = foo.first(); + ``` + + + ```cpp + vector foo = { 1, 2, 3 }; + int bar = foo[0]; + ``` +
## CMakeLists-specific diff --git a/src/crepe/api/Script.h b/src/crepe/api/Script.h index f0b9c73..a0870cb 100644 --- a/src/crepe/api/Script.h +++ b/src/crepe/api/Script.h @@ -22,6 +22,10 @@ class ComponentManager; * \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(). * + * \warning Concrete scripts are allowed do create a custom constructor, but the utility + * functions should not be called inside the constructor as they rely on late references that + * are only available after the constructor returns. + * * \see feature_script */ class Script { @@ -63,8 +67,7 @@ protected: * * \returns Reference to component * - * \throws std::runtime_error if this game object does not have a component matching type \c - * T + * \throws std::runtime_error if this game object does not have a component with type \c T */ template T & get_component() const; diff --git a/src/doc/internal/component.dox b/src/doc/internal/component.dox new file mode 100644 index 0000000..0dd4cb5 --- /dev/null +++ b/src/doc/internal/component.dox @@ -0,0 +1,41 @@ +// vim:ft=doxygen +namespace crepe { +/** + +\defgroup internal_component Components +\ingroup internal +\brief ECS Components + +Components are attached to GameObject instances and are composed by the game +programmer to create specific entities in the game world. While they are +implemented as C++ classes, components should be treated as C-style structs, +meaning all members are public and they do not contain functions. + +A basic component has the following structure: +```cpp +#include + +class MyComponent : public crepe::Component { +public: + // Add your custom component's ininitializer properties after the `id` + // parameter. The first parameter is controlled by GameObject::add_component, + // while all other parameters are forwarded using std::forward. + MyComponent(game_object_id_t id, ...); + + // Optionally define the `get_instances_max` method to limit the amount of + // instances of this component per GameObject. The default implementation for + // this function returns -1, which means the instance count does not have an + // upper limit: + virtual int get_instances_max() const { return -1; } + + // Properties + // ... +}; +``` + +Generally, components are "handled" by \ref internal_system "systems", which may +optionally change the components' state. Components' state may also be +controlled by the game programmer through \ref feature_script "scripts". + +*/ +} diff --git a/src/doc/internal/resource.dox b/src/doc/internal/resource.dox new file mode 100644 index 0000000..56f1de0 --- /dev/null +++ b/src/doc/internal/resource.dox @@ -0,0 +1,12 @@ +// vim:ft=doxygen +namespace crepe { +/** + +\defgroup internal_resource Resources +\ingroup internal +\brief Concrete resources + +\todo This section is incomplete + +*/ +} diff --git a/src/doc/internal/style.dox b/src/doc/internal/style.dox new file mode 100644 index 0000000..dad2df0 --- /dev/null +++ b/src/doc/internal/style.dox @@ -0,0 +1,9 @@ +// vim:ft=doxygen +/** + +\defgroup internal_style Code style +\ingroup internal +\brief Coding conventions +\include{doc} contributing.md + +*/ diff --git a/src/doc/internal/system.dox b/src/doc/internal/system.dox new file mode 100644 index 0000000..17a101e --- /dev/null +++ b/src/doc/internal/system.dox @@ -0,0 +1,26 @@ +// vim:ft=doxygen +namespace crepe { +/** + +\defgroup internal_system Systems +\ingroup internal +\brief ECS Systems + +\todo This section is incomplete + +A system is responsible for processing the data stored in \ref +internal_component "components". + +A basic system has the following structure: +```cpp +#include + +class MySystem : public System { +public: + using System::System; + void update() override; +}; +``` + +*/ +} diff --git a/src/doc/internals.dox b/src/doc/internals.dox new file mode 100644 index 0000000..2d2ca56 --- /dev/null +++ b/src/doc/internals.dox @@ -0,0 +1,10 @@ +// vim:ft=doxygen +/** + +\defgroup internal Internals +\brief Internal engine structure and other conventions + +\todo This page is incomplete +\todo Anything about Contexts? + +*/ diff --git a/src/doc/layout.xml b/src/doc/layout.xml index 6bde443..fb4cc0c 100644 --- a/src/doc/layout.xml +++ b/src/doc/layout.xml @@ -1,13 +1,13 @@ - + - + @@ -15,9 +15,9 @@ - + - + @@ -28,20 +28,20 @@ - + - + - + - + diff --git a/src/doc/style.css b/src/doc/style.css index 08bc9f5..daabd39 100644 --- a/src/doc/style.css +++ b/src/doc/style.css @@ -2,3 +2,5 @@ address { display: none; } + +h2.groupheader { margin-top: revert; } -- cgit v1.2.3 From 004ee3aafb6beb4e984877186bced560010f4ddb Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Fri, 22 Nov 2024 21:57:08 +0100 Subject: fix RenderSystem unit test path resolution + reset Config before each test --- src/crepe/api/Config.h | 13 ++++++------- src/crepe/api/Texture.cpp | 13 ++++--------- src/crepe/api/Texture.h | 12 +++--------- src/test/AssetTest.cpp | 14 +++++--------- src/test/RenderSystemTest.cpp | 8 ++++---- src/test/main.cpp | 24 ++++++++++++++++++++---- 6 files changed, 42 insertions(+), 42 deletions(-) (limited to 'src/crepe/api') diff --git a/src/crepe/api/Config.h b/src/crepe/api/Config.h index 13eabd1..0c9d116 100644 --- a/src/crepe/api/Config.h +++ b/src/crepe/api/Config.h @@ -11,19 +11,18 @@ namespace crepe { * modified *before* execution is handed over from the game programmer to the engine (i.e. the * main loop is started). */ -class Config { +class Config final { public: //! Retrieve handle to global Config instance static Config & get_instance(); private: Config() = default; - - // singleton - Config(const Config &) = delete; - Config(Config &&) = delete; - Config & operator=(const Config &) = delete; - Config & operator=(Config &&) = delete; + ~Config() = default; + Config(const Config &) = default; + Config(Config &&) = default; + Config & operator=(const Config &) = default; + Config & operator=(Config &&) = default; public: //! Logging-related settings diff --git a/src/crepe/api/Texture.cpp b/src/crepe/api/Texture.cpp index 9be9421..264d7b1 100644 --- a/src/crepe/api/Texture.cpp +++ b/src/crepe/api/Texture.cpp @@ -9,14 +9,9 @@ using namespace crepe; using namespace std; -Texture::Texture(unique_ptr res) { +Texture::Texture(const Asset & src) { dbg_trace(); - this->load(std::move(res)); -} - -Texture::Texture(const char * src) { - dbg_trace(); - this->load(make_unique(src)); + this->load(src); } Texture::~Texture() { @@ -24,9 +19,9 @@ Texture::~Texture() { this->texture.reset(); } -void Texture::load(unique_ptr res) { +void Texture::load(const Asset & res) { SDLContext & ctx = SDLContext::get_instance(); - this->texture = std::move(ctx.texture_from_path(res->get_path())); + this->texture = ctx.texture_from_path(res.get_path()); } int Texture::get_width() const { diff --git a/src/crepe/api/Texture.h b/src/crepe/api/Texture.h index 6965223..b4f7d07 100644 --- a/src/crepe/api/Texture.h +++ b/src/crepe/api/Texture.h @@ -24,17 +24,11 @@ class Animator; class Texture { public: - /** - * \brief Constructs a Texture from a file path. - * \param src Path to the image file to be loaded as a texture. - */ - Texture(const char * src); - /** * \brief Constructs a Texture from an Asset resource. - * \param res Unique pointer to an Asset resource containing texture data. + * \param src Asset with texture data to load. */ - Texture(std::unique_ptr res); + Texture(const Asset & src); /** * \brief Destroys the Texture instance, freeing associated resources. @@ -59,7 +53,7 @@ private: * \brief Loads the texture from an Asset resource. * \param res Unique pointer to an Asset resource to load the texture from. */ - void load(std::unique_ptr res); + void load(const Asset & res); private: //! The texture of the class from the library diff --git a/src/test/AssetTest.cpp b/src/test/AssetTest.cpp index 8aa7629..c3c166c 100644 --- a/src/test/AssetTest.cpp +++ b/src/test/AssetTest.cpp @@ -7,20 +7,16 @@ using namespace std; using namespace crepe; using namespace testing; -class AssetTest : public Test { -public: - Config & cfg = Config::get_instance(); - void SetUp() override { this->cfg.asset.root_pattern = ".crepe-root"; } -}; - -TEST_F(AssetTest, Existant) { ASSERT_NO_THROW(Asset{"asset/texture/img.png"}); } +TEST(AssetTest, Existant) { ASSERT_NO_THROW(Asset{"asset/texture/img.png"}); } -TEST_F(AssetTest, Nonexistant) { ASSERT_ANY_THROW(Asset{"asset/nonexistant"}); } +TEST(AssetTest, Nonexistant) { ASSERT_ANY_THROW(Asset{"asset/nonexistant"}); } -TEST_F(AssetTest, Rootless) { +TEST(AssetTest, Rootless) { + Config & cfg = Config::get_instance(); cfg.asset.root_pattern.clear(); string arbitrary = "\\/this is / /../passed through as-is"; Asset asset{arbitrary}; ASSERT_EQ(arbitrary, asset.get_path()); } + diff --git a/src/test/RenderSystemTest.cpp b/src/test/RenderSystemTest.cpp index ac479d3..f37fb56 100644 --- a/src/test/RenderSystemTest.cpp +++ b/src/test/RenderSystemTest.cpp @@ -30,7 +30,7 @@ public: void SetUp() override { auto & sprite1 - = entity1.add_component(make_shared("../asset/texture/img.png"), + = entity1.add_component(make_shared("asset/texture/img.png"), Color(0, 0, 0, 0), FlipSettings{false, false}); ASSERT_NE(sprite1.sprite_image.get(), nullptr); sprite1.order_in_layer = 5; @@ -38,7 +38,7 @@ public: EXPECT_EQ(sprite1.order_in_layer, 5); EXPECT_EQ(sprite1.sorting_in_layer, 5); auto & sprite2 - = entity2.add_component(make_shared("../asset/texture/img.png"), + = entity2.add_component(make_shared("asset/texture/img.png"), Color(0, 0, 0, 0), FlipSettings{false, false}); ASSERT_NE(sprite2.sprite_image.get(), nullptr); sprite2.sorting_in_layer = 2; @@ -48,7 +48,7 @@ public: EXPECT_EQ(sprite2.order_in_layer, 1); auto & sprite3 - = entity3.add_component(make_shared("../asset/texture/img.png"), + = entity3.add_component(make_shared("asset/texture/img.png"), Color(0, 0, 0, 0), FlipSettings{false, false}); ASSERT_NE(sprite3.sprite_image.get(), nullptr); sprite3.sorting_in_layer = 1; @@ -58,7 +58,7 @@ public: EXPECT_EQ(sprite3.order_in_layer, 2); auto & sprite4 - = entity4.add_component(make_shared("../asset/texture/img.png"), + = entity4.add_component(make_shared("asset/texture/img.png"), Color(0, 0, 0, 0), FlipSettings{false, false}); ASSERT_NE(sprite4.sprite_image.get(), nullptr); sprite4.sorting_in_layer = 1; diff --git a/src/test/main.cpp b/src/test/main.cpp index 241015d..e03a989 100644 --- a/src/test/main.cpp +++ b/src/test/main.cpp @@ -1,15 +1,31 @@ -#include - #include +#define protected public +#define private public + +#include + using namespace crepe; using namespace testing; +class GlobalConfigReset : public EmptyTestEventListener { +public: + Config & cfg = Config::get_instance(); + Config cfg_default = Config(); + + // This function is called before each test + void OnTestStart(const TestInfo &) override { + cfg = cfg_default; + cfg.log.level = Log::Level::WARNING; + } +}; + int main(int argc, char ** argv) { InitGoogleTest(&argc, argv); - auto & cfg = Config::get_instance(); - cfg.log.level = Log::Level::ERROR; + UnitTest & ut = *UnitTest::GetInstance(); + ut.listeners().Append(new GlobalConfigReset); return RUN_ALL_TESTS(); } + -- cgit v1.2.3 From 2f10649bf08bbe507458fa4da8be1dd7b02213da Mon Sep 17 00:00:00 2001 From: max-001 Date: Sun, 24 Nov 2024 11:52:48 +0100 Subject: Implemented feedback --- src/crepe/api/SceneManager.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/crepe/api') diff --git a/src/crepe/api/SceneManager.hpp b/src/crepe/api/SceneManager.hpp index ccc755f..5c8e417 100644 --- a/src/crepe/api/SceneManager.hpp +++ b/src/crepe/api/SceneManager.hpp @@ -10,10 +10,11 @@ void SceneManager::add_scene(Args &&... args) { static_assert(is_base_of::value, "T must be derived from Scene"); Scene * scene = new T(std::forward(args)...); + unique_ptr unique_scene(scene); - scene->component_manager = this->component_manager; + unique_scene->component_manager = this->component_manager; - this->scenes.emplace_back(unique_ptr(scene)); + this->scenes.emplace_back(std::move(unique_scene)); // The first scene added, is the one that will be loaded at the beginning if (next_scene.empty()) { -- cgit v1.2.3 From be5ccbe24086d5d4fb407f268c649dcbc36eda6b Mon Sep 17 00:00:00 2001 From: Loek Le Blansch Date: Sun, 24 Nov 2024 19:03:06 +0100 Subject: nitpick #50 --- src/crepe/api/Scene.h | 3 ++- src/doc/feature/scene.dox | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'src/crepe/api') diff --git a/src/crepe/api/Scene.h b/src/crepe/api/Scene.h index 22aadab..f6fdb2a 100644 --- a/src/crepe/api/Scene.h +++ b/src/crepe/api/Scene.h @@ -1,8 +1,9 @@ #pragma once -#include "util/OptionalRef.h" #include +#include "../util/OptionalRef.h" + namespace crepe { class SceneManager; diff --git a/src/doc/feature/scene.dox b/src/doc/feature/scene.dox index bac1c87..d81df4c 100644 --- a/src/doc/feature/scene.dox +++ b/src/doc/feature/scene.dox @@ -36,7 +36,7 @@ concrete scene to be added. #include #include #include -#include +#include using namespace crepe; -- cgit v1.2.3