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

Entity Relations RFC #18

Closed
wants to merge 86 commits into from
Closed

Conversation

BoxyUwU
Copy link
Member

@BoxyUwU BoxyUwU commented Apr 23, 2021

RFC mostly written by alice with some help from me

Rendered

Partial Impl

rfcs/min-relations.md Outdated Show resolved Hide resolved

## Motivation

Entities often need to be aware of each other in complex ways: an attack that targets another unit, groups of entities that are controlled en-masse, or complex parent-child hierarchies that need to move together.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the InFrustum(camera_id) would be especially understandable here. It's easy to see why it can't be handled by simple tag component here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry that multiple camera frustums are bit too obscure of a topic for the average gameplay programmer referring to this RFC. I agree that they're a clear example though!

- How do we despawn an entity and all of its children (with many types of child-like entities)?
- How do we quickly access data on the entity that we're attached to?
- How do we traverse this graph of entities?
- How do we ensure that our graph is acylic?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this particular point is handled in any way by proposed relations. Afaict cycles are still an issue (in cases where you explicitly don't want them).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The framing could be clearer. These problems are possible to address in a cohesive way at an engine-level by including relations as a feature. Min-relations won't hit all of these though :(

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so it's more of a future prospect. I guess that's fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarified!

Co-authored-by: Paweł Grabarz <frizi09@gmail.com>
rfcs/min-relations.md Outdated Show resolved Hide resolved
rfcs/min-relations.md Outdated Show resolved Hide resolved
alice-i-cecile and others added 2 commits April 23, 2021 20:07
Co-authored-by: Paweł Grabarz <frizi09@gmail.com>
Co-authored-by: Paweł Grabarz <frizi09@gmail.com>
rfcs/min-relations.md Outdated Show resolved Hide resolved
rfcs/min-relations.md Outdated Show resolved Hide resolved
rfcs/min-relations.md Outdated Show resolved Hide resolved
Co-authored-by: Paweł Grabarz <frizi09@gmail.com>
Co-authored-by: tigregalis <38416468+tigregalis@users.noreply.github.com>
@BoxyUwU
Copy link
Member Author

BoxyUwU commented May 2, 2021

Why is this the case? Relations could build on SparseSet - I guess the question is, now does every entity have a Relation?

@tigregalis relations can be in any StorageType just like components- regardless of Table/SparseSet storage this still fragments archetypes as in bevy "archetypes" means something subtly different than other ECS'- in bevy archetypes are just a pile of metadata for what tables and sparse sets all the components are stored in for that set of entities

It's still bad to fragment our archetypes here though as non-dense queries (queries that interact with SparseSet stored components/relations) have to iterate over archetypes so keep the amount of archetypes low is good for performance.

(both of these are external to the components/resources/archetypes)

Storing relations in such a way that they dont fragment archetypes is something I would very much like to support- however, fragmenting archetypes isnt always bad. I wrote up some thoughts a few days ago about the problems with fragmenting archetypes and also why we probably want to support doing it

(In this RFC, we call this the **Entity-in-component pattern**.)
But this quickly starts to balloon in complexity.

- How do we ensure that these components are cleaned up when the entity they're pointing to is?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- How do we ensure that these components are cleaned up when the entity they're pointing to is?
- How do we ensure that these components are cleaned up when the entity they're pointing to is removed?

The set of features expressed here is useful enough to be a clear improvement over the existing patterns in at least some use cases.
There's a lot of value in getting this feature in front of users to see the response at scale.

### Why is the implementation for this so *slow*?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be helpful to provide some measure of how slow this is, and what operations are particularly bad.

  • Are queries fast?
  • Is it only adding/changing relations that is slow?

A starting point could be to run the ECS benchmark as-is with some relation filters on to see if there is a tangible impact. Perhaps seeing how adding/removing relations compares to component add/removes. Having some information, even if it isn't complete or perfect, would help users understand the order of magnitude they should expect.

Many uses of relations are not performance-critical,

I only say this because "performance-critical" is a hugely relative metric.

@alice-i-cecile
Copy link
Member

alice-i-cecile commented May 21, 2021

Status update! A branch on the Bevy repo has been opened up for this work. Plan is:

  1. Do basic review on Entity Relations bevy#1627.
  2. Merge in that PR so others have a stable base to work off of.
  3. Put bevy_ecs in "slow mode" to reduce the level of merge conflicts we get while this is live.
  4. Work on that branch as a group to benchmark relations, improve performance and add features before they go live.

From @cart on Discord, his main concerns were:

  1. I want to see perf comparisons between "manually implemented reactive indexing" (which represents the theoretical upper limit to a real reactive system) and relations, with benches that cover the spectrum of best and worst case performance. Ideally we have some "real world" scenario tests too.
  2. I want a summary of those tests in the context of "how we would report the current perf limitations to users".

We can then use these to determine if we merge the current relations impl (or what the criteria for merging will be). If relations are the general purpose indexing api we're committing to, but the current impl doesn't meet those needs yet, we need to come up with a concrete plan for the future with a very high chance of success and few unknowns (edit for clarity: ... if we want to merge the current impl).

This work should be able to be distributed nicely once we have a basic implementation in place on that branch.

@alice-i-cecile
Copy link
Member

This Reddit thread is a fantastic use case for relations (especially with graph invariants): they want to have one bird chasing another (again, the Entity-in-a-component pattern) but are running up against the borrow checker, since they can't guarantee that something won't target itself.

Implementing a workaround for this in user code is very clunky, but if we can get simple graph invariants working (i.e. no self-targeting relations of this type) we can seamlessly enable this in the engine.

@BoxyUwU BoxyUwU changed the title Entity Relations Entity Relations RFC Jul 28, 2021
@LegNeato
Copy link

Facebook calls these "associations" in TAO (their graph db) and their data access application code layer FYI:

(Pdf warning)

https://www.usenix.org/system/files/conference/atc13/atc13-bronson.pdf&ved=2ahUKEwif9qnSyYzyAhVFgp4KHYhIAqEQFjABegQICRAC&usg=AOvVaw3IBhg6Xn4t8igN25axxu-h

@LegNeato
Copy link

Also check out differential dataflow, which makes storing and operating on graphs extremely fast and performant:

https://github.com/TimelyDataflow/differential-dataflow

@alice-i-cecile
Copy link
Member

I've been reading about differential dataflow :) Seems like an advanced high-performance abstraction we can add on later and use internally. It will likely be too complex for most end user cases, but I'm curious to see what sort of high-performance APIs for common operations we can create.

@LegNeato
Copy link

LegNeato commented Aug 1, 2021

As I used to work at FB...all I think about and have experience with are graphs :-). Happy to talk about what worked and what didn't at scale. I think exposing graph semantics are extremely powerful and let framework authors optimize a lot while keeping a simple interface for product code (mainly entities and associations / edges and how to traverse them).

@HackerFoo
Copy link

I'm using relations in my CSG editor to represent operations on two (or more) shapes. For this, binary relations (graph edges) are not enough, because there are (at least) three entities involved: the two operands, and the result.
It also useful to iterate through all relations of a specific type (for computing the operations), which implies that each relation should be an entity.

This is how relations are represented in relational databases, where each relation is a table.

Here's a gist, but much more work would be required to make this easy to use. Note that this uses a Component containing nothing but Entitys, so it's the opposite approach :)

