Closed Doraku closed 5 years ago
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.
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.
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.
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.
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.
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?
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!
It might be a little bit confusing with the whole
EntityRecord
mirroringEntity
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
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:
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?
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
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 :/
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)!
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.
Some quick benchmarks:
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.
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.
And done :)
Awesome! I look forward to taking it for a spin.
It's not possible to add/remove components or create/dispose entities when updating a system. There should be an easy mechanism to deffer those action at the end of a multithreading update. Problem is we do not want to stress gc so the model need to be allocation free apart from the initialization. How to handle component operations that way? custom memory allocator? All Entity operations should be available.
edit: this will probably be used for #20 afterward.