Alternative name component used for faster comparisons #1109
Conversation
|
A candidate for |
|
This could be the default way of naming entites, thats why I put it on bevy_core |
| #[reflect(Component)] | ||
| pub struct Name { | ||
| hash: u64, // TODO: Shouldn't be serialized | ||
| name: String, |
There was a problem hiding this comment.
Could this be std::borrow::Cow<'static, str>, or could this whole struct be more general then use:
type Name = FastCompare<String>;
There was a problem hiding this comment.
Im cool with making this less general for now to cut down on the amount of abstraction. If we find multiple use cases later and it seems worth it, we can always factor this out.
|
Can you think of a reason not to just extend the existing |
|
No other that a name must be one per entity; We could so something similar to |
|
|
||
| impl Hash for Name { | ||
| fn hash<H: Hasher>(&self, state: &mut H) { | ||
| self.name.hash(state); |
There was a problem hiding this comment.
It will be correct to use self.hash.hash(state)?
There was a problem hiding this comment.
I think we could probably get away with that. Its effectively double-hashing, so our collision risk almost certainly goes up.
But short of implementing a custom hasher for every Name use case, im not sure we have a better option.
There was a problem hiding this comment.
Eh actually I'd rather be safe now and optimize later if we prove its not too risky.
|
I do think this is probably a good candidate for a bevy_extra crate. I'm not yet convinced we need "canonical names" built in to the engine by default. "selecting by string name" isn't a general pattern we should be encouraging (we should encourage marker components), and the cases where it makes sense are very context specific (and therefore should probably live in game code). Even the Labels system we currently have should probably live in bevy_extra (at least for now). I added them for some debugging scenarios and because Godot has a similar feature, but i think the Labels system will only really start getting useful in the context of a visual scene editor. I think its time to get bevy_extra rolling. It would be a separate repo (to draw hard lines between the projects and denote a different level of support). |
|
to add context on why
|
|
Yeah thats fair. And I guess canonical names are good for visual scene organization (ex the identifier used when rendering entity hierarchies in the editor). In general I have a reflex to not add something like this until we have a concrete scenario that uses it. But I guess it probably makes sense for GLTF-loaded entities to use the GLTF node name as the "canonical name displayed in the bevy editor hierarchy viewer". Ok I think im convinced that we should have this in-engine. |
Nameis a better alternative to name entities instead of usingOption<String>, because it allows for faster searches. I'm currently using this component for find entities in deep nested hierarchies in my animation system;It works by also storing a hash along side name, then when peforming a comparison between two
Names it first check if the hashes matches, if don't the strings are different and the acctual string comparision is skipped.I'm using ahash because it's the fastest hasing algo that I know of;