From 11b4030ca3a4aafed2881f2853c7d1f3dbb5bd86 Mon Sep 17 00:00:00 2001 From: Ricardo Antunes Date: Sun, 19 Nov 2023 21:23:55 +0000 Subject: [PATCH] refactor(ecs): remove pack/unpack methods --- .../cubos/core/ecs/component/manager.hpp | 16 ----- .../cubos/core/ecs/component/registry.hpp | 1 + .../cubos/core/ecs/component/storage.hpp | 43 -------------- .../cubos/core/ecs/resource/manager.hpp | 2 + .../cubos/core/ecs/system/commands.hpp | 1 + core/include/cubos/core/ecs/system/system.hpp | 1 + core/include/cubos/core/ecs/world.hpp | 17 +----- core/src/cubos/core/ecs/component/manager.cpp | 12 +--- core/src/cubos/core/ecs/world.cpp | 59 ------------------- core/tests/ecs/blueprint.cpp | 13 +--- core/tests/ecs/registry.cpp | 21 ------- core/tests/ecs/world.cpp | 54 ----------------- engine/src/cubos/engine/scene/bridge.cpp | 1 + 13 files changed, 9 insertions(+), 232 deletions(-) diff --git a/core/include/cubos/core/ecs/component/manager.hpp b/core/include/cubos/core/ecs/component/manager.hpp index d1567524e..a58fc0aa5 100644 --- a/core/include/cubos/core/ecs/component/manager.hpp +++ b/core/include/cubos/core/ecs/component/manager.hpp @@ -158,22 +158,6 @@ namespace cubos::core::ecs /// @copydoc storage(std::size_t) const IStorage* storage(std::size_t id) const; - /// @brief Creates a package from a component of an entity. - /// @param id Entity index. - /// @param componentId Component identifier. - /// @param context Optional context to use for serialization. - /// @return Package containing the component. - data::old::Package pack(uint32_t id, std::size_t componentId, data::old::Context* context) const; - - /// @brief Inserts a component into an entity, by unpacking a package. - /// @param id Entity index. - /// @param componentId Component identifier. - /// @param package Package to unpack. - /// @param context Optional context to use for deserialization. - /// @return Whether the unpacking was successful. - bool unpack(uint32_t id, std::size_t componentId, const data::old::Package& package, - data::old::Context* context); - private: struct Entry { diff --git a/core/include/cubos/core/ecs/component/registry.hpp b/core/include/cubos/core/ecs/component/registry.hpp index 7454ebb8e..9e0effb90 100644 --- a/core/include/cubos/core/ecs/component/registry.hpp +++ b/core/include/cubos/core/ecs/component/registry.hpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include diff --git a/core/include/cubos/core/ecs/component/storage.hpp b/core/include/cubos/core/ecs/component/storage.hpp index bee2d9474..a243b09fc 100644 --- a/core/include/cubos/core/ecs/component/storage.hpp +++ b/core/include/cubos/core/ecs/component/storage.hpp @@ -4,8 +4,6 @@ #pragma once -#include -#include #include namespace cubos::core::ecs @@ -39,23 +37,6 @@ namespace cubos::core::ecs /// @param index Index of the value to be retrieved. /// @return Pointer to the value. virtual const void* get(uint32_t index) const = 0; - - /// @brief Packages a value. If the value doesn't exist, undefined behavior will occur. - /// @param index Index of the value to package. - /// @param context Optional context used for serialization. - /// @return Packaged value. - virtual data::old::Package pack(uint32_t index, data::old::Context* context) const = 0; - - /// @brief Unpackages a value. - /// @param index Index of the value to unpackage. - /// @param package Package to unpackage. - /// @param context Optional context used for deserialization. - /// @return Whether the unpackaging was successful. - virtual bool unpack(uint32_t index, const data::old::Package& package, data::old::Context* context) = 0; - - /// @brief Gets the type the components being stored here. - /// @return Component type. - virtual std::type_index type() const = 0; }; /// @brief Abstract container for a component type @p T. @@ -67,29 +48,5 @@ namespace cubos::core::ecs public: /// @brief Component type. using Type = T; - - // Implementation. - - inline data::old::Package pack(uint32_t index, data::old::Context* context) const override - { - return data::old::Package::from(*static_cast(this->get(index)), context); - } - - inline bool unpack(uint32_t index, const data::old::Package& package, data::old::Context* context) override - { - T value; - if (package.into(value, context)) - { - this->insert(index, &value); - return true; - } - - return false; - } - - inline std::type_index type() const override - { - return std::type_index(typeid(T)); - } }; } // namespace cubos::core::ecs diff --git a/core/include/cubos/core/ecs/resource/manager.hpp b/core/include/cubos/core/ecs/resource/manager.hpp index 8aeabefc2..d9cf2eb35 100644 --- a/core/include/cubos/core/ecs/resource/manager.hpp +++ b/core/include/cubos/core/ecs/resource/manager.hpp @@ -4,12 +4,14 @@ #pragma once +#include #include #include #include #include #include +#include namespace cubos::core::ecs { diff --git a/core/include/cubos/core/ecs/system/commands.hpp b/core/include/cubos/core/ecs/system/commands.hpp index bc78421aa..fc00616cf 100644 --- a/core/include/cubos/core/ecs/system/commands.hpp +++ b/core/include/cubos/core/ecs/system/commands.hpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include diff --git a/core/include/cubos/core/ecs/system/system.hpp b/core/include/cubos/core/ecs/system/system.hpp index 5def10624..5e73bf289 100644 --- a/core/include/cubos/core/ecs/system/system.hpp +++ b/core/include/cubos/core/ecs/system/system.hpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include diff --git a/core/include/cubos/core/ecs/world.hpp b/core/include/cubos/core/ecs/world.hpp index cdcbbaf38..8a130d199 100644 --- a/core/include/cubos/core/ecs/world.hpp +++ b/core/include/cubos/core/ecs/world.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -96,22 +97,6 @@ namespace cubos::core::ecs /// @copydoc components(Entity) ConstComponents components(Entity entity) const; - /// @brief Creates a package from the components of an entity. - /// @param entity Entity identifier. - /// @param context Optional context for serializing the components. - /// @return Package containing the components of the entity. - data::old::Package pack(Entity entity, data::old::Context* context = nullptr) const; - - /// @brief Unpacks components specified in a package into an entity. - /// - /// Removes any components that are already present in the entity. - /// - /// @param entity Entity identifier. - /// @param package Package to unpack. - /// @param context Optional context for deserializing the components. - /// @return Whether the package was unpacked successfully. - bool unpack(Entity entity, const data::old::Package& package, data::old::Context* context = nullptr); - /// @brief Returns an iterator which points to the first entity of the world. /// @return Iterator. Iterator begin() const; diff --git a/core/src/cubos/core/ecs/component/manager.cpp b/core/src/cubos/core/ecs/component/manager.cpp index 8d7ebaec1..ab93f9ca6 100644 --- a/core/src/cubos/core/ecs/component/manager.cpp +++ b/core/src/cubos/core/ecs/component/manager.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -74,14 +75,3 @@ ComponentManager::Entry::Entry(std::unique_ptr storage) { this->mutex = std::make_unique(); } - -data::old::Package ComponentManager::pack(uint32_t id, std::size_t componentId, data::old::Context* context) const -{ - return mEntries[componentId - 1].storage->pack(id, context); -} - -bool ComponentManager::unpack(uint32_t id, std::size_t componentId, const data::old::Package& package, - data::old::Context* context) -{ - return mEntries[componentId - 1].storage->unpack(id, package, context); -} diff --git a/core/src/cubos/core/ecs/world.cpp b/core/src/cubos/core/ecs/world.cpp index c63cfae3a..9b78a0ecd 100644 --- a/core/src/cubos/core/ecs/world.cpp +++ b/core/src/cubos/core/ecs/world.cpp @@ -45,65 +45,6 @@ World::ConstComponents World::components(Entity entity) const return ConstComponents{*this, entity}; } -data::old::Package World::pack(Entity entity, data::old::Context* context) const -{ - CUBOS_ASSERT(this->isAlive(entity), "Entity is not alive"); - - Entity::Mask mask = mEntityManager.getMask(entity); - - auto pkg = data::old::Package(data::old::Package::Type::Object); - for (std::size_t i = 1; i < mask.size(); ++i) - { - if (mask.test(i)) - { - auto name = mComponentManager.getType(i).name(); - pkg.fields().push_back({name, mComponentManager.pack(entity.index, i, context)}); - } - } - - return pkg; -} - -bool World::unpack(Entity entity, const data::old::Package& package, data::old::Context* context) -{ - if (package.type() != data::old::Package::Type::Object) - { - CUBOS_ERROR("Entities must be packaged as objects"); - return false; - } - - // Remove all existing components. - mComponentManager.removeAll(entity.index); - - Entity::Mask mask{1}; - bool success = true; - - for (const auto& field : package.fields()) - { - const auto* type = Registry::type(field.first); - if (type == nullptr) - { - CUBOS_ERROR("Unknown component type '{}'", field.first); - success = false; - continue; - } - - auto id = mComponentManager.getID(*type); - if (mComponentManager.unpack(entity.index, id, field.second, context)) - { - mask.set(id); - } - else - { - CUBOS_ERROR("Could not unpack component '{}'", field.first); - success = false; - } - } - - mEntityManager.setMask(entity, mask); - return success; -} - World::Iterator World::begin() const { return mEntityManager.begin(); diff --git a/core/tests/ecs/blueprint.cpp b/core/tests/ecs/blueprint.cpp index 90a2244fd..ceb2424e3 100644 --- a/core/tests/ecs/blueprint.cpp +++ b/core/tests/ecs/blueprint.cpp @@ -6,8 +6,6 @@ #include "utils.hpp" -using cubos::core::data::old::Package; -using cubos::core::data::old::Unpackager; using cubos::core::ecs::Blueprint; using cubos::core::ecs::CommandBuffer; using cubos::core::ecs::Commands; @@ -51,16 +49,7 @@ TEST_CASE("ecs::Blueprint") auto bar = blueprint.create("bar"); auto baz = blueprint.create("baz"); blueprint.add(baz, IntegerComponent{2}); - - // Add a ParentComponent to "baz" with id = "bar", using a deserializer. - { - // Pack the identifier of "bar". - auto barPkg = Package::from(bar); - - // Unpack the identifier of "bar" into a ParentComponent. - Unpackager unpackager{barPkg}; - Registry::create("ParentComponent", unpackager, blueprint, baz); - } + blueprint.add(baz, ParentComponent{bar}); SUBCASE("spawn the blueprint") { diff --git a/core/tests/ecs/registry.cpp b/core/tests/ecs/registry.cpp index b51ed82ec..63f12df4b 100644 --- a/core/tests/ecs/registry.cpp +++ b/core/tests/ecs/registry.cpp @@ -1,6 +1,5 @@ #include -#include #include #include #include @@ -8,9 +7,6 @@ #include "utils.hpp" -using cubos::core::data::old::Package; -using cubos::core::data::old::Unpackager; -using cubos::core::ecs::Blueprint; using cubos::core::ecs::CommandBuffer; using cubos::core::ecs::Commands; using cubos::core::ecs::Registry; @@ -23,8 +19,6 @@ TEST_CASE("ecs::Registry") World world{}; CommandBuffer cmdBuffer{world}; Commands cmds{cmdBuffer}; - Blueprint blueprint{}; - auto entity = blueprint.create("entity"); // Initially type int isn't registered. CHECK(Registry::type("int") == nullptr); @@ -37,19 +31,4 @@ TEST_CASE("ecs::Registry") // Create a storage for it and check if its of the correct type. auto storage = Registry::createStorage(reflect()); CHECK(dynamic_cast*>(storage.get()) != nullptr); - - // Instantiate the component into a blueprint, from a package. - auto pkg = Package::from(42); - Unpackager unpackager{pkg}; - REQUIRE(Registry::create("int", unpackager, blueprint, entity)); - - // Spawn the blueprint into the world. - world.registerComponent(); - entity = cmds.spawn(blueprint).entity("entity"); - cmdBuffer.commit(); - - // Package the entity and check if the component was correctly instantiated. - pkg = world.pack(entity); - REQUIRE(pkg.fields().size() == 1); - CHECK(pkg.field("int").get() == 42); } diff --git a/core/tests/ecs/world.cpp b/core/tests/ecs/world.cpp index cff902c7d..e55811256 100644 --- a/core/tests/ecs/world.cpp +++ b/core/tests/ecs/world.cpp @@ -5,7 +5,6 @@ #include "utils.hpp" -using cubos::core::data::old::Package; using cubos::core::ecs::Entity; using cubos::core::ecs::World; @@ -82,59 +81,6 @@ TEST_CASE("ecs::World") CHECK(destroyed); } - SUBCASE("add and remove components through pack and unpack") - { - // Create an entity. - auto foo = world.create(); - - // Add an integer component. - auto pkg = world.pack(foo); - pkg.fields().emplace_back("IntegerComponent", Package::from(1)); - CHECK(world.unpack(foo, pkg)); - CHECK(world.components(foo).has()); - CHECK_FALSE(world.components(foo).has()); - - // Check if the component was added. - pkg = world.pack(foo); - CHECK(pkg.fields().size() == 1); - CHECK(pkg.field("IntegerComponent").get() == 1); - - // Remove the integer component and add a parent component. - pkg.removeField("IntegerComponent"); - pkg.fields().emplace_back("ParentComponent", Package::from(Entity{})); - CHECK(world.unpack(foo, pkg)); - CHECK_FALSE(world.components(foo).has()); - CHECK(world.components(foo).has()); - - // Check if the component was added. - pkg = world.pack(foo); - CHECK(pkg.fields().size() == 1); - CHECK(pkg.field("ParentComponent").get().isNull()); - } - - SUBCASE("change a component through pack and unpack") - { - // Create an entity which has an integer component and a parent component. - auto bar = world.create(); - auto foo = world.create(); - world.components(foo).add(IntegerComponent{0}).add(ParentComponent{bar}); - CHECK(world.components(foo).has()); - CHECK(world.components(foo).has()); - - // Change the value of the integer component. - auto pkg = world.pack(foo); - pkg.field("IntegerComponent").set(1); - CHECK(world.unpack(foo, pkg)); - CHECK(world.components(foo).has()); - CHECK(world.components(foo).has()); - - // Check if the value was changed. - pkg = world.pack(foo); - CHECK(pkg.fields().size() == 2); - CHECK(pkg.field("IntegerComponent").get() == 1); - CHECK(pkg.field("ParentComponent").get() == bar); - } - SUBCASE("components are correctly destructed when their entity is destroyed") { bool destroyed = false; diff --git a/engine/src/cubos/engine/scene/bridge.cpp b/engine/src/cubos/engine/scene/bridge.cpp index f7c500cf1..27a422496 100644 --- a/engine/src/cubos/engine/scene/bridge.cpp +++ b/engine/src/cubos/engine/scene/bridge.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include