Component polymorphism support

[zheka2304]

Adds extension, that allows to declare and work with polymorphic components. See discussion in the delated issue #859.

Example usage (will be updated in case of changes):

#include <entt/entity/polymorphic.hpp>

struct ticking : public entt::inherit<> { // declare as polymorphic component without parents
    virtual void tick(const entt::entity) = 0;
};

class physics : public entt::inherit<ticking> { // inherit from ticking
    // ...
    void tick(const entt::entity e) override {
         // ...
    }
};

class ai : public entt::inherit<ticking> { // inherit from ticking
    // ...
    void tick(const entt::entity e) override {
         // ...
    }
};

// iterate all components, derived from ticking for given entity
for (ticking& ticking : entt::algorithm::poly_get_all<ticking>(reg, e)) {
    ticking.tick(e);
}

// try get any component, derived from ticking for given entity
if (ticking* ticking = entt::algorithm::poly_get_any<ticking>(reg, e); ticking != nullptr) {
    ticking->tick(e);
}

// iterate all components, derived from ticking in the registry
entt::algorithm::poly_each<ticking>(reg, [] (entt::entity e, ticking& c) {
    ticking.tick(e);
});

// count components, derived from ticking 
entt::algorithm::poly_count<ticking>(reg, e); // attached to entity
entt::algorithm::poly_count<ticking>(reg); // in the registry

// remove all components, derived from ticking, attached to entity
entt::algorithm::poly_remove<ticking>(reg, e); 

[codecov-commenter]

Codecov Report

Merging #871 (6255e8b) into wip (83009ba) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##               wip      #871    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          129       134     +5     
  Lines        19539     20019   +480     
==========================================
+ Hits         19539     20019   +480     

Impacted Files	Coverage Δ
src/entt/entity/poly_storage_mixin.hpp	100.00% <100.00%> (ø)
src/entt/entity/polymorphic.hpp	100.00% <100.00%> (ø)
test/entt/common/polymorphic_type.hpp	100.00% <100.00%> (ø)
test/entt/entity/poly_type_traits.cpp	100.00% <100.00%> (ø)
test/entt/entity/polymorphic.cpp	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 83009ba...6255e8b. Read the comment docs.

[skypjack]

Updated the base branch to wip. It fails to compile on all platforms though (yeah, the toolset v141 is always a pleasure to work with in 2022 😅).

[zheka2304]

Updated the base branch to wip. It fails to compile on all platforms though (yeah, the toolset v141 is always a pleasure to work with in 2022 😅).

Thank you, I've reproduced this one on my machine, and I will commit the fix tomorrow

[zheka2304]

It seems, the problem goes much deeper, than just a bug with parameter pack expansion in static assert. It looks like there are some problems with if constexpr in this toolset.

Here, when Type is a pointer, it fails to compile the else branch, that should not be compiled at all in this case.

if constexpr(std::is_pointer_v<Type>) {
    // Type and ChildType are pointers, no need for getting address
    ChildType ptr = static_cast<StorageType*>(pool)->get(entity);
    return static_cast<Type>(ptr);
} else {
    // Type is a base of ChildType, do pointer conversion
    return static_cast<Type*>(std::addressof(static_cast<StorageType*>(pool)->get(entity)));
}

However, this code compiles successfully, both for T = int and T = int*. In either case the unevaluated branch is invalid, but if constexpr works as expected here, and everything is fine.

template<typename T>
void test_if_constexpr(T a, T b) {
    if constexpr(std::is_pointer_v<T>)
        *a = *b;
    else
        a = a + b;
}

Maybe you've encountered something like this and got some ideas about the solution to this problem? The only solution I see right now is just not using if constexpr here and moving conversion logic into some helper template function with enable_if.

[zheka2304]

First pass to leave some comments and questions before proceeding. I need to review the whole thing again for sure, mainly because there are some parts that aren't that clear to me yet.

I really appreciate the effort to make it an external tool. However, if I get this as the bare minimum, I see lot of code here and I wonder whether it's necessary and if we can reduce and simplify it. Unfortunately I'm not (yet) into all the details, so I can't say where and how to intervene. My gut feeling is that we can shrink some stuff probably but I might be utterly wrong.

In any case, get ready for a second pass soon, according with my schedule. 👍

