skypjack / entt

Gaming meets modern C++ - a fast and reliable entity component system (ECS) and much more
https://github.com/skypjack/entt/wiki
MIT License
10.01k stars 876 forks source link

[Feature Request] component (raw) data access using only component_type (id) #179

Closed dBagrat closed 5 years ago

dBagrat commented 5 years ago

Cannot handle some "complicated" cases in my new game-project without one of these (manually added) methods:

//registry::
std::byte* get_raw(const entity_type entity, const component_type ctype, const size_t component_size) ENTT_NOEXCEPT {
        assert(valid(entity));
        auto& cpool = *pools[ctype];
        auto& dpool = (sparse_set<Entity, std::byte>&)cpool;
        return dpool.raw() + component_size * cpool.get(entity);
}

or / and

 template<typename Type>
 Type & get_raw(const entity_type entity, const component_type ctype, const size_t component_size, const size_t offset = 0) ENTT_NOEXCEPT {
    assert(valid(entity));
    auto& cpool = *pools[ctype];
    auto& dpool = (sparse_set< Entity, Type >&)cpool;
    return dpool.raw()[(component_size / sizeof(Type)) * cpool.get(entity) + offset / sizeof(Type)];
}

btw, I totally understand that this kind of type-casting is not a brilliant idea :) but, it helps a lot in some "less hard coded" situations in my graphics engine.. for example, when some component-system needs to read data from another component of unknown Type, by using only (EntityId & TypeId).

skypjack commented 5 years ago

when some component-system needs to read data from another component of unknown Type

It may seem obvious, but how can you read data from a component of unknown type? On the other side, if the type is known, why can't you use it?

I'm pretty sure I'm missing something from your description.

To me it seems invalid. Can you provide more details about your use case, the API you would expect and the reasons for which this is required from your point of view? Thanks.

dBagrat commented 5 years ago

I'm storing a component's type id as a property of a special "reference" component. In other words, I would like components to be able to dynamically refer to other components, which isn't possible to do using a compile-time type. ex:

//some component
struct NoiseVec3 {
    enum
    {
        p_freq,
        p_strength,
        p_offset,
        p_seed,
    };
    float freq;
    Vec3f strength;
    Vec3f offset;
    uint32 seed;

    Vec3f output;
    float maybeAnotherOutput;
};

class NoiseSystem : public System
{
....
void Update( const float dt ) override;
};

and in other place:


struct ParamRef 
{
    Entity entity;
    ComponentTypeId typeId;
    uint32 offset;
};  

//special component for retrieving Vec3f values from other component/entity
struct PositionLink
{
    ParamRef ref;
    .....   
};
.....

somewhere at "Scene" setup:

ParamRef controllerRef( noiseEntity, registry::type< NoiseVec3 >(), offsetof( NoiseVec3, result ) );

systemManager.Link( entity, registry::type< PositionLink>(), controllerRef );

after, we may not have info about real NoiseVec3 type; or animation could be linked to any of other procedural animation component in his own entity. Also, scene may be constructed from "JSON" or some script language, and (for ex) NoiseVec3 will be created from "AbstractFactory" and using meta-style initialization.

TransformSystem::Update( float dt ) override
{
    auto view = registry.view< Transform >();
    for( auto entity : view )
    {
        auto& transform registry.get( entity );
        if( transform.useRef )
        {
            auto& ref = registry.get< PositionLink  >( entity );
            auto& value = registry.get_raw<Vec3f>( ref.entity, ref.typeId, GetSizeByID( ref.typeId ), ref.offset );
            //or
            //auto& value = *( (Vec3f*)( registry.get_raw( ref.entity, ref.typeId, GetSizeByID( ref.typeId ) ) + ref.offset ) );
            transform.pos = value;
        }
        else
        {
            //static position       
        }   
    }
}

