add some kind of EntityCommandBuffer about defaultecs HOT 16 CLOSED

Haha. This was on my todo list. Glad I noticed this issue. I guess I can leave it to you since it will be making its way into DefECS directly! No need for me to make a separate one outside of the library.

from defaultecs.

Well wanting to make and making can be two entirely different beasts :D I have a theory on how to do it with my constrains which need some testing, hopefully it will work out.

from defaultecs.

I hadn’t fully thought it out yet but I was going to start with a pool of command objects. One command object per Entity operation. When the buffer gets flushed at a barrier system (or whenever it happens) the commands get played back and each command object gets put back in the pool.

from defaultecs.

The main problem is stocking your component values without stressing the GC (if you want to be as crazy as me). You need generic commands and I don't think having a global pool for each type sounds like a good idea.
It's probably insane but I want to allocate some memory so I can put struct commands as I please on it with a succession of nextCommandOffset, command, nextCommandOffset, command, ... All commands would implement ICommand so I can just unsafely cast my memory and play each command, jumping to the next point easily without bothering the GC. This should work nice for unmanaged component types but there is probably some more stuff to think for reference types.

from defaultecs.

That will definitely work for unmanaged, blittable Types. The moment someone throws a pointer in there it will explode though.

For my specific case it would actually work. What I did was kinda similar to how Unity does things. All Components must implement an empty interface. Anything implementing the interface gets checked in debug builds that it is a struct with only blittable types in it.

There is a second interface for structs that have non-blittables (textures, lists, etc). This so far is helping keep things super clean. Makes figuring out what to serialize easier and stuff like an EntityCommandBuffer becomes easier. Even if the structs have to be copied if they are all blittable they will be tiny.

from defaultecs.

So I've made some progress on this. The code is not as clean I hoped it to be since I had to make some changes and I still have to handle non blittable component types (I have some ideas on that but it may allocate a tiny bit sometimes :( ).
This is the current API:

public sealed unsafe class EntityCommandRecorder
{
    // gives you an EntityRecord from an entity to record action on the given entity
    public EntityRecord Record(in Entity entity);

    // gives you an EntityRecord to record action on an entity which will be later created
    public EntityRecord CreateEntity();

    // execute what has been recorder, new entity will be created on the given world, and clear recording
    public void Execute(World world);
}

public readonly ref struct EntityRecord
{
    public void Enable();

    public void Disable();

    public void Enable<T>();

    public void Disable<T>();

    public void Set<T>(in T component = default);

    public void SetSameAs<T>(in EntityRecord reference);

    public void Remove<T>();

    public void SetAsChildOf(in EntityRecord parent);

    public void SetAsParentOf(in EntityRecord child);

    public void RemoveFromChildrenOf(in EntityRecord parent);

    public void RemoveFromParentsOf(in EntityRecord child);

    public void Dispose();
}

Except the Execute method obviously, everything is threadsafe.
The recorder currently is independent from a world instance, hence the need to give one to the Execute method to know where to create entity should it needs to but I am not sure it has a use. Maybe because I want to reuse this for #20 I try to mix them a little too much, what do you think?

from defaultecs.

That looks like it covers everything. It might be a little bit confusing with the whole EntityRecord mirroring Entity but once you see it the first time it makes sense. Also totally makes sense that it is independent from the World. I like it 👍

As for #20, you can probably guess my opinion on that one by now 😉. I don't think the ECS should know or care about serialization at all. DefaultECS doesn't know about my games Components, what fields to serialize, what format to serialize them in, etc. And it shouldn't ever know anything about it. In my opinion that should be game specific or at the very least removed from DefaultECS and put into a separate package.

I'm a big fan of how the Systems are all optional in DefaultECS and just there as example implementations. I don't think they should be in the main package either. They are better suited for a separate package or just put in with the Samples since they arent required. A mean, lean, focused base ECS is the key. The rest is up to the game developer to add.

Anyway, that's just my two cents! Keep up the good work!

from defaultecs.

It might be a little bit confusing with the whole EntityRecord mirroring Entity

