From 15ac36f6d533c5be41d929a118fdfa50ac90c32b Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 14 Apr 2025 18:25:37 -0700 Subject: [PATCH] Panic on overlapping one-to-one relationships (#18833) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective One to one relationships (added in https://github.com/bevyengine/bevy/pull/18087) can currently easily be invalidated by having two entities relate to the same target. Alternative to #18817 (removing one-to-one relationships) ## Solution Panic if a RelationshipTarget is already targeted. Thanks @urben1680 for the idea! --------- Co-authored-by: François Mockers --- .../relationship_source_collection.rs | 60 ++++++++++++++++++- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/relationship/relationship_source_collection.rs b/crates/bevy_ecs/src/relationship/relationship_source_collection.rs index 4cf693e69d..1a83c6c825 100644 --- a/crates/bevy_ecs/src/relationship/relationship_source_collection.rs +++ b/crates/bevy_ecs/src/relationship/relationship_source_collection.rs @@ -326,7 +326,7 @@ impl RelationshipSourceCollection for SmallVec<[Entity; N]> { } impl RelationshipSourceCollection for Entity { - type SourceIter<'a> = core::iter::Once; + type SourceIter<'a> = core::option::IntoIter; fn new() -> Self { Entity::PLACEHOLDER @@ -339,6 +339,12 @@ impl RelationshipSourceCollection for Entity { } fn add(&mut self, entity: Entity) -> bool { + assert_eq!( + *self, + Entity::PLACEHOLDER, + "Entity {entity} attempted to target an entity with a one-to-one relationship, but it is already targeted by {}. You must remove the original relationship first.", + *self + ); *self = entity; true @@ -355,7 +361,11 @@ impl RelationshipSourceCollection for Entity { } fn iter(&self) -> Self::SourceIter<'_> { - core::iter::once(*self) + if *self == Entity::PLACEHOLDER { + None.into_iter() + } else { + Some(*self).into_iter() + } } fn len(&self) -> usize { @@ -372,7 +382,13 @@ impl RelationshipSourceCollection for Entity { fn shrink_to_fit(&mut self) {} fn extend_from_iter(&mut self, entities: impl IntoIterator) { - if let Some(entity) = entities.into_iter().last() { + for entity in entities { + assert_eq!( + *self, + Entity::PLACEHOLDER, + "Entity {entity} attempted to target an entity with a one-to-one relationship, but it is already targeted by {}. You must remove the original relationship first.", + *self + ); *self = entity; } } @@ -530,4 +546,42 @@ mod tests { assert!(world.get::(b).is_none()); assert_eq!(a, world.get::(c).unwrap().0); } + + #[test] + #[should_panic] + fn one_to_one_relationship_shared_target() { + #[derive(Component)] + #[relationship(relationship_target = Below)] + struct Above(Entity); + + #[derive(Component)] + #[relationship_target(relationship = Above)] + struct Below(Entity); + + let mut world = World::new(); + let a = world.spawn_empty().id(); + let b = world.spawn_empty().id(); + let c = world.spawn_empty().id(); + + world.entity_mut(a).insert(Above(c)); + world.entity_mut(b).insert(Above(c)); + } + + #[test] + fn one_to_one_relationship_reinsert() { + #[derive(Component)] + #[relationship(relationship_target = Below)] + struct Above(Entity); + + #[derive(Component)] + #[relationship_target(relationship = Above)] + struct Below(Entity); + + let mut world = World::new(); + let a = world.spawn_empty().id(); + let b = world.spawn_empty().id(); + + world.entity_mut(a).insert(Above(b)); + world.entity_mut(a).insert(Above(b)); + } }