Thank you for reviewing, I greatly appreciate your effort. I've replied to the comments above, and uploaded quick fixes for some of them. However there are still some things to be done and to discuss, so I am working on it. Looking forward for the second pass :)

[zheka2304]

Some of the builds have failed again, I will try reproduce it and commit the fix, as it will be ready, sorry.

[zheka2304]

Hello, I'm very sorry for disappearing, I unfortunately had a very little free time. However now I am ready to continue the discussion and further improvement.

Everything seems to build properly, but codecov now does something strange, both in polymorhic.cpp and even view.cpp that was not affected at all by my changes, do you have any idea what is happening?

[skypjack]

The one in view.cpp is due to an error of mine that is already fixed on wip, so don't worry. As for the other one, it's in a new file, so very likely to be fixed. I'll try to find some time for another review during the next couple of days. 👍

[PaulJohnson1]

Hey, I was testing some stuff out on the experimental branch of the fork, and I tested for memory leaks using valgrind. I noticed a memory leak when you emplace a base component and a derived component on the same entity. It leaked 81,920 bytes in 1 block according to valgrind. Here is my code if you would like to see the memory leak in action. I have put a comment above the faulty lines. I am thinking that an assertion would be the best way to prevent this. I do not know how the codebase works, this is just a suggestion.

#include <cstring>
#include <iostream>

#include "entt/src/entt/entity/registry.hpp"
#include "entt/src/entt/entity/polymorphic.hpp"

class CameraComponent : public entt::inherit<>
{
};

class SendableCameraComponent : public entt::inherit<CameraComponent>
{
};

int main()
{
    std::cout << "format" << std::endl << "testHasCameraComponent test2HasCameraComponent" << std::endl << "testHasCameraEntity    test2HasCameraEntity" << std::endl << std::endl;

    entt::registry registry;
    entt::entity test = registry.create();
    entt::entity test2 = registry.create();

    registry.emplace<CameraComponent>(test);
    registry.emplace<SendableCameraComponent>(test2);

    std::cout << "test has CameraComponent; test2 has SendableCameraComponent" << std::endl;
    std::cout << std::to_string(registry.all_of<CameraComponent>(test)) << " " << std::to_string(registry.all_of<CameraComponent>(test2)) << std::endl;
    std::cout << std::to_string(registry.all_of<SendableCameraComponent>(test)) << " " << std::to_string(registry.all_of<SendableCameraComponent>(test2)) << std::endl;

    std::cout << std::endl;
    std::cout << "deleting CameraComponent from test2" << std::endl;

    registry.erase<CameraComponent>(test2);

    std::cout << std::to_string(registry.all_of<CameraComponent>(test)) << " " << std::to_string(registry.all_of<CameraComponent>(test2)) << std::endl;
    std::cout << std::to_string(registry.all_of<SendableCameraComponent>(test)) << " " << std::to_string(registry.all_of<SendableCameraComponent>(test2)) << std::endl;

    std::cout << std::endl;
    std::cout << "adding SendableCameraComponent; removing SendableCameraComponent" << std::endl;

    registry.emplace<SendableCameraComponent>(test2);
    registry.erase<SendableCameraComponent>(test2);

    std::cout << std::to_string(registry.all_of<CameraComponent>(test)) << " " << std::to_string(registry.all_of<CameraComponent>(test2)) << std::endl;
    std::cout << std::to_string(registry.all_of<SendableCameraComponent>(test)) << " " << std::to_string(registry.all_of<SendableCameraComponent>(test2)) << std::endl;

    std::cout << std::endl;
    std::cout << "adding CameraComponent; adding SendableCameraComponent" << std::endl;
  
    // MEMORY LEAK!
    registry.emplace<CameraComponent>(test2);
    registry.emplace<SendableCameraComponent>(test2);

    std::cout << std::to_string(registry.all_of<CameraComponent>(test)) << " " << std::to_string(registry.all_of<CameraComponent>(test2)) << std::endl;
    std::cout << std::to_string(registry.all_of<SendableCameraComponent>(test)) << " " << std::to_string(registry.all_of<SendableCameraComponent>(test2)) << std::endl;

    std::cout << std::endl;
    std::cout << "removing SendableCameraComponent" << std::endl;
    
    registry.erase<SendableCameraComponent>(test2);

    std::cout << std::to_string(registry.all_of<CameraComponent>(test)) << " " << std::to_string(registry.all_of<CameraComponent>(test2)) << std::endl;
    std::cout << std::to_string(registry.all_of<SendableCameraComponent>(test)) << " " << std::to_string(registry.all_of<SendableCameraComponent>(test2)) << std::endl;

    std::cout << std::endl;
    std::cout << "remove all components from all tests" << std::endl;

    registry.erase<CameraComponent>(test);
    registry.erase<CameraComponent>(test2);

    std::cout << std::to_string(registry.all_of<CameraComponent>(test)) << " " << std::to_string(registry.all_of<CameraComponent>(test2)) << std::endl;
    std::cout << std::to_string(registry.all_of<SendableCameraComponent>(test)) << " " << std::to_string(registry.all_of<SendableCameraComponent>(test2)) << std::endl;

    registry.destroy(test);
    registry.destroy(test2);
}