And of course, some SystemManager handles all the sorting of components and systems respect to dependency graph, for correct order of "updates" execution. Also, the thing is, that any component in unique entity can be shared as a input value for other components/systems.

(tried to simplify the real working code for this explanation)

//I am working on the new graphics-engine, and decided to use ECS pattern in the most cases, but, also trying to reuse some practical/handy (maybe some sort of OOP style) concepts like a animation-chains, expressions, graphs, from my previous enigne that I used in game "Shadowmatic" (maybe you saw/played) :)

Any thoughts? maybe this is wrong programming trick/concept at all, but I didn't found any better and universal (and fast) solution for this kind of tasks.

Actually, this is my first time of using 3rd-party fundamental library in my engine and EnTT looks really very promising! Thank you!!

indianakernick commented 5 years ago

If I understand this correctly, ParamRef refers to a property on a component type. ParamRef could refer to NoiseVec3::result on a particular entity. The ParamRef inside of PositionLink always refers to a Vec3f.

I don't really know enough about your code to understand why you choose to write this unsafe C-style hack instead of a clean, modern C++ approach. That call to registry.get_raw<float> looks wrong. The component is a Vec3f but you're getting a float. The type system cannot spot these errors for you.

Why do you need this reference component that dynamically refers to properties on other components? Does PositionLink always refer to a property on the current entity? I'm sure there's a clean solution to your problem that doesn't involve reinterpret_cast, offsetof and code that just looks like C.

dBagrat commented 5 years ago

@Kerndog73 Yes, correct. I can check type-compatibility at the phase of attachment / creating of this "reference". Yeap, I don't like this kind of hacks too, but practically, every time I have several of these in the finished projects. idk how to become a better programmer :))

For example, I need this to be able for share one component for multiple entities, or reuse it's computation result as a value in different component/systems. procedural animations, etc..

some fresh idea: maybe i can store the function pointer of "correct" and type-safe component Getter() in this component-system ? //update: then i will kill inlining optimization :( it seems to be a same virtual-call i guess

indianakernick commented 5 years ago

reuse it's computation result as a value in different system

For this, you would typically chop-up your big component into smaller ones. If I understand correctly, NoiseSystem uses freq, strength, offset and seed to calculate output and maybeAnotherOutput. You should create a separate component for the output of the NoiseSystem.

// I don't really know what this output means so you could think of a better name
struct NoiseOutput {
    Vec3f output;
    float maybeAnotherOutput;
};

Then, other systems could read NoiseOutput and use it to do other stuff.

I need this to be able for share one component for multiple entities

This is tricky. You could have a component that just stores a std::shared_ptr<SharedComponent>. Or maybe you could store a pointer to some external data structure like QuadTreeNode * or something.

dBagrat commented 5 years ago

For this, you would typically chop-up your big component into smaller ones. If I understand correctly, NoiseSystem uses freq, strength, offset and seed to calculate output and maybeAnotherOutput. You should create a separate component for the output of the NoiseSystem.

agree, I really tried this a couple of days ago. then I thought, that it's a less cache-friendly idea to split "result" part to another memory location.

This is tricky. You could have a component that just stores a std::shared_ptr<SharedComponent>.

yeap, thanks, In some cases this is a best solution maybe..

indianakernick commented 5 years ago

then I thought, that it's a less cache-friendly idea to split "result" part to another memory location.

To make iterating a multi-component view more cache-friendly, you can sort one of the component pools to be in the same order as another pool. For example, the NoiseSystem iterates NoiseVec3 and NoiseOutput (and maybe some other components too). To make the iteration more efficient, you would do this:

reg.sort<NoiseOutput, NoiseVec3>();

This will sort the NoiseOutput pool to be in the same order as the NoiseVec3 pool. Iterating a view of both of these components will now be a little faster. Policies might be able to provide further optimizations in the future.

dBagrat commented 5 years ago