Yeah at first I had all those methods on EntityCommandRecorder directly, with an Entity as parameter but when I added the possibility to create new Entity through the recorder, this EntityRecord emerged so I could keep track of the new item. Having all those methods overloaded with both Entity and EntityRecord seemed pretty awful (SetSameAs taking (Entity, Entity), (Entity, EntityRecord), (EntityRecord, Entity) and (EntityRecord, EntityRecord)???) so I just moved everything to this mirror type. It may feel strange but I think it makes much more sens that way :)

As for #20, you can probably guess my opinion on that one by now 😉.

Haha I should have expected it! That's why I try to keep in the main namespace what is really necessary and push optional stuff to other ones. Maybe one day it will be separated in modules :p

from defaultecs.

API for the command buffers looks great! I’ve got one small suggestion: creating a command buffer requires a magical, arbitrary byte[] size to be passed in. At buffer creation time I have no idea if I’ll need 0 or 1,000,000 bytes! Perhaps it would be cleaner and more user-friendly if that param was omitted. Internally you could:

• pull a reasonably sized, default byte array from ArrayPool (to avoid allocations)
• if the user adds too many commands instead of throwing an exception pull a 1.5x sized byte[] from ArrayPool, BlockCopy to the new array and return the old one to the ArrayPool

That approach will avoid making extra large command buffers at runtime just as a defensive mechanism to avoid the exception. Cleans up the public API as well since knowing the byte[] size ahead of time is nigh impossible.

What do you think?

from defaultecs.

Indeed this magic size looks like the beginning of the World type before I changed it to be able to grow as needed (you had to set a maximum entity count). I'll probably end up with something similar

public EntityCommandRecorder();

public EntityCommandRecorder(int maxSize);

but I am not sure at what size to start the inner buffer... This is currently the size of each command

• record an existing entity or create a new entity: 13 bytes
• enable, disable or dispose an entity: 9 bytes
• enable, disable or remove a component: 13 bytes
• set a component: 13 bytes + (size of the component if blittable or 4 bytes if not but this can cause boxing and stress the gc if it's a struct, should be ok if it's a class)
• set same as component : 17 bytes
• set as child of or remove from child: 13 bytes

I would say 1Mo just because, those recorders can outlive and be shared between systems/scenes anyway.

On the ArrayPool, this is a good idea to remove the gc stress but that mean you would end up with lot of useless array in the pool in the long run. This is probably fine on windows/linux but on closed hardware/mobile it could weight too much on the memory.
Which is why I may add public EntityCommandRecorder(int startSize, int maxSize); so you can ease the growth of the buffer. I'm still on the fence for the constructors :/

from defaultecs.

Typing those sizes, I think I can probably shrink all of them by 4 bytes (the offset of the next command in the buffer is not really needed)!

from defaultecs.

Woohoo! Another 4 bytes saved!

As for ArrayPool, it’s really nice for byte arrays because internally .NET framework classes are all going to be using it. If you ask for a 1k buffer the system will give you any buffer it already has that is 1k or bigger. For command buffers, these are all short lived (one frame max) so chances are each frame you’ll get back the same buffers you had the previous frame.

Anyway, it’s just a thought. As long as they auto-expand I think it should be quite useful for all.

from defaultecs.

Some quick benchmarks:

• base (some operations done directly on entities): 285ns
• deferred with static size recorder: 883ns
• deferred with a grow-able recorder: 1326ns

Needless to say I am not too happy about the expandable buffer :/ As it need to be thread safe, I used a ReaderWriterLockSlim for the expandable implementation ("reader" to write commands, "writer" to expand the array) and I don't see how this could be improved. The problem is that once you fix the buffer to do magic pointer things to write a command, you can't have an other command expanding the buffer by blockcopying and changing the reference to the inner array without any lock mechanism.

So I guess I'll give both.

// this one is expandable
public EntityCommandRecorder();

// this one is not
public EntityCommandRecorder(int maxCapacity);

// start as expandable but once current capacity == maxCapacity, fallback to the fast path
public EntityCommandRecorder(int startCapacity, int maxCapacity);

What do you think?

edit: once on the fast path, it falls down to 970ns.

from defaultecs.

That's not too bad considering it is a deferred system that essentially has to run all the commands twice and store/retrieve them. Sounds like as long as the initial buffer isnt set too small to avoid resizing more than 1 time at worst then it should be fine.

from defaultecs.

And done :)

from defaultecs.

Awesome! I look forward to taking it for a spin.

from defaultecs.