Add more methods to RelationshipSourceCollection
#18421
Conversation
alice-i-cecile
added
A-ECS
| @@ -16,9 +16,19 @@ pub trait RelationshipSourceCollection { | |||
| where | |||
| Self: 'a; | |||
|
|
|||
| /// Creates a new empty instance. | |||
| fn new() -> Self; | |||
There was a problem hiding this comment.
Should this always be equivalent to Default::default()? If so, then I might vote for removing this method and putting Default as a supertrait.
... Ah, that wouldn't work for Entity.
It might help to have a comment explaining how this is different from Default::default(), although I'm not quite sure what to write there.
crates/bevy_ecs/src/relationship/relationship_source_collection.rs
Outdated
Show resolved
Hide resolved
| /// Reserves capacity for at least `additional` more entities to be inserted. | ||
| /// | ||
| /// Not all collections support this operation, in which case it is a no-op. | ||
| fn reserve(&mut self, additional: usize); |
There was a problem hiding this comment.
Would it make sense to offer default implementations of the optional methods? Even with_capacity could default to returning Self::new().
On the other hand, Entity is the only collection that doesn't implement them, so it wouldn't really save much.
| fn reserve(&mut self, additional: usize); | |
| fn reserve(&mut self, _additional: usize) {} |
There was a problem hiding this comment.
Using with_capacity in new or vice versa would require adding a Self: Sized bound. I decide to not bother and just implement the method manually everywhere.
alice-i-cecile
added
S-Ready-For-Final-Review
Co-authored-by: Chris Russell <[email protected]>
|
I think I got those merge conflicts right. @Brezak please yell at me and/or fix them if CI fails again <3 |
Head branch was pushed to by a user without write access
|
@alice-i-cecile formatting has been fixed but the auto-merge was disabled. |
# Objective While working on #18058 I realized I could use `RelationshipTargetCollection::new`, so I added it. ## Solution - Add `RelationshipTargetCollection::new` - Add `RelationshipTargetCollection::reserve`. Could generally be useful when doing micro-optimizations. - Add `RelationshipTargetCollection::shrink_to_fit`. Rust collections generally don't shrink when removing elements. Might be a good idea to call this once in a while. ## Testing `cargo clippy` --- ## Showcase `RelationshipSourceCollection` now implements `new`, `reserve` and `shrink_to_fit` to give greater control over how much memory it consumes. ## Migration Guide Any type implementing `RelationshipSourceCollection` now needs to also implement `new`, `reserve` and `shrink_to_fit`. `reserve` and `shrink_to_fit` can be made no-ops if they conceptually mean nothing to a collection. --------- Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Chris Russell <[email protected]>
Objective
While working on #18058 I realized I could use
RelationshipTargetCollection::new, so I added it.Solution
RelationshipTargetCollection::newRelationshipTargetCollection::reserve. Could generally be useful when doing micro-optimizations.RelationshipTargetCollection::shrink_to_fit. Rust collections generally don't shrink when removing elements. Might be a good idea to call this once in a while.Testing
cargo clippyShowcase
RelationshipSourceCollectionnow implementsnew,reserveandshrink_to_fitto give greater control over how much memory it consumes.Migration Guide
Any type implementing
RelationshipSourceCollectionnow needs to also implementnew,reserveandshrink_to_fit.reserveandshrink_to_fitcan be made no-ops if they conceptually mean nothing to a collection.