Entity path methods and bsn entity path resolving#24018
Entity path methods and bsn entity path resolving#24018alice-i-cecile merged 24 commits intobevyengine:mainfrom
Conversation
|
Small note, should I change the methods to have more descriptive errors? |
Co-authored-by: Freyja-moth <156322843+Freyja-moth@users.noreply.github.com>
|
Currently using an immutable borrow of world. This does cause it to return |
alice-i-cecile
left a comment
There was a problem hiding this comment.
I quite like this idea in practice: this seems extremely useful for both animation and UI. However, I think there are two important cleanups to do:
- Make the error handling better.
- Extract out a pair of common case methods, and give those the nice names.
|
It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note. Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes. |
|
Moved the previous methods to Not fully happy with the names so welcoming bikeshedding. |
|
Will fix whatever issues come up tomorrow, I've spent way too much time on this tonight. |
alice-i-cecile
left a comment
There was a problem hiding this comment.
I'm happy with this now, but it still needs a test for the BSN usage.
The release note is super exciting: what an incredibly useful thing to do on cold paths. I've left a few suggestions to improve it.
Good luck with CI 🫡
|
With any luck this should be the last of the changes. |
|
Ci passed, I'm free! |
|
The implementation differs from the PR description; the path is &str instead of &[C]. What's the reason? I feel the performance may not be fast, as it involves String and Vec allocations and string comparisons in the loop. |
|
The implementation was changed in response to feedback and I forgot to change the pr description. As for performance implications, the loop is only ran on all entities that match the top element of the path which shouldn't be too many, and since most usecases will be for initializing scenes, which only happens once and usually with other elements it's probably fine. If you can find a more optimal way of searching feel free to open a pr after this one. |
Objective
Fixes: #16201
Originally based on and closes: #20848
Solution
Add two to methods to world
Original methods were taken from #20848, but changed to focus on bottom up search instead of a top down search, which should hopefully be more efficient and made them generic over the relationship.
Also added an
EntityPathvariant toEntityTemplate.Testing
Tests included and working.
Showcase