@Kerndog73 sound good, I have tried reg.sort for a very tiny component already for changing the order of View iteration (respect to some dependencies) Interested to test the performance of sorting and it's "memory move operations" in real / practical situations Thank you! btw: accessing directly to the "main" component still seducing me :D will think a little...

indianakernick commented 5 years ago

I should mention that this sorting is unnecessary if the two components are being added and removed from the entity together. If you have some kind of factory that adds the two components to an entity and you never have an entity with just one or just the other then the pools should already be in the same order and sorting will do nothing.

dBagrat commented 5 years ago

I should mention that this sorting is unnecessary if the two components are being added and removed from the entity together. If you have some kind of factory that adds the two components to an entity and you never have an entity with just one or just the other then the pools should already be in the same order and sorting will do nothing.

mmm that's really cool!! Super! Thanks for the info, didn't thought about that before. @Kerndog73 In this case, this could be even faster to store result as a "tiny" components, for future read process! :)

dBagrat commented 5 years ago

@Kerndog73 Imagine case, if for the some reason i needed to store 2-results of the same type, and access them later. anyway i can't use struct NoiseOutput as a component output type, because i will have the same problem later as before.. I should store Struct with the same type as the Input or with some standart type. like a Vec3, etc... or maybe

struct VecXX_output
{
union { float, Vec2, Vec3, Vec4 }
};
struct VecXX_output2 ...
indianakernick commented 5 years ago

So you have two separate systems that both produce their own separate NoiseOutput and you need to differentiate between the two? Well, surely these separate NoiseOutputs have different meanings right? Just create two separate structs with different names that describe what these NoiseOutputs mean.

indianakernick commented 5 years ago

You might want to check the EnTT in Action section of the Wiki. There are loads of examples and articles there about EnTT. EnTT Pacman is pretty cool 😄.

dBagrat commented 5 years ago

You might want to check the EnTT in Action section of the Wiki. There are loads of examples and articles there about EnTT.

Thank you, I'll do. EnTT Pacman is pretty cool 😄.

Wow, is this your project? Nice!, I'll definitely look into it soon :D

skypjack commented 5 years ago

This shadowmatic?
Never played it but the idea is really good, great job. What technologies did you use for that one? Just curious now.

In other words, I would like components to be able to dynamically refer to other components, which isn't possible to do using a compile-time type.

This assumption is wrong. At the call site where you get the numerical component type, you know the actual type (otherwise you couldn't ask for its identifier) and you know it at compile-time. EnTT is mostly about compile-time.
Instead of storing the component type, store a function through a function pointer and fill it with a specialization of a function template, so as to avoid risky things like the ones you're doing.

As an example, consider this:

ParamRef controllerRef( noiseEntity, registry::type< NoiseVec3 >(), offsetof( NoiseVec3, result ) );
systemManager.Link( entity, registry::type< PositionLink>(), controllerRef );
// ...
auto& ref = registry.get< PositionLink  >( entity );
auto& value = registry.get_raw<Vec3f>( ref.entity, ref.typeId, GetSizeByID( ref.typeId ), ref.offset );

Don't do this, do something like this instead:

struct ParamRef {
    template<typename Type>
    using get_value_fn = Type(registry &, entity_type);

    Entity entity;
    get_value_fn *get_value;
};

template<typename Type, auto Member>
auto get_value_proto(registry &reg, entity_type entity) {
    return (reg.get<Type>(entity).*Member);
}

// ...

ParamRef controllerRef( noiseEntity, &get_value_proto<NoiseVec3, &NoiseVec3::result> );
systemManager.Link( entity, registry::type< PositionLink>(), controllerRef );
// ...
auto& ref = registry.get< PositionLink  >( entity );
auto& value = ref.get_value(registry, ref.entity);

I don't expect it to work, but it should give you an idea of what I mean.
Let me know if you need more details on this.

The fact is that you're asking to introduce in the registry an unsafe function to do risky things, while C++ and what the registry already offers allow you to do the same in a much safer manner and without changes.
This looks a lot like an XY-problem.

dBagrat commented 5 years ago

This shadowmatic? Never played it but the idea is really good, great job. What technologies did you use for that one? Just curious now.

Yes. Thank you! I wrote that engine from scratch, using OpenGL ES (2.0-3.1) and Metal for modern iOS devices. The only 3rd-party library that I used there, is the ZLib and ofc some texture compression SDK/Tools like a PVRTC and ASTC (astcenc). Initially it was released for iOS, and after 1.5 years ported to Android too. Porting to Android was really hard, because of too much GPU types / drivers, and the testing process was painful too. More than 50 android devices are used/tested at office and 1000+ with cloud testing services (often AWS) This assumption is wrong. At the call site where you get the numerical component type, you know the actual type (otherwise you couldn't ask for its identifier) and you know it at compile-time. EnTT is mostly about compile-time.

I see, now I agree with this.

Instead of storing the component type, store a function through a function pointer and fill it with a specialization of a function template, so as to avoid risky things like the ones you're doing.

Nice, thanks for the idea, I will definitely try that. Should it be a little faster that virtual-call ? (or same?) The fact is that you're asking to introduce in the registry an unsafe function to do risky things, while C++ and what the registry already offers allow you to do the same in a much safer manner and without changes.

Right, now i see. btw, my problem was solved by backing to solution of using another "result" component with expected and known type for other system. Thank you.

Also a new question: why assure<Component...>() is called every time through registry.get<Component>(entity) ? maybe I miss something, but it has a couple of (maybe unnecessary) checks even in optimized binary. Thanks.

skypjack commented 5 years ago

Nice, thanks for the idea, I will definitely try that. Should it be a little faster that virtual-call ? (or same?)

Yes, it is. You won't appreciate it anytime soon, but you can easily benchmark different models based on type erasure against virtual calls to see the difference.
That being said, in your case it's a matter of few calls each loop probably, so I wouldn't care of the difference and use the one with which I'm more comfortable.

btw, my problem was solved by backing to solution of using another "result" component with expected and known type for other system. Thank you.

You're welcome. I'm closing the issue because of this, but feel free to continue the discussion here if you have other questions.

why assure() is called every time through registry.get(entity) ? maybe I miss something, but it has a couple of (maybe unnecessary) checks even in optimized binary.

In fact, it could be removed. Mainly because unlikely there will be the component if there isn't the pool and your software will crash in any case sooner or later.
However, registry::get is known to be slower than view::get and you should use the latter during iterations. On the other side, using registry::view for sporadic access won't affect your code anyway.

I see you're concerned a lot about performance, but use the right measure. If you're iterating 200k entities each tick in a loop, it makes sense to ask for something more and avoid virtual calls. If you're going to do 1000 access through a registry::get, you can freely ignore indirections, virtual calls or whatever, you wouldn't notice the difference in any case probably.

My two cents. :wink:

skypjack commented 5 years ago

As a side note, I suggest you to join the gitter channel if you have any other question.
You can find people far more skilled than me there and receive answers quite fast. :+1:

dBagrat commented 5 years ago

In fact, it could be removed. Mainly because unlikely there will be the component if there isn't the pool and your software will crash in any case sooner or later.

👍 However, registry::get is known to be slower than view::get and you should use the latter during iterations. On the other side, using registry::view for sporadic access won't affect your code anyway.

yes ofc, I'm using registry::get in loops rarely, only when spontaneous/random "other" component access needed. I see you're concerned a lot about performance, but use the right measure. If you're iterating 200k entities each tick in a loop, it makes sense to ask for something more and avoid virtual calls. If you're going to do 1000 access through a registry::get, you can freely ignore indirections, virtual calls or whatever, you wouldn't notice the difference in any case probably.

Yeah, agree with you. I think that I have a "disease" since the 90's to open disassembly often and see what really happened 😄 But, yeap, should use the "right" measure mostly 👍