Since each relation is an entity, this supports higher-order relations, as well as using an existing entity as a relation, e.g. parent/child relations could be represented with the RelationIndex pointing to the inverse relation, so that a child's RelationIndex points to the parents Children relation, containing the child's entity.

@alice-i-cecile
Copy link
Member

Closing out, as this no longer represents the current direction we tend to take this feature set.

@vultix
Copy link

vultix commented Jan 12, 2022

@alice-i-cecile What is the current plan for this feature set?

@alice-i-cecile
Copy link
Member

@vultix good question! There are two competing paths forward, very roughly:

  1. Implement some form of reliable indexing. At the moment I expect that atomic ordering constraints are probably the most likely strategy here.
  2. Build out a very basic prototype that uses an resource to track the graph centrally and the reliable indexing tech to keep this in sync.
  3. Build out a pretty API on top of that prototype, which should be able to mostly look like the design in this RFC.

Alternatively:

  1. Forbid direction mutations of parent-child components using type-level permissions.
  2. Only allow modification via commands.
  3. Extend the parent / child commands API to be generic over user-generated relation types.

The first is my preference, but the second may be easier to get to (and the rest of the team can disagree with me!). The gist of the problem is that we can't efficiently store lookups of the relations graph within the archetype graph, because of the large non-localized performance implications of such widespread archetype fragmentation.

Most of the relevant ECS folks are tied up with trying to improve scheduling APIs so this feature has been on the backburner.

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

Successfully merging this pull request may close these issues.