Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Better systems managing #31

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

opengs
Copy link
Contributor

@opengs opengs commented Oct 25, 2023

Implemented:

  1. System groups
  2. Automatic multithreading (based on requested read/write access to components)
  3. Systems ordering
  4. And a lot more related to systems. Check updated Readme.

Added Example with collision detection (multiple systems with ordering running in parallel). Here is screenshot from example:
result

If you don't like this approach, please let me know.

@unitoftime
Copy link
Owner

Oh wow! Nice work! I'm still kind of going through it. but just from my first pass I do have a few concerns.

  1. [Minor] I'd prefer to not have this library depend on other major libraries (such as Raylib)
  2. Theoretically, because systems contain a pointer to the main World, they can very easily bypass the lock-enforced restrictions.
  3. Queries should ideally be cached (rebuilding queries each frame is somewhat expensive because of all of the work done to figure out of what archetypes it needs to run for). So in your MovementSystem, for example, it'd be better to have the struct maintain a reference to the Query rather than a reference to the world. Doing that would actually fix Problem 2 that I mentioned above.
  4. [Minor] I'd prefer for systems to be just functions if possible, rather than full structs. Just to so that adding systems is as easy as possible.

The way I was thinking of going with this is somewhat similar to how Bevy does it. Of course some of Go's language "quirks" make this more difficult:

  1. System function signatures contain all of the resources that are required to run that function. So for in your MovementSystem example, you might have: func MovementSystem(dt time.Duration, query *ecs.View2[Position, Velocity]) { /* query.MapId(...) { ... } */ } (Also, maybe dt time.Duration might be a Resource).
  2. When you add the system to a group, some sort of dependency injection-type code will run, generating a struct (similar to what you have), which defines all of the requirements: Queries, Read Dependencies, Write Dependencies, System Name (from function name).
  3. I think GetRunAfter can be computed based on these read and write dependencies: A system must run after every system before it in the system list which writes to anything that it reads from. There is hopefully a way to convert the list of systems into a DAG based on their dependencies and then execute them down that tree. It might even be possible to do this locklessly as well.

Obviously my idea has a lot of problems based on the current code base:

  1. Queries don't define read vs write dependencies, technically everything is a pointer to the underlying slice, so everything is a write dependency.
  2. Filters are also passed in dynamically (rather than being compile time-typed), so they also make it hard to statically know the required component dependencies of a query
  3. The library needs better formalization of how Writes should occur in systems. I've played around with a lot of ideas but haven't really liked any of them to be honest. Was thinking of doing something similar to Bevy Commands
  4. I was kind of playing around with how the dependency injection would work here: https://github.com/unitoftime/ecs/blob/master/system.go#L33-L82 But couldn't really come up with anything that I loved. Go makes it hard because you end up with functions like NewSystem1, NewSystem2, NewSystem3, which can be kind of painful.

One thing to keep in mind also is that the amount of time it takes to do all of this preparation work doesn't matter, because It's only going to happen at app launch time, so we would want to precalculate and pre-cache as many queries as we can.

All that said, great work on this. It looks like a great start. This is obviously a very challenging problem to solve.
Thanks,
Unit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants