One-to-One Relationships

[dmyyy]

Objective

Minimal implementation of directed one-to-one relationships via implementing RelationshipSourceCollection for Entity.

Now you can do

#[derive(Component)]
#[relationship(relationship_target = Below)]
pub struct Above(Entity);

#[derive(Component)]
#[relationship_target(relationship = Above)]
pub struct Below(Entity);

Future Work

It would be nice if the relationships could be fully symmetrical in the future - in the example above, since Above is the source of truth you can't add Below to an entity and have Above added automatically.

Testing

Wrote unit tests for new relationship sources and and verified adding/removing relationships maintains connection as expected.

[alice-i-cecile]

This definitely feels sus. What if we impl'ed this for Option<Entity> instead?

[dmyyy]

I did it that way at first. It felt weird to me that it's expressable for the RelationshipTarget to be None while the Relationship still points to that entity. To be fair - manually changing the RelationshipTarget to None is just as weird as setting the RelationshipTarget to some random entity (we mess up the relationship).

This felt like the more minimal change that preserves symmetry (but I'm open to changing my mind)

[Carter0]

I like it! Though I like the Option idea more personally

[ItsDoot]

LGTM. FWIW, I prefer the Entity approach because it better matches how its interacted with; assuming all invariants are upheld, users will always have access to a single valid entity.

[dmyyy]

The Option<Entity> approach feels like a leaky abstraction - if I'm trying to make a one-to-one relationship I wouldn't be able to guess that I need to write it like this without looking through the source code which is a bit of a red flag IMO.

#[derive(Component)]
#[relationship(relationship_target = Below)]
pub struct Above(Entity);

#[derive(Component)]
#[relationship_target(relationship = Above)]
pub struct Below(Option<Entity>);

@alice-i-cecile Is it possible for this to be considered for 0.16?

[Carter0]

Yeah I see what y'all mean. There should always be a 1-1 relationship between the two entities as an invariant.

Not something to consider for this PR perhaps, but I wonder if this is a code smell then. It sorta weird to use the word "collection" in the code when really it's just one item.

[alice-i-cecile]

Yep, I'll make the call today in my merge train :) The 0.16 milestone isn't exhaustive: it just captures "stuff that I would consider blocking the release for".

[alice-i-cecile]

The Option<Entity> approach feels like a leaky abstraction - if I'm trying to make a one-to-one relationship I wouldn't be able to guess that I need to write it like this without looking through the source code which is a bit of a red flag IMO.

#[derive(Component)]
#[relationship(relationship_target = Below)]
pub struct Above(Entity);

#[derive(Component)]
#[relationship_target(relationship = Above)]
pub struct Below(Option<Entity>);

This comment convinced me. Merging.