[PaulJohnson1]

I have also noticed, while testing stuff, that it has no support for calling the base component class with parameters. Am I missing something? Is there not a way to do that currently?

[zheka2304]

Hello, and sorry for a late reply! I'm really glad that you've tried and tested my extension to entt.

Hey, I was testing some stuff out on the experimental branch of the fork, and I tested for memory leaks using valgrind. I noticed a memory leak when you emplace a base component and a derived component on the same entity. It leaked 81,920 bytes in 1 block according to valgrind.

I see that you are using my first prototype, which was considered too complicated and introduced too many changes. Currently discussed implementation is located in the polymorpic branch and I highly recommend to try it instead.

The old implementation, that you've used, internally allocates blocks of memory to store tightly packed reference lists, those are stored statically, and it seems, that I've forgot to free them, when program terminates. This is the most probable source of this leak.

I have also noticed, while testing stuff, that it has no support for calling the base component class with parameters. Am I missing something? Is there not a way to do that currently?

Sorry, I'm not getting the idea here, can you please provide an example?

[PaulJohnson1]

I see that you are using my first prototype, which was considered too complicated and introduced too many changes. Currently discussed implementation is located in the polymorphic branch and I highly recommend to try it instead.

The reason that I was on that branch instead of the polymorphic one is because I was having a few issues compiling it. I have included the source code, along with the errors using clang and gcc in the screenshot below. Am I missing something? I don't really understand these errors.

#include <cstring>
#include <iostream>

#include "entt/src/entt/entity/registry.hpp"
#include "entt/src/entt/entity/polymorphic.hpp"

class CameraComponent : public entt::inherit<>
{
};

class SendableCameraComponent : public entt::inherit<CameraComponent>
{
};

int main()
{
    entt::registry registry;
    entt::entity test = registry.create();

    registry.emplace<CameraComponent>(test);
}

Sorry, I'm not getting the idea here, can you please provide an example? The reason I ask this is because I want to be able to do something such as this with but with entt::inherit. Is this supported in any way?

class CameraComponent
{
public:
    CameraComponent(int32_t anything) {}
};

class SendableCameraComponent : public CameraComponent
{
public:
    SendableCameraComponent() : CameraComponent(int32_t(0)) {}
};

[zheka2304]

The reason that I was on that branch instead of the polymorphic one is because I was having a few issues compiling it

Thanks for providing code to reproduce the issue. The reason was in the optimization for empty component types, due to which storages don't contain empty components and return void from get. I've fixed and uploaded the fix, but for empty types it will return nullptr, because instances of empty types arent stored anywhere. So for testing purposes I suggest to add some data to components.

The reason I ask this is because I want to be able to do something such as this with but with entt::inherit. Is this supported in any way?

entt::inherit adds another class between base and derived classes, to add information about parents, so when you write A : entt::inherit<B>, B is not a direct parent of A and you cannot call its constructor. However there are other ways of declaring list or parent types, and entt::inherit is just most minimalistic way to do it, so you can just do it like this for example:

struct CameraComponent {
    CameraComponent(int32_t anything) {}
};

struct SendableCameraComponent : public CameraComponent {
    using direct_parent_types = type_list<CameraComponent>;

    SendableCameraComponent() : CameraComponent(int32_t(0)) {}
};

[PaulJohnson1]

I see now, thank you for your help!