templated sigh mixin with registry type #1079
Conversation
| using underlying_type = Type; | ||
| template<typename Storage, typename Registry> | ||
| class sigh_mixin final: public Storage { | ||
| using underlying_type = Storage; | ||
| using basic_registry_type = basic_registry<typename underlying_type::entity_type, typename underlying_type::base_type::allocator_type>; |
There was a problem hiding this comment.
I think you want to update this member type and use it all over as it happened before already.
All other changes seem unnecessary once you updated this one. Am I wrong?
There was a problem hiding this comment.
It's needed in one place, in bind. The issue being that the any value passed there is of type basic_registry, not of the inherited one. And actually this part is the one that I'm not so certain about the approach, cause of the reinterpret_cast needed there
There was a problem hiding this comment.
Regarding my comment above, added a static_assert that the template parameter Registry is indeed inheriting from basic_registry
| template<typename Entity = entity, typename = std::allocator<Entity>> | ||
| class basic_registry; | ||
|
|
||
| template<typename Storage, typename Registry = basic_registry<typename Storage::entity_type, typename Storage::base_type::allocator_type>> |
There was a problem hiding this comment.
Not sure about this one in the long term but let it be, we can still change it later on. 👍
test/entt/entity/observer.cpp
Outdated
| }; | ||
|
|
||
|
|
||
| TEST(Observer, CustomInheritedRegistry) { |
There was a problem hiding this comment.
To be honest, this test seems unnecessary to me. The one for the sigh_mixin is enough to test it. I don't see what this one brings to the table.
There was a problem hiding this comment.
It tests that the observer setup, through setting up the required signals, via the sigh_mixin storage, is working correctly. Can remove it though if you still think it's redundant
This enables inheriting basic_registry and getting the inherited registry type (instead of basic_registry) in the signal handlers callback as the first argument
actondev
force-pushed
the
feat/templated-sigh_mixin-registry-type
branch
from
a0885db to
154bdfd
Compare
|
Sorry for the inconvenience but you'll have to wait a few days. I got a flu and I'm not exactly in the mood to work on PRs. 🙂 |
No inconvenience at all, take some rest & get better! 🍵 |
closes #1078
This a proposed approach for the